Re: [PATCH v4 4/5] HID: asus: add support for xgm led
From: Denis Benato
Date: Tue Jun 16 2026 - 20:18:23 EST
On 6/15/26 23:55, Antheas Kapenekakis wrote:
> On Mon, 15 Jun 2026 at 18:51, 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.
>>
>> Signed-off-by: Denis Benato <denis.benato@xxxxxxxxx>
>> ---
>> drivers/hid/hid-asus.c | 91 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 91 insertions(+)
>>
>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>> index 6896730efafc..e68a6d93369d 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
>> +
>> #define ROG_ALLY_REPORT_SIZE 64
>> #define ROG_ALLY_X_MIN_MCU 313
>> #define ROG_ALLY_MIN_MCU 319
>> @@ -143,6 +145,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;
>> @@ -169,6 +176,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);
>> @@ -1119,6 +1127,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;
>> +}
>> +
>> static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> {
>> struct input_dev *input = hi->input;
>> @@ -1343,9 +1371,52 @@ static int asus_start_multitouch(struct hid_device *hdev)
>> return 0;
>> }
>>
>> +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;
>> +
>> + /* 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);
>> + int ret;
>>
>> /*
>> * If we have a backlight listener registered, restore the previous state,
>> @@ -1355,7 +1426,17 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
>> if (drvdata->listener.brightness_set)
>> asus_kbd_backlight_set(&drvdata->listener, drvdata->kbd_backlight_brightness);
>>
>> + if (drvdata->xgm_led) {
>> + ret = asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
>> + if (ret) {
>> + hid_err(hdev, "Asus failed to restore xgm brightness: %d\n", ret);
>> + goto asus_resume_err;
>> + }
>> + }
>> +
>> return 0;
>> +asus_resume_err:
>> + return ret;
>> }
>>
>> static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
>> @@ -1484,6 +1565,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> }
>> }
>>
>> + 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;
>> + }
>> + }
>> +
> reset_resume has a special meaning and perhaps should not have been
> used for this driver at all. Check if resume works and if it does use
> that instead.
I am not using reset_resume for xgm.
reset_resume will become important to recover the ally,
but for xg mobile is unused (and useless).
> Other than that and the refactor patch I'd rather see separately, the
> other 4 patches look fine to me.
Fair, I'll wait for maintainers to tell what they like the most
and go from there.
I agree there are things not really much related here
but at least this way the order to apply them is clear.
> Reviewed-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
Thanks,
Denis
> Best,
> Antheas
>
>> /* 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)) &&
>> --
>> 2.47.3
>>
>>