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.
When two tasks entered dcd_edpt_xfer() it was possible that
first disabled interrupt to setup total_len and actual_len
but second task for another endpoint enabled interrupt
between total_len and actual_len resulting in race
condition with interrupt, hence mutex is added on top of interrupt being blocked.
In multi-thread mode starting DMA in thread mode was
prone to race condition resulting in infinite loop.
It may happen on single core CPU with strict priority based
tasks scheduler where ready high prio task never yields to
ready low prio task (Mynewt).
Sequence that failed (T1 - low priority task, T2 - high priority task)
- T1 called start_dma()
- T1 set _dcd.dma_running (DMA not started yet, context switch happens)
- T2 took CPU and saw that _dcd.dma_running is set, so waits for _dcd.dma_running to be 0
- T1 never gets CPU again, DMA is not started T2 waits forever
OSAL mutex resolves problem of DMA starting from thread-context.
Mynewt (similar to Soft Device) has its own reference counting for
HFXO oscillator.
So far TinyUSB requested HFXO when VBUS was detected and stopped when
VBUS was removed.
But with Mynewt running HFXO can be stopped when other interested parties
don't require HFXO anymore. This results in very difficult to track
USB transmission errors.
This change enables Mynewt specific HFXO management in Soft Device fashion.
When dcd_edpt_xfer() starts new transfer two separate problems were observed.
For both problems stream of OUT packets was pouring from host.
First problem was that total_len and actual_len were not atomic.
In case where incoming OUT packets are less (63) than MPS (64), actual_len and total_len
are set 63.
Then transfer complete from USBD is called that will schedule next 64 bytes transfer.
At that point incoming packet would start DMA if there is place in RAM, normally
it does not happen since actual_len == total_len.
If packets arrives and interrupt is raised after total_len is set (64) but actual_len is still 63 from
previous transfer, interrupt code sees that there is place in ram (1 byte) and transfer this 1 byte
to buffer that was already filled with previous packet.
To remedy this USB interrupt is blocked during transfer setup.
Second problem can happen when dcd_edpt_xfer setups xfer->total_len and actual_len correctly
but then context switch happens before xfer->data_received is checked.
If during this time two packets arrive one will be copied to RAM second will stay in endpoint with
data_received set to 1.
Then when xfer_edpt_xfer() checks data_receive flag it starts DMA again overwriting data.
To remedy this, data_received is checked together with check if data was already transferred.
If transfer was complete, there is no need to start DMA yet.
In such case data_received will be handled in same place by next xfer_edpt_xfer() correctly.
When NRF5x device is reset by software (after DFU for example),
power event is ready from the beginning.
When power interrupt is triggered before tud_init() finished
USBD_IRQn is enabled before it would be enabled in tud_init().
This in turn may result in BUS RESET event being sent from
USB interrupt to USB task when queue is not initialized yet.
This scenario often happens in Mynewt build where queue creation
takes more time.
To prevent this scenario USBD_IRQn is not enabled in power event
interrupt handler before dcd_init() was called.
During Adafruit Bootloader compilation, I spotted bellow error which do not allow me build project.
``` c
inlined from 'hfclk_running' at lib/tinyusb/src/portable/nordic/nrf5x/dcd_nrf5x.c:785:13:
lib/tinyusb/src/portable/nordic/nrf5x/dcd_nrf5x.c:792:31: error: 'is_running' may be used uninitialized [-Werror=maybe-uninitialized]
792 | return (is_running ? true : false);
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~
```
Errata code referred to NRF_USBD_BASE.
This definition is not present in NRF5340 but both NRF52 and NRF53
do have NRF_USBD which maps to NRF_USBD_BASE for NRF52 and
to NRF_USBD_S_BASE for NRF5340.
This just make build possible for NRF5340.
NRF_USBD->SIZE.EPOUT[epnum] only need to write once to enable
Bulk/Interrupt transfer. We only need to do it in dcd_edpt_open() and
dcd_edpt_clear_stall()
ISO endpoints were not covered so far by the driver code.
This adds support for ISO IN and OUT endpoint handling.
Registers for ISO IN(OUT) endpoints are placed just after normal IN(OUT)
so in some cases common code could be used for handling all type of
transfers.
Generally code synchronizes ISO endpoint handling to SOF interrupt.
This code does not change the way of how non-ISO endpoints are treated.
Code uses strategy outlined in nRF52840 Produce Specification v1.0
sections 6.35.11.1 and 6.35.11.2.
Closing endpoints can be important when there are alternate
instances. This adds functionality of closing endpoints
similar to what exists in other drivers.
Operator < used in while condition was obviously incorrect.
Loop starts with checking if unsigned variable is less then 0.
This condition is always false.
This reverses condition to follow intention of of the code.
