2020-07-13 04:08 AM
Hi! I have several questions regarding standard USB host implementation.
1) Host mode: File "stm32f4xx_ll_usb.c", function "USB_HC_StartXfer" there is a piece of code
if (dma == 0U) /* Slave mode */
{
if ((hc->ep_is_in == 0U) && (hc->xfer_len > 0U))
{
switch (hc->ep_type)
{
/* Non periodic transfer */
case EP_TYPE_CTRL:
case EP_TYPE_BULK:
len_words = (uint16_t)((hc->xfer_len + 3U) / 4U);
/* check if there is enough space in FIFO space */
if (len_words > (USBx->HNPTXSTS & 0xFFFFU))
{
/* need to process data in nptxfempty interrupt */
USBx->GINTMSK |= USB_OTG_GINTMSK_NPTXFEM;
}
break;
/* Periodic transfer */
case EP_TYPE_INTR:
case EP_TYPE_ISOC:
len_words = (uint16_t)((hc->xfer_len + 3U) / 4U);
/* check if there is enough space in FIFO space */
if (len_words > (USBx_HOST->HPTXSTS & 0xFFFFU)) /* split the transfer */
{
/* need to process data in ptxfempty interrupt */
USBx->GINTMSK |= USB_OTG_GINTMSK_PTXFEM;
}
break;
default:
break;
}
/* Write packet into the Tx FIFO. */
(void)USB_WritePacket(USBx, hc->xfer_buff, hc->ch_num, (uint16_t)hc->xfer_len, 0);
}
}
It is supposed to check if there is a space available in TXFIFO, but it writes the data regardless of free space. Moreover, it unmasks the interrupt for NPTXFIFO/PTXFIFO to deal with it later. But the interrupt handler "HAL_HCD_IRQHandler" doesn't give a care about those interrupts, NPTXE isn't even cleared:
if (__HAL_HCD_GET_FLAG(hhcd, USB_OTG_GINTSTS_PTXFE))
{
/* Incorrect mode, acknowledge the interrupt */
__HAL_HCD_CLEAR_FLAG(hhcd, USB_OTG_GINTSTS_PTXFE);
}
Am I missing something here?
2) General: What the point of masking + unmasking an interrupt inside an interrupt handler?
USB_MASK_INTERRUPT(hhcd->Instance, USB_OTG_GINTSTS_RXFLVL);
HCD_RXQLVL_IRQHandler(hhcd);
USB_UNMASK_INTERRUPT(hhcd->Instance, USB_OTG_GINTSTS_RXFLVL);
3) Device mode: Endpoint address is masked with 0x7F instead of 0xF inside generated file usbd_conf.c
uint8_t USBD_LL_IsStallEP(USBD_HandleTypeDef *pdev, uint8_t ep_addr)
{
PCD_HandleTypeDef *hpcd = (PCD_HandleTypeDef*) pdev->pData;
if((ep_addr & 0x80) == 0x80)
{
return hpcd->IN_ep[ep_addr & 0x7F].is_stall; // 7f error!
}
else
{
return hpcd->OUT_ep[ep_addr & 0x7F].is_stall; // 7f error!
}
4) Device: In StdRequest handler device's address is set before the status is sent to host:
pdev->dev_address = dev_addr;
(void)USBD_LL_SetUSBAddress(pdev, dev_addr);
(void)USBD_CtlSendStatus(pdev);
Is it okay?
5) There are a lot of copy pasted comments which doesn't make any sense. There are even wrong ones, suggesting to use "15" (0xF) value to flush all EPs/channels, although the correct one is "16" (0x10).
6) There are some minor bugs found everywhere in code, although i didn't noted all of them.
7) Am i using an outdated software package? My version is STM32Cube_FW_F4_V1.25.0
2020-07-13 11:11 PM
2 is probably artefact from development which might run partially outside the interrupt; in 3 it doesn't matter as ep_addr is never supposed to have bits 4 to 6 set. 4 is probably okay, that's what all the documentation says and it works; the Synopsys OTG module is incredibly wasteful in some of the most irrelevant details and then inother important things it forces the user to jump through hoops so that they could save a few gates; it's an organically grown *** rather than a properly engineered module. The rest I don't know and I don't care.
As you've might already noticed, the provided code is not the clearest, Cube/HAL adds to the unnecessary entanglement (and the SPL version is not any better, you may want to have a look at it). This all nicely complements the *** documentation. Read the licence - there's no guarantee there.
There are 3rd party stacks out there, or write your own.
JW
2020-07-14 01:01 AM
Tnx for your answers. I'm not an experienced developer, writing my own implementation of USB stuff is an impossible task - I'm struggle to understand how this peripheral should work. Reading the reference manual doesn't make any better, but rather worse.
As for #4, USB specification says the device address must be changed after the status is acknowledged by the host, but yes, it seems to be OK nevertheless.
Answering my own #1 question, this weirdness is probably due the code reentrancy issues multiplied by laziness, I'm not sure why it is impossible to mask some interrupts here when the FIFO is written from outside of interrupt. ST's host stack is mostly FSM-driven-from-a-loop, unlike the device one - it process data i/o inside an interrupt handler.
I imagine in current implementation, if you try to write data which can't fit into the NPTXFIFO, it will just silently stuck inside an IRQ handler never clearing the NPTXFE interrupt. It must be a bug =\
2020-07-14 02:26 AM
> writing my own implementation of USB stuff is an impossible task
That's your decision. You may use the ST implementation - or resort to the older SPL-based one, or use some of the open source out there (chibios for example has one, but it's of course bound to chibios itself)- and then endure any consequences, including debugging it at times. Note, that you've already penetrated it at places maybe deeper than those who wrote that.
> Reading the reference manual doesn't make any better, but rather worse.
Yes, that's what I was talking about above. I can't stop using expletives whenever it comes to the synopsys dwc otg module. It's only partially ST's fault - they purchased the IP from Synopsys together with the accompanying "documentation". Other chips/manufacturers using the same IP have the same "documentation", too.
> As for #4, USB specification says the device address must be changed after the status is acknowledged by the host,
Yes, and it indeed does so. That's why I wrote the module is wasteful - there's obviously extra logic which takes the newly written address into account only after the status handshake is accomplished.
JW
2020-07-14 07:42 AM
Interestingly, SPL handles both interrupts from question #1:
static uint32_t USB_OTG_USBH_handle_nptxfempty_ISR (USB_OTG_CORE_HANDLE *pdev)
{
USB_OTG_GINTMSK_TypeDef intmsk;
USB_OTG_HNPTXSTS_TypeDef hnptxsts;
uint16_t len_words , len;
hnptxsts.d32 = USB_OTG_READ_REG32(&pdev->regs.GREGS->HNPTXSTS);
len_words = (pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_len + 3) / 4;
while ((hnptxsts.b.nptxfspcavail > len_words)&&
(pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_len != 0))
{
len = hnptxsts.b.nptxfspcavail * 4;
if (len > pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_len)
{
/* Last packet */
len = pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_len;
intmsk.d32 = 0;
intmsk.b.nptxfempty = 1;
USB_OTG_MODIFY_REG32( &pdev->regs.GREGS->GINTMSK, intmsk.d32, 0);
}
len_words = (pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_len + 3) / 4;
USB_OTG_WritePacket (pdev , pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_buff, hnptxsts.b.nptxqtop.chnum, len);
pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_buff += len;
pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_len -= len;
pdev->host.hc[hnptxsts.b.nptxqtop.chnum].xfer_count += len;
hnptxsts.d32 = USB_OTG_READ_REG32(&pdev->regs.GREGS->HNPTXSTS);
}
return 1;
}
Although, if i'm getting it right, there would be a lot of problems with SPL implementation due to race conditions. Is there any alternative to stm32f407 having two USB ports capable of running in host an device modes? Sorry for bothering you)
2020-07-14 08:49 AM
> Is there any alternative to stm32f407 having two USB ports capable of running in host an device modes?
I don't understand your question. All OTG modules in STM32 are variants (various versions in time, with undocumented changes) of the same Synopsys DWC OTG IP, so all STM32 which have two OTG (upper-end 'F4, 'F7, 'H7) have two USB ports, of which each can be either host or device (or OTG).
JW
2020-07-14 08:52 AM
I mean MCU, which has a clear documentation and less buggy software, maybe other manufacturer.
2020-07-14 01:00 PM
Ah, no, I don't know, I don't research these.
JW
2020-07-14 03:31 PM
You may want to look at this project and join the development: https://github.com/hathach/tinyusb
-- pa