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

[Rong Zhang <i@rong.moe>]

Hi Ciro,

On Tue, 2026-11-10 at 17:34 -0300, Ciro Pedrini wrote:
> Hi Rong,
> 
> On Tue, 2026-11-10 at 22:56 +0800, Rong Zhang wrote:
> > Thanks for working on this.
> 
> Thanks for looking at it (and for the clear direction, it helped a lot).
> 
> [...]
> 
> > 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.)
> 
> Makes sense. v2 only does c197, and it doesn't depend on any LampArray
> work:
> https://lore.kernel.org/r/20261110-legion-lighting-rfc-v2-0-845726afb71c@gmail.com
> 
> About your question: yes, the firmware does all of that by itself.
> Fn+Up/Down change the brightness (in steps of 3, and it's stored) and
> the controller sends the new level on a vendor input report. The driver
> only exposes the brightness as kbd_backlight and forwards the hotkey
> changes as brightness_hw_changed. Fn+Space switches between the stored
> effects (6 states counting off). The controller can also read and set
> the active effect, but I left that out of v2.
> 
> > I am also working on a independent hid-lamparray implementation that is
> > far more generic than Aaron and Werner's series.
> 
> Something you may want to know for your driver: on this laptop c193
> doesn't show anything on its own. c197 has to hand the LEDs over first,
> with its vendor command 0xAA (07 AA 02 00 00 00 gives them to the
> LampArray, 07 AA 02 00 01 00 gives them back). The controller remembers
> it after power-off, but it only matters while the LampArray is in host
> mode. In autonomous mode you see the controller's effects anyway. So a
> generic driver on c193 alone would register lamps that do nothing until
> c197 is switched.

What if (four permutations):

- c197 sends 07 AA 02 00 00/01 00
- c193 sends autonomous_mode=on/off

And what if the sequence is reversed (another four permutations)?

> 
> v2 doesn't touch 0xAA. How would you like to handle it with your
> driver? A sysfs attribute on c197, something with your autonomous mode
> trigger, the component framework? 
> 

I expect 07 AA 02 00 01 00 acts like a hardware control trigger. It may
be able to reuse the autonomous mode trigger, but it depends on the
answer to the previous question.

In either way, the handover is controlled by the lamparray LED classdev's
hardware control trigger.

My implementation connects the LampArray application to hid-input, so any
HID driver with hid_hw_start(HID_CONNECT_DEFAULT) or
hid_hw_start(HID_CONNECT_HIDINPUT | ...) will initialize the LampArray
application. Therefore, I suppose the only thing we need is a simple glue
driver[1] for c193. The glue driver binds to the component master[2],
pre-allocates[3] the hid_lamparray struct with some necessary
callbacks[4] registered, and call hid_hw_start() to wire things up.

In this manner, you can get rid of the one-driver-two-device nightmare.
The component master and the component can be located in different
drivers and coordinate using the component framework.

Does that make sense to you?

[1]: I will call it hid-lenovo-legion-la

[2]: hid-lenovo-legion (c197)

[3]: Usually it's allocated by hid-input when hid_hw_start() is called.
Pre-allocate it with prefilled callbacks is sufficient to provide extra
functionalities to the HID LampArray application helper

[4]: hid-lenovo-legion passes a device handle along with several
callbacks to hid-lenovo-legion-la. hid-lenovo-legion-la passes some
wrapper callbacks that calls the former's callbacks with the device
handle to hid-lamparray

> Once your series is out I'm happy to
> write that part and test it here.

I will CC you when I submit it. For now let's focus on the driver for
c197 first.

Thanks,
Rong

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