* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-16 12:14 UTC (permalink / raw)
To: Denis Benato
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen
In-Reply-To: <160c3adf-9333-4486-ba4c-d3359ea73337@gmail.com>
On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/13/25 22:15, 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.
> >
> > All comments with these patches had been addressed since V4.
> I have loaded this patchset for users of asus-linux project to try out.
>
> One of them opened a bug report about a kernel bug that happens
> consistently when closing the lid of his laptop [1].
>
> He also sent another piece of kernel log, but didn't specify anything more
> about this [2].
>
> [1] https://pastebin.com/akZx1w10
> [2] https://pastebin.com/sKdczPgf
Can you provide a link to the bug report? [2] seems unrelated.
As for [1], it looks like a trace that stems from a sysfs write to
brightness stemming from userspace that follows the same chain it
would on a stock kernel and times out. Is it present on a stock
kernel?
Ilpo should know more about this, could the spinlock be interfering?
My testing on devices that have WMI led controls is a bit limited
unfortunately. However, most of our asus users have been happy with
this series for around half a year now.
Antheas
> > ---
> > 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 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 (7):
> > HID: asus: refactor init sequence per spec
> > HID: asus: prevent binding to all HID devices on ROG
> > platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> > HID: asus: listen to the asus-wmi brightness device instead of
> > creating one
> > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > platform/x86: asus-wmi: add keyboard brightness event handler
> > HID: asus: add support for the asus-wmi brightness handler
> >
> > drivers/hid/hid-asus.c | 235 +++++++++++----------
> > drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> > include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> > 3 files changed, 291 insertions(+), 170 deletions(-)
> >
> >
> > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
>
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-10-16 12:19 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen
In-Reply-To: <CAGwozwGzOQ-LCk6B202-CuKq=gepn6Mt4LitJJZ7dfMLaDVs7Q@mail.gmail.com>
On 10/16/25 14:14, Antheas Kapenekakis wrote:
> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 10/13/25 22:15, 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.
>>>
>>> All comments with these patches had been addressed since V4.
>> I have loaded this patchset for users of asus-linux project to try out.
>>
>> One of them opened a bug report about a kernel bug that happens
>> consistently when closing the lid of his laptop [1].
>>
>> He also sent another piece of kernel log, but didn't specify anything more
>> about this [2].
>>
>> [1] https://pastebin.com/akZx1w10
>> [2] https://pastebin.com/sKdczPgf
> Can you provide a link to the bug report? [2] seems unrelated.
The log in [2] was posted without additional context in the same
discord message as [1].
I think I will tell the user to open a proper bug report because
I do agree on the fact that it's looking unrelated.
> As for [1], it looks like a trace that stems from a sysfs write to
> brightness stemming from userspace that follows the same chain it
> would on a stock kernel and times out. Is it present on a stock
> kernel?
I have asked more details to the user. The user is not online ATM
so I will get to you with more details when I can.
> Ilpo should know more about this, could the spinlock be interfering?
> My testing on devices that have WMI led controls is a bit limited
> unfortunately. However, most of our asus users have been happy with
> this series for around half a year now.
Unless they have looked to kernel logs they won't be able to tell
since apparently there are no visible consequences.
> Antheas
>
>>> ---
>>> 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 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 (7):
>>> HID: asus: refactor init sequence per spec
>>> HID: asus: prevent binding to all HID devices on ROG
>>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
>>> HID: asus: listen to the asus-wmi brightness device instead of
>>> creating one
>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
>>> platform/x86: asus-wmi: add keyboard brightness event handler
>>> HID: asus: add support for the asus-wmi brightness handler
>>>
>>> drivers/hid/hid-asus.c | 235 +++++++++++----------
>>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
>>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
>>> 3 files changed, 291 insertions(+), 170 deletions(-)
>>>
>>>
>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-16 12:28 UTC (permalink / raw)
To: Denis Benato
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen
In-Reply-To: <ce8cc332-54ec-4e12-aa7c-a6d5e2b4fa9d@gmail.com>
On Thu, 16 Oct 2025 at 14:19, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/16/25 14:14, Antheas Kapenekakis wrote:
> > On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >> On 10/13/25 22:15, 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.
> >>>
> >>> All comments with these patches had been addressed since V4.
> >> I have loaded this patchset for users of asus-linux project to try out.
> >>
> >> One of them opened a bug report about a kernel bug that happens
> >> consistently when closing the lid of his laptop [1].
> >>
> >> He also sent another piece of kernel log, but didn't specify anything more
> >> about this [2].
> >>
> >> [1] https://pastebin.com/akZx1w10
> >> [2] https://pastebin.com/sKdczPgf
> > Can you provide a link to the bug report? [2] seems unrelated.
> The log in [2] was posted without additional context in the same
> discord message as [1].
Link me the kernel sources. Is it linux-g14 in the AUR?
> I think I will tell the user to open a proper bug report because
> I do agree on the fact that it's looking unrelated.
> > As for [1], it looks like a trace that stems from a sysfs write to
> > brightness stemming from userspace that follows the same chain it
> > would on a stock kernel and times out. Is it present on a stock
> > kernel?
> I have asked more details to the user. The user is not online ATM
> so I will get to you with more details when I can.
> > Ilpo should know more about this, could the spinlock be interfering?
> > My testing on devices that have WMI led controls is a bit limited
> > unfortunately. However, most of our asus users have been happy with
> > this series for around half a year now.
> Unless they have looked to kernel logs they won't be able to tell
> since apparently there are no visible consequences.
> > Antheas
> >
> >>> ---
> >>> 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 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 (7):
> >>> HID: asus: refactor init sequence per spec
> >>> HID: asus: prevent binding to all HID devices on ROG
> >>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> >>> HID: asus: listen to the asus-wmi brightness device instead of
> >>> creating one
> >>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
> >>> platform/x86: asus-wmi: add keyboard brightness event handler
> >>> HID: asus: add support for the asus-wmi brightness handler
> >>>
> >>> drivers/hid/hid-asus.c | 235 +++++++++++----------
> >>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> >>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> >>> 3 files changed, 291 insertions(+), 170 deletions(-)
> >>>
> >>>
> >>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
>
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-10-16 12:46 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen
In-Reply-To: <CAGwozwHrWxxE_vyswe39W=ui8N6ej4ZPFvuKVueyw4xLL8C4ZQ@mail.gmail.com>
On 10/16/25 14:28, Antheas Kapenekakis wrote:
> On Thu, 16 Oct 2025 at 14:19, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 10/16/25 14:14, Antheas Kapenekakis wrote:
>>> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
>>>> On 10/13/25 22:15, 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.
>>>>>
>>>>> All comments with these patches had been addressed since V4.
>>>> I have loaded this patchset for users of asus-linux project to try out.
>>>>
>>>> One of them opened a bug report about a kernel bug that happens
>>>> consistently when closing the lid of his laptop [1].
>>>>
>>>> He also sent another piece of kernel log, but didn't specify anything more
>>>> about this [2].
>>>>
>>>> [1] https://pastebin.com/akZx1w10
>>>> [2] https://pastebin.com/sKdczPgf
>>> Can you provide a link to the bug report? [2] seems unrelated.
>> The log in [2] was posted without additional context in the same
>> discord message as [1].
> Link me the kernel sources. Is it linux-g14 in the AUR?
Someone has replicated it on the AUR but it's just an out-of-sync replica.
The true source code is here:
https://gitlab.com/asus-linux/linux-g14 branch 6.17
>> I think I will tell the user to open a proper bug report because
>> I do agree on the fact that it's looking unrelated.
>>> As for [1], it looks like a trace that stems from a sysfs write to
>>> brightness stemming from userspace that follows the same chain it
>>> would on a stock kernel and times out. Is it present on a stock
>>> kernel?
>> I have asked more details to the user. The user is not online ATM
>> so I will get to you with more details when I can.
>>> Ilpo should know more about this, could the spinlock be interfering?
>>> My testing on devices that have WMI led controls is a bit limited
>>> unfortunately. However, most of our asus users have been happy with
>>> this series for around half a year now.
>> Unless they have looked to kernel logs they won't be able to tell
>> since apparently there are no visible consequences.
>>> Antheas
>>>
>>>>> ---
>>>>> 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 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 (7):
>>>>> HID: asus: refactor init sequence per spec
>>>>> HID: asus: prevent binding to all HID devices on ROG
>>>>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
>>>>> HID: asus: listen to the asus-wmi brightness device instead of
>>>>> creating one
>>>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
>>>>> platform/x86: asus-wmi: add keyboard brightness event handler
>>>>> HID: asus: add support for the asus-wmi brightness handler
>>>>>
>>>>> drivers/hid/hid-asus.c | 235 +++++++++++----------
>>>>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
>>>>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
>>>>> 3 files changed, 291 insertions(+), 170 deletions(-)
>>>>>
>>>>>
>>>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-16 12:51 UTC (permalink / raw)
To: Denis Benato
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen
In-Reply-To: <13c53469-58fd-462a-a462-626975d6055f@gmail.com>
On Thu, 16 Oct 2025 at 14:46, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/16/25 14:28, Antheas Kapenekakis wrote:
> > On Thu, 16 Oct 2025 at 14:19, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >> On 10/16/25 14:14, Antheas Kapenekakis wrote:
> >>> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>> On 10/13/25 22:15, 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.
> >>>>>
> >>>>> All comments with these patches had been addressed since V4.
> >>>> I have loaded this patchset for users of asus-linux project to try out.
> >>>>
> >>>> One of them opened a bug report about a kernel bug that happens
> >>>> consistently when closing the lid of his laptop [1].
> >>>>
> >>>> He also sent another piece of kernel log, but didn't specify anything more
> >>>> about this [2].
> >>>>
> >>>> [1] https://pastebin.com/akZx1w10
> >>>> [2] https://pastebin.com/sKdczPgf
> >>> Can you provide a link to the bug report? [2] seems unrelated.
> >> The log in [2] was posted without additional context in the same
> >> discord message as [1].
> > Link me the kernel sources. Is it linux-g14 in the AUR?
> Someone has replicated it on the AUR but it's just an out-of-sync replica.
>
> The true source code is here:
> https://gitlab.com/asus-linux/linux-g14 branch 6.17
Ok, lets wait for the user to replicate on a stock kernel
> >> I think I will tell the user to open a proper bug report because
> >> I do agree on the fact that it's looking unrelated.
> >>> As for [1], it looks like a trace that stems from a sysfs write to
> >>> brightness stemming from userspace that follows the same chain it
> >>> would on a stock kernel and times out. Is it present on a stock
> >>> kernel?
> >> I have asked more details to the user. The user is not online ATM
> >> so I will get to you with more details when I can.
> >>> Ilpo should know more about this, could the spinlock be interfering?
> >>> My testing on devices that have WMI led controls is a bit limited
> >>> unfortunately. However, most of our asus users have been happy with
> >>> this series for around half a year now.
> >> Unless they have looked to kernel logs they won't be able to tell
> >> since apparently there are no visible consequences.
> >>> Antheas
> >>>
> >>>>> ---
> >>>>> 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 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 (7):
> >>>>> HID: asus: refactor init sequence per spec
> >>>>> HID: asus: prevent binding to all HID devices on ROG
> >>>>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> >>>>> HID: asus: listen to the asus-wmi brightness device instead of
> >>>>> creating one
> >>>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
> >>>>> platform/x86: asus-wmi: add keyboard brightness event handler
> >>>>> HID: asus: add support for the asus-wmi brightness handler
> >>>>>
> >>>>> drivers/hid/hid-asus.c | 235 +++++++++++----------
> >>>>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> >>>>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> >>>>> 3 files changed, 291 insertions(+), 170 deletions(-)
> >>>>>
> >>>>>
> >>>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
>
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-10-16 14:32 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen, fooqux
In-Reply-To: <CAGwozwHmTUb3Bcbn9Zc44sqe7DHtnnk0chvhjN7jJNrmePr8fw@mail.gmail.com>
On 10/16/25 14:51, Antheas Kapenekakis wrote:
> On Thu, 16 Oct 2025 at 14:46, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 10/16/25 14:28, Antheas Kapenekakis wrote:
>>> On Thu, 16 Oct 2025 at 14:19, Denis Benato <benato.denis96@gmail.com> wrote:
>>>> On 10/16/25 14:14, Antheas Kapenekakis wrote:
>>>>> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>>> On 10/13/25 22:15, 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.
>>>>>>>
>>>>>>> All comments with these patches had been addressed since V4.
>>>>>> I have loaded this patchset for users of asus-linux project to try out.
>>>>>>
>>>>>> One of them opened a bug report about a kernel bug that happens
>>>>>> consistently when closing the lid of his laptop [1].
>>>>>>
>>>>>> He also sent another piece of kernel log, but didn't specify anything more
>>>>>> about this [2].
>>>>>>
>>>>>> [1] https://pastebin.com/akZx1w10
>>>>>> [2] https://pastebin.com/sKdczPgf
>>>>> Can you provide a link to the bug report? [2] seems unrelated.
>>>> The log in [2] was posted without additional context in the same
>>>> discord message as [1].
>>> Link me the kernel sources. Is it linux-g14 in the AUR?
>> Someone has replicated it on the AUR but it's just an out-of-sync replica.
>>
>> The true source code is here:
>> https://gitlab.com/asus-linux/linux-g14 branch 6.17
> Ok, lets wait for the user to replicate on a stock kernel
>
User said "yes I just confirmed it: it is only on the asus kernel, mainline doesn't have this issue".
With "asus kernel" he is referring to -g14.
I added him in CC.
>>>> I think I will tell the user to open a proper bug report because
>>>> I do agree on the fact that it's looking unrelated.
>>>>> As for [1], it looks like a trace that stems from a sysfs write to
>>>>> brightness stemming from userspace that follows the same chain it
>>>>> would on a stock kernel and times out. Is it present on a stock
>>>>> kernel?
>>>> I have asked more details to the user. The user is not online ATM
>>>> so I will get to you with more details when I can.
>>>>> Ilpo should know more about this, could the spinlock be interfering?
>>>>> My testing on devices that have WMI led controls is a bit limited
>>>>> unfortunately. However, most of our asus users have been happy with
>>>>> this series for around half a year now.
>>>> Unless they have looked to kernel logs they won't be able to tell
>>>> since apparently there are no visible consequences.
>>>>> Antheas
>>>>>
>>>>>>> ---
>>>>>>> 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 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 (7):
>>>>>>> HID: asus: refactor init sequence per spec
>>>>>>> HID: asus: prevent binding to all HID devices on ROG
>>>>>>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
>>>>>>> HID: asus: listen to the asus-wmi brightness device instead of
>>>>>>> creating one
>>>>>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
>>>>>>> platform/x86: asus-wmi: add keyboard brightness event handler
>>>>>>> HID: asus: add support for the asus-wmi brightness handler
>>>>>>>
>>>>>>> drivers/hid/hid-asus.c | 235 +++++++++++----------
>>>>>>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
>>>>>>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
>>>>>>> 3 files changed, 291 insertions(+), 170 deletions(-)
>>>>>>>
>>>>>>>
>>>>>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-16 14:44 UTC (permalink / raw)
To: Denis Benato
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen, fooqux
In-Reply-To: <f43f4f0e-9b98-4629-872e-b623bfc9b6b1@gmail.com>
On Thu, 16 Oct 2025 at 16:32, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/16/25 14:51, Antheas Kapenekakis wrote:
> > On Thu, 16 Oct 2025 at 14:46, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >> On 10/16/25 14:28, Antheas Kapenekakis wrote:
> >>> On Thu, 16 Oct 2025 at 14:19, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>> On 10/16/25 14:14, Antheas Kapenekakis wrote:
> >>>>> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>>>> On 10/13/25 22:15, 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.
> >>>>>>>
> >>>>>>> All comments with these patches had been addressed since V4.
> >>>>>> I have loaded this patchset for users of asus-linux project to try out.
> >>>>>>
> >>>>>> One of them opened a bug report about a kernel bug that happens
> >>>>>> consistently when closing the lid of his laptop [1].
> >>>>>>
> >>>>>> He also sent another piece of kernel log, but didn't specify anything more
> >>>>>> about this [2].
> >>>>>>
> >>>>>> [1] https://pastebin.com/akZx1w10
> >>>>>> [2] https://pastebin.com/sKdczPgf
> >>>>> Can you provide a link to the bug report? [2] seems unrelated.
> >>>> The log in [2] was posted without additional context in the same
> >>>> discord message as [1].
> >>> Link me the kernel sources. Is it linux-g14 in the AUR?
> >> Someone has replicated it on the AUR but it's just an out-of-sync replica.
> >>
> >> The true source code is here:
> >> https://gitlab.com/asus-linux/linux-g14 branch 6.17
> > Ok, lets wait for the user to replicate on a stock kernel
> >
> User said "yes I just confirmed it: it is only on the asus kernel, mainline doesn't have this issue".
>
> With "asus kernel" he is referring to -g14.
>
> I added him in CC.
If possible, try the bazzite kernel, its linux-bazzite-bin. It has
this series + another older armoury series. If it still happens with
that, we can cut a release with just this series to begin isolating
this issue
Antheas
> >>>> I think I will tell the user to open a proper bug report because
> >>>> I do agree on the fact that it's looking unrelated.
> >>>>> As for [1], it looks like a trace that stems from a sysfs write to
> >>>>> brightness stemming from userspace that follows the same chain it
> >>>>> would on a stock kernel and times out. Is it present on a stock
> >>>>> kernel?
> >>>> I have asked more details to the user. The user is not online ATM
> >>>> so I will get to you with more details when I can.
> >>>>> Ilpo should know more about this, could the spinlock be interfering?
> >>>>> My testing on devices that have WMI led controls is a bit limited
> >>>>> unfortunately. However, most of our asus users have been happy with
> >>>>> this series for around half a year now.
> >>>> Unless they have looked to kernel logs they won't be able to tell
> >>>> since apparently there are no visible consequences.
> >>>>> Antheas
> >>>>>
> >>>>>>> ---
> >>>>>>> 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 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 (7):
> >>>>>>> HID: asus: refactor init sequence per spec
> >>>>>>> HID: asus: prevent binding to all HID devices on ROG
> >>>>>>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> >>>>>>> HID: asus: listen to the asus-wmi brightness device instead of
> >>>>>>> creating one
> >>>>>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
> >>>>>>> platform/x86: asus-wmi: add keyboard brightness event handler
> >>>>>>> HID: asus: add support for the asus-wmi brightness handler
> >>>>>>>
> >>>>>>> drivers/hid/hid-asus.c | 235 +++++++++++----------
> >>>>>>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> >>>>>>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> >>>>>>> 3 files changed, 291 insertions(+), 170 deletions(-)
> >>>>>>>
> >>>>>>>
> >>>>>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
>
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-16 14:44 UTC (permalink / raw)
To: Denis Benato
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen, fooqux
In-Reply-To: <CAGwozwECtoh-7uK9RuB+WSbognjwHiBx7iXi6OJwyasoAfHemw@mail.gmail.com>
On Thu, 16 Oct 2025 at 16:44, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Thu, 16 Oct 2025 at 16:32, Denis Benato <benato.denis96@gmail.com> wrote:
> >
> >
> > On 10/16/25 14:51, Antheas Kapenekakis wrote:
> > > On Thu, 16 Oct 2025 at 14:46, Denis Benato <benato.denis96@gmail.com> wrote:
> > >>
> > >> On 10/16/25 14:28, Antheas Kapenekakis wrote:
> > >>> On Thu, 16 Oct 2025 at 14:19, Denis Benato <benato.denis96@gmail.com> wrote:
> > >>>> On 10/16/25 14:14, Antheas Kapenekakis wrote:
> > >>>>> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
> > >>>>>> On 10/13/25 22:15, 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.
> > >>>>>>>
> > >>>>>>> All comments with these patches had been addressed since V4.
> > >>>>>> I have loaded this patchset for users of asus-linux project to try out.
> > >>>>>>
> > >>>>>> One of them opened a bug report about a kernel bug that happens
> > >>>>>> consistently when closing the lid of his laptop [1].
> > >>>>>>
> > >>>>>> He also sent another piece of kernel log, but didn't specify anything more
> > >>>>>> about this [2].
> > >>>>>>
> > >>>>>> [1] https://pastebin.com/akZx1w10
> > >>>>>> [2] https://pastebin.com/sKdczPgf
> > >>>>> Can you provide a link to the bug report? [2] seems unrelated.
> > >>>> The log in [2] was posted without additional context in the same
> > >>>> discord message as [1].
> > >>> Link me the kernel sources. Is it linux-g14 in the AUR?
> > >> Someone has replicated it on the AUR but it's just an out-of-sync replica.
> > >>
> > >> The true source code is here:
> > >> https://gitlab.com/asus-linux/linux-g14 branch 6.17
> > > Ok, lets wait for the user to replicate on a stock kernel
> > >
> > User said "yes I just confirmed it: it is only on the asus kernel, mainline doesn't have this issue".
> >
> > With "asus kernel" he is referring to -g14.
> >
> > I added him in CC.
>
> If possible, try the bazzite kernel, its linux-bazzite-bin. It has
> this series + another older armoury series. If it still happens with
> that, we can cut a release with just this series to begin isolating
> this issue
>
> Antheas
To that end, using the previous version of that kernel without this
series is also a possibility.
> > >>>> I think I will tell the user to open a proper bug report because
> > >>>> I do agree on the fact that it's looking unrelated.
> > >>>>> As for [1], it looks like a trace that stems from a sysfs write to
> > >>>>> brightness stemming from userspace that follows the same chain it
> > >>>>> would on a stock kernel and times out. Is it present on a stock
> > >>>>> kernel?
> > >>>> I have asked more details to the user. The user is not online ATM
> > >>>> so I will get to you with more details when I can.
> > >>>>> Ilpo should know more about this, could the spinlock be interfering?
> > >>>>> My testing on devices that have WMI led controls is a bit limited
> > >>>>> unfortunately. However, most of our asus users have been happy with
> > >>>>> this series for around half a year now.
> > >>>> Unless they have looked to kernel logs they won't be able to tell
> > >>>> since apparently there are no visible consequences.
> > >>>>> Antheas
> > >>>>>
> > >>>>>>> ---
> > >>>>>>> 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 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 (7):
> > >>>>>>> HID: asus: refactor init sequence per spec
> > >>>>>>> HID: asus: prevent binding to all HID devices on ROG
> > >>>>>>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> > >>>>>>> HID: asus: listen to the asus-wmi brightness device instead of
> > >>>>>>> creating one
> > >>>>>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > >>>>>>> platform/x86: asus-wmi: add keyboard brightness event handler
> > >>>>>>> HID: asus: add support for the asus-wmi brightness handler
> > >>>>>>>
> > >>>>>>> drivers/hid/hid-asus.c | 235 +++++++++++----------
> > >>>>>>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> > >>>>>>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> > >>>>>>> 3 files changed, 291 insertions(+), 170 deletions(-)
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> >
^ permalink raw reply
* Re: [PATCH v3 1/4] firmware_loader: expand firmware error codes with up-to-date error
From: Marco Felsch @ 2025-10-16 14:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Russ Weight, Luis Chamberlain, Greg Kroah-Hartman,
Rafael J. Wysocki, Andrew Morton, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Marco Felsch,
Henrik Rydberg, Danilo Krummrich, linux-kernel, devicetree,
linux-input
In-Reply-To: <5tlhy2jl77etqxsna42ksdmvu3x3bsp5c44poshkt45agldfsj@bkzlvbfoshsl>
Hi all,
On 25-09-20, Dmitry Torokhov wrote:
> On Wed, Aug 27, 2025 at 03:29:33PM -0600, Russ Weight wrote:
> >
> > On Thu, Aug 21, 2025 at 07:26:36PM +0200, Marco Felsch wrote:
> > > Add FW_UPLOAD_ERR_DUPLICATE to allow drivers to inform the firmware_loader
> > > framework that the update is not required. This can be the case if the
> > > user provided firmware matches the current running firmware.
> > >
> > > Sync lib/test_firmware.c accordingly.
> > >
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >
> > Reviewed-by: Russ Weight <russ.weight@linux.dev>
>
> Does this mean I should merge this through input tree?
may I ask how this is planned to go further?
Regards,
Marco
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Ilpo Järvinen @ 2025-10-16 15:08 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Denis Benato, platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede
In-Reply-To: <CAGwozwGzOQ-LCk6B202-CuKq=gepn6Mt4LitJJZ7dfMLaDVs7Q@mail.gmail.com>
On Thu, 16 Oct 2025, Antheas Kapenekakis wrote:
> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
> > On 10/13/25 22:15, 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.
> > >
> > > All comments with these patches had been addressed since V4.
> > I have loaded this patchset for users of asus-linux project to try out.
> >
> > One of them opened a bug report about a kernel bug that happens
> > consistently when closing the lid of his laptop [1].
> >
> > He also sent another piece of kernel log, but didn't specify anything more
> > about this [2].
> >
> > [1] https://pastebin.com/akZx1w10
> > [2] https://pastebin.com/sKdczPgf
>
> Can you provide a link to the bug report? [2] seems unrelated.
>
> As for [1], it looks like a trace that stems from a sysfs write to
> brightness stemming from userspace that follows the same chain it
> would on a stock kernel and times out. Is it present on a stock
> kernel?
>
> Ilpo should know more about this, could the spinlock be interfering?
[1] certainly seems to do schedule() from do_kbd_led_set() so it's not
possible to use spinlock there.
So we're back to what requires the spinlock? And what the spinlock
protects?
Not related to this particular email in this thread, if the users are
testing something with different kernels, it's also important to make sure
that the lockdep configs are enabled in both. As it could be that in one
kernel lockdep is not enabled and thus it won't do the splat.
--
i.
> My testing on devices that have WMI led controls is a bit limited
> unfortunately. However, most of our asus users have been happy with
> this series for around half a year now.
>
> > > ---
> > > 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 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 (7):
> > > HID: asus: refactor init sequence per spec
> > > HID: asus: prevent binding to all HID devices on ROG
> > > platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> > > HID: asus: listen to the asus-wmi brightness device instead of
> > > creating one
> > > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > platform/x86: asus-wmi: add keyboard brightness event handler
> > > HID: asus: add support for the asus-wmi brightness handler
> > >
> > > drivers/hid/hid-asus.c | 235 +++++++++++----------
> > > drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> > > include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> > > 3 files changed, 291 insertions(+), 170 deletions(-)
> > >
> > >
> > > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> >
>
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-10-16 15:16 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Ilpo Järvinen, fooqux
In-Reply-To: <CAGwozwECtoh-7uK9RuB+WSbognjwHiBx7iXi6OJwyasoAfHemw@mail.gmail.com>
On 10/16/25 16:44, Antheas Kapenekakis wrote:
> On Thu, 16 Oct 2025 at 16:32, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 10/16/25 14:51, Antheas Kapenekakis wrote:
>>> On Thu, 16 Oct 2025 at 14:46, Denis Benato <benato.denis96@gmail.com> wrote:
>>>> On 10/16/25 14:28, Antheas Kapenekakis wrote:
>>>>> On Thu, 16 Oct 2025 at 14:19, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>>> On 10/16/25 14:14, Antheas Kapenekakis wrote:
>>>>>>> On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>>>>> On 10/13/25 22:15, 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.
>>>>>>>>>
>>>>>>>>> All comments with these patches had been addressed since V4.
>>>>>>>> I have loaded this patchset for users of asus-linux project to try out.
>>>>>>>>
>>>>>>>> One of them opened a bug report about a kernel bug that happens
>>>>>>>> consistently when closing the lid of his laptop [1].
>>>>>>>>
>>>>>>>> He also sent another piece of kernel log, but didn't specify anything more
>>>>>>>> about this [2].
>>>>>>>>
>>>>>>>> [1] https://pastebin.com/akZx1w10
>>>>>>>> [2] https://pastebin.com/sKdczPgf
>>>>>>> Can you provide a link to the bug report? [2] seems unrelated.
>>>>>> The log in [2] was posted without additional context in the same
>>>>>> discord message as [1].
>>>>> Link me the kernel sources. Is it linux-g14 in the AUR?
>>>> Someone has replicated it on the AUR but it's just an out-of-sync replica.
>>>>
>>>> The true source code is here:
>>>> https://gitlab.com/asus-linux/linux-g14 branch 6.17
>>> Ok, lets wait for the user to replicate on a stock kernel
>>>
>> User said "yes I just confirmed it: it is only on the asus kernel, mainline doesn't have this issue".
>>
>> With "asus kernel" he is referring to -g14.
>>
>> I added him in CC.
> If possible, try the bazzite kernel, its linux-bazzite-bin. It has
> this series + another older armoury series. If it still happens with
> that, we can cut a release with just this series to begin isolating
> this issue
If deemed necessary I will provide the user a kernel matching exactly
what doesn't give him the error and add to it this patch series.
However the problem does appear to be tied to the use of spinlock,
so perhaps it's better to try a version with some other type of locking.
> Antheas
>
>>>>>> I think I will tell the user to open a proper bug report because
>>>>>> I do agree on the fact that it's looking unrelated.
>>>>>>> As for [1], it looks like a trace that stems from a sysfs write to
>>>>>>> brightness stemming from userspace that follows the same chain it
>>>>>>> would on a stock kernel and times out. Is it present on a stock
>>>>>>> kernel?
>>>>>> I have asked more details to the user. The user is not online ATM
>>>>>> so I will get to you with more details when I can.
>>>>>>> Ilpo should know more about this, could the spinlock be interfering?
>>>>>>> My testing on devices that have WMI led controls is a bit limited
>>>>>>> unfortunately. However, most of our asus users have been happy with
>>>>>>> this series for around half a year now.
>>>>>> Unless they have looked to kernel logs they won't be able to tell
>>>>>> since apparently there are no visible consequences.
>>>>>>> Antheas
>>>>>>>
>>>>>>>>> ---
>>>>>>>>> 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 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 (7):
>>>>>>>>> HID: asus: refactor init sequence per spec
>>>>>>>>> HID: asus: prevent binding to all HID devices on ROG
>>>>>>>>> platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
>>>>>>>>> HID: asus: listen to the asus-wmi brightness device instead of
>>>>>>>>> creating one
>>>>>>>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
>>>>>>>>> platform/x86: asus-wmi: add keyboard brightness event handler
>>>>>>>>> HID: asus: add support for the asus-wmi brightness handler
>>>>>>>>>
>>>>>>>>> drivers/hid/hid-asus.c | 235 +++++++++++----------
>>>>>>>>> drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
>>>>>>>>> include/linux/platform_data/x86/asus-wmi.h | 69 +++---
>>>>>>>>> 3 files changed, 291 insertions(+), 170 deletions(-)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-16 16:16 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Denis Benato, platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede
In-Reply-To: <c075a9f4-8103-dbcc-a1e7-4eaec5e90597@linux.intel.com>
On Thu, 16 Oct 2025 at 17:09, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Thu, 16 Oct 2025, Antheas Kapenekakis wrote:
> > On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
> > > On 10/13/25 22:15, 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.
> > > >
> > > > All comments with these patches had been addressed since V4.
> > > I have loaded this patchset for users of asus-linux project to try out.
> > >
> > > One of them opened a bug report about a kernel bug that happens
> > > consistently when closing the lid of his laptop [1].
> > >
> > > He also sent another piece of kernel log, but didn't specify anything more
> > > about this [2].
> > >
> > > [1] https://pastebin.com/akZx1w10
> > > [2] https://pastebin.com/sKdczPgf
> >
> > Can you provide a link to the bug report? [2] seems unrelated.
> >
> > As for [1], it looks like a trace that stems from a sysfs write to
> > brightness stemming from userspace that follows the same chain it
> > would on a stock kernel and times out. Is it present on a stock
> > kernel?
> >
> > Ilpo should know more about this, could the spinlock be interfering?
>
> [1] certainly seems to do schedule() from do_kbd_led_set() so it's not
> possible to use spinlock there.
>
> So we're back to what requires the spinlock? And what the spinlock
> protects?
For that invocation, since it is coming from the cdev device owned by
asus_wmi, it protects asus_ref.listeners under do_kbd_led_set.
asus_wmi is protected by the fact it is owned by that device. Spinlock
is not required in this invocation due to not being an IRQ.
Under asus_hid_event (second to last patch), which is called from an
IRQ, a spinlock is required for protecting both listeners and the
asus_ref.asus, and I suspect that scheduling from an IRQ is not
allowed either. Is that correct?
Antheas
>
> Not related to this particular email in this thread, if the users are
> testing something with different kernels, it's also important to make sure
> that the lockdep configs are enabled in both. As it could be that in one
> kernel lockdep is not enabled and thus it won't do the splat.
>
> --
> i.
>
>
> > My testing on devices that have WMI led controls is a bit limited
> > unfortunately. However, most of our asus users have been happy with
> > this series for around half a year now.
> >
> > > > ---
> > > > 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 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 (7):
> > > > HID: asus: refactor init sequence per spec
> > > > HID: asus: prevent binding to all HID devices on ROG
> > > > platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> > > > HID: asus: listen to the asus-wmi brightness device instead of
> > > > creating one
> > > > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > > platform/x86: asus-wmi: add keyboard brightness event handler
> > > > HID: asus: add support for the asus-wmi brightness handler
> > > >
> > > > drivers/hid/hid-asus.c | 235 +++++++++++----------
> > > > drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> > > > include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> > > > 3 files changed, 291 insertions(+), 170 deletions(-)
> > > >
> > > >
> > > > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> > >
> >
>
>
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Add HIDPP_QUIRK_RESET_HI_RES_SCROLL
From: Jiri Kosina @ 2025-10-16 16:20 UTC (permalink / raw)
To: Stuart Hayhurst
Cc: linux-input, linux-kernel, Benjamin Tissoires, Bastien Nocera,
Filipe Laíns
In-Reply-To: <20251006010726.189197-2-stuart.a.hayhurst@gmail.com>
On Mon, 6 Oct 2025, Stuart Hayhurst wrote:
> The Logitech G502 Hero Wireless's high resolution scrolling resets after
> being unplugged without notifying the driver, causing extremely slow
> scrolling.
>
> The only indication of this is a battery update packet, so add a quirk to
> detect when the device is unplugged and re-enable the scrolling.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218037
> Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
> ---
>
> I assume this affects more than just my mouse, but I don't have the hardware
> to find out which other mice need this too.
We will find out about more devices later for sure.
Thanks, now applied to hid.git#for-6.18/upstream-fixes.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 00/32] media: Use %pe format specifier
From: Sakari Ailus @ 2025-10-16 19:22 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Linus Walleij, Dmitry Torokhov, Mauro Carvalho Chehab,
Hans Verkuil, Krzysztof Hałasa, Tomi Valkeinen, Leon Luo,
Kieran Bingham, Jacopo Mondi, Kieran Bingham, Laurent Pinchart,
Niklas Söderlund, Julien Massot, Jacopo Mondi, Daniel Scally,
Dave Stevenson, Benjamin Mugnier, Sylvain Petinot, Yong Zhi,
Bingbu Cao, Tianshu Qiu, Tiffany Lin, Andrew-CT Chen, Yunfei Dong,
Matthias Brugger, AngeloGioacchino Del Regno, Rui Miguel Silva,
Laurent Pinchart, Martin Kepplinger, Purism Kernel Team,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Dafna Hirschfeld, Heiko Stuebner, Sylwester Nawrocki,
Krzysztof Kozlowski, Alim Akhtar, Yemike Abhilash Chandra,
Greg Kroah-Hartman, linux-input, linux-kernel, linux-media,
linux-arm-kernel, linux-mediatek, imx, linux-renesas-soc,
linux-rockchip, linux-samsung-soc, linux-staging
In-Reply-To: <20251013-ptr_err-v1-0-2c5efbd82952@chromium.org>
Hi Ricardo,
On Mon, Oct 13, 2025 at 02:14:40PM +0000, Ricardo Ribalda wrote:
> The %pe format specifier is designed to print error pointers. It prints
> a symbolic error name (eg. -EINVAL) and it makes the code simpler by
> omitting PTR_ERR().
>
> The recently introduced test scripts/coccinelle/misc/ptr_err_to_pe.cocci
> checks that we use %pe. Let's make it happy.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Thanks for the set.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v3 06/13] hwmon: Add Apple Silicon SMC hwmon driver
From: Janne Grunau @ 2025-10-16 21:51 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, Jonathan Corbet,
asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251007-macsmc-subdevs-v3-6-d7d3bfd7ae02@gmail.com>
On Tue, Oct 07, 2025 at 09:16:47PM +1000, James Calligeros wrote:
> The System Management Controller on Apple Silicon devices is responsible
> for integrating and exposing the data reported by the vast array of
> hardware monitoring sensors present on these devices. It is also
> responsible for fan control, and allows users to manually set fan
> speeds if they so desire. Add a hwmon driver to expose current,
> power, temperature, and voltage monitoring sensors, as well as
> fan speed monitoring and control via the SMC on Apple Silicon devices.
>
> The SMC firmware has no consistency between devices, even when they
> share an SoC. The FourCC keys used to access sensors are almost
> random. An M1 Mac mini will have different FourCCs for its CPU core
> temperature sensors to an M1 MacBook Pro, for example. For this
> reason, the valid sensors for a given device are specified in a
> child of the SMC Devicetree node. The driver uses this information
> to determine which sensors to make available at runtime.
>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
> Co-developed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Janne Grunau <j@jannau.net>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> Documentation/hwmon/macsmc-hwmon.rst | 71 +++
> MAINTAINERS | 2 +
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/macsmc-hwmon.c | 850 +++++++++++++++++++++++++
> 5 files changed, 936 insertions(+)
...
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -148,6 +148,7 @@ obj-$(CONFIG_SENSORS_LTC4260) += ltc4260.o
> obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o
> obj-$(CONFIG_SENSORS_LTC4282) += ltc4282.o
> obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o
> +obj-$(CONFIG_SENSORS_MACSMC_HWMON) += macsmc-hwmon.o
> obj-$(CONFIG_SENSORS_MAX1111) += max1111.o
> obj-$(CONFIG_SENSORS_MAX127) += max127.o
> obj-$(CONFIG_SENSORS_MAX16065) += max16065.o
> diff --git a/drivers/hwmon/macsmc-hwmon.c b/drivers/hwmon/macsmc-hwmon.c
> new file mode 100644
> index 000000000000..342fe3a5ff62
> --- /dev/null
> +++ b/drivers/hwmon/macsmc-hwmon.c
> @@ -0,0 +1,850 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Apple SMC hwmon driver for Apple Silicon platforms
> + *
> + * The System Management Controller on Apple Silicon devices is responsible for
> + * measuring data from sensors across the SoC and machine. These include power,
> + * temperature, voltage and current sensors. Some "sensors" actually expose
> + * derived values. An example of this is the key PHPC, which is an estimate
> + * of the heat energy being dissipated by the SoC.
> + *
> + * While each SoC only has one SMC variant, each platform exposes a different
> + * set of sensors. For example, M1 MacBooks expose battery telemetry sensors
> + * which are not present on the M1 Mac mini. For this reason, the available
> + * sensors for a given platform are described in the device tree in a child
> + * node of the SMC device. We must walk this list of available sensors and
> + * populate the required hwmon data structures at runtime.
> + *
> + * Originally based on a concept by Jean-Francois Bortolotti <jeff@borto.fr>
> + *
> + * Copyright The Asahi Linux Contributors
> + */
> +
missing linux/bitfield.h include as noted by kernel robot
> +#include <linux/hwmon.h>
> +#include <linux/mfd/macsmc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
...
> +static int macsmc_hwmon_probe(struct platform_device *pdev)
> +{
> + struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
> + struct macsmc_hwmon *hwmon;
> + int ret;
> +
> + /*
> + * The MFD driver will try to probe us unconditionally. Some devices
> + * with the SMC do not have hwmon capabilities. Only probe if we have
> + * a hwmon node.
> + */
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon),
> + GFP_KERNEL);
> + if (!hwmon)
> + return -ENOMEM;
> +
> + hwmon->dev = &pdev->dev;
> + hwmon->smc = smc;
> +
> + ret = macsmc_hwmon_populate_sensors(hwmon, hwmon->dev->of_node);
> + if (ret) {
> + dev_err(hwmon->dev, "Could not parse sensors\n");
> + return ret;
> + }
> +
> + if (!hwmon->curr.count && !hwmon->fan.count &&
> + !hwmon->power.count && !hwmon->temp.count &&
> + !hwmon->volt.count) {
> + dev_err(hwmon->dev,
> + "No valid sensors found of any supported type\n");
> + return -ENODEV;
> + }
> +
> + ret = macsmc_hwmon_create_infos(hwmon);
> + if (ret)
> + return ret;
> +
> + hwmon->chip_info.ops = &macsmc_hwmon_ops;
> + hwmon->chip_info.info =
> + (const struct hwmon_channel_info *const *)&hwmon->channel_infos;
> +
> + hwmon->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> + "macsmc_hwmon", hwmon,
> + &hwmon->chip_info, NULL);
> + if (IS_ERR(hwmon->hwmon_dev))
> + return dev_err_probe(hwmon->dev, PTR_ERR(hwmon->hwmon_dev),
> + "Probing SMC hwmon device failed\n");
> +
> + dev_info(hwmon->dev, "Registered SMC hwmon device. Sensors:");
> + dev_info(hwmon->dev,
printing non-errors during probe is strongly discouraged. I also do not
see much value in this message outside of development so please change
to dev_dbg().
Janne
^ permalink raw reply
* Re: [PATCH v3 08/13] input: macsmc-input: New driver to handle the Apple Mac SMC buttons/lid
From: Janne Grunau @ 2025-10-16 21:58 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, Jonathan Corbet,
asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, linux-doc, Hector Martin
In-Reply-To: <20251007-macsmc-subdevs-v3-8-d7d3bfd7ae02@gmail.com>
On Tue, Oct 07, 2025 at 09:16:49PM +1000, James Calligeros wrote:
> From: Hector Martin <marcan@marcan.st>
>
> This driver implements power button and lid switch support for Apple Mac
> devices using SMC controllers driven by the macsmc driver.
>
> In addition to basic input support, this also responds to the final
> shutdown warning (when the power button is held down long enough) by
> doing an emergency kernel poweroff. This allows the NVMe controller to
> be cleanly shut down, which prevents data loss for in-cache data.
>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Co-developed-by: Sven Peter <sven@kernel.org>
> Signed-off-by: Sven Peter <sven@kernel.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> MAINTAINERS | 1 +
> drivers/input/misc/Kconfig | 11 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/macsmc-input.c | 208 +++++++++++++++++++++++++
> 4 files changed, 221 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 79b9f40224a9..e8283f127f11 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2451,6 +2451,7 @@ F: drivers/hwmon/macsmc-hwmon.c
> F: drivers/pmdomain/apple/
> F: drivers/i2c/busses/i2c-pasemi-core.c
> F: drivers/i2c/busses/i2c-pasemi-platform.c
> +F: drivers/input/misc/macsmc-input.c
> F: drivers/input/touchscreen/apple_z2.c
> F: drivers/iommu/apple-dart.c
> F: drivers/iommu/io-pgtable-dart.c
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 0e6b49fb54bc..5ab8a4729e0a 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -981,4 +981,15 @@ config INPUT_STPMIC1_ONKEY
> To compile this driver as a module, choose M here: the
> module will be called stpmic1_onkey.
>
> +config INPUT_MACSMC_INPUT
INPUT_MACSMC_INPUT looks redundant, is there a reason for not just using
INPUT_MACSMC?
> + tristate "Apple Mac SMC lid/buttons"
> + depends on MFD_MACSMC
> + help
> + Say Y here if you want to use the input events delivered via the
> + SMC controller on Apple Mac machines using the macsmc driver.
> + This includes lid open/close and the power button.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called macsmc-input.
> +
> endif
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index ae857c24f48e..480a0d08d4ae 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_INPUT_IQS7222) += iqs7222.o
> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> +obj-$(CONFIG_INPUT_MACSMC_INPUT) += macsmc-input.o
> obj-$(CONFIG_INPUT_MAX7360_ROTARY) += max7360-rotary.o
> obj-$(CONFIG_INPUT_MAX77650_ONKEY) += max77650-onkey.o
> obj-$(CONFIG_INPUT_MAX77693_HAPTIC) += max77693-haptic.o
> diff --git a/drivers/input/misc/macsmc-input.c b/drivers/input/misc/macsmc-input.c
> new file mode 100644
> index 000000000000..ebbc7dfc31f5
> --- /dev/null
> +++ b/drivers/input/misc/macsmc-input.c
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Apple SMC input event driver
> + * Copyright The Asahi Linux Contributors
> + *
> + * This driver exposes HID events from the SMC as an input device.
s/HID //
> + * This includes the lid open/close and power button notifications.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/macsmc.h>
> +#include <linux/module.h>
> +#include <linux/reboot.h>
> +
> +/**
> + * struct macsmc_input
> + * @dev: Underlying struct device for the input sub-device
> + * @smc: Pointer to apple_smc struct of the mfd parent
> + * @input: Allocated input_dev; devres managed
> + * @nb: Notifier block used for incoming events from SMC (e.g. button pressed down)
> + * @wakeup_mode: Set to true when system is suspended and power button events should wake it
> + */
> +struct macsmc_input {
> + struct device *dev;
> + struct apple_smc *smc;
> + struct input_dev *input;
> + struct notifier_block nb;
> + bool wakeup_mode;
> +};
> +
> +#define SMC_EV_BTN 0x7201
> +#define SMC_EV_LID 0x7203
> +
> +#define BTN_POWER 0x01 /* power button on e.g. Mac Mini chasis pressed */
> +#define BTN_TOUCHID 0x06 /* combined TouchID / power button on MacBooks pressed */
> +#define BTN_POWER_HELD_SHORT 0xfe /* power button briefly held down */
> +#define BTN_POWER_HELD_LONG 0x00 /* power button held down; sent just before forced poweroff */
> +
> +static void macsmc_input_event_button(struct macsmc_input *smcin, unsigned long event)
> +{
> + u8 button = (event >> 8) & 0xff;
> + u8 state = !!(event & 0xff);
> +
> + switch (button) {
> + case BTN_POWER:
> + case BTN_TOUCHID:
> + if (smcin->wakeup_mode) {
> + if (state)
> + pm_wakeup_event(smcin->dev, 0);
> + } else {
> + input_report_key(smcin->input, KEY_POWER, state);
> + input_sync(smcin->input);
> + }
> + break;
> + case BTN_POWER_HELD_SHORT: /* power button held down; ignore */
> + break;
> + case BTN_POWER_HELD_LONG:
> + /*
> + * If we get here the power button has been held down for a while and
> + * we have about 4 seconds before forced power-off is triggered by SMC.
> + * Try to do an emergency shutdown to make sure the NVMe cache is
> + * flushed. macOS actually does this by panicing (!)...
> + */
> + if (state) {
> + dev_crit(smcin->dev, "Triggering forced shutdown!\n");
> + if (kernel_can_power_off())
> + kernel_power_off();
> + else /* Missing macsmc-reboot driver? */
> + kernel_restart("SMC power button triggered restart");
> + }
> + break;
> + default:
> + dev_warn(smcin->dev, "Unknown SMC button event: %04lx\n", event & 0xffff);
> + }
> +}
> +
> +static void macsmc_input_event_lid(struct macsmc_input *smcin, unsigned long event)
> +{
> + u8 lid_state = !!((event >> 8) & 0xff);
> +
> + if (smcin->wakeup_mode && !lid_state)
> + pm_wakeup_event(smcin->dev, 0);
> +
> + input_report_switch(smcin->input, SW_LID, lid_state);
> + input_sync(smcin->input);
> +}
> +
> +static int macsmc_input_event(struct notifier_block *nb, unsigned long event, void *data)
> +{
> + struct macsmc_input *smcin = container_of(nb, struct macsmc_input, nb);
> + u16 type = event >> 16;
> +
> + switch (type) {
> + case SMC_EV_BTN:
> + macsmc_input_event_button(smcin, event);
> + return NOTIFY_OK;
> + case SMC_EV_LID:
> + macsmc_input_event_lid(smcin, event);
> + return NOTIFY_OK;
> + default:
> + /* SMC event meant for another driver */
> + return NOTIFY_DONE;
> + }
> +}
> +
> +static int macsmc_input_probe(struct platform_device *pdev)
> +{
> + struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
> + struct macsmc_input *smcin;
> + bool have_lid, have_power;
> + int error;
> +
> + /* Bail early if this SMC neither supports power button nor lid events */
> + have_lid = apple_smc_key_exists(smc, SMC_KEY(MSLD));
> + have_power = apple_smc_key_exists(smc, SMC_KEY(bHLD));
> + if (!have_lid && !have_power)
> + return -ENODEV;
> +
> + smcin = devm_kzalloc(&pdev->dev, sizeof(*smcin), GFP_KERNEL);
> + if (!smcin)
> + return -ENOMEM;
> +
> + smcin->dev = &pdev->dev;
> + smcin->smc = smc;
> + platform_set_drvdata(pdev, smcin);
> +
> + smcin->input = devm_input_allocate_device(&pdev->dev);
> + if (!smcin->input)
> + return -ENOMEM;
> +
> + smcin->input->phys = "macsmc-input (0)";
> + smcin->input->name = "Apple SMC power/lid events";
> +
> + if (have_lid)
> + input_set_capability(smcin->input, EV_SW, SW_LID);
> + if (have_power)
> + input_set_capability(smcin->input, EV_KEY, KEY_POWER);
> +
> + if (have_lid) {
> + u8 val;
> +
> + error = apple_smc_read_u8(smc, SMC_KEY(MSLD), &val);
> + if (error < 0)
> + dev_warn(&pdev->dev, "Failed to read initial lid state\n");
> + else
> + input_report_switch(smcin->input, SW_LID, val);
> + }
> +
> + if (have_power) {
> + u32 val;
> +
> + error = apple_smc_read_u32(smc, SMC_KEY(bHLD), &val);
> + if (error < 0)
> + dev_warn(&pdev->dev, "Failed to read initial power button state\n");
> + else
> + input_report_key(smcin->input, KEY_POWER, val & 1);
> + }
> +
> + error = input_register_device(smcin->input);
> + if (error) {
> + dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
> + return error;
> + }
> +
> + input_sync(smcin->input);
> +
> + smcin->nb.notifier_call = macsmc_input_event;
> + blocking_notifier_chain_register(&smc->event_handlers, &smcin->nb);
> +
> + device_init_wakeup(&pdev->dev, 1);
> +
> + return 0;
> +}
> +
> +static int macsmc_input_pm_prepare(struct device *dev)
> +{
> + struct macsmc_input *smcin = dev_get_drvdata(dev);
> +
> + smcin->wakeup_mode = true;
> + return 0;
> +}
> +
> +static void macsmc_input_pm_complete(struct device *dev)
> +{
> + struct macsmc_input *smcin = dev_get_drvdata(dev);
> +
> + smcin->wakeup_mode = false;
> +}
> +
> +static const struct dev_pm_ops macsmc_input_pm_ops = {
> + .prepare = macsmc_input_pm_prepare,
> + .complete = macsmc_input_pm_complete,
> +};
> +
> +static struct platform_driver macsmc_input_driver = {
> + .driver = {
> + .name = "macsmc-input",
> + .pm = &macsmc_input_pm_ops,
> + },
> + .probe = macsmc_input_probe,
> +};
> +module_platform_driver(macsmc_input_driver);
> +
> +MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_DESCRIPTION("Apple SMC input driver");
mssing 'MODULE_ALIAS("platform:macsmc-input");'. This is required when
using MFD_CELL_NAME(). My ask for it to be removed was based on my
mistaken idea to add a pointless node to the DT and use MFD_CELL_OF().
Janne
^ permalink raw reply
* Re: [PATCH v3 00/13] mfd: macsmc: add rtc, hwmon and hid subdevices
From: Janne Grunau @ 2025-10-16 22:15 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, Jonathan Corbet,
asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, linux-doc, Mark Kettenis, Hector Martin
In-Reply-To: <20251007-macsmc-subdevs-v3-0-d7d3bfd7ae02@gmail.com>
On Tue, Oct 07, 2025 at 09:16:41PM +1000, James Calligeros wrote:
> Hi all,
>
> This series adds support for the remaining SMC subdevices. These are the
> RTC, hwmon, and HID devices. They are being submitted together as the RTC
> and hwmon drivers both require changes to the SMC DT schema.
>
> The RTC driver is responsible for getting and setting the system clock,
> and requires an NVMEM cell. This series replaces Sven's original RTC driver
> submission [1].
>
> The hwmon function is an interesting one. While each Apple Silicon device
> exposes pretty similar sets of sensors, these all seem to be paired to
> different SMC keys in the firmware interface. This is true even when the
> sensors are on the SoC. For example, an M1 MacBook Pro will use different
> keys to access the LITTLE core temperature sensors to an M1 Mac mini. This
> necessitates describing which keys correspond to which sensors for each
> device individually, and populating the hwmon structs at runtime. We do
> this with a node in the device tree. This series includes only the keys
> for sensors which we know to be common to all devices. The SMC is also
> responsible for monitoring and controlling fan speeds on systems with fans,
> which we expose via the hwmon driver.
The split of the hwmon dts changes looks weird to me. It's not a lot of
changes so squashing everything together into a single commit might be
ok. If you want to split the commits splitting them by SoC (t8103,
t8112, t600x, ...) and adding common sensor defines as needed might work
better.
> The SMC also handles the hardware power button and lid switch. Power
> button presses and lid opening/closing are emitted as HID events, so we
> add an input subdevice to handle them.
The cover letter doesn't mention a merge strategy for this series. I
don't think there are any dependencies between different parts. That
means the dt-bindings and driver additions can be merged through their
subsystem trees. The single line patches wiring the devices up to the
macsmc mfd driver should be merged together through mfd tree as they
(trivially) conflict. The dts changes will be merged through the apple
soc tree.
Janne
^ permalink raw reply
* 答复: [External Mail]Re: [PATCH v2] HID: hid-input: only ignore 0 battery events for digitizers
From: 卢国宏 @ 2025-10-17 0:38 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Benjamin Tissoires, kenalba@google.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
李鹏, 卢国宏
In-Reply-To: <p7675qor-q8qp-spr8-3o6r-pp3qp42qp4q4@xreary.bet>
Hi Jiri!
Where can I find information about your commits? Is it "https://web.git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.17/upstream-fixes"? So far, I haven't seen any information about this commit. I'll need this information when I submit this code to Google's GKI. Thank you!
________________________________________
发件人: Jiri Kosina <jikos@kernel.org>
发送时间: 2025年10月15日 23:15:18
收件人: Dmitry Torokhov
抄送: Benjamin Tissoires; 卢国宏; kenalba@google.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
主题: [External Mail]Re: [PATCH v2] HID: hid-input: only ignore 0 battery events for digitizers
[外部邮件] 此邮件来源于小米公司外部,请谨慎处理。若对邮件安全性存疑,请将邮件转发给misec@xiaomi.com进行反馈
On Tue, 14 Oct 2025, Dmitry Torokhov wrote:
> Commit 581c4484769e ("HID: input: map digitizer battery usage") added
> handling of battery events for digitizers (typically for batteries
> presented in stylii). Digitizers typically report correct battery levels
> only when stylus is actively touching the surface, and in other cases
> they may report battery level of 0. To avoid confusing consumers of the
> battery information the code was added to filer out reports with 0
> battery levels.
>
> However there exist other kinds of devices that may legitimately report
> 0 battery levels. Fix this by filtering out 0-level reports only for
> digitizer usages, and continue reporting them for other kinds of devices
> (Smart Batteries, etc).
>
> Reported-by: 卢国宏 <luguohong@xiaomi.com>
> Fixes: 581c4484769e ("HID: input: map digitizer battery usage")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> v2: rebased on top of linux-next, dropped Tested-by: tag
Applied, thanks!
--
Jiri Kosina
SUSE Labs
#/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本邮件! This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#
^ permalink raw reply
* RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking
From: Zhang, Lixu @ 2025-10-17 0:52 UTC (permalink / raw)
To: Jiri Kosina
Cc: srinivas pandruvada, linux-input@vger.kernel.org,
benjamin.tissoires@redhat.com, Wang, Selina
In-Reply-To: <46948s8n-10q4-44rp-qs7q-q0qn1oq2pss7@xreary.bet>
>-----Original Message-----
>From: Jiri Kosina <jikos@kernel.org>
>Sent: Thursday, October 16, 2025 4:28 PM
>To: Zhang, Lixu <lixu.zhang@intel.com>
>Cc: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; linux-
>input@vger.kernel.org; benjamin.tissoires@redhat.com; Wang, Selina
><selina.wang@intel.com>
>Subject: RE: [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to
>prevent resume blocking
>
>On Thu, 16 Oct 2025, Zhang, Lixu wrote:
>
>> >
>> >We can definitely do that. The question is whether this one should go
>> >in right now for 6.18 still, or we queue both for 6.19.
>> You can queue both for 6.19. Thanks.
>
>Could you then please send me the power management series rebased on top
>of it, and I'll create a -v2 topic branch with all of it together.
Sure. By the way, I have made a small fix change for power management series.
I'll squash it into v2.
Thanks,
Lixu
>
>Thanks,
>
>--
>Jiri Kosina
>SUSE Labs
^ permalink raw reply
* [PATCH v2 0/6] HID: intel-ish-hid: Various power management improvements for hibernation
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
- Separating hibernate callbacks in dev_pm_ops for clearer power state transitions
- Using IPC RESET in ish_wakeup() to ensure reliable device wakeup
- Scheduling firmware reset work on RESET_NOTIFY/ACK for robust recovery
- Resetting client state on resume from D3 to maintain consistency
- Enhancing resume logic in ishtp-hid-client for better stability
These patches enhance reliability, improve power management flow. All changes
have been validated on TwinLake (ISH 5.4), ArrowLake (ISH 5.6), and PantherLake
(ISH 5.8) platforms.
v2:
- Rebased on top of [PATCH] HID: intel-ish-hid: Use dedicated unbound workqueues to prevent resume blocking
- Changes in [PATCH v2 5/6] HID: intel-ish-hid: Use IPC RESET instead of void message in ish_wakeup()
* Set the HW ready timeout to 10 seconds, matching the original timeout
value used in ish_wakeup(), to prevent timeout issues on devices like
the Lenovo ThinkPad X1 Titanium Gen 1 that require approximately 4
seconds to become ready after wakeup.
* Added RECVD_HW_READY_TIMEOUT macro for better code maintainability.
Zhang Lixu (6):
HID: intel-ish-hid: Add ishtp_get_connection_state() interface
HID: intel-ishtp-hid: Clear suspended flag only after connected on
resume
HID: intel-ish-ipc: Reset clients state on resume from D3
HID: intel-ish-hid: ipc: Always schedule FW reset work on
RESET_NOTIFY/ACK
HID: intel-ish-hid: Use IPC RESET instead of void message in
ish_wakeup()
HID: intel-ish-hid: ipc: Separate hibernate callbacks in dev_pm_ops
drivers/hid/intel-ish-hid/ipc/ipc.c | 78 +++++++++-----------
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 29 +++++++-
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 11 ++-
drivers/hid/intel-ish-hid/ishtp/client.c | 6 ++
include/linux/intel-ish-client-if.h | 1 +
5 files changed, 76 insertions(+), 49 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/6] HID: intel-ish-hid: Add ishtp_get_connection_state() interface
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20251017022218.1292451-1-lixu.zhang@intel.com>
Add the ishtp_get_connection_state() function for struct ishtp_cl, allowing
ishtp client drivers to retrieve the current connection state.
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp/client.c | 6 ++++++
include/linux/intel-ish-client-if.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index 21a2c0773cc2..40f510b1c072 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -1261,6 +1261,12 @@ void ishtp_set_connection_state(struct ishtp_cl *cl, int state)
}
EXPORT_SYMBOL(ishtp_set_connection_state);
+int ishtp_get_connection_state(struct ishtp_cl *cl)
+{
+ return cl->state;
+}
+EXPORT_SYMBOL(ishtp_get_connection_state);
+
void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id)
{
cl->fw_client_id = fw_client_id;
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index b235fd84f478..2cd4f65aaa37 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -109,6 +109,7 @@ struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl);
void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size);
void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size);
void ishtp_set_connection_state(struct ishtp_cl *cl, int state);
+int ishtp_get_connection_state(struct ishtp_cl *cl);
void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id);
void ishtp_put_device(struct ishtp_cl_device *cl_dev);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/6] HID: intel-ishtp-hid: Clear suspended flag only after connected on resume
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20251017022218.1292451-1-lixu.zhang@intel.com>
When resuming from suspend-to-RAM or hibernate, the ISH firmware is powered
on from D3, causing all previous client connections between the firmware
and driver to be lost. Although the underlying ishtp bus driver initiates a
client reconnection flow, this process is asynchronous. As a result, when
hid_ishtp_cl_resume_handler() is executed, the connection may not have been
re-established yet. Clearing the suspended flag prematurely in this
scenario can lead to a timeout when the upper-layer HID sensor driver set
feature during resume.
To prevent such timeouts, only clear the suspended flag after confirming
that the connection state is ISHTP_CL_CONNECTED.
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index f61add862b6b..f37b3bc2bb7d 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -757,8 +757,15 @@ static void hid_ishtp_cl_resume_handler(struct work_struct *work)
struct ishtp_cl *hid_ishtp_cl = client_data->hid_ishtp_cl;
if (ishtp_wait_resume(ishtp_get_ishtp_device(hid_ishtp_cl))) {
- client_data->suspended = false;
- wake_up_interruptible(&client_data->ishtp_resume_wait);
+ /*
+ * Clear the suspended flag only when the connection is established.
+ * If the connection is not established, the suspended flag will be cleared after
+ * the connection is made.
+ */
+ if (ishtp_get_connection_state(hid_ishtp_cl) == ISHTP_CL_CONNECTED) {
+ client_data->suspended = false;
+ wake_up_interruptible(&client_data->ishtp_resume_wait);
+ }
} else {
hid_ishtp_trace(client_data, "hid client: wait for resume timed out");
dev_err(cl_data_to_dev(client_data), "wait for resume timed out");
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/6] HID: intel-ish-ipc: Reset clients state on resume from D3
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20251017022218.1292451-1-lixu.zhang@intel.com>
When ISH resumes from D3, the connection between ishtp clients and firmware
is lost. The ish_resume() function schedules resume_work asynchronously to
re-initiate the connection and then returns immediately. This can cause a
race where the upper-layer ishtp client driver's .resume() may execute
before the connection is fully restored, leaving the client in a stale
connected state. If the client sends messages during this window, the
firmware cannot respond.
To avoid this, reset the ishtp clients' state before returning from
ish_resume() if ISH is resuming from D3.
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index b748ac6fbfdc..e4499c83c62e 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -147,6 +147,12 @@ static inline bool ish_should_enter_d0i3(struct pci_dev *pdev)
static inline bool ish_should_leave_d0i3(struct pci_dev *pdev)
{
+ struct ishtp_device *dev = pci_get_drvdata(pdev);
+ u32 fwsts = dev->ops->get_fw_status(dev);
+
+ if (dev->suspend_flag || !IPC_IS_ISH_ILUP(fwsts))
+ return false;
+
return !pm_resume_via_firmware() || pdev->device == PCI_DEVICE_ID_INTEL_ISH_CHV;
}
@@ -277,10 +283,8 @@ static void __maybe_unused ish_resume_handler(struct work_struct *work)
{
struct pci_dev *pdev = to_pci_dev(ish_resume_device);
struct ishtp_device *dev = pci_get_drvdata(pdev);
- uint32_t fwsts = dev->ops->get_fw_status(dev);
- if (ish_should_leave_d0i3(pdev) && !dev->suspend_flag
- && IPC_IS_ISH_ILUP(fwsts)) {
+ if (ish_should_leave_d0i3(pdev)) {
if (device_may_wakeup(&pdev->dev))
disable_irq_wake(pdev->irq);
@@ -384,6 +388,10 @@ static int __maybe_unused ish_resume(struct device *device)
ish_resume_device = device;
dev->resume_flag = 1;
+ /* If ISH resume from D3, reset ishtp clients before return */
+ if (!ish_should_leave_d0i3(pdev))
+ ishtp_reset_handler(dev);
+
queue_work(dev->unbound_wq, &resume_work);
return 0;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 4/6] HID: intel-ish-hid: ipc: Always schedule FW reset work on RESET_NOTIFY/ACK
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20251017022218.1292451-1-lixu.zhang@intel.com>
Both ISH firmware and driver can actively send MNG_RESET_NOTIFY to initiate
an FW reset handshake. Upon receiving this, the peer should reply with
MNG_RESET_NOTIFY_ACK. Therefore, the driver should schedule the FW reset
handshake work function when receiving either MNG_RESET_NOTIFY or
MNG_RESET_NOTIFY_ACK.
Previously, driver only scheduled the work function on MNG_RESET_NOTIFY.
This patch ensures the work function is scheduled on both messages, but
only replies with MNG_RESET_NOTIFY_ACK when receiving MNG_RESET_NOTIFY.
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ipc/ipc.c | 39 ++++++++++++++---------------
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 9958f2968c4f..01a139e17cb5 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -481,6 +481,20 @@ static int timed_wait_for_timeout(struct ishtp_device *dev, int condition,
return ret;
}
+static void ish_send_reset_notify_ack(struct ishtp_device *dev)
+{
+ /* Read reset ID */
+ u32 reset_id = ish_reg_read(dev, IPC_REG_ISH2HOST_MSG) & 0xFFFF;
+
+ /*
+ * Set HOST2ISH.ILUP. Apparently we need this BEFORE sending
+ * RESET_NOTIFY_ACK - FW will be checking for it
+ */
+ ish_set_host_rdy(dev);
+ /* Send RESET_NOTIFY_ACK (with reset_id) */
+ ipc_send_mng_msg(dev, MNG_RESET_NOTIFY_ACK, &reset_id, sizeof(u32));
+}
+
#define TIME_SLICE_FOR_FW_RDY_MS 100
#define TIME_SLICE_FOR_INPUT_RDY_MS 100
#define TIMEOUT_FOR_FW_RDY_MS 2000
@@ -496,13 +510,9 @@ static int timed_wait_for_timeout(struct ishtp_device *dev, int condition,
*/
static int ish_fw_reset_handler(struct ishtp_device *dev)
{
- uint32_t reset_id;
unsigned long flags;
int ret;
- /* Read reset ID */
- reset_id = ish_reg_read(dev, IPC_REG_ISH2HOST_MSG) & 0xFFFF;
-
/* Clear IPC output queue */
spin_lock_irqsave(&dev->wr_processing_spinlock, flags);
list_splice_init(&dev->wr_processing_list, &dev->wr_free_list);
@@ -521,15 +531,6 @@ static int ish_fw_reset_handler(struct ishtp_device *dev)
/* Send clock sync at once after reset */
ishtp_dev->prev_sync = 0;
- /*
- * Set HOST2ISH.ILUP. Apparently we need this BEFORE sending
- * RESET_NOTIFY_ACK - FW will be checking for it
- */
- ish_set_host_rdy(dev);
- /* Send RESET_NOTIFY_ACK (with reset_id) */
- ipc_send_mng_msg(dev, MNG_RESET_NOTIFY_ACK, &reset_id,
- sizeof(uint32_t));
-
/* Wait for ISH FW'es ILUP and ISHTP_READY */
ret = timed_wait_for_timeout(dev, WAIT_FOR_FW_RDY,
TIME_SLICE_FOR_FW_RDY_MS,
@@ -563,8 +564,6 @@ static void fw_reset_work_fn(struct work_struct *work)
if (!rv) {
/* ISH is ILUP & ISHTP-ready. Restart ISHTP */
msleep_interruptible(TIMEOUT_FOR_HW_RDY_MS);
- ishtp_dev->recvd_hw_ready = 1;
- wake_up_interruptible(&ishtp_dev->wait_hw_ready);
/* ISHTP notification in IPC_RESET sequence completion */
if (!work_pending(work))
@@ -625,15 +624,14 @@ static void recv_ipc(struct ishtp_device *dev, uint32_t doorbell_val)
break;
case MNG_RESET_NOTIFY:
- if (!ishtp_dev) {
- ishtp_dev = dev;
- }
- queue_work(dev->unbound_wq, &fw_reset_work);
- break;
+ ish_send_reset_notify_ack(ishtp_dev);
+ fallthrough;
case MNG_RESET_NOTIFY_ACK:
dev->recvd_hw_ready = 1;
wake_up_interruptible(&dev->wait_hw_ready);
+ if (!work_pending(&fw_reset_work))
+ queue_work(dev->unbound_wq, &fw_reset_work);
break;
}
}
@@ -1001,6 +999,7 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev)
list_add_tail(&tx_buf->link, &dev->wr_free_list);
}
+ ishtp_dev = dev;
ret = devm_work_autocancel(&pdev->dev, &fw_reset_work, fw_reset_work_fn);
if (ret) {
dev_err(dev->devc, "Failed to initialise FW reset work\n");
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/6] HID: intel-ish-hid: Use IPC RESET instead of void message in ish_wakeup()
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20251017022218.1292451-1-lixu.zhang@intel.com>
On ISH power-up, the bootloader enters sleep after preparing to load the
main firmware, waiting for the driver to be ready. When the driver is
ready, it sends a void message to wake up the bootloader and load the main
firmware. The main firmware then sends MNG_RESET_NOTIFY to the driver for
handshake.
This void message-based IPC handshake only works if the main firmware has
not been loaded. During hibernation resume, if the restore kernel has the
ISH driver, the driver wakes up the bootloader to load the main firmware
and perform IPC handshake. However, when switching to the image kernel,
since the main firmware is already loaded, sending a void message in the
.restore() callback does not trigger IPC handshake.
By sending MNG_RESET_NOTIFY (IPC RESET message) in ish_wakeup() instead of
a void message, we can explicitly wake up the bootloader and perform IPC
handshake, regardless of the firmware state. Additionally, since
ish_ipc_reset() already waits for recvd_hw_ready, the redundant wait for
recvd_hw_ready in ish_hw_start() is removed.
The timeout for waiting for HW ready is set to 10 seconds, matching the
original timeout value used in ish_wakeup(), to ensure reliable wakeup on
hardware that requires more time, such as the Lenovo ThinkPad X1 Titanium
Gen 1.
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ipc/ipc.c | 39 ++++++++++++-----------------
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 01a139e17cb5..59355e4a61f8 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -728,22 +728,28 @@ int ish_disable_dma(struct ishtp_device *dev)
* ish_wakeup() - wakeup ishfw from waiting-for-host state
* @dev: ishtp device pointer
*
- * Set the dma enable bit and send a void message to FW,
+ * Set the dma enable bit and send a IPC RESET message to FW,
* it wil wakeup FW from waiting-for-host state.
+ *
+ * Return: 0 for success else error code.
*/
-static void ish_wakeup(struct ishtp_device *dev)
+static int ish_wakeup(struct ishtp_device *dev)
{
+ int ret;
+
/* Set dma enable bit */
ish_reg_write(dev, IPC_REG_ISH_RMP2, IPC_RMP2_DMA_ENABLED);
/*
- * Send 0 IPC message so that ISH FW wakes up if it was already
+ * Send IPC RESET message so that ISH FW wakes up if it was already
* asleep.
*/
- ish_reg_write(dev, IPC_REG_HOST2ISH_DRBL, IPC_DRBL_BUSY_BIT);
+ ret = ish_ipc_reset(dev);
/* Flush writes to doorbell and REMAP2 */
ish_reg_read(dev, IPC_REG_ISH_HOST_FWSTS);
+
+ return ret;
}
/**
@@ -792,11 +798,11 @@ static int _ish_hw_reset(struct ishtp_device *dev)
pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, csr);
/* Now we can enable ISH DMA operation and wakeup ISHFW */
- ish_wakeup(dev);
-
- return 0;
+ return ish_wakeup(dev);
}
+#define RECVD_HW_READY_TIMEOUT (10 * HZ)
+
/**
* _ish_ipc_reset() - IPC reset
* @dev: ishtp device pointer
@@ -831,7 +837,8 @@ static int _ish_ipc_reset(struct ishtp_device *dev)
}
wait_event_interruptible_timeout(dev->wait_hw_ready,
- dev->recvd_hw_ready, 2 * HZ);
+ dev->recvd_hw_ready,
+ RECVD_HW_READY_TIMEOUT);
if (!dev->recvd_hw_ready) {
dev_err(dev->devc, "Timed out waiting for HW ready\n");
rv = -ENODEV;
@@ -855,21 +862,7 @@ int ish_hw_start(struct ishtp_device *dev)
set_host_ready(dev);
/* After that we can enable ISH DMA operation and wakeup ISHFW */
- ish_wakeup(dev);
-
- /* wait for FW-initiated reset flow */
- if (!dev->recvd_hw_ready)
- wait_event_interruptible_timeout(dev->wait_hw_ready,
- dev->recvd_hw_ready,
- 10 * HZ);
-
- if (!dev->recvd_hw_ready) {
- dev_err(dev->devc,
- "[ishtp-ish]: Timed out waiting for FW-initiated reset\n");
- return -ENODEV;
- }
-
- return 0;
+ return ish_wakeup(dev);
}
/**
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox