tinyusb icon indicating copy to clipboard operation
tinyusb copied to clipboard

RNF52/NRF53 incorrect size DMA transfers due to hardware undocumented behavior

Open kasjer opened this issue 2 years ago • 0 comments

Operating System

Linux

Board

pca10095 or pca10056 or other NRF52/NRF53

Firmware

Latest master can be 0.13.0. mynewt/nimble build with BTH and MSC enabled

What happened ?

When DMA is started for one OUT endpoint values read from SIZE.EPOUT[n] for other endpoint are not valid. This behavior is not documented (yet) by Nordic but it can be observed on NRF52/NRF53. When two packets arrive on two different endpoints single interrupt will detect and try to handle both. It calls function xact_out_dma() for both endpoints one after the other. For the first endpoint (one with lower number) DMA will start transferring data from endpoint to RAM. Second call xact_out_dma() for next ready endpoint will not start DMA right away since it's already started, it will call usbd_defer_func() with task start register. This looks like correct sequence however there is problem in following code inside xact_out_dma()

    // Following line may compute incorrect value !!!!!!!!!!!!!
    xact_len = tu_min16((uint16_t) NRF_USBD->SIZE.EPOUT[epnum], xfer->total_len - xfer->actual_len);
    // Trigger DMA move data from Endpoint -> SRAM
    NRF_USBD->EPOUT[epnum].PTR = (uint32_t) xfer->buffer;
    NRF_USBD->EPOUT[epnum].MAXCNT = xact_len; // <- Possible wrong value

    edpt_dma_start(&NRF_USBD->TASKS_STARTEPOUT[epnum]);

first line can incorrectly computes xact_len while DMA is ongoing. Read from NRF_USBD->SIZE.EPOUT[epnum] yields incorrect values. This incorrect value is then set to NRF_USBD->EPOUT[epnum].MAXCNT and later DMA will copy less bytes that it should and resulting in broken protocols.

Following code could be used to trap error

// helper to start DMA
static void start_dma(volatile uint32_t* reg_startep)
{
  for (int i = 0; i < 8; ++i) stored_size_epout1[i] = NRF_USBD->SIZE.EPOUT[i];
  (*reg_startep) = 1;
  __ISB(); __DSB();
  for (int i = 0; i < 8; ++i) stored_size_epout2[i] = NRF_USBD->SIZE.EPOUT[i];

  if (memcmp(stored_size_epout1, stored_size_epout2, 8))
    TU_BREAKPOINT();

How to reproduce ?

I think it maybe it could be reproduce in cdc_msc modified sample (at least two OUT endpoint are needed) and some massive transfer on both endpoints.

Debug Log as txt file

No response

Screenshots

No response

kasjer avatar May 26 '22 08:05 kasjer