USBD Driver - Bug when removing ISO endpoint

[George Beckstien]

Hello,

I am working on implementing the USB HAL for the nRF52840 for Mbed-OS's upcoming USBDevice feature release. I am trying to get my implementation to pass all of Mbed's automated tests.

I have encountered an issue with the Nordic USBD driver in SDK14.2, but have determined the bug is still present in SDK15.2

The issue is this:

During a `set_interface` call from the upper layers of the Mbed stack, endpoints that are part of the current interface are removed. My implementation calls `nrf_drv_usbd_ep_disable` during this call, which works fine for all non-ISO endpoints.

At some point in the call flow, the endpoint being removed is aborted, resulting in a call to `nrf_usbd_epout_clear`. In this function, there is an `ASSERT` that will always fail for the `ISOOUT` endpoint. When you pass in the `ISOOUT` endpoint number (8), the ASSERT fails because the endpoint's index is outside the bounds of the `NRF_USBD->SIZE.EPOUT[]` array. I think what should happen when the `ISOOUT` endpoint index is passed is the driver should access `NRF_USBD->SIZE.ISOOUT` instead.

See the culprit line here:

https://github.com/ARMmbed/mbed-os/blob/8f48104842a8932382fd18f952e994facb1513e2/targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_SDK_14_2/drivers_nrf/hal/nrf_usbd.h#L1202-L1208

Check out the Github conversation here:

https://github.com/ARMmbed/mbed-os/issues/7674

Here's a screenshot during debugging that shows the call stack in Eclipse:

[George Beckstien]

After looking more closely at the datasheet, it seems the `SIZE.ISOOUT` register is read-only. So maybe this function is correct in ASSERT-ing when given the ISOOUT endpoint as an argument. However, is it possible to remove an ISO endpoint without encountering this ASSERT since calling `nrf_drv_usbd_ep_disable` causes this?

It seems that `nrf_drv_usbd_ep_disable` should make sure not to call `usbd_ep_abort` on ISO endpoints... not sure if there is a way to abort ISO transfers.

[Håkon Alseth]

Hi,

 

ISO transfers will occur anyway as long as the bus is available, so aborting it may not make much sense. I still see this as an issue from our side, as the _disable() should not call _abort() on ISO EPs. I'll forward this to our USB developers so they can have a look, and see if any improvement can be made.

 

Cheers,

Håkon