Re: [PATCH v4 1/3] hwmon: pmbus: event notification with alarms

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the PMBus core notifies the wrong kobject when hardware monitoring events occur, functionally breaking IRQ-based notifications.
--

--- Patch [1]: [PATCH v4 1/3] hwmon: pmbus: event notification with alarms ---
commit eeca7812901a45049c95eb874da22e86da04ee7e
Author: Vincent Jardin <[email protected]>

hwmon: pmbus: event notification with alarms

Let's add an accessor for the hwmon class device the pmbus core
registered for an i2c_client. Export it with PMBUS so chip
drivers can call hwmon_notify_event() on per-sensor *_alarm
attributes from the driver's work items.

The needs: for boards when the chip's SMBALERT# pin is
not available for the CPU, pmbus_irq_setup() cannot deliver
poll(POLLPRI) wakes or udev change@... events on the inX_alarm
files. So the drivers can install a delayed_work-based polling
fallback (like the lm90 driver) that periodically reads STATUS_WORD
and calls hwmon_notify_event() on 0->1 transitions.

> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 3143b9e0316c4..6ba00ad732973 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -695,6 +695,14 @@ const struct pmbus_driver_info *pmbus_get_driver_info(struct i2c_client *client)
>  }
>  EXPORT_SYMBOL_NS_GPL(pmbus_get_driver_info, "PMBUS");
>  
> +struct device *pmbus_get_hwmon_device(struct i2c_client *client)
> +{
> +	struct pmbus_data *data = i2c_get_clientdata(client);
> +
> +	return data->hwmon_dev;
> +}
> +EXPORT_SYMBOL_NS_GPL(pmbus_get_hwmon_device, "PMBUS");

[Severity: High]
This isn't a bug introduced by this patch, but the PMBus core notifies the
wrong kobject when hardware monitoring events occur.

While this patch adds a way for polling drivers to notify the hwmon device
properly, does the standard interrupt path silently fail to notify userspace?

When an interrupt is triggered, the event propagates through
pmbus_fault_handler() -> _pmbus_get_flags() -> pmbus_notify().

Looking at pmbus_notify() in drivers/hwmon/pmbus/pmbus_core.c, it appears to
notify the I2C parent device's kobject rather than the hwmon device:

drivers/hwmon/pmbus/pmbus_core.c:pmbus_notify() {
    ...
		if (reg == sreg && page == spage && (smask & flags)) {
			dev_dbg(data->dev, "sysfs notify: %s", da->attr.name);
			sysfs_notify(&data->dev->kobj, NULL, da->attr.name);
			kobject_uevent(&data->dev->kobj, KOBJ_CHANGE);
			flags &= ~smask;
		}
    ...
}

Since the sysfs attributes are registered on the hwmon class device
(data->hwmon_dev), doesn't calling sysfs_notify() and kobject_uevent() on
data->dev->kobj prevent userspace applications from receiving POLLPRI or udev
events when an interrupt triggers an alarm?

Could this be fixed to use data->hwmon_dev instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1