Re: [PATCH v5 05/13] HID: asus: add vibration strength configuration
From: sashiko-bot@kernel.org
Date: 2026-09-04 15:39:20
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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