class/midi: skip requeue in midih_xfer_cb on transfer failure - #3838
Open
sanjay-nagesh wants to merge 1 commit into
Open
class/midi: skip requeue in midih_xfer_cb on transfer failure#3838sanjay-nagesh wants to merge 1 commit into
sanjay-nagesh wants to merge 1 commit into
Conversation
When a MIDI device is unplugged mid-transfer, the pending IN transfer on the interrupt/bulk endpoint completes with XFER_RESULT_FAILED, but midih_xfer_cb ignored the result and unconditionally re-queued a new transfer on the same endpoint buffer. On rp2040, this re-queue raced with the host controller's own abort handling for the same buffer control register, hitting the "buf_ctrl already available" panic. Bail out immediately when result != XFER_RESULT_SUCCESS, matching the existing pattern in cdc_host.c's xfer callback.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is minimal, consistent with existing host-driver patterns, and directly addresses the reported unplug panic by preventing endpoint re-queue on failed transfers.
Pull request overview
Fixes a MIDI Host robustness issue where midih_xfer_cb() would re-arm the RX endpoint even when the underlying transfer failed (e.g., device unplug mid-transfer), aligning MIDI Host behavior with existing host-class patterns (notably CDC) and avoiding RP2xxx controller-side abort/rearm collisions.
Changes:
- Stop processing and re-queuing transfers in
midih_xfer_cb()whenresult != XFER_RESULT_SUCCESS. - Remove the previous
(void) result;discard to ensure failure is handled immediately.
File summaries
| File | Description |
|---|---|
src/class/midi/midi_host.c |
Adds an early-return guard on failed transfers in the MIDI host transfer callback to prevent re-arming endpoints after unplug/failure. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Hardware-in-the-loop (HIL) Test ReportNo HIL run for this push (no affected boards, or hardware testing did not run). |
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 size
Changes <1% in size
No changes
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| frdm_kl25z/midi_rx | 18,584 → 18,592 (+8) | — | — | — | 18,744 → 18,752 (+8) | +0.0% |
| stm32c542nucleo/midi_rx | 19,592 → 19,600 (+8) | — | — | — | 20,536 → 20,544 (+8) | +0.0% |
| stm32c071nucleo/midi_rx | 21,736 → 21,744 (+8) | — | — | — | 22,540 → 22,548 (+8) | +0.0% |
| stm32g0b1nucleo/midi_rx | 23,776 → 23,784 (+8) | — | — | — | 24,580 → 24,588 (+8) | +0.0% |
| frdm_rw612/midi_rx | 24,472 → 24,480 (+8) | — | — | — | 26,612 → 26,620 (+8) | +0.0% |
| raspberry_pi_pico/midi_rx | 25,732 → 25,740 (+8) | — | — | — | 26,688 → 26,696 (+8) | +0.0% |
| b_u585i_iot2a/midi_rx | 32,792 → 32,800 (+8) | — | — | — | 34,112 → 34,120 (+8) | +0.0% |
| lpcxpresso1769/midi_rx | 16,732 → 16,736 (+4) | — | — | — | 17,252 → 17,256 (+4) | +0.0% |
| lpcxpresso18s37/midi_rx | 18,748 → 18,752 (+4) | — | — | — | 19,342 → 19,346 (+4) | +0.0% |
| portenta_c33/midi_rx | 19,260 → 19,264 (+4) | — | — | — | 19,424 → 19,428 (+4) | +0.0% |
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.
PR for #3805 - RP2xxx MIDI Host panics on device unplug
midih_xfer_cb()was just throwing away the transfer result and always re-queueing the next RX transfer regardless of what happened. So, when you unplug a MIDI device mid-transfer, the pending In transfer comes back as XFER_RESULT_FAILED, but the driver didn't noticed that and tried to re-arm the endpoint regardless. On RP2040, that collides with the host's controller abort/cleanup on the same buffer-control register and it panics withbuf_ctrl already availableSo, I fixed it by making t so that it bails out early if
result != XFER_RESULT_SUCCESS.cdc_host.calready handles this in (in the same way)cdch_xfer_cbso this just brings the MIDI in line with other patterns in the host stack.