Re: [PATCH v6 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:
- [Medium] Button remapping attributes are never exposed for devices lacking turbo support because `ally_create_button_attributes` is incorrectly gated by `if (cfg->turbo_support)` in the caller.
--

commit f62bd64b66b8edafa21a4dcfaec466f614d7bae9
Author: Denis Benato <denis.benato@linux.dev>
Subject: HID: asus: add support for btn remapping

Adds support for button remapping to the ROG Ally, exposing remap_index,
remap, and macro sysfs attributes for physical buttons on the device.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 5cdbcc2bd1bc7..f8655c1ab7359 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -2932,15 +3842,12 @@ static void ally_btn_turbo_init_attrs(struct ally_btn_sysfs_entry *entry)
>   * Return: 0 on success, negative errno on failure
>   */
>  static int ally_create_button_attributes(struct hid_device *hdev,
> -					 struct ally_handheld *ally,
> -					 struct ally_config *cfg)
> +					  struct ally_handheld *ally,
> +					  struct ally_config *cfg)
>  {
>  	struct ally_btn_sysfs_entry *entries;
>  	int i, ret;
>  
> -	if (!cfg->turbo_support)
> -		return 0;
> -

[ ... ]

> +		/* Set up attributes array based on what's supported */
> +		if (cfg->turbo_support) {
> +			entries[i].attrs[4] =
> +				&entries[i].remap_attr->dev_attr.attr;
> +			entries[i].attrs[5] =
> +				&entries[i].macro_attr->dev_attr.attr;
> +			entries[i].attrs[6] =
> +				&entries[i].attr_remap_index.attr;
> +			entries[i].attrs[7] = NULL;
> +		} else {
> +			entries[i].attrs[0] =
> +				&entries[i].remap_attr->dev_attr.attr;
> +			entries[i].attrs[1] =
> +				&entries[i].macro_attr->dev_attr.attr;
> +			entries[i].attrs[2] =
> +				&entries[i].attr_remap_index.attr;
> +			entries[i].attrs[3] = NULL;
> +		}

[Severity: Medium]
Since ally_create_button_attributes() is now designed to create remap
attributes even when the device lacks turbo support, should the caller
ally_config_create() also be updated to match?

Currently, ally_config_create() still completely gates calling this function
on cfg->turbo_support:

	if (cfg->turbo_support) {
		ret = ally_create_button_attributes(hdev, ally, cfg);
		if (ret < 0) {
			...
		}
	}

Does this result in a regression where the new remap attributes are
completely inaccessible for devices without turbo support?

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