On 04/07/2015 11:07 PM, Stephen Warren wrote:
On 04/04/2015 07:12 AM, Paul Kocialkowski wrote:
This may happen when using an USB1 device on a controller that only supports USB2 (e.g. EHCI). Reading the first descriptor will fail (read 0 byte), so we can abort the process at this point instead of failing later and wasting time.
FYI, this patch breaks USB keyboard (or perhaps any USB 1.x device) support on the RPi.
diff --git a/common/usb.c b/common/usb.c
@@ -956,7 +956,7 @@ int usb_new_device(struct usb_device *dev) */ #ifndef CONFIG_USB_XHCI err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
- if (err < 0) {
- if (err < sizeof(struct usb_device_descriptor)) { debug("usb_new_device: usb_get_descriptor() failed\n"); return -EIO; }
The value returned here is 8 not 18 (the sizeof the descriptor), so the code bails out with an error.
Given the description of what this patch is attempting to achieve, shouldn't the replacement check be:
if (err <= 0) {
My guess for why the value 8 is returned is because the device's maxpacket is 8, and the DWC2 driver only attempts to transfer 1 packet because the requested transfer size is 64, and the default packet size is 64, which means 1 packet. Note that DWC2 HW (perhaps unlike e.g. EHCI) requires the driver to specify the number of packets transferred, not a byte count. However, I haven't validated that yet. My device is a USB 1.x FS keyboard.
Yes, I can avoid the error by hacking dwc2.c's assignment of max:
int chunk_msg(struct usb_device *dev, unsigned long pipe, int *pid, int in, void *buffer, int len, bool ignore_ack) {
...
int max = usb_maxpacket(dev, pipe);
and forcing that to 8 rather than 64 for the data transfer phase of the first transaction to each USB device (my keyboard has a built-in hub, so I need to override a few different transactions).
Thinking about the fix more, perhaps something like the following is appropriate:
if (err < offsetof(struct usb_device_descriptor, idVendor))
... since idVendor is the first field that's not used by the code that processes the results of the initial descriptor read. Hopefully there are no devices with a control endpoint bMaxPacketSize0 less than 8 (which is what that offsetof evaluates to). http://www.usbmadesimple.co.uk/ums_3.htm seems to imply that's the case, saying for control transfers:
The max packet size for the data stage is 8 bytes at low speed, 8, 16, 32 or 64 at full Speed and 64 for high speed.
(although one of my keyboards has a maxPacket of 4 for EP 2, but I thinkt hat's something different)
Also note the comment a little before the code this patch affects in U-Boot's common/usb.c:
/* send 64-byte GET-DEVICE-DESCRIPTOR request. Since the descriptor is * only 18 bytes long, this will terminate with a short packet. But if * the maxpacket size is 8 or 16 the device may be waiting to transmit * some more, or keeps on retransmitting the 8 byte header. */