Re: [PATCH v1 4/4] hwmon: (acpi_power_meter) Add the print of no notification that hardware limit is enforced

[Guenter Roeck <linux@roeck-us.net>]

On 11/25/24 01:34, Huisong Li wrote:
> As ACPI spec said, the bit3 of the supported capabilities in _PMC indicates
> that the power meter supports notifications when the hardware limit is
> enforced. If one platform doesn't report this bit, but support hardware
> forced limit through some out-of-band mechanism. Driver wouldn't receive
> the related notifications to notify the OSPM to re-read the hardware limit.
> So add the print of no notifcation that hardware limit is enforced.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>   drivers/hwmon/acpi_power_meter.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
> index 3500859ff0bf..d3f144986fae 100644
> --- a/drivers/hwmon/acpi_power_meter.c
> +++ b/drivers/hwmon/acpi_power_meter.c
> @@ -712,6 +712,10 @@ static int setup_attrs(struct acpi_power_meter_resource *resource)
>   			goto skip_unsafe_cap;
>   		}
>   
> +		if (resource->caps.flags & POWER_METER_CAN_NOTIFY == 0)

== has higher precedence than &, so this expression will never be true.

And, indeed:

drivers/hwmon/acpi_power_meter.c: In function ‘setup_attrs’:
drivers/hwmon/acpi_power_meter.c:701:42: error: suggest parentheses around comparison in operand of ‘&’

> +			dev_info(&resource->acpi_dev->dev,
> +				 "no notifcation when the hardware limit is enforced.\n");
> +
>   		if (resource->caps.configurable_cap)
>   			res = register_attrs(resource, rw_cap_attrs);
>   		else

On top of that, I don't see the value in this patch.

Overall, really, this driver could benefit from a complete overhaul.
Its use of the deprecated hwmon_device_register() should tell it all.
There is lots of questionable code, such as the unprotected calls to
remove_attrs() followed by setup_attrs() in the notification handler.
Any updates should be limited to bug fixes and not try to make minor
improvements for little if any gain.

Guenter