search

zeule / asus-wmi-ec-sensors

Linux HWMON sensors driver for ASUS motherboards to get sensor readings from the embedded controller
7 stars 2 forks source link

Provide modinfo for automatic module loading #3

Closed zeule closed 2 years ago

zeule commented 2 years ago

This somewhat refactors the board detection and module initialization code, and also removes the branch for non-platform-device structures and code.

t-8ch commented 2 years ago

Wouldn't it be more idiomatic to drop the platform device and rely on the WMI bus instead?

zeule commented 2 years ago

Wouldn't it be more idiomatic to drop the platform device and rely on the WMI bus instead?

As far as I understand, platform devices provide automatic memory cleanup. The other (pure WMI) branch was non-functional anyway.

t-8ch commented 2 years ago

The wmi bus should also provide automatic resource management when you use the devm_() APIs. When I submitted a similar driver upstream (gigabyte-wmi) I was strongly encouraged to use the WMI bus. The resulting code was also much nicer.

zeule commented 2 years ago

Thank you, I've take a look. However, this driver is already submitted to the mainline kernel by another person, and where it is being merged with asus-wmi-sensors. So, I'm really puzzled how to proceed with changes now.

I also consider to implement the same sensor reading using direct EC communication, because the WMI method is rather slow. Then I would reuse the platform device calls, I guess?

t-8ch commented 2 years ago

As Andy requested the driver would better belong to the platform-x86 subsystem instead of hwmon. When submitted to the pdx86 maintainer they will probably request the WMI bus logic.

I don't know you it would be handled when talking to the EC. How would you detect and communicate with the EC? For ACPI and PNP there are dedicated busses and I would expect those should be used if possible. (I have no experience with that though) When binding to a bus device you get the probing and lifecycle handling for free which you would have to perform manually with a platform device.

zeule commented 2 years ago

Sorry, I can't understand what's going on with that patch set. Do not understand why did Denis decide to merge this driver and asus-wmi-sensors, to begin with, as they have very little in common (interface to hwmon) and can never be active simultaneously.