fix ISO OUT endpoint failure on alt-setting MPS change#3575
Conversation
TinyUSB Average Code Size Metrics
Input files
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| xmc4500_relax/dfu_runtime | 12,244 → 12,308 (+64) | — | — | — | 12,252 → 12,316 (+64) | +0.5% |
| xmc4500_relax/hid_generic_inout | 13,160 → 13,224 (+64) | — | — | — | 13,168 → 13,232 (+64) | +0.5% |
| sipeed_longan_nano/dfu_runtime | 12,820 → 12,884 (+64) | — | — | — | 14,034 → 14,098 (+64) | +0.5% |
| xmc4500_relax/hid_composite | 14,124 → 14,188 (+64) | — | — | — | 14,132 → 14,196 (+64) | +0.5% |
| xmc4500_relax/msc_dual_lun | 15,112 → 15,176 (+64) | — | — | — | 15,120 → 15,184 (+64) | +0.4% |
| sipeed_longan_nano/hid_generic_inout | 14,104 → 14,168 (+64) | — | — | — | 15,186 → 15,250 (+64) | +0.4% |
| xmc4500_relax/printer_to_cdc | 15,228 → 15,292 (+64) | — | — | — | 15,236 → 15,300 (+64) | +0.4% |
| xmc4500_relax/audio_test | 15,320 → 15,384 (+64) | — | — | — | 15,328 → 15,392 (+64) | +0.4% |
| xmc4500_relax/webusb_serial | 15,588 → 15,652 (+64) | — | — | — | 15,596 → 15,660 (+64) | +0.4% |
| xmc4500_relax/usbtmc | 16,000 → 16,064 (+64) | — | — | — | 16,008 → 16,072 (+64) | +0.4% |
| // Full register write (not |=) to clear all bits including active/enabled state | ||
| epout->doepctl = DOEPCTL_SNAK; | ||
| epout->doepint = 0xFFFFFFFFu; | ||
| epout->doeptsiz = 0; |
There was a problem hiding this comment.
Thank you for the fix but what I think the only thing needed is adding SNAK to edpt_disable:
doeptsiz, doepdma and xfer state will be set on next transfer.
There was a problem hiding this comment.
Also could you dump the DWC2 config with dwc2_info.py ? It'a new issue specific to nRF54LM20.
|
I investigated the LM20 DWC2 config and also reduced the fix further. For the DWC2 config dump, I read the registers on hardware and added a dedicated These differ from the existing generic I tested the All the other writes I had in the earlier version ( The extra helper function is not strictly required anymore at this point; the same logic could be inlined. I kept it for now because I think it makes the special-case intent a bit clearer, but that part is mostly a matter of preference and I can inline it if you prefer. Edit: Actually i just noticed that i mixed up the this PR and #3574 a little bit maybe should have modified dwc2_info in that PR, hope that isnt too much of an issue. |
|
Thank you, it looks like something is changed in GID 5.00b. But I don't understand why settings Could you check Disabling an endpoint without following $7.5.1 procedure is not a good idea. |
|
I investigated further, it seems the issue is not that You are right, its probably better to not skip the normal disable flow. I changed the sequence to:
This only applies for ISO OUT when the MPS changes. The actual reason why the For completeness, here are the DOEPCTL values before and after edpt_activate() for the current fix and without the fix:
The only difference is bit 15 being cleared before re-activation. I considered putting this into the generic OUT path in |
|
I think it's safe to clear ActEP in Is it work for you : diff --git a/src/portable/synopsys/dwc2/dcd_dwc2.c b/src/portable/synopsys/dwc2/dcd_dwc2.c
index 8685ec6dc..e32dedd1b 100644
--- a/src/portable/synopsys/dwc2/dcd_dwc2.c
+++ b/src/portable/synopsys/dwc2/dcd_dwc2.c
@@ -345,6 +345,11 @@ static void edpt_disable(uint8_t rhport, uint8_t ep_addr, bool stall) {
dwc2->dctl |= DCTL_CGONAK;
}
}
+
+ // Clear ActEP
+ if (!stall && epnum != 0) {
+ dep->ctl &= ~EPCTL_USBAEP;
+ }
}
// Since this function returns void, it is not possible to return a boolean success message |
…iso_activate(), Clear active EP bit in edpt_disable()
|
Alright, sounds good to me! This probably is the most elegant solution anyway I was just afraid that it might break something somewhere else. |
2 participants
Summary
When an ISO OUT endpoint is reused across alternate settings with a different
wMaxPacketSize(e.g. UAC2 speaker switching sample rates), the DWC2dcd_edpt_iso_activate()path leaves stale hardware state behind. After the alt-setting switch, the first packets fault and no data reaches the application.The existing code called
edpt_disable()before re-activation, but its full GONAK/handshake sequence does not clear residualdoeptsiz/doepdmavalues from the previous configuration on all DWC2 variants.Fix
Add
edpt_iso_out_reset()a register reset function that clears endpoint HW state and transfer bookkeeping before reactivation. Only triggered when all three conditions are met:All other paths (IN endpoints, same MPS, first activation) should be unchanged.
I kept it separate from
edpt_disable()intentionally as its a pre-activation state reset, not a full endpoint teardown.Context
Discovered while bringing up the nRF54LM20 DK (#3574) with the
uac2_headsetexample. Also tested on a NUCLEO-H7S3L8, havent seen any regression issues, alt-setting switches work correctly on both boards. Testing was not extensive though, just basic functional verification. I dont have access to other DWC2 platforms (ESP32-S2/S3, etc.).Also ran the ceedling unit tests, all passed.