Re: [PATCH v5 05/13] HID: asus: add vibration strength configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Vibration intensity restoration logic added to `hid_asus_ally_init` is bypassed during device probe because the function exits early when `ally_drvdata.config` is NULL.
--

commit 9d78d1047388b95822776aa7476f2875ba47b078
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: add vibration strength configuration

ASUS ROG Ally handhelds support the vibration strength to be configured:
add sysfs attributes to allow userspace configure motors vibration
intensity.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 4714ebb4a5435..659956ebaefca 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

> @@ -1215,6 +1399,17 @@ static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *all
>  				 ret);
>  	}
>  
> +	/*
> +	 * The MCU does not keep the vibration intensity across a reset:
> +	 * send the recorded values again, whichever interface probed.
> +	 */
> +	ret = ally_set_vibration_intensity(ally, hdev,
> +					   cfg->vibration_intensity_left,
> +					   cfg->vibration_intensity_right);
> +	if (ret < 0)
> +		hid_warn(hdev, "Failed to restore vibration intensity: %d\n",
> +			 ret);
> +
>  	return 0;
>  }
>  

[Severity: Medium]
Will this initialization ever run during the device probe?

Looking at hid_asus_ally_probe(), hid_asus_ally_init() is called before the
configuration is actually assigned to the handheld structure:

drivers/hid/hid-asus.c:hid_asus_ally_probe() {
    ...
    ally_cfg = ally_config_create(hdev, &ally_drvdata);
    if (IS_ERR(ally_cfg)) {
        ...
    }

    ret = hid_asus_ally_init(hdev, &ally_drvdata);
    if (ret < 0) {
        ...
    }

    spin_lock_irqsave(&ally_data_lock, flags);
    ally_drvdata.config = ally_cfg;
    ...
}

Because ally_drvdata.config is not yet assigned, the early check inside
hid_asus_ally_init() will always trigger and return early:

drivers/hid/hid-asus.c:hid_asus_ally_init() {
    ...
    cfg = ally_get_config(ally);
    if (!cfg)
        return 0;
    ...
}

Does the vibration intensity restoration logic get unintentionally bypassed
during probe, causing the device to lose user-configured vibration settings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904145845.184887-1-denis.benato@linux.dev?part=5