Re: [PATCH RFC 0/2] Keyboard lighting for the Lenovo Legion Pro 7 Gen 10

[Rong Zhang <i@rong.moe>]

Hi Ciro,

Thanks for working on this.

On Mon, 2026-11-09 at 22:50 +0000, Ciro Pedrini via B4 Relay wrote:
> The keyboard and chassis lighting of the Lenovo Legion Pro 7 16IAX10H
> (2025) has no Linux support today: on my unit nothing ever lit, because
> the keyboard controller's brightness was 0 from the factory and nothing on
> Linux exposes it. This series makes it a regular keyboard backlight.
> 
> The lighting is two USB HID devices:
> 
> - 048d:c197, the ITE 8258 keyboard controller: the built-in keyboard,
>   plus a vendor protocol on feature report 0x07 (960 bytes) with a
>   hardware brightness (0..9, stored), the controller's own effects
>   (stored profiles), and a switch that hands the LEDs to the LampArray
>   below (also stored). Fn+Up/Down/Space are handled by the controller,
>   which reports the result on a vendor input report.

IMHO, the support for 048d:c197 is the only thing you should include in
the series.
> 
> - 048d:c193 interface 1, a standard HID LampArray: 319 lamps (keys, front
>   strip, rear vents, lid logo) with positions and key usages. It shows
>   nothing until the controller hands the LEDs over, its colours are scaled
>   by the controller's brightness, and after resume from S3 the kernel
>   resets it and it comes back in autonomous mode.

IOW, do not add a device-specific driver when it could be handled by a
generic one, i.e., the ongoing hid-lamparray series. While you can
rewrite your series to make it work with hid-lamparray, it would be much
easier to review and accept your series when it has no dependence on a
ongoing work.

Let's take it step by step, and submit a driver for 048d:c197 first. As
far as I understood, the driver should be able to handle "hardware
brightness (0..9, stored), the controller's own effects (stored
profiles)," and "Fn+Up/Down/Space" by its own. (Please correct me if I
misunderstood your words.)

I am also working on a independent hid-lamparray implementation that is
far more generic than Aaron and Werner's series. It still needs some
polishing and depends on another series of mime [1]. I've mentioned some
details of my implementation there.

[1]: https://msgid.link/20261110-hid-haptic-kmod-v2-0-b835a067cb62@rong.moe

> 
> Patch 1 makes ideapad-laptop ignore the EC's copies of those three
> hotkeys, which reach userspace as KEY_UNKNOWN today.
> 
> Patch 2 adds hid-lenovo-legion, binding both devices: one multicolor LED
> (<name>:rgb:kbd_backlight) whose brightness is the controller's, with
> brightness_hw_changed from the hotkeys, and whose colour goes to every
> lamp. lighting_mode selects "firmware" (default: the controller's effects;
> the driver leaves the LEDs alone, so a userspace tool can drive the
> LampArray over hidraw) or "color". Per-key control stays in userspace.
> 
> This is an RFC because of its shape:
> 

I am not going to review PATCH 2, unless your next revision no longer
adds a device-specific driver for 048d:c193.

>  one driver spanning two USB devices,
> paired by their USB bus. 
> 

But I did have a glance at PATCH 2, things could easily go wrong when you
invent your own mechanism to bind things. And it's not scalable anyway.

Please use the component framework (not in your next revision, it should
only add support for 048d:c197).

> Is that acceptable, or would you rather have it
> differently, e.g. a vendor hook in the generic LampArray helper that Aaron
> and Werner are working on ("HID: generic: add LampArray support", v7),
> which on this machine would register a kbd_backlight that does nothing
> visible? The two patches touch two subsystems; I'm happy to split them.
> 
> Tested on the 16IAX10H (83F5, BIOS Q7CN77WW, Spanish ISO keyboard) with
> 7.2, as modules: brightness from sysfs and KDE, the hotkeys' OSD, colours
> on every zone, suspend/resume in both modes, a userspace LampArray client
> in "firmware" mode, unbinding back to hid-generic. The series is based on
> v7.4-rc2 and builds there with W=1.
> 
> I worked out the protocol from the device itself (usbmon captures, the
> LampArray's attributes) and from analysing the vendor's Windows software
> for interoperability. The patches were prepared with Claude Code (Claude
> Opus 5.5), see the Assisted-by tags; I reviewed and tested them.
> checkpatch reports those tags (non-standard signature, no email address),
> and a MAINTAINERS hint for the new files that patch 2 already adds.
> 
> To: Mark Pearson <mpearson-lenovo@squebb.ca>
> To: Derek J. Clark <derekjohn.clark@gmail.com>
> To: Rong Zhang <i@rong.moe>
> To: Ike Panhc <ikepanhc@gmail.com>
> To: Hans de Goede <hansg@kernel.org>
> To: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> To: Jiri Kosina <jikos@kernel.org>
> To: Benjamin Tissoires <bentiss@kernel.org>
> Cc: platform-driver-x86@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-input@vger.kernel.org
> 
> ---
> Ciro Pedrini (2):
>       platform/x86: ideapad-laptop: Ignore keyboard backlight hotkeys
>       HID: lenovo-legion: Add keyboard lighting driver for Legion Gen 10
> 
>  .../testing/sysfs-class-led-driver-lenovo-legion   |  20 +
>  MAINTAINERS                                        |   2 +
>  drivers/hid/Kconfig                                |  13 +
>  drivers/hid/Makefile                               |   1 +
>  drivers/hid/hid-ids.h                              |   2 +
>  drivers/hid/hid-lenovo-legion.c                    | 662 +++++++++++++++++++++
>  drivers/platform/x86/lenovo/ideapad-laptop.c       |   4 +
>  7 files changed, 704 insertions(+)
> ---
> base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
> change-id: 20261109-legion-lighting-rfc-dc32165df52d
> 
> Best regards,