On 11/25/24 19:15, lihuisong (C) wrote:
在 2024/11/26 0:13, Guenter Roeck 写道:
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@xxxxxxxxxx>
---
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.
Indeed.
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 ‘&’
What compilation parameters did you use to intercept this?😁
Nothing special.
make allmodconfig; make drivers/hwmon/acpi_power_meter.o
Guenter
+ 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.
From the current implement, the value of this patch is little. It's just telling the user that he won't be notified. Notifications are not available.
Actually, I'd like to add some necessary updates in the notification handler when OSPM receive some notifications, like 0x82, 0x83 event.
These updates are necessary for this driver, which more follow ACPI spec.
But I don't know how do handle the notify 0x81 to fix the trip points, so I don't modify it yet.
Overall, really, this driver could benefit from a complete overhaul.
Its use of the deprecated hwmon_device_register() should tell it all.
Yes, I also found it.
But I don't know how to handle struct hwmon_chip_info and if it is appropriate to this driver yet.
It will be a big modification if it is ok.
There is lots of questionable code, such as the unprotected calls to
remove_attrs() followed by setup_attrs() in the notification handler.
Agreed.
In addition, using struct sensor_template to create sysfs interface is hard to maintain and not good to me.
The show_val and show_str are to display based on the index in struct sensor_template.
Any updates should be limited to bug fixes and not try to make minor
improvements for little if any gain.
Yes
.
