Re: [PATCH] hwmon: Ensure that 'dev' passed to hwmon_notify_event() is a hwmon device

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

Pre-existing issues:
- [High] The newly added WARN will be unconditionally triggered by the existing bnxt driver, causing a loud regression.
--

--- Patch [1]: [PATCH] hwmon: Ensure that 'dev' passed to hwmon_notify_event() is a hwmon device ---
commit d2961b1d473d8c128ab5a2aa551a3dc59e8bed39
Author: Guenter Roeck <[email protected]>

hwmon: Ensure that 'dev' passed to hwmon_notify_event() is a hwmon device

This patch enforces that the device passed to hwmon_notify_event() is actually
a hardware monitoring device. It introduces a warning and aborts the event
notification if an incorrect device class is provided.

> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 41755910a25a0..4110b5c45ee8d 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
[ ... ]
> @@ -782,6 +787,9 @@ int hwmon_notify_event(struct device *dev, enum hwmon_sensor_types type,
>  	const char *template;
>  	int base;
>  
> +	if (WARN(!is_hwmon_device(dev), "%s is not a hardware monitoring device\n",
> +		 dev_name(dev)))
> +		return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but will this strict check cause a
regression for the bnxt driver?

Looking at bnxt_hwmon_notify_event() in
drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c, it currently passes the
parent PCI device instead of the hwmon device:

void bnxt_hwmon_notify_event(struct bnxt *bp)
{
	u32 attr;
	...
	hwmon_notify_event(&bp->pdev->dev, hwmon_temp, attr, 0);
}

Since the device passed here is not a hwmon device, won't this new WARN
trigger unconditionally on every thermal event for bnxt hardware?

For systems configured with panic_on_warn, wouldn't this lead to a panic
during normal hardware thermal events?

>  	if (type >= ARRAY_SIZE(__templates))
>  		return -EINVAL;
>  	if (attr >= __templates_size[type])

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