stm32l4xx_hal_pcd incorrect setup request handling

[LeoG1]

Hi,

It tooke me several days to found that there is wrong handling of setup packet with data in HAL driver.

On windows I'm sending setup packet like this:

uint32_t data = 0xAABBCCDD;

libusb_control_transfer(mDeviceHandle, LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_VENDOR | LIBUSB_RECIPIENT_DEVICE, 0xAA, 0, 0, &data, 4, 0);

Later in hal driver in PCD_EP_ISR_Handler function there is check for status bits:

wEPVal = PCD_GET_ENDPOINT(hpcd->Instance, PCD_ENDP0);
if ((wEPVal & USB_EP_SETUP) != 0U) {
...
} else
if ((wEPVal & USB_EP_CTR_RX) != 0U)
{
...
}

Which leads to:

1. receiving setup packet without data
2. receving data without setup fields

Additionally in stm32l4xx_ll_usb.c driver in USB_EPStartXfer function

There is handling for IN endpoint (call to USB_WritePMA) but there is no handling for OUT endpoint - no call to

USB_ReadPMA

[waclawek.jan]

To put this into context: this is about the device-only USB found in the lower-end 'L4 (i.e. not the OTG USB in higher-end 'L4 and 'L4+).

Can you please elaborate, what exactly is wrong with the code and what sequence of events (USB packets) leads to an error? Receiving SETUP packets and then OUT packets is normal within a Control transfer, as outlined in chapter 8.5.3 of USB2.0.

JW

[LeoG1]

---

@waclawek.jan wrote:

To put this into context: this is about the device-only USB found in the lower-end 'L4 (i.e. not the OTG USB in higher-end 'L4 and 'L4+).

Can you please elaborate, what exactly is wrong with the code and what sequence of events (USB packets) leads to an error? Receiving SETUP packets and then OUT packets is normal within a Control transfer, as outlined in chapter 8.5.3 of USB2.0.

JW

---

Problem as I see it is when HAL receives SETUP packet it forwards it to upper level hanlers without any data, only request fields filled.

Later indeed IRQ hanler receives data from SETUP packet but it doesn't forward it to class handlers.

For the second I thought that it's ok and I can force to read data from EP0 when I received setup request with wLength > 0 by calling USBD_CtlPrepareRx, but down the chain USB_EPStartXfer also ignores data.

[waclawek.jan]

I don't use Cube so I don't know the intended logic of downstream control transfers in Cube, and I am not interested in investigating it and improving it.

There are no standard (chapter 9.4) requests which have downstream data except SYNCH_FRAME, which is used as feedback for isochronous connections,  but AFAIK there's no such example in Cube (well, also SET_CONFIGURATION, which is optional and probably never implemented anywhere). So, the developers of Cube probably never felt this to be needed.

You should perhaps see Cube more as a set of ad-hoc examples than a concise library providing all possible use cases.

JW

@Amel NASRI, maybe this is worth discussing with the team which does USB in Cube?

[LeoG1]

Well.. indeed I'm talking about "vendor" requests. But USB supports such transfers and I was sure STM33 drivers do as well.

I think this problem is part of more general one - to lack of support for custom "vendor" class in Cube. While STM32 is widely used by DIY developers it would be nice to have control of device through USB channel. E.g. to control GPIO, or read write I2C/SPI buses etc... 

Anyway if somebody want's take care of this issue, I'm more than happy to contribute

[waclawek.jan]

From reading the Cube sources it appears, that the higher-level parts of protocol are supposed to be handled through HAL_PCD_SetupStageCallback()/HAL_PCD_DataOutStageCallback() which are upon you to implement.

The Cube examples (in Projects directory) these callbacks call USBD_LL_SetupStage() and USBD_LL_DataOutStage(), which are implemented in the Middleware and appear to handle the higher-level portions of the Control Transfer - after receiving all data, finishing by calling pdev->pClass->EP0_RxReady(pdev).

Again, I'm not interested in penetrating the Cube's stack's logic, but maybe this is the direction where you should look.

JW

PS. UM1734 page 19 appears to be relevant for this thread.

[MWB_CHa]

Thanks @waclawek.jan , I wouldn't answer differently.