linux-kernel - Re: [PATCH v5 10/11] HID: asus: add RGB support to the ROG Ally units

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: 
 <CAGwozwGqy344a_hr=2sKOiB3zUN36M12nOKp4_JpUuhW-pYBXA@mail.gmail.com>
Date: Thu, 10 Apr 2025 21:39:08 +0200
From: Antheas Kapenekakis <lkml@...heas.dev>
To: "Luke D. Jones" <luke@...nes.dev>
Cc: platform-driver-x86@...r.kernel.org, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, Jiri Kosina <jikos@...nel.org>,
	Benjamin Tissoires <bentiss@...nel.org>,
 Corentin Chary <corentin.chary@...il.com>,
	Hans de Goede <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v5 10/11] HID: asus: add RGB support to the ROG Ally units

On Mon, 31 Mar 2025 at 09:52, Antheas Kapenekakis <lkml@...heas.dev> wrote:
>
> On Mon, 31 Mar 2025 at 00:11, Luke D. Jones <luke@...nes.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@...nes.dev>
> > > Signed-off-by: Antheas Kapenekakis <lkml@...heas.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.