STM32 USB fixes #7322
STM32 USB fixes #7322
Conversation
Revert the patch "Create HAL_PCD_EP_Abort" in preparation for an alternate fix.
c1728p9
requested review from
jeromecoutant,
fkjagodzinski and
bruman01
and removed request for
bruman01
| } | ||
|
|
||
| /* stop the transfer */ | ||
| while (count++ < 200000U) |
There was a problem hiding this comment.
Can you use a #define for this value ? And also add a comment explaining the approx timeout delay if you know it ?
Why you didn't use the same timeout method as done in other ST HAL files (i.e. using HAL_GetTick) ?
There was a problem hiding this comment.
I'll add a comment here. In theory this loop should should execute at most once and that is when a setup packet is received. After the setup packet has been received the data (or status) phase of the control transfer can be will be naked which prevents further packet reception. Maybe its best to update this to remove the loop entirely.
I used a while loop with a count rather than HAL_GetTick to keep consistent with the other USB code (see snippit below). I can update this to use HAL_GetTick if you would like.
mbed-os/targets/TARGET_STM/TARGET_STM32F2/device/stm32f2xx_ll_usb.c
Lines 335 to 342 in f3424da
There was a problem hiding this comment.
No that's ok. Since the RTC issues due to the HAL_GetTick function, I am not very confident to use it...
| if (endpoint & 0x80) { | ||
| HAL_PCDEx_SetTxFiFo(&hpcd, endpoint & 0x7f, (max_packet / 4) + 1); | ||
| HAL_PCDEx_SetTxFiFo(&hpcd, endpoint & 0x7f, 17); |
There was a problem hiding this comment.
Can you add the comment you put in the commit message here also ?
Update the patch "Create HAL_PCD_EP_Abort" to fix bugs. This patch adds the low level functions USB_EPStopXfer, USB_EPSetNak, USB_EPClearNak and the high level function HAL_PCD_EP_Abort so that transfers can be stopped. The functions USB_EPSetNak and USB_EPClearNak allow nak to be enabled or disabled for an endpoint, preventing or allowing further transfers. The function USB_EPStopXfer stops pending reads and writes started by USB_EPStartXfer along with clearing and masking any interrupts enabled by USB_EPStartXfer. The function HAL_PCD_EP_Abort aborts any transfers on the given endpoint. When this function completes the transfer interrupt is guarenteed not to fire for this endpoint. Furthermore, the size of data transferred during an aborted read can be found by calling the function HAL_PCD_EP_GetRxCount. Other notes on this Change: 1. Prior to this patch the interrupt USB_OTG_DOEPINT_EPDISD was not handled. When an OUT endpoint was disabled this interrupt occurred causing the CPU to get stuck repeatedly handling this interrupt. This is because this interrupt was unmasked but nothing cleared this interrupt. This patch also adds code to handle and clear this interrupt to prevent a lockup. 2. Stopping a transfer on an OUT endpoint requires global nak OUT to be in effect. Even with this being done, having entries in the rx fifo prevented an OUT endpoint from being disabled. This behavior is not mentioned in the Reference Manual.
Make use of the added function HAL_PCD_EP_Abort to implement endpoint_abort.
Also always use the max endpoint size for SetTxFifo since re-configuring fifos when endpoints are added or removed causes tests to fail.
When activating an endpoint assign new data rather than ORing data to it. This ensures that values set from the previous use do not effect the current configuration.
|
@bcostm @jeromecoutant @fkjagodzinski If y'all could re-review. |
|
/morph build |
Build : SUCCESSBuild number : 2482 Triggering tests/morph test |
Test : SUCCESSBuild number : 2257 |
Exporter Build : SUCCESSBuild number : 2121 |
Description
This PR addresses issues found by upcoming USB tests in #6880. It contains a working implementation of endpoint abort along with other fixes.
Pull request type