* [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
@ 2025-11-22 11:00 Antheas Kapenekakis
2025-11-26 13:37 ` Antheas Kapenekakis
2025-12-02 11:28 ` Benjamin Tissoires
0 siblings, 2 replies; 16+ messages in thread
From: Antheas Kapenekakis @ 2025-11-22 11:00 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato,
Antheas Kapenekakis
This is a two part series which does the following:
- Clean-up init sequence
- Unify 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.
For more context, see cover letter of V1. Since V5, I removed some patches
to make this easier to merge.
---
V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
Changes since V9:
- No functional changes
- Rebase to review-ilpo-next
- Fix armoury series conflict by removing the file asus-wmi-leds-ids on
"remove unused keyboard backlight quirk" + imports
Dismiss Luke's review as this patch diverged
- Reword paragraph in "Add support for multiple kbd led handlers" to be
more verbose
- Use kfree in fortify patch
- Fix minor style quirks from --nonstict checkpatch run
Changes since V8:
- No functional changes
- Move legacy init patch to second, modify first patch so that their
diff is minimized
- Split "prevent binding to all HID devices on ROG" into two patches:
- moving backlight initialization into probe
- early exit to skip ->init check and rename
- Remove skipping vendor fixups for non-vendor devices. It is not possible
to read usages before the report fixups are applied, so it did not work
- In that patch, reword a comment to be single line and make is_vendor a bool
- Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
has drifted too far since he reviewed/tested it.
Changes since V7:
- Readd legacy init quirk for Dennis
- Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
- Fix warning due to enum_backlight receiving negative values
Changes since V6:
- Split initialization refactor into three patches, update commit text
to be clearer in what it does
- Replace spinlock accesses with guard and scoped guard in all patches
- Add missing includes mentioned by Ilpo
- Reflow, tweak comment in prevent binding to all HID devices on ROG
- Replace asus_ref.asus with local reference in all patches
- Add missing kernel doc comments
- Other minor nits from Ilpo
- User reported warning due to scheduling work while holding a spinlock.
Restructure patch for multiple handlers to limit when spinlock is held to
variable access only. In parallel, setup a workqueue to handle registration
of led device and setting brightness. This is required as registering the
led device triggers kbd_led_get which needs to hold the spinlock to
protect the led_wk value. The workqueue is also required for the hid
event passthrough to avoid scheduling work while holding the spinlock.
Apply the workqueue to wmi brightness buttons as well, as that was
omitted before this series and WMI access was performed.
- On "HID: asus: prevent binding to all HID devices on ROG", rename
quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
This allows other quirks to apply (applies quirk that fixes keyboard
being named as a pointer device).
Changes since V5:
- It's been a long time
- Remove addition of RGB as that had some comments I need to work on
- Remove folio patch (already merged)
- Remove legacy fix patch 11 from V4. There is a small chance that
without this patch, some old NKEY keyboards might not respond to
RGB commands according to Luke, but the kernel driver does not do
RGB currently. The 0x5d init is done by Armoury crate software in
Windows. If an issue is found, we can re-add it or just remove patches
1/2 before merging. However, init could use the cleanup.
Changes since V4:
- Fix KConfig (reported by kernel robot)
- Fix Ilpo's nits, if I missed anything lmk
Changes since V3:
- Add initializer for 0x5d for old NKEY keyboards until it is verified
that it is not needed for their media keys to function.
- Cover init in asus-wmi with spinlock as per Hans
- If asus-wmi registers WMI handler with brightness, init the brightness
in USB Asus keyboards, per Hans.
- Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
- Fix oops when unregistering asus-wmi by moving unregister outside of
the spin lock (but after the asus reference is set to null)
Changes since V2:
- Check lazy init succeds in asus-wmi before setting register variable
- make explicit check in asus_hid_register_listener for listener existing
to avoid re-init
- rename asus_brt to asus_hid in most places and harmonize everything
- switch to a spinlock instead of a mutex to avoid kernel ooops
- fixup hid device quirks to avoid multiple RGB devices while still exposing
all input vendor devices. This includes moving rgb init to probe
instead of the input_configured callbacks.
- Remove fan key (during retest it appears to be 0xae that is already
supported by hid-asus)
- Never unregister asus::kbd_backlight while asus-wmi is active, as that
- removes fds from userspace and breaks backlight functionality. All
- current mainline drivers do not support backlight hotplugging, so most
userspace software (e.g., KDE, UPower) is built with that assumption.
For the Ally, since it disconnects its controller during sleep, this
caused the backlight slider to not work in KDE.
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: simplify RGB init sequence
HID: asus: initialize additional endpoints only for legacy devices
HID: asus: use same report_id in response
HID: asus: fortify keyboard handshake
HID: asus: move vendor initialization to probe
HID: asus: early return for ROG devices
platform/x86: asus-wmi: Add support for multiple kbd led 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
drivers/hid/hid-asus.c | 205 ++++++++--------
drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
.../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
include/linux/platform_data/x86/asus-wmi.h | 28 +++
4 files changed, 322 insertions(+), 184 deletions(-)
delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
--
2.52.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-22 11:00 Antheas Kapenekakis
@ 2025-11-26 13:37 ` Antheas Kapenekakis
2025-11-26 15:23 ` Ilpo Järvinen
2025-12-02 11:28 ` Benjamin Tissoires
1 sibling, 1 reply; 16+ messages in thread
From: Antheas Kapenekakis @ 2025-11-26 13:37 UTC (permalink / raw)
To: platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato
On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> This is a two part series which does the following:
> - Clean-up init sequence
> - Unify 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.
>
> For more context, see cover letter of V1. Since V5, I removed some patches
> to make this easier to merge.
Slight bump on this. It addresses both of the remarks Denis made in
the previous version.
I begrudgingly argued a bit for those because I did not want to resend
the series and they were not functional changes, so sorry about that.
But they are fixed in this version incl. with the conflict with the
armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
Add support for multiple kbd led handlers" even though I addressed
your comment, so you may want to add that.
As for "HID: asus: early return for ROG devices" changing the name of
the devices of this driver, I will veto backporting it if it happens,
so inputplumber will have the two full months to remove the name
match. This is not a breaking change in the sense that software cannot
be made to work on both previous and latter versions and there is no
other software to my knowledge relying on name matches for Asus
keyboards. Moreover, an early exit is needed to prevent ejecting HID
endpoints without an ->input parameter so it is a needed fix anyway.
Postponing it will prevent Xbox Ally users from having RGB control
through userspace on a stock kernel but it is also not worth arguing
about
It is also fine for me for this series to merge for 6.20, but I'd
rather we handle it now since there will be some turbulence for asus
users due to armoury merging so it makes sense to have this transition
once.
Antheas
> ---
> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>
> Changes since V9:
> - No functional changes
> - Rebase to review-ilpo-next
> - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> "remove unused keyboard backlight quirk" + imports
> Dismiss Luke's review as this patch diverged
> - Reword paragraph in "Add support for multiple kbd led handlers" to be
> more verbose
> - Use kfree in fortify patch
> - Fix minor style quirks from --nonstict checkpatch run
>
> Changes since V8:
> - No functional changes
> - Move legacy init patch to second, modify first patch so that their
> diff is minimized
> - Split "prevent binding to all HID devices on ROG" into two patches:
> - moving backlight initialization into probe
> - early exit to skip ->init check and rename
> - Remove skipping vendor fixups for non-vendor devices. It is not possible
> to read usages before the report fixups are applied, so it did not work
> - In that patch, reword a comment to be single line and make is_vendor a bool
> - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> has drifted too far since he reviewed/tested it.
>
> Changes since V7:
> - Readd legacy init quirk for Dennis
> - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> - Fix warning due to enum_backlight receiving negative values
>
> Changes since V6:
> - Split initialization refactor into three patches, update commit text
> to be clearer in what it does
> - Replace spinlock accesses with guard and scoped guard in all patches
> - Add missing includes mentioned by Ilpo
> - Reflow, tweak comment in prevent binding to all HID devices on ROG
> - Replace asus_ref.asus with local reference in all patches
> - Add missing kernel doc comments
> - Other minor nits from Ilpo
> - User reported warning due to scheduling work while holding a spinlock.
> Restructure patch for multiple handlers to limit when spinlock is held to
> variable access only. In parallel, setup a workqueue to handle registration
> of led device and setting brightness. This is required as registering the
> led device triggers kbd_led_get which needs to hold the spinlock to
> protect the led_wk value. The workqueue is also required for the hid
> event passthrough to avoid scheduling work while holding the spinlock.
> Apply the workqueue to wmi brightness buttons as well, as that was
> omitted before this series and WMI access was performed.
> - On "HID: asus: prevent binding to all HID devices on ROG", rename
> quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> This allows other quirks to apply (applies quirk that fixes keyboard
> being named as a pointer device).
>
> Changes since V5:
> - It's been a long time
> - Remove addition of RGB as that had some comments I need to work on
> - Remove folio patch (already merged)
> - Remove legacy fix patch 11 from V4. There is a small chance that
> without this patch, some old NKEY keyboards might not respond to
> RGB commands according to Luke, but the kernel driver does not do
> RGB currently. The 0x5d init is done by Armoury crate software in
> Windows. If an issue is found, we can re-add it or just remove patches
> 1/2 before merging. However, init could use the cleanup.
>
> Changes since V4:
> - Fix KConfig (reported by kernel robot)
> - Fix Ilpo's nits, if I missed anything lmk
>
> Changes since V3:
> - Add initializer for 0x5d for old NKEY keyboards until it is verified
> that it is not needed for their media keys to function.
> - Cover init in asus-wmi with spinlock as per Hans
> - If asus-wmi registers WMI handler with brightness, init the brightness
> in USB Asus keyboards, per Hans.
> - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> - Fix oops when unregistering asus-wmi by moving unregister outside of
> the spin lock (but after the asus reference is set to null)
>
> Changes since V2:
> - Check lazy init succeds in asus-wmi before setting register variable
> - make explicit check in asus_hid_register_listener for listener existing
> to avoid re-init
> - rename asus_brt to asus_hid in most places and harmonize everything
> - switch to a spinlock instead of a mutex to avoid kernel ooops
> - fixup hid device quirks to avoid multiple RGB devices while still exposing
> all input vendor devices. This includes moving rgb init to probe
> instead of the input_configured callbacks.
> - Remove fan key (during retest it appears to be 0xae that is already
> supported by hid-asus)
> - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> - removes fds from userspace and breaks backlight functionality. All
> - current mainline drivers do not support backlight hotplugging, so most
> userspace software (e.g., KDE, UPower) is built with that assumption.
> For the Ally, since it disconnects its controller during sleep, this
> caused the backlight slider to not work in KDE.
>
> 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: simplify RGB init sequence
> HID: asus: initialize additional endpoints only for legacy devices
> HID: asus: use same report_id in response
> HID: asus: fortify keyboard handshake
> HID: asus: move vendor initialization to probe
> HID: asus: early return for ROG devices
> platform/x86: asus-wmi: Add support for multiple kbd led 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
>
> drivers/hid/hid-asus.c | 205 ++++++++--------
> drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> include/linux/platform_data/x86/asus-wmi.h | 28 +++
> 4 files changed, 322 insertions(+), 184 deletions(-)
> delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>
>
> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> --
> 2.52.0
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-26 13:37 ` Antheas Kapenekakis
@ 2025-11-26 15:23 ` Ilpo Järvinen
2025-11-26 15:28 ` Antheas Kapenekakis
2025-11-26 17:34 ` Hans de Goede
0 siblings, 2 replies; 16+ messages in thread
From: Ilpo Järvinen @ 2025-11-26 15:23 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > This is a two part series which does the following:
> > - Clean-up init sequence
> > - Unify 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.
> >
> > For more context, see cover letter of V1. Since V5, I removed some patches
> > to make this easier to merge.
>
> Slight bump on this. It addresses both of the remarks Denis made in
> the previous version.
>
> I begrudgingly argued a bit for those because I did not want to resend
> the series and they were not functional changes, so sorry about that.
> But they are fixed in this version incl. with the conflict with the
> armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> Add support for multiple kbd led handlers" even though I addressed
> your comment, so you may want to add that.
FYI, there's no direct relation that mandates a person to give a rev-by
even if all his/her comments were addressed.
But generally yes, it would be useful to hear whether Denis is fine with
v10, especially those patches that had contention earlier but you've
modified post-v8.
> As for "HID: asus: early return for ROG devices" changing the name of
> the devices of this driver, I will veto backporting it if it happens,
> so inputplumber will have the two full months to remove the name
> match. This is not a breaking change in the sense that software cannot
> be made to work on both previous and latter versions and there is no
> other software to my knowledge relying on name matches for Asus
> keyboards.
Did Hans give some opinion about this rename earlier, at least I don't
remember nor could find from lore archives?
--
i.
> Moreover, an early exit is needed to prevent ejecting HID
> endpoints without an ->input parameter so it is a needed fix anyway.
> Postponing it will prevent Xbox Ally users from having RGB control
> through userspace on a stock kernel but it is also not worth arguing
> about
>
> It is also fine for me for this series to merge for 6.20, but I'd
> rather we handle it now since there will be some turbulence for asus
> users due to armoury merging so it makes sense to have this transition
> once.
>
> > ---
> > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> >
> > Changes since V9:
> > - No functional changes
> > - Rebase to review-ilpo-next
> > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > "remove unused keyboard backlight quirk" + imports
> > Dismiss Luke's review as this patch diverged
> > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > more verbose
> > - Use kfree in fortify patch
> > - Fix minor style quirks from --nonstict checkpatch run
> >
> > Changes since V8:
> > - No functional changes
> > - Move legacy init patch to second, modify first patch so that their
> > diff is minimized
> > - Split "prevent binding to all HID devices on ROG" into two patches:
> > - moving backlight initialization into probe
> > - early exit to skip ->init check and rename
> > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > to read usages before the report fixups are applied, so it did not work
> > - In that patch, reword a comment to be single line and make is_vendor a bool
> > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > has drifted too far since he reviewed/tested it.
> >
> > Changes since V7:
> > - Readd legacy init quirk for Dennis
> > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > - Fix warning due to enum_backlight receiving negative values
> >
> > Changes since V6:
> > - Split initialization refactor into three patches, update commit text
> > to be clearer in what it does
> > - Replace spinlock accesses with guard and scoped guard in all patches
> > - Add missing includes mentioned by Ilpo
> > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > - Replace asus_ref.asus with local reference in all patches
> > - Add missing kernel doc comments
> > - Other minor nits from Ilpo
> > - User reported warning due to scheduling work while holding a spinlock.
> > Restructure patch for multiple handlers to limit when spinlock is held to
> > variable access only. In parallel, setup a workqueue to handle registration
> > of led device and setting brightness. This is required as registering the
> > led device triggers kbd_led_get which needs to hold the spinlock to
> > protect the led_wk value. The workqueue is also required for the hid
> > event passthrough to avoid scheduling work while holding the spinlock.
> > Apply the workqueue to wmi brightness buttons as well, as that was
> > omitted before this series and WMI access was performed.
> > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > This allows other quirks to apply (applies quirk that fixes keyboard
> > being named as a pointer device).
> >
> > Changes since V5:
> > - It's been a long time
> > - Remove addition of RGB as that had some comments I need to work on
> > - Remove folio patch (already merged)
> > - Remove legacy fix patch 11 from V4. There is a small chance that
> > without this patch, some old NKEY keyboards might not respond to
> > RGB commands according to Luke, but the kernel driver does not do
> > RGB currently. The 0x5d init is done by Armoury crate software in
> > Windows. If an issue is found, we can re-add it or just remove patches
> > 1/2 before merging. However, init could use the cleanup.
> >
> > Changes since V4:
> > - Fix KConfig (reported by kernel robot)
> > - Fix Ilpo's nits, if I missed anything lmk
> >
> > Changes since V3:
> > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > that it is not needed for their media keys to function.
> > - Cover init in asus-wmi with spinlock as per Hans
> > - If asus-wmi registers WMI handler with brightness, init the brightness
> > in USB Asus keyboards, per Hans.
> > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > the spin lock (but after the asus reference is set to null)
> >
> > Changes since V2:
> > - Check lazy init succeds in asus-wmi before setting register variable
> > - make explicit check in asus_hid_register_listener for listener existing
> > to avoid re-init
> > - rename asus_brt to asus_hid in most places and harmonize everything
> > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > all input vendor devices. This includes moving rgb init to probe
> > instead of the input_configured callbacks.
> > - Remove fan key (during retest it appears to be 0xae that is already
> > supported by hid-asus)
> > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > - removes fds from userspace and breaks backlight functionality. All
> > - current mainline drivers do not support backlight hotplugging, so most
> > userspace software (e.g., KDE, UPower) is built with that assumption.
> > For the Ally, since it disconnects its controller during sleep, this
> > caused the backlight slider to not work in KDE.
> >
> > 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: simplify RGB init sequence
> > HID: asus: initialize additional endpoints only for legacy devices
> > HID: asus: use same report_id in response
> > HID: asus: fortify keyboard handshake
> > HID: asus: move vendor initialization to probe
> > HID: asus: early return for ROG devices
> > platform/x86: asus-wmi: Add support for multiple kbd led 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
> >
> > drivers/hid/hid-asus.c | 205 ++++++++--------
> > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > 4 files changed, 322 insertions(+), 184 deletions(-)
> > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> >
> >
> > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > --
> > 2.52.0
> >
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-26 15:23 ` Ilpo Järvinen
@ 2025-11-26 15:28 ` Antheas Kapenekakis
2025-11-26 15:29 ` Antheas Kapenekakis
2025-11-26 17:34 ` Hans de Goede
1 sibling, 1 reply; 16+ messages in thread
From: Antheas Kapenekakis @ 2025-11-26 15:28 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>
> > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > >
> > > This is a two part series which does the following:
> > > - Clean-up init sequence
> > > - Unify 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.
> > >
> > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > to make this easier to merge.
> >
> > Slight bump on this. It addresses both of the remarks Denis made in
> > the previous version.
> >
> > I begrudgingly argued a bit for those because I did not want to resend
> > the series and they were not functional changes, so sorry about that.
> > But they are fixed in this version incl. with the conflict with the
> > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > Add support for multiple kbd led handlers" even though I addressed
> > your comment, so you may want to add that.
>
> FYI, there's no direct relation that mandates a person to give a rev-by
> even if all his/her comments were addressed.
True, this is just a reminder because I did not hear from him and
since he added a rev-by on the kfree patch. There's no obligation from
my side.
> But generally yes, it would be useful to hear whether Denis is fine with
> v10, especially those patches that had contention earlier but you've
> modified post-v8.
>
> > As for "HID: asus: early return for ROG devices" changing the name of
> > the devices of this driver, I will veto backporting it if it happens,
> > so inputplumber will have the two full months to remove the name
> > match. This is not a breaking change in the sense that software cannot
> > be made to work on both previous and latter versions and there is no
> > other software to my knowledge relying on name matches for Asus
> > keyboards.
>
> Did Hans give some opinion about this rename earlier, at least I don't
> remember nor could find from lore archives?
Hans jumped in on the ayaneo controller patch. I don't think I saw
activity on this series
Antheas
> --
> i.
>
> > Moreover, an early exit is needed to prevent ejecting HID
> > endpoints without an ->input parameter so it is a needed fix anyway.
> > Postponing it will prevent Xbox Ally users from having RGB control
> > through userspace on a stock kernel but it is also not worth arguing
> > about
> >
> > It is also fine for me for this series to merge for 6.20, but I'd
> > rather we handle it now since there will be some turbulence for asus
> > users due to armoury merging so it makes sense to have this transition
> > once.
> >
> > > ---
> > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > >
> > > Changes since V9:
> > > - No functional changes
> > > - Rebase to review-ilpo-next
> > > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > "remove unused keyboard backlight quirk" + imports
> > > Dismiss Luke's review as this patch diverged
> > > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > more verbose
> > > - Use kfree in fortify patch
> > > - Fix minor style quirks from --nonstict checkpatch run
> > >
> > > Changes since V8:
> > > - No functional changes
> > > - Move legacy init patch to second, modify first patch so that their
> > > diff is minimized
> > > - Split "prevent binding to all HID devices on ROG" into two patches:
> > > - moving backlight initialization into probe
> > > - early exit to skip ->init check and rename
> > > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > to read usages before the report fixups are applied, so it did not work
> > > - In that patch, reword a comment to be single line and make is_vendor a bool
> > > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > has drifted too far since he reviewed/tested it.
> > >
> > > Changes since V7:
> > > - Readd legacy init quirk for Dennis
> > > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > - Fix warning due to enum_backlight receiving negative values
> > >
> > > Changes since V6:
> > > - Split initialization refactor into three patches, update commit text
> > > to be clearer in what it does
> > > - Replace spinlock accesses with guard and scoped guard in all patches
> > > - Add missing includes mentioned by Ilpo
> > > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > - Replace asus_ref.asus with local reference in all patches
> > > - Add missing kernel doc comments
> > > - Other minor nits from Ilpo
> > > - User reported warning due to scheduling work while holding a spinlock.
> > > Restructure patch for multiple handlers to limit when spinlock is held to
> > > variable access only. In parallel, setup a workqueue to handle registration
> > > of led device and setting brightness. This is required as registering the
> > > led device triggers kbd_led_get which needs to hold the spinlock to
> > > protect the led_wk value. The workqueue is also required for the hid
> > > event passthrough to avoid scheduling work while holding the spinlock.
> > > Apply the workqueue to wmi brightness buttons as well, as that was
> > > omitted before this series and WMI access was performed.
> > > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > This allows other quirks to apply (applies quirk that fixes keyboard
> > > being named as a pointer device).
> > >
> > > Changes since V5:
> > > - It's been a long time
> > > - Remove addition of RGB as that had some comments I need to work on
> > > - Remove folio patch (already merged)
> > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > without this patch, some old NKEY keyboards might not respond to
> > > RGB commands according to Luke, but the kernel driver does not do
> > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > Windows. If an issue is found, we can re-add it or just remove patches
> > > 1/2 before merging. However, init could use the cleanup.
> > >
> > > Changes since V4:
> > > - Fix KConfig (reported by kernel robot)
> > > - Fix Ilpo's nits, if I missed anything lmk
> > >
> > > Changes since V3:
> > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > that it is not needed for their media keys to function.
> > > - Cover init in asus-wmi with spinlock as per Hans
> > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > in USB Asus keyboards, per Hans.
> > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > the spin lock (but after the asus reference is set to null)
> > >
> > > Changes since V2:
> > > - Check lazy init succeds in asus-wmi before setting register variable
> > > - make explicit check in asus_hid_register_listener for listener existing
> > > to avoid re-init
> > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > all input vendor devices. This includes moving rgb init to probe
> > > instead of the input_configured callbacks.
> > > - Remove fan key (during retest it appears to be 0xae that is already
> > > supported by hid-asus)
> > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > - removes fds from userspace and breaks backlight functionality. All
> > > - current mainline drivers do not support backlight hotplugging, so most
> > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > For the Ally, since it disconnects its controller during sleep, this
> > > caused the backlight slider to not work in KDE.
> > >
> > > 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: simplify RGB init sequence
> > > HID: asus: initialize additional endpoints only for legacy devices
> > > HID: asus: use same report_id in response
> > > HID: asus: fortify keyboard handshake
> > > HID: asus: move vendor initialization to probe
> > > HID: asus: early return for ROG devices
> > > platform/x86: asus-wmi: Add support for multiple kbd led 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
> > >
> > > drivers/hid/hid-asus.c | 205 ++++++++--------
> > > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > > 4 files changed, 322 insertions(+), 184 deletions(-)
> > > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > >
> > >
> > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > --
> > > 2.52.0
> > >
> > >
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-26 15:28 ` Antheas Kapenekakis
@ 2025-11-26 15:29 ` Antheas Kapenekakis
2025-11-26 15:31 ` Ilpo Järvinen
0 siblings, 1 reply; 16+ messages in thread
From: Antheas Kapenekakis @ 2025-11-26 15:29 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> >
> > > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > >
> > > > This is a two part series which does the following:
> > > > - Clean-up init sequence
> > > > - Unify 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.
> > > >
> > > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > > to make this easier to merge.
> > >
> > > Slight bump on this. It addresses both of the remarks Denis made in
> > > the previous version.
> > >
> > > I begrudgingly argued a bit for those because I did not want to resend
> > > the series and they were not functional changes, so sorry about that.
> > > But they are fixed in this version incl. with the conflict with the
> > > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > > Add support for multiple kbd led handlers" even though I addressed
> > > your comment, so you may want to add that.
> >
> > FYI, there's no direct relation that mandates a person to give a rev-by
> > even if all his/her comments were addressed.
>
> True, this is just a reminder because I did not hear from him and
> since he added a rev-by on the kfree patch. There's no obligation from
> my side.
>
> > But generally yes, it would be useful to hear whether Denis is fine with
> > v10, especially those patches that had contention earlier but you've
> > modified post-v8.
> >
> > > As for "HID: asus: early return for ROG devices" changing the name of
> > > the devices of this driver, I will veto backporting it if it happens,
> > > so inputplumber will have the two full months to remove the name
> > > match. This is not a breaking change in the sense that software cannot
> > > be made to work on both previous and latter versions and there is no
> > > other software to my knowledge relying on name matches for Asus
> > > keyboards.
> >
> > Did Hans give some opinion about this rename earlier, at least I don't
> > remember nor could find from lore archives?
>
> Hans jumped in on the ayaneo controller patch. I don't think I saw
> activity on this series
Hans had some feedback around half a year ago for the latter part of
this series that binds the devices together
> Antheas
>
> > --
> > i.
> >
> > > Moreover, an early exit is needed to prevent ejecting HID
> > > endpoints without an ->input parameter so it is a needed fix anyway.
> > > Postponing it will prevent Xbox Ally users from having RGB control
> > > through userspace on a stock kernel but it is also not worth arguing
> > > about
> > >
> > > It is also fine for me for this series to merge for 6.20, but I'd
> > > rather we handle it now since there will be some turbulence for asus
> > > users due to armoury merging so it makes sense to have this transition
> > > once.
> > >
> > > > ---
> > > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > > >
> > > > Changes since V9:
> > > > - No functional changes
> > > > - Rebase to review-ilpo-next
> > > > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > > "remove unused keyboard backlight quirk" + imports
> > > > Dismiss Luke's review as this patch diverged
> > > > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > > more verbose
> > > > - Use kfree in fortify patch
> > > > - Fix minor style quirks from --nonstict checkpatch run
> > > >
> > > > Changes since V8:
> > > > - No functional changes
> > > > - Move legacy init patch to second, modify first patch so that their
> > > > diff is minimized
> > > > - Split "prevent binding to all HID devices on ROG" into two patches:
> > > > - moving backlight initialization into probe
> > > > - early exit to skip ->init check and rename
> > > > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > > to read usages before the report fixups are applied, so it did not work
> > > > - In that patch, reword a comment to be single line and make is_vendor a bool
> > > > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > > has drifted too far since he reviewed/tested it.
> > > >
> > > > Changes since V7:
> > > > - Readd legacy init quirk for Dennis
> > > > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > > - Fix warning due to enum_backlight receiving negative values
> > > >
> > > > Changes since V6:
> > > > - Split initialization refactor into three patches, update commit text
> > > > to be clearer in what it does
> > > > - Replace spinlock accesses with guard and scoped guard in all patches
> > > > - Add missing includes mentioned by Ilpo
> > > > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > > - Replace asus_ref.asus with local reference in all patches
> > > > - Add missing kernel doc comments
> > > > - Other minor nits from Ilpo
> > > > - User reported warning due to scheduling work while holding a spinlock.
> > > > Restructure patch for multiple handlers to limit when spinlock is held to
> > > > variable access only. In parallel, setup a workqueue to handle registration
> > > > of led device and setting brightness. This is required as registering the
> > > > led device triggers kbd_led_get which needs to hold the spinlock to
> > > > protect the led_wk value. The workqueue is also required for the hid
> > > > event passthrough to avoid scheduling work while holding the spinlock.
> > > > Apply the workqueue to wmi brightness buttons as well, as that was
> > > > omitted before this series and WMI access was performed.
> > > > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > > This allows other quirks to apply (applies quirk that fixes keyboard
> > > > being named as a pointer device).
> > > >
> > > > Changes since V5:
> > > > - It's been a long time
> > > > - Remove addition of RGB as that had some comments I need to work on
> > > > - Remove folio patch (already merged)
> > > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > > without this patch, some old NKEY keyboards might not respond to
> > > > RGB commands according to Luke, but the kernel driver does not do
> > > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > > Windows. If an issue is found, we can re-add it or just remove patches
> > > > 1/2 before merging. However, init could use the cleanup.
> > > >
> > > > Changes since V4:
> > > > - Fix KConfig (reported by kernel robot)
> > > > - Fix Ilpo's nits, if I missed anything lmk
> > > >
> > > > Changes since V3:
> > > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > > that it is not needed for their media keys to function.
> > > > - Cover init in asus-wmi with spinlock as per Hans
> > > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > > in USB Asus keyboards, per Hans.
> > > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > > the spin lock (but after the asus reference is set to null)
> > > >
> > > > Changes since V2:
> > > > - Check lazy init succeds in asus-wmi before setting register variable
> > > > - make explicit check in asus_hid_register_listener for listener existing
> > > > to avoid re-init
> > > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > > all input vendor devices. This includes moving rgb init to probe
> > > > instead of the input_configured callbacks.
> > > > - Remove fan key (during retest it appears to be 0xae that is already
> > > > supported by hid-asus)
> > > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > > - removes fds from userspace and breaks backlight functionality. All
> > > > - current mainline drivers do not support backlight hotplugging, so most
> > > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > > For the Ally, since it disconnects its controller during sleep, this
> > > > caused the backlight slider to not work in KDE.
> > > >
> > > > 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: simplify RGB init sequence
> > > > HID: asus: initialize additional endpoints only for legacy devices
> > > > HID: asus: use same report_id in response
> > > > HID: asus: fortify keyboard handshake
> > > > HID: asus: move vendor initialization to probe
> > > > HID: asus: early return for ROG devices
> > > > platform/x86: asus-wmi: Add support for multiple kbd led 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
> > > >
> > > > drivers/hid/hid-asus.c | 205 ++++++++--------
> > > > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > > > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > > > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > > > 4 files changed, 322 insertions(+), 184 deletions(-)
> > > > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > > >
> > > >
> > > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > > --
> > > > 2.52.0
> > > >
> > > >
> > >
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-26 15:29 ` Antheas Kapenekakis
@ 2025-11-26 15:31 ` Ilpo Järvinen
2025-11-26 19:58 ` Denis Benato
0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2025-11-26 15:31 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
[-- Attachment #1: Type: text/plain, Size: 13042 bytes --]
On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> > >
> > > > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > > >
> > > > > This is a two part series which does the following:
> > > > > - Clean-up init sequence
> > > > > - Unify 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.
> > > > >
> > > > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > > > to make this easier to merge.
> > > >
> > > > Slight bump on this. It addresses both of the remarks Denis made in
> > > > the previous version.
> > > >
> > > > I begrudgingly argued a bit for those because I did not want to resend
> > > > the series and they were not functional changes, so sorry about that.
> > > > But they are fixed in this version incl. with the conflict with the
> > > > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > > > Add support for multiple kbd led handlers" even though I addressed
> > > > your comment, so you may want to add that.
> > >
> > > FYI, there's no direct relation that mandates a person to give a rev-by
> > > even if all his/her comments were addressed.
> >
> > True, this is just a reminder because I did not hear from him and
> > since he added a rev-by on the kfree patch. There's no obligation from
> > my side.
> >
> > > But generally yes, it would be useful to hear whether Denis is fine with
> > > v10, especially those patches that had contention earlier but you've
> > > modified post-v8.
> > >
> > > > As for "HID: asus: early return for ROG devices" changing the name of
> > > > the devices of this driver, I will veto backporting it if it happens,
> > > > so inputplumber will have the two full months to remove the name
> > > > match. This is not a breaking change in the sense that software cannot
> > > > be made to work on both previous and latter versions and there is no
> > > > other software to my knowledge relying on name matches for Asus
> > > > keyboards.
> > >
> > > Did Hans give some opinion about this rename earlier, at least I don't
> > > remember nor could find from lore archives?
> >
> > Hans jumped in on the ayaneo controller patch. I don't think I saw
> > activity on this series
>
> Hans had some feedback around half a year ago for the latter part of
> this series that binds the devices together
Yes but I was interested on specifically about this userspace visible
change. He has been around much much longer than me so his insight on
userspace visible changes is way more valuable than mine.
--
i.
>
> > Antheas
> >
> > > --
> > > i.
> > >
> > > > Moreover, an early exit is needed to prevent ejecting HID
> > > > endpoints without an ->input parameter so it is a needed fix anyway.
> > > > Postponing it will prevent Xbox Ally users from having RGB control
> > > > through userspace on a stock kernel but it is also not worth arguing
> > > > about
> > > >
> > > > It is also fine for me for this series to merge for 6.20, but I'd
> > > > rather we handle it now since there will be some turbulence for asus
> > > > users due to armoury merging so it makes sense to have this transition
> > > > once.
> > > >
> > > > > ---
> > > > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > > > >
> > > > > Changes since V9:
> > > > > - No functional changes
> > > > > - Rebase to review-ilpo-next
> > > > > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > > > "remove unused keyboard backlight quirk" + imports
> > > > > Dismiss Luke's review as this patch diverged
> > > > > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > > > more verbose
> > > > > - Use kfree in fortify patch
> > > > > - Fix minor style quirks from --nonstict checkpatch run
> > > > >
> > > > > Changes since V8:
> > > > > - No functional changes
> > > > > - Move legacy init patch to second, modify first patch so that their
> > > > > diff is minimized
> > > > > - Split "prevent binding to all HID devices on ROG" into two patches:
> > > > > - moving backlight initialization into probe
> > > > > - early exit to skip ->init check and rename
> > > > > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > > > to read usages before the report fixups are applied, so it did not work
> > > > > - In that patch, reword a comment to be single line and make is_vendor a bool
> > > > > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > > > has drifted too far since he reviewed/tested it.
> > > > >
> > > > > Changes since V7:
> > > > > - Readd legacy init quirk for Dennis
> > > > > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > > > - Fix warning due to enum_backlight receiving negative values
> > > > >
> > > > > Changes since V6:
> > > > > - Split initialization refactor into three patches, update commit text
> > > > > to be clearer in what it does
> > > > > - Replace spinlock accesses with guard and scoped guard in all patches
> > > > > - Add missing includes mentioned by Ilpo
> > > > > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > > > - Replace asus_ref.asus with local reference in all patches
> > > > > - Add missing kernel doc comments
> > > > > - Other minor nits from Ilpo
> > > > > - User reported warning due to scheduling work while holding a spinlock.
> > > > > Restructure patch for multiple handlers to limit when spinlock is held to
> > > > > variable access only. In parallel, setup a workqueue to handle registration
> > > > > of led device and setting brightness. This is required as registering the
> > > > > led device triggers kbd_led_get which needs to hold the spinlock to
> > > > > protect the led_wk value. The workqueue is also required for the hid
> > > > > event passthrough to avoid scheduling work while holding the spinlock.
> > > > > Apply the workqueue to wmi brightness buttons as well, as that was
> > > > > omitted before this series and WMI access was performed.
> > > > > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > > > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > > > This allows other quirks to apply (applies quirk that fixes keyboard
> > > > > being named as a pointer device).
> > > > >
> > > > > Changes since V5:
> > > > > - It's been a long time
> > > > > - Remove addition of RGB as that had some comments I need to work on
> > > > > - Remove folio patch (already merged)
> > > > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > > > without this patch, some old NKEY keyboards might not respond to
> > > > > RGB commands according to Luke, but the kernel driver does not do
> > > > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > > > Windows. If an issue is found, we can re-add it or just remove patches
> > > > > 1/2 before merging. However, init could use the cleanup.
> > > > >
> > > > > Changes since V4:
> > > > > - Fix KConfig (reported by kernel robot)
> > > > > - Fix Ilpo's nits, if I missed anything lmk
> > > > >
> > > > > Changes since V3:
> > > > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > > > that it is not needed for their media keys to function.
> > > > > - Cover init in asus-wmi with spinlock as per Hans
> > > > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > > > in USB Asus keyboards, per Hans.
> > > > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > > > the spin lock (but after the asus reference is set to null)
> > > > >
> > > > > Changes since V2:
> > > > > - Check lazy init succeds in asus-wmi before setting register variable
> > > > > - make explicit check in asus_hid_register_listener for listener existing
> > > > > to avoid re-init
> > > > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > > > all input vendor devices. This includes moving rgb init to probe
> > > > > instead of the input_configured callbacks.
> > > > > - Remove fan key (during retest it appears to be 0xae that is already
> > > > > supported by hid-asus)
> > > > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > > > - removes fds from userspace and breaks backlight functionality. All
> > > > > - current mainline drivers do not support backlight hotplugging, so most
> > > > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > > > For the Ally, since it disconnects its controller during sleep, this
> > > > > caused the backlight slider to not work in KDE.
> > > > >
> > > > > 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: simplify RGB init sequence
> > > > > HID: asus: initialize additional endpoints only for legacy devices
> > > > > HID: asus: use same report_id in response
> > > > > HID: asus: fortify keyboard handshake
> > > > > HID: asus: move vendor initialization to probe
> > > > > HID: asus: early return for ROG devices
> > > > > platform/x86: asus-wmi: Add support for multiple kbd led 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
> > > > >
> > > > > drivers/hid/hid-asus.c | 205 ++++++++--------
> > > > > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > > > > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > > > > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > > > > 4 files changed, 322 insertions(+), 184 deletions(-)
> > > > > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > > > >
> > > > >
> > > > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > > > --
> > > > > 2.52.0
> > > > >
> > > > >
> > > >
> > >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-26 15:23 ` Ilpo Järvinen
2025-11-26 15:28 ` Antheas Kapenekakis
@ 2025-11-26 17:34 ` Hans de Goede
2025-11-26 19:57 ` Denis Benato
1 sibling, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2025-11-26 17:34 UTC (permalink / raw)
To: Ilpo Järvinen, Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Denis Benato
Hi,
On 26-Nov-25 4:23 PM, Ilpo Järvinen wrote:
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
...
>> As for "HID: asus: early return for ROG devices" changing the name of
>> the devices of this driver, I will veto backporting it if it happens,
>> so inputplumber will have the two full months to remove the name
>> match. This is not a breaking change in the sense that software cannot
>> be made to work on both previous and latter versions and there is no
>> other software to my knowledge relying on name matches for Asus
>> keyboards.
>
> Did Hans give some opinion about this rename earlier, at least I don't
> remember nor could find from lore archives?
I don't remember commenting on this myself either.
So generally speaking there are plenty of cases where /dev/input/event#
nodes for a specific device have their name changed by some kernel patches.
Typically HID input devices are matched in userspace by their
bus:vend-id:prod-id triplet not by the name. The name might even
change by a fwupdate of the device itself.
So I'm not overly worried about this and inputplumber seems nice
enough and already is very much not a plug-and-play tool.
One possible concern with laptop keyboard input-device name changes
though is hwdb entries to fixup scancode -> ev-key-code mappings.
See: /lib/udev/hwdb.d/60-keyboard.hwdb on any standard Linux systems
an then the big comment at the top.
An input-device name change might break this match pattern:
# - Input driver device name and DMI data match:
# evdev:name:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn*
# <input device name> is the name device specified by the
# driver, <vendor> is the firmware-provided string exported
# by the kernel DMI modalias, see /sys/class/dmi/id/modalias
As well as the extended version of this and for laptops with USB
keyboards this is the only match type which allows a DMI match
which is what we want for laptop kbd mappings. Looking at the Asus
section of the upstream 60-keyboard.hwdb I do not see any such
matches though.
There not being such matches kinda make sense since for USB-HID
devices any special scancode -> ev-key-code mappings are typically
handled in a vendor specific HID driver like hid-asus.
TL;DR: I think that the input-device name should be fine.
Regards,
Hans
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-26 17:34 ` Hans de Goede
@ 2025-11-26 19:57 ` Denis Benato
0 siblings, 0 replies; 16+ messages in thread
From: Denis Benato @ 2025-11-26 19:57 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones
On 11/26/25 18:34, Hans de Goede wrote:
> Hi,
>
> On 26-Nov-25 4:23 PM, Ilpo Järvinen wrote:
>> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> ...
>
>>> As for "HID: asus: early return for ROG devices" changing the name of
>>> the devices of this driver, I will veto backporting it if it happens,
>>> so inputplumber will have the two full months to remove the name
>>> match. This is not a breaking change in the sense that software cannot
>>> be made to work on both previous and latter versions and there is no
>>> other software to my knowledge relying on name matches for Asus
>>> keyboards.
>> Did Hans give some opinion about this rename earlier, at least I don't
>> remember nor could find from lore archives?
> I don't remember commenting on this myself either.
>
> So generally speaking there are plenty of cases where /dev/input/event#
> nodes for a specific device have their name changed by some kernel patches.
>
> Typically HID input devices are matched in userspace by their
> bus:vend-id:prod-id triplet not by the name. The name might even
> change by a fwupdate of the device itself.
>
> So I'm not overly worried about this and inputplumber seems nice
> enough and already is very much not a plug-and-play tool.
>
> One possible concern with laptop keyboard input-device name changes
> though is hwdb entries to fixup scancode -> ev-key-code mappings.
>
> See: /lib/udev/hwdb.d/60-keyboard.hwdb on any standard Linux systems
> an then the big comment at the top.
>
> An input-device name change might break this match pattern:
>
> # - Input driver device name and DMI data match:
> # evdev:name:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn*
> # <input device name> is the name device specified by the
> # driver, <vendor> is the firmware-provided string exported
> # by the kernel DMI modalias, see /sys/class/dmi/id/modalias
>
> As well as the extended version of this and for laptops with USB
> keyboards this is the only match type which allows a DMI match
> which is what we want for laptop kbd mappings. Looking at the Asus
> section of the upstream 60-keyboard.hwdb I do not see any such
> matches though.
>
> There not being such matches kinda make sense since for USB-HID
> devices any special scancode -> ev-key-code mappings are typically
> handled in a vendor specific HID driver like hid-asus.
>
> TL;DR: I think that the input-device name should be fine.
Thank you very much for this write-up! Now I feel much more confident
in giving approval!
> Regards,
>
> Hans
Thank you,
Denis
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-26 15:31 ` Ilpo Järvinen
@ 2025-11-26 19:58 ` Denis Benato
0 siblings, 0 replies; 16+ messages in thread
From: Denis Benato @ 2025-11-26 19:58 UTC (permalink / raw)
To: Ilpo Järvinen, Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede
On 11/26/25 16:31, Ilpo Järvinen wrote:
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>
>> On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>> On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
>>> <ilpo.jarvinen@linux.intel.com> wrote:
>>>> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>>>>
>>>>> On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>>>> This is a two part series which does the following:
>>>>>> - Clean-up init sequence
>>>>>> - Unify 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.
>>>>>>
>>>>>> For more context, see cover letter of V1. Since V5, I removed some patches
>>>>>> to make this easier to merge.
>>>>> Slight bump on this. It addresses both of the remarks Denis made in
>>>>> the previous version.
>>>>>
>>>>> I begrudgingly argued a bit for those because I did not want to resend
>>>>> the series and they were not functional changes, so sorry about that.
>>>>> But they are fixed in this version incl. with the conflict with the
>>>>> armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
>>>>> Add support for multiple kbd led handlers" even though I addressed
>>>>> your comment, so you may want to add that.
>>>> FYI, there's no direct relation that mandates a person to give a rev-by
>>>> even if all his/her comments were addressed.
>>> True, this is just a reminder because I did not hear from him and
>>> since he added a rev-by on the kfree patch. There's no obligation from
>>> my side.
Lack of time: I will review the rest shortly unless some more unexpected
thing shows up.
>>>> But generally yes, it would be useful to hear whether Denis is fine with
>>>> v10, especially those patches that had contention earlier but you've
>>>> modified post-v8.
>>>>
>>>>> As for "HID: asus: early return for ROG devices" changing the name of
>>>>> the devices of this driver, I will veto backporting it if it happens,
>>>>> so inputplumber will have the two full months to remove the name
>>>>> match. This is not a breaking change in the sense that software cannot
>>>>> be made to work on both previous and latter versions and there is no
>>>>> other software to my knowledge relying on name matches for Asus
>>>>> keyboards.
>>>> Did Hans give some opinion about this rename earlier, at least I don't
>>>> remember nor could find from lore archives?
>>> Hans jumped in on the ayaneo controller patch. I don't think I saw
>>> activity on this series
>> Hans had some feedback around half a year ago for the latter part of
>> this series that binds the devices together
> Yes but I was interested on specifically about this userspace visible
> change. He has been around much much longer than me so his insight on
> userspace visible changes is way more valuable than mine.
>
> --
> i.
>
>>> Antheas
>>>
>>>> --
>>>> i.
>>>>
>>>>> Moreover, an early exit is needed to prevent ejecting HID
>>>>> endpoints without an ->input parameter so it is a needed fix anyway.
>>>>> Postponing it will prevent Xbox Ally users from having RGB control
>>>>> through userspace on a stock kernel but it is also not worth arguing
>>>>> about
>>>>>
>>>>> It is also fine for me for this series to merge for 6.20, but I'd
>>>>> rather we handle it now since there will be some turbulence for asus
>>>>> users due to armoury merging so it makes sense to have this transition
>>>>> once.
>>>>>
>>>>>> ---
>>>>>> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
>>>>>> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
>>>>>> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
>>>>>> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
>>>>>> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
>>>>>> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
>>>>>> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
>>>>>> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
>>>>>> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>>>>>>
>>>>>> Changes since V9:
>>>>>> - No functional changes
>>>>>> - Rebase to review-ilpo-next
>>>>>> - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
>>>>>> "remove unused keyboard backlight quirk" + imports
>>>>>> Dismiss Luke's review as this patch diverged
>>>>>> - Reword paragraph in "Add support for multiple kbd led handlers" to be
>>>>>> more verbose
>>>>>> - Use kfree in fortify patch
>>>>>> - Fix minor style quirks from --nonstict checkpatch run
>>>>>>
>>>>>> Changes since V8:
>>>>>> - No functional changes
>>>>>> - Move legacy init patch to second, modify first patch so that their
>>>>>> diff is minimized
>>>>>> - Split "prevent binding to all HID devices on ROG" into two patches:
>>>>>> - moving backlight initialization into probe
>>>>>> - early exit to skip ->init check and rename
>>>>>> - Remove skipping vendor fixups for non-vendor devices. It is not possible
>>>>>> to read usages before the report fixups are applied, so it did not work
>>>>>> - In that patch, reword a comment to be single line and make is_vendor a bool
>>>>>> - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
>>>>>> has drifted too far since he reviewed/tested it.
>>>>>>
>>>>>> Changes since V7:
>>>>>> - Readd legacy init quirk for Dennis
>>>>>> - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
>>>>>> - Fix warning due to enum_backlight receiving negative values
>>>>>>
>>>>>> Changes since V6:
>>>>>> - Split initialization refactor into three patches, update commit text
>>>>>> to be clearer in what it does
>>>>>> - Replace spinlock accesses with guard and scoped guard in all patches
>>>>>> - Add missing includes mentioned by Ilpo
>>>>>> - Reflow, tweak comment in prevent binding to all HID devices on ROG
>>>>>> - Replace asus_ref.asus with local reference in all patches
>>>>>> - Add missing kernel doc comments
>>>>>> - Other minor nits from Ilpo
>>>>>> - User reported warning due to scheduling work while holding a spinlock.
>>>>>> Restructure patch for multiple handlers to limit when spinlock is held to
>>>>>> variable access only. In parallel, setup a workqueue to handle registration
>>>>>> of led device and setting brightness. This is required as registering the
>>>>>> led device triggers kbd_led_get which needs to hold the spinlock to
>>>>>> protect the led_wk value. The workqueue is also required for the hid
>>>>>> event passthrough to avoid scheduling work while holding the spinlock.
>>>>>> Apply the workqueue to wmi brightness buttons as well, as that was
>>>>>> omitted before this series and WMI access was performed.
>>>>>> - On "HID: asus: prevent binding to all HID devices on ROG", rename
>>>>>> quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
>>>>>> This allows other quirks to apply (applies quirk that fixes keyboard
>>>>>> being named as a pointer device).
>>>>>>
>>>>>> Changes since V5:
>>>>>> - It's been a long time
>>>>>> - Remove addition of RGB as that had some comments I need to work on
>>>>>> - Remove folio patch (already merged)
>>>>>> - Remove legacy fix patch 11 from V4. There is a small chance that
>>>>>> without this patch, some old NKEY keyboards might not respond to
>>>>>> RGB commands according to Luke, but the kernel driver does not do
>>>>>> RGB currently. The 0x5d init is done by Armoury crate software in
>>>>>> Windows. If an issue is found, we can re-add it or just remove patches
>>>>>> 1/2 before merging. However, init could use the cleanup.
>>>>>>
>>>>>> Changes since V4:
>>>>>> - Fix KConfig (reported by kernel robot)
>>>>>> - Fix Ilpo's nits, if I missed anything lmk
>>>>>>
>>>>>> Changes since V3:
>>>>>> - Add initializer for 0x5d for old NKEY keyboards until it is verified
>>>>>> that it is not needed for their media keys to function.
>>>>>> - Cover init in asus-wmi with spinlock as per Hans
>>>>>> - If asus-wmi registers WMI handler with brightness, init the brightness
>>>>>> in USB Asus keyboards, per Hans.
>>>>>> - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
>>>>>> - Fix oops when unregistering asus-wmi by moving unregister outside of
>>>>>> the spin lock (but after the asus reference is set to null)
>>>>>>
>>>>>> Changes since V2:
>>>>>> - Check lazy init succeds in asus-wmi before setting register variable
>>>>>> - make explicit check in asus_hid_register_listener for listener existing
>>>>>> to avoid re-init
>>>>>> - rename asus_brt to asus_hid in most places and harmonize everything
>>>>>> - switch to a spinlock instead of a mutex to avoid kernel ooops
>>>>>> - fixup hid device quirks to avoid multiple RGB devices while still exposing
>>>>>> all input vendor devices. This includes moving rgb init to probe
>>>>>> instead of the input_configured callbacks.
>>>>>> - Remove fan key (during retest it appears to be 0xae that is already
>>>>>> supported by hid-asus)
>>>>>> - Never unregister asus::kbd_backlight while asus-wmi is active, as that
>>>>>> - removes fds from userspace and breaks backlight functionality. All
>>>>>> - current mainline drivers do not support backlight hotplugging, so most
>>>>>> userspace software (e.g., KDE, UPower) is built with that assumption.
>>>>>> For the Ally, since it disconnects its controller during sleep, this
>>>>>> caused the backlight slider to not work in KDE.
>>>>>>
>>>>>> 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: simplify RGB init sequence
>>>>>> HID: asus: initialize additional endpoints only for legacy devices
>>>>>> HID: asus: use same report_id in response
>>>>>> HID: asus: fortify keyboard handshake
>>>>>> HID: asus: move vendor initialization to probe
>>>>>> HID: asus: early return for ROG devices
>>>>>> platform/x86: asus-wmi: Add support for multiple kbd led 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
>>>>>>
>>>>>> drivers/hid/hid-asus.c | 205 ++++++++--------
>>>>>> drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
>>>>>> .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
>>>>>> include/linux/platform_data/x86/asus-wmi.h | 28 +++
>>>>>> 4 files changed, 322 insertions(+), 184 deletions(-)
>>>>>> delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>>>>>>
>>>>>>
>>>>>> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
>>>>>> --
>>>>>> 2.52.0
>>>>>>
>>>>>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-11-22 11:00 Antheas Kapenekakis
2025-11-26 13:37 ` Antheas Kapenekakis
@ 2025-12-02 11:28 ` Benjamin Tissoires
1 sibling, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2025-12-02 11:28 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Corentin Chary, Luke D . Jones, Hans de Goede, Ilpo Järvinen,
Denis Benato
On Nov 22 2025, Antheas Kapenekakis wrote:
> This is a two part series which does the following:
> - Clean-up init sequence
> - Unify 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.
>
> For more context, see cover letter of V1. Since V5, I removed some patches
> to make this easier to merge.
For the HID part:
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Again, as mentioned in one of the other patch. Feel free to take this
into the platform-x86 tree.
Cheers,
Benjamin
>
> ---
> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>
> Changes since V9:
> - No functional changes
> - Rebase to review-ilpo-next
> - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> "remove unused keyboard backlight quirk" + imports
> Dismiss Luke's review as this patch diverged
> - Reword paragraph in "Add support for multiple kbd led handlers" to be
> more verbose
> - Use kfree in fortify patch
> - Fix minor style quirks from --nonstict checkpatch run
>
> Changes since V8:
> - No functional changes
> - Move legacy init patch to second, modify first patch so that their
> diff is minimized
> - Split "prevent binding to all HID devices on ROG" into two patches:
> - moving backlight initialization into probe
> - early exit to skip ->init check and rename
> - Remove skipping vendor fixups for non-vendor devices. It is not possible
> to read usages before the report fixups are applied, so it did not work
> - In that patch, reword a comment to be single line and make is_vendor a bool
> - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> has drifted too far since he reviewed/tested it.
>
> Changes since V7:
> - Readd legacy init quirk for Dennis
> - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> - Fix warning due to enum_backlight receiving negative values
>
> Changes since V6:
> - Split initialization refactor into three patches, update commit text
> to be clearer in what it does
> - Replace spinlock accesses with guard and scoped guard in all patches
> - Add missing includes mentioned by Ilpo
> - Reflow, tweak comment in prevent binding to all HID devices on ROG
> - Replace asus_ref.asus with local reference in all patches
> - Add missing kernel doc comments
> - Other minor nits from Ilpo
> - User reported warning due to scheduling work while holding a spinlock.
> Restructure patch for multiple handlers to limit when spinlock is held to
> variable access only. In parallel, setup a workqueue to handle registration
> of led device and setting brightness. This is required as registering the
> led device triggers kbd_led_get which needs to hold the spinlock to
> protect the led_wk value. The workqueue is also required for the hid
> event passthrough to avoid scheduling work while holding the spinlock.
> Apply the workqueue to wmi brightness buttons as well, as that was
> omitted before this series and WMI access was performed.
> - On "HID: asus: prevent binding to all HID devices on ROG", rename
> quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> This allows other quirks to apply (applies quirk that fixes keyboard
> being named as a pointer device).
>
> Changes since V5:
> - It's been a long time
> - Remove addition of RGB as that had some comments I need to work on
> - Remove folio patch (already merged)
> - Remove legacy fix patch 11 from V4. There is a small chance that
> without this patch, some old NKEY keyboards might not respond to
> RGB commands according to Luke, but the kernel driver does not do
> RGB currently. The 0x5d init is done by Armoury crate software in
> Windows. If an issue is found, we can re-add it or just remove patches
> 1/2 before merging. However, init could use the cleanup.
>
> Changes since V4:
> - Fix KConfig (reported by kernel robot)
> - Fix Ilpo's nits, if I missed anything lmk
>
> Changes since V3:
> - Add initializer for 0x5d for old NKEY keyboards until it is verified
> that it is not needed for their media keys to function.
> - Cover init in asus-wmi with spinlock as per Hans
> - If asus-wmi registers WMI handler with brightness, init the brightness
> in USB Asus keyboards, per Hans.
> - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> - Fix oops when unregistering asus-wmi by moving unregister outside of
> the spin lock (but after the asus reference is set to null)
>
> Changes since V2:
> - Check lazy init succeds in asus-wmi before setting register variable
> - make explicit check in asus_hid_register_listener for listener existing
> to avoid re-init
> - rename asus_brt to asus_hid in most places and harmonize everything
> - switch to a spinlock instead of a mutex to avoid kernel ooops
> - fixup hid device quirks to avoid multiple RGB devices while still exposing
> all input vendor devices. This includes moving rgb init to probe
> instead of the input_configured callbacks.
> - Remove fan key (during retest it appears to be 0xae that is already
> supported by hid-asus)
> - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> - removes fds from userspace and breaks backlight functionality. All
> - current mainline drivers do not support backlight hotplugging, so most
> userspace software (e.g., KDE, UPower) is built with that assumption.
> For the Ally, since it disconnects its controller during sleep, this
> caused the backlight slider to not work in KDE.
>
> 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: simplify RGB init sequence
> HID: asus: initialize additional endpoints only for legacy devices
> HID: asus: use same report_id in response
> HID: asus: fortify keyboard handshake
> HID: asus: move vendor initialization to probe
> HID: asus: early return for ROG devices
> platform/x86: asus-wmi: Add support for multiple kbd led 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
>
> drivers/hid/hid-asus.c | 205 ++++++++--------
> drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> include/linux/platform_data/x86/asus-wmi.h | 28 +++
> 4 files changed, 322 insertions(+), 184 deletions(-)
> delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>
>
> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> --
> 2.52.0
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
@ 2025-12-05 22:13 Kelsios
2025-12-05 23:03 ` Antheas Kapenekakis
0 siblings, 1 reply; 16+ messages in thread
From: Kelsios @ 2025-12-05 22:13 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen, Denis Benato
Hello,
I would like to report a regression affecting keyboard backlight brightness control on my ASUS ROG Zephyrus G16 (model GU605CW).
Using kernel 6.17.9-arch1-1.1-g14 with the latest HID ASUS patchset v10, keyboard *color* control works correctly, but *brightness* control no longer responds at all. The issue is reproducible on every boot. This problem is not present when using patchset v8, where both color and brightness work as expected.
Important detail: the issue occurs even **without** asusctl installed, so it must be within the kernel HID/WMI handling and is unrelated to userspace tools.
Output of dmesg is available here [1], please let me know if any additional information is required.
Thank you for your time and work on supporting these ASUS laptops.
Best regards,
Kelsios
[1] https://pastebin.com/ZFC13Scf
On 11/22/25 1:00 PM, Antheas Kapenekakis wrote:
> This is a two part series which does the following:
> - Clean-up init sequence
> - Unify 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.
>
> For more context, see cover letter of V1. Since V5, I removed some patches
> to make this easier to merge.
>
> ---
> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>
> Changes since V9:
> - No functional changes
> - Rebase to review-ilpo-next
> - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> "remove unused keyboard backlight quirk" + imports
> Dismiss Luke's review as this patch diverged
> - Reword paragraph in "Add support for multiple kbd led handlers" to be
> more verbose
> - Use kfree in fortify patch
> - Fix minor style quirks from --nonstict checkpatch run
>
> Changes since V8:
> - No functional changes
> - Move legacy init patch to second, modify first patch so that their
> diff is minimized
> - Split "prevent binding to all HID devices on ROG" into two patches:
> - moving backlight initialization into probe
> - early exit to skip ->init check and rename
> - Remove skipping vendor fixups for non-vendor devices. It is not possible
> to read usages before the report fixups are applied, so it did not work
> - In that patch, reword a comment to be single line and make is_vendor a bool
> - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> has drifted too far since he reviewed/tested it.
>
> Changes since V7:
> - Readd legacy init quirk for Dennis
> - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> - Fix warning due to enum_backlight receiving negative values
>
> Changes since V6:
> - Split initialization refactor into three patches, update commit text
> to be clearer in what it does
> - Replace spinlock accesses with guard and scoped guard in all patches
> - Add missing includes mentioned by Ilpo
> - Reflow, tweak comment in prevent binding to all HID devices on ROG
> - Replace asus_ref.asus with local reference in all patches
> - Add missing kernel doc comments
> - Other minor nits from Ilpo
> - User reported warning due to scheduling work while holding a spinlock.
> Restructure patch for multiple handlers to limit when spinlock is held to
> variable access only. In parallel, setup a workqueue to handle registration
> of led device and setting brightness. This is required as registering the
> led device triggers kbd_led_get which needs to hold the spinlock to
> protect the led_wk value. The workqueue is also required for the hid
> event passthrough to avoid scheduling work while holding the spinlock.
> Apply the workqueue to wmi brightness buttons as well, as that was
> omitted before this series and WMI access was performed.
> - On "HID: asus: prevent binding to all HID devices on ROG", rename
> quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> This allows other quirks to apply (applies quirk that fixes keyboard
> being named as a pointer device).
>
> Changes since V5:
> - It's been a long time
> - Remove addition of RGB as that had some comments I need to work on
> - Remove folio patch (already merged)
> - Remove legacy fix patch 11 from V4. There is a small chance that
> without this patch, some old NKEY keyboards might not respond to
> RGB commands according to Luke, but the kernel driver does not do
> RGB currently. The 0x5d init is done by Armoury crate software in
> Windows. If an issue is found, we can re-add it or just remove patches
> 1/2 before merging. However, init could use the cleanup.
>
> Changes since V4:
> - Fix KConfig (reported by kernel robot)
> - Fix Ilpo's nits, if I missed anything lmk
>
> Changes since V3:
> - Add initializer for 0x5d for old NKEY keyboards until it is verified
> that it is not needed for their media keys to function.
> - Cover init in asus-wmi with spinlock as per Hans
> - If asus-wmi registers WMI handler with brightness, init the brightness
> in USB Asus keyboards, per Hans.
> - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> - Fix oops when unregistering asus-wmi by moving unregister outside of
> the spin lock (but after the asus reference is set to null)
>
> Changes since V2:
> - Check lazy init succeds in asus-wmi before setting register variable
> - make explicit check in asus_hid_register_listener for listener existing
> to avoid re-init
> - rename asus_brt to asus_hid in most places and harmonize everything
> - switch to a spinlock instead of a mutex to avoid kernel ooops
> - fixup hid device quirks to avoid multiple RGB devices while still exposing
> all input vendor devices. This includes moving rgb init to probe
> instead of the input_configured callbacks.
> - Remove fan key (during retest it appears to be 0xae that is already
> supported by hid-asus)
> - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> - removes fds from userspace and breaks backlight functionality. All
> - current mainline drivers do not support backlight hotplugging, so most
> userspace software (e.g., KDE, UPower) is built with that assumption.
> For the Ally, since it disconnects its controller during sleep, this
> caused the backlight slider to not work in KDE.
>
> 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: simplify RGB init sequence
> HID: asus: initialize additional endpoints only for legacy devices
> HID: asus: use same report_id in response
> HID: asus: fortify keyboard handshake
> HID: asus: move vendor initialization to probe
> HID: asus: early return for ROG devices
> platform/x86: asus-wmi: Add support for multiple kbd led 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
>
> drivers/hid/hid-asus.c | 205 ++++++++--------
> drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> include/linux/platform_data/x86/asus-wmi.h | 28 +++
> 4 files changed, 322 insertions(+), 184 deletions(-)
> delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>
>
> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-12-05 22:13 [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling Kelsios
@ 2025-12-05 23:03 ` Antheas Kapenekakis
2025-12-05 23:04 ` Antheas Kapenekakis
0 siblings, 1 reply; 16+ messages in thread
From: Antheas Kapenekakis @ 2025-12-05 23:03 UTC (permalink / raw)
To: Kelsios
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen, Denis Benato
On Fri, 5 Dec 2025 at 23:13, Kelsios <K3lsios@proton.me> wrote:
>
> Hello,
>
> I would like to report a regression affecting keyboard backlight brightness control on my ASUS ROG Zephyrus G16 (model GU605CW).
>
> Using kernel 6.17.9-arch1-1.1-g14 with the latest HID ASUS patchset v10, keyboard *color* control works correctly, but *brightness* control no longer responds at all. The issue is reproducible on every boot. This problem is not present when using patchset v8, where both color and brightness work as expected.
>
> Important detail: the issue occurs even **without** asusctl installed, so it must be within the kernel HID/WMI handling and is unrelated to userspace tools.
>
> Output of dmesg is available here [1], please let me know if any additional information is required.
>
> Thank you for your time and work on supporting these ASUS laptops.
>
> Best regards,
> Kelsios
>
> [1] https://pastebin.com/ZFC13Scf
[ 1.035986] asus 0003:0B05:19B6.0001: Asus failed to receive handshake ack: -32
Oh yeah, asus_kbd_init no longer works with spurious inits so it broke
devices marked with QUIRK_ROG_NKEY_LEGACY
There are three ways to approach this. One is to ignore the error...
second is to drop the quirk... third is to check for the usages for ID1, ID2...
I would tend towards dropping the ID2 init and ignoring the error for
ID1... Unless an EPIPE would cause the device to close
Antheas
> On 11/22/25 1:00 PM, Antheas Kapenekakis wrote:
> > This is a two part series which does the following:
> > - Clean-up init sequence
> > - Unify 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.
> >
> > For more context, see cover letter of V1. Since V5, I removed some patches
> > to make this easier to merge.
> >
> > ---
> > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> >
> > Changes since V9:
> > - No functional changes
> > - Rebase to review-ilpo-next
> > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > "remove unused keyboard backlight quirk" + imports
> > Dismiss Luke's review as this patch diverged
> > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > more verbose
> > - Use kfree in fortify patch
> > - Fix minor style quirks from --nonstict checkpatch run
> >
> > Changes since V8:
> > - No functional changes
> > - Move legacy init patch to second, modify first patch so that their
> > diff is minimized
> > - Split "prevent binding to all HID devices on ROG" into two patches:
> > - moving backlight initialization into probe
> > - early exit to skip ->init check and rename
> > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > to read usages before the report fixups are applied, so it did not work
> > - In that patch, reword a comment to be single line and make is_vendor a bool
> > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > has drifted too far since he reviewed/tested it.
> >
> > Changes since V7:
> > - Readd legacy init quirk for Dennis
> > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > - Fix warning due to enum_backlight receiving negative values
> >
> > Changes since V6:
> > - Split initialization refactor into three patches, update commit text
> > to be clearer in what it does
> > - Replace spinlock accesses with guard and scoped guard in all patches
> > - Add missing includes mentioned by Ilpo
> > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > - Replace asus_ref.asus with local reference in all patches
> > - Add missing kernel doc comments
> > - Other minor nits from Ilpo
> > - User reported warning due to scheduling work while holding a spinlock.
> > Restructure patch for multiple handlers to limit when spinlock is held to
> > variable access only. In parallel, setup a workqueue to handle registration
> > of led device and setting brightness. This is required as registering the
> > led device triggers kbd_led_get which needs to hold the spinlock to
> > protect the led_wk value. The workqueue is also required for the hid
> > event passthrough to avoid scheduling work while holding the spinlock.
> > Apply the workqueue to wmi brightness buttons as well, as that was
> > omitted before this series and WMI access was performed.
> > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > This allows other quirks to apply (applies quirk that fixes keyboard
> > being named as a pointer device).
> >
> > Changes since V5:
> > - It's been a long time
> > - Remove addition of RGB as that had some comments I need to work on
> > - Remove folio patch (already merged)
> > - Remove legacy fix patch 11 from V4. There is a small chance that
> > without this patch, some old NKEY keyboards might not respond to
> > RGB commands according to Luke, but the kernel driver does not do
> > RGB currently. The 0x5d init is done by Armoury crate software in
> > Windows. If an issue is found, we can re-add it or just remove patches
> > 1/2 before merging. However, init could use the cleanup.
> >
> > Changes since V4:
> > - Fix KConfig (reported by kernel robot)
> > - Fix Ilpo's nits, if I missed anything lmk
> >
> > Changes since V3:
> > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > that it is not needed for their media keys to function.
> > - Cover init in asus-wmi with spinlock as per Hans
> > - If asus-wmi registers WMI handler with brightness, init the brightness
> > in USB Asus keyboards, per Hans.
> > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > the spin lock (but after the asus reference is set to null)
> >
> > Changes since V2:
> > - Check lazy init succeds in asus-wmi before setting register variable
> > - make explicit check in asus_hid_register_listener for listener existing
> > to avoid re-init
> > - rename asus_brt to asus_hid in most places and harmonize everything
> > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > all input vendor devices. This includes moving rgb init to probe
> > instead of the input_configured callbacks.
> > - Remove fan key (during retest it appears to be 0xae that is already
> > supported by hid-asus)
> > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > - removes fds from userspace and breaks backlight functionality. All
> > - current mainline drivers do not support backlight hotplugging, so most
> > userspace software (e.g., KDE, UPower) is built with that assumption.
> > For the Ally, since it disconnects its controller during sleep, this
> > caused the backlight slider to not work in KDE.
> >
> > 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: simplify RGB init sequence
> > HID: asus: initialize additional endpoints only for legacy devices
> > HID: asus: use same report_id in response
> > HID: asus: fortify keyboard handshake
> > HID: asus: move vendor initialization to probe
> > HID: asus: early return for ROG devices
> > platform/x86: asus-wmi: Add support for multiple kbd led 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
> >
> > drivers/hid/hid-asus.c | 205 ++++++++--------
> > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > 4 files changed, 322 insertions(+), 184 deletions(-)
> > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> >
> >
> > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-12-05 23:03 ` Antheas Kapenekakis
@ 2025-12-05 23:04 ` Antheas Kapenekakis
2025-12-09 9:17 ` Ilpo Järvinen
0 siblings, 1 reply; 16+ messages in thread
From: Antheas Kapenekakis @ 2025-12-05 23:04 UTC (permalink / raw)
To: Kelsios
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen, Denis Benato
On Sat, 6 Dec 2025 at 00:03, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Fri, 5 Dec 2025 at 23:13, Kelsios <K3lsios@proton.me> wrote:
> >
> > Hello,
> >
> > I would like to report a regression affecting keyboard backlight brightness control on my ASUS ROG Zephyrus G16 (model GU605CW).
> >
> > Using kernel 6.17.9-arch1-1.1-g14 with the latest HID ASUS patchset v10, keyboard *color* control works correctly, but *brightness* control no longer responds at all. The issue is reproducible on every boot. This problem is not present when using patchset v8, where both color and brightness work as expected.
> >
> > Important detail: the issue occurs even **without** asusctl installed, so it must be within the kernel HID/WMI handling and is unrelated to userspace tools.
> >
> > Output of dmesg is available here [1], please let me know if any additional information is required.
> >
> > Thank you for your time and work on supporting these ASUS laptops.
> >
> > Best regards,
> > Kelsios
> >
> > [1] https://pastebin.com/ZFC13Scf
>
> [ 1.035986] asus 0003:0B05:19B6.0001: Asus failed to receive handshake ack: -32
>
> Oh yeah, asus_kbd_init no longer works with spurious inits so it broke
> devices marked with QUIRK_ROG_NKEY_LEGACY
>
> There are three ways to approach this. One is to ignore the error...
> second is to drop the quirk... third is to check for the usages for ID1, ID2...
>
> I would tend towards dropping the ID2 init and ignoring the error for
> ID1... Unless an EPIPE would cause the device to close
Benjamin correctly caught the deviation
> Antheas
>
> > On 11/22/25 1:00 PM, Antheas Kapenekakis wrote:
> > > This is a two part series which does the following:
> > > - Clean-up init sequence
> > > - Unify 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.
> > >
> > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > to make this easier to merge.
> > >
> > > ---
> > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > >
> > > Changes since V9:
> > > - No functional changes
> > > - Rebase to review-ilpo-next
> > > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > "remove unused keyboard backlight quirk" + imports
> > > Dismiss Luke's review as this patch diverged
> > > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > more verbose
> > > - Use kfree in fortify patch
> > > - Fix minor style quirks from --nonstict checkpatch run
> > >
> > > Changes since V8:
> > > - No functional changes
> > > - Move legacy init patch to second, modify first patch so that their
> > > diff is minimized
> > > - Split "prevent binding to all HID devices on ROG" into two patches:
> > > - moving backlight initialization into probe
> > > - early exit to skip ->init check and rename
> > > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > to read usages before the report fixups are applied, so it did not work
> > > - In that patch, reword a comment to be single line and make is_vendor a bool
> > > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > has drifted too far since he reviewed/tested it.
> > >
> > > Changes since V7:
> > > - Readd legacy init quirk for Dennis
> > > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > - Fix warning due to enum_backlight receiving negative values
> > >
> > > Changes since V6:
> > > - Split initialization refactor into three patches, update commit text
> > > to be clearer in what it does
> > > - Replace spinlock accesses with guard and scoped guard in all patches
> > > - Add missing includes mentioned by Ilpo
> > > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > - Replace asus_ref.asus with local reference in all patches
> > > - Add missing kernel doc comments
> > > - Other minor nits from Ilpo
> > > - User reported warning due to scheduling work while holding a spinlock.
> > > Restructure patch for multiple handlers to limit when spinlock is held to
> > > variable access only. In parallel, setup a workqueue to handle registration
> > > of led device and setting brightness. This is required as registering the
> > > led device triggers kbd_led_get which needs to hold the spinlock to
> > > protect the led_wk value. The workqueue is also required for the hid
> > > event passthrough to avoid scheduling work while holding the spinlock.
> > > Apply the workqueue to wmi brightness buttons as well, as that was
> > > omitted before this series and WMI access was performed.
> > > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > This allows other quirks to apply (applies quirk that fixes keyboard
> > > being named as a pointer device).
> > >
> > > Changes since V5:
> > > - It's been a long time
> > > - Remove addition of RGB as that had some comments I need to work on
> > > - Remove folio patch (already merged)
> > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > without this patch, some old NKEY keyboards might not respond to
> > > RGB commands according to Luke, but the kernel driver does not do
> > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > Windows. If an issue is found, we can re-add it or just remove patches
> > > 1/2 before merging. However, init could use the cleanup.
> > >
> > > Changes since V4:
> > > - Fix KConfig (reported by kernel robot)
> > > - Fix Ilpo's nits, if I missed anything lmk
> > >
> > > Changes since V3:
> > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > that it is not needed for their media keys to function.
> > > - Cover init in asus-wmi with spinlock as per Hans
> > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > in USB Asus keyboards, per Hans.
> > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > the spin lock (but after the asus reference is set to null)
> > >
> > > Changes since V2:
> > > - Check lazy init succeds in asus-wmi before setting register variable
> > > - make explicit check in asus_hid_register_listener for listener existing
> > > to avoid re-init
> > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > all input vendor devices. This includes moving rgb init to probe
> > > instead of the input_configured callbacks.
> > > - Remove fan key (during retest it appears to be 0xae that is already
> > > supported by hid-asus)
> > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > - removes fds from userspace and breaks backlight functionality. All
> > > - current mainline drivers do not support backlight hotplugging, so most
> > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > For the Ally, since it disconnects its controller during sleep, this
> > > caused the backlight slider to not work in KDE.
> > >
> > > 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: simplify RGB init sequence
> > > HID: asus: initialize additional endpoints only for legacy devices
> > > HID: asus: use same report_id in response
> > > HID: asus: fortify keyboard handshake
> > > HID: asus: move vendor initialization to probe
> > > HID: asus: early return for ROG devices
> > > platform/x86: asus-wmi: Add support for multiple kbd led 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
> > >
> > > drivers/hid/hid-asus.c | 205 ++++++++--------
> > > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > > 4 files changed, 322 insertions(+), 184 deletions(-)
> > > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > >
> > >
> > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> >
> >
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-12-05 23:04 ` Antheas Kapenekakis
@ 2025-12-09 9:17 ` Ilpo Järvinen
2025-12-09 9:49 ` Antheas Kapenekakis
0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2025-12-09 9:17 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Kelsios, platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
On Sat, 6 Dec 2025, Antheas Kapenekakis wrote:
> On Sat, 6 Dec 2025 at 00:03, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > On Fri, 5 Dec 2025 at 23:13, Kelsios <K3lsios@proton.me> wrote:
> > >
> > > Hello,
> > >
> > > I would like to report a regression affecting keyboard backlight brightness control on my ASUS ROG Zephyrus G16 (model GU605CW).
> > >
> > > Using kernel 6.17.9-arch1-1.1-g14 with the latest HID ASUS patchset v10, keyboard *color* control works correctly, but *brightness* control no longer responds at all. The issue is reproducible on every boot. This problem is not present when using patchset v8, where both color and brightness work as expected.
> > >
> > > Important detail: the issue occurs even **without** asusctl installed, so it must be within the kernel HID/WMI handling and is unrelated to userspace tools.
> > >
> > > Output of dmesg is available here [1], please let me know if any additional information is required.
> > >
> > > Thank you for your time and work on supporting these ASUS laptops.
> > >
> > > Best regards,
> > > Kelsios
> > >
> > > [1] https://pastebin.com/ZFC13Scf
> >
> > [ 1.035986] asus 0003:0B05:19B6.0001: Asus failed to receive handshake ack: -32
> >
> > Oh yeah, asus_kbd_init no longer works with spurious inits so it broke
> > devices marked with QUIRK_ROG_NKEY_LEGACY
> >
> > There are three ways to approach this. One is to ignore the error...
> > second is to drop the quirk... third is to check for the usages for ID1, ID2...
> >
> > I would tend towards dropping the ID2 init and ignoring the error for
> > ID1... Unless an EPIPE would cause the device to close
>
> Benjamin correctly caught the deviation
BTW, we want to record this knowledge also into the changelog so that the
next person who'd want to make the check stricter does not need to guess
whether it was based on a real observed problem or mere guessing there
could be a problem.
--
i.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-12-09 9:17 ` Ilpo Järvinen
@ 2025-12-09 9:49 ` Antheas Kapenekakis
2025-12-10 17:19 ` Kelsios
0 siblings, 1 reply; 16+ messages in thread
From: Antheas Kapenekakis @ 2025-12-09 9:49 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Kelsios, platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
On Tue, 9 Dec 2025 at 10:17, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Sat, 6 Dec 2025, Antheas Kapenekakis wrote:
>
> > On Sat, 6 Dec 2025 at 00:03, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > >
> > > On Fri, 5 Dec 2025 at 23:13, Kelsios <K3lsios@proton.me> wrote:
> > > >
> > > > Hello,
> > > >
> > > > I would like to report a regression affecting keyboard backlight brightness control on my ASUS ROG Zephyrus G16 (model GU605CW).
> > > >
> > > > Using kernel 6.17.9-arch1-1.1-g14 with the latest HID ASUS patchset v10, keyboard *color* control works correctly, but *brightness* control no longer responds at all. The issue is reproducible on every boot. This problem is not present when using patchset v8, where both color and brightness work as expected.
> > > >
> > > > Important detail: the issue occurs even **without** asusctl installed, so it must be within the kernel HID/WMI handling and is unrelated to userspace tools.
> > > >
> > > > Output of dmesg is available here [1], please let me know if any additional information is required.
> > > >
> > > > Thank you for your time and work on supporting these ASUS laptops.
> > > >
> > > > Best regards,
> > > > Kelsios
> > > >
> > > > [1] https://pastebin.com/ZFC13Scf
> > >
> > > [ 1.035986] asus 0003:0B05:19B6.0001: Asus failed to receive handshake ack: -32
> > >
> > > Oh yeah, asus_kbd_init no longer works with spurious inits so it broke
> > > devices marked with QUIRK_ROG_NKEY_LEGACY
> > >
> > > There are three ways to approach this. One is to ignore the error...
> > > second is to drop the quirk... third is to check for the usages for ID1, ID2...
> > >
> > > I would tend towards dropping the ID2 init and ignoring the error for
> > > ID1... Unless an EPIPE would cause the device to close
> >
> > Benjamin correctly caught the deviation
>
> BTW, we want to record this knowledge also into the changelog so that the
> next person who'd want to make the check stricter does not need to guess
> whether it was based on a real observed problem or mere guessing there
> could be a problem.
If we keep the spurious inits, the stricter check will catch them and
throw errors. This is problematic.
Kelsios, you have a device that allegedly would not work without those
inits. Perhaps you could try removing the legacy quirk from your
device and see if everything is ok?
If it is, then we have a tested device and a case for removing the
legacy quirk altogether
Antheas
> --
> i.
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
2025-12-09 9:49 ` Antheas Kapenekakis
@ 2025-12-10 17:19 ` Kelsios
0 siblings, 0 replies; 16+ messages in thread
From: Kelsios @ 2025-12-10 17:19 UTC (permalink / raw)
To: Antheas Kapenekakis, Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
On 12/9/25 11:49 AM, Antheas Kapenekakis wrote:
> On Tue, 9 Dec 2025 at 10:17, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
>>
>> On Sat, 6 Dec 2025, Antheas Kapenekakis wrote:
>>
>>> On Sat, 6 Dec 2025 at 00:03, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>>
>>>> On Fri, 5 Dec 2025 at 23:13, Kelsios <K3lsios@proton.me> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> I would like to report a regression affecting keyboard backlight brightness control on my ASUS ROG Zephyrus G16 (model GU605CW).
>>>>>
>>>>> Using kernel 6.17.9-arch1-1.1-g14 with the latest HID ASUS patchset v10, keyboard *color* control works correctly, but *brightness* control no longer responds at all. The issue is reproducible on every boot. This problem is not present when using patchset v8, where both color and brightness work as expected.
>>>>>
>>>>> Important detail: the issue occurs even **without** asusctl installed, so it must be within the kernel HID/WMI handling and is unrelated to userspace tools.
>>>>>
>>>>> Output of dmesg is available here [1], please let me know if any additional information is required.
>>>>>
>>>>> Thank you for your time and work on supporting these ASUS laptops.
>>>>>
>>>>> Best regards,
>>>>> Kelsios
>>>>>
>>>>> [1] https://pastebin.com/ZFC13Scf
>>>>
>>>> [ 1.035986] asus 0003:0B05:19B6.0001: Asus failed to receive handshake ack: -32
>>>>
>>>> Oh yeah, asus_kbd_init no longer works with spurious inits so it broke
>>>> devices marked with QUIRK_ROG_NKEY_LEGACY
>>>>
>>>> There are three ways to approach this. One is to ignore the error...
>>>> second is to drop the quirk... third is to check for the usages for ID1, ID2...
>>>>
>>>> I would tend towards dropping the ID2 init and ignoring the error for
>>>> ID1... Unless an EPIPE would cause the device to close
>>>
>>> Benjamin correctly caught the deviation
>>
>> BTW, we want to record this knowledge also into the changelog so that the
>> next person who'd want to make the check stricter does not need to guess
>> whether it was based on a real observed problem or mere guessing there
>> could be a problem.
>
> If we keep the spurious inits, the stricter check will catch them and
> throw errors. This is problematic.
>
> Kelsios, you have a device that allegedly would not work without those
> inits. Perhaps you could try removing the legacy quirk from your
> device and see if everything is ok?
>
> If it is, then we have a tested device and a case for removing the
> legacy quirk altogether
>
> Antheas
>
>> --
>> i.
>>
>>
>
Hello,
I was able to narrow it down while testing linux-next with the v10 HID ASUS patchset.
Just like you mentioned in the previous email, on this machine the ID2 initialization returns a negative value. Though, when I comment out the two lines that return early after the FEATURE_KBD_LED_REPORT_ID2 init call, brightness control starts working normally again, even after sending the LED reports.
Patchset v8 did not show this behavior.
Best regards,
Kelsios
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-12-10 17:19 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-05 22:13 [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling Kelsios
2025-12-05 23:03 ` Antheas Kapenekakis
2025-12-05 23:04 ` Antheas Kapenekakis
2025-12-09 9:17 ` Ilpo Järvinen
2025-12-09 9:49 ` Antheas Kapenekakis
2025-12-10 17:19 ` Kelsios
-- strict thread matches above, loose matches on Subject: below --
2025-11-22 11:00 Antheas Kapenekakis
2025-11-26 13:37 ` Antheas Kapenekakis
2025-11-26 15:23 ` Ilpo Järvinen
2025-11-26 15:28 ` Antheas Kapenekakis
2025-11-26 15:29 ` Antheas Kapenekakis
2025-11-26 15:31 ` Ilpo Järvinen
2025-11-26 19:58 ` Denis Benato
2025-11-26 17:34 ` Hans de Goede
2025-11-26 19:57 ` Denis Benato
2025-12-02 11:28 ` Benjamin Tissoires
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).