Re: [PATCH v4 1/5] HID: asus: refactor the two workqueues and init sequence

[Denis Benato]

On 6/16/26 03:00, Denis Benato wrote:
> On 6/15/26 23:49, Antheas Kapenekakis wrote:
>> On Mon, 15 Jun 2026 at 18:51, Denis Benato <denis.benato@xxxxxxxxx> wrote:
>>> Multiple issues have been found within the hid-asus driver:
>>> - unchecked size in asus_raw_event()
>>> - unclean teardown of asus_probe on failure
>>> - possible use-after-free in asus_probe
>>> - multiple workqueue used for jobs where one was enough
>>> - sleeping calls in atomic context
>>> - packets of incorrect size being sent to the keyboard controller
>>>
>>> Join the two workqueues into one reusing the stopping mechanism
>>> of the brightness workqueue, use the joined workqueue to also
>>> move the asus_wmi_send_event() sleeping call away from atomic
>>> context and add a size check in asus_raw_event().
>>>
>>> Fixes: f631011e36b8 ("HID: hid-asus: Implement fn lock for Asus ProArt P16")
>>> Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
>>> Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
>> I'm not sure b34b5945a769 caused an issue, perhaps drop it from fixes.
>> Does f631011e36b8 do a WMI from a hid interrupt? If not, perhaps it
>> does not need to be changed
> Maybe I confused some tags.
I looked it up: https://lore.kernel.org/all/20260612143838.BD8651F000E9@xxxxxxxxxxxxxxx/
>> But moreso, this patch is too large and will take a bit to review. Can
>> you send it separately after the rest of the series goes through
>> unless users report warnings?
> I agree on it being large. I tried doing it by pieces, but got a bunch of compile
> errors.
>
> I ended up deciding trying it when after a few hours I had to modify code
> I did not want to modify (just yet) just for the thing to compile.
>
> Probably result of frustration more than technical limitation...
>
> I am fine with waiting and will be unavailable for a few days,
> in the meanwhile I hope in some input from hid maintainers
> too.
>> Antheas
>>
>>> Reported-by: sahiko-bot@xxxxxxxxxx
>>> Closes: https://lore.kernel.org/all/20260613154732.60A4B1F000E9@xxxxxxxxxxxxxxx/
>>> Signed-off-by: Denis Benato <denis.benato@xxxxxxxxx>
>>> ---
>>>  drivers/hid/hid-asus.c | 393 +++++++++++++++++++++++++++++------------
>>>  1 file changed, 282 insertions(+), 111 deletions(-)
>>>
>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>> index d34d74df3dc0..05ca6665e0a4 100644
>>> --- a/drivers/hid/hid-asus.c
>>> +++ b/drivers/hid/hid-asus.c
>>> @@ -109,11 +109,36 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
>>>
>>>  #define TRKID_SGN       ((TRKID_MAX + 1) >> 1)
>>>
>>> -struct asus_kbd_leds {
>>> -       struct asus_hid_listener listener;
>>> +enum asus_work_action_type {
>>> +       FN_LOCK_SYNC,
>>> +       BRIGHTNESS_SET,
>>> +       WMI_FAN,
>>> +};
>>> +
>>> +struct hid_raw_event_data {
>>> +       u8 report_data[FEATURE_KBD_REPORT_SIZE];
>>> +       size_t report_size;
>>> +};
>>> +
>>> +struct asus_work_action {
>>> +       struct list_head node;
>>> +       enum asus_work_action_type type;
>>> +       union {
>>> +               /* Data for BRIGHTNESS_SET */
>>> +               unsigned int brightness;
>>> +
>>> +               /* Data for FN_LOCK_SYNC */
>>> +               bool fn_lock;
>>> +
>>> +               /* Data for WMI_FAN */
>>> +               struct hid_raw_event_data fan_hid_data;
>>> +       } data;
>>> +};
>>> +
>>> +struct asus_worker {
>>>         struct hid_device *hdev;
>>>         struct work_struct work;
>>> -       unsigned int brightness;
>>> +       struct list_head actions;
>>>         spinlock_t lock;
>>>         bool removed;
>>>  };
>>> @@ -133,7 +158,8 @@ struct asus_drvdata {
>>>         struct hid_device *hdev;
>>>         struct input_dev *input;
>>>         struct input_dev *tp_kbd_input;
>>> -       struct asus_kbd_leds *kbd_backlight;
>>> +       struct asus_worker *worker;
>>> +       unsigned int kbd_backlight_brightness;
>>>         const struct asus_touchpad_info *tp;
>>>         struct power_supply *battery;
>>>         struct power_supply_desc battery_desc;
>>> @@ -141,7 +167,7 @@ struct asus_drvdata {
>>>         int battery_stat;
>>>         bool battery_in_query;
>>>         unsigned long battery_next_query;
>>> -       struct work_struct fn_lock_sync_work;
>>> +       struct asus_hid_listener listener;
>>>         bool fn_lock;
>>>  };
>>>
>>> @@ -211,6 +237,29 @@ static const u8 asus_report_id_init[] = {
>>>         FEATURE_KBD_LED_REPORT_ID2
>>>  };
>>>
>>> +/*
>>> + * Send events to asus-wmi driver for handling special keys
>>> + */
>>> +static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
>>> +{
>>> +       int err;
>>> +       u32 retval;
>>> +
>>> +       err = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DEVS,
>>> +                                      ASUS_WMI_METHODID_NOTIF, code, &retval);
>>> +       if (err) {
>>> +               pr_warn("Failed to notify asus-wmi: %d\n", err);
>>> +               return err;
>>> +       }
>>> +
>>> +       if (retval != 0) {
>>> +               pr_warn("Failed to notify asus-wmi (retval): 0x%x\n", retval);
>>> +               return -EIO;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>>  static void asus_report_contact_down(struct asus_drvdata *drvdat,
>>>                 int toolType, u8 *data)
>>>  {
>>> @@ -331,25 +380,71 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
>>>  }
>>>
>>>  /*
>>> - * Send events to asus-wmi driver for handling special keys
>>> + * Used in atomic contexts to schedule work involving sleeps operations or
>>> + * asus-wmi interactions.
>>> + *
>>> + * Caller is responsible to store relevant data in the structure to carry out
>>> + * the required action.
>>> + *
>>> + * This function must be called while the spin lock protecting the workqueue
>>> + * is already being held.
>>>   */
>>> -static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
>>> +static void asus_worker_schedule(struct asus_worker *worker, struct asus_work_action *action)
>>>  {
>>> -       int err;
>>> -       u32 retval;
>>> -
>>> -       err = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DEVS,
>>> -                                      ASUS_WMI_METHODID_NOTIF, code, &retval);
>>> -       if (err) {
>>> -               pr_warn("Failed to notify asus-wmi: %d\n", err);
>>> -               return err;
>>> +       if (worker->removed) {
>>> +               kfree(action);
>>> +               return;
>>>         }
>>>
>>> -       if (retval != 0) {
>>> -               pr_warn("Failed to notify asus-wmi (retval): 0x%x\n", retval);
>>> -               return -EIO;
>>> +       list_add_tail(&action->node, &worker->actions);
>>> +       schedule_work(&worker->work);
>>> +}
>>> +
>>> +static int asus_kbd_fn_lock_set(struct asus_drvdata *drvdata, bool enabled)
>>> +{
>>> +       struct asus_work_action *action;
>>> +       unsigned long flags;
>>> +
>>> +       action = kzalloc_obj(struct asus_work_action);
>>> +       if (!action)
>>> +               return -ENOMEM;
>>> +
>>> +       drvdata->fn_lock = enabled;
>>> +       action->type = FN_LOCK_SYNC;
>>> +       action->data.fn_lock = drvdata->fn_lock;
>>> +       INIT_LIST_HEAD(&action->node);
>>> +
>>> +       spin_lock_irqsave(&drvdata->worker->lock, flags);
>>> +       asus_worker_schedule(drvdata->worker, action);
>>> +       spin_unlock_irqrestore(&drvdata->worker->lock, flags);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int asus_kbd_wmi_fan_send(struct asus_drvdata *drvdata, u8 *report_data,
>>> +                                size_t report_size)
>>> +{
>>> +       struct asus_work_action *action;
>>> +       unsigned long flags;
>>> +
>>> +       if (report_size > FEATURE_KBD_REPORT_SIZE) {
>>> +               hid_err(drvdata->hdev, "Invalid report size for fan event: %zu\n", report_size);
>>> +               return -EINVAL;
>>>         }
>>>
>>> +       action = kzalloc_obj(struct asus_work_action);
>>> +       if (!action)
>>> +               return -ENOMEM;
>>> +
>>> +       action->type = WMI_FAN;
>>> +       action->data.fan_hid_data.report_size = report_size;
>>> +       memcpy(action->data.fan_hid_data.report_data, report_data, report_size);
>>> +       INIT_LIST_HEAD(&action->node);
>>> +
>>> +       spin_lock_irqsave(&drvdata->worker->lock, flags);
>>> +       asus_worker_schedule(drvdata->worker, action);
>>> +       spin_unlock_irqrestore(&drvdata->worker->lock, flags);
>>> +
>>>         return 0;
>>>  }
>>>
>>> @@ -357,6 +452,7 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
>>>                       struct hid_usage *usage, __s32 value)
>>>  {
>>>         struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>>> +       int ret;
>>>
>>>         if ((usage->hid & HID_USAGE_PAGE) == HID_UP_ASUSVENDOR &&
>>>             (usage->hid & HID_USAGE) != 0x00 &&
>>> @@ -375,8 +471,11 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
>>>                         return !asus_hid_event(ASUS_EV_BRTTOGGLE);
>>>                 case KEY_FN_ESC:
>>>                         if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
>>> -                               drvdata->fn_lock = !drvdata->fn_lock;
>>> -                               schedule_work(&drvdata->fn_lock_sync_work);
>>> +                               ret = asus_kbd_fn_lock_set(drvdata, !drvdata->fn_lock);
>>> +                               if (ret) {
>>> +                                       hid_err(hdev, "Error while toggling FN lock: %d\n", ret);
>>> +                                       return ret;
>>> +                               }
>>>                         }
>>>                         break;
>>>                 }
>>> @@ -389,6 +488,12 @@ static int asus_raw_event(struct hid_device *hdev,
>>>                 struct hid_report *report, u8 *data, int size)
>>>  {
>>>         struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>>> +       int ret;
>>> +
>>> +       if (size < 2) {
>>> +               hid_dbg(hdev, "Unexpected keyboard report size %d\n", size);
>>> +               return 0;
>>> +       }
>>>
>>>         if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
>>>                 return asus_report_battery(drvdata, data, size);
>>> @@ -414,19 +519,13 @@ static int asus_raw_event(struct hid_device *hdev,
>>>                          * pass to userspace so it can implement its own fan control.
>>>                          */
>>>                         if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
>>> -                               int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
>>> +                               ret = asus_kbd_wmi_fan_send(drvdata, data, size);
>>>
>>> -                               if (ret == 0) {
>>> -                                       /* Successfully handled by asus-wmi, block event */
>>> +                               /* if execution deferred successfully block event */
>>> +                               if (ret == 0)
>>>                                         return -1;
>>> -                               }
>>>
>>> -                               /*
>>> -                                * Warn if asus-wmi failed (but not if it's unavailable).
>>> -                                * Let the event reach userspace in all failure cases.
>>> -                                */
>>> -                               if (ret != -ENODEV)
>>> -                                       hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
>>> +                               return ret;
>>>                         }
>>>
>>>                         /*
>>> @@ -569,59 +668,151 @@ static int asus_kbd_disable_oobe(struct hid_device *hdev)
>>>         return 0;
>>>  }
>>>
>>> -static int asus_kbd_set_fn_lock(struct hid_device *hdev, bool enabled)
>>> +static void asus_kbd_set_fn_lock(struct hid_device *hdev, bool enabled)
>>>  {
>>> -       u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x4e, !!enabled };
>>> +       const u8 buf[FEATURE_KBD_REPORT_SIZE] = { FEATURE_KBD_REPORT_ID, 0xd0, 0x4e, !!enabled };
>>> +       int ret;
>>>
>>> -       return asus_kbd_set_report(hdev, buf, sizeof(buf));
>>> +       ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
>>> +       if (ret < 0)
>>> +               hid_err(hdev, "Asus failed to set fn lock: %d\n", ret);
>>>  }
>>>
>>> -static void asus_sync_fn_lock(struct work_struct *work)
>>> +static void asus_kbd_set_brightness(struct hid_device *hdev, u8 brightness)
>>>  {
>>> -       struct asus_drvdata *drvdata =
>>> -       container_of(work, struct asus_drvdata, fn_lock_sync_work);
>>> +       const u8 buf[FEATURE_KBD_REPORT_SIZE] = {
>>> +               FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, brightness
>>> +       };
>>> +       int ret;
>>>
>>> -       asus_kbd_set_fn_lock(drvdata->hdev, drvdata->fn_lock);
>>> +       ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
>>> +       if (ret < 0)
>>> +               hid_err(hdev, "Asus failed to set keyboard backlight: %d\n", ret);
>>>  }
>>>
>>> -static void asus_schedule_work(struct asus_kbd_leds *led)
>>> +static void asus_kbd_wmi_fan(struct hid_device *hdev, struct hid_raw_event_data *data)
>>>  {
>>> +       struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>>> +       int ret;
>>> +
>>> +       ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
>>> +
>>> +       /*
>>> +        * Warn if asus-wmi failed (but not if it's unavailable).
>>> +        * Let the event reach userspace in all failure cases.
>>> +        */
>>> +       switch (ret) {
>>> +       case -ENODEV:
>>> +               break;
>>> +       case 0:
>>> +               return;
>>> +       default:
>>> +               hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
>>> +               break;
>>> +       }
>>> +
>>> +       /* Fallback: pass the raw event to the HID core */
>>> +       hid_report_raw_event(hdev, HID_INPUT_REPORT,
>>> +                            data->report_data, data->report_size,
>>> +                            data->report_size, 1);
>>> +}
>>> +
>>> +static void asus_kbd_backlight_set(struct asus_hid_listener *listener, int brightness)
>>> +{
>>> +       struct asus_drvdata *drvdata = container_of(listener, struct asus_drvdata, listener);
>>> +       struct asus_worker *worker = drvdata->worker;
>>> +       struct asus_work_action *action;
>>>         unsigned long flags;
>>>
>>> -       spin_lock_irqsave(&led->lock, flags);
>>> -       if (!led->removed)
>>> -               schedule_work(&led->work);
>>> -       spin_unlock_irqrestore(&led->lock, flags);
>>> +       drvdata->kbd_backlight_brightness = brightness;
>>> +
>>> +       action = kzalloc_obj(struct asus_work_action);
>>> +       if (!action) {
>>> +               hid_warn(drvdata->hdev, "Failed to allocate memory for backlight action\n");
>>> +               return;
>>> +       }
>>> +
>>> +       action->type = BRIGHTNESS_SET;
>>> +       action->data.brightness = brightness;
>>> +       INIT_LIST_HEAD(&action->node);
>>> +
>>> +       spin_lock_irqsave(&worker->lock, flags);
>>> +       asus_worker_schedule(worker, action);
>>> +       spin_unlock_irqrestore(&worker->lock, flags);
>>>  }
>>>
>>> -static void asus_kbd_backlight_set(struct asus_hid_listener *listener,
>>> -                                  int brightness)
>>> +static void asus_work(struct work_struct *work)
>>>  {
>>> -       struct asus_kbd_leds *led = container_of(listener, struct asus_kbd_leds,
>>> -                                                listener);
>>> +       struct asus_worker *worker = container_of(work, struct asus_worker, work);
>>> +       struct asus_work_action *action = NULL;
>>>         unsigned long flags;
>>>
>>> -       spin_lock_irqsave(&led->lock, flags);
>>> -       led->brightness = brightness;
>>> -       spin_unlock_irqrestore(&led->lock, flags);
>>> +       /* Save the action to be performed and clear the flag */
>>> +       spin_lock_irqsave(&worker->lock, flags);
>>> +       if (!list_empty(&worker->actions)) {
>>> +               action = list_first_entry(&worker->actions,
>>> +                                         struct asus_work_action, node);
>>> +               list_del(&action->node);
>>> +       }
>>> +       spin_unlock_irqrestore(&worker->lock, flags);
>>> +
>>> +       if (!action)
>>> +               return;
>>> +
>>> +       switch (action->type) {
>>> +       case BRIGHTNESS_SET:
>>> +               asus_kbd_set_brightness(worker->hdev, action->data.brightness);
>>> +               break;
>>> +       case FN_LOCK_SYNC:
>>> +               asus_kbd_set_fn_lock(worker->hdev, action->data.fn_lock);
>>> +               break;
>>> +       case WMI_FAN:
>>> +               asus_kbd_wmi_fan(worker->hdev, &action->data.fan_hid_data);
>>> +               break;
>>> +       default:
>>> +               hid_err(worker->hdev, "Invalid action type: %d\n", action->type);
>>> +               break;
>>> +       }
>>> +
>>> +       kfree(action);
>>>
>>> -       asus_schedule_work(led);
>>> +       /* Re-schedule if there are more pending actions */
>>> +       spin_lock_irqsave(&worker->lock, flags);
>>> +       if (!list_empty(&worker->actions))
>>> +               schedule_work(&worker->work);
>>> +       spin_unlock_irqrestore(&worker->lock, flags);
>>>  }
>>>
>>> -static void asus_kbd_backlight_work(struct work_struct *work)
>>> +static int asus_worker_create(struct hid_device *hdev, struct asus_drvdata *drvdata)
>>>  {
>>> -       struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work);
>>> -       u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
>>> -       int ret;
>>> +       drvdata->worker = devm_kzalloc(&hdev->dev, sizeof(struct asus_worker), GFP_KERNEL);
>>> +       if (!drvdata->worker)
>>> +               return -ENOMEM;
>>> +
>>> +       drvdata->worker->removed = false;
>>> +       drvdata->worker->hdev = hdev;
>>> +       INIT_LIST_HEAD(&drvdata->worker->actions);
>>> +
>>> +       INIT_WORK(&drvdata->worker->work, asus_work);
>>> +       spin_lock_init(&drvdata->worker->lock);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static void asus_worker_stop(struct asus_worker *worker)
>>> +{
>>> +       struct asus_work_action *action, *tmp;
>>>         unsigned long flags;
>>>
>>> -       spin_lock_irqsave(&led->lock, flags);
>>> -       buf[4] = led->brightness;
>>> -       spin_unlock_irqrestore(&led->lock, flags);
>>> +       spin_lock_irqsave(&worker->lock, flags);
>>> +       worker->removed = true;
>>> +       list_for_each_entry_safe(action, tmp, &worker->actions, node) {
>>> +               list_del(&action->node);
>>> +               kfree(action);
>>> +       }
>>> +       spin_unlock_irqrestore(&worker->lock, flags);
>>>
>>> -       ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf));
>>> -       if (ret < 0)
>>> -               hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret);
>>> +       cancel_work_sync(&worker->work);
>>>  }
>>>
>>>  /*
>>> @@ -760,23 +951,11 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>                         le16_to_cpu(udev->descriptor.idProduct));
>>>         }
>>>
>>> -       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
>>> -                                             sizeof(struct asus_kbd_leds),
>>> -                                             GFP_KERNEL);
>>> -       if (!drvdata->kbd_backlight)
>>> -               return -ENOMEM;
>>> -
>>> -       drvdata->kbd_backlight->removed = false;
>>> -       drvdata->kbd_backlight->brightness = 0;
>>> -       drvdata->kbd_backlight->hdev = hdev;
>>> -       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 = asus_hid_register_listener(&drvdata->kbd_backlight->listener);
>>> +       drvdata->listener.brightness_set = asus_kbd_backlight_set;
>>> +       ret = asus_hid_register_listener(&drvdata->listener);
>>>         if (ret < 0) {
>>> -               /* No need to have this still around */
>>> -               devm_kfree(&hdev->dev, drvdata->kbd_backlight);
>>> +               hid_err(hdev, "Unable to register kbd brightness listener: %d\n", ret);
>>> +               drvdata->listener.brightness_set = NULL;
>>>         }
>>>
>>>         return ret;
>>> @@ -965,11 +1144,9 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>>>                 }
>>>         }
>>>
>>> -       if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
>>> -               drvdata->fn_lock = true;
>>> -               INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
>>> -               asus_kbd_set_fn_lock(hdev, true);
>>> -       }
>>> +       if ((drvdata->quirks & QUIRK_HID_FN_LOCK) &&
>>> +           (asus_kbd_fn_lock_set(drvdata, true)))
>>> +               hid_warn(hdev, "Error while setting FN lock to ON\n");
>>>
>>>         if (drvdata->tp) {
>>>                 int ret;
>>> @@ -1004,11 +1181,9 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
>>>
>>>         drvdata->input = input;
>>>
>>> -       if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
>>> -               drvdata->fn_lock = true;
>>> -               INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
>>> -               asus_kbd_set_fn_lock(hdev, true);
>>> -       }
>>> +       if ((drvdata->quirks & QUIRK_HID_FN_LOCK) &&
>>> +           (asus_kbd_fn_lock_set(drvdata, true)))
>>> +               hid_warn(hdev, "Error while setting FN lock to ON\n");
>>>
>>>         return 0;
>>>  }
>>> @@ -1171,20 +1346,16 @@ static int asus_start_multitouch(struct hid_device *hdev)
>>>  static int __maybe_unused asus_resume(struct hid_device *hdev)
>>>  {
>>>         struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>>> -       int ret = 0;
>>>
>>> -       if (drvdata->kbd_backlight) {
>>> -               const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4,
>>> -                               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);
>>> -                       goto asus_resume_err;
>>> -               }
>>> -       }
>>> +       /*
>>> +        * If we have a backlight listener registered, restore the previous state,
>>> +        * in case of error do not fail: most models restore the backlight
>>> +        * automatically, and the error is non-fatal.
>>> +        */
>>> +       if (drvdata->listener.brightness_set)
>>> +               asus_kbd_backlight_set(&drvdata->listener, drvdata->kbd_backlight_brightness);
>>>
>>> -asus_resume_err:
>>> -       return ret;
>>> +       return 0;
>>>  }
>>>
>>>  static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
>>> @@ -1294,6 +1465,12 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>>                         is_vendor = true;
>>>         }
>>>
>>> +       ret = asus_worker_create(hdev, drvdata);
>>> +       if (ret) {
>>> +               hid_warn(hdev, "Failed to initialize worker: %d\n", ret);
>>> +               return ret;
>>> +       }
>>> +
>>>         ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>>>         if (ret) {
>>>                 hid_err(hdev, "Asus hw start failed: %d\n", ret);
>>> @@ -1343,6 +1520,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>>
>>>         return 0;
>>>  err_stop_hw:
>>> +       if (drvdata->listener.brightness_set)
>>> +               asus_hid_unregister_listener(&drvdata->listener);
>>> +
>>> +       asus_worker_stop(drvdata->worker);
>>>         hid_hw_stop(hdev);
>>>         return ret;
>>>  }
>>> @@ -1350,21 +1531,11 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>>  static void asus_remove(struct hid_device *hdev)
>>>  {
>>>         struct asus_drvdata *drvdata = hid_get_drvdata(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);
>>> -
>>> -               cancel_work_sync(&drvdata->kbd_backlight->work);
>>> -       }
>>>
>>> -       if (drvdata->quirks & QUIRK_HID_FN_LOCK)
>>> -               cancel_work_sync(&drvdata->fn_lock_sync_work);
>>> +       if (drvdata->listener.brightness_set)
>>> +               asus_hid_unregister_listener(&drvdata->listener);
>>>
>>> +       asus_worker_stop(drvdata->worker);
>>>         hid_hw_stop(hdev);
>>>  }
>>>
>>> --
>>> 2.47.3
>>>
>>>