Re: [RFC] hwmon: (acpi_power_meter) Replace hwmon_device_register

["lihuisong (C)" <lihuisong@huawei.com>]

Hi Guenter,

在 2025/2/25 21:01, Guenter Roeck 写道:
> On 2/25/25 00:51, Huisong Li wrote:
>> When load this mode, we can see the following log:
>> "power_meter ACPI000D:00: hwmon_device_register() is deprecated. Please
>>   convert the driver to use hwmon_device_register_with_info()."
>>
>> So replace hwmon_device_register with hwmon_device_register_with_info.
>>
>> To avoid any changes in the display of some sysfs interfaces, some of
>> necessary changes in hwmon.c must be made:
>> 1> For 'power1_average_interval_max/min' interface, insert 'average' 
>> to the
>>     string corresponding to hwmon_power_average_interval_max/max in
>>     hwmon_power_attr_templates[]. I guess that is what's missing.
>> 2> Add some string attributes in power sensor type because of below 
>> items:
>>     a) power1_accuracy  --> display like '90.0%'
>>     b) power1_cap_hyst  --> display 'unknown' when its value is 
>> 0xFFFFFFFF
>>     c) power1_average_min/max --> display 'unknown' when its value is
>>                                   negative.
>> Note: All the attributes modified above in hwmon core are not used by 
>> other
>> drivers.
>>
>
> That is not a reason to change the ABI, much less so hiding the change
> in a driver patch.
>
>
I am trying to replace the deprecated hwmon_device_register with 
hwmon_device_register_with_info for acpi power meter driver.

To avoid any changes in the display of some sysfs interfaces, there are 
two modifications in hwmon core as follows:
(1) The first modification in hwmon is as follows:
-->
@@ -646,8 +653,8 @@ static const char * const 
hwmon_power_attr_templates[] = {
      [hwmon_power_enable] = "power%d_enable",
      [hwmon_power_average] = "power%d_average",
      [hwmon_power_average_interval] = "power%d_average_interval",
-    [hwmon_power_average_interval_max] = "power%d_interval_max",
-    [hwmon_power_average_interval_min] = "power%d_interval_min",
+    [hwmon_power_average_interval_max] = "power%d_average_interval_max",
+    [hwmon_power_average_interval_min] = "power%d_average_interval_min",
      [hwmon_power_average_highest] = "power%d_average_highest",

The string names, "power%d_interval_max/min", are missing 'average'.
I think the meaning of these attributes are unclear If no this word. It 
can be regarded as a fault.
And power attribute name in acpi power meter is 
"power1_average_interval_min/max".

(2)The second modification changes the attribute of 'power_accuracy', 
'power_cap_hyst', 'power_average_min' and 'power_average_max' from data 
to string.
It is appropriate to assign 'power_accuracy' to string attribute.
Because it can be displayed as '%' and also include decimal point like 
acpi power meter driver, which is more in line with the meaning of this 
attribute.
It might be better to keep other attributes as data types. But it breaks 
the cornor display of these attributes in acpi power meter driver as 
said below.
    a) power1_cap_hyst  --> display 'unknown' when its value is 0xFFFFFFFF
    b) power1_average_min/max --> display 'unknown' when its value is 
negative.

I want to say that all the attributes modified above in hwmon core are 
not used by other drivers, so don't break ABI of some driver.
These can't be solved in this driver side.

AFAICS, acpi power meter driver can't replace the deprecated API because 
their sysfs interfaces will be broken if there's no any modification in 
hwmon core.


/Huisong
>
> .