Re: [PATCH v11 11/18] HID: asus: add support for btn remapping

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

[-- Attachment #1: Type: text/plain, Size: 12158 bytes --]

On Fri, 30 Oct 2026, Denis Benato wrote:

> 
> On 10/26/26 15:03, Ilpo Järvinen wrote:
> > On Sun, 25 Oct 2026, Denis Benato wrote:
> >
> >> Add support for buttons remapping: the ability to assign different
> >> button events to each one of the physical button on the device.
> >>
> >> Each button exposes a remap_index sysfs attribute listing the valid
> >> remap target names for the current gamepad mode: M1 and M2 can be
> >> remapped to anything at any time, while other buttons may only be
> >> remapped to gamepad codes in gamepad mode and keyboard/mouse/media
> >> codes in desktop mode.
> >>
> >> When the gamepad mode is changed the firmware automatically restores
> >> its internal defaults, so the software cache is refreshed to reflect
> >> the hardware state.
> >>
> >> Signed-off-by: Luke Jones <luke@ljones.dev>
> >> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> >> Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
> >> ---
> >>  drivers/hid/hid-asus.c | 1059 +++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 1037 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> >> index ed95b2cdec81..ea27495c3df6 100644
> >> --- a/drivers/hid/hid-asus.c
> >> +++ b/drivers/hid/hid-asus.c
> >> @@ -275,9 +275,25 @@ struct ally_turbo_config {
> >>  	struct ally_btn_turbo_params btn_m1;
> >>  };
> >>  
> >> +/* Button remap attribute structure */
> >> +struct button_remap_attr {
> >> +	struct device_attribute dev_attr;
> >> +	enum ally_button_id button_id;
> >> +	bool is_macro;
> >> +	/*
> >> +	 * The attributes are created while hid_asus_ally_probe() is still
> >> +	 * running, before asus_probe() publishes drvdata->rog_ally: carry
> >> +	 * the pointer like the turbo entries do, so the callbacks work
> >> +	 * from the moment the files exist.
> >> +	 */
> >> +	struct ally_handheld *ally;
> >> +};
> >> +
> >> +#define to_button_remap_attr(x) container_of(x, struct button_remap_attr, dev_attr)
> >> +
> >>  struct ally_btn_sysfs_entry {
> >>  	struct attribute_group group;
> >> -	struct attribute *attrs[5]; /* turbo_period + toggle_period + ranges + NULL */
> >> +	struct attribute *attrs[8]; /* turbo + ranges + remap + macro + remap_index + NULL */
> >>  	/*
> >>  	 * The entries are created while hid_asus_ally_probe() is still running,
> >>  	 * before asus_probe() publishes drvdata->rog_ally: carrying the shared
> >> @@ -289,6 +305,9 @@ struct ally_btn_sysfs_entry {
> >>  	enum ally_button_id btn;
> >>  	struct device_attribute attr_turbo_period;
> >>  	struct device_attribute attr_toggle_period;
> >> +	struct button_remap_attr *remap_attr;
> >> +	struct button_remap_attr *macro_attr;
> >> +	struct device_attribute attr_remap_index;
> >>  };
> >>  
> >>  enum ally_gamepad_mode_index {
> >> @@ -296,6 +315,35 @@ enum ally_gamepad_mode_index {
> >>  	ALLY_GAMEPAD_MODE_KEYBOARD = 0x02,
> >>  };
> >>  
> >> +/* Button pair indexes for mapping commands */
> >> +enum btn_pair_index {
> >> +	BTN_PAIR_DPAD_UPDOWN    = 0x01,
> >> +	BTN_PAIR_DPAD_LEFTRIGHT = 0x02,
> >> +	BTN_PAIR_STICK_LR       = 0x03,
> >> +	BTN_PAIR_BUMPER_LR      = 0x04,
> >> +	BTN_PAIR_AB             = 0x05,
> >> +	BTN_PAIR_XY             = 0x06,
> >> +	BTN_PAIR_VIEW_MENU      = 0x07,
> >> +	BTN_PAIR_M1M2           = 0x08,
> >> +	BTN_PAIR_TRIGGER_LR     = 0x09,
> >> +};
> >> +
> >> +struct button_map {
> >> +	const struct btn_code_map *remap;
> >> +	const struct btn_code_map *macro;
> >> +};
> >> +
> >> +struct button_pair_map {
> >> +	enum btn_pair_index pair_index;
> >> +	struct button_map first;
> >> +	struct button_map second;
> >> +};
> >> +
> >> +/* Store button mapping per gamepad mode */
> >> +struct ally_button_mapping {
> >> +	struct button_pair_map button_pairs[9]; /* 9 button pairs */
> >> +};
> >> +
> >>  struct ally_config {
> >>  	/* Must be locked if the data is being changed */
> >>  	struct mutex config_mutex;
> >> @@ -331,6 +379,8 @@ struct ally_config {
> >>  
> >>  	struct ally_turbo_config turbo;
> >>  	struct ally_btn_sysfs_entry *button_entries;
> >> +	/* Button mappings of the current gamepad mode */
> >> +	struct ally_button_mapping button_mappings;
> >>  
> >>  	struct ally_joystick_resp_curve left_curve;
> >>  	struct ally_joystick_resp_curve right_curve;
> >> @@ -1072,6 +1122,295 @@ static int ally_set_gamepad_mode(struct ally_handheld *ally, struct hid_device *
> >>  	return 0;
> >>  }
> >>  
> >> +enum btn_map_type {
> >> +	BTN_TYPE_NONE = 0,
> >> +	BTN_TYPE_PAD = 0x01,
> >> +	BTN_TYPE_KB = 0x02,
> >> +	BTN_TYPE_MOUSE = 0x03,
> >> +	BTN_TYPE_MEDIA = 0x05,
> >> +};
> >> +
> >> +struct btn_code_map {
> >> +	unsigned char type;
> >> +	unsigned char value;
> >> +	const char *name;
> >> +};
> >> +
> >> +static const struct btn_code_map ally_btn_codes[] = {
> >> +	{ BTN_TYPE_NONE, 0x00, "NONE" },
> >> +	/* Gamepad button codes */
> >> +	{ BTN_TYPE_PAD, 0x01, "BTN_SOUTH" },
> >> +	{ BTN_TYPE_PAD, 0x02, "BTN_EAST" },
> >> +	{ BTN_TYPE_PAD, 0x03, "BTN_NORTH" },
> >> +	{ BTN_TYPE_PAD, 0x04, "BTN_WEST" },
> >> +	{ BTN_TYPE_PAD, 0x05, "BTN_TL" },
> >> +	{ BTN_TYPE_PAD, 0x06, "BTN_TR" },
> >> +	{ BTN_TYPE_PAD, 0x07, "BTN_THUMBL" },
> >> +	{ BTN_TYPE_PAD, 0x08, "BTN_THUMBR" },
> >> +	{ BTN_TYPE_PAD, 0x09, "ABS_HAT0Y_NEG" },
> >> +	{ BTN_TYPE_PAD, 0x0A, "ABS_HAT0Y_POS" },
> >> +	{ BTN_TYPE_PAD, 0x0B, "ABS_HAT0X_NEG" },
> >> +	{ BTN_TYPE_PAD, 0x0C, "ABS_HAT0X_POS" },
> >> +	{ BTN_TYPE_PAD, 0x0D, "ABS_Z" },
> >> +	{ BTN_TYPE_PAD, 0x0E, "ABS_RZ" },
> >> +	{ BTN_TYPE_PAD, 0x11, "BTN_SELECT" },
> >> +	{ BTN_TYPE_PAD, 0x12, "BTN_START" },
> >> +	{ BTN_TYPE_PAD, 0x13, "BTN_MODE" },
> >> +
> >> +	/* Keyboard button codes */
> >> +	{ BTN_TYPE_KB, 0x8E, "FN_M2" },
> >> +	{ BTN_TYPE_KB, 0x8F, "FN_M1" },
> >> +	{ BTN_TYPE_KB, 0x76, "KEY_ESC" },
> >> +	{ BTN_TYPE_KB, 0x05, "KEY_F1" },
> >> +	{ BTN_TYPE_KB, 0x06, "KEY_F2" },
> >> +	{ BTN_TYPE_KB, 0x04, "KEY_F3" },
> >> +	{ BTN_TYPE_KB, 0x0C, "KEY_F4" },
> >> +	{ BTN_TYPE_KB, 0x03, "KEY_F5" },
> >> +	{ BTN_TYPE_KB, 0x0B, "KEY_F6" },
> >> +	{ BTN_TYPE_KB, 0x80, "KEY_F7" },
> >> +	{ BTN_TYPE_KB, 0x0A, "KEY_F8" },
> >> +	{ BTN_TYPE_KB, 0x01, "KEY_F9" },
> >> +	{ BTN_TYPE_KB, 0x09, "KEY_F10" },
> >> +	{ BTN_TYPE_KB, 0x78, "KEY_F11" },
> >> +	{ BTN_TYPE_KB, 0x07, "KEY_F12" },
> >> +	{ BTN_TYPE_KB, 0x08, "KEY_F13" },
> >> +	{ BTN_TYPE_KB, 0x10, "KEY_F14" },
> >> +	{ BTN_TYPE_KB, 0x18, "KEY_F15" },
> >> +	{ BTN_TYPE_KB, 0x0E, "KEY_GRAVE" }, // backtick
> >> +	{ BTN_TYPE_KB, 0x16, "KEY_1" },
> >> +	{ BTN_TYPE_KB, 0x1E, "KEY_2" },
> >> +	{ BTN_TYPE_KB, 0x26, "KEY_3" },
> >> +	{ BTN_TYPE_KB, 0x25, "KEY_4" },
> >> +	{ BTN_TYPE_KB, 0x2E, "KEY_5" },
> >> +	{ BTN_TYPE_KB, 0x36, "KEY_6" },
> >> +	{ BTN_TYPE_KB, 0x3D, "KEY_7" },
> >> +	{ BTN_TYPE_KB, 0x3E, "KEY_8" },
> >> +	{ BTN_TYPE_KB, 0x46, "KEY_9" },
> >> +	{ BTN_TYPE_KB, 0x45, "KEY_0" },
> >> +	{ BTN_TYPE_KB, 0x4E, "KEY_MINUS" }, // hyphen
> >> +	{ BTN_TYPE_KB, 0x55, "KEY_EQUAL" },
> >> +	{ BTN_TYPE_KB, 0x66, "KEY_BACKSPACE" },
> >> +	{ BTN_TYPE_KB, 0x0D, "KEY_TAB" },
> >> +	{ BTN_TYPE_KB, 0x15, "KEY_Q" },
> >> +	{ BTN_TYPE_KB, 0x1D, "KEY_W" },
> >> +	{ BTN_TYPE_KB, 0x24, "KEY_E" },
> >> +	{ BTN_TYPE_KB, 0x2D, "KEY_R" },
> >> +	{ BTN_TYPE_KB, 0x2C, "KEY_T" },
> >> +	{ BTN_TYPE_KB, 0x35, "KEY_Y" },
> >> +	{ BTN_TYPE_KB, 0x3C, "KEY_U" },
> >> +	{ BTN_TYPE_KB, 0x43, "KEY_I" },
> >> +	{ BTN_TYPE_KB, 0x44, "KEY_O" },
> >> +	{ BTN_TYPE_KB, 0x4D, "KEY_P" },
> >> +	{ BTN_TYPE_KB, 0x54, "KEY_LEFTBRACE" },
> >> +	{ BTN_TYPE_KB, 0x5B, "KEY_RIGHTBRACE" },
> >> +	{ BTN_TYPE_KB, 0x5D, "KEY_BACKSLASH" },
> >> +	{ BTN_TYPE_KB, 0x58, "KEY_CAPSLOCK" },
> >> +	{ BTN_TYPE_KB, 0x1C, "KEY_A" },
> >> +	{ BTN_TYPE_KB, 0x1B, "KEY_S" },
> >> +	{ BTN_TYPE_KB, 0x23, "KEY_D" },
> >> +	{ BTN_TYPE_KB, 0x2B, "KEY_F" },
> >> +	{ BTN_TYPE_KB, 0x34, "KEY_G" },
> >> +	{ BTN_TYPE_KB, 0x33, "KEY_H" },
> >> +	{ BTN_TYPE_KB, 0x3B, "KEY_J" },
> >> +	{ BTN_TYPE_KB, 0x42, "KEY_K" },
> >> +	{ BTN_TYPE_KB, 0x4B, "KEY_L" },
> >> +	{ BTN_TYPE_KB, 0x4C, "KEY_SEMICOLON" },
> >> +	{ BTN_TYPE_KB, 0x52, "KEY_APOSTROPHE" },
> >> +	{ BTN_TYPE_KB, 0x5A, "KEY_ENTER" },
> >> +	{ BTN_TYPE_KB, 0x88, "KEY_LEFTSHIFT" },
> >> +	{ BTN_TYPE_KB, 0x1A, "KEY_Z" },
> >> +	{ BTN_TYPE_KB, 0x22, "KEY_X" },
> >> +	{ BTN_TYPE_KB, 0x21, "KEY_C" },
> >> +	{ BTN_TYPE_KB, 0x2A, "KEY_V" },
> >> +	{ BTN_TYPE_KB, 0x32, "KEY_B" },
> >> +	{ BTN_TYPE_KB, 0x31, "KEY_N" },
> >> +	{ BTN_TYPE_KB, 0x3A, "KEY_M" },
> >> +	{ BTN_TYPE_KB, 0x41, "KEY_COMMA" },
> >> +	{ BTN_TYPE_KB, 0x49, "KEY_DOT" },
> >> +	{ BTN_TYPE_KB, 0x4A, "KEY_SLASH" },
> >> +	{ BTN_TYPE_KB, 0x89, "KEY_RIGHTSHIFT" },
> >> +	{ BTN_TYPE_KB, 0x82, "KEY_LEFTMETA" },
> >> +	{ BTN_TYPE_KB, 0x8A, "KEY_LEFTALT" },
> >> +	{ BTN_TYPE_KB, 0x29, "KEY_SPACE" },
> >> +	{ BTN_TYPE_KB, 0x8B, "KEY_RIGHTALT" },
> >> +	{ BTN_TYPE_KB, 0x84, "KEY_COMPOSE" },
> >> +	{ BTN_TYPE_KB, 0x8D, "KEY_RIGHTCTRL" },
> >> +	{ BTN_TYPE_KB, 0xC3, "KEY_SYSRQ" },
> >> +	{ BTN_TYPE_KB, 0x7E, "KEY_SCROLLLOCK" },
> >> +	{ BTN_TYPE_KB, 0x91, "KEY_PAUSE" },
> >> +	{ BTN_TYPE_KB, 0xC2, "KEY_INSERT" },
> >> +	{ BTN_TYPE_KB, 0x94, "KEY_HOME" },
> >> +	{ BTN_TYPE_KB, 0x96, "KEY_PAGEUP" },
> >> +	{ BTN_TYPE_KB, 0xC0, "KEY_DELETE" },
> >> +	{ BTN_TYPE_KB, 0x95, "KEY_END" },
> >> +	{ BTN_TYPE_KB, 0x97, "KEY_PAGEDOWN" },
> >> +	{ BTN_TYPE_KB, 0x98, "KEY_UP" },
> >> +	{ BTN_TYPE_KB, 0x99, "KEY_DOWN" },
> >> +	{ BTN_TYPE_KB, 0x9A, "KEY_LEFT" },
> >> +	{ BTN_TYPE_KB, 0x9B, "KEY_RIGHT" },
> >> +
> >> +	/* Numpad button codes */
> >> +	{ BTN_TYPE_KB, 0x77, "KEY_NUMLOCK" },
> >> +	{ BTN_TYPE_KB, 0x90, "KEY_KPSLASH" },
> >> +	{ BTN_TYPE_KB, 0x7C, "KEY_KPASTERISK" },
> >> +	{ BTN_TYPE_KB, 0x7B, "KEY_KPMINUS" },
> >> +	{ BTN_TYPE_KB, 0x70, "KEY_KP0" },
> >> +	{ BTN_TYPE_KB, 0x69, "KEY_KP1" },
> >> +	{ BTN_TYPE_KB, 0x72, "KEY_KP2" },
> >> +	{ BTN_TYPE_KB, 0x7A, "KEY_KP3" },
> >> +	{ BTN_TYPE_KB, 0x6B, "KEY_KP4" },
> >> +	{ BTN_TYPE_KB, 0x73, "KEY_KP5" },
> >> +	{ BTN_TYPE_KB, 0x74, "KEY_KP6" },
> >> +	{ BTN_TYPE_KB, 0x6C, "KEY_KP7" },
> >> +	{ BTN_TYPE_KB, 0x75, "KEY_KP8" },
> >> +	{ BTN_TYPE_KB, 0x7D, "KEY_KP9" },
> >> +	{ BTN_TYPE_KB, 0x79, "KEY_KPPLUS" },
> >> +	{ BTN_TYPE_KB, 0x81, "KEY_KPENTER" },
> >> +	{ BTN_TYPE_KB, 0x71, "KEY_KPDOT" },
> >> +
> >> +	/* Mouse button codes */
> >> +	{ BTN_TYPE_MOUSE, 0x01, "BTN_LEFT" },
> >> +	{ BTN_TYPE_MOUSE, 0x02, "BTN_RIGHT" },
> >> +	{ BTN_TYPE_MOUSE, 0x03, "BTN_MIDDLE" },
> >> +	{ BTN_TYPE_MOUSE, 0x04, "REL_WHEEL_HI_RES_UP" },
> >> +	{ BTN_TYPE_MOUSE, 0x05, "REL_WHEEL_HI_RES_DOWN" },
> >> +
> >> +	/* Media button codes */
> >> +	{ BTN_TYPE_MEDIA, 0x16, "MEDIA_SCREENSHOT" },
> >> +	{ BTN_TYPE_MEDIA, 0x19, "MEDIA_SHOW_KEYBOARD" },
> >> +	{ BTN_TYPE_MEDIA, 0x1C, "MEDIA_SHOW_DESKTOP" },
> >> +	{ BTN_TYPE_MEDIA, 0x1E, "MEDIA_START_RECORDING" },
> >> +	{ BTN_TYPE_MEDIA, 0x01, "MEDIA_MIC_OFF" },
> >> +	{ BTN_TYPE_MEDIA, 0x02, "MEDIA_VOL_DOWN" },
> >> +	{ BTN_TYPE_MEDIA, 0x03, "MEDIA_VOL_UP" },
> >> +};
> >> +
> >> +static const size_t keymap_len = ARRAY_SIZE(ally_btn_codes);
> > Why is this a variable???? Can't the users just directly use ARRAY_SIZE on 
> > the array????
> 
> Not understood: you say variable, but both are const(s), isn't const
> the exact opposite of variable?

My point was that you should use ARRAY_SIZE(ally_btn_codes) directly in 
the 2 place (IIRC) instead of keymap_len. Then you can just drop 
keymap_len entirely.

> I am stepping away for today, but I have cleaned up much of this
> mess.
>
> > At v11 it's odd to find this much garbage still remaining... :-(
> 
> At a certain (late) point of the development I noticed that remapping
> buttons on the X applied immediately, while on my Ally 1 it had
> to go through apply_all old sysfs interface, so I attempted to
> unify them late on development, sorry.

I think my comment was a bit uncalled for anyway, but please see it in 
the light how much lines I had read at that point.

-- 
 i.