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

[sashiko-bot@kernel.org]

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 <denis.benato@linux.dev>

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.

> 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

[ ... ]

> @@ -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