From: "Daniel J. Ogorchock" djogorchock@gmail.com
[ Upstream commit 4a0381080397e77792a5168069f174d3e56175ff ]
Ensure we don't stall or panic the kernel when using bluetooth-connected controllers. This was reported as an issue on android devices using kernel 6.6 due to the resume hook which had been added for usb joycons.
First, set a new state value to JOYCON_CTLR_STATE_SUSPENDED in a newly-added nintendo_hid_suspend. This makes sure we will not stall out the kernel waiting for input reports during led classdev suspend. The stalls could happen if connectivity is unreliable or lost to the controller prior to suspend.
Second, since we lose connectivity during suspend, do not try joycon_init() for bluetooth controllers in the nintendo_hid_resume path.
Tested via multiple suspend/resume flows when using the controller both in USB and bluetooth modes.
Signed-off-by: Daniel J. Ogorchock djogorchock@gmail.com Reviewed-by: Silvan Jegen s.jegen@gmail.com Signed-off-by: Jiri Kosina jkosina@suse.com Signed-off-by: Sasha Levin sashal@kernel.org ---
**YES**
This commit should be backported to stable kernel trees. Here's the extensive analysis:
## Bug Severity and Impact
The commit fixes a serious bug that causes **kernel stalls or panics during suspend/resume** when using Bluetooth-connected Nintendo controllers. This was specifically reported on Android devices running kernel 6.6, indicating real-world user impact. Kernel stalls/panics during suspend/resume are critical issues that can lead to data loss and poor user experience.
## Root Cause Analysis
Looking at the code changes and the `joycon_enforce_subcmd_rate()` function at lines 825-854:
```c do { joycon_wait_for_input_report(ctlr); // ... other code ... } while ((consecutive_valid_deltas < JC_SUBCMD_VALID_DELTA_REQ || subcmd_delta < JC_SUBCMD_RATE_LIMITER_MS(ctlr)) && ctlr->ctlr_state == JOYCON_CTLR_STATE_READ && attempts < JC_SUBCMD_RATE_MAX_ATTEMPTS); ```
This function contains a blocking loop that waits for input reports. If a Bluetooth controller loses connectivity before suspend, this loop could continue indefinitely (up to max attempts), causing the suspend process to stall.
## Fix Analysis
The fix is elegant and minimal:
1. **New SUSPENDED state**: Adds `JOYCON_CTLR_STATE_SUSPENDED` to prevent blocking operations during suspend 2. **Suspend handler**: Sets the controller state to SUSPENDED, which breaks the blocking loop in `joycon_enforce_subcmd_rate()` since it checks for `ctlr_state == JOYCON_CTLR_STATE_READ` 3. **Resume handler modification**: - For Bluetooth controllers: Simply resets state to READ without reinitializing (since BT connection is lost anyway) - For USB controllers: Maintains existing behavior with `joycon_init()`
## Backport Suitability
1. **Small and contained**: The changes are minimal - adding one enum value, one suspend function, and modifying the resume logic 2. **No architectural changes**: Pure bug fix with no feature additions 3. **Low regression risk**: Changes only affect suspend/resume paths for this specific driver 4. **Similar to accepted backports**: - Similar Commit #2 (backported) fixed USB controller resume issues - Similar Commit #4 (backported) fixed a null pointer dereference
## Stable Tree Compliance
The fix meets all stable kernel criteria: - Fixes a real bug affecting users (kernel stalls/panics) - Minimal change scope - No new features or APIs - Confined to a single driver subsystem - Clear problem and solution
The fact that this was reported on production Android devices using kernel 6.6 further emphasizes the need for backporting to ensure stable kernel users don't experience these suspend/resume failures.
drivers/hid/hid-nintendo.c | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c index 839d5bcd72b1e..fb4985988615b 100644 --- a/drivers/hid/hid-nintendo.c +++ b/drivers/hid/hid-nintendo.c @@ -308,6 +308,7 @@ enum joycon_ctlr_state { JOYCON_CTLR_STATE_INIT, JOYCON_CTLR_STATE_READ, JOYCON_CTLR_STATE_REMOVED, + JOYCON_CTLR_STATE_SUSPENDED, };
/* Controller type received as part of device info */ @@ -2750,14 +2751,46 @@ static void nintendo_hid_remove(struct hid_device *hdev)
static int nintendo_hid_resume(struct hid_device *hdev) { - int ret = joycon_init(hdev); + struct joycon_ctlr *ctlr = hid_get_drvdata(hdev); + int ret; + + hid_dbg(hdev, "resume\n"); + if (!joycon_using_usb(ctlr)) { + hid_dbg(hdev, "no-op resume for bt ctlr\n"); + ctlr->ctlr_state = JOYCON_CTLR_STATE_READ; + return 0; + }
+ ret = joycon_init(hdev); if (ret) - hid_err(hdev, "Failed to restore controller after resume"); + hid_err(hdev, + "Failed to restore controller after resume: %d\n", + ret); + else + ctlr->ctlr_state = JOYCON_CTLR_STATE_READ;
return ret; }
+static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message) +{ + struct joycon_ctlr *ctlr = hid_get_drvdata(hdev); + + hid_dbg(hdev, "suspend: %d\n", message.event); + /* + * Avoid any blocking loops in suspend/resume transitions. + * + * joycon_enforce_subcmd_rate() can result in repeated retries if for + * whatever reason the controller stops providing input reports. + * + * This has been observed with bluetooth controllers which lose + * connectivity prior to suspend (but not long enough to result in + * complete disconnection). + */ + ctlr->ctlr_state = JOYCON_CTLR_STATE_SUSPENDED; + return 0; +} + #endif
static const struct hid_device_id nintendo_hid_devices[] = { @@ -2796,6 +2829,7 @@ static struct hid_driver nintendo_hid_driver = {
#ifdef CONFIG_PM .resume = nintendo_hid_resume, + .suspend = nintendo_hid_suspend, #endif }; static int __init nintendo_init(void)