HID: asus: wait for MCU ready before detecting Ally capabilities (cold-boot race leaves gamepad endpoint silent)

[jlobue10]

Problem

ally_config_create() fires the seven CMD_CHECK_* capability queries as the first traffic on the configuration interface — before anything has confirmed the MCU is ready. Each query is single-shot with no retry, and a failed/garbled reply is indistinguishable from "unsupported". ally_gamepad_check_ready() (which does retry) only runs afterwards, from hid_asus_ally_init(), violating its own documented contract that it should precede this kind of traffic.

On a cold boot the MCU can still be starting up at probe time. When the race fires, every support flag reads false, so:

• the turbo/button sysfs attributes are never created and capability-gated attributes read -ENODEV;
• worse, since xbox_controller_support is false, CMD_SET_XBOX_CONTROLLER(1) is never sent — the gamepad interrupt endpoint stays silent from boot unless some userspace component (e.g. InputPlumber) happens to enable reporting over hidraw on its own.

Warm reboots probe with the MCU already up and work fine, which makes this intermittent and easy to blame on other components. We spent considerable effort on exactly that before isolating it (originally reported in the #2 test thread).

Field evidence (ROG Xbox Ally X, RC73XA)

• Affected boot: xbox_controller read -ENODEV, capability-gated attributes missing — yet a raw CMD_CHECK_XBOX_SUPPORT (5a d1 0c 01 00) issued moments later over the same hidraw answered supported (5a d1 0c 01 01), a reply the driver's own parser would accept.
• Subsequent warm boot: xbox_controller=1, full attribute tree present, driver sent the enable itself — same kernel, same module.

Fix

Call ally_gamepad_check_ready() at the top of ally_config_create(), before ally_detect_capabilities(). One forward declaration + one call site; the existing retry policy is unchanged. The later ready check in hid_asus_ally_init() is kept — once the MCU is up it's a single fast round-trip, and it still covers the resume path. If the MCU never becomes ready, cfg-interface probe now fails in ally_config_create() instead of moments later in hid_asus_ally_init() — same terminal outcome as today.

Cross-model: CMD_CHECK_READY is common MCU protocol used identically for all Ally models on this interface; the change only reorders traffic on the same endpoint under the same handshake precondition (hid_asus_ally_init() already runs later in this probe path).

Validation

Compile-tested W=1 clean against the 7.2 ally headers. Runtime: the bug's cold-boot intermittency makes on-demand reproduction slow; diagnosis above is from live hardware, and I'm happy to run a cold-boot soak of this branch on the Xbox Ally X if you want field confirmation before merging.

Independent of #6 (different functions; both branches are based on current ally and won't conflict).

🤖 Generated with Claude Code

https://claude.ai/code/session_01M3pWeekLNWsrqafc9erby3

[jlobue10]

Full diagnostic detail (for the record)

Code path on the current ally head (9bc9553), by line:

• hid_asus_ally_probe() CFG-interface branch (hid-asus.c:4949) calls ally_config_create() (4506), whose first action after allocation is ally_detect_capabilities() (4515 → 1962).
• ally_detect_capabilities() issues seven CMD_CHECK_* queries via ally_check_capability() (1936). Each is a single ally_gamepad_send_receive_packet() with no retry, and the function returns false on any failure — transport error, allocation failure, or a reply that doesn't echo the command — so "MCU not ready yet" is indistinguishable from "hardware doesn't support this". ally_detect_capabilities() itself returns 0 regardless, so probe continues normally with all-false flags.
• Only later does probe reach hid_asus_ally_init() (4958 → 4861), which performs ally_gamepad_check_ready() (4869 → 4611) — the sole call site with retry logic, and whose own doc comment (4605) states it should run "before any remapping attempts, and on driver init/resume, after the asus handshake".

Downstream effects of the all-false race, all inside ally_config_create():

• cfg->turbo_support == false → per-button attribute tree never created (4530).
• cfg->xbox_controller_support == false → the ally_set_xbox_controller(hdev, cfg, true) enable at 4572 is skipped → the 0x87 gamepad interrupt endpoint emits nothing from boot.
• All capability-gated sysfs stores reject writes for the life of the binding; re-detection never happens.

Field investigation that isolated this (ROG Xbox Ally X, 0b05:1b4c, out-of-tree builds of this branch's driver):

• Symptom was a maddening alive/dead flip of all gamepad input between test runs. usbmon ground truth in the dead state: interrupt-IN URB armed on the gamepad endpoint, zero completions while buttons were physically pressed — the MCU wasn't reporting, kernel delivery path exonerated.
• In the same dead state, a raw CMD_CHECK_XBOX_SUPPORT (5a d1 0c 01 00 SET_REPORT on the cfg interface, reply via GET_REPORT 0x0D) answered 5a d1 0c 01 01 = supported — a reply the driver's own parser accepts. So the probe-time reads had returned garbage/nothing, not "unsupported".
• The flip correlated perfectly with InputPlumber sessions: IP's own hidraw init enables reporting, masking the driver bug; each test script's cleanup restarting IP re-rolled the state.
• Determinism check: a warm reboot probed with xbox_controller=1 and the full attribute tree — same kernel, same module — confirming this is a cold-boot timing race, not a capability or parsing defect.

Why the fix is shaped this way:

• The ready-wait goes at the top of ally_config_create() because that's the first MCU traffic; the contract in ally_gamepad_check_ready()'s comment is then satisfied for everything that follows.
• The handshake precondition holds at this point: hid_asus_ally_init() already runs later in the same probe invocation with the comment "assumes the asus-specific initialization to have been performed already".
• The second ready check in hid_asus_ally_init() is intentionally kept — once the MCU is up it's one fast round-trip, and it still covers the resume path (5044).
• Never-ready behavior is unchanged: today probe of the cfg interface fails at hid_asus_ally_init(); with this patch it fails a few lines earlier in ally_config_create(). Same terminal outcome, but no more half-initialized all-false binding in between.
• Retry policy (HID_ALLY_READY_MAX_TRIES) is deliberately untouched; if field data ever shows the window is too short for a cold MCU, bumping that constant is a separate knob.

Cross-model: CMD_CHECK_READY is common MCU protocol issued identically for every Ally generation on this interface — this only reorders traffic on the same endpoint under the same precondition. The original Ally (RC71L) runs the identical CFG-probe path and benefits the same way.

Compatibility with #6: verified beyond inspection — this patch applies cleanly on top of the #6 branch, and the combined tree (ally head + #6 + this) compiles W=1 with zero warnings against 7.2-based headers. The two touch different functions and can merge in either order.

Validation status: compile-tested only, by design — the race is cold-boot intermittent, so on-demand repro is slow. The diagnosis above is from live hardware. Offer stands to run a cold-boot soak of this branch (repeated cold boots checking xbox_controller reads 1 and the attribute tree is complete) whenever useful.