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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f7274b8d-cb91-4975-be94-69f041e9c4ee@roeck-us.net>
Date: Mon, 25 Nov 2024 08:13:16 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Huisong Li <lihuisong@...wei.com>, linux-hwmon@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: jdelvare@...e.com, liuyonglong@...wei.com, zhanjie9@...ilicon.com,
 zhenglifeng1@...wei.com
Subject: Re: [PATCH v1 4/4] hwmon: (acpi_power_meter) Add the print of no
 notification that hardware limit is enforced

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@...wei.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