Re: [PATCH RFC 2/2] HID: lenovo-legion: Add keyboard lighting driver for Legion Gen 10

[Ciro Pedrini]

Hi Ilpo,

On Tue, 10 Nov 2026, Ilpo Järvinen wrote:
> Thanks for the patch.

Thanks a lot for the thorough review, and sorry for all the noise in v1.

I sent v2:
https://lore.kernel.org/r/20261110-legion-lighting-rfc-v2-0-845726afb71c@xxxxxxxxx

Rong asked to keep only the keyboard controller, so all the LampArray
code is gone, including the pair lock and the byte arrays for those
reports. The rest of your comments are done (struct with __le16,
guard(), devm_mutex_init(), no magic numbers, includes). Some answers
below.

> Please don't include how things were prepared into changelog, but you can
> use the area below the first --- so it won't end up into the final commit
> message if you patch is applied (our tools will automatically cut the
> part below ---).

Fixed, it's below the --- now. Sorry for the rookie mistake.

> > + spinlock_t event_lock; /* protects ready */
>
> Why does bool need a spinlock? That sounds like overkill if it's all it
> protects.

[...]

> The racing reader side might have already reading ready and unlocked. So
> what does this lock actually protect for real?

You're right, nothing. I checked hid-core: it holds driver_input_lock
during probe() and takes it before remove(), and raw_event is only
called when that lock can be taken. So raw_event can't run before
probe() is done or after remove() starts, and ready was always true. I
removed both.

> Can't the work be managed with devm_ ?

I removed this too. It was only there to call
led_classdev_notify_brightness_hw_changed() outside raw_event, but
that's fine to call from raw_event (hid-lg-g15 does it).

I kept the LED itself without devm on purpose though: devm would
unregister it after remove(), so after hid_hw_stop(), and in between
someone could still write to brightness.

Thanks,
Ciro