Re: [PATCH v4 13/13] HID: asus: add support for btn remapping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional probe failure due to duplicated sysfs group registration
--

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

    HID: asus: add support for btn remapping

This commit adds support for button remapping on the Asus ROG Ally, 
allowing assignment of different events to physical buttons. It exposes a 
remap_index sysfs attribute and refreshes the software cache when the 
hardware defaults are automatically restored upon gamepad mode changes.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 7fc04d29de8de..b921e2af53662 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -3098,6 +4052,38 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
>  		}
>  	}
>  
> +	for (sysfs_i = 0; sysfs_i < ARRAY_SIZE(ally_attr_groups); sysfs_i++) {
> +		ret = devm_device_add_group(&hdev->dev, &ally_attr_groups[sysfs_i]);

[Severity: High]
Does this code inadvertently duplicate the sysfs registration blocks?

Looking at the full ally_config_create() function after this patch, there are
now two identical loops registering ally_attr_groups and two identical
blocks registering ally_cal_attr_groups and ally_create_button_attributes().

When the driver initialization reaches this newly added second loop,
devm_device_add_group() will attempt to register the exact same sysfs
attributes that were successfully registered in the first pass. The sysfs
core detects this duplicate file creation and returns -EEXIST, which causes
an unconditional early exit to the error handler.

Can this second set of registrations be removed to prevent the probe from
always failing?

> +		if (ret < 0) {
> +			hid_err(hdev, "Failed to create sysfs group '%s': %d\n",
> +				ally_attr_groups[sysfs_i].name ?: "", ret);
> +			goto ally_config_create_sysfs_err;
> +		}
> +	}
> +
> +	/*
> +	 * Skip the calibration, anti-deadzone and response curve groups when
> +	 * none of the features they expose is supported.
> +	 */
> +	if (cfg->user_cal_support || cfg->anti_deadzone_support ||
> +	    cfg->resp_curve_support) {
> +		for (sysfs_i = 0; sysfs_i < ARRAY_SIZE(ally_cal_attr_groups); sysfs_i++) {
> +			ret = devm_device_add_group(&hdev->dev,
> +						    ally_cal_attr_groups[sysfs_i]);
> +			if (ret < 0) {
> +				hid_err(hdev, "Failed to create sysfs group '%s': %d\n",
> +					ally_cal_attr_groups[sysfs_i]->name, ret);
> +				goto ally_config_create_sysfs_err;
> +			}
> +		}
> +	}
> +
> +	ret = ally_create_button_attributes(hdev, ally, cfg);
> +	if (ret < 0) {
> +		hid_err(hdev, "Failed to create button attributes: %d\n", ret);
> +		goto ally_config_create_sysfs_err;
> +	}
> +
>  	/* So far the only hardware this is supported is the Ally 1 */
>  	if (cfg->xbox_controller_support) {
>  		ret = ally_set_xbox_controller(hdev, ally, cfg, true);

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