HID: asus: fix CMD_SET_MAPPING packet layout + Xbox Ally X long-press codes (full test report inside)

[jlobue10]

Full test report for the ally branch (b9a5f6e) on a ROG Xbox Ally X (RC73YA), plus fixes for the two bugs found. Everything below was verified live on hardware, on a 7.2-rc4 base with the series applied (the patched hid-asus is the sole claimant of 0B05:1B4C on that tree); fixes were validated by rebuilding the module out-of-tree and re-testing.

Requested checks — all pass (with the fixes in this PR)

Check	Result
find /sys -name 'gamepad*'	attrs present on iface .0003
gamepad_mode desktop/gamepad round-trip	OK, readback + index correct
turbo timing write/readback (btn_a/turbo_period 4, 10)	OK; out-of-range 21 rejected
remap write/readback (btn_a/remap → PAD_B, restore)	OK; bogus name rejected
evdev coverage, all buttons press+release	OK (see front-button notes)
FF rumble on "ASUS ROG Ally X Gamepad" node	OK (felt on device)
suspend/resume, then all of the above again	OK

Commit 1 — CMD_SET_MAPPING packet malformed; writing any remap/macro attr kills BOTH buttons of the pair

Writing e.g. btn_a/remap sent a packet the MCU parses as garbage: it unmaps A and B (the packet always carries the full pair), the buttons stop emitting events entirely, and the state persists in MCU NVRAM until a correct mapping packet is sent. Two independent errors in ally_set_button_mapping(), either one alone is fatal:

1. Block misalignment. The mapping block is four consecutive 11-byte entries starting at buf[5]: first remap 5–15, first macro 16–26, second remap 27–37, second macro 38–48 (cf. __btn_pair_to_pkt / BTN_CODE_LEN in hid-asus-ally). The code copied the remap blocks as 10 bytes and wrote the macro type bytes at buf[15]/buf[37] — one byte early.
2. Wrong value-byte position for pad codes. The value position is per-type: pad = byte 1, kb = 2, media = 3, mouse = 4. The code lumped pad/kb/media together at byte 2, so a pad mapping went out as {0x01, 0x00, code, ...} and the MCU read value 0x00 = unmapped.

Empirical proof: only the AB pair was ever written during testing, and only A/B died while every untouched pair kept working. Fixing only error 1 did not restore them; fixing both and re-sending the pair restored A/B immediately (clean BTN_SOUTH/BTN_EAST press+release in raw evdev capture).

Commit 2 — Xbox Ally X front-button long-presses (verified codes)

Button	Short press	Long press (hardware code)
Left	0xA6 → KEY_F16	keyboard usage 0x70 (F21)
Right	0x93 → KEY_PROG1	keyboard usage 0x71 (F22)

The original Ally's 0xA7→F17 and 0x38→F19 paths never fire on this hardware — long-presses arrive as plain keyboard-page usages through the generic HID path. The commit remaps 0x70→KEY_F17 and 0x71→KEY_F19 (scoped to QUIRK_ROG_ALLY_XPAD) so userspace sees the same codes across Ally generations. MSC_SCAN still carries the raw usage. Behavioral note: these are real held keys (press/autorepeat/release), unlike the synthetic instant press+release from handle_ally_event().

Not fixed here, for your consideration

• Trigger range min/max never initialized at probe: ally_config.left/right_trigger_min/max are only written by the sysfs stores, so readback of range_upper_limit shows 0 (outside its own advertised 70 100 range) — and worse, the first write to any ONE trigger-range attr calls ally_set_trigger_ranges() with the other three cached zeros, pushing a zero-width trigger range to the hardware until all four attrs are written once. Suggested: init mins=0/maxes=100 at probe (or query the device).
• 0x38 is mapped to KEY_PROG1 in asus_input_mapping() ("ROG key") but to KEY_F19 in handle_ally_event() — two contradictory handlers for the same code.
• RGB LED work is enqueued on a deprecated workqueue (7.2 API warning at probe).

Test environment

7.2-rc4 (CachyOS base; mt76/mac80211 untouched by its patches) + this series. Unrelated: 7.2-rc4 has an mt76/MT7922 regression on this device (WiFi MCU dead at probe → rtnl-held D-state cascade); all testing was done with mt7921e blacklisted — being reported to linux-wireless separately, mentioned so nobody chases phantom hid-asus hangs.

🤖 Generated with Claude Code

[jlobue10]

Xbox Ally X (RC73YA) — round 2 test results + new findings

Follow-up to the original test report in this PR. All testing on real hardware
(ASUS Xbox Ally X, 7.2.0-rc4 base + this series), latest round on a combined
tree: series head b9a5f6e0a7 + this PR's fixes + PR #1 rebased + the two
applicable fixes from PR #3 — everything below applies to the series itself
unless noted.

Validation summary (all PASS)

Test	Result
Guided button matrix (ABXY, bumpers, sticks+clicks, dpad, triggers, View/Menu/Xbox, M1/M2, front short+long)	25/25
Stick ranges ±32768, triggers 0–1023 full travel	OK
Suspend/resume ×3 (s2idle)	25/25 checks
Suspend/resume ×2 with mcu_powersave=1 (forced reset_resume / USB re-enum)	17/17 checks
Post-resume: re-bind of all 6 ifaces, input nodes, config persistence (turbo_period marker), button flow, rumble	OK every cycle
RGB (see notes): live color changes, 50 unbind cycles, suspend survival	OK, zero oops
Button remap pipeline (btn_m1/btn_m2 → KB_F14/KB_F15 and back)	OK — mapping-packet fix from this PR exercised again

New driver findings

1. ally_detect_capabilities() runs before the MCU ready-check (cold-boot race)

hid_asus_ally_probe() calls ally_config_create() → ally_detect_capabilities()
before hid_asus_ally_init() → ally_gamepad_check_ready(). On cold boots the
MCU isn't ready yet, all 7 capability queries return garbage, every support flag
ends up false, and:

• xbox_controller (and user_cal/turbo/response-curve etc.) sysfs reads -ENODEV;
• the xbox_controller_support gate never sends CMD_SET_XBOX_CONTROLLER(1),
  so the gamepad interrupt endpoint stays silent unless userspace (InputPlumber)
  happens to enable it.

Evidence it's a race, not a hard failure: raw CHECK_XBOX_SUPPORT
(5a d1 0c 01 00) answers ... 01 = supported while the flags say otherwise,
and on a warm reboot the same probe path succeeds (xbox_controller reads 1).
Suggested fix: run capability detection after ally_gamepad_check_ready()
(or retry on failure), and have the driver own the reporting enable at
probe/resume instead of inheriting whatever userspace last left in the MCU.

2. Gamepad interrupt stream halts without periodic config-channel traffic

With InputPlumber stopped, the MCU stops emitting on the gamepad interrupt-IN
endpoint ~40–60 s after the last config-channel activity — even under
continuous evdev/interrupt polling (usbmon shows the armed URB and zero
completions; kernel side fully exonerated). Once halted it is not revived by
mode re-assert, module reload, or USB reset.

Empirical fix, now validated end-to-end: any harmless SET_REPORT on the config
interface every 10 s (we used the CMD_SET_XBOX_CONTROLLER(1) enable) keeps the
stream alive indefinitely — through a full ~5 min button matrix and across all
suspend cycles (it also doubles as the post-resume re-enable, ≤10 s after wake).
The driver probably wants to own such a keepalive (or find the real sustain
mechanism ASUS firmware expects) — otherwise the pad goes dead for any user
whose userspace stack doesn't happen to poke the config channel periodically.

3. Back paddles: MCU sends byte-identical reports at default mapping

At the default KB_M1/KB_M2 mapping, both paddles produce the identical
raw report on the config interface — 5a a5 00 00 00 00 press /
5a 00 ... release — i.e. vendor usage 0xA5 → KEY_F15 for either paddle.
Verified by tapping all six hidraw nodes: no distinguishing byte anywhere, so
the paddle identity exists only inside the MCU (Armoury macro semantics) and
the driver cannot emit distinct events at default mapping. Not a driver bug,
but two follow-ups:

• the 0xa5 ... /* ROG Ally left back */ comment in asus_input_mapping is
  misleading — it's either rear paddle, at least on Xbox Ally X;
• worth documenting that distinct paddle events require an MCU remap (the
  sysfs remap works perfectly: btn_m1→KB_F14 gives F14 on the right paddle,
  btn_m2→KB_F15 gives F15 on the left, immediate and restorable).
  Userspace projects hit this wall too (HandheldCompanion #1254 "M1/M2 cloned
  on AllyX").

Pair-slot assignment (M1=first, M2=second) is confirmed correct on this
hardware.

4. multi_intensity clamped by max_brightness (hue distortion)

leds-multicolor clamps multi_intensity writes to max_brightness (100), so
255 128 0 reads back 100 100 0 and the hardware never sees >100 per
channel. Intensity should be 0–255 with brightness a separate scale factor.

5. Trigger range init (re-confirmed still present)

left/right_trigger_min/max still never initialized at probe — readback 0,
and the first write to any one of the four attrs pushes cached zeros for the
other three. (Reported previously in this PR; unchanged.)

Interop / ecosystem notes

• LED name: this series (and PR Draft: HID: asus: Ally X SteamOS LED multicolor + FF/haptics support #1) registers asus:rgb:joystick_rings,
  but InputPlumber's 50-rog_xbox_ally.yaml matches ally:rgb:joystick_rings
  (hid-asus-ally naming) — the LED group never binds, so Steam/IP RGB control
  silently does nothing. Whichever name is intended, it needs coordination.
• Master brightness gotcha: the MCU scales all RGB zones by
  asus::kbd_backlight (0–3); power management daemons (powerdevil) zero it
  on battery, making RGB look dead while every command path works. Cost us a
  full invalid test round — worth a note in the docs.
• Minor, from earlier rounds, still applicable: RGB work "enqueued on
  deprecated workqueue" (7.2 API), and 0x38 is handled in both
  asus_input_mapping and handle_ally_event with different keycodes
  (PROG1 vs F19).

Compatibility

The combined tree (this PR + PR #1 rebased onto the current framework + the
two applicable PR #3 fixes) compiles clean and passed everything above on
hardware, for whatever that's worth when deciding how to sequence the three
open PRs.

Logs for everything referenced are archived; happy to attach any specific one.

[jlobue10]

Follow-up to my earlier test report: I extended the suspend/resume matrix with dedicated coverage for the two front buttons (short and long press), since those have historically been the problematic inputs on this hardware, and re-ran it on the Xbox Ally X.

Result: front buttons fully survive suspend/resume — every check passed on every cycle.

Test setup

• Same combined tree as my previous comment (ally base + this PR's fixes, including the xbox-ally-x-longpress-remap change), kernel 7.2.0-rc4, module srcversion 64E5F3ED7D4381195661674.
• Per cycle (rtcwake -m mem -s 20), after the existing rebind/input-node/config-marker/kmsg/A-press/rumble checks, the matrix now verifies each front button: short press must emit the F16/PROG1 pair and long press (~2s hold) the F17/F19 pair — either physical order, but the two buttons must emit distinct codes, with F21/F22 flagged as the remap commit missing. A pre-suspend baseline of the same four checks runs first so any post-resume failure is attributable to the suspend cycle.
• InputPlumber stopped for the whole run; the 10s config-channel keepalive (from the stream-halt workaround discussed previously) was active, doubling as a post-resume re-enable.

Results

Run	Cycles	Front-button checks	Outcome
Plain s2idle	3	baseline + 3× post-resume (16 checks)	41/41 PASS (all checks incl. rebind, marker, A, rumble)
mcu_powersave=1 (reset_resume / USB re-enumeration path)	2	baseline + 2× post-resume (12 checks)	29/29 PASS

Across both baselines and all 5 resume cycles the mapping was completely stable:

• Front button 1: short → KEY_F16, long → KEY_F17
• Front button 2: short → KEY_PROG1, long → KEY_F19

No legacy F21/F22 events (long-press remap working after resume), no lost or swapped codes after any cycle, all 6 HID interfaces re-bound to asus every time, config marker (btn_a/turbo_period) survived, kmsg clean (no oops/UAF/driver errors), and post-resume rumble worked on every cycle — including the forced reset_resume path.

Logs: suspend-matrix-20260724-195042.log (s2idle ×3) and suspend-matrix-20260724-195349.log (mcu_powersave ×2); happy to attach if useful.