Hello, Please see my comment on commit 48867208444cb2a82e2af9c3249e90b7ed4a1751 below.
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. > > The USB-driver ISR may set dev->status, I believe that is why the code is waiting for for a couple of ms. This patch makes the LEON3 UHCI support fail in usb_control_msg(), changing back to the old code the LEON3 UHCI driver (cpu/leon3/usb_uhci.c) works again. I believe this is also the case for board/MAI/AmigaOneG3SE/usb_uhci.c and board/mpl/common/usb_uhci.c, they are also modifying the flag from the interrupt handler. Best regards, Daniel Hellstrom >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 <[email protected]> >--- > common/usb.c | 56 > ++++++++++++++++++++++++------------------------- > drivers/usb/usb_ohci.c | 14 +++++------- > include/usb.h | 16 +++++++++++--- > 3 files changed, 47 insertions(+), 39 deletions(-) > >Index: u-boot-git-22092008/common/usb.c >=================================================================== >--- u-boot-git-22092008.orig/common/usb.c 2008-10-10 09:42:59.000000000 >+0200 >+++ u-boot-git-22092008/common/usb.c 2008-10-10 10:20:43.000000000 +0200 >@@ -196,15 +196,16 @@ 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; >- else >+ if (dev->status != 0) { >+ /* >+ * Let's wait a while for the timeout to elapse. >+ * It has no real use, but it keeps the interface happy. >+ */ >+ wait_ms(timeout); > return -1; >+ } >+ >+ return dev->act_len; > } > > /*------------------------------------------------------------------- >@@ -442,14 +443,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); >- if (result < 8) { >+ result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, buffer, 9); >+ if (result < 9) { > if (result < 0) > printf("unable to get descriptor, error %lX\n", > dev->status); > else > printf("config descriptor too short " \ >- "(expected %i, got %i)\n", 8, result); >+ "(expected %i, got %i)\n", 9, result); > return -1; > } > tmp = le16_to_cpu(config->wTotalLength); >@@ -777,7 +778,7 @@ int usb_new_device(struct usb_device *de > * Several USB stick devices report ERR: CTL_TIMEOUT, caused by an > * invalid header while reading 8 bytes as device descriptor. */ > dev->descriptor.bMaxPacketSize0 = 8; /* Start off at 8 bytes */ >- dev->maxpacketsize = 0; /* Default to 8 byte max packet size */ >+ dev->maxpacketsize = PACKET_SIZE_8; > dev->epmaxpacketin [0] = 8; > dev->epmaxpacketout[0] = 8; > >@@ -788,15 +789,12 @@ int usb_new_device(struct usb_device *de > return 1; > } > #else >- /* this is a Windows scheme of initialization sequence, with double >- * reset of the device (Linux uses the same sequence, but without double >- * reset. This double reset is not considered harmful and matches the >- * Windows behaviour) >+ /* This is a Windows scheme of initialization sequence, with double >+ * reset of the device (Linux uses the same sequence) > * Some equipment is said to work only with such init sequence; this > * patch is based on the work by Alan Stern: > * > http://sourceforge.net/mailarchive/forum.php?thread_id=5729457&forum_id=5398 > */ >- int j; > struct usb_device_descriptor *desc; > int port = -1; > struct usb_device *parent = dev->parent; >@@ -809,20 +807,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; > } >+ > dev->descriptor.bMaxPacketSize0 = desc->bMaxPacketSize0; > > /* find the port number we're at */ > if (parent) { >+ int j; > > for (j = 0; j < parent->maxchild; j++) { > if (parent->children[j] == dev) { >@@ -847,10 +847,10 @@ int usb_new_device(struct usb_device *de > dev->epmaxpacketin [0] = dev->descriptor.bMaxPacketSize0; > dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0; > switch (dev->descriptor.bMaxPacketSize0) { >- case 8: dev->maxpacketsize = 0; break; >- case 16: dev->maxpacketsize = 1; break; >- case 32: dev->maxpacketsize = 2; break; >- case 64: dev->maxpacketsize = 3; break; >+ case 8: dev->maxpacketsize = PACKET_SIZE_8; break; >+ case 16: dev->maxpacketsize = PACKET_SIZE_16; break; >+ case 32: dev->maxpacketsize = PACKET_SIZE_32; break; >+ case 64: dev->maxpacketsize = PACKET_SIZE_64; break; > } > dev->devnum = addr; > >Index: u-boot-git-22092008/include/usb.h >=================================================================== >--- u-boot-git-22092008.orig/include/usb.h 2008-10-10 09:42:59.000000000 >+0200 >+++ u-boot-git-22092008/include/usb.h 2008-10-10 09:43:01.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, >+}; > > 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 */ >- unsigned int toggle[2]; /* one bit for each endpoint ([0] = IN, >[1] = OUT) */ >- unsigned int halted[2]; /* endpoint halts; one bit per endpoint ># & direction; */ >+ /* Maximum packet size; one of: PACKET_SIZE_* */ >+ int maxpacketsize; >+ /* one bit for each endpoint ([0] = IN, [1] = OUT) */ >+ unsigned int toggle[2]; >+ /* endpoint halts; one bit per endpoint # & direction; */ >+ unsigned int halted[2]; > /* [0] = IN, [1] = OUT */ > int epmaxpacketin[16]; /* INput endpoint specific maximums */ > int epmaxpacketout[16]; /* OUTput endpoint specific maximums */ >Index: u-boot-git-22092008/drivers/usb/usb_ohci.c >=================================================================== >--- u-boot-git-22092008.orig/drivers/usb/usb_ohci.c 2008-10-10 >09:42:59.000000000 +0200 >+++ u-boot-git-22092008/drivers/usb/usb_ohci.c 2008-10-10 09:43:01.000000000 >+0200 >@@ -856,8 +856,7 @@ static void td_fill(ohci_t *ohci, unsign > td->index = index; > td->data = (__u32)data; > #ifdef OHCI_FILL_TRACE >- if ((usb_pipetype(urb_priv->pipe) == PIPE_BULK) && >- usb_pipeout(urb_priv->pipe)) { >+ if (usb_pipebulk(urb_priv->pipe) && usb_pipeout(urb_priv->pipe)) { > for (i = 0; i < len; i++) > printf("td->data[%d] %#2x ", i, ((unsigned char *)td->data)[i]); > printf("\n"); >@@ -983,7 +982,7 @@ static void dl_transfer_length(td_t *td) > tdBE = m32_swap(td->hwBE); > tdCBP = m32_swap(td->hwCBP); > >- if (!(usb_pipetype(lurb_priv->pipe) == PIPE_CONTROL && >+ if (!(usb_pipecontrol(lurb_priv->pipe) && > ((td->index == 0) || (td->index == lurb_priv->length - 1)))) { > if (tdBE != 0) { > if (td->hwCBP == 0) >@@ -1096,8 +1095,7 @@ static int takeback_td(ohci_t *ohci, td_ > dbg("dl_done_list: processing TD %x, len %x\n", > lurb_priv->td_cnt, lurb_priv->length); > >- if (ed->state != ED_NEW && >- (usb_pipetype(lurb_priv->pipe) != PIPE_INTERRUPT)) { >+ if (ed->state != ED_NEW && (!usb_pipeint(lurb_priv->pipe))) { > edHeadP = m32_swap(ed->hwHeadP) & 0xfffffff0; > edTailP = m32_swap(ed->hwTailP); > >@@ -1287,7 +1285,7 @@ pkt_print(NULL, dev, pipe, buffer, trans > #else > wait_ms(1); > #endif >- if ((pipe & PIPE_INTERRUPT) == PIPE_INTERRUPT) { >+ if (usb_pipeint(pipe)) { > info("Root-Hub submit IRQ: NOT implemented"); > return 0; > } >@@ -1538,7 +1536,7 @@ int submit_common_msg(struct usb_device > > /* allow more time for a BULK device to react - some are slow */ > #define BULK_TO 5000 /* timeout in milliseconds */ >- if (usb_pipetype(pipe) == PIPE_BULK) >+ if (usb_pipebulk(pipe)) > timeout = BULK_TO; > else > timeout = 100; >@@ -1591,7 +1589,7 @@ int submit_common_msg(struct usb_device > #endif > > /* free TDs in urb_priv */ >- if (usb_pipetype(pipe) != PIPE_INTERRUPT) >+ if (!usb_pipeint(pipe)) > urb_free_priv(urb); > return 0; > } > > > _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
