[PATCH 0/4] HID: lamparray: fixes from testing on Acer Predator PT14-52T

Cristian Mazzotta posted 4 patches 2 weeks, 2 days ago
drivers/hid/hid-generic.c     |  27 ++++++
drivers/hid/hid-lamparray.c   | 162 +++++++++++++++++++++++++++++-----
include/linux/hid-lamparray.h |  35 ++++++++
3 files changed, 200 insertions(+), 24 deletions(-)
[PATCH 0/4] HID: lamparray: fixes from testing on Acer Predator PT14-52T
Posted by Cristian Mazzotta 2 weeks, 2 days ago
These are four fixes on top of the v5 series, posted 2026-09-03:
https://lore.kernel.org/linux-input/20260903073602.3815258-1-aer@tuxedocomputers.com/

Tested on an Acer Predator PT14-52T, which has two LampArray devices: a
USB keyboard (05AF:767A) and an I2C ENE controller (0CF2:5130).

With v5 as posted, the keyboard does not probe. LampCount reads back 0,
and once that is fixed the level counts read back 0 as well, which
divides by zero in led_mc_calc_color_components() during probe and takes
the USB hub worker down with it. Patches 1 and 2 cover the reads and the
validation; this is the same problem Aaron and Werner discussed in this
thread, with a backtrace from hardware that hits it.

Patch 3 makes use_leds_uapi transfer control in both directions.
Autonomous mode is currently only set at probe, so writing 1 re-registers
the LED class device without taking the hardware back, and updates sent
afterwards are ignored.

Patch 4 blanks the lamps across suspend. On this machine, the lighting
accounts for 9.21W of the 12.35W s2idle power draw against 3.14W with
the lamps blanked from this driver, so this is most of the suspend power
on a laptop with RGB. This was also tested with one of the 2 lights on
the ENE controller still on; this patch does not control both zones.

Please treat these as input for v6 rather than a separate series;
squashing them in is fine by me, and I will rebase and retest against v6
when it arrives.

Two things I have not fixed:

- The ENE controller exposes two LampArrayAttributesReport collections.
  lamparray_parse_update_report() has no first-match guard, so the second
  overwrites the first and only one zone is claimed; the other stays in
  firmware control. This is the multi-zone question from Werner's reply.
  I really believe that multi-zone support should exist, but I have not
  included it because I haven't started it; uni is taking away a lot of
  my time currently.

- hid_device_io_start() in lamparray_register() may no longer be needed:
  hid_hw_raw_request() is synchronous and does not go through
  hid_input_report(), so it does not need driver_input_lock released.
  That would also address the second [High] item from the Sashiko review.

Cristian Mazzotta (4):
  HID: lamparray: read attribute reports synchronously
  HID: lamparray: raise log level of fatal probe errors
  HID: lamparray: transfer control when use_leds_uapi changes
  HID: lamparray: blank lamps across suspend and restore on resume

 drivers/hid/hid-generic.c     |  27 ++++++
 drivers/hid/hid-lamparray.c   | 162 +++++++++++++++++++++++++++++-----
 include/linux/hid-lamparray.h |  35 ++++++++
 3 files changed, 200 insertions(+), 24 deletions(-)


base-commit: 9b298109e37e5caf4b6800198c4907a5a6bf00ae
-- 
2.55.0
Re: [PATCH 0/4] HID: lamparray: fixes from testing on Acer Predator PT14-52T
Posted by Aaron Erhardt 2 weeks ago
Am 09.09.26 um 18:52 schrieb Cristian Mazzotta:
> These are four fixes on top of the v5 series, posted 2026-09-03:
> https://lore.kernel.org/linux-input/20260903073602.3815258-1-aer@tuxedocomputers.com/
> 
> Tested on an Acer Predator PT14-52T, which has two LampArray devices: a
> USB keyboard (05AF:767A) and an I2C ENE controller (0CF2:5130).
> 
> With v5 as posted, the keyboard does not probe. LampCount reads back 0,
> and once that is fixed the level counts read back 0 as well, which
> divides by zero in led_mc_calc_color_components() during probe and takes
> the USB hub worker down with it. Patches 1 and 2 cover the reads and the
> validation; this is the same problem Aaron and Werner discussed in this
> thread, with a backtrace from hardware that hits it.
> 
> Patch 3 makes use_leds_uapi transfer control in both directions.
> Autonomous mode is currently only set at probe, so writing 1 re-registers
> the LED class device without taking the hardware back, and updates sent
> afterwards are ignored.
> 
> Patch 4 blanks the lamps across suspend. On this machine, the lighting
> accounts for 9.21W of the 12.35W s2idle power draw against 3.14W with
> the lamps blanked from this driver, so this is most of the suspend power
> on a laptop with RGB. This was also tested with one of the 2 lights on
> the ENE controller still on; this patch does not control both zones.
> 
> Please treat these as input for v6 rather than a separate series;
> squashing them in is fine by me, and I will rebase and retest against v6
> when it arrives.

Thanks for your patches! I have reviewed and tested them and the only
thing I noticed is that wrapping the resume/suspend methods with pm_ptr
is missing in the final patch. Otherwise everything LGTM.

I will follow up with v6 soon, adding you with "Co-developed-by" on
the relevant commits.

Best regards
Aaron

> 
> Two things I have not fixed:
> 
> - The ENE controller exposes two LampArrayAttributesReport collections.
>   lamparray_parse_update_report() has no first-match guard, so the second
>   overwrites the first and only one zone is claimed; the other stays in
>   firmware control. This is the multi-zone question from Werner's reply.
>   I really believe that multi-zone support should exist, but I have not
>   included it because I haven't started it; uni is taking away a lot of
>   my time currently.

Ack. I think this is better suited for a separate patch series, unless it
is trivial to add.

> 
> - hid_device_io_start() in lamparray_register() may no longer be needed:
>   hid_hw_raw_request() is synchronous and does not go through
>   hid_input_report(), so it does not need driver_input_lock released.
>   That would also address the second [High] item from the Sashiko review.

Ack. I will look into this for v6.

> 
> Cristian Mazzotta (4):
>   HID: lamparray: read attribute reports synchronously
>   HID: lamparray: raise log level of fatal probe errors
>   HID: lamparray: transfer control when use_leds_uapi changes
>   HID: lamparray: blank lamps across suspend and restore on resume
> 
>  drivers/hid/hid-generic.c     |  27 ++++++
>  drivers/hid/hid-lamparray.c   | 162 +++++++++++++++++++++++++++++-----
>  include/linux/hid-lamparray.h |  35 ++++++++
>  3 files changed, 200 insertions(+), 24 deletions(-)
> 
> 
> base-commit: 9b298109e37e5caf4b6800198c4907a5a6bf00ae