Re: [RFC PATCH 0/3] platform: lenovo: architecture independent home for the Lenovo WMI drivers

[Hans de Goede <hansg@kernel.org>]

Hi,

On 15-Nov-26 17:02, Armin Wolf wrote:
> Am 15.11.26 um 16:33 schrieb Hans de Goede:
> 
>> <sorry for any double-send (first reply incomplete) Thunderbird has a new Ctrl+s send
>>   shortcut which I sometimes fat-finger when copy/pasting>
>>
>> Hi,
>>
>> On 13-Nov-26 23:27, David Cemin wrote:
>>> Lenovo now ships ACPI based arm64 laptops whose firmware exposes the
>>> same ACPI-WMI interfaces as the x86 IdeaPad and Legion machines: the Fn
>>> key event GUID consumed by ideapad-laptop, the hotkey utility GUID used
>>> for the microphone mute LED, and the GameZone, Other Mode and capability
>>> data GUIDs behind the thermal operation modes. Since 884d53763d30
>>> ("platform/wmi: Remove dependency on CONFIG_X86") the ACPI-WMI core
>>> builds on arm64, but the Lenovo WMI drivers still live under
>>> drivers/platform/x86, which is gated by X86_PLATFORM_DEVICES, so none of
>>> that code can be used where the firmware is.
>>>
>>> This series:
>>>    1. moves the architecture independent Lenovo WMI drivers into a new
>>>       drivers/platform/lenovo directory, following drivers/platform/surface
>>>       which exists for the same reason (Surface ships x86 and arm64);
>>>       ideapad-laptop, thinkpad_acpi, think-lmi and the Yoga tablet drivers
>>>       stay under x86 because they need i8042, port I/O or x86 only devices;
>>>    2. adds a small WMI hotkeys driver for the Fn key event GUID on machines>      where ideapad-laptop cannot bind;
>>>    3. makes ideapad-laptop's own WMI event driver decline the GUIDs when
>>>       the IdeaPad VPC device is absent, since it can never report events
>>>       there.
>>>
>>> Patch 2 was tested on a Lenovo Yoga 9 2-in-1 16 (arm64): microphone
>>> mute, airplane mode, the Smart Key, the Smart Connect key and the Fn+Q
>>> operation mode key all report input events. The same key set is dead on
>>> a Lenovo Yoga Pro 9 (arm64) today and uses the same GUID.
>>> Patches 1 and 3 are compile tested on x86_64 and arm64 (defconfig plus
>>> all LENOVO_WMI options as modules).
>>>
>>> Two questions I would like the maintainers' view on before this goes
>>> further:
>>>
>>>    a. Is a vendor directory (drivers/platform/lenovo, like surface) the
>>>       right home, or should the architecture independent Lenovo WMI
>>>       drivers move under drivers/platform/wmi next to the core instead?
>> I think that drivers/platform/lenovo makes sense since eventually we may
>> want more then just WMI drivers, e.g. the thinkpad_acpi driver which
>> actually sort of works on Qualcomm Snapdragon laptops when booting these
>> with my experimental DT-ACPI hybrid mode patches:
>>
>> https://github.com/jwrdegoede/linux-sunxi/commits/acpi-dt-hybrid-v0.4/
> 
> Agreed, the drivers/platform/wmi directory should only contain core WMI
> infrastructure code and maybe some generic drivers like wmi-bmof.
> 
>>>    b. Should the Fn key event handling stay a separate small driver, or
>>>       be factored out of ideapad-laptop so x86 and arm64 share one path?
>>>       ideapad-laptop handles the event only when its VPC platform driver
>>>       is bound, and the arm64 machines expose none of the VPC interface
>>>       it is built on (no _CFG, VPCR/VPCW, DYTC or HALS/SALS), so making
>>>       it the single driver means gating most of its probe. Two smaller
>>>       options: lift ideapad-laptop's own Fn key wmi_driver and keymap
>>>       into this file and let ideapad-laptop register for the events it
>>>       still wants (Fn lock LED, DYTC cycle), or the hid-asus/asus-wmi
>>>       shape where a companion WMI module exports the event to a
>>>       subsystem driver. The keymap in patch 2 duplicates entries of
>>>       ideapad-laptop's table on purpose until that is decided.
>> IIRC the ideapad-laptop driver is a bit of a mess atm, so factoring
>> a common handling for the the Fn key events out of this and sharing
>> that sounds like a nice cleanup.
>>
>> I see that patch 2/3 uses the new wmi-driver/-bus infra which is
>> nice and clean.
>>
>> So maybe just keep your existing stand-alone driver for both x86_64 +
>> arm64 and drop the WMI event handling from ideapad-laptop.
>>
>> I guess this may hit some problems where WMI event handlig is used
>> in ideapad-laptops integrating with some VPC functionality.
>>
>> For that purpose I think that the new
>> drivers/platform/lenovo/wmi-hotkeys.c could maybe expose some
>> notifier and call that for events it does not handle itself
>> and then ideapad-laptop can depend on some function to register
>> a listener on that notifier (1) and register a notifier handler
>> staying out of WMI handling itself.
>>
>> No idea if this is feasible / how clean the end result will be,
>> just brainstorming and saying yes to doing some refactoring of
>> the somewhat messy ideapad-laptop code.
>>
>> Note that https://github.com/systemd/systemd/blob/main/hwdb.d/60-keyboard.hwdb
>> has some mappings for "Ideapad extra buttons" input devices
>> (+ DMI match), so if any of the buttons remapped there end up
>> under the new WMI driver then that should probably give its
>> input device the same name, to make sure those mappings still
>> apply.
>>
>> Regards,
>>
>> Hans
> 
> I like the idea of using a notifier to allow ideapad-laptop to consume WMI
> events from wmi-hotkeys. This would even allow ideapad-laptop to override the
> sparse keymap inside wmi-hotkeys by simply returning NOTIFY_STOP when "catching"
> an WMI event that has to be handled internally (like event 0x3D (Performance Key)).
> wmi-hotkeys can check for this return value and not emit a key code in such a case.

Ah yes, I was thinking about only calling the notifier after not finding
an entry in the sparse keymap, but calling it before allowing notifier
handlers to return NOTIFY_STOP and filter out further processing is
a good idea.

Regards,

Hans