Re: [PATCH 01/12] HID: asus: reinitialize the device after exiting a sleep state
From: sashiko-bot@kernel.org
Date: 2026-08-13 14:55:08
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The newly added error handling blocks in `asus_probe()` and `asus_reset_resume()` are completely dead code because `asus_initialize_reports()` unconditionally returns 0. --
--- Patch [1]: [PATCH 01/12] HID: asus: reinitialize the device after exiting a sleep state ---commit 0ca493a924cc77202a10af1860055debe0b725bc Author: Denis Benato [off-list ref] HID: asus: reinitialize the device after exiting a sleep state This patch extracts keyboard initialization into a helper function and calls it during reset resume, ensuring devices like the ROG ally have their EC string sent back after s2idle.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1360,6 +1360,22 @@ static int asus_start_multitouch(struct hid_device *hdev) return 0; } +static int asus_initialize_reports(struct hid_device *hdev) +{ + int ret; + + for (int r = 0; r < ARRAY_SIZE(asus_report_id_init); r++) { + if (asus_has_report_id(hdev, asus_report_id_init[r])) { + ret = asus_kbd_init(hdev, asus_report_id_init[r]); + if (ret < 0) + hid_warn(hdev, "Failed to initialize 0x%x: %d.\n", + asus_report_id_init[r], ret); + } + } + + return 0; +}
[Severity: Low] Does this helper function need to return the error code? Because it unconditionally returns 0, the error codes captured in the ret variable from asus_kbd_init() are dropped after the warning is printed.
static int __maybe_unused asus_reset_resume(struct hid_device *hdev)
{
struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+
+ ret = asus_initialize_reports(hdev);
+ if (ret) {
+ hid_err(hdev, "Asus initialize reports failed: %d\n", ret);
+ goto asus_reset_resume_err;
+ }[Severity: Low] Since asus_initialize_reports() unconditionally returns 0, does that make this error path dead code? If the intent was to abort on initialization failure, should the helper return the error code instead? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1493,13 +1518,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id) return ret; } - for (int r = 0; r < ARRAY_SIZE(asus_report_id_init); r++) { - if (asus_has_report_id(hdev, asus_report_id_init[r])) { - ret = asus_kbd_init(hdev, asus_report_id_init[r]); - if (ret < 0) - hid_warn(hdev, "Failed to initialize 0x%x: %d.\n", - asus_report_id_init[r], ret); - } + ret = asus_initialize_reports(hdev); + if (ret) { + hid_err(hdev, "Asus initialize reports failed: %d\n", ret); + goto err_stop_hw; }
[Severity: Low] Similar to the check in asus_reset_resume(), is this error handling reachable? The original code in asus_probe() only printed a warning on failure and continued, so returning 0 preserves the old behavior, but it leaves this new goto err_stop_hw path inaccessible. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813144736.2477941-1-denis.benato@linux.dev?part=1