Re: [RFC PATCH] hwmon: (acpi_power_meter): Convert to hwmon_device_register_with_info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/11/22 07:37, Zhang Rui wrote:
Hi, Guenter,

On Wed, 2022-05-11 at 06:12 -0700, Guenter Roeck wrote:
On 5/11/22 00:54, Zhang Rui wrote:
The acpi_power_meter driver doesn't create any standard hwmon sysfs
attributes under its hwmon device node, but instead, the driver has
its
own code to create the hwmon style sysfs attributes in the ACPI
device
node of the ACPI Power Meter device.
I'm not clear why it was designed in that way.

In order to elimite
[   79.960333] power_meter ACPI000D:00: hwmon_device_register() is
deprecated. Please convert the driver to use
hwmon_device_register_with_info().
convert the driver to use the new API, no chip_info or sysfs_groups
parameter needed.

The only difference brought by this patch is that the "name"
attribute
will be created under the hwmon device node. Not sure if this
matters or
not.

Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>

No, this is not a conversion and not acceptable. Corentin Labbe is
working on the real thing. See

https://patchwork.kernel.org/project/linux-hwmon/patch/20220509063010.3878134-3-clabbe@xxxxxxxxxxxx/


Thanks for the pointer. And this was my original intension about how to
do the conversion.

But then I realized that, just like I described in the changelog,
the original sysfs attributes in this driver, although they're hwmon
style, but they are actually located under the ACPI device node.
And the patch above will move them to the hwmon device node, right?

With any patch, this is what I got under the hwmon device node
# ls /sys/class/hwmon/hwmon0/
device  power  subsystem  uevent

and this is what I got under the ACPI device node
# ls /sys/class/hwmon/hwmon0/device/
driver  hid  hwmon  measures  modalias  name  path  physical_node  powe
r  power1_model_number  power1_oem_info  power1_serial_number  status
subsystem  uevent  uid


That is part of the conversion to any of the new APIs, and intentional.
If that is unacceptable for some reason, you'll have to stick with the old
API and accept the warning. Abusing the new APIs is not acceptable.

Plus, in that patch, I don't see how to handle the power meter
capabilities change, i.e. METER_NOTIFY_CONFIG event, in
acpi_power_meter_notify().
According to the previous logic, we may need to remove/add different
attributes based on the new capabilities.

In section 10.4.1 of ACPI Spec 6.4, it says that the _PMC information
"remains constant unless either the power meter's firmware or the BMC
hardware changes, at which time the platform is required to send
Notify(power_meter, 0x80) for the OSPM to re-evaluate _PMC"

If this could happen in real life, we cannot rely on a fixed
hwmon_chip_info and attribute_groups at driver registration phase.


You have a point. However, if anything, that means that we might have
to improve/extend the hwmon API to make attributes visible or invisible
dynamically (ie add an API call such as hwmon_update_visibility()
and have it call sysfs_update_groups()).

Thanks,
Guenter



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux