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

[Armin Wolf <W_Armin@gmx.de>]

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.

Thanks,
Armin Wolf

>
> 1) +Kconfig dep, which will also avoid regressions by wrong
> Kconfig
>
>> Patch 2 declines to bind when the IdeaPad VPC device (VPC2004) is
>> present, so machines handled by ideapad-laptop keep it; patch 3 is the
>> mirror on the ideapad-laptop side. The new MAINTAINERS entry uses
>> platform-driver-x86, the list that already covers drivers/platform/arm64,
>> the ACPI-WMI core and the other Lenovo WMI drivers.
>>
>>
>> David Cemin (3):
>>    platform: lenovo: move the architecture independent WMI drivers out of
>>      x86
>>    platform: lenovo: add WMI hotkeys driver
>>    platform/x86: ideapad-laptop: do not bind the WMI event devices
>>      without the VPC
>>
>>   MAINTAINERS                                   |   9 +-
>>   drivers/platform/Kconfig                      |   2 +
>>   drivers/platform/Makefile                     |   1 +
>>   drivers/platform/lenovo/Kconfig               | 101 ++++++++++++
>>   drivers/platform/lenovo/Makefile              |  23 +++
>>   .../platform/{x86 => }/lenovo/wmi-camera.c    |   0
>>   .../platform/{x86 => }/lenovo/wmi-capdata.c   |   0
>>   .../platform/{x86 => }/lenovo/wmi-capdata.h   |   0
>>   .../platform/{x86 => }/lenovo/wmi-events.c    |   0
>>   .../platform/{x86 => }/lenovo/wmi-events.h    |   0
>>   .../platform/{x86 => }/lenovo/wmi-gamezone.c  |   0
>>   .../platform/{x86 => }/lenovo/wmi-helpers.c   |   0
>>   .../platform/{x86 => }/lenovo/wmi-helpers.h   |   0
>>   .../{x86 => }/lenovo/wmi-hotkey-utilities.c   |   0
>>   drivers/platform/lenovo/wmi-hotkeys.c         | 151 ++++++++++++++++++
>>   drivers/platform/{x86 => }/lenovo/wmi-other.c |   0
>>   drivers/platform/x86/lenovo/Kconfig           |  67 --------
>>   drivers/platform/x86/lenovo/Makefile          |   7 -
>>   drivers/platform/x86/lenovo/ideapad-laptop.c  |   8 +
>>   19 files changed, 294 insertions(+), 75 deletions(-)
>>   create mode 100644 drivers/platform/lenovo/Kconfig
>>   create mode 100644 drivers/platform/lenovo/Makefile
>>   rename drivers/platform/{x86 => }/lenovo/wmi-camera.c (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-capdata.c (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-capdata.h (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-events.c (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-events.h (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-gamezone.c (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-helpers.c (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-helpers.h (100%)
>>   rename drivers/platform/{x86 => }/lenovo/wmi-hotkey-utilities.c (100%)
>>   create mode 100644 drivers/platform/lenovo/wmi-hotkeys.c
>>   rename drivers/platform/{x86 => }/lenovo/wmi-other.c (100%)
>>
>>
>> base-commit: 2800868f63227c8237d70c41e70d441c919620d6