Re: [PATCH v4 04/13] HID: asus: add gamepad configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The allocated configuration structure `ally_cfg` is never assigned to the global driver data, making all sysfs attributes non-functional and initialization logic dead code.
--

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

HID: asus: add gamepad configuration

This patch adds the base configuration structures for the gamepad
configuration, detects capabilities, and initializes the device in a
known state.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index f36cf93e380d5..7fdb77432171e 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -1004,6 +1307,14 @@ static struct ally_handheld *hid_asus_ally_probe(struct hid_device *hdev)
>  			return ERR_PTR(ret);
>  		}
>  
> +		ally_cfg = ally_config_create(hdev, &ally_drvdata);
> +		if (IS_ERR(ally_cfg)) {
> +			hid_err(hdev, "Failed to create Ally cfg: %ld\n",
> +				PTR_ERR(ally_cfg));
> +			ally_put_udev_if_orphaned();
> +			return ERR_PTR(PTR_ERR(ally_cfg));
> +		}
> +

[Severity: Medium]
Does this code intentionally skip assigning the newly created configuration
to the driver data?

Looking at hid_asus_ally_probe(), ally_cfg is allocated via
ally_config_create(), but it is never assigned to ally_drvdata.config
(for example, alongside the ally_drvdata.cfg_hdev = hdev assignment later
in the function).

Without this assignment, won't ally_get_config() always return NULL?
This appears to make the gamepad sysfs attributes non-functional, as
subsequent sysfs callbacks will unconditionally return -ENODEV when
attempting to read or write the unassigned configuration.

>  		ret = hid_asus_ally_init(hdev, &ally_drvdata);
>  		if (ret < 0) {
>  			ally_put_udev_if_orphaned();

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