Received-SPF: pass (google.com: domain of linux-kernel+bounces-107546-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <a2c2ef97-3830-4277-8560-b97cfb8eb78e@gmx.de>
Date: Tue, 19 Mar 2024 14:00:06 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC 0/1] hwmon: (hp-wmi-sensors) Support autoloading
To: James Seo <james@equiv.tech>
Cc: jdelvare@suse.com, linux@roeck-us.net, linux-hwmon@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240318215732.322798-1-W_Armin@gmx.de>
 <Zfkm71dmnRsdmYJz@equiv.tech>
Content-Language: en-US
From: Armin Wolf <W_Armin@gmx.de>
In-Reply-To: <Zfkm71dmnRsdmYJz@equiv.tech>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
UI-OutboundReport: notjunk:1;M01:P0:mXACdlmHwYA=;fwGvU2m/7SqJuxtXPb/0Gzv84yz
 ubycw5gUJAGAmWxK3UTYnZWwoSvJgpxdXdgBgMPT4uXO470+TEwevjoFvmtWQ3yieNIMAallx
 h0wp8ZFq/wk/rLFrEgTmZ7fkLCxoq4mF3glW6CkD12rqedXzJayG1/t8AwEoTKIUNScN5cEZp
 hT+RuwJ1tXg/fIpqcUvH/Gp4AjY8uKX7eODeWjeGrB3FhkwEM3FwyK8BBxRd0vk1qU1uCxJU9
 L4SfM0f8/1dFcJisAO0PpGOGNjFXvVApSXBwx9ccMBu3yrrn4R1ZzniKd0pXy1bUrQltC37cc
 YlPpEymLXCCxAlWnxhc7aL3+f0gi/h6OZS/WYxoCTagTigbttFUFptm2u0kXAU6RbHV/vtfzn
 dna1FXveXCho0jeLxrzkQDTBilDYBYRI0lxtH2yeAPgjciDUf3PdLXbwyJmQdYpJrGabXwV1w
 tsD1qaVI+Ibe+rp/TPKW7I3Z2d2rOblWJRDY8jymvS6c1DPLC0kKnz1pomMTC4ZfB7VyXG7FH
 C+IP17oVrukFZ9VSY+q4q/Il7xKVaP5+4/H4movOtHAPtqGFYUPNnUv+bAcjwNrCLR+85R9Wn
 KAsw7s0vUPHfhzSPTruYii4defKyzt7NmoM/SaRkxmdI4ivlO3/uIX7f1R+yssRWcg6hwZTZt
 9tDCpUn6oqI6PWE/P2Ft3i6C1RPRmOFmhVjDTuO+mP9D2JvkbwTeymirkfahXfuPR1cH+yOTq
 YQUJlLkSLCLuyUSPdy+PjklgykzfUqTCLLtUeG4GcGYKym1Xj1dHVyLxXTjelIUx3bx4LtdxE
 DhKe4QML86UGSXV9In/HG7FquIsSCaPdNCsq2Mapk39Uw=

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/
>