Re: [PATCH v11 09/18] HID: asus: add support for gamepad mode

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Sun, 25 Oct 2026, Denis Benato wrote:

> ROG Ally devices can emulate either a mouse+keyboard (desktop mode)
> or an gamepad device (xbox360 controller in ROG ally and a custom
> DInput device on newer models): add support for switching the current
> controller mode.
> 
> Signed-off-by: Luke Jones <luke@ljones.dev>
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
>  drivers/hid/hid-asus.c | 158 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 157 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index a3a21a1493aa..fae5f36a9fa6 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -198,6 +198,11 @@ struct ally_joystick_resp_curve {
>  	struct ally_joystick_resp_curve_param entry_4;
>  } __packed;
>  
> +enum ally_gamepad_mode_index {
> +	ALLY_GAMEPAD_MODE_GAMEPAD = 0x01,
> +	ALLY_GAMEPAD_MODE_KEYBOARD = 0x02,
> +};
> +
>  struct ally_config {
>  	/* Must be locked if the data is being changed */
>  	struct mutex config_mutex;
> @@ -411,6 +416,15 @@ enum ally_command_codes {
>  /* XInput rumble magnitudes use the hardware's 0..100 intensity range. */
>  #define ALLY_FF_MAX_INTENSITY 100
>  
> +static const char *const ally_gamepad_mode_text[] = {
> +	"gamepad", "desktop"

So this paired with ally_gamepad_mode, because ally_gamepad_mode_index is 
off-by-one?

> +};
> +
> +static const u8 ally_gamepad_mode[] = {
> +	ALLY_GAMEPAD_MODE_GAMEPAD,
> +	ALLY_GAMEPAD_MODE_KEYBOARD

Almost duplicate of _index variant?

Couldn't you use [xx] = style initializers with ally_gamepad_mode_text[] 
instead?

> +};
> +
>  static const u8 ALLY_FORCE_FEEDBACK_OFF[] = {
>  	0x0D, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xEB
>  };
> @@ -930,6 +944,137 @@ static ssize_t xbox_controller_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(xbox_controller);
>  
> +/**
> + * ally_set_gamepad_mode() - Set the gamepad operating mode
> + * @ally: ally handheld structure
> + * @hdev: HID device
> + * @mode: gamepad mode to set
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +static int ally_set_gamepad_mode(struct ally_handheld *ally, struct hid_device *hdev, u8 mode)
> +{
> +	u8 payload[] = { mode };
> +	int ret;
> +
> +	if (mode < ALLY_GAMEPAD_MODE_GAMEPAD ||
> +	    mode > ALLY_GAMEPAD_MODE_KEYBOARD) {
> +		hid_err(hdev, "Invalid gamepad mode: %u\n", mode);
> +		return -EINVAL;
> +	}
> +
> +	u8 *buf __free(kfree) = ally_alloc_cmd(CMD_SET_GAMEPAD_MODE, payload, sizeof(payload));
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = ally_gamepad_send_packet(ally, hdev, buf, ROG_ALLY_REPORT_SIZE);
> +	if (ret < 0) {
> +		hid_err(hdev, "Failed to set gamepad mode: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t gamepad_mode_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 *ally = drvdata->rog_ally;
> +	struct ally_config *cfg;
> +	u8 mode_byte;
> +	int i;
> +
> +	if (!ally)
> +		return -ENODEV;
> +
> +	cfg = ally_get_config(ally);
> +	if (!cfg)
> +		return -ENODEV;
> +
> +	guard(mutex)(&cfg->config_mutex);
> +
> +	mode_byte = cfg->gamepad_mode;
> +
> +	for (i = 0; i < ARRAY_SIZE(ally_gamepad_mode); i++) {
> +		if (ally_gamepad_mode[i] == mode_byte)
> +			return sysfs_emit(buf, "%s\n", ally_gamepad_mode_text[i]);
> +	}
> +
> +	/*
> +	 * The recorded mode only comes from the table above: an unknown
> +	 * byte means memory corruption, not a userspace-visible value.
> +	 */
> +	return -EIO;
> +}
> +
> +static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	struct hid_device *hdev = to_hid_device(dev);
> +	struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
> +	struct ally_handheld *ally = drvdata->rog_ally;
> +	struct ally_config *cfg;
> +	u8 mode_byte;
> +	int mode;
> +	int ret;
> +
> +	if (!ally)
> +		return -ENODEV;
> +
> +	cfg = ally_get_config(ally);
> +	if (!cfg)
> +		return -ENODEV;
> +
> +	mode = sysfs_match_string(ally_gamepad_mode_text, buf);
> +	if (mode < 0) {
> +		hid_err(hdev, "Unknown gamepad mode\n");
> +		return mode;
> +	}
> +
> +	/* Convert the index of the text mode array to the byte
> +	 * that will be accepted by the ally MCU.
> +	 */
> +	mode_byte = ally_gamepad_mode[mode];
> +
> +	guard(mutex)(&cfg->config_mutex);
> +
> +	ret = ally_set_gamepad_mode(ally, hdev, mode_byte);
> +	if (ret < 0)
> +		return ret;
> +
> +	cfg->gamepad_mode = mode_byte;
> +
> +	hid_dbg(hdev, "Set gamepad mode to %s\n", ally_gamepad_mode_text[mode]);
> +
> +	sysfs_notify(&dev->kobj, NULL, "gamepad_mode");
> +
> +	return count;
> +}
> +
> +static ssize_t gamepad_mode_index_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf)
> +{
> +	int i;
> +	ssize_t len = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(ally_gamepad_mode_text); i++) {
> +		if (!ally_gamepad_mode_text[i] || ally_gamepad_mode_text[i][0] == '\0')

[i][0] == '\0' is not true for anything??

> +			continue;
> +		len += sysfs_emit_at(buf, len, "%s ", ally_gamepad_mode_text[i]);
> +	}
> +
> +	/* Replace the last space with a newline */
> +	if (len > 0)
> +		buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR_RW(gamepad_mode);
> +static DEVICE_ATTR_RO(gamepad_mode_index);
> +
>  /**
>   * ally_set_vibration_intensity() - Set vibration intensity values
>   * @ally: ally handheld structure
> @@ -2217,6 +2362,8 @@ static struct device_attribute dev_attr_right_response_curve_index =
>  
>  static struct attribute *ally_config_attrs[] = {
>  	&dev_attr_xbox_controller.attr,
> +	&dev_attr_gamepad_mode.attr,
> +	&dev_attr_gamepad_mode_index.attr,
>  	NULL
>  };
>  
> @@ -2356,7 +2503,7 @@ static struct ally_config *ally_config_create(struct hid_device *hdev, struct al
>  	}
>  
>  	if (!cfg->initialized) {
> -		cfg->gamepad_mode = 0x01;
> +		cfg->gamepad_mode = ALLY_GAMEPAD_MODE_GAMEPAD;
>  		cfg->xbox_controller_enabled = true;
>  		cfg->left_deadzone = 10;
>  		cfg->left_outer_threshold = 90;
> @@ -2926,6 +3073,15 @@ static int hid_asus_ally_init(struct hid_device *hdev, struct ally_handheld *all
>  				 ret);
>  	}
>  
> +	/*
> +	 * An MCU reset switches the gamepad mode back to its default:
> +	 * send the recorded one first, so that every other setting
> +	 * restored below is applied on top of the expected mode.
> +	 */
> +	ret = ally_set_gamepad_mode(ally, hdev, cfg->gamepad_mode);
> +	if (ret < 0)
> +		hid_warn(hdev, "Failed to restore gamepad mode: %d\n", ret);
> +
>  	/*
>  	 * The MCU does not keep the vibration intensity across a reset:
>  	 * send the recorded values again, whichever interface probed.
> 

-- 
 i.