Re: [PATCH 3/3] HID: asus: add support for xgm led

[Antheas Kapenekakis]

On Tue, 15 Sept 2026 at 20:12, Denis Benato <denis.benato@xxxxxxxxx> wrote:
>
> XG mobile stations have very bright leds behind the fan that can be
> turned either ON or OFF: add a cled interface to allow controlling the
> brightness of those red leds.

Hi Denis,
as of last month, I am also the proud owner of a XG Mobile (2025 in my
case). Therefore, I can now comment on this series and give you
feedback.

First, I have some interim patches that are not ready yet to post. The
device still has some led behavior that I need to investigate, but I
attach them here for your reference. You may send them on my behalf if
you want to see them sooner though.

https://github.com/anatase-org/patchwork/commit/af3e5e1755f720a2fffd1b67cb1253e11efbc5bb
https://github.com/anatase-org/patchwork/commit/7718c4b64b03ca9be043e8c56f1833e5c4880fff

Note that these two patches are essentially a replacement for this
patch essentially, except for the binding, where your device is I2C
where mine is thunderbolt, so that still needs to be added. There are
still leftover issues with the LED I have to investigate before I
submit these patches.

As you know, USB keyboards in Asus laptops are connected over WMI
through s2idle as well, which is what makes them turn off their
backlight during suspend. However, our devices are external and cannot
use the same path. I suspect that there is a different notifier for
these devices over armoury crate we need to investigate first.

> Let the led core manage the power transitions: the classdev is flagged
> with LED_CORE_SUSPENDRESUME, so it is switched off at suspend and its
> last brightness is restored at resume. The EC drives its own blinking
> pattern during s2idle anyway, so the led state while the machine is
> asleep is not meaningful.
>
> Cc: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> Signed-off-by: Denis Benato <denis.benato@xxxxxxxxx>
> ---
>  drivers/hid/hid-asus.c | 84 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 03150d29eec5..a427e272563d 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -51,6 +51,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>  #define FEATURE_KBD_LED_REPORT_ID1 0x5d
>  #define FEATURE_KBD_LED_REPORT_ID2 0x5e
>
> +#define ROG_XGM_REPORT_SIZE 300

Here, you define an additional report size var. This is not necessary,
as ROG_ALLY_REPORT_SIZE is only used for reads currently in the driver
and we defer to hid core to set the write length. Therefore, you might
increase ROG_ALLY_REPORT_SIZE to 300 universally instead, as a
correctness fix. This way, when Aura devices attempt to write 300
bytes, they still work regardless of the quirk. This is what
af3e5e1755f720a2fffd1b67cb1253e11efbc5bb.

> +
>  #define ROG_ALLY_REPORT_SIZE 64
>  #define ROG_ALLY_X_MIN_MCU 313
>  #define ROG_ALLY_MIN_MCU 319
> @@ -144,6 +146,11 @@ struct asus_worker {
>         bool removed;
>  };
>
> +struct asus_xgm_led {
> +       struct led_classdev cdev;
> +       struct hid_device *hdev;
> +};
> +
>  struct asus_touchpad_info {
>         int max_x;
>         int max_y;
> @@ -170,6 +177,7 @@ struct asus_drvdata {
>         unsigned long battery_next_query;
>         struct asus_hid_listener listener;
>         bool fn_lock;
> +       struct asus_xgm_led *xgm_led;
>  };
>
>  static int asus_report_battery(struct asus_drvdata *, u8 *, int);
> @@ -1161,6 +1169,26 @@ static int asus_battery_probe(struct hid_device *hdev)
>         return ret;
>  }
>
> +static int asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
> +{
> +       const u8 buf[ROG_XGM_REPORT_SIZE] = {
> +               FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
> +       };
> +       struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
> +       int ret;
> +
> +       ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
> +       if (ret < 0) {
> +               hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
> +               return ret;
> +       } else if (ret != ROG_XGM_REPORT_SIZE) {
> +               hid_err(xgm->hdev, "Unexpected partial transfer to XG mobile: %d\n", ret);
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +

Moreover, as this is an Aura device, and by setting
ROG_XGM_REPORT_SIZE to 300, you can now reuse the initial inits and
brightness handler instead of adding new ones as you did here. This
way, for asus laptops with a wmi handler, the brightness keyboard
shortcut of the device will also control the eGPU, which is preferable
behavior. This is what 7718c4b64b03ca9be043e8c56f1833e5c4880fff does
in part.

I have not tested the common path yet. I have been travelling so I
have not flashed the new kernel on my z13. I have been using the eGPU
with a different device.

7718c4b64b03ca9be043e8c56f1833e5c4880fff also fixes the "blinking
pattern" of the EC, as you note in your subject. With the init,
suspend and resume handlers sending the init of that patch, during
boot the light blinks, it becomes solid once hid-asus binds, then
during the transition to sleep, it starts to blink until it turns red,
and does the reverse during resume. It would be good for you to give
feedback for that. I have not verified the Windows behavior there, and
we should do that for correctness. The init is referenced from
g-helper in Windows, but maybe there is another command that signals
sleep better.

I do not have an answer to the sleep leds unfortunately. My XG's RGB
stays on while there is a device plugged in, regardless of whether its
sleeping or shutdown. This makes me think that we are missing a
notifier command, and the brightness patch you propose here just
papers over the issue. So we should investigate that first.

Let me know how you'd like to proceed.

Best,
Antheas


>  static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>  {
>         struct input_dev *input = hi->input;
> @@ -1406,6 +1434,49 @@ static void asus_initialize_reports(struct hid_device *hdev)
>         }
>  }
>
> +static int asus_xgm_init(struct hid_device *hdev, struct asus_drvdata *drvdata)
> +{
> +       const char *name;
> +       int ret;
> +
> +       drvdata->xgm_led = devm_kzalloc(&hdev->dev, sizeof(*drvdata->xgm_led), GFP_KERNEL);
> +       if (drvdata->xgm_led == NULL)
> +               return -ENOMEM;
> +
> +       name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "asus:xgm-%s:led",
> +                             strlen(hdev->uniq) ? hdev->uniq : dev_name(&hdev->dev));
> +
> +       if (name == NULL) {
> +               ret = -ENOMEM;
> +               goto asus_xgm_init_err;
> +       }
> +
> +       drvdata->xgm_led->hdev = hdev;
> +       drvdata->xgm_led->cdev.name = name;
> +       drvdata->xgm_led->cdev.brightness = 1;
> +       drvdata->xgm_led->cdev.max_brightness = 1;
> +       drvdata->xgm_led->cdev.brightness_set_blocking = asus_xgm_led_set;
> +       drvdata->xgm_led->cdev.flags = LED_CORE_SUSPENDRESUME;
> +
> +       /* LED state is arbitrary on boot, set a default */
> +       ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
> +       if (ret) {
> +               hid_err(hdev, "Asus failed to set xgm led: %d\n", ret);
> +               goto asus_xgm_init_err;
> +       }
> +
> +       ret = devm_led_classdev_register(&hdev->dev, &drvdata->xgm_led->cdev);
> +       if (ret) {
> +               hid_err(hdev, "Asus failed to register xgm led: %d\n", ret);
> +               goto asus_xgm_init_err;
> +       }
> +
> +       return 0;
> +asus_xgm_init_err:
> +       drvdata->xgm_led = NULL;
> +       return ret;
> +}
> +
>  static int __maybe_unused asus_resume(struct hid_device *hdev)
>  {
>         struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> @@ -1545,6 +1616,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         if (!drvdata->tp)
>                 asus_initialize_reports(hdev);
>
> +       if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
> +           ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
> +            (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
> +               ret = asus_xgm_init(hdev, drvdata);
> +               if (ret) {
> +                       hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
> +                       goto err_stop_hw;
> +               }
> +       }
> +
>         /* Laptops keyboard backlight is always at 0x5a */
>         if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
>             (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
> @@ -1594,6 +1675,9 @@ static void asus_remove(struct hid_device *hdev)
>         if (drvdata->listener.brightness_set)
>                 asus_hid_unregister_listener(&drvdata->listener);
>
> +       if (drvdata->xgm_led)
> +               devm_led_classdev_unregister(&hdev->dev, &drvdata->xgm_led->cdev);
> +
>         asus_worker_stop(drvdata->worker);
>         hid_hw_stop(hdev);
>  }
> --
> 2.47.3
>
>