apache
usbdev: composite: update USB_REQ_SETCONFIGURATION logic
Summary
DO NOT MERGE AS IT
discussion about composite driver behaviour. I cannot get composite driver to work using multiple USB devices without this on stm32.
Impact
Breaks all the usbdev drivers.
Testing
Can someone help me on this topic ? Should I implement the fix in composite_ep0submit() instead ?
Can someone help me on this topic ? Should I implement the fix in composite_ep0submit() instead ?
Hi Simon, could you please describe more clearly the issue you were facing and why do you implement this way?
Looking the code it appears that you modified the set configuration to reset the previous set device case an error happening setting a device. But your comment about endpoint 0 (EP0) shouldn't be set is not clear. Do you have some point for the spec that explain about it?
Can someone help me on this topic ? Should I implement the fix in composite_ep0submit() instead ?
Hi Simon, could you please describe more clearly the issue you were facing and why do you implement this way?
Looking the code it appears that you modified the set configuration to reset the previous set device case an error happening setting a device. But your comment about endpoint 0 (EP0) shouldn't be set is not clear. Do you have some point for the spec that explain about it?
The issue occurs when the host sends USB_REQ_SETCONFIGURATION to select a USB configuration on NuttX device. The current behaviour is that the USB_REQ_SETCONFIGURATION is forwarded to the usb device drivers (CLASS_SETUP()), with is good. The issue is that all the device drivers will then submit the result to EP0. This leads to a situation where you are trying to submit multiple times on EP0, But the host PC only expects one reply to know if the USB_REQ_SETCONFIGURATION request is ok or not. On stm32, multiple calls to EP0_SUBMIT leads to memory corruption. I did not try on other devices.
The solution would be to check the return value of CLASS_SETUP() calls in composite driver for each USB_REQ_SETCONFIGURATION, and let the composite driver manage the EP0_SUBMIT in the end.
@spiriou do we still need this patch?
@xiaoxiang781216 It has been 2 years without anyone else reporting usbdev composite problems so either there is no problem, or no one uses this feature.
The problem I had is still the same and not fixed, see description above. To me usbdev composite with more than 1 device is broken and cannot work because composite driver expects at least one usbdev driver to call EP_SUBMIT(dev->ep0, ctrlreq) when handling USB_REQ_SETCONFIGURATION (see dispatched = true; in this PR). This leads to all the usbdev under the composite device calling EP_SUBMIT(dev->ep0, ctrlreq) where usb host expects only one reply, leading to corruption.
The complexity in this fix is that all the usbdev drivers must be updated to not send USB_REQ_SETCONFIGURATION reply when calling CLASS_SETUP() if usbdev composite is enabled, which requires testing.
Ok, we will use USB in the next project, let's see what happen. @zhuyanlin111 please pay attention.