cancel
Showing results for 
Search instead for 
Did you mean: 

Next error in stm32f4xx_ll_usb.c?

Marek Kuziel
Associate II

In function USB_EP0_OutStart is:

 if (dma == 1U)

 {

   USBx_OUTEP(0U)->DOEPDMA = (uint32_t)psetup;

   /* EP enable */

   USBx_OUTEP(0U)->DOEPCTL |= USB_OTG_DOEPCTL_EPENA | USB_OTG_DOEPCTL_USBAEP;

 }

In my opinion enabling endpoint should be out of DMA condition:

 if (dma == 1U)

 {

   USBx_OUTEP(0U)->DOEPDMA = (uint32_t)psetup;

 }

 /* EP enable */

 USBx_OUTEP(0U)->DOEPCTL |= USB_OTG_DOEPCTL_EPENA | USB_OTG_DOEPCTL_USBAEP;

STM32CubeF4 version 1.24.1

The same situation exists in STM32CubeF7.

1 ACCEPTED SOLUTION

Accepted Solutions

It's not an error.

USBAEP for EP0 is always set to 1 in hardware, so it's a bit redundant to do anything with it.

EPENA enables *DATA* transfers, SETUP transfers are always enabled on EP0, so the non-DMA dataflow sets it only after decoding the SETUP packet, if it determines that OUT DATA stage is appropriate. This allows the non-DMA device to STALL unsupported SETUP transactions early, and potentially spare some precious Rx FIFO space and bus and processing time.

You probably can enable EPENA early, as you suggest, but then you have to make sure the rest of the stack can deal with the fact that data will arrive to EP0 unconditionally.

JW

View solution in original post

4 REPLIES 4

@Marek Kuziel​ - It would be great if you could explain when and how the original code creates improper operation,

Thanks,

Best Regards, Dave

Marek Kuziel
Associate II

@Dave Nadler​ Here we have whole function (original code):

/**
  * @brief  Prepare the EP0 to start the first control setup
  * @param  USBx  Selected device
  * @param  dma USB dma enabled or disabled
  *          This parameter can be one of these values:
  *           0 : DMA feature not used
  *           1 : DMA feature used
  * @param  psetup  pointer to setup packet
  * @retval HAL status
  */
HAL_StatusTypeDef USB_EP0_OutStart(USB_OTG_GlobalTypeDef *USBx, uint8_t dma, uint8_t *psetup)
{
  uint32_t USBx_BASE = (uint32_t)USBx;
  uint32_t gSNPSiD = *(__IO uint32_t *)(&USBx->CID + 0x1U);
 
  if (gSNPSiD > USB_OTG_CORE_ID_300A)
  {
    if ((USBx_OUTEP(0U)->DOEPCTL & USB_OTG_DOEPCTL_EPENA) == USB_OTG_DOEPCTL_EPENA)
    {
      return HAL_OK;
    }
  }
 
  USBx_OUTEP(0U)->DOEPTSIZ = 0U;
  USBx_OUTEP(0U)->DOEPTSIZ |= (USB_OTG_DOEPTSIZ_PKTCNT & (1U << 19));
  USBx_OUTEP(0U)->DOEPTSIZ |= (3U * 8U);
  USBx_OUTEP(0U)->DOEPTSIZ |=  USB_OTG_DOEPTSIZ_STUPCNT;
 
  if (dma == 1U)
  {
    USBx_OUTEP(0U)->DOEPDMA = (uint32_t)psetup;
    /* EP enable */
    USBx_OUTEP(0U)->DOEPCTL |= USB_OTG_DOEPCTL_EPENA | USB_OTG_DOEPCTL_USBAEP;
  }
 
  return HAL_OK;
}

It seems to me that in original code enable of EP occurs only when is selected DMA mode. It means that when dma == 0 then endpoint is not enabled at all.

Hence my suggestion to move line 33 after the dma condition (after line 34).

It's not an error.

USBAEP for EP0 is always set to 1 in hardware, so it's a bit redundant to do anything with it.

EPENA enables *DATA* transfers, SETUP transfers are always enabled on EP0, so the non-DMA dataflow sets it only after decoding the SETUP packet, if it determines that OUT DATA stage is appropriate. This allows the non-DMA device to STALL unsupported SETUP transactions early, and potentially spare some precious Rx FIFO space and bus and processing time.

You probably can enable EPENA early, as you suggest, but then you have to make sure the rest of the stack can deal with the fact that data will arrive to EP0 unconditionally.

JW

Yeah, good to read manuals...

Thanx Jan