On Tue, Mar 19, 2024 at 02:00:06PM +0100, Armin Wolf wrote: > 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 investigated further. Every HP machine in the Linux Hardware Database that has \\.\root\WMI\hpqBEvnt at 95F24279-4D7B-4334-9387-ACCDC67EF61C also has \\.\root\WMI\HPBIOS_BIOSEvent at 2B814318-4BE8-4707-9D84-A190A859B5D0. > 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. No objections from me for this specific use case to work around the GUID conflict. hp-wmi-sensors should indeed subscribe on 2B814318-4BE8-4707-9D84-A190A859B5D0 for some of those machines. Any ideas for getting rid of wmi_query_block() for fetching \\.\root\HP\InstrumentedBIOS\HPBIOS_PlatformEvents? I know other drivers are also using it for getting blocks other than their "main" GUID. > 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. Happy to test. (Also happy to try it myself, but I'd need some help.) > Thanks, > Armin Wolf > >> >> [1] https://lore.kernel.org/linux-hwmon/cd81a7d6-4b81-f074-1f28-6d1b5300b937@xxxxxx/ >>
