Message-ID: <a2c2ef97-3830-4277-8560-b97cfb8eb78e@gmx.de>
Date: Tue, 19 Mar 2024 14:00:06 +0100
From: Armin Wolf <W_Armin@....de>
To: James Seo <james@...iv.tech>
Cc: jdelvare@...e.com, linux@...ck-us.net, linux-hwmon@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading

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@gmx.de/
>

Powered by blists - more mailing lists