fix(MAXUSB): Service EP0 (control) OUT & SETUP transactions#1523
fix(MAXUSB): Service EP0 (control) OUT & SETUP transactions#1523ttmut merged 3 commits intoanalogdevicesinc:mainfrom
Conversation
|
@Brandon-Hurst Why is this a draft? @ttmut this is a dependency for the max32 driver in zephyrproject-rtos/zephyr#103493, which we're trying to complete for zephyr 4.4 |
|
Ran into an issue retesting on raw MSDK examples with my APARD board which I am now comfortable chalking up to hardware problems. The board would suspend and the USB PHY clock was not coming up. However, I have tested this change on both MAX32690FTHR and MAX32666FTHR. I also tested old working code using TinyUSB and my APARD board still wouldn't work. So, I think my board was damaged. I tested USB_CDCACM on both boards and USB_MassStorage on MAX32690FTHR (not supported at the moment on MAX32666FTHR). I was seeing these use-cases work in Zephyr with my reworked changes as well. Anyway, opening it now. Just wanted to get it into a draft and then do a couple tests to rule out user error here. |
Can you update it to latest rework-udc? |
I rebased it, thanks for the heads up. I'll change it and test later today. |
Brandon-Hurst
force-pushed
the
maxusb/fix-ep0-out-txns
branch
2 times, most recently
from
48d743d to
f3a70f7
Compare
MXC_USB_ReadEndpoint currently only services non-control endpoints. The justification for this was that EP0 out doesn't have an OUT interrupt. There is some ambiguity in the UG about this, but the key issue is that ReadEndpoint is just not servicing Control OUT transactions as a result. Currently, if a new SETUP event arrives when a DATA / STATUS is in progress, all transfers are aborted, and the SETUP event is dropped. The new SETUP event should be handled, which this commit addresses. - Add handling for EP0 OUT so that control transactions can proceed properly without manual intervention. - Handle a new SETUP event interrupting a control Data or Status stage. - Add CSR0_DATA_END flag set to end of data transactions in MXC_USB_ReadEndpoint Signed-off-by: Brandon Hurst <brandon.hurst@analog.com>
Signed-off-by: Brandon Hurst <brandon.hurst@analog.com>
The #ifndef removed in this commit is unnecessary for Zephyr USB.
For reference, the stack is being reworked so that all that is
needed is the following:
```C
static int udc_event_setup(const struct device *dev)
{
int ret = 0;
MXC_USB_SetupPkt setup_pkt;
/* Get setup data from FIFO */
ret = MXC_USB_GetSetup(&setup_pkt);
if (ret != 0) {
LOG_ERR("Failed to get setup data");
return ret;
}
/* Explicit ACK for setup if no follow-on data
(checked by MXC_USB) */
MXC_USB_Ackstat(0);
/* Clear EP0 previous requests */
ret = MXC_USB_ResetEp(0);
if (ret != 0) {
LOG_ERR("Failed to reset EP0");
return ret;
}
udc_setup_received(dev, &setup_pkt, true);
return 0;
}
```
As is mentioned in musbhsfc/usb.c, SETUP transactions with no
follow-on data are ACK'd explicitly with MXC_USB_AckStat. In
the case where follow-on data is required, this line can ACK
where needed.
Testing:
- MAX32690FTHR, Zephyr USB `cdc-acm` and `mass` examples.
- MAX32690FTHR, MSDK USB_CDC-ACM sample.
Signed-off-by: Brandon Hurst <brandon.hurst@analog.com>
Brandon-Hurst
force-pushed
the
maxusb/fix-ep0-out-txns
branch
from
f3a70f7 to
bbb1d22
Compare
|
Changes completed as requested. Re-tested on MAX32690FTHR, CDC and Mass use-cases in Zephyr and MSDK examples. There is still one CI task failing but it doesn't appear related to something I can fix on my side: |
5 participants
Description
MXC_USB_ReadEndpointcurrently only services non-control endpoints. The justification for this was that EP0 out doesn't have an OUT interrupt. The issue with this is that ReadEndpoint is just not servicing Control OUT transactions as a result, which breaks the USB enumeration / configuration process.This PR adds handling for EP0 OUT so that control transactions can proceed properly without manual intervention. Otherwise some manual servicing of EP0 out transactions would be needed on the part of higher-level drivers.
An additional fix was added to fix a problem where a new SETUP event would not be handled if it was aborting other DATA / STATUS transactions on EP0. This commit switches the order of checks in the IRQ Handler so that the new SETUP is serviced before the (!aborted) check for e.g. a DATA transfer.
Originally submitted to
hal_adi: zephyrproject-rtos/hal_adi#31The motivation is to prepare for a rework to the Zephyr USB UDC driver; these changes are necessary to assist that development.
I tested the MAXUSB CDC_ACM example with these changes on the APARD32690, and they worked. I also tested the Zephyr
cdc_acmandmasssamples with the driver rework that depends on these.Checklist Before Requesting Review