[U-Boot] [patch 1/2] fix USB initialisation procedure

[Jean-Christophe PLAGNIOL-VILLARD]

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 at 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.