[U-Boot] [Bug Report] USB: descriptor handling

[Stefan Althoefer <stefan.althoefer@web.de>]

Hi,

I found a bug when working with the u-boot USB subsystem on IXP425 processor
(big endian Xscale aka ARMv5).
I recognized that the second usb_endpoint_descriptor of the attached memory
stick was corrupted.

The reason for this are the packed structures below (either u-boot and
u-boot-usb):

--------------
/* Endpoint descriptor */
struct usb_endpoint_descriptor {
	unsigned char  bLength;
	unsigned char  bDescriptorType;
	unsigned char  bEndpointAddress;
	unsigned char  bmAttributes;
	unsigned short wMaxPacketSize;
	unsigned char  bInterval;
	unsigned char  bRefresh;
	unsigned char  bSynchAddress;

} __attribute__ ((packed));
/* Interface descriptor */
struct usb_interface_descriptor {
	unsigned char  bLength;
	unsigned char  bDescriptorType;
	unsigned char  bInterfaceNumber;
	unsigned char  bAlternateSetting;
	unsigned char  bNumEndpoints;
	unsigned char  bInterfaceClass;
	unsigned char  bInterfaceSubClass;
	unsigned char  bInterfaceProtocol;
	unsigned char  iInterface;

	unsigned char  no_of_ep;
	unsigned char  num_altsetting;
	unsigned char  act_altsetting;
	struct usb_endpoint_descriptor ep_desc[USB_MAXENDPOINTS];
} __attribute__ ((packed));
------------

As usb_endpoint_descriptor is only 7byte in length, the start of all
odd ep_desc[] structures is not word aligned. This makes wMaxPacketSize
of these structures also not word aligned.

ARMv5 Architecture however does not support non-aligned multibyte
data type (see A2.8 of ARM Architecture Reference Manual).


--- this piece of code in usb.c -------
		case USB_DT_ENDPOINT:
			epno = dev->config.if_desc[ifno].no_of_ep;
			/* found an endpoint */
			{ int ii;
				USB_PRINTF("EPD(%d): ", buffer[index]);
				for(ii=0; ii<buffer[index]; ii++){
					USB_PRINTF("%.2x ", buffer[index+ii]);
				}
				USB_PRINTF("\n");
			}
			dev->config.if_desc[ifno].no_of_ep++;
			memcpy(&dev->config.if_desc[ifno].ep_desc[epno],
				&buffer[index], buffer[index]);
			le16_to_cpus(&(dev->config.if_desc[ifno].ep_desc[epno].\
							       wMaxPacketSize));
			{ int ii;
				USB_PRINTF("EPD(%d): ", buffer[index]);
				for(ii=0; ii<buffer[index]; ii++){
					USB_PRINTF("%.2x ",*((unsigned char *)(&dev->config.if_desc[ifno].ep_desc[epno])+ii));
				}
				USB_PRINTF("\n");
			}
			USB_PRINTF("if %d, ep %d\n", ifno, epno);
			break;
----and following output (shortened) ---
<first descriptor>
EPD(7): 07 05 81 02 40 00 00    <- before endian swapping
EPD(7): 07 05 81 02 00 40 00    <- after endian swapping
<second descriptor>
EPD(7): 07 05 02 02 40 00 00
EPD(7): 07 05 02 02 00 40 00
-------

It seems that the actual ARM implementation does support reading of non-aligned
words (else USB would never have run on ARM) but writing is
definitely not. You see the data of the second descriptor garbled.
As writing is only needed on big endian ARM (for data swapping)
this issue has been undetected up to now.

Either wMaxPacket size must be accessed with special code, or it
must be aligned to modulo-2 address.

I suggest:

------------
diff --git a/include/usb.h b/include/usb.h
index 9a2e72c..b36272b 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -93,8 +93,7 @@ struct usb_endpoint_descriptor {
        unsigned char  bInterval;
        unsigned char  bRefresh;
        unsigned char  bSynchAddress;
-
-} __attribute__ ((packed));
+} __attribute__ ((packed))  __attribute__ ((aligned(2)));
 /* Interface descriptor */
 struct usb_interface_descriptor {
        unsigned char  bLength;
------------

With this change memory stick is identied correctly. I can do "fatls",
but I cannot read data correctly from the disk. But might be something
completely different issue ....

--- Stefan