Backport patches for Raspberry Pi dwc2 driver#4231
Conversation
Raspberry Pi Linux update to 6.12.34 broken some USB devices, mostly USB-Serial converters connected to Yellow, but there are reports of some other peripherals connected to RPi boards too. This is a known RPi upstream issue [1] fixed by a PR [2] that's not been merged to RPi stable kernel yet. Applying patches from this PR fixes the issues. Fixes #4228, refs #4229 [1] raspberrypi/linux#6941 [2] raspberrypi/linux#6936
sairon
added
board/raspberrypi
📝 WalkthroughWalkthroughPatches modify DWC2 USB host controller behavior: masquerade split-interrupt endpoints as control during channel init and restore in interrupt handler; and enforce 8-byte max packet size for control setup stage in split-IN transfers. Changes are confined to drivers/usb/dwc2/hcd.c and drivers/usb/dwc2/hcd_intr.c. Changes
Sequence Diagram(s)sequenceDiagram
participant USB_Device
participant USB_Hub
participant DWC2_HCD as DWC2 HCD
participant Host
Host->>DWC2_HCD: Initialize channel (INT endpoint, do_split)
DWC2_HCD->>DWC2_HCD: Masquerade ep_type = CONTROL
DWC2_HCD->>USB_Device: Schedule via non-periodic path
USB_Device-->>DWC2_HCD: Transfer completes/interrupt
DWC2_HCD->>DWC2_HCD: In interrupt, restore original ep_type
Note over DWC2_HCD: For Control Setup (split-IN), set xfer_len=8, max_packet=8
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
buildroot-external/board/raspberrypi/patches/linux/0005-Fixup-usb-dwc2-limit-maximum-packet-size-for-split-I.patch (1)
10-13: Nit: fix minor grammar in commit messageSmall improvement for clarity:
- “Control Setup phase transfers needs” -> “Control Setup phase transfers need”
- Consider hyphenating “8-byte” and “max-packet” for readability.
Proposed edit to the patch header text:
-Control Setup phase transfers needs an 8 byte maxpacket override, or +Control Setup phase transfers need an 8-byte max-packet override, or drivers doing a GET with a small buffer as a split transaction would cause broken transfers.buildroot-external/board/raspberrypi/patches/linux/0004-usb-dwc2-masquerade-split-interrupt-transfers.patch (1)
37-47: Optional: avoid mutating chan->ep_type by deriving HCCHAR EPTYPE locallyAs an alternative design (to reduce state churn and the need to un-masquerade), you could compute the eptype used for HCCHAR without mutating chan->ep_type. This keeps channel state consistent and still routes via non-periodic.
Illustrative change inside the patch (for hcd.c):
-+ if (chan->do_split && chan->ep_type == USB_ENDPOINT_XFER_INT) -+ chan->ep_type = USB_ENDPOINT_XFER_CONTROL; -+ - hcchar |= chan->ep_type << HCCHAR_EPTYPE_SHIFT & HCCHAR_EPTYPE_MASK; + { + u32 eptype = chan->ep_type; + if (chan->do_split && eptype == USB_ENDPOINT_XFER_INT) + eptype = USB_ENDPOINT_XFER_CONTROL; + hcchar |= eptype << HCCHAR_EPTYPE_SHIFT & HCCHAR_EPTYPE_MASK; + }Not required for correctness (your current approach is fine), but it’s a cleaner pattern and reduces the chance of future regressions if new code starts relying on chan->ep_type in this path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
buildroot-external/board/raspberrypi/patches/linux/0004-usb-dwc2-masquerade-split-interrupt-transfers.patch(1 hunks)buildroot-external/board/raspberrypi/patches/linux/0005-Fixup-usb-dwc2-limit-maximum-packet-size-for-split-I.patch(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare build
🔇 Additional comments (3)
buildroot-external/board/raspberrypi/patches/linux/0005-Fixup-usb-dwc2-limit-maximum-packet-size-for-split-I.patch (2)
26-31: SETUP stage MPS override is correct and aligns with USB specOverriding chan->max_packet to 8 for CONTROL_SETUP is the right fix. SETUP is always 8 bytes regardless of ep0’s wMaxPacketSize and this prevents short-buffer split transactions from breaking.
26-31: Confirm max_packet is reset for subsequent CONTROL_DATA/STATUS stagesBecause you temporarily set chan->max_packet = 8 in CONTROL_SETUP, please confirm that dwc2_hc_init_xfer sets chan->max_packet appropriately again for DWC2_CONTROL_DATA and DWC2_CONTROL_STATUS, so the 8-byte override doesn’t bleed into later stages.
buildroot-external/board/raspberrypi/patches/linux/0004-usb-dwc2-masquerade-split-interrupt-transfers.patch (1)
37-47: Masquerading INT-split as CONTROL to route via non-periodic looks goodThe approach is consistent with the rationale: it nudges scheduling to the non-periodic path while leaving tokens intact. The early un-masquerade in the interrupt path (see Lines 59-61) prevents type confusion downstream.
| + /* Un-masquerade the transfer type */ | ||
| + if (chan->do_split) | ||
| + chan->ep_type = chan->qh->ep_type; |
There was a problem hiding this comment.
Add null-check for chan->qh before un-masquerading
dwc2_hc_n_intr() runs on channel interrupts; teardown/dequeue paths can momentarily leave chan->qh unset. Guarding avoids a potential NULL dereference.
Apply this diff within the patch:
-+ /* Un-masquerade the transfer type */
-+ if (chan->do_split)
-+ chan->ep_type = chan->qh->ep_type;
+ /* Un-masquerade the transfer type */
+ if (chan->do_split && chan->qh)
+ chan->ep_type = chan->qh->ep_type;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + /* Un-masquerade the transfer type */ | |
| + if (chan->do_split) | |
| + chan->ep_type = chan->qh->ep_type; | |
| /* Un-masquerade the transfer type */ | |
| if (chan->do_split && chan->qh) | |
| chan->ep_type = chan->qh->ep_type; |
🤖 Prompt for AI Agents
In
buildroot-external/board/raspberrypi/patches/linux/0004-usb-dwc2-masquerade-split-interrupt-transfers.patch
around lines 59 to 61, the code un-masquerades chan->ep_type by reading chan->qh
without checking for NULL; modify the condition to guard the access by checking
chan->qh (e.g., only assign chan->ep_type when chan->do_split && chan->qh) so
the interrupt path cannot dereference a NULL chan->qh.
2 participants
Raspberry Pi Linux update to 6.12.34 broke some USB devices, mostly USB-Serial converters connected to Yellow, but there are reports of some other peripherals connected to RPi boards too.
This is a known RPi upstream issue [1] fixed by a PR [2] that's not been merged to RPi stable kernel yet. Applying patches from this PR fixes the issues.
Fixes #4228, refs #4229
[1] raspberrypi/linux#6941
[2] raspberrypi/linux#6936
Summary by CodeRabbit