[U-Boot] [PATCH] cmd_usb.c: implemented standard subcommand handling

[Detlev Zundel <dzu@denx.de>]

Hi Frans,

> also added a missing \n in the help messages
>
> Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
> ---
>  common/cmd_usb.c |  391 ++++++++++++++++++++++++++++++++----------------------
>  1 files changed, 233 insertions(+), 158 deletions(-)
>
> as discussed before on the mailing list
> tested on sheevaplug
>
> diff --git a/common/cmd_usb.c b/common/cmd_usb.c
> index 9de515c..ef80d78 100644
> --- a/common/cmd_usb.c
> +++ b/common/cmd_usb.c
> @@ -502,199 +502,274 @@ int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  }
>  #endif /* CONFIG_USB_STORAGE */
>  
> +static int do_usb_reset(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
> +{
> +	int i;
>  
> -/******************************************************************************
> - * usb command intepreter
> - */
> -int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> +	usb_stop();
> +	printf("(Re)start USB...\n");
> +	i = usb_init();
> +#ifdef CONFIG_USB_STORAGE
> +	/* try to recognize storage devices immediately */
> +	if (i >= 0)
> +		usb_stor_curr_dev = usb_stor_scan(1);
> +#endif
> +	return 0;
> +}
> +
> +extern char usb_started;
> +
> +static int do_usb_tree(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  {
> +	if (!usb_started) {
> +		printf("USB is stopped. Please issue 'usb start' first.\n");
> +		return 1;
> +	}

I really wonder about this check.  If we know what to do in order to
proceed, why why don't we issue the command automatically?

Also this code now repeats a lot of times, so it should be factored out
into a function - what about "ensure_usb_started"?

> +	printf("\nDevice Tree:\n");
> +	usb_show_tree(usb_get_dev_index(0));
> +	return 0;
> +}
>  
> +static int do_usb_info(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
> +{
> +	int d;
>  	int i;
>  	struct usb_device *dev = NULL;
> -	extern char usb_started;
> -#ifdef CONFIG_USB_STORAGE
> -	block_dev_desc_t *stor_dev;
> -#endif
>  
> -	if ((strncmp(argv[1], "reset", 5) == 0) ||
> -		 (strncmp(argv[1], "start", 5) == 0)) {
> -		usb_stop();
> -		printf("(Re)start USB...\n");
> -		i = usb_init();
> -#ifdef CONFIG_USB_STORAGE
> -		/* try to recognize storage devices immediately */
> -		if (i >= 0)
> -			usb_stor_curr_dev = usb_stor_scan(1);
> -#endif
> +	if (!usb_started) {
> +		printf("USB is stopped. Please issue 'usb start' first.\n");
> +		return 1;
> +	}
> +
> +	if (argc == 1) {
> +		for (d = 0; d < USB_MAX_DEVICE; d++) {
> +			dev = usb_get_dev_index(d);
> +			if (dev == NULL)
> +				break;
> +			usb_display_desc(dev);
> +			usb_display_config(dev);
> +		}
>  		return 0;
> +	} else {
> +		i = simple_strtoul(argv[1], NULL, 16);
> +		printf("config for device %d\n", i);
> +		for (d = 0; d < USB_MAX_DEVICE; d++) {
> +			dev = usb_get_dev_index(d);
> +			if (dev == NULL)
> +				break;
> +			if (dev->devnum == i)
> +				break;
> +		}
> +		if (dev == NULL) {
> +			printf("*** NO Device avaiable ***\n");

Ouch, this message looks strange ;)

> +			return 0;
> +		} else {
> +			usb_display_desc(dev);
> +			usb_display_config(dev);
> +		}
>  	}
> -	if (strncmp(argv[1], "stop", 4) == 0) {
> +	return 0;
> +}
> +
> +#if defined(CONFIG_USB_STORAGE)
> +static int do_usb_stop(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
> +{

Why do we define usb_stop only if USB_STORAGE is defined?  What if
USB_KEYBOARD is defined but not USB_STORAGE?

Cheers
  Detlev

-- 
"The number you have dialed is imaginary. Please rotate your phone 90
degrees and try again."
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de