Linux Input/HID development
 help / color / mirror / Atom feed
From: Mike Lothian <mike@fireburn.co.uk>
To: "Daniel J . Ogorchock" <djogorchock@gmail.com>
Cc: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <bentiss@kernel.org>,
	Silvan Jegen <s.jegen@gmail.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Mike Lothian <mike@fireburn.co.uk>
Subject: [PATCH v2] HID: nintendo: relax subcommand limiter until cadence is proven
Date: Wed,  9 Sep 2026 00:43:15 +0100	[thread overview]
Message-ID: <20260908234315.117903-1-mike@fireburn.co.uk> (raw)
In-Reply-To: <20260908224532.114357-1-mike@fireburn.co.uk>

joycon_config_rumble() queues a zero-rumble packet during probe. The
rumble worker sends it once ctlr_state becomes READ, and
joycon_handle_rumble_report() retries it a few more times. This is the
first subcommand sent on every connection and it goes out before any
input report has been seen, so consecutive_valid_report_deltas is still
0. joycon_enforce_subcmd_rate_strict() needs 3 consecutive reports in
the 8-17ms window before it releases a subcommand, so the send exhausts
all 25 attempts and warns on every connection.

Add a subcmd_rate_unproven flag, set until the cadence has been
observed, and use the legacy flat-delay throttle while it is set.
joycon_parse_report() clears it after JC_SUBCMD_VALID_DELTA_REQ
consecutive reports at a valid cadence. USB is excluded, as its
consecutive_valid_report_deltas is forced to the requirement anyway.

The flag is separate from subcmd_rate_relaxed so that the exhaustion
fallback added by commit 781f8e020a78 ("HID: nintendo: fix rumble
starved by the input report cadence gate") keeps its semantics.
subcmd_rate_relaxed is written from the subcommand worker under
output_mutex, subcmd_rate_unproven only from joycon_parse_report().

Found with btmon over Bluetooth on a MediaTek mt7921e: the three
JC_SUBCMD_RATE_MAX_ATTEMPTS warnings on connect match three
JC_OUTPUT_RUMBLE_ONLY (0x10) frames from the rumble worker's
zero-countdown retries. Tested on a Pro Controller, which now connects
reliably, where before it dropped within the first ~70 seconds of a
fresh connection about half the time.

Fixes: d750d1480362 ("HID: nintendo: fix rumble rate limiter")
Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
Assisted-by: Claude:Opus-5 [Claude Code]
---
v2:
- Use a separate subcmd_rate_unproven flag rather than reusing
  subcmd_rate_relaxed. In v1 joycon_parse_report() cleared
  subcmd_rate_relaxed from softirq under ctlr->lock, while
  joycon_enforce_subcmd_rate_strict() sets it from the subcommand
  worker under output_mutex. Clearing it after the exhaustion counter
  had passed JC_SUBCMD_RATE_MAX_FAILURES would stop the
  ++subcmd_rate_exhaustions == JC_SUBCMD_RATE_MAX_FAILURES test from
  ever matching again, permanently disabling the fallback added by
  781f8e020a78.
- Start USB controllers proven. consecutive_valid_report_deltas is
  forced to JC_SUBCMD_VALID_DELTA_REQ for USB at the end of
  joycon_parse_report(), so the v1 promotion could never run and USB
  would have been left on the legacy throttle.
- v1 was assisted by Claude Sonnet 5, v2 by Claude Opus 5.

Both issues in v1 were spotted by the Sashiko AI review:
https://lore.kernel.org/linux-input/20260908225815.952CE1F00A3A@smtp.kernel.org/

That review also flagged a pre-existing issue: that the probe error
path does not set ctlr_state to JOYCON_CTLR_STATE_REMOVED, so a queued
rumble worker could run against stopped hardware during
destroy_workqueue(). As far as I can tell that is not reachable. The
worker can only be queued from joycon_handle_rumble_report(), which
runs via joycon_ctlr_read_handler() and is gated on ctlr_state ==
JOYCON_CTLR_STATE_READ, and from joycon_play_effect(), which needs the
input device registered. input_register_device() is the last call in
joycon_input_create() that can fail, and ctlr_state is set to READ
immediately after it returns, so there is no window in which the worker
is queued and probe can still fail.

v1: https://lore.kernel.org/linux-input/20260908224532.114357-1-mike@fireburn.co.uk/

 drivers/hid/hid-nintendo.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 43e0f2aaea3b..5b3d97c0ad39 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -611,6 +611,7 @@ struct joycon_ctlr {
 	unsigned int consecutive_valid_report_deltas;
 	unsigned int subcmd_rate_exhaustions;
 	bool subcmd_rate_relaxed;
+	bool subcmd_rate_unproven;
 
 	/* factory calibration data */
 	struct joycon_stick_cal left_stick_cal_x;
@@ -917,7 +918,7 @@ static void joycon_enforce_subcmd_rate_legacy(struct joycon_ctlr *ctlr)
 
 static void joycon_enforce_subcmd_rate(struct joycon_ctlr *ctlr)
 {
-	if (ctlr->subcmd_rate_relaxed)
+	if (ctlr->subcmd_rate_relaxed || READ_ONCE(ctlr->subcmd_rate_unproven))
 		joycon_enforce_subcmd_rate_legacy(ctlr);
 	else
 		joycon_enforce_subcmd_rate_strict(ctlr);
@@ -1797,8 +1798,11 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
 	 */
 	if (report_delta_ms >= JC_INPUT_REPORT_MIN_DELTA &&
 	    report_delta_ms <= JC_INPUT_REPORT_MAX_DELTA) {
-		if (ctlr->consecutive_valid_report_deltas < JC_SUBCMD_VALID_DELTA_REQ)
+		if (ctlr->consecutive_valid_report_deltas < JC_SUBCMD_VALID_DELTA_REQ) {
 			ctlr->consecutive_valid_report_deltas++;
+			if (ctlr->consecutive_valid_report_deltas == JC_SUBCMD_VALID_DELTA_REQ)
+				WRITE_ONCE(ctlr->subcmd_rate_unproven, false);
+		}
 	} else {
 		ctlr->consecutive_valid_report_deltas = 0;
 	}
@@ -2730,6 +2734,7 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 
 	ctlr->hdev = hdev;
 	ctlr->ctlr_state = JOYCON_CTLR_STATE_INIT;
+	ctlr->subcmd_rate_unproven = hdev->bus != BUS_USB;
 	ctlr->rumble_queue_head = 0;
 	ctlr->rumble_queue_tail = 0;
 	hid_set_drvdata(hdev, ctlr);
-- 
2.55.0


      parent reply	other threads:[~2026-09-08 23:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 22:45 [PATCH] HID: nintendo: relax subcommand limiter until cadence is proven Mike Lothian
2026-09-08 22:58 ` sashiko-bot
2026-09-08 23:43 ` Mike Lothian [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260908234315.117903-1-mike@fireburn.co.uk \
    --to=mike@fireburn.co.uk \
    --cc=bentiss@kernel.org \
    --cc=djogorchock@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=s.jegen@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox