nrf5x: Fix DMA access

[kasjer]

Describe the PR There were two problems:

• dma_running flag could be checked in USB interrupt (not set yet) then higher priority interrupt could start transfer, check dma_running (not set yet) set it to true start DMA; then when USB interrupt continues it starts another DMA that is not allowed
• when DMA is started some registers can't be safely accessed, read can yield invalid values (SIZE.EPOUT, SIZE.EPISO) current implementation could start DMA for one OUT endpoint then check that another endpoint also has data and while DMA was not started right away, SIZE.EPOUT was copied already to MAXCNT register. Later on when DMA was started not all data was read from endpoint due to incorrect DMA size previously set.

To prevent both cases dma_running is changed in atomic way. Only code that actually set this value to true starts DMA, code that tried and had dma_running flag already set simply defers DMA start to USB task. This eliminates also need to have mutex that was there to limit access to dma_running flag to one task only. transfer also now has started flag that is set only after dcd_edpt_xfer() sets up total_len and actua_len. Previously USB interrupt was disabled when total_len and actual_len were setup to prevent race condition when data arrived to ednpoint before transfer was setup was finished.

A clear and concise description of what this PR solve.

Additional context This make obsolete #1407 (same race condition is fixed here in different way) It also fixes #1474 (discussion about SIZE.EPOUT on Nordic DevZone https://devzone.nordicsemi.com/f/nordic-q-a/88301/nrf5340-usb-size-epout-n-value-not-stable)

[sjanc]

Hi,

Any update on this? We are using this in product and test it extensively. In long term we would like to avoid having to use our own fork on tinyusb..

[sjanc]

I just wanted to confirm that this patch fixes all the USB issues we've been seeing when stress-testing USB on nRF5340. It would be really great if this fix is merged into upstream repo.

[tannewt]

Could this bug cause data written out of usb to be read in? That's been a longstanding issue in CircuitPython on the nRF52840.

[pigrew]

@hathach , I see this is using the C11 atomics. I was thinking about using them in the STM32 driver, too, but was not sure if TinyUSB can require that new of a compiler?

[kasjer]

Could this bug cause data written out of usb to be read in? That's been a longstanding issue in CircuitPython on the nRF52840.

One of the issue this fixes is that DMA is reported as finished while number of bytes received is less than what was requested (including 0) in that case if same buffer was used for write and read it could manifest like echo I guess. I think that at one point I also had an impression about mismatch between what was send in both directions but it turned out that buffer was not filled even though DMA finished. So I guess this could be the fix for what you saw in CircuitPython

[Nikitarc]

TinyUSB already requires gcc C11 to get rid of warnings about unnamed structs/unions when [-Wpedantic] is used (ISO C99 doesn't support unnamed structs/unions). Why not use other features of C11?

[hathach]

Could this bug cause data written out of usb to be read in? That's been a longstanding issue in CircuitPython on the nRF52840.

Yeah, I think this will possibly fix the hiccup issue with nrf cdc fast transferring.

@hathach , I see this is using the C11 atomics. I was thinking about using them in the STM32 driver, too, but was not sure if TinyUSB can require that new of a compiler?

I actually really like c11, we already in 22' already. C11 is much better and safer than C99, one of the major C version where it is under lots of improvement for a very long time (since C99). So yeah, please go ahead with your changes. I also made an issue regarding LDREX/STREX on M3/M4 #662. Note since M0 does not support these instruction, atomic operation on M0 will probably is generated to __disable_irq() (I could be wrong, but please check the dissambly to be sure).

Any update on this? We are using this in product and test it extensively. In long term we would like to avoid having to use our own fork on tinyusb..

Thank you for the reminder, sorry, this is my bad, this PR is perfect, I just got buried in other works, and kind of forgot :weary: :weary: