[U-Boot] [PATCH v6] cmd: usb: ignore blk, emulation devices in usb tree/info display

[Marek Vasut]

On 10/19/2017 03:22 AM, Suneel Garapati wrote:
> usb tree/info commands iterate over all usb uclass devices
> recursively. blk uclass child devices are created for mass storage,
> treating these as usb uclass devices and referencing usb config
> interface descriptors cause crash. To fix, ignore blk and usb_emul
> uclass devices(sandbox)
                 ^^^^^^^ what's this about ? USB_EMUL devices can be
enables elsewhere too, right ?

Anyway, shouldn't you rather filter for positive matches (usb uclass
devices etc) , rather than filtering out a few negative matches (blk
etc) which might break in the future ?

> in usb_show_info and usb_tree_graph.
> also avoid addition of preamble for blk uclass child devices,
> otherwise tree dump gets messed up.

Also, sentences start with capital letter. This should be in a separate
patch if it's a separate change ...

> Signed-off-by: Suneel Garapati <suneelglinux at gmail.com>
> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> Tested-by: Bin Meng <bmeng.cn at gmail.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
> Changes v6:
>  - re-write commit message with detail
> Changes v5:
>  - add usb_emul to ignore list in usb_show_info
>  - modify description
> Changes v4:
>  - do not set preamble if child is block device for mass storage
> Changes v3:
>  - remove 'check on parent uclass' in description
> Changes v2:
>  - remove check on parent uclass
> Changes v1:
>  - add separate check on blk uclass
>  - modify description
>  - add separate check on parent uclass as usb
> 
>  cmd/usb.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/cmd/usb.c b/cmd/usb.c
> index d95bcf5..907debe 100644
> --- a/cmd/usb.c
> +++ b/cmd/usb.c
> @@ -349,6 +349,16 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
>  	printf(" %s", pre);
>  #ifdef CONFIG_DM_USB
>  	has_child = device_has_active_children(dev->dev);
> +	if (device_get_uclass_id(dev->dev) == UCLASS_MASS_STORAGE) {
> +		struct udevice *child;
> +
> +		for (device_find_first_child(dev->dev, &child);
> +		     child;
> +		     device_find_next_child(&child)) {
> +			if (device_get_uclass_id(child) == UCLASS_BLK)
> +				has_child = 0;
> +		}
> +	}
>  #else
>  	/* check if the device has connected children */
>  	int i;
> @@ -414,8 +424,12 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
>  
>  		udev = dev_get_parent_priv(child);
>  
> -		/* Ignore emulators, we only want real devices */
> -		if (device_get_uclass_id(child) != UCLASS_USB_EMUL) {
> +		/*
> +		 * Ignore emulators and block child devices, we only want
> +		 * real devices
> +		 */
> +		if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
> +		    (device_get_uclass_id(child) != UCLASS_BLK)) {
>  			usb_show_tree_graph(udev, pre);
>  			pre[index] = 0;
>  		}
> @@ -605,7 +619,9 @@ static void usb_show_info(struct usb_device *udev)
>  	for (device_find_first_child(udev->dev, &child);
>  	     child;
>  	     device_find_next_child(&child)) {
> -		if (device_active(child)) {
> +		if (device_active(child) &&
> +		    (device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
> +		    (device_get_uclass_id(child) != UCLASS_BLK)) {
>  			udev = dev_get_parent_priv(child);
>  			usb_show_info(udev);
>  		}
> 


-- 
Best regards,
Marek Vasut