Re: [PATCH 00/11] HID: Asus: Add RGB Support to Asus Z13, Ally, unify backlight asus-wmi, and Z13 QOL

["Luke D. Jones" <luke@ljones.dev>]

On 21/03/25 11:09, Antheas Kapenekakis wrote:
> This is a three part series which does the following:
>    - Clean init sequence, fix the keyboard of the Z13 (touchpad,fan button)
>    - Unifies backlight handling to happen under asus-wmi so that all Aura
>      devices have synced brightness controls and the backlight button works
>      properly when it is on a USB laptop keyboard instead of one w/ WMI.
>    - Adds RGB support to hid-asus, solid colors only, and for the ROG Ally
>      units and the Asus Z13 2025 first.
> 
> For context, see cover letter of V1.
> 
> The last two patches are still a bit experimental, the rest is getting to
> be pretty stable by now. I will test my Ally in the weekend. Also, I am
> not a fan of the asus-0003:0B05:1A30.0001-led name, so suggestions would
> be appreciated.
> 
> ---
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/

Hi Antheas, just a very quick note before I review - did you forget to 
add `-v2` to git format-patch? Don't do it now, it's just a reminder for 
next version.

Also I guess asus_brt_ means asus_bright_, but maybe we can rename to 
asus_led_ or even asus_rgbw_? I think something a tad more descriptive 
while still keeping the length down would help future contributors 
quickly understand intent. I'll mention it again when I get to that 
actual patch during test and review.

Looks like good progress so far.

Cheers,
Luke.

> Changes since V1:
>    - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
>    - Fix ifdef else having an invalid signature (reported by kernel robot)
>    - Restore input arguments to init and keyboard function so they can
>      be re-used for RGB controls.
>    - Remove Z13 delay (it did not work to fix the touchpad) and replace it
>      with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
>      keyboard rename into it.
>    - Unregister brightness listener before removing work queue to avoid
>      a race condition causing corruption
>    - Remove spurious mutex unlock in asus_brt_event
>    - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
>      relocking the mutex and causing a deadlock when unregistering leds
>    - Add extra check during unregistering to avoid calling unregister when
>      no led device is registered.
>    - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
>      the driver to create 4 RGB handlers per device. I also suspect some
>      extra events sneak through (KDE had the @@@@@@).
> 
> Antheas Kapenekakis (11):
>    HID: asus: refactor init sequence per spec
>    HID: asus: prevent binding to all HID devices on ROG
>    HID: asus: add Asus Z13 2025 Fan key
>    HID: Asus: add Z13 folio to generic group for multitouch to work
>    platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
>    HID: asus: listen to the asus-wmi brightness device instead of
>      creating one
>    platform/x86: asus-wmi: remove unused keyboard backlight quirk
>    platform/x86: asus-wmi: add keyboard brightness event handler
>    HID: asus: add support for the asus-wmi brightness handler
>    HID: asus: add basic RGB support
>    HID: asus: add RGB support to the ROG Ally units
> 
>   drivers/hid/hid-asus.c                     | 342 ++++++++++++++++-----
>   drivers/hid/hid-ids.h                      |   2 +-
>   drivers/platform/x86/asus-wmi.c            | 138 ++++++++-
>   include/linux/platform_data/x86/asus-wmi.h |  67 ++--
>   4 files changed, 411 insertions(+), 138 deletions(-)
> 
> 
> base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1