HAL_PCD_EP_GetRxCount returning wrong value ?

[Geoffrey1]

Posted on May 18, 2017 at 20:28

I'm using  

STM32Cube_FW_L4_V1.6.0 for long bulk transfers (up to 4096 bytes).   When the stm32 receives a packet longer that 64 bytes,  HAL_PCD_EP_GetRxCount returns a number 1-64 (e.g. a packet of 1024 returns a length of 64).  The received data seem to be correct, so I'm confused about the incorrect packet length (plus it's impossible to detect errors !).   Has anybody seen a similar problem ?

Geoffrey

[Imen.D]

Posted on May 19, 2017 at 11:35

Hi

Brown.Geoffrey

,

Please use the last STM32CubeL4 firmware version 1.8.0 as contains update on HAL PCD driver.

Then, keep us updated about your progress on this.

Thanks

Imen

[Geoffrey1]

Posted on May 19, 2017 at 14:32

I've moved to the latest 1.8.0 firmware, without a change. Here's what I am attempting to accomplish. I have written a bulk driver and want to receive large packets. In my code I call prepare receive:

 USBD_LL_PrepareReceive(pdev,
 BULK_OUT_EP,
 hcdc->RxBuffer,
 hcdc->RxLength);�?�?�?�?�?�?�?�?

In the specific case that is causing me trouble, hcdc->RxLength is greater than 64 bytes (e.g. 1024). In my ***_DataOut function, I attempt to capture the length of data received.

 hcdc->RxLength = USBD_LL_GetRxDataSize(pdev, epnum);�?�?�?

USBD_LL_GetRxDataSize is simply a wrapper for HAL_PCD_EP_GetRxCount.

***_DataOut is called by the usual chain

HAL_PCD_DataOutStageCallback -->

USBD_

LL_Da

taOutStage --> ***_DataOut

The problem is that USBD_LL_GetRxDataSize seems to return the size of literally the last packet (e.g. 64 bytes or less) received rather than the total amount received since the last call to USBD_LL_PrepareReceive.

Either I'm misunderstanding the intended behavior of USBD_LL_GetRxDataSize (not unlikely: as a side note, to the STM documentation team, procedure prototypes document syntax, but not semantics), or there is a but in the libraries. If the former, please give a hint about how to accomplish my goal. If the later, please give a workaround.

[Geoffrey1]

Posted on May 19, 2017 at 14:55

ok, I hate having to grovel around in the HAL code just to figure out what is going on. Here's a bit of stm32l4xx_hal_pcd.c that seems wrong (around line 1585):

 /*multi-packet on the NON control OUT endpoint*/
 ep->xfer_count+=count;
ep->xfer_buff+=count;
 if ((ep->xfer_len == 0) || (count < ep->maxpacket))
{
 /* RX COMPLETE */
 HAL_PCD_DataOutStageCallback(hpcd, ep->num);
 }
else
 {
 HAL_PCD_EP_Receive(hpcd, ep->num, ep->xfer_buff, ep->xfer_len);
 }
�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

It seems the intent is to repeatedly call HAL_PCD_EP_Receive until the full packet is received. The code moves the buffer pointer (line 3), but in line 12 continues to provide the original length. Looks like a potential buffer overrun

(hint: it should probably be ep->xfer_len - ep->xfer_count).

[Geoffrey1]

Posted on May 19, 2017 at 15:24

Ok, I see the driver bug now. In the call to HAL_PCD_EP_Receive above, ep->xfer_count is reset to zero:

HAL_StatusTypeDef HAL_PCD_EP_Receive(PCD_HandleTypeDef *hpcd, uint8_t ep_addr, uint8_t *pBuf, uint32_t len)
{
 PCD_EPTypeDef *ep = NULL;
 ep = &hpcd->OUT_ep[ep_addr & 0x7F];
 /*setup and start the Xfer */
 ep->xfer_buff = pBuf;
 ep->xfer_len = len;
 ep->xfer_count = 0;
 ep->is_in = 0;
 ep->num = ep_addr & 0x7F;
 if ((ep_addr & 0x7F) == 0 )
 {
 USB_EP0StartXfer(hpcd->Instance, ep, hpcd->Init.dma_enable);
 }
 else
 {
 USB_EPStartXfer(hpcd->Instance, ep, hpcd->Init.dma_enable);
 }
 return HAL_OK;
}
�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

So, it behaves exactly as I said -- only the length of the last received packet is preserved. All that business of incrementing ep->xfer_count is for naught. Here's a dirty fix:

 /*multi-packet on the NON control OUT endpoint*/
 ep->xfer_count+=count;
 ep->xfer_buff+=count;
 if ((ep->xfer_len == 0) || (count < ep->maxpacket))
 {
 /* RX COMPLETE */
 HAL_PCD_DataOutStageCallback(hpcd, ep->num);
 }
 else
 {
 uint32_t tmp = ep->xfer_count;
 HAL_PCD_EP_Receive(hpcd, ep->num, ep->xfer_buff, ep->xfer_len - tmp);
 ep->xfer_count = tmp;
 }�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

I hope that ST will do the right thing and test their PCD code a bit more thoroughly.

BTW, the same bugs exist for HAL_PCD_EP_Transmit, and a similar fix is required if xfer_count is expected to be meaningful, and it to eliminate the possibility of a buffer overrun.

[Imen.D]

Posted on May 19, 2017 at 18:30

Thank you

Brown.Geoffrey

for your contribution.

Your reported issue is underinvestigation and I will keep you informed about the taken actions/explanation if needed.

Thanks

Imen