USB data toggling with DMA

[skeeter]

Hey folks,

I've seen many posts from frustrated users of USB OTG HAL, especially when enabling DMA. I created a fix for data toggle errors (DTERR) when using DMA with USB_OTG_HS HCD paired with full speed embedded PHY on a STM32F4xx.  Hopefully this fixes more than just F4xx.

The latest stm32f4xx_hal_hcd.c generated by CubeMX fixes a great deal from the previous HAL code, but still has a bug that causes data toggle errors when using DMA.

The interrupt service routine HCD_HC_IN_IRQHandler() handles XFRC interrupts with the following toggling logic:

    if (hhcd->Init.dma_enable == 1U)
    {
      if ((((hhcd->hc[chnum].xfer_count + hhcd->hc[chnum].max_packet - 1U) / hhcd->hc[chnum].max_packet) & 1U) != 0U)
      {
        hhcd->hc[chnum].toggle_in ^= 1U;
      }
    }
    else
    {
      hhcd->hc[chnum].toggle_in ^= 1U;
    }

Changing it to this fixed my woes:

    if (hhcd->Init.dma_enable == 1U)
    {
      // Toggle if packet count is odd or xfer_count is an even multiple of max_packet
      uint32_t pkt_count = (hhcd->hc[chnum].xfer_count + hhcd->hc[chnum].max_packet - 1U) / hhcd->hc[chnum].max_packet;
      if (((pkt_count & 1U) != 0U) || ((pkt_count * hhcd->hc[chnum].max_packet) == hhcd->hc[chnum].xfer_count))
      {
        hhcd->hc[chnum].toggle_in ^= 1U;
      }
    }
    else
    {
      hhcd->hc[chnum].toggle_in ^= 1U;
    }

Essentially, ST got it half correct. With DMA, multiple packets occur within a single XFRC so you have to do extra work to figure out the data toggling after a receive completes. You should still toggle if an odd number of packets were received, but you should also toggle if the total received bytes is an even multiple of the endpoint's max packet size (64 in my case).  This is true for all endpoint types, not just BULK and INTR.

Not only does this fix the data toggle error interrupts (which show up as URB_NOTREADY), but it also fixes having to deal with zero-length packets (ZLP's) in the URB ISR callback.

Hope this helps someone.

Cheers

[FBL]

Hi @skeeter 

1. I guess data toggling is managed itself within the control transfer protocol. On the other hand, Isochronous endpoints prioritize timing critical delivery over data integrity, making this mechanism unnecessary for them.
    
2.  About,  "you should also toggle if the total received bytes is an even multiple of the endpoint's max packet size" Not sure about this? Could you provide your reference? 

[skeeter]

1. The current HAL library (stm32f4xx_hal_hcd.c) performs a data toggle on every XFRC interrupt for all endpoint types, including Isochronous. I didn't change that functionality. If you believe that data toggling should not happen for Isochronous then you should change the HAL to no longer perform data toggling for Isochronous.

2. USB protocol states that a receive is considered complete if:

• The exact number of bytes requested is received.
• The number of bytes in a frame is less than max packet size for the endpoint.
• A zero byte packet (ZLP) is received.

I noticed there were data toggle errors (DTERR) on the port causing data loss whenever the number of bytes received was an even multiple of the endpoint max packet size. This is because a ZLP was received by the HAL and the HAL was not performing a data toggle. My update essentially adds a data toggle for this special case.

It's possible that there are USB devices that don't conform to the ZLP requirement and my change would then cause data toggle errors for those devices. But for my device I didn't want the errors and the data loss was not acceptable.

Perhaps there is a more elegant solution that would allow the driver that sits on top of the HAL to manage the data toggling. Currently, the HAL is managing it so I modified the HAL.

What is your recommendation @FBL ?

[skeeter]

An alternate way of handling this without modifying the HAL is to update the toggling from within the UrbChange ISR callback.

void HAL_HCD_HC_NotifyURBChange_Callback(HCD_HandleTypeDef *hhcd, uint8_t chnum, HCD_URBStateTypeDef urb_state)
{
   if (urb_state == URB_DONE)
   {
      HCD_HCTypeDef *hc = &hhcd->hc[chnum];
      if (hc->ep_is_in != 0U && hc->ep_type != EP_TYPE_ISOC)
      {
         if ((hc->xfer_count & (hc->max_packet - 1U)) == 0U && hc->xfer_count != hc->xfer_len)
         {
            hc->toggle_in ^= 1;
         }
      }
   }
   ...signal your thread here
}