fix:(port/ch32/usb_dc_usbhs.c) Add isochronous out transfer support and fix endpoint config bug#310
Conversation
Add Synchronous Out transmission of devices.
WalkthroughThe changes refine endpoint management in the USB device controller for the CH32 platform. Endpoint enabling, type configuration, and stall status handling are now performed on a per-endpoint, per-direction basis, rather than globally or generically. Isochronous endpoint handling during read operations is also made more precise. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant usb_dc_init
participant usbd_ep_open
participant usbd_ep_close
participant usbd_ep_is_stalled
participant usbd_ep_start_read
participant USBHS_DEVICE
App->>usb_dc_init: Initialize USB device
usb_dc_init->>USBHS_DEVICE: Do not enable all endpoints by default
App->>usbd_ep_open: Open endpoint (IN/OUT)
usbd_ep_open->>USBHS_DEVICE: Set endpoint enable/type bits per endpoint/direction
App->>usbd_ep_close: Close endpoint (IN/OUT)
usbd_ep_close->>USBHS_DEVICE: Clear endpoint enable bit for specified endpoint/direction
App->>usbd_ep_is_stalled: Check if endpoint is stalled
usbd_ep_is_stalled->>USBHS_DEVICE: Read stall status from RX/TX control register
App->>usbd_ep_start_read: Start read on endpoint
usbd_ep_start_read->>USBHS_DEVICE: Set RX control for isochronous or other endpoints
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 2
🧹 Nitpick comments (1)
port/ch32/usb_dc_usbhs.c (1)
138-143: Type bit is left uncleared onep_close
usbd_ep_close()disables the endpoint inENDP_CONFIG, but the correspondingENDP_TYPEbit remains set.
If the application later re-opens the same index with a different transfer type, the leftover bit will override the new configuration.Consider clearing the type bit when you disable the endpoint:
- USBHS_DEVICE->ENDP_CONFIG &= ~(1 << (ep_idx + 16)); + USBHS_DEVICE->ENDP_CONFIG &= ~(1 << (ep_idx + 16)); + USBHS_DEVICE->ENDP_TYPE &= ~(1 << (ep_idx + 16));(similar change for the IN branch).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
port/ch32/usb_dc_usbhs.c(5 hunks)
🔇 Additional comments (2)
port/ch32/usb_dc_usbhs.c (2)
113-117: Verify OUT/IN bit-mapping against datasheetFor OUT endpoints you set
ENDP_TYPEusing1 << (ep_idx + 16), while IN endpoints use1 << ep_idx.
The same mapping is employed forENDP_CONFIG, so this is probably intentional, but the CH32 USBHS manual places the RX (OUT) bits in the lower half and TX (IN) bits in the upper half (some revisions swap them).Please double-check with the latest reference manual for your exact chip revision to avoid silent data corruption.
266-270: Potentially clears AUTOTOG and current toggle on isochronous OUTFor isochronous OUT you clear both
RESandTOGmasks and forceTOG_0.
On this controller the hardware still manages toggling for ISO endpoints whenAUTOTOGis set; by masking-outUSBHS_EP_R_AUTOTOGyou might inadvertently break the next micro-frame.If the intention is only to ensure an ACK response, preserve the other bits:
- USB_SET_RX_CTRL(ep_idx, (USB_GET_RX_CTRL(ep_idx) & ~(USBHS_EP_R_RES_MASK | USBHS_EP_R_TOG_MASK)) | USBHS_EP_R_RES_ACK | USBHS_EP_R_TOG_0); + USB_SET_RX_CTRL(ep_idx, + (USB_GET_RX_CTRL(ep_idx) & ~(USBHS_EP_R_RES_MASK)) | USBHS_EP_R_RES_ACK);Please confirm with hardware tests before merging.
…nd fix endpoint config bug
sumengqi
changed the title
2 participants
Add Synchronous Out transmission of devices.
Summary by CodeRabbit