Re: [PATCH v5 0/2] HID: generic: add LampArray support via hid-lamparray helper

Am 04.09.26 um 22:49 schrieb Armin Wolf:

Am 03.09.26 um 09:35 schrieb Aaron Erhardt:

quoted

Add a new hid-lamparray helper module and integrate it with the
hid-generic driver.

While more complex lamparray handling should be done in userspace via
hidraw, providing a small module to add basic lamparray support makes it
possible for userspace software to interact with lamparrays by simply
using well-known APIs of the LED subsystem. One use-case would be to
enable desktop environments to support keyboard backlight control out of
the box for HID lamparray devices without having to implement the whole
HID protocol themselves.

This patch is based on previous discussions:
https://lore.kernel.org/all/1fb08a74-62c7-4d0c-ba5d-648e23082dcb@tuxedocomputers.com/ (local)

The helper provides basic support for devices exposing a
Lighting/LampArray application collection (usage page 0x59) and
registers a single-zone RGB LED representation via the LED
subsystem.

hid-generic now checks for LampArray support after hid_parse() and
optionally registers a lamparray instance. Failures in the helper
do not abort device probe to keep the driver logic otherwise unchanged.

LampArray resources are released on driver remove.

This commit was successfully tested on the Microsoft MacroPad reference
implementation (https://github.com/microsoft/RP2040MacropadHidSample
1d6c3ad) and in combination with the tuxedo_nb04_wmi driver, albeit
only fully functional with a recent fix posted to the LKML
(https://lore.kernel.org/all/20260826081149.235487-2-aer@tuxedocomputers.com (local)).

Nice work, it works on my ASUS Prime B650-Plus. However the behavior of the brightness
attribute is a bit strange:

- manually setting "brightness" does not change anything (max. is 1)
- setting RGB to "0 0 0" causes "brightness" to become 0
- setting RGB to a non-zero value causes "brightness" to become 1

Any idea why this happens? I can check if the same problems also exists under Windows,
if requested.

Thanks,
Armin Wolf

It is completely normal for LampArray devices to only offer two brightness
values (1 and 0) for turning the whole LED on and off. Since a lot of
userspace software seems to never use brightness (it is more convenient to
adjust the RGB channels directly), this was not even properly implemented in
the MacropadHidSample until recently:
https://github.com/microsoft/RP2040MacropadHidSample/commit/cfc29120a3910c5772976da29ecd57392dfd44d6

Thus, I think it is likely, that the implementation is broken and simply
ignores brightness. The driver just forwards this to the device.

However, the RGB values (aka. multi_intensity) should not interfere with the
brightness. Yet, I wasn't able to reproduce this on the Macropad. Capping the
brightness to 1 is normal on the other hand, at least if that's the
maxIntensity reported by your device.

So the only really odd thing for me would be the RGB values influencing the
brightness. Please provide more detailed feedback if you can since I can't
reproduce this on the hardware available to me.

If you want to investigate the LampArray capabilities of your device, you're
probably better off with userspace tooling like my lampctl fork:
https://github.com/tuxedo-aer/lampctl

You can adjust the hardcoded brightness here to see whether your device honors
the brightness value or not:
https://github.com/tuxedo-aer/lampctl/blob/main/crates/lamparray/src/hid.rs#L36

quoted

v5:
- Proper hardware detection (no quirks necessary anymore)
- Add documentation for new sysfs knob
- Pass limits of the device to sysfs (intesities & brightness)
- More flexible Kconfig (use tristate)
- Improved locking
- Several memory leak and (de-)initialization fixes
- Don't read current color values from hardware (the HID spec does not
   offer this option)
- Remove redundant report dump functionality
v4:
- Restrict CONFIG_HID_LAMPARRAY to built-in configurations only to fix
   additional randconfig build errors
v3:
- Squash V1 and V2 into one patch
v2:
- Fix Kconfig to avoid build errors when LEDS_CLASS_MULTICOLOR is
   disabled

Aaron Erhardt (2):
   HID: lamparray: add new LampArray helper module
   HID: generic: add LampArray support via hid-lamparray helper

  .../ABI/testing/sysfs-driver-hid-lamparray    |  16 +
  drivers/hid/Kconfig                           |  18 +
  drivers/hid/Makefile                          |   2 +
  drivers/hid/hid-generic.c                     |  38 +
  drivers/hid/hid-lamparray.c                   | 812 ++++++++++++++++++
  include/linux/hid-lamparray.h                 |  88 ++
  6 files changed, 974 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-lamparray
  create mode 100644 drivers/hid/hid-lamparray.c
  create mode 100644 include/linux/hid-lamparray.h