usbd: disable xfer_isr path for ch32 usbfs/usbhs#3728
Open
HiFiPhile wants to merge 2 commits into
Open
Conversation
Signed-off-by: HiFiPhile <admin@hifiphile.com>
Signed-off-by: HiFiPhile <admin@hifiphile.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR disables the USBD xfer_isr callback path for WCH CH32 USBFS/USBHS targets to reduce ISR processing time (mitigating isochronous transfer corruption/data loss), and documents a partial workaround for the WCH ISO limitations.
Changes:
- Add a
TUP_USBD_XFER_ISRconfiguration knob (default on) and force it off for CH32 (WCH) MCU families intusb_mcu.h. - Make
xfer_isroptional/conditional across USBD core and class-driver declarations/registration. - Update the audio device class to re-arm transfers from
xfer_cbwhenxfer_isris disabled; add documentation notes for the CH32 ISO issue workaround.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/device/usbd.c |
Gates the xfer_isr dispatch path and class driver table initializers behind TUP_USBD_XFER_ISR. |
src/device/usbd_pvt.h |
Makes the usbd_class_driver_t::xfer_isr callback conditional on TUP_USBD_XFER_ISR. |
src/common/tusb_mcu.h |
Defines TUP_USBD_XFER_ISR (default 1) and forces it to 0 for WCH CH32 MCU families. |
src/class/audio/audio_device.h |
Gates the audiod_xfer_isr prototype behind TUP_USBD_XFER_ISR. |
src/class/audio/audio_device.c |
Refactors audio endpoint completion handling so transfers can be re-armed from task context when ISR path is disabled. |
docs/reference/device_issues.rst |
Adds a “Partial workaround” section for the WCH CH32 isochronous endpoint corruption issue. |
Comments suppressed due to low confidence (1)
src/class/audio/audio_device.c:554
- audiod_tx_done() now runs in both ISR and task context (depending on the is_isr argument / TUP_USBD_XFER_ISR), but it still calls tud_audio_tx_done_isr(). This function is documented and named as ISR-only, so calling it from task context when TUP_USBD_XFER_ISR==0 is surprising for application code and can lead to incorrect assumptions about what is safe to do in the callback. At minimum, document this behavior near the call (or preferably update the public API docs / naming to reflect the actual context).
// Call a weak callback here - a possibility for user to get informed former TX was completed and data gets now loaded into EP in buffer
TU_VERIFY(tud_audio_tx_done_isr(rhport, n_bytes_sent, idx_audio_fct, audio->ep_in, audio->ep_in_alt));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in sizeNo entries. Changes <1% in size
No changes
|
Hardware-in-the-loop (HIL) Test Reporthfp-iar
Legend: ✅ pass · ❌ fail · ⚪ skipped · blank not run hfp.json
Legend: ✅ pass · ❌ fail · ⚪ skipped · blank not run tinyusb.json
Legend: ✅ pass · ❌ fail · ⚪ skipped · blank not run |
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| ch32v103c_bluepill/printer_to_cdc | 11,908 → 11,628 (-280) | — | — | — | 12,268 → 11,988 (-280) | -2.3% |
| ch582m_evt/printer_to_cdc | 9,032 → 8,760 (-272) | — | — | — | 12,568 → 12,296 (-272) | -2.2% |
| ch32v103c_bluepill/webusb_serial | 12,248 → 11,980 (-268) | — | — | — | 12,664 → 12,396 (-268) | -2.1% |
| ch32v103c_bluepill/dfu_runtime | 8,556 → 8,368 (-188) | — | — | — | 8,924 → 8,736 (-188) | -2.1% |
| ch32v307v_r1_1v0/printer_to_cdc | 13,308 → 13,024 (-284) | — | — | — | 13,796 → 13,512 (-284) | -2.1% |
| ch32v203c_r0_1v0/printer_to_cdc | 14,036 → 13,756 (-280) | — | — | — | 14,332 → 14,052 (-280) | -2.0% |
| ch32v307v_r1_1v0/dfu_runtime | 9,380 → 9,188 (-192) | — | — | — | 9,876 → 9,684 (-192) | -1.9% |
| ch582m_evt/webusb_serial | 9,464 → 9,212 (-252) | — | — | — | 13,048 → 12,796 (-252) | -1.9% |
| ch32v307v_r1_1v0/webusb_serial | 13,528 → 13,260 (-268) | — | — | — | 14,072 → 13,804 (-268) | -1.9% |
| ch32v103c_bluepill/cdc_msc_throughput | 13,956 → 13,688 (-268) | — | — | — | 14,316 → 14,048 (-268) | -1.9% |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disable xfer_isr path for ch32 usbfs/usbhs as extended processing time inside interrupt handler will lead to ISO data loss.
Added a note about ISO issue workaround, together with #3727
uac2_headsetanduac2_speaker_fbexamples works on CH32V305.