USB_WritePacket stuck in infinite loop

[Strongato]

 

Hello ST Team,

I developed a custom USB DFU HOST class on an STM32F446ZE. I soldered a USB-C cable directly to the D+/D-, 5V and GND pins.

The Problem: The stack works correctly for transfers up to 128 bytes. However, when I set phost->Control.setup.b.wLength.w > 128 (e.g., 192 bytes) and call USBH_CtlReq, the code enters an infinite loop inside USB_WritePacket.

I have traced the execution line-by-line, monitoring the HCTSIZ register values: size (Transfer Size) and cnt (Packet Count).

Below is the detailed trace comparing a working 128-byte transfer vs. a failing 192-byte transfer.

1. Successful Scenario: Transferring 128 Bytes

Initial State: size = 128, cnt = 2.

Loop Iteration 1:

• i=15: USBx_DFIFO written. Register updates: size = 64, cnt = 2.

• i=31: USBx_DFIFO written. Register updates: size = 64, cnt = 1.

Loop Iteration 2:

• Initial State: size = 128, cnt = 2.

• i=15: USBx_DFIFO written. Register updates: size = 64, cnt = 1.

• i=31: USBx_DFIFO written. Register updates: size = 64, cnt = 0.

• Result: Transfer Complete (Success).

---

2. Failing Scenario: Transferring 192 Bytes

Initial State: size = 192, cnt = 3.

Loop Iteration 1:

• i=15: USBx_DFIFO written. Register updates: size = 128, cnt = 3.

• i=31: USBx_DFIFO written. Register updates: size = 64, cnt = 2.

• i=47: USBx_DFIFO written. Register updates: size = 64, cnt = 1.

Loop Iteration 2 (Anomaly Begins):

• Initial State: size = 128, cnt = 2

• i=15: USBx_DFIFO written. Register updates: size = 128, cnt = 2.

• i=31: USBx_DFIFO written. Register updates: size = 64, cnt = 1.

• i=47: USBx_DFIFO written. Register updates: size = 64, cnt = 1.

  •  cnt value didn't change in last transfer?

Loop Iteration 3 (Corruption & Infinite Loop):

• Transfer initialized with: size = 128, cnt = 3

  •  cnt value set to 3 again?

• i=15: USBx_DFIFO written. Register updates: size = 128, cnt = 3.

• i=31: USBx_DFIFO written. Register updates: size = 64, cnt = 3.

  • cnt value is now stuck at 3.

• i=47: USBx_DFIFO written. Register updates: size = 64, cnt = 3.

Comparison with STM32G0: I ported this exact USB DFU Host class to an STM32G0C1CEU6, and the transfers work perfectly there with no issues. My application code remained identical. The only major difference is the hardware architecture (F4 uses FIFO vs G0 uses PMA) and the low-level drivers.

This is my first post so if I missed something or any additional data is needed, I'm happy to provide it.

 

[FBL]

Hi ​@Strongato 

We found a bug in the host channel toggle handling for multi packet USB transfers.
The issue was that the code was updating the toggle_in / toggle_out state using only the transferred byte count, which was not always enough to correctly determine how many USB packets had actually been sent or received.

This caused the DATA0 / DATA1 PID to become incorrect after some transfers, especially when:

• the transfer length was an exact multiple of the endpoint max_packet,
• the last packet was a short packet or zero-length packet,
• or the transfer was split across multiple transactions.

As a result, the device could detect a data toggle error, which is exactly what we saw in the USB analyzer.

Fix will be implemented in HAL in HCD_HC_OUT_IRQHandler, the toggle update was previously limited to only interrupt and bulk endpoints. Now, we add control endpoints, if (hhcd->hc[chnum].ep_type != EP_TYPE_ISOC) for all non isochronous endpoints, toggle handling is applied. So control transfers use correct PID sequencing when they have multiple packets.

-      if ((hhcd->hc[chnum].ep_type == EP_TYPE_BULK) ||
-          (hhcd->hc[chnum].ep_type == EP_TYPE_INTR))
+      if (hhcd->hc[chnum].ep_type != EP_TYPE_ISOC)
       {
         if (hhcd->Init.dma_enable == 0U)
         {
           hhcd->hc[chnum].toggle_out ^= 1U;
         }
-
-        if ((hhcd->Init.dma_enable == 1U) && (hhcd->hc[chnum].xfer_len > 0U))
+        else
         {
-          num_packets = (hhcd->hc[chnum].xfer_len + hhcd->hc[chnum].max_packet - 1U) / hhcd->hc[chnum].max_packet;
-
-          if ((num_packets & 1U) != 0U)
+          if ((hhcd->hc[chnum].ep_type == EP_TYPE_BULK) ||
+              (hhcd->hc[chnum].ep_type == EP_TYPE_INTR))
           {
-            hhcd->hc[chnum].toggle_out ^= 1U;
+            if (hhcd->hc[chnum].xfer_len == 0U)
+            {
+              packet_count = 1U;
+            }
+            else
+            {
+              packet_count = (hhcd->hc[chnum].xfer_len + hhcd->hc[chnum].max_packet - 1U) / hhcd->hc[chnum].max_packet;
+            }
+
+            if ((packet_count & 1U) != 0U)
+            {
+              hhcd->hc[chnum].toggle_out ^= 1U;
+            }

 

Also in HCD_HC_IN_IRQHandler, to compute number of packets, if xfer_count == 0, we force packet_count = 1 and if xfer_count is exactly aligned to max_packet and the transfer is not finished yet, we add one more packet:

if (hhcd->hc[chnum].xfer_count == 0U)
{
  packet_count = 1U; //if xfer_count == 0, we force packet_count = 1
}
else
{
  packet_count = (hhcd->hc[chnum].xfer_count + hhcd->hc[chnum].max_packet - 1U) /
                 hhcd->hc[chnum].max_packet;
  if (((hhcd->hc[chnum].xfer_count % hhcd->hc[chnum].max_packet) == 0U) &&
      (hhcd->hc[chnum].xfer_count < hhcd->hc[chnum].xfer_len))
  {
    packet_count++; //if xfer_count is exactly aligned to max_packet and the transfer is not finished yet, we add one more packet

  }
}
if ((packet_count & 1U) != 0U)

Official fix will be shared soon in official HAL on GitHub as confirmed by our expert.

Thank you again for reporting the issue.

[Strongato]

Hi ​@FBL 

Great discovery!!!

Unfortunately I couldn’t get it to work on my machine, I’m using the HAL version 1.8.5

$ git diff Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c
diff --git a/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c b/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c
index 37afce8..9f3fe2a 100644
--- a/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c
+++ b/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c
@@ -1601,7 +1601,7 @@ static void HCD_HC_OUT_IRQHandler(HCD_HandleTypeDef *hhcd, uint8_t chnum)
   const USB_OTG_GlobalTypeDef *USBx = hhcd->Instance;
   uint32_t USBx_BASE = (uint32_t)USBx;
   uint32_t tmpreg;
-  uint32_t num_packets;
+  uint32_t packet_count;

   if (__HAL_HCD_GET_CH_FLAG(hhcd, chnum, USB_OTG_HCINT_AHBERR))
   {
@@ -1745,21 +1745,35 @@ static void HCD_HC_OUT_IRQHandler(HCD_HandleTypeDef *hhcd, uint8_t chnum)
       hhcd->hc[chnum].state = HC_HALTED;
       hhcd->hc[chnum].urb_state = URB_DONE;

-      if ((hhcd->hc[chnum].ep_type == EP_TYPE_BULK) ||
-          (hhcd->hc[chnum].ep_type == EP_TYPE_INTR))
+      if (hhcd->hc[chnum].ep_type != EP_TYPE_ISOC)
       {
         if (hhcd->Init.dma_enable == 0U)
         {
           hhcd->hc[chnum].toggle_out ^= 1U;
         }
-
-        if ((hhcd->Init.dma_enable == 1U) && (hhcd->hc[chnum].xfer_len > 0U))
+        else
         {
-          num_packets = (hhcd->hc[chnum].xfer_len + hhcd->hc[chnum].max_packet - 1U) / hhcd->hc[chnum].max_packet;
-
-          if ((num_packets & 1U) != 0U)
+          if ((hhcd->hc[chnum].ep_type == EP_TYPE_BULK) ||
+              (hhcd->hc[chnum].ep_type == EP_TYPE_INTR))
           {
-            hhcd->hc[chnum].toggle_out ^= 1U;
+            if (hhcd->hc[chnum].xfer_count == 0U)
+            {
+              packet_count = 1U; //if xfer_count == 0, we force packet_count = 1
+            }
+            else
+            {
+              packet_count = (hhcd->hc[chnum].xfer_count + hhcd->hc[chnum].max_packet - 1U) /
+                             hhcd->hc[chnum].max_packet;
+              if (((hhcd->hc[chnum].xfer_count % hhcd->hc[chnum].max_packet) == 0U) &&
+                  (hhcd->hc[chnum].xfer_count < hhcd->hc[chnum].xfer_len))
+              {
+                packet_count++; //if xfer_count is exactly aligned to max_packet and the transfer is not finished yet, we add one more packet
+              }
+            }
+            if ((packet_count & 1U) != 0U)
+            {
+              hhcd->hc[chnum].toggle_out ^= 1U;
+            }
           }
         }
       }
(END)

“Also in HCD_HC_IN_IRQHandler, to compute number of packets, if xfer_count == 0, we force packet_count = 1 and if xfer_count is exactly aligned to max_packet and the transfer is not finished yet, we add one more packet:”

I searched for the last line inside that code snippet “if ((packet_count & 1U) != 0U)”, and its not inside HCD_HC_IN_IRQHandler, its also inside HCD_HC_OUT_IRQHandler so I assume that was a typo.

Did you also like me change the variable num_packets to packet_count?

I also saw that you changed:
num_packets = (hhcd->hc[chnum].xfer_len + hhcd->hc[chnum].max_packet - 1U) / hhcd->hc[chnum].max_packet;
into:
packet_count = (hhcd->hc[chnum].xfer_count + hhcd->hc[chnum].max_packet - 1U) / hhcd->hc[chnum].max_packet;

which now uses xfer_count instead of xfer_len. I can wait for the release just to confirm I didn’t miss anything myself.

Kind regards,
Strongato

[Strongato]

Hi ​@FBL

were there any updates since we last spoke or is the issue on-going?

Kind regards,
Strongato

[FBL]

Sorry for my late reply ​@Strongato 

I assume as of now, the issue is linked to download handled by HCD_HC_OUT_IRQHandler.

I will get back to you ASAP

[FBL]

 Hi ​@Strongato 

Can you test this patch also in control transfer in CTRL_DATA_OUT ? The workaround proposed is valid for non DMA   Keeping one packet per HCD request makes a NAK retry independent of the FIFO state.  Attached HCD file, as well to for data toggling covering control transfers. 

[Strongato]

Hi ​@FBL 

Thank you so much for your reply, amazing work!

After replacing those 2 files, I get compile errors, could you please provide the additional files that are also needed:
 

../Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c: In function 'HAL_HCD_HC_Activate':
../Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c:1255:9: warning: implicit declaration of function 'USB_HC_Activate' [-Wimplicit-function-declaration]
 1255 |   (void)USB_HC_Activate(hhcd->Instance, (uint8_t)ch_num, hhcd->hc[ch_num].ch_dir);
      |         ^~~~~~~~~~~~~~~
../Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c:1255:74: error: 'HCD_HCTypeDef' {aka 'USB_HCTypeDef'} has no member named 'ch_dir'
 1255 |   (void)USB_HC_Activate(hhcd->Instance, (uint8_t)ch_num, hhcd->hc[ch_num].ch_dir);
      |                                                                          ^
../Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c: In function 'HCD_HC_IN_IRQHandler':
../Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.c:1400:20: error: 'HCD_HCTypeDef' {aka 'USB_HCTypeDef'} has no member named 'NakCnt'
 1400 |     hhcd->hc[chnum].NakCnt = 0U;
      |                    ^
make: *** [Drivers/STM32F4xx_HAL_Driver/Src/subdir.mk:61: Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_hcd.o] Error 1
make: *** Waiting for unfinished jobs....
"make -j14 all" terminated with exit code 2. Build might be incomplete.

12:37:14 Build Failed. 4 errors, 1 warnings. (took 1s.353ms)

Kind regards,
Strongato

[FBL]

Thank you ​@Strongato for your patience

I have shared with you in private message the project.

USB_HCTypeDef etc should be defined in f4_ll_usb.h