* Re: [PATCH v12 2/3] HID: cp2112: Fwnode Support
From: Andy Shevchenko @ 2025-11-26 21:23 UTC (permalink / raw)
To: Danny Kaehn
Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Andi Shyti,
Conor Dooley, Jiri Kosina, devicetree, linux-input,
Dmitry Torokhov, Bartosz Golaszewski, Ethan Twardy, linux-i2c,
linux-kernel, Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <20251126193251.GA269764@LNDCL34533.neenah.na.plexus.com>
On Wed, Nov 26, 2025 at 01:32:51PM -0600, Danny Kaehn wrote:
> On Wed, Nov 26, 2025 at 08:27:19PM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 26, 2025 at 11:05:25AM -0600, Danny Kaehn wrote:
...
> > > For ACPI, the i2c_adapter will use the child with _ADR Zero and the
> > > gpio_chip will use the child with _ADR One. For DeviceTree, the
> > > i2c_adapter will use the child with name "i2c", but the gpio_chip
> > > will share a firmware node with the CP2112.
> >
> > Hmm... Is there any explanation why DT decided to go that way?
>
> I don't have an explanation, but Rob H. had directed that I make this
> change in [1].
>
> In v11, I then removed that child node for both ACPI and DT, hoping to
> maintain unity, but you had directed that wouldn't be intuitive for ACPI
> in [2].
>
> Thus, in this v12, I have just entirely split the two, as it seemed
> unlikely that any compromise to unify the schema between the two
> firmware languages would be possible for a change/driver this
> inconsquential to the overall kernel.
Even though, would be nice to try to get a rationale from Rob on this.
Then we can put it in the commit message to explain. Otherwise it will
confuse history diggers in the future.
> [1]:
> https://lore.kernel.org/all/20240213152825.GA1223720-robh@kernel.org/
>
> [2]:
> https://lore.kernel.org/all/ZmISaEIGlxZVK_jf@smile.fi.intel.com/
...
> > > + switch (addr) {
> > > + case CP2112_I2C_ADR:
> > > + device_set_node(&dev->adap.dev, child);
> > > + break;
> > > + case CP2112_GPIO_ADR:
> > > + dev->gc.fwnode = child;
> > > + break;
> >
> > If by any chance we have malformed table and there are more devices with
> > the same address? Maybe we don't need to address this right now, just
> > asking... (I believe ACPI compiler won't allow that, but table can be
> > crafted directly in the binary format.)
> >
>
> You're sugggesting perhaps that we explicitly keep track of which
> addresses have been encountered, and refuse to do any fwnode parsing
> if we detect the same address used twice? I believe the current behavior
> would be that the "last node wins"; not sure if it should be a "first node
> wins" or a full error scenario...
I'm suggesting to think about this, not acting right now. I don't believe in
such a case IRL.
> > > + }
...
> > > + device_set_node(&dev->adap.dev,
> > > + device_get_named_child_node(&hdev->dev, "i2c"));
> >
> > Here we bump the reference count, where is it going to be dropped?
> >
> > Note, in the other branch (ACPI) the reference count is not bumped in
> > the current code.
>
> Great point, forgot that I had dropped that handling in v9. The old
> behavior was that the CP2112 driver maintained a reference to each node
> during the lifetime of the device (and released during probe errors,
> etc..). I'm still a bit confused as to whether that is correct or not,
> or if the references should immediately be dropped once they're done
> being parsed during probe()... My understanding previously was that I
> should keep the reference count for the child fwnodes for the lifetime
> of the CP2112, since the pointers to those are stored in the child
> devices but would usually be managed by the parent bus-level code, does
> that seem correct?
While there is a (theoretical) possibility to have lifetime of fwnode shorter
than a device's, I don't think we have or ever will have such a practical
example. So, assumption is that, the fwnode that struct device holds has
the same or longer lifetime.
Note, I haven't investigated overlays (DT and ACPI) behaviour. IIRC you
experimented with ACPI SSDT on this device, perhaps you can try to see
what happens if there is a confirmed that the above is not only a theoretical
problem.
TL;DR: I would drop reference count just after we got a respective fwnode.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler
From: Antheas Kapenekakis @ 2025-11-26 20:39 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: <713cc4fd-e6a1-4db4-907a-a0575a2b7b8f@gmail.com>
On Wed, 26 Nov 2025 at 21:23, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 11/22/25 12:00, Antheas Kapenekakis wrote:
> > The keyboard brightness control of Asus WMI keyboards is handled in
> > kernel, which leads to the shortcut going from brightness 0, to 1,
> > to 2, and 3.
> >
> > However, for HID keyboards it is exposed as a key and handled by the
> > user's desktop environment. For the toggle button, this means that
> > brightness control becomes on/off. In addition, in the absence of a
> > DE, the keyboard brightness does not work.
> >
> > Therefore, expose an event handler for the keyboard brightness control
> > which can then be used by hid-asus. Since this handler is called from
> > an interrupt context, defer the actual work to a workqueue.
> >
> > In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> > for maximum brightness since it is shared between hid-asus/asus-wmi.
> >
> > Reviewed-by: Luke D. Jones <luke@ljones.dev>
> > Tested-by: Luke D. Jones <luke@ljones.dev>
> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> > drivers/platform/x86/asus-wmi.c | 46 +++++++++++++++++++---
> > include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
> > 2 files changed, 54 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index 84cde34ab6a8..a69464e45ca4 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
> > }
> > }
> >
> > +/*
> > + * This function is called from hid-asus to inform asus-wmi of brightness
> > + * changes initiated by the keyboard backlight keys.
> > + */
> > +int asus_hid_event(enum asus_hid_event event)
> > +{
> > + struct asus_wmi *asus;
> > + int brightness;
> > +
> > + guard(spinlock_irqsave)(&asus_ref.lock);
> > + asus = asus_ref.asus;
> > + if (!asus || !asus->kbd_led_registered)
> > + return -EBUSY;
> > +
> > + brightness = asus->kbd_led_wk;
> > +
> > + switch (event) {
> > + case ASUS_EV_BRTUP:
> > + brightness += 1;
> > + break;
> > + case ASUS_EV_BRTDOWN:
> > + brightness -= 1;
> > + break;
> > + case ASUS_EV_BRTTOGGLE:
> > + if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> > + brightness = 0;
> > + else
> > + brightness += 1;
> > + break;
> > + }
> > +
> > + asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> > + asus->kbd_led_notify = true;
> > + queue_work(asus->led_workqueue, &asus->kbd_led_work);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_hid_event);
> > +
> > /*
> > * These functions actually update the LED's, and are called from a
> LEDs as opposed to LED's?
I agree with you, but the author of that line probably wouldn't -
depends on author dialect and both are usually correct.
When making acronyms plural, adding a ' is usually accepted as
correct. But this is not added as part of this series, so you can do a
reword commit if you wish
Antheas
> > * workqueue. By doing this as separate work rather than when the LED
> > @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> > {
> > struct asus_hid_listener *listener;
> > struct asus_wmi *asus;
> > - int max_level;
> >
> > asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> > - max_level = asus->kbd_led.max_brightness;
> >
> > scoped_guard(spinlock_irqsave, &asus_ref.lock)
> > - asus->kbd_led_wk = clamp_val(value, 0, max_level);
> > + asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
> >
> > if (asus->kbd_led_avail)
> > kbd_led_update(asus);
> > @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> > asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> > asus->kbd_led.brightness_set = kbd_led_set;
> > asus->kbd_led.brightness_get = kbd_led_get;
> > - asus->kbd_led.max_brightness = 3;
> > + asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> > asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> > INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
> >
> > @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> > return;
> > }
> > if (code == NOTIFY_KBD_BRTTOGGLE) {
> > - if (led_value == asus->kbd_led.max_brightness)
> > + if (led_value == ASUS_EV_MAX_BRIGHTNESS)
> > kbd_led_set_by_kbd(asus, 0);
> > else
> > kbd_led_set_by_kbd(asus, led_value + 1);
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > index d347cffd05d5..7b872b5d0960 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -178,6 +178,14 @@ struct asus_hid_listener {
> > void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> > };
> >
> > +enum asus_hid_event {
> > + ASUS_EV_BRTUP,
> > + ASUS_EV_BRTDOWN,
> > + ASUS_EV_BRTTOGGLE,
> > +};
> > +
> > +#define ASUS_EV_MAX_BRIGHTNESS 3
> > +
> > #if IS_REACHABLE(CONFIG_ASUS_WMI)
> > void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> > void set_ally_mcu_powersave(bool enabled);
> > @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> > int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> > int asus_hid_register_listener(struct asus_hid_listener *cdev);
> > void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> > +int asus_hid_event(enum asus_hid_event event);
> > #else
> > static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> > {
> > @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> > static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> > {
> > }
> > +static inline int asus_hid_event(enum asus_hid_event event)
> > +{
> > + return -ENODEV;
> > +}
> > #endif
> >
> > #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
>
^ permalink raw reply
* Re: [PATCH v10 02/11] HID: asus: initialize additional endpoints only for legacy devices
From: Denis Benato @ 2025-11-26 20:27 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251122110032.4274-3-lkml@antheas.dev>
On 11/22/25 12:00, Antheas Kapenekakis wrote:
> Currently, ID1/ID2 initializations are performed for all NKEY devices.
> However, ID1 initializations are only required for RGB control and are
> only supported for RGB capable devices. ID2 initializations are only
> required for initializing the Anime display endpoint which is only
> supported on devices with an Anime display. Both of these
> initializations are out of scope for this driver (this is a brightness
> control and keyboard shortcut driver) and they should not be performed
> for devices that do not support them in any case.
>
> At the same time, there are older NKEY devices that have only been
> tested with these initializations in the kernel and it is not possible
> to recheck them. There is a possibility that especially with the ID1
> initialization, certain laptop models might have their shortcuts stop
> working (currently unproven).
>
> For an abundance of caution, only initialize ID1/ID2 for those older
> NKEY devices by introducing a quirk for them and replacing the NKEY
> quirk in the block that performs the inits with that.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/hid/hid-asus.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a82286a427b8..9004814fb0d3 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define QUIRK_ROG_NKEY_KEYBOARD BIT(11)
> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> #define QUIRK_ROG_ALLY_XPAD BIT(13)
> +#define QUIRK_ROG_NKEY_LEGACY BIT(14)
>
> #define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
> QUIRK_NO_INIT_REPORTS | \
> @@ -652,7 +653,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> return -ENODEV;
>
> - if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> + if (drvdata->quirks & QUIRK_ROG_NKEY_LEGACY) {
> ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> if (ret < 0)
> return ret;
> @@ -1376,10 +1377,10 @@ static const struct hid_device_id asus_devices[] = {
> QUIRK_USE_KBD_BACKLIGHT },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD),
> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
> - QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
> + QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_NKEY_LEGACY },
> { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
> USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
> QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
^ permalink raw reply
* Re: [PATCH v10 10/11] platform/x86: asus-wmi: add keyboard brightness event handler
From: Denis Benato @ 2025-11-26 20:23 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251122110032.4274-11-lkml@antheas.dev>
On 11/22/25 12:00, Antheas Kapenekakis wrote:
> The keyboard brightness control of Asus WMI keyboards is handled in
> kernel, which leads to the shortcut going from brightness 0, to 1,
> to 2, and 3.
>
> However, for HID keyboards it is exposed as a key and handled by the
> user's desktop environment. For the toggle button, this means that
> brightness control becomes on/off. In addition, in the absence of a
> DE, the keyboard brightness does not work.
>
> Therefore, expose an event handler for the keyboard brightness control
> which can then be used by hid-asus. Since this handler is called from
> an interrupt context, defer the actual work to a workqueue.
>
> In the process, introduce ASUS_EV_MAX_BRIGHTNESS to hold the constant
> for maximum brightness since it is shared between hid-asus/asus-wmi.
>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Tested-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/platform/x86/asus-wmi.c | 46 +++++++++++++++++++---
> include/linux/platform_data/x86/asus-wmi.h | 13 ++++++
> 2 files changed, 54 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 84cde34ab6a8..a69464e45ca4 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -1719,6 +1719,44 @@ static void kbd_led_update_all(struct work_struct *work)
> }
> }
>
> +/*
> + * This function is called from hid-asus to inform asus-wmi of brightness
> + * changes initiated by the keyboard backlight keys.
> + */
> +int asus_hid_event(enum asus_hid_event event)
> +{
> + struct asus_wmi *asus;
> + int brightness;
> +
> + guard(spinlock_irqsave)(&asus_ref.lock);
> + asus = asus_ref.asus;
> + if (!asus || !asus->kbd_led_registered)
> + return -EBUSY;
> +
> + brightness = asus->kbd_led_wk;
> +
> + switch (event) {
> + case ASUS_EV_BRTUP:
> + brightness += 1;
> + break;
> + case ASUS_EV_BRTDOWN:
> + brightness -= 1;
> + break;
> + case ASUS_EV_BRTTOGGLE:
> + if (brightness >= ASUS_EV_MAX_BRIGHTNESS)
> + brightness = 0;
> + else
> + brightness += 1;
> + break;
> + }
> +
> + asus->kbd_led_wk = clamp_val(brightness, 0, ASUS_EV_MAX_BRIGHTNESS);
> + asus->kbd_led_notify = true;
> + queue_work(asus->led_workqueue, &asus->kbd_led_work);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_event);
> +
> /*
> * These functions actually update the LED's, and are called from a
LEDs as opposed to LED's?
> * workqueue. By doing this as separate work rather than when the LED
> @@ -1801,13 +1839,11 @@ static void do_kbd_led_set(struct led_classdev *led_cdev, int value)
> {
> struct asus_hid_listener *listener;
> struct asus_wmi *asus;
> - int max_level;
>
> asus = container_of(led_cdev, struct asus_wmi, kbd_led);
> - max_level = asus->kbd_led.max_brightness;
>
> scoped_guard(spinlock_irqsave, &asus_ref.lock)
> - asus->kbd_led_wk = clamp_val(value, 0, max_level);
> + asus->kbd_led_wk = clamp_val(value, 0, ASUS_EV_MAX_BRIGHTNESS);
>
> if (asus->kbd_led_avail)
> kbd_led_update(asus);
> @@ -2011,7 +2047,7 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> asus->kbd_led.flags = LED_BRIGHT_HW_CHANGED;
> asus->kbd_led.brightness_set = kbd_led_set;
> asus->kbd_led.brightness_get = kbd_led_get;
> - asus->kbd_led.max_brightness = 3;
> + asus->kbd_led.max_brightness = ASUS_EV_MAX_BRIGHTNESS;
> asus->kbd_led_avail = !kbd_led_read(asus, &led_val, NULL);
> INIT_WORK(&asus->kbd_led_work, kbd_led_update_all);
>
> @@ -4530,7 +4566,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> return;
> }
> if (code == NOTIFY_KBD_BRTTOGGLE) {
> - if (led_value == asus->kbd_led.max_brightness)
> + if (led_value == ASUS_EV_MAX_BRIGHTNESS)
> kbd_led_set_by_kbd(asus, 0);
> else
> kbd_led_set_by_kbd(asus, led_value + 1);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index d347cffd05d5..7b872b5d0960 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -178,6 +178,14 @@ struct asus_hid_listener {
> void (*brightness_set)(struct asus_hid_listener *listener, int brightness);
> };
>
> +enum asus_hid_event {
> + ASUS_EV_BRTUP,
> + ASUS_EV_BRTDOWN,
> + ASUS_EV_BRTTOGGLE,
> +};
> +
> +#define ASUS_EV_MAX_BRIGHTNESS 3
> +
> #if IS_REACHABLE(CONFIG_ASUS_WMI)
> void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
> void set_ally_mcu_powersave(bool enabled);
> @@ -186,6 +194,7 @@ int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval);
> int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> int asus_hid_register_listener(struct asus_hid_listener *cdev);
> void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> +int asus_hid_event(enum asus_hid_event event);
> #else
> static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> {
> @@ -213,6 +222,10 @@ static inline int asus_hid_register_listener(struct asus_hid_listener *bdev)
> static inline void asus_hid_unregister_listener(struct asus_hid_listener *bdev)
> {
> }
> +static inline int asus_hid_event(enum asus_hid_event event)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
^ permalink raw reply
* Re: [PATCH v10 09/11] platform/x86: asus-wmi: remove unused keyboard backlight quirk
From: Denis Benato @ 2025-11-26 20:19 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251122110032.4274-10-lkml@antheas.dev>
On 11/22/25 12:00, Antheas Kapenekakis wrote:
> The quirk for selecting whether keyboard backlight should be controlled
> by HID or WMI is not needed anymore, so remove the file containing it.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> .../platform_data/x86/asus-wmi-leds-ids.h | 50 -------------------
> 1 file changed, 50 deletions(-)
> delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>
> diff --git a/include/linux/platform_data/x86/asus-wmi-leds-ids.h b/include/linux/platform_data/x86/asus-wmi-leds-ids.h
> deleted file mode 100644
> index 034a039c4e37..000000000000
> --- a/include/linux/platform_data/x86/asus-wmi-leds-ids.h
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
> -#define __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H
> -
> -#include <linux/dmi.h>
> -#include <linux/types.h>
> -
> -/* To be used by both hid-asus and asus-wmi to determine which controls kbd_brightness */
> -#if IS_REACHABLE(CONFIG_ASUS_WMI) || IS_REACHABLE(CONFIG_HID_ASUS)
> -static const struct dmi_system_id asus_use_hid_led_dmi_ids[] = {
> - {
> - .matches = {
> - DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Zephyrus"),
> - },
> - },
> - {
> - .matches = {
> - DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Strix"),
> - },
> - },
> - {
> - .matches = {
> - DMI_MATCH(DMI_PRODUCT_FAMILY, "ROG Flow"),
> - },
> - },
> - {
> - .matches = {
> - DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
> - },
> - },
> - {
> - .matches = {
> - DMI_MATCH(DMI_BOARD_NAME, "GA403U"),
> - },
> - },
> - {
> - .matches = {
> - DMI_MATCH(DMI_BOARD_NAME, "GU605M"),
> - },
> - },
> - {
> - .matches = {
> - DMI_MATCH(DMI_BOARD_NAME, "RC71L"),
> - },
> - },
> - { },
> -};
> -#endif
> -
> -#endif /* __PLATFORM_DATA_X86_ASUS_WMI_LEDS_IDS_H */
^ permalink raw reply
* Re: [PATCH v10 08/11] HID: asus: listen to the asus-wmi brightness device instead of creating one
From: Denis Benato @ 2025-11-26 20:19 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251122110032.4274-9-lkml@antheas.dev>
On 11/22/25 12:00, Antheas Kapenekakis wrote:
> Some ROG laptops expose multiple interfaces for controlling the
> keyboard/RGB brightness. This creates a name conflict under
> asus::kbd_brightness, where the second device ends up being
> named asus::kbd_brightness_1 and they are both broken.
>
> Therefore, register a listener to the asus-wmi brightness device
> instead of creating a new one.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> Reviewed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/hid/hid-asus.c | 65 +++++++-----------------------------------
> 1 file changed, 10 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 461b0f8efdfb..475e34187326 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -27,7 +27,6 @@
> #include <linux/hid.h>
> #include <linux/module.h>
> #include <linux/platform_data/x86/asus-wmi.h>
> -#include <linux/platform_data/x86/asus-wmi-leds-ids.h>
> #include <linux/input/mt.h>
> #include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
> #include <linux/power_supply.h>
> @@ -103,7 +102,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define TRKID_SGN ((TRKID_MAX + 1) >> 1)
>
> struct asus_kbd_leds {
> - struct led_classdev cdev;
> + struct asus_hid_listener listener;
> struct hid_device *hdev;
> struct work_struct work;
> unsigned int brightness;
> @@ -493,11 +492,11 @@ static void asus_schedule_work(struct asus_kbd_leds *led)
> spin_unlock_irqrestore(&led->lock, flags);
> }
>
> -static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> - enum led_brightness brightness)
> +static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
> + int brightness)
> {
> - struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> - cdev);
> + struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
> + listener);
> unsigned long flags;
>
> spin_lock_irqsave(&led->lock, flags);
> @@ -507,20 +506,6 @@ static void asus_kbd_backlight_set(struct led_classdev *led_cdev,
> asus_schedule_work(led);
> }
>
> -static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev)
> -{
> - struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds,
> - cdev);
> - enum led_brightness brightness;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&led->lock, flags);
> - brightness = led->brightness;
> - spin_unlock_irqrestore(&led->lock, flags);
> -
> - return brightness;
> -}
> -
> static void asus_kbd_backlight_work(struct work_struct *work)
> {
> struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
> @@ -537,34 +522,6 @@ static void asus_kbd_backlight_work(struct work_struct *work)
> hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> }
>
> -/* WMI-based keyboard backlight LED control (via asus-wmi driver) takes
> - * precedence. We only activate HID-based backlight control when the
> - * WMI control is not available.
> - */
> -static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
> -{
> - struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> - u32 value;
> - int ret;
> -
> - if (!IS_ENABLED(CONFIG_ASUS_WMI))
> - return false;
> -
> - if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
> - dmi_check_system(asus_use_hid_led_dmi_ids)) {
> - hid_info(hdev, "using HID for asus::kbd_backlight\n");
> - return false;
> - }
> -
> - ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
> - ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
> - hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
> - if (ret)
> - return false;
> -
> - return !!(value & ASUS_WMI_DSTS_PRESENCE_BIT);
> -}
> -
> /*
> * We don't care about any other part of the string except the version section.
> * Example strings: FGA80100.RC72LA.312_T01, FGA80100.RC71LS.318_T01
> @@ -709,14 +666,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> drvdata->kbd_backlight->removed = false;
> drvdata->kbd_backlight->brightness = 0;
> drvdata->kbd_backlight->hdev = hdev;
> - drvdata->kbd_backlight->cdev.name = "asus::kbd_backlight";
> - drvdata->kbd_backlight->cdev.max_brightness = 3;
> - drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set;
> - drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get;
> + drvdata->kbd_backlight->listener.brightness_set = asus_kbd_backlight_set;
> INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work);
> spin_lock_init(&drvdata->kbd_backlight->lock);
>
> - ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev);
> + ret = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
> if (ret < 0) {
> /* No need to have this still around */
> devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> @@ -1105,7 +1059,7 @@ static int __maybe_unused asus_resume(struct hid_device *hdev) {
>
> if (drvdata->kbd_backlight) {
> const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
> - drvdata->kbd_backlight->cdev.brightness };
> + drvdata->kbd_backlight->brightness };
> ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> if (ret < 0) {
> hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
> @@ -1231,7 +1185,6 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> }
>
> if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
> - !asus_kbd_wmi_led_control_present(hdev) &&
> asus_kbd_register_leds(hdev))
> hid_warn(hdev, "Failed to initialize backlight.\n");
>
> @@ -1278,6 +1231,8 @@ static void asus_remove(struct hid_device *hdev)
> unsigned long flags;
>
> if (drvdata->kbd_backlight) {
> + asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> +
> spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags);
> drvdata->kbd_backlight->removed = true;
> spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags);
^ permalink raw reply
* Re: [PATCH v10 04/11] HID: asus: fortify keyboard handshake
From: Denis Benato @ 2025-11-26 20:09 UTC (permalink / raw)
To: Antheas Kapenekakis, platform-driver-x86, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Corentin Chary,
Luke D . Jones, Hans de Goede, Ilpo Järvinen
In-Reply-To: <20251122110032.4274-5-lkml@antheas.dev>
On 11/22/25 12:00, Antheas Kapenekakis wrote:
> Handshaking with an Asus device involves sending it a feature report
> with the string "ASUS Tech.Inc." and then reading it back to verify the
> handshake was successful, under the feature ID the interaction will
> take place.
>
> Currently, the driver only does the first part. Add the readback to
> verify the handshake was successful. As this could cause breakages,
> allow the verification to fail with a dmesg error until we verify
> all devices work with it (they seem to).
>
> Since the response is more than 16 bytes, increase the buffer size
> to 64 as well to avoid overflow errors.
Reviewed-by: Denis Benato <benato.denis96@gmail.com>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/hid/hid-asus.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index c1a36a8beb35..2a412e10f916 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -49,7 +49,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> #define FEATURE_REPORT_ID 0x0d
> #define INPUT_REPORT_ID 0x5d
> #define FEATURE_KBD_REPORT_ID 0x5a
> -#define FEATURE_KBD_REPORT_SIZE 16
> +#define FEATURE_KBD_REPORT_SIZE 64
> #define FEATURE_KBD_LED_REPORT_ID1 0x5d
> #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>
> @@ -395,13 +395,37 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
>
> static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
> {
> + /*
> + * The handshake is first sent as a set_report, then retrieved
> + * from a get_report. They should be equal.
> + */
> const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
> 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> int ret;
>
> ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> - if (ret < 0)
> - hid_err(hdev, "Asus failed to send init command: %d\n", ret);
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to send handshake: %d\n", ret);
> + return ret;
> + }
> +
> + u8 *readbuf __free(kfree) = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> + if (!readbuf)
> + return -ENOMEM;
> +
> + ret = hid_hw_raw_request(hdev, report_id, readbuf,
> + FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
> + if (ret < 0) {
> + hid_err(hdev, "Asus failed to receive handshake ack: %d\n", ret);
> + } else if (memcmp(readbuf, buf, sizeof(buf)) != 0) {
> + hid_warn(hdev, "Asus handshake returned invalid response: %*ph\n",
> + FEATURE_KBD_REPORT_SIZE, readbuf);
> + /*
> + * Do not return error if handshake is wrong until this is
> + * verified to work for all devices.
> + */
> + }
>
> return ret;
> }
^ permalink raw reply
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-11-26 19:58 UTC (permalink / raw)
To: Ilpo Järvinen, Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede
In-Reply-To: <b96548e0-d788-b9bf-d8ae-33df98bdf814@linux.intel.com>
On 11/26/25 16:31, Ilpo Järvinen wrote:
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>
>> On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>> On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
>>> <ilpo.jarvinen@linux.intel.com> wrote:
>>>> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>>>>
>>>>> On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>>>> This is a two part series which does the following:
>>>>>> - Clean-up init sequence
>>>>>> - Unify backlight handling to happen under asus-wmi so that all Aura
>>>>>> devices have synced brightness controls and the backlight button works
>>>>>> properly when it is on a USB laptop keyboard instead of one w/ WMI.
>>>>>>
>>>>>> For more context, see cover letter of V1. Since V5, I removed some patches
>>>>>> to make this easier to merge.
>>>>> Slight bump on this. It addresses both of the remarks Denis made in
>>>>> the previous version.
>>>>>
>>>>> I begrudgingly argued a bit for those because I did not want to resend
>>>>> the series and they were not functional changes, so sorry about that.
>>>>> But they are fixed in this version incl. with the conflict with the
>>>>> armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
>>>>> Add support for multiple kbd led handlers" even though I addressed
>>>>> your comment, so you may want to add that.
>>>> FYI, there's no direct relation that mandates a person to give a rev-by
>>>> even if all his/her comments were addressed.
>>> True, this is just a reminder because I did not hear from him and
>>> since he added a rev-by on the kfree patch. There's no obligation from
>>> my side.
Lack of time: I will review the rest shortly unless some more unexpected
thing shows up.
>>>> But generally yes, it would be useful to hear whether Denis is fine with
>>>> v10, especially those patches that had contention earlier but you've
>>>> modified post-v8.
>>>>
>>>>> As for "HID: asus: early return for ROG devices" changing the name of
>>>>> the devices of this driver, I will veto backporting it if it happens,
>>>>> so inputplumber will have the two full months to remove the name
>>>>> match. This is not a breaking change in the sense that software cannot
>>>>> be made to work on both previous and latter versions and there is no
>>>>> other software to my knowledge relying on name matches for Asus
>>>>> keyboards.
>>>> Did Hans give some opinion about this rename earlier, at least I don't
>>>> remember nor could find from lore archives?
>>> Hans jumped in on the ayaneo controller patch. I don't think I saw
>>> activity on this series
>> Hans had some feedback around half a year ago for the latter part of
>> this series that binds the devices together
> Yes but I was interested on specifically about this userspace visible
> change. He has been around much much longer than me so his insight on
> userspace visible changes is way more valuable than mine.
>
> --
> i.
>
>>> Antheas
>>>
>>>> --
>>>> i.
>>>>
>>>>> Moreover, an early exit is needed to prevent ejecting HID
>>>>> endpoints without an ->input parameter so it is a needed fix anyway.
>>>>> Postponing it will prevent Xbox Ally users from having RGB control
>>>>> through userspace on a stock kernel but it is also not worth arguing
>>>>> about
>>>>>
>>>>> It is also fine for me for this series to merge for 6.20, but I'd
>>>>> rather we handle it now since there will be some turbulence for asus
>>>>> users due to armoury merging so it makes sense to have this transition
>>>>> once.
>>>>>
>>>>>> ---
>>>>>> V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
>>>>>> V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
>>>>>> V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
>>>>>> V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
>>>>>> V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
>>>>>> V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
>>>>>> V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
>>>>>> V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
>>>>>> V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
>>>>>>
>>>>>> Changes since V9:
>>>>>> - No functional changes
>>>>>> - Rebase to review-ilpo-next
>>>>>> - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
>>>>>> "remove unused keyboard backlight quirk" + imports
>>>>>> Dismiss Luke's review as this patch diverged
>>>>>> - Reword paragraph in "Add support for multiple kbd led handlers" to be
>>>>>> more verbose
>>>>>> - Use kfree in fortify patch
>>>>>> - Fix minor style quirks from --nonstict checkpatch run
>>>>>>
>>>>>> Changes since V8:
>>>>>> - No functional changes
>>>>>> - Move legacy init patch to second, modify first patch so that their
>>>>>> diff is minimized
>>>>>> - Split "prevent binding to all HID devices on ROG" into two patches:
>>>>>> - moving backlight initialization into probe
>>>>>> - early exit to skip ->init check and rename
>>>>>> - Remove skipping vendor fixups for non-vendor devices. It is not possible
>>>>>> to read usages before the report fixups are applied, so it did not work
>>>>>> - In that patch, reword a comment to be single line and make is_vendor a bool
>>>>>> - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
>>>>>> has drifted too far since he reviewed/tested it.
>>>>>>
>>>>>> Changes since V7:
>>>>>> - Readd legacy init quirk for Dennis
>>>>>> - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
>>>>>> - Fix warning due to enum_backlight receiving negative values
>>>>>>
>>>>>> Changes since V6:
>>>>>> - Split initialization refactor into three patches, update commit text
>>>>>> to be clearer in what it does
>>>>>> - Replace spinlock accesses with guard and scoped guard in all patches
>>>>>> - Add missing includes mentioned by Ilpo
>>>>>> - Reflow, tweak comment in prevent binding to all HID devices on ROG
>>>>>> - Replace asus_ref.asus with local reference in all patches
>>>>>> - Add missing kernel doc comments
>>>>>> - Other minor nits from Ilpo
>>>>>> - User reported warning due to scheduling work while holding a spinlock.
>>>>>> Restructure patch for multiple handlers to limit when spinlock is held to
>>>>>> variable access only. In parallel, setup a workqueue to handle registration
>>>>>> of led device and setting brightness. This is required as registering the
>>>>>> led device triggers kbd_led_get which needs to hold the spinlock to
>>>>>> protect the led_wk value. The workqueue is also required for the hid
>>>>>> event passthrough to avoid scheduling work while holding the spinlock.
>>>>>> Apply the workqueue to wmi brightness buttons as well, as that was
>>>>>> omitted before this series and WMI access was performed.
>>>>>> - On "HID: asus: prevent binding to all HID devices on ROG", rename
>>>>>> quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
>>>>>> This allows other quirks to apply (applies quirk that fixes keyboard
>>>>>> being named as a pointer device).
>>>>>>
>>>>>> Changes since V5:
>>>>>> - It's been a long time
>>>>>> - Remove addition of RGB as that had some comments I need to work on
>>>>>> - Remove folio patch (already merged)
>>>>>> - Remove legacy fix patch 11 from V4. There is a small chance that
>>>>>> without this patch, some old NKEY keyboards might not respond to
>>>>>> RGB commands according to Luke, but the kernel driver does not do
>>>>>> RGB currently. The 0x5d init is done by Armoury crate software in
>>>>>> Windows. If an issue is found, we can re-add it or just remove patches
>>>>>> 1/2 before merging. However, init could use the cleanup.
>>>>>>
>>>>>> Changes since V4:
>>>>>> - Fix KConfig (reported by kernel robot)
>>>>>> - Fix Ilpo's nits, if I missed anything lmk
>>>>>>
>>>>>> Changes since V3:
>>>>>> - Add initializer for 0x5d for old NKEY keyboards until it is verified
>>>>>> that it is not needed for their media keys to function.
>>>>>> - Cover init in asus-wmi with spinlock as per Hans
>>>>>> - If asus-wmi registers WMI handler with brightness, init the brightness
>>>>>> in USB Asus keyboards, per Hans.
>>>>>> - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
>>>>>> - Fix oops when unregistering asus-wmi by moving unregister outside of
>>>>>> the spin lock (but after the asus reference is set to null)
>>>>>>
>>>>>> Changes since V2:
>>>>>> - Check lazy init succeds in asus-wmi before setting register variable
>>>>>> - make explicit check in asus_hid_register_listener for listener existing
>>>>>> to avoid re-init
>>>>>> - rename asus_brt to asus_hid in most places and harmonize everything
>>>>>> - switch to a spinlock instead of a mutex to avoid kernel ooops
>>>>>> - fixup hid device quirks to avoid multiple RGB devices while still exposing
>>>>>> all input vendor devices. This includes moving rgb init to probe
>>>>>> instead of the input_configured callbacks.
>>>>>> - Remove fan key (during retest it appears to be 0xae that is already
>>>>>> supported by hid-asus)
>>>>>> - Never unregister asus::kbd_backlight while asus-wmi is active, as that
>>>>>> - removes fds from userspace and breaks backlight functionality. All
>>>>>> - current mainline drivers do not support backlight hotplugging, so most
>>>>>> userspace software (e.g., KDE, UPower) is built with that assumption.
>>>>>> For the Ally, since it disconnects its controller during sleep, this
>>>>>> caused the backlight slider to not work in KDE.
>>>>>>
>>>>>> Changes since V1:
>>>>>> - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
>>>>>> - Fix ifdef else having an invalid signature (reported by kernel robot)
>>>>>> - Restore input arguments to init and keyboard function so they can
>>>>>> be re-used for RGB controls.
>>>>>> - Remove Z13 delay (it did not work to fix the touchpad) and replace it
>>>>>> with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
>>>>>> keyboard rename into it.
>>>>>> - Unregister brightness listener before removing work queue to avoid
>>>>>> a race condition causing corruption
>>>>>> - Remove spurious mutex unlock in asus_brt_event
>>>>>> - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
>>>>>> relocking the mutex and causing a deadlock when unregistering leds
>>>>>> - Add extra check during unregistering to avoid calling unregister when
>>>>>> no led device is registered.
>>>>>> - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
>>>>>> the driver to create 4 RGB handlers per device. I also suspect some
>>>>>> extra events sneak through (KDE had the @@@@@@).
>>>>>>
>>>>>> Antheas Kapenekakis (11):
>>>>>> HID: asus: simplify RGB init sequence
>>>>>> HID: asus: initialize additional endpoints only for legacy devices
>>>>>> HID: asus: use same report_id in response
>>>>>> HID: asus: fortify keyboard handshake
>>>>>> HID: asus: move vendor initialization to probe
>>>>>> HID: asus: early return for ROG devices
>>>>>> platform/x86: asus-wmi: Add support for multiple kbd led handlers
>>>>>> HID: asus: listen to the asus-wmi brightness device instead of
>>>>>> creating one
>>>>>> platform/x86: asus-wmi: remove unused keyboard backlight quirk
>>>>>> platform/x86: asus-wmi: add keyboard brightness event handler
>>>>>> HID: asus: add support for the asus-wmi brightness handler
>>>>>>
>>>>>> drivers/hid/hid-asus.c | 205 ++++++++--------
>>>>>> drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
>>>>>> .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
>>>>>> include/linux/platform_data/x86/asus-wmi.h | 28 +++
>>>>>> 4 files changed, 322 insertions(+), 184 deletions(-)
>>>>>> delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
>>>>>>
>>>>>>
>>>>>> base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
>>>>>> --
>>>>>> 2.52.0
>>>>>>
>>>>>>
^ permalink raw reply
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-11-26 19:57 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones
In-Reply-To: <4bcc6708-77ff-403d-83cf-10a40934263a@kernel.org>
On 11/26/25 18:34, Hans de Goede wrote:
> Hi,
>
> On 26-Nov-25 4:23 PM, Ilpo Järvinen wrote:
>> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> ...
>
>>> As for "HID: asus: early return for ROG devices" changing the name of
>>> the devices of this driver, I will veto backporting it if it happens,
>>> so inputplumber will have the two full months to remove the name
>>> match. This is not a breaking change in the sense that software cannot
>>> be made to work on both previous and latter versions and there is no
>>> other software to my knowledge relying on name matches for Asus
>>> keyboards.
>> Did Hans give some opinion about this rename earlier, at least I don't
>> remember nor could find from lore archives?
> I don't remember commenting on this myself either.
>
> So generally speaking there are plenty of cases where /dev/input/event#
> nodes for a specific device have their name changed by some kernel patches.
>
> Typically HID input devices are matched in userspace by their
> bus:vend-id:prod-id triplet not by the name. The name might even
> change by a fwupdate of the device itself.
>
> So I'm not overly worried about this and inputplumber seems nice
> enough and already is very much not a plug-and-play tool.
>
> One possible concern with laptop keyboard input-device name changes
> though is hwdb entries to fixup scancode -> ev-key-code mappings.
>
> See: /lib/udev/hwdb.d/60-keyboard.hwdb on any standard Linux systems
> an then the big comment at the top.
>
> An input-device name change might break this match pattern:
>
> # - Input driver device name and DMI data match:
> # evdev:name:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn*
> # <input device name> is the name device specified by the
> # driver, <vendor> is the firmware-provided string exported
> # by the kernel DMI modalias, see /sys/class/dmi/id/modalias
>
> As well as the extended version of this and for laptops with USB
> keyboards this is the only match type which allows a DMI match
> which is what we want for laptop kbd mappings. Looking at the Asus
> section of the upstream 60-keyboard.hwdb I do not see any such
> matches though.
>
> There not being such matches kinda make sense since for USB-HID
> devices any special scancode -> ev-key-code mappings are typically
> handled in a vendor specific HID driver like hid-asus.
>
> TL;DR: I think that the input-device name should be fine.
Thank you very much for this write-up! Now I feel much more confident
in giving approval!
> Regards,
>
> Hans
Thank you,
Denis
^ permalink raw reply
* Re: [PATCH v12 2/3] HID: cp2112: Fwnode Support
From: Danny Kaehn @ 2025-11-26 19:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Andi Shyti,
Conor Dooley, Jiri Kosina, devicetree, linux-input,
Dmitry Torokhov, Bartosz Golaszewski, Ethan Twardy, linux-i2c,
linux-kernel, Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <aSdGh3i_KYocE3L3@smile.fi.intel.com>
Hi Andy,
Thanks for the review!
On Wed, Nov 26, 2025 at 08:27:19PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 26, 2025 at 11:05:25AM -0600, Danny Kaehn wrote:
> > Support describing the CP2112's I2C and GPIO interfaces in firmware.
> >
> > Bindings between the firmware nodes and the functions of the device
> > are distinct between ACPI and DeviceTree.
> >
> > For ACPI, the i2c_adapter will use the child with _ADR Zero and the
> > gpio_chip will use the child with _ADR One. For DeviceTree, the
> > i2c_adapter will use the child with name "i2c", but the gpio_chip
> > will share a firmware node with the CP2112.
>
> Hmm... Is there any explanation why DT decided to go that way?
>
I don't have an explanation, but Rob H. had directed that I make this
change in [1].
In v11, I then removed that child node for both ACPI and DT, hoping to
maintain unity, but you had directed that wouldn't be intuitive for ACPI
in [2].
Thus, in this v12, I have just entirely split the two, as it seemed
unlikely that any compromise to unify the schema between the two
firmware languages would be possible for a change/driver this
inconsquential to the overall kernel.
[1]:
https://lore.kernel.org/all/20240213152825.GA1223720-robh@kernel.org/
[2]:
https://lore.kernel.org/all/ZmISaEIGlxZVK_jf@smile.fi.intel.com/
> ...
>
> > + if (is_acpi_device_node(hdev->dev.fwnode)) {
>
> Please, do not dereference fwnode, use dev_fwnode() or other APIs for that
> (actually the same applies to OF node, but people too much neglect that).
>
Thanks, will do.
> > + device_for_each_child_node(&hdev->dev, child) {
> > + ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > + if (ret)
> > + continue;
> > +
> > + switch (addr) {
> > + case CP2112_I2C_ADR:
> > + device_set_node(&dev->adap.dev, child);
> > + break;
> > + case CP2112_GPIO_ADR:
> > + dev->gc.fwnode = child;
> > + break;
>
> If by any chance we have malformed table and there are more devices with
> the same address? Maybe we don't need to address this right now, just
> asking... (I believe ACPI compiler won't allow that, but table can be
> crafted directly in the binary format.)
>
You're sugggesting perhaps that we explicitly keep track of which
addresses have been encountered, and refuse to do any fwnode parsing
if we detect the same address used twice? I believe the current behavior
would be that the "last node wins"; not sure if it should be a "first node
wins" or a full error scenario...
> > + }
> > + }
> > + } else {
> > + device_set_node(&dev->adap.dev,
> > + device_get_named_child_node(&hdev->dev, "i2c"));
>
> Here we bump the reference count, where is it going to be dropped?
>
> Note, in the other branch (ACPI) the reference count is not bumped in
> the current code.
>
Great point, forgot that I had dropped that handling in v9. The old
behavior was that the CP2112 driver maintained a reference to each node
during the lifetime of the device (and released during probe errors,
etc..). I'm still a bit confused as to whether that is correct or not,
or if the references should immediately be dropped once they're done
being parsed during probe()... My understanding previously was that I
should keep the reference count for the child fwnodes for the lifetime
of the CP2112, since the pointers to those are stored in the child
devices but would usually be managed by the parent bus-level code, does
that seem correct?
> > + }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Thanks,
Danny Kaehn
^ permalink raw reply
* Re: [PATCH v12 0/3] Firmware Support for USB-HID Devices and CP2112
From: Andy Shevchenko @ 2025-11-26 18:29 UTC (permalink / raw)
To: Danny Kaehn
Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Andi Shyti,
Conor Dooley, Jiri Kosina, devicetree, linux-input,
Dmitry Torokhov, Bartosz Golaszewski, Ethan Twardy, linux-i2c,
linux-kernel, Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <20251126-cp2112-dt-v12-0-2cdba6481db3@plexus.com>
On Wed, Nov 26, 2025 at 11:05:23AM -0600, Danny Kaehn wrote:
> This patchset allows USB-HID devices to have Firmware bindings through sharing
> the USB fwnode with the HID driver, and adds such a binding and driver
> implementation for the CP2112 USB to SMBus Bridge (which necessitated the
> USB-HID change). This change allows a CP2112 permanently attached in hardware to
> be described in DT and ACPI and interoperate with other drivers.
>
> Changes in v12:
> - dt-binding changes:
> - Drop "on the host controller" from top-level description based on
> comment from Rob H.
> - Correct "Properties must precede subnodes" dt_binding_check error by
> moving gpio_chip-related properties above the i2c subnode in the
> binding and in the example.
> - Include `interrupt-controller` property in the example
> - Modify hid-cp2112.c to support separate schemas for DT vs. ACPI - DT
> combines gpio subnode with the CP2112's node, but will have an I2C
> subnode; while ACPI will maintain separate child nodes for the GPIO
> I2C devices
Thanks for pursuing this! I have a few comments, but in general I'm fine with
the design.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v12 3/3] HID: cp2112: Configure I2C Bus Speed from Firmware
From: Andy Shevchenko @ 2025-11-26 18:28 UTC (permalink / raw)
To: Danny Kaehn
Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Andi Shyti,
Conor Dooley, Jiri Kosina, devicetree, linux-input,
Dmitry Torokhov, Bartosz Golaszewski, Ethan Twardy, linux-i2c,
linux-kernel, Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <20251126-cp2112-dt-v12-3-2cdba6481db3@plexus.com>
On Wed, Nov 26, 2025 at 11:05:26AM -0600, Danny Kaehn wrote:
> Now that the I2C adapter on the CP2112 can have an associated firmware
> node, set the bus speed based on firmware configuration
Missed period at the end.
With that being addressed,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v12 2/3] HID: cp2112: Fwnode Support
From: Andy Shevchenko @ 2025-11-26 18:27 UTC (permalink / raw)
To: Danny Kaehn
Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Andi Shyti,
Conor Dooley, Jiri Kosina, devicetree, linux-input,
Dmitry Torokhov, Bartosz Golaszewski, Ethan Twardy, linux-i2c,
linux-kernel, Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <20251126-cp2112-dt-v12-2-2cdba6481db3@plexus.com>
On Wed, Nov 26, 2025 at 11:05:25AM -0600, Danny Kaehn wrote:
> Support describing the CP2112's I2C and GPIO interfaces in firmware.
>
> Bindings between the firmware nodes and the functions of the device
> are distinct between ACPI and DeviceTree.
>
> For ACPI, the i2c_adapter will use the child with _ADR Zero and the
> gpio_chip will use the child with _ADR One. For DeviceTree, the
> i2c_adapter will use the child with name "i2c", but the gpio_chip
> will share a firmware node with the CP2112.
Hmm... Is there any explanation why DT decided to go that way?
...
> + if (is_acpi_device_node(hdev->dev.fwnode)) {
Please, do not dereference fwnode, use dev_fwnode() or other APIs for that
(actually the same applies to OF node, but people too much neglect that).
> + device_for_each_child_node(&hdev->dev, child) {
> + ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> + if (ret)
> + continue;
> +
> + switch (addr) {
> + case CP2112_I2C_ADR:
> + device_set_node(&dev->adap.dev, child);
> + break;
> + case CP2112_GPIO_ADR:
> + dev->gc.fwnode = child;
> + break;
If by any chance we have malformed table and there are more devices with
the same address? Maybe we don't need to address this right now, just
asking... (I believe ACPI compiler won't allow that, but table can be
crafted directly in the binary format.)
> + }
> + }
> + } else {
> + device_set_node(&dev->adap.dev,
> + device_get_named_child_node(&hdev->dev, "i2c"));
Here we bump the reference count, where is it going to be dropped?
Note, in the other branch (ACPI) the reference count is not bumped in
the current code.
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v12 2/3] HID: cp2112: Fwnode Support
From: Danny Kaehn @ 2025-11-26 17:05 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
Andy Shevchenko, Danny Kaehn, Andi Shyti, Conor Dooley
Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
Bartosz Golaszewski, Ethan Twardy, linux-i2c, linux-kernel,
Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <20251126-cp2112-dt-v12-0-2cdba6481db3@plexus.com>
Support describing the CP2112's I2C and GPIO interfaces in firmware.
Bindings between the firmware nodes and the functions of the device
are distinct between ACPI and DeviceTree.
For ACPI, the i2c_adapter will use the child with _ADR Zero and the
gpio_chip will use the child with _ADR One. For DeviceTree, the
i2c_adapter will use the child with name "i2c", but the gpio_chip
will share a firmware node with the CP2112.
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
drivers/hid/hid-cp2112.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 803b883ae875..fb301c27c712 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -29,6 +29,16 @@
#include <linux/usb/ch9.h>
#include "hid-ids.h"
+/**
+ * enum cp2112_child_acpi_cell_addrs - Child ACPI addresses for CP2112 sub-functions
+ * @CP2112_I2C_ADR: Address for I2C node
+ * @CP2112_GPIO_ADR: Address for GPIO node
+ */
+enum cp2112_child_acpi_cell_addrs {
+ CP2112_I2C_ADR = 0,
+ CP2112_GPIO_ADR = 1,
+};
+
#define CP2112_REPORT_MAX_LENGTH 64
#define CP2112_GPIO_CONFIG_LENGTH 5
#define CP2112_GPIO_GET_LENGTH 2
@@ -1208,7 +1218,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct cp2112_device *dev;
u8 buf[3];
struct cp2112_smbus_config_report config;
+ struct fwnode_handle *child;
struct gpio_irq_chip *girq;
+ u32 addr;
int ret;
dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1226,6 +1238,26 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
+ if (is_acpi_device_node(hdev->dev.fwnode)) {
+ device_for_each_child_node(&hdev->dev, child) {
+ ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
+ if (ret)
+ continue;
+
+ switch (addr) {
+ case CP2112_I2C_ADR:
+ device_set_node(&dev->adap.dev, child);
+ break;
+ case CP2112_GPIO_ADR:
+ dev->gc.fwnode = child;
+ break;
+ }
+ }
+ } else {
+ device_set_node(&dev->adap.dev,
+ device_get_named_child_node(&hdev->dev, "i2c"));
+ }
+
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
--
2.25.1
^ permalink raw reply related
* [PATCH v12 0/3] Firmware Support for USB-HID Devices and CP2112
From: Danny Kaehn @ 2025-11-26 17:05 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
Andy Shevchenko, Danny Kaehn, Andi Shyti, Conor Dooley
Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
Bartosz Golaszewski, Ethan Twardy, linux-i2c, linux-kernel,
Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
This patchset allows USB-HID devices to have Firmware bindings through sharing
the USB fwnode with the HID driver, and adds such a binding and driver
implementation for the CP2112 USB to SMBus Bridge (which necessitated the
USB-HID change). This change allows a CP2112 permanently attached in hardware to
be described in DT and ACPI and interoperate with other drivers.
Changes in v12:
- dt-binding changes:
- Drop "on the host controller" from top-level description based on
comment from Rob H.
- Correct "Properties must precede subnodes" dt_binding_check error by
moving gpio_chip-related properties above the i2c subnode in the
binding and in the example.
- Include `interrupt-controller` property in the example
- Modify hid-cp2112.c to support separate schemas for DT vs. ACPI - DT
combines gpio subnode with the CP2112's node, but will have an I2C
subnode; while ACPI will maintain separate child nodes for the GPIO
I2C devices
Changes in v11:
- Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
from Rob H.
- Edit hid-cp2112.c to match for ACPI index and fall back to matching by
name (instead of the other way around)
- Separate CP2112 I2C bus speed configuration into a separate patch
Changes in v10:
- Define an enumeration and mapping for CP2112 ACPI _ADRs and devicetree
child node names, and use these in the scanning of child nodes
- Address other miscellaneous
Changes in v9:
- Add _ADR-based ACPI binding of child nodes (I2C is _ADR Zero, GPIO is _ADR One)
- Use a loop-based approach for assigning child nodes within probe().
As a consequence, hid-cp2112.c no longer maintains references to the
child fwnodes during the lifetime of the device. (plese correct if this
is actually needed for this use-case)
Changes in v8:
- Apply Review tags retroactively to patches previously reviewed
Changes in v7:
- Use dev_fwnode when calling fwnod_handle_put in i2c_adapter in hid-cp2112.c
- Capitalize I2C and GPIO in commit message for patch 0003
Changes in v6:
- Fix fwnode_handle reference leaks in hid-cp21112.c
- Simplify hog node pattern in silabs,cp2112.yaml
Changes in v5:
- Use fwnode API instead of of_node api in hid-core.c and hid-cp2112.c
- Include sda-gpios and scl-gpios in silabs,cp2112.yaml
- Additional fixups to silabs,cp2112.yaml to address comments
- Remove ngpios property
- Constrain the hog pattern to a single naming scheme
- Remove unneeded properties from the gpio hog which are provided by
the parent schema
- Submit threaded interrupt bugfix separately from this patchset, as requested
Changes in v4:
- Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/i2c
Changes in v3:
- Additional fixups to silabs,cp2112.yaml to address comments
Changes in v2:
- Added more detail to silabs,cp2112.yaml dt-binding
- Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/input
- Added support for setting smbus clock-frequency from DT in hid-cp2112.c
- Added freeing of of_nodes on error paths of _probe in hid-cp2112.c
Danny Kaehn (3):
dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
HID: usbhid: Share USB device firmware node with child HID device
HID: cp2112: Fwnode Support
.../bindings/i2c/silabs,cp2112.yaml | 113 ++++++++++++++++++
drivers/hid/hid-cp2112.c | 50 ++++++++
drivers/hid/usbhid/hid-core.c | 2 +
3 files changed, 165 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
--
2.25.1
---
Danny Kaehn (3):
dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
HID: cp2112: Fwnode Support
HID: cp2112: Configure I2C Bus Speed from Firmware
.../devicetree/bindings/i2c/silabs,cp2112.yaml | 107 +++++++++++++++++++++
drivers/hid/hid-cp2112.c | 36 +++++++
2 files changed, 143 insertions(+)
---
base-commit: 1c772200c9dcb23a304f84a9334fe2e0d9529ab0
change-id: 20240605-cp2112-dt-7cdc95448e8a
Best regards,
--
Danny Kaehn <danny.kaehn@plexus.com>
^ permalink raw reply
* [PATCH v12 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
From: Danny Kaehn @ 2025-11-26 17:05 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
Andy Shevchenko, Danny Kaehn, Andi Shyti, Conor Dooley
Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
Bartosz Golaszewski, Ethan Twardy, linux-i2c, linux-kernel,
Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <20251126-cp2112-dt-v12-0-2cdba6481db3@plexus.com>
This is a USB HID device which includes an I2C controller and 8 GPIO pins.
The binding allows describing the chip's gpio and i2c controller in DT,
with the i2c controller being bound to a subnode named "i2c". This is
intended to be used in configurations where the CP2112 is permanently
connected in hardware.
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
.../devicetree/bindings/i2c/silabs,cp2112.yaml | 107 +++++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
new file mode 100644
index 000000000000..4b5c1af3673d
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/silabs,cp2112.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CP2112 HID USB to SMBus/I2C Bridge
+
+maintainers:
+ - Danny Kaehn <danny.kaehn@plexus.com>
+
+description:
+ The CP2112 is a USB HID device which includes an integrated I2C controller
+ and 8 GPIO pins. Its GPIO pins can each be configured as inputs, open-drain
+ outputs, or push-pull outputs.
+
+properties:
+ compatible:
+ const: usb10c4,ea90
+
+ reg:
+ maxItems: 1
+ description: The USB port number
+
+ interrupt-controller: true
+ "#interrupt-cells":
+ const: 2
+
+ gpio-controller: true
+ "#gpio-cells":
+ const: 2
+
+ gpio-line-names:
+ minItems: 1
+ maxItems: 8
+
+ i2c:
+ description: The SMBus/I2C controller node for the CP2112
+ $ref: /schemas/i2c/i2c-controller.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ sda-gpios:
+ maxItems: 1
+
+ scl-gpios:
+ maxItems: 1
+
+ clock-frequency:
+ minimum: 10000
+ default: 100000
+ maximum: 400000
+
+patternProperties:
+ "-hog(-[0-9]+)?$":
+ type: object
+
+ required:
+ - gpio-hog
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ usb {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cp2112: device@1 {
+ compatible = "usb10c4,ea90";
+ reg = <1>;
+
+ gpio-controller;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ #gpio-cells = <2>;
+ gpio-line-names = "CP2112_SDA", "CP2112_SCL", "TEST2",
+ "TEST3","TEST4", "TEST5", "TEST6";
+
+ fan-rst-hog {
+ gpio-hog;
+ gpios = <7 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "FAN_RST";
+ };
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sda-gpios = <&cp2112 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ scl-gpios = <&cp2112 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+
+ temp@48 {
+ compatible = "national,lm75";
+ reg = <0x48>;
+ };
+ };
+
+ };
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v12 3/3] HID: cp2112: Configure I2C Bus Speed from Firmware
From: Danny Kaehn @ 2025-11-26 17:05 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
Andy Shevchenko, Danny Kaehn, Andi Shyti, Conor Dooley
Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
Bartosz Golaszewski, Ethan Twardy, linux-i2c, linux-kernel,
Leo Huang, Arun D Patil, Willie Thai, Ting-Kai Chen
In-Reply-To: <20251126-cp2112-dt-v12-0-2cdba6481db3@plexus.com>
Now that the I2C adapter on the CP2112 can have an associated firmware
node, set the bus speed based on firmware configuration
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
drivers/hid/hid-cp2112.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index fb301c27c712..801fe3ccad5e 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1220,6 +1220,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct cp2112_smbus_config_report config;
struct fwnode_handle *child;
struct gpio_irq_chip *girq;
+ struct i2c_timings timings;
u32 addr;
int ret;
@@ -1303,6 +1304,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto err_power_normal;
}
+ i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
+
+ config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
config.retry_time = cpu_to_be16(1);
ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Hans de Goede @ 2025-11-26 17:34 UTC (permalink / raw)
To: Ilpo Järvinen, Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Denis Benato
In-Reply-To: <b91fa2c8-e342-9e46-f401-8c3d0590cd38@linux.intel.com>
Hi,
On 26-Nov-25 4:23 PM, Ilpo Järvinen wrote:
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
...
>> As for "HID: asus: early return for ROG devices" changing the name of
>> the devices of this driver, I will veto backporting it if it happens,
>> so inputplumber will have the two full months to remove the name
>> match. This is not a breaking change in the sense that software cannot
>> be made to work on both previous and latter versions and there is no
>> other software to my knowledge relying on name matches for Asus
>> keyboards.
>
> Did Hans give some opinion about this rename earlier, at least I don't
> remember nor could find from lore archives?
I don't remember commenting on this myself either.
So generally speaking there are plenty of cases where /dev/input/event#
nodes for a specific device have their name changed by some kernel patches.
Typically HID input devices are matched in userspace by their
bus:vend-id:prod-id triplet not by the name. The name might even
change by a fwupdate of the device itself.
So I'm not overly worried about this and inputplumber seems nice
enough and already is very much not a plug-and-play tool.
One possible concern with laptop keyboard input-device name changes
though is hwdb entries to fixup scancode -> ev-key-code mappings.
See: /lib/udev/hwdb.d/60-keyboard.hwdb on any standard Linux systems
an then the big comment at the top.
An input-device name change might break this match pattern:
# - Input driver device name and DMI data match:
# evdev:name:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn*
# <input device name> is the name device specified by the
# driver, <vendor> is the firmware-provided string exported
# by the kernel DMI modalias, see /sys/class/dmi/id/modalias
As well as the extended version of this and for laptops with USB
keyboards this is the only match type which allows a DMI match
which is what we want for laptop kbd mappings. Looking at the Asus
section of the upstream 60-keyboard.hwdb I do not see any such
matches though.
There not being such matches kinda make sense since for USB-HID
devices any special scancode -> ev-key-code mappings are typically
handled in a vendor specific HID driver like hid-asus.
TL;DR: I think that the input-device name should be fine.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] drivers: hid: renegotiate resolution multipliers with device after reset
From: Jiri Kosina @ 2025-11-26 16:22 UTC (permalink / raw)
To: Benedek Kupper; +Cc: linux-input, linux-kernel, bentiss
In-Reply-To: <20251007203544.9963-1-kupper.benedek@gmail.com>
On Tue, 7 Oct 2025, Benedek Kupper wrote:
> The scroll resolution multipliers are set in the context of
> hidinput_connect(), which is only called at probe time: when the host
> changes the value on the device with a SET_REPORT(FEATURE), and the device
> accepts it, these multipliers are stored on the host side, and used to
> calculate the final scroll event values sent to userspace.
>
> After a USB suspend, the resume operation on many hubs and chipsets
> involve a USB reset signal as well. A reset on the device side clears all
> previous state information, including the value of the multiplier report.
> This reset is not handled by the multiplier handling logic, so what ends up
> happening is the host is still expecting high-resolution scroll events,
> but the device is reset to default resolution, making the effective,
> user-perceived scroll speed incredibly slow.
>
> The solution is to renegotiate the multiplier selection after each reset.
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1] HID: Fix Report Descriptor for Evision Wireless Receiver 320f:226f
From: Jiri Kosina @ 2025-11-26 16:18 UTC (permalink / raw)
To: Terry Junge; +Cc: Benjamin Tissoires, Artem, linux-input, linux-kernel
In-Reply-To: <20251120014931.580340-1-linuxhid@cosmicgizmosystems.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH V3 1/1] HID: i2c-hid: Add API to wait for device reset completion
From: Jiri Kosina @ 2025-11-26 16:16 UTC (permalink / raw)
To: LI Qingwu
Cc: bentiss, dianders, treapking, alex.vinarskis, dan.carpenter,
superm1, guanwentao, kl, linux-input, linux-kernel, liqind
In-Reply-To: <20251117081046.3140656-2-Qing-wu.Li@leica-geosystems.com.cn>
On Mon, 17 Nov 2025, LI Qingwu wrote:
> Some HID over I2C devices need to signal reset completion to the host
> after firmware updates or device resets. Per the HID over I2C spec,
> devices signal completion by sending an empty input report (0x0000).
>
> Add i2c_hid_wait_reset_complete() to allow drivers to synchronize
> with device reset operations. The function sets I2C_HID_RESET_PENDING
> and waits for the device's completion signal.
>
> Returns: 0 on success, -ETIMEDOUT on timeout.
>
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
> drivers/hid/i2c-hid/i2c-hid-core.c | 27 +++++++++++++++++++++++++++
> drivers/hid/i2c-hid/i2c-hid.h | 1 +
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 63f46a2e5788..067ad0770dd9 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1401,6 +1401,33 @@ const struct dev_pm_ops i2c_hid_core_pm = {
> };
> EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
>
> +int i2c_hid_wait_reset_complete(struct device *dev, unsigned long timeout_ms)
> +{
> + struct i2c_client *client;
> + struct i2c_hid *ihid;
> +
> + if (!dev)
> + return -ENODEV;
> +
> + client = to_i2c_client(dev);
> + if (!client)
> + return -ENODEV;
> +
> + ihid = i2c_get_clientdata(client);
> + if (!ihid)
> + return -ENODEV;
> +
> + set_bit(I2C_HID_RESET_PENDING, &ihid->flags);
> + if (wait_event_timeout(ihid->wait,
> + !test_bit(I2C_HID_RESET_PENDING, &ihid->flags),
> + msecs_to_jiffies(timeout_ms)))
> + return 0;
> +
> + clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
> + return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL_GPL(i2c_hid_wait_reset_complete);
> +
> MODULE_DESCRIPTION("HID over I2C core driver");
> MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
> index 1724a435c783..8e5482baa679 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.h
> +++ b/drivers/hid/i2c-hid/i2c-hid.h
> @@ -42,6 +42,7 @@ void i2c_hid_core_remove(struct i2c_client *client);
>
> void i2c_hid_core_shutdown(struct i2c_client *client);
>
> +int i2c_hid_wait_reset_complete(struct device *dev, unsigned long timeout_ms);
> extern const struct dev_pm_ops i2c_hid_core_pm;
Please resubmit this together in one go with the driver that makes use of
the functionality.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: multitouch: Add palm rejection quirks to HP Spectre Trackpad.
From: Jiri Kosina @ 2025-11-26 16:05 UTC (permalink / raw)
To: Jon Moeller; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <CABWf9sZuKy2=XjSo7WAB-9KanvDjhaXn-8P2J3_z2LgtcpwWDg@mail.gmail.com>
On Sun, 9 Nov 2025, Jon Moeller wrote:
> This patch fixes an issue where palm rejection signals from the
> ELAN trackpad in HP Spectre laptops are ignored, causing lots of
> false touches.
>
> Signed-off-by: Jon Moeller <jmoeller@gmail.com>
> ---
> drivers/hid/hid-multitouch.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 179dc316b4b5..cb5d80172131 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -229,6 +229,7 @@ static void mt_post_parse(struct mt_device *td,
> struct mt_application *app);
> #define MT_CLS_RAZER_BLADE_STEALTH 0x0112
> #define MT_CLS_SMART_TECH 0x0113
> #define MT_CLS_APPLE_TOUCHBAR 0x0114
> +#define MT_CLS_HP_SPECTRE_ELAN_HAPTIC 0x0115
> #define MT_CLS_SIS 0x0457
>
> #define MT_DEFAULT_MAXCONTACT 10
> @@ -420,6 +421,13 @@ static const struct mt_class mt_classes[] = {
> MT_QUIRK_APPLE_TOUCHBAR,
> .maxcontacts = 11,
> },
> + { .name = MT_CLS_HP_SPECTRE_ELAN_HAPTIC,
> + .quirks = MT_QUIRK_ALWAYS_VALID |
> + MT_QUIRK_SLOT_IS_CONTACTID |
> + MT_QUIRK_CONTACT_CNT_ACCURATE |
> + MT_QUIRK_CONFIDENCE |
> + MT_QUIRK_WIN8_PTP_BUTTONS,
> + },
> { .name = MT_CLS_SIS,
> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
> MT_QUIRK_ALWAYS_VALID |
> @@ -2148,6 +2156,12 @@ static const struct hid_device_id mt_devices[] = {
> HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> USB_VENDOR_ID_ELAN, 0x3148) },
>
> + { .driver_data = MT_CLS_HP_SPECTRE_ELAN_HAPTIC,
> + HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x32c8) },
> +
> + { .driver_data = MT_CLS_HP_SPECTRE_ELAN_HAPTIC,
> + HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, 0x310a) },
> +
> { .driver_data = MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU,
> HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> USB_VENDOR_ID_ELAN, 0x32ae) },
Thanks a lot for the patch.
Unfortunately it has been whitespace-damaged by your mail client, and so
it can't be applied.
Could you please fix that on your side, and resubmit?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Ilpo Järvinen @ 2025-11-26 15:31 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <CAGwozwHZrLcCAzH1m-AEKLJTZ2BE2fL1w0RY7-mF8Ck_yko8eA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 13042 bytes --]
On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> > >
> > > > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > > >
> > > > > This is a two part series which does the following:
> > > > > - Clean-up init sequence
> > > > > - Unify backlight handling to happen under asus-wmi so that all Aura
> > > > > devices have synced brightness controls and the backlight button works
> > > > > properly when it is on a USB laptop keyboard instead of one w/ WMI.
> > > > >
> > > > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > > > to make this easier to merge.
> > > >
> > > > Slight bump on this. It addresses both of the remarks Denis made in
> > > > the previous version.
> > > >
> > > > I begrudgingly argued a bit for those because I did not want to resend
> > > > the series and they were not functional changes, so sorry about that.
> > > > But they are fixed in this version incl. with the conflict with the
> > > > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > > > Add support for multiple kbd led handlers" even though I addressed
> > > > your comment, so you may want to add that.
> > >
> > > FYI, there's no direct relation that mandates a person to give a rev-by
> > > even if all his/her comments were addressed.
> >
> > True, this is just a reminder because I did not hear from him and
> > since he added a rev-by on the kfree patch. There's no obligation from
> > my side.
> >
> > > But generally yes, it would be useful to hear whether Denis is fine with
> > > v10, especially those patches that had contention earlier but you've
> > > modified post-v8.
> > >
> > > > As for "HID: asus: early return for ROG devices" changing the name of
> > > > the devices of this driver, I will veto backporting it if it happens,
> > > > so inputplumber will have the two full months to remove the name
> > > > match. This is not a breaking change in the sense that software cannot
> > > > be made to work on both previous and latter versions and there is no
> > > > other software to my knowledge relying on name matches for Asus
> > > > keyboards.
> > >
> > > Did Hans give some opinion about this rename earlier, at least I don't
> > > remember nor could find from lore archives?
> >
> > Hans jumped in on the ayaneo controller patch. I don't think I saw
> > activity on this series
>
> Hans had some feedback around half a year ago for the latter part of
> this series that binds the devices together
Yes but I was interested on specifically about this userspace visible
change. He has been around much much longer than me so his insight on
userspace visible changes is way more valuable than mine.
--
i.
>
> > Antheas
> >
> > > --
> > > i.
> > >
> > > > Moreover, an early exit is needed to prevent ejecting HID
> > > > endpoints without an ->input parameter so it is a needed fix anyway.
> > > > Postponing it will prevent Xbox Ally users from having RGB control
> > > > through userspace on a stock kernel but it is also not worth arguing
> > > > about
> > > >
> > > > It is also fine for me for this series to merge for 6.20, but I'd
> > > > rather we handle it now since there will be some turbulence for asus
> > > > users due to armoury merging so it makes sense to have this transition
> > > > once.
> > > >
> > > > > ---
> > > > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > > > >
> > > > > Changes since V9:
> > > > > - No functional changes
> > > > > - Rebase to review-ilpo-next
> > > > > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > > > "remove unused keyboard backlight quirk" + imports
> > > > > Dismiss Luke's review as this patch diverged
> > > > > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > > > more verbose
> > > > > - Use kfree in fortify patch
> > > > > - Fix minor style quirks from --nonstict checkpatch run
> > > > >
> > > > > Changes since V8:
> > > > > - No functional changes
> > > > > - Move legacy init patch to second, modify first patch so that their
> > > > > diff is minimized
> > > > > - Split "prevent binding to all HID devices on ROG" into two patches:
> > > > > - moving backlight initialization into probe
> > > > > - early exit to skip ->init check and rename
> > > > > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > > > to read usages before the report fixups are applied, so it did not work
> > > > > - In that patch, reword a comment to be single line and make is_vendor a bool
> > > > > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > > > has drifted too far since he reviewed/tested it.
> > > > >
> > > > > Changes since V7:
> > > > > - Readd legacy init quirk for Dennis
> > > > > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > > > - Fix warning due to enum_backlight receiving negative values
> > > > >
> > > > > Changes since V6:
> > > > > - Split initialization refactor into three patches, update commit text
> > > > > to be clearer in what it does
> > > > > - Replace spinlock accesses with guard and scoped guard in all patches
> > > > > - Add missing includes mentioned by Ilpo
> > > > > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > > > - Replace asus_ref.asus with local reference in all patches
> > > > > - Add missing kernel doc comments
> > > > > - Other minor nits from Ilpo
> > > > > - User reported warning due to scheduling work while holding a spinlock.
> > > > > Restructure patch for multiple handlers to limit when spinlock is held to
> > > > > variable access only. In parallel, setup a workqueue to handle registration
> > > > > of led device and setting brightness. This is required as registering the
> > > > > led device triggers kbd_led_get which needs to hold the spinlock to
> > > > > protect the led_wk value. The workqueue is also required for the hid
> > > > > event passthrough to avoid scheduling work while holding the spinlock.
> > > > > Apply the workqueue to wmi brightness buttons as well, as that was
> > > > > omitted before this series and WMI access was performed.
> > > > > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > > > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > > > This allows other quirks to apply (applies quirk that fixes keyboard
> > > > > being named as a pointer device).
> > > > >
> > > > > Changes since V5:
> > > > > - It's been a long time
> > > > > - Remove addition of RGB as that had some comments I need to work on
> > > > > - Remove folio patch (already merged)
> > > > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > > > without this patch, some old NKEY keyboards might not respond to
> > > > > RGB commands according to Luke, but the kernel driver does not do
> > > > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > > > Windows. If an issue is found, we can re-add it or just remove patches
> > > > > 1/2 before merging. However, init could use the cleanup.
> > > > >
> > > > > Changes since V4:
> > > > > - Fix KConfig (reported by kernel robot)
> > > > > - Fix Ilpo's nits, if I missed anything lmk
> > > > >
> > > > > Changes since V3:
> > > > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > > > that it is not needed for their media keys to function.
> > > > > - Cover init in asus-wmi with spinlock as per Hans
> > > > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > > > in USB Asus keyboards, per Hans.
> > > > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > > > the spin lock (but after the asus reference is set to null)
> > > > >
> > > > > Changes since V2:
> > > > > - Check lazy init succeds in asus-wmi before setting register variable
> > > > > - make explicit check in asus_hid_register_listener for listener existing
> > > > > to avoid re-init
> > > > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > > > all input vendor devices. This includes moving rgb init to probe
> > > > > instead of the input_configured callbacks.
> > > > > - Remove fan key (during retest it appears to be 0xae that is already
> > > > > supported by hid-asus)
> > > > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > > > - removes fds from userspace and breaks backlight functionality. All
> > > > > - current mainline drivers do not support backlight hotplugging, so most
> > > > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > > > For the Ally, since it disconnects its controller during sleep, this
> > > > > caused the backlight slider to not work in KDE.
> > > > >
> > > > > Changes since V1:
> > > > > - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > > > > - Fix ifdef else having an invalid signature (reported by kernel robot)
> > > > > - Restore input arguments to init and keyboard function so they can
> > > > > be re-used for RGB controls.
> > > > > - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > > > > with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > > > > keyboard rename into it.
> > > > > - Unregister brightness listener before removing work queue to avoid
> > > > > a race condition causing corruption
> > > > > - Remove spurious mutex unlock in asus_brt_event
> > > > > - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > > > > relocking the mutex and causing a deadlock when unregistering leds
> > > > > - Add extra check during unregistering to avoid calling unregister when
> > > > > no led device is registered.
> > > > > - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > > > > the driver to create 4 RGB handlers per device. I also suspect some
> > > > > extra events sneak through (KDE had the @@@@@@).
> > > > >
> > > > > Antheas Kapenekakis (11):
> > > > > HID: asus: simplify RGB init sequence
> > > > > HID: asus: initialize additional endpoints only for legacy devices
> > > > > HID: asus: use same report_id in response
> > > > > HID: asus: fortify keyboard handshake
> > > > > HID: asus: move vendor initialization to probe
> > > > > HID: asus: early return for ROG devices
> > > > > platform/x86: asus-wmi: Add support for multiple kbd led handlers
> > > > > HID: asus: listen to the asus-wmi brightness device instead of
> > > > > creating one
> > > > > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > > > platform/x86: asus-wmi: add keyboard brightness event handler
> > > > > HID: asus: add support for the asus-wmi brightness handler
> > > > >
> > > > > drivers/hid/hid-asus.c | 205 ++++++++--------
> > > > > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > > > > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > > > > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > > > > 4 files changed, 322 insertions(+), 184 deletions(-)
> > > > > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > > > >
> > > > >
> > > > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > > > --
> > > > > 2.52.0
> > > > >
> > > > >
> > > >
> > >
>
^ permalink raw reply
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-11-26 15:29 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <CAGwozwGWL4Vr1XfVZBQoGCq7tKE4mBcPYS2n-p_GAt-V8LeKgg@mail.gmail.com>
On Wed, 26 Nov 2025 at 16:28, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
> >
> > > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > > >
> > > > This is a two part series which does the following:
> > > > - Clean-up init sequence
> > > > - Unify backlight handling to happen under asus-wmi so that all Aura
> > > > devices have synced brightness controls and the backlight button works
> > > > properly when it is on a USB laptop keyboard instead of one w/ WMI.
> > > >
> > > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > > to make this easier to merge.
> > >
> > > Slight bump on this. It addresses both of the remarks Denis made in
> > > the previous version.
> > >
> > > I begrudgingly argued a bit for those because I did not want to resend
> > > the series and they were not functional changes, so sorry about that.
> > > But they are fixed in this version incl. with the conflict with the
> > > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > > Add support for multiple kbd led handlers" even though I addressed
> > > your comment, so you may want to add that.
> >
> > FYI, there's no direct relation that mandates a person to give a rev-by
> > even if all his/her comments were addressed.
>
> True, this is just a reminder because I did not hear from him and
> since he added a rev-by on the kfree patch. There's no obligation from
> my side.
>
> > But generally yes, it would be useful to hear whether Denis is fine with
> > v10, especially those patches that had contention earlier but you've
> > modified post-v8.
> >
> > > As for "HID: asus: early return for ROG devices" changing the name of
> > > the devices of this driver, I will veto backporting it if it happens,
> > > so inputplumber will have the two full months to remove the name
> > > match. This is not a breaking change in the sense that software cannot
> > > be made to work on both previous and latter versions and there is no
> > > other software to my knowledge relying on name matches for Asus
> > > keyboards.
> >
> > Did Hans give some opinion about this rename earlier, at least I don't
> > remember nor could find from lore archives?
>
> Hans jumped in on the ayaneo controller patch. I don't think I saw
> activity on this series
Hans had some feedback around half a year ago for the latter part of
this series that binds the devices together
> Antheas
>
> > --
> > i.
> >
> > > Moreover, an early exit is needed to prevent ejecting HID
> > > endpoints without an ->input parameter so it is a needed fix anyway.
> > > Postponing it will prevent Xbox Ally users from having RGB control
> > > through userspace on a stock kernel but it is also not worth arguing
> > > about
> > >
> > > It is also fine for me for this series to merge for 6.20, but I'd
> > > rather we handle it now since there will be some turbulence for asus
> > > users due to armoury merging so it makes sense to have this transition
> > > once.
> > >
> > > > ---
> > > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > > >
> > > > Changes since V9:
> > > > - No functional changes
> > > > - Rebase to review-ilpo-next
> > > > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > > "remove unused keyboard backlight quirk" + imports
> > > > Dismiss Luke's review as this patch diverged
> > > > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > > more verbose
> > > > - Use kfree in fortify patch
> > > > - Fix minor style quirks from --nonstict checkpatch run
> > > >
> > > > Changes since V8:
> > > > - No functional changes
> > > > - Move legacy init patch to second, modify first patch so that their
> > > > diff is minimized
> > > > - Split "prevent binding to all HID devices on ROG" into two patches:
> > > > - moving backlight initialization into probe
> > > > - early exit to skip ->init check and rename
> > > > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > > to read usages before the report fixups are applied, so it did not work
> > > > - In that patch, reword a comment to be single line and make is_vendor a bool
> > > > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > > has drifted too far since he reviewed/tested it.
> > > >
> > > > Changes since V7:
> > > > - Readd legacy init quirk for Dennis
> > > > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > > - Fix warning due to enum_backlight receiving negative values
> > > >
> > > > Changes since V6:
> > > > - Split initialization refactor into three patches, update commit text
> > > > to be clearer in what it does
> > > > - Replace spinlock accesses with guard and scoped guard in all patches
> > > > - Add missing includes mentioned by Ilpo
> > > > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > > - Replace asus_ref.asus with local reference in all patches
> > > > - Add missing kernel doc comments
> > > > - Other minor nits from Ilpo
> > > > - User reported warning due to scheduling work while holding a spinlock.
> > > > Restructure patch for multiple handlers to limit when spinlock is held to
> > > > variable access only. In parallel, setup a workqueue to handle registration
> > > > of led device and setting brightness. This is required as registering the
> > > > led device triggers kbd_led_get which needs to hold the spinlock to
> > > > protect the led_wk value. The workqueue is also required for the hid
> > > > event passthrough to avoid scheduling work while holding the spinlock.
> > > > Apply the workqueue to wmi brightness buttons as well, as that was
> > > > omitted before this series and WMI access was performed.
> > > > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > > This allows other quirks to apply (applies quirk that fixes keyboard
> > > > being named as a pointer device).
> > > >
> > > > Changes since V5:
> > > > - It's been a long time
> > > > - Remove addition of RGB as that had some comments I need to work on
> > > > - Remove folio patch (already merged)
> > > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > > without this patch, some old NKEY keyboards might not respond to
> > > > RGB commands according to Luke, but the kernel driver does not do
> > > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > > Windows. If an issue is found, we can re-add it or just remove patches
> > > > 1/2 before merging. However, init could use the cleanup.
> > > >
> > > > Changes since V4:
> > > > - Fix KConfig (reported by kernel robot)
> > > > - Fix Ilpo's nits, if I missed anything lmk
> > > >
> > > > Changes since V3:
> > > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > > that it is not needed for their media keys to function.
> > > > - Cover init in asus-wmi with spinlock as per Hans
> > > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > > in USB Asus keyboards, per Hans.
> > > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > > the spin lock (but after the asus reference is set to null)
> > > >
> > > > Changes since V2:
> > > > - Check lazy init succeds in asus-wmi before setting register variable
> > > > - make explicit check in asus_hid_register_listener for listener existing
> > > > to avoid re-init
> > > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > > all input vendor devices. This includes moving rgb init to probe
> > > > instead of the input_configured callbacks.
> > > > - Remove fan key (during retest it appears to be 0xae that is already
> > > > supported by hid-asus)
> > > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > > - removes fds from userspace and breaks backlight functionality. All
> > > > - current mainline drivers do not support backlight hotplugging, so most
> > > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > > For the Ally, since it disconnects its controller during sleep, this
> > > > caused the backlight slider to not work in KDE.
> > > >
> > > > Changes since V1:
> > > > - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > > > - Fix ifdef else having an invalid signature (reported by kernel robot)
> > > > - Restore input arguments to init and keyboard function so they can
> > > > be re-used for RGB controls.
> > > > - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > > > with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > > > keyboard rename into it.
> > > > - Unregister brightness listener before removing work queue to avoid
> > > > a race condition causing corruption
> > > > - Remove spurious mutex unlock in asus_brt_event
> > > > - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > > > relocking the mutex and causing a deadlock when unregistering leds
> > > > - Add extra check during unregistering to avoid calling unregister when
> > > > no led device is registered.
> > > > - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > > > the driver to create 4 RGB handlers per device. I also suspect some
> > > > extra events sneak through (KDE had the @@@@@@).
> > > >
> > > > Antheas Kapenekakis (11):
> > > > HID: asus: simplify RGB init sequence
> > > > HID: asus: initialize additional endpoints only for legacy devices
> > > > HID: asus: use same report_id in response
> > > > HID: asus: fortify keyboard handshake
> > > > HID: asus: move vendor initialization to probe
> > > > HID: asus: early return for ROG devices
> > > > platform/x86: asus-wmi: Add support for multiple kbd led handlers
> > > > HID: asus: listen to the asus-wmi brightness device instead of
> > > > creating one
> > > > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > > platform/x86: asus-wmi: add keyboard brightness event handler
> > > > HID: asus: add support for the asus-wmi brightness handler
> > > >
> > > > drivers/hid/hid-asus.c | 205 ++++++++--------
> > > > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > > > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > > > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > > > 4 files changed, 322 insertions(+), 184 deletions(-)
> > > > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > > >
> > > >
> > > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > > --
> > > > 2.52.0
> > > >
> > > >
> > >
> >
^ permalink raw reply
* Re: [PATCH v10 00/11] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-11-26 15:28 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
Denis Benato
In-Reply-To: <b91fa2c8-e342-9e46-f401-8c3d0590cd38@linux.intel.com>
On Wed, 26 Nov 2025 at 16:24, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 26 Nov 2025, Antheas Kapenekakis wrote:
>
> > On Sat, 22 Nov 2025 at 12:01, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> > >
> > > This is a two part series which does the following:
> > > - Clean-up init sequence
> > > - Unify backlight handling to happen under asus-wmi so that all Aura
> > > devices have synced brightness controls and the backlight button works
> > > properly when it is on a USB laptop keyboard instead of one w/ WMI.
> > >
> > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > to make this easier to merge.
> >
> > Slight bump on this. It addresses both of the remarks Denis made in
> > the previous version.
> >
> > I begrudgingly argued a bit for those because I did not want to resend
> > the series and they were not functional changes, so sorry about that.
> > But they are fixed in this version incl. with the conflict with the
> > armoury patchset. Denis, you omitted a rby on "platform/x86: asus-wmi:
> > Add support for multiple kbd led handlers" even though I addressed
> > your comment, so you may want to add that.
>
> FYI, there's no direct relation that mandates a person to give a rev-by
> even if all his/her comments were addressed.
True, this is just a reminder because I did not hear from him and
since he added a rev-by on the kfree patch. There's no obligation from
my side.
> But generally yes, it would be useful to hear whether Denis is fine with
> v10, especially those patches that had contention earlier but you've
> modified post-v8.
>
> > As for "HID: asus: early return for ROG devices" changing the name of
> > the devices of this driver, I will veto backporting it if it happens,
> > so inputplumber will have the two full months to remove the name
> > match. This is not a breaking change in the sense that software cannot
> > be made to work on both previous and latter versions and there is no
> > other software to my knowledge relying on name matches for Asus
> > keyboards.
>
> Did Hans give some opinion about this rename earlier, at least I don't
> remember nor could find from lore archives?
Hans jumped in on the ayaneo controller patch. I don't think I saw
activity on this series
Antheas
> --
> i.
>
> > Moreover, an early exit is needed to prevent ejecting HID
> > endpoints without an ->input parameter so it is a needed fix anyway.
> > Postponing it will prevent Xbox Ally users from having RGB control
> > through userspace on a stock kernel but it is also not worth arguing
> > about
> >
> > It is also fine for me for this series to merge for 6.20, but I'd
> > rather we handle it now since there will be some turbulence for asus
> > users due to armoury merging so it makes sense to have this transition
> > once.
> >
> > > ---
> > > V9: https://lore.kernel.org/all/20251120094617.11672-1-lkml@antheas.dev/
> > > V8: https://lore.kernel.org/all/20251101104712.8011-1-lkml@antheas.dev/
> > > V7: https://lore.kernel.org/all/20251018101759.4089-1-lkml@antheas.dev/
> > > V6: https://lore.kernel.org/all/20251013201535.6737-1-lkml@antheas.dev/
> > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > >
> > > Changes since V9:
> > > - No functional changes
> > > - Rebase to review-ilpo-next
> > > - Fix armoury series conflict by removing the file asus-wmi-leds-ids on
> > > "remove unused keyboard backlight quirk" + imports
> > > Dismiss Luke's review as this patch diverged
> > > - Reword paragraph in "Add support for multiple kbd led handlers" to be
> > > more verbose
> > > - Use kfree in fortify patch
> > > - Fix minor style quirks from --nonstict checkpatch run
> > >
> > > Changes since V8:
> > > - No functional changes
> > > - Move legacy init patch to second, modify first patch so that their
> > > diff is minimized
> > > - Split "prevent binding to all HID devices on ROG" into two patches:
> > > - moving backlight initialization into probe
> > > - early exit to skip ->init check and rename
> > > - Remove skipping vendor fixups for non-vendor devices. It is not possible
> > > to read usages before the report fixups are applied, so it did not work
> > > - In that patch, reword a comment to be single line and make is_vendor a bool
> > > - Dismiss Luke's tags from "Add support for multiple kbd led handlers" as it
> > > has drifted too far since he reviewed/tested it.
> > >
> > > Changes since V7:
> > > - Readd legacy init quirk for Dennis
> > > - Remove HID_QUIRK_INPUT_PER_APP as a courtesy to asusctl
> > > - Fix warning due to enum_backlight receiving negative values
> > >
> > > Changes since V6:
> > > - Split initialization refactor into three patches, update commit text
> > > to be clearer in what it does
> > > - Replace spinlock accesses with guard and scoped guard in all patches
> > > - Add missing includes mentioned by Ilpo
> > > - Reflow, tweak comment in prevent binding to all HID devices on ROG
> > > - Replace asus_ref.asus with local reference in all patches
> > > - Add missing kernel doc comments
> > > - Other minor nits from Ilpo
> > > - User reported warning due to scheduling work while holding a spinlock.
> > > Restructure patch for multiple handlers to limit when spinlock is held to
> > > variable access only. In parallel, setup a workqueue to handle registration
> > > of led device and setting brightness. This is required as registering the
> > > led device triggers kbd_led_get which needs to hold the spinlock to
> > > protect the led_wk value. The workqueue is also required for the hid
> > > event passthrough to avoid scheduling work while holding the spinlock.
> > > Apply the workqueue to wmi brightness buttons as well, as that was
> > > omitted before this series and WMI access was performed.
> > > - On "HID: asus: prevent binding to all HID devices on ROG", rename
> > > quirk HANDLE_GENERIC to SKIP_REPORT_FIXUP and only skip report fixup.
> > > This allows other quirks to apply (applies quirk that fixes keyboard
> > > being named as a pointer device).
> > >
> > > Changes since V5:
> > > - It's been a long time
> > > - Remove addition of RGB as that had some comments I need to work on
> > > - Remove folio patch (already merged)
> > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > without this patch, some old NKEY keyboards might not respond to
> > > RGB commands according to Luke, but the kernel driver does not do
> > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > Windows. If an issue is found, we can re-add it or just remove patches
> > > 1/2 before merging. However, init could use the cleanup.
> > >
> > > Changes since V4:
> > > - Fix KConfig (reported by kernel robot)
> > > - Fix Ilpo's nits, if I missed anything lmk
> > >
> > > Changes since V3:
> > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > that it is not needed for their media keys to function.
> > > - Cover init in asus-wmi with spinlock as per Hans
> > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > in USB Asus keyboards, per Hans.
> > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > the spin lock (but after the asus reference is set to null)
> > >
> > > Changes since V2:
> > > - Check lazy init succeds in asus-wmi before setting register variable
> > > - make explicit check in asus_hid_register_listener for listener existing
> > > to avoid re-init
> > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > all input vendor devices. This includes moving rgb init to probe
> > > instead of the input_configured callbacks.
> > > - Remove fan key (during retest it appears to be 0xae that is already
> > > supported by hid-asus)
> > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > - removes fds from userspace and breaks backlight functionality. All
> > > - current mainline drivers do not support backlight hotplugging, so most
> > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > For the Ally, since it disconnects its controller during sleep, this
> > > caused the backlight slider to not work in KDE.
> > >
> > > Changes since V1:
> > > - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > > - Fix ifdef else having an invalid signature (reported by kernel robot)
> > > - Restore input arguments to init and keyboard function so they can
> > > be re-used for RGB controls.
> > > - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > > with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > > keyboard rename into it.
> > > - Unregister brightness listener before removing work queue to avoid
> > > a race condition causing corruption
> > > - Remove spurious mutex unlock in asus_brt_event
> > > - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > > relocking the mutex and causing a deadlock when unregistering leds
> > > - Add extra check during unregistering to avoid calling unregister when
> > > no led device is registered.
> > > - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > > the driver to create 4 RGB handlers per device. I also suspect some
> > > extra events sneak through (KDE had the @@@@@@).
> > >
> > > Antheas Kapenekakis (11):
> > > HID: asus: simplify RGB init sequence
> > > HID: asus: initialize additional endpoints only for legacy devices
> > > HID: asus: use same report_id in response
> > > HID: asus: fortify keyboard handshake
> > > HID: asus: move vendor initialization to probe
> > > HID: asus: early return for ROG devices
> > > platform/x86: asus-wmi: Add support for multiple kbd led handlers
> > > HID: asus: listen to the asus-wmi brightness device instead of
> > > creating one
> > > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > platform/x86: asus-wmi: add keyboard brightness event handler
> > > HID: asus: add support for the asus-wmi brightness handler
> > >
> > > drivers/hid/hid-asus.c | 205 ++++++++--------
> > > drivers/platform/x86/asus-wmi.c | 223 +++++++++++++++---
> > > .../platform_data/x86/asus-wmi-leds-ids.h | 50 ----
> > > include/linux/platform_data/x86/asus-wmi.h | 28 +++
> > > 4 files changed, 322 insertions(+), 184 deletions(-)
> > > delete mode 100644 include/linux/platform_data/x86/asus-wmi-leds-ids.h
> > >
> > >
> > > base-commit: 2643187ccb8628144246ee9d44da5e3ac428f9c3
> > > --
> > > 2.52.0
> > >
> > >
> >
>
^ permalink raw reply
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