Re: [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context

[Antheas Kapenekakis]

On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@xxxxxxxxx> wrote:
>
> Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
> asus_raw_event() that is called in atomic context.
>
> This commit changes when methods are called, not method parameters:
> the driver behaves as before.

Observe the new AI guidelines and add assisted by tags if necessary.
Or write the patches manually for mainline. This sentence is an AI
euphemism

Who let 1489a34e97ef through? I implemented the plumbing for WMI
callbacks with asus_hid_event last year because WMI cannot be called
through HID. Brightness events had the exact same problem.
kbd_led_work/kbd_led_update_all could be renamed and used for this. I
would rather avoid introducing a new work queue. The one I added is
enough if not too much

Antheas

> Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
> Reported-by: sahiko-bot@xxxxxxxxxx
> Signed-off-by: Denis Benato <denis.benato@xxxxxxxxx>
> ---
>  drivers/hid/hid-asus.c | 67 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index f38b18ad65c6..a6467172c455 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -150,6 +150,13 @@ struct asus_drvdata {
>         unsigned long battery_next_query;
>         struct work_struct fn_lock_sync_work;
>         bool fn_lock;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       struct work_struct wmi_work;
> +       bool wmi_work_disabled;
> +       u8 wmi_data[FEATURE_KBD_REPORT_SIZE];
> +       int wmi_size;
> +       spinlock_t wmi_lock;
> +#endif
>         struct asus_xgm_led *xgm_led;
>  };
>
> @@ -338,6 +345,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
>         return 0;
>  }
>
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>  /*
>   * Send events to asus-wmi driver for handling special keys
>   */
> @@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
>         return 0;
>  }
>
> +static void asus_wmi_work(struct work_struct *work)
> +{
> +       struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
> +       u8 report_data[FEATURE_KBD_REPORT_SIZE];
> +       int report_size;
> +       unsigned long flags;
> +       int ret;
> +
> +       spin_lock_irqsave(&drvdata->wmi_lock, flags);
> +       memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
> +       report_size = drvdata->wmi_size;
> +       spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> +       ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
> +       if (ret) {
> +               if (ret != -ENODEV)
> +                       hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
> +
> +               /* Fallback: pass the raw event to the HID core */
> +               hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
> +                                    report_data, report_size,
> +                                    report_size, 1);
> +       }
> +}
> +#endif
> +
>  static int asus_event(struct hid_device *hdev, struct hid_field *field,
>                       struct hid_usage *usage, __s32 value)
>  {
> @@ -397,6 +431,9 @@ 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);
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       unsigned long flags;
> +#endif
>
>         if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
>                 return asus_report_battery(drvdata, data, size);
> @@ -422,19 +459,18 @@ 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);
> -
> -                               if (ret == 0) {
> -                                       /* Successfully handled by asus-wmi, block event */
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +                               spin_lock_irqsave(&drvdata->wmi_lock, flags);
> +                               memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
> +                               drvdata->wmi_size = size;
> +                               spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
> +
> +                               if (!drvdata->wmi_work_disabled) {
> +                                       schedule_work(&drvdata->wmi_work);
> +                                       /* Successfully handled asynchronously, block event */
>                                         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);
> +#endif
>                         }
>
>                         /*
> @@ -1350,6 +1386,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>
>         drvdata->hdev = hdev;
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
> +       spin_lock_init(&drvdata->wmi_lock);
> +#endif
>
>         if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
>                 ret = asus_battery_probe(hdev);
> @@ -1460,6 +1500,11 @@ static void asus_remove(struct hid_device *hdev)
>         if (drvdata->quirks & QUIRK_HID_FN_LOCK)
>                 cancel_work_sync(&drvdata->fn_lock_sync_work);
>
> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
> +       drvdata->wmi_work_disabled = true;
> +       cancel_work_sync(&drvdata->wmi_work);
> +#endif
> +
>         if (drvdata->xgm_led)
>                 led_classdev_unregister(&drvdata->xgm_led->cdev);
>
> --
> 2.47.3
>
>