Thread (24 messages) flat view 24 messages, 2 authors, 7d ago

Re: [PATCH v4 08/13] HID: asus: add joysticks anti-deadzone configuration

From: sashiko-bot@kernel.org
Date: 2026-08-30 23:57:26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `ally_drvdata.config` is never initialized with the allocated `ally_cfg` pointer, making all configuration sysfs attributes return `-ENODEV`.
--

commit a0409ef85990017ccc5539d469ac0af6eff50ada
Author: Denis Benato [off-list ref]

HID: asus: add joysticks anti-deadzone configuration

This commit adds sysfs attributes to configure the anti-deadzone parameters
for the resistive joysticks on ROG Ally devices, allowing userspace to
compensate for drift over time.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 6688615f644a1..c365b96d4486f 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1318,6 +1318,193 @@ static struct device_attribute dev_attr_right_joystick_outer_threshold =
[ ... ]
+static ssize_t left_joystick_anti_deadzone_show(struct device *dev, struct device_attribute *attr,
+						char *buf)
+{
+	struct hid_device *hdev = to_hid_device(dev);
+	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
+	struct ally_handheld *const ally = drvdata->rog_ally;
+	struct ally_config *cfg;
+
+	if (!ally)
+		return -ENODEV;
+
+	cfg = ally_get_config(ally);
+	if (!cfg)
+		return -ENODEV;
[Severity: High]
This is a pre-existing issue, but won't this newly added sysfs attribute (and
all other configuration attributes) unconditionally return -ENODEV here?

Looking at hid_asus_ally_probe(), the successfully allocated configuration
object is never assigned to the controller's configuration pointer:

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.cfg_hdev = hdev;
        /* missing: ally_drvdata.config = ally_cfg; */
        spin_unlock_irqrestore(&ally_data_lock, flags);
        ...
}

Because of this, won't ally_get_config() consistently read a NULL pointer,
rendering the configuration features completely unusable and causing
hid_asus_ally_init() to silently skip hardware configuration on probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830234320.114356-1-denis.benato@linux.dev?part=8
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help