On 10:54 Wed 08 Oct , Remy Bohmer wrote:

The max packet size is encoded as 0,1,2,3 for 8,16,32,64 bytes. At some places directly 8,16,32,64 was used instead of the encoded value. Made a enum for the options to make this more clear and to help preventing similar errors in the future.

After fixing this bug it became clear that another bug existed where the 'pipe' is and-ed with PIPE_* flags, where it should have been 'usb_pipetype(pipe)', or even better usb_pipeint(pipe).

Also removed the triple 'get_device_descriptor' sequence, it has no use, and Windows nor Linux behaves that way. There is also a poll going on with a timeout when usb_control_msg() fails. However, the poll is useless, because the flag will never be set on a error, because there is no code that runs in a parallel that can set this flag. Changed this to something more logical.

Tested on AT91SAM9261ek and compared the flow on the USB bus to what Linux is doing. There is no difference anymore in the early initialisation sequence.

Signed-off-by: Remy Bohmer linux@bohmer.net

common/usb.c | 50 +++++++++++++++++++++++-------------------------- drivers/usb/usb_ohci.c | 14 +++++-------- include/usb.h | 16 ++++++++++++--- 3 files changed, 43 insertions(+), 37 deletions(-)

Index: u-boot-git-22092008/common/usb.c

--- u-boot-git-22092008.orig/common/usb.c 2008-10-08 10:51:49.000000000 +0200 +++ u-boot-git-22092008/common/usb.c 2008-10-08 10:51:50.000000000 +0200 @@ -196,15 +196,14 @@ int usb_control_msg(struct usb_device *d if (timeout == 0) return (int)size;

• while (timeout--) {

• if (!((volatile unsigned long)dev->status & USB_ST_NOT_PROC))
  

• 	break;
  

• wait_ms(1);
  

• } if (dev->status == 0) return dev->act_len;

please add {} to if too or remove the else

• else

• else {

• /* Let's wait a while for the timeout to elaps.
  

•  * it has no real use, but it keeps the interface happy. */
  

• wait_ms(timeout);
  

  return -1;
• }

}

/*------------------------------------------------------------------- @@ -442,14 +441,14 @@ int usb_get_configuration_no(struct usb_

config = (struct usb_config_descriptor *)&buffer[0];

• result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, buffer, 8); struct usb_device *parent = dev->parent;

@@ -809,20 +805,22 @@ int usb_new_device(struct usb_device *de

desc = (struct usb_device_descriptor *)tmpbuf; dev->descriptor.bMaxPacketSize0 = 64; /* Start off at 64 bytes */

• dev->maxpacketsize = 64; /* Default to 64 byte max packet size */

• /* Default to 64 byte max packet size */
• dev->maxpacketsize = PACKET_SIZE_64; dev->epmaxpacketin [0] = 64; dev->epmaxpacketout[0] = 64;

• for (j = 0; j < 3; ++j) {

• err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
  

• if (err < 0) {
  

• 	USB_PRINTF("usb_new_device: 64 byte descr\n");
  

• 	break;
  

• }
  

• err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
• if (err < 0) {

• USB_PRINTF("usb_new_device: usb_get_descriptor() failed\n");
  

• return 1;
  

  }

Index: u-boot-git-22092008/include/usb.h

--- u-boot-git-22092008.orig/include/usb.h 2008-10-08 10:51:45.000000000 +0200 +++ u-boot-git-22092008/include/usb.h 2008-10-08 10:51:50.000000000 +0200 @@ -129,6 +129,13 @@ struct usb_config_descriptor { struct usb_interface_descriptor if_desc[USB_MAXINTERFACES]; } __attribute__ ((packed));

+enum {

• /* Maximum packet size; encoded as 0,1,2,3 = 8,16,32,64 */
• PACKET_SIZE_8 = 0,
• PACKET_SIZE_16 = 1,
• PACKET_SIZE_32 = 2,
• PACKET_SIZE_64 = 3,

why not diectly the value?

+};

struct usb_device { int devnum; /* Device number on USB bus */ @@ -137,9 +144,12 @@ struct usb_device { char prod[32]; /* product */ char serial[32]; /* serial number */

• int maxpacketsize; /* Maximum packet size; encoded as 0,1,2,3 = 8,16,32,64 */

Best Regards, J.