[v5,10/11] HID: asus: add RGB support to the ROG Ally units

Apply the RGB quirk to the QOG Ally units to enable basic RGB support.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hid/hid-asus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On 26/03/25 07:45, Antheas Kapenekakis wrote:
> Apply the RGB quirk to the QOG Ally units to enable basic RGB support.
> 
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/hid/hid-asus.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c135c9ff87b74..fa8ec237efe26 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1424,10 +1424,10 @@ static const struct hid_device_id asus_devices[] = {
>   	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>   	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>   	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
> -	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> +	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
>   	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
>   	    USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
>   	  QUIRK_ROG_CLAYMORE_II_KEYBOARD },

Hi Antheas,

I have some good news for you, ASUS got back to me, there *is* a way to 
get the feature level of a keyboard.

## 2.2. Configuration command

In order to confirm what functions are the USB device supported, host 
retrieves the
configuration information by feature report method. Therefore, the 
firmware has to
return the data (byte 0x06~) to the host.

### 2.2.1. Set feature

| Byte 0    | Byte 1    | Byte 2   | Byte 3   | Byte 4     | Byte 5  |
|-----------|-----------|----------|----------|------------|---------|
| Report ID | OP code   | Addr_L   | Addr_H   | Read ROM   | Length  |
| Report ID | 0x05      | 0x20     | 0x31     | 0x00       | 0x08    |

### 2.2.2. Get feature

| Byte 0    | Byte 1    | Byte 2   | Byte 3   | Byte 4     | Byte 5  |
|-----------|-----------|----------|----------|------------|---------|
| Report ID | 0x05      | 0x20     | 0x31     | 0x00       | 0x08    |

**Byte 6**
- 0x00: KB, 1-zone with single color
- 0x01: KB, QWERASD-partition
- 0x02: KB, 4-zone with RGB
- 0x03: KB, Per-key with RGB
- 0x04: KB, 1-zone with RGB
- Other: reserved

**Byte 7(keyboard language)**
- 0x01: US
- 0x02: UK
- 0x03: JP
- Other: reserved

I've not done anything with this myself yet, circumstances last week 
weren't great for me. If you implement this in driver I will ensure I 
get it tested as I have both single colour and rgb laptops.

What i *do* know is:

- 0x00: KB, 1-zone with single color
- 0x01: KB, QWERASD-partition
These can be standard kb_backlight

- 0x02: KB, 4-zone with RGB
- 0x03: KB, Per-key with RGB
- 0x04: KB, 1-zone with RGB
These work with the regular EC-mode RGB command for static/solid colour 
and you don't need to worry about zone/per-key. It would be good to 
document those as defines or enum or something for future.

Hope this helps.

Cheers,
Luke.

On Mon, 31 Mar 2025 at 09:52, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Mon, 31 Mar 2025 at 00:11, Luke D. Jones <luke@ljones.dev> wrote:
> >
> > On 26/03/25 07:45, Antheas Kapenekakis wrote:
> > > Apply the RGB quirk to the QOG Ally units to enable basic RGB support.
> > >
> > > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > ---
> > >   drivers/hid/hid-asus.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > > index c135c9ff87b74..fa8ec237efe26 100644
> > > --- a/drivers/hid/hid-asus.c
> > > +++ b/drivers/hid/hid-asus.c
> > > @@ -1424,10 +1424,10 @@ static const struct hid_device_id asus_devices[] = {
> > >         QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
> > >       { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> > >           USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
> > > -       QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> > > +       QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
> > >       { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> > >           USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
> > > -       QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> > > +       QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_RGB },
> > >       { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> > >           USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
> > >         QUIRK_ROG_CLAYMORE_II_KEYBOARD },
> >
> > Hi Antheas,
> >
> > I have some good news for you, ASUS got back to me, there *is* a way to
> > get the feature level of a keyboard.
> >
> > ## 2.2. Configuration command
> >
> > In order to confirm what functions are the USB device supported, host
> > retrieves the
> > configuration information by feature report method. Therefore, the
> > firmware has to
> > return the data (byte 0x06~) to the host.
> >
> > ### 2.2.1. Set feature
> >
> > | Byte 0    | Byte 1    | Byte 2   | Byte 3   | Byte 4     | Byte 5  |
> > |-----------|-----------|----------|----------|------------|---------|
> > | Report ID | OP code   | Addr_L   | Addr_H   | Read ROM   | Length  |
> > | Report ID | 0x05      | 0x20     | 0x31     | 0x00       | 0x08    |
> >
> > ### 2.2.2. Get feature
> >
> > | Byte 0    | Byte 1    | Byte 2   | Byte 3   | Byte 4     | Byte 5  |
> > |-----------|-----------|----------|----------|------------|---------|
> > | Report ID | 0x05      | 0x20     | 0x31     | 0x00       | 0x08    |
> >
> > **Byte 6**
> > - 0x00: KB, 1-zone with single color
>
> Nice find. The asus-hid driver already implements this and checks for
> 0x00 to bail the backlight.
>
> So that check should be removed as it does not work with single color
> keyboards and instead checked for 2,3,4 to enable RGB.
>
> This also means removing the RGB check and getting global support in one go.
>
> Antheas
>
> > - 0x01: KB, QWERASD-partition
> > - 0x02: KB, 4-zone with RGB
> > - 0x03: KB, Per-key with RGB
> > - 0x04: KB, 1-zone with RGB
> > - Other: reserved
> >
> > **Byte 7(keyboard language)**
> > - 0x01: US
> > - 0x02: UK
> > - 0x03: JP
> > - Other: reserved
> >
> > I've not done anything with this myself yet, circumstances last week
> > weren't great for me. If you implement this in driver I will ensure I
> > get it tested as I have both single colour and rgb laptops.
> >
> > What i *do* know is:
> >
> > - 0x00: KB, 1-zone with single color
> > - 0x01: KB, QWERASD-partition
> > These can be standard kb_backlight
> >
> > - 0x02: KB, 4-zone with RGB
> > - 0x03: KB, Per-key with RGB
> > - 0x04: KB, 1-zone with RGB
> > These work with the regular EC-mode RGB command for static/solid colour
> > and you don't need to worry about zone/per-key. It would be good to
> > document those as defines or enum or something for future.

Let's start slowly getting back into this.

Ok, the lightbar of the Z13 returns:
5a05203100080100010423000100060214020000000000000153550000000000

And the keyboard returns:
5a052031000801000004250501002e0000000000000000000000000000000000

That is 01 for both the lightbar and keyboard when it comes to zone
(QWERASD) and 00/undefined when it comes to keyboard?

Now, if they meant byte 6 _after_ the header:
5a0520310008 01 00 01 04 23 00 01 00060214020000000000000153550000000000
5a0520310008 01 00 00 04 25 05 01 002e0000000000000000000000000000000000

It is still inconclusive

So I am unsure what to make of this

Antheas

> > Hope this helps.
> >
> > Cheers,
> > Luke.