在 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?😁
+ 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
.
