Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

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

 



Am 19.03.24 um 06:47 schrieb James Seo:

On Mon, Mar 18, 2024 at 10:57:31PM +0100, Armin Wolf wrote:
Currently, the hp-wmi-sensors driver needs to be loaded manually
on supported machines. This however is unnecessary since the WMI
id table can be used to support autoloading.

However the driver might conflict with the hp-wmi driver since both
seem to use the same WMI GUID for registering notify handler.

I am thus submitting this patch as an RFC for now.

Armin Wolf (1):
   hwmon: (hp-wmi-sensors) Add missing MODULE_DEVICE_TABLE()

  drivers/hwmon/hp-wmi-sensors.c | 2 ++
  1 file changed, 2 insertions(+)

--
2.39.2

Autoloading was deliberately left out for now because of the GUID
conflict with hp-wmi's WMI notify handler.

HP's GUID reuse across product lines for different types of WMI
objects with different names and shapes means that with a patch like
this, many systems that should only load hp-wmi-sensors but not
hp-wmi will try to autoload both. (Perhaps all of them; I want to say
that the GUID 5FB7F034-2C63-45e9-BE91-3D44E2C707E4, which is the
second of the two GUIDs that hp-wmi uses to autoload, exists on every
HP system I've examined.)

Meanwhile, hp-wmi does various other platform things, and there's so
much hardware out there that who knows, maybe there are some systems
that really should load both. I don't think so but I can't rule it
out.

Unlike hp-wmi-sensors, hp-wmi doesn't survive failure to install its
notify handler, which sets up a potential race condition depending on
when hp-wmi and hp-wmi-sensors loads on a given system.

Therefore, I intended to add autoloading at the same time as
converting hp-wmi-sensors to use the bus-based WMI interface once
aggregate WMI devices are better supported.

As you mentioned [1], I ran into issues when I tried to do the
conversion by simply adding the GUID to struct wmi_driver.id_table.
That resulted in two separate independent instances of hp_wmi_sensors
being loaded, which isn't what I wanted.

After taking a look at a ACPI table dump of a HP machine, i noticed that
HPBIOS_BIOSEvent has the GUID 2B814318-4BE8-4707-9D84-A190A859B5D0, which is
different than the event GUID used by hp-wmi.

According your comment in hp_wmi_notify(), i assume that some machines have
mixed-up event GUIDs.

I thing it would be best to create a separate WMI driver for the event and
use a notifier chain (see include/linux/notifier.h) to distribute the event data.

In case of event GUID 95F24279-4D7B-4334-9387-ACCDC67EF61C, both hp-wmi and
hp-wmi-sensors can subscribe on this notifier and receive event data without
stepping on each other's toes.

The same can be done for the event GUID 2B814318-4BE8-4707-9D84-A190A859B5D0,
with a separate notifier chain.

This would decouple the event handling from the event data consumers, allowing
both hp-wmi and hp-wmi-sensors to coexist.

I can provide a prototype implementation, but unfortunately i have no HP machine
myself for testing. But i might be able to find one to test my changes.

Thanks,
Armin Wolf


[1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@xxxxxx/





[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