[U-Boot] [PATCH v1] cmd: usb: add blk devices to ignore list in tree graph

[U-Boot] [PATCH v1] cmd: usb: add blk devices to ignore list in tree graph

[Suneel Garapati]

add blk child devices to ignore list while displaying
usb tree graph, otherwise usb tree and info commands
may cause crash treating blk as usb device.

Signed-off-by: Suneel Garapati <suneelglinux@gmail.com>
---

Changes v1:
 - add separate check on blk uclass
 - modify description
 - add separate check on parent uclass as usb

 cmd/usb.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/cmd/usb.c b/cmd/usb.c
index d95bcf5..cb0e183 100644
--- a/cmd/usb.c
+++ b/cmd/usb.c
@@ -414,8 +414,13 @@ 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, with check on
+		 * parent uclass as usb, we only want real devices
+		 */
+		if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
+		    (device_get_uclass_id(child) != UCLASS_BLK) &&
+		    (device_get_uclass_id(child->parent) == UCLASS_USB)) {
 			usb_show_tree_graph(udev, pre);
 			pre[index] = 0;
 		}
@@ -605,7 +610,8 @@ 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_BLK)) {
 			udev = dev_get_parent_priv(child);
 			usb_show_info(udev);
 		}
-- 
2.7.4

[U-Boot] [PATCH v1] cmd: usb: add blk devices to ignore list in tree graph

[Simon Glass]

On 6 September 2017 at 11:01, Suneel Garapati <suneelglinux@gmail.com> wrote:
> add blk child devices to ignore list while displaying
> usb tree graph, otherwise usb tree and info commands
> may cause crash treating blk as usb device.
>
> Signed-off-by: Suneel Garapati <suneelglinux@gmail.com>
> ---
>
> Changes v1:
>  - add separate check on blk uclass
>  - modify description
>  - add separate check on parent uclass as usb
>
>  cmd/usb.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v1] cmd: usb: add blk devices to ignore list in tree graph

[Bin Meng]

Hi Suneel,

On Mon, Sep 11, 2017 at 2:17 PM, Simon Glass <sjg@chromium.org> wrote:
> On 6 September 2017 at 11:01, Suneel Garapati <suneelglinux@gmail.com> wrote:
>> add blk child devices to ignore list while displaying
>> usb tree graph, otherwise usb tree and info commands
>> may cause crash treating blk as usb device.
>>
>> Signed-off-by: Suneel Garapati <suneelglinux@gmail.com>
>> ---
>>
>> Changes v1:
>>  - add separate check on blk uclass
>>  - modify description
>>  - add separate check on parent uclass as usb
>>
>>  cmd/usb.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

'usb tree' is seriously broken with this patch. See below.

=> usb tree
USB device tree:
  1  Hub (5 Gb/s, 0mA)
  |  U-Boot XHCI Host Controller
  |

Regards,
Bin

[U-Boot] [PATCH v1] cmd: usb: add blk devices to ignore list in tree graph

[Suneel Garapati]

Hi Bin,

On Thu, Sep 14, 2017 at 11:27 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Suneel,
>
> On Mon, Sep 11, 2017 at 2:17 PM, Simon Glass <sjg@chromium.org> wrote:
>> On 6 September 2017 at 11:01, Suneel Garapati <suneelglinux@gmail.com> wrote:
>>> add blk child devices to ignore list while displaying
>>> usb tree graph, otherwise usb tree and info commands
>>> may cause crash treating blk as usb device.
>>>
>>> Signed-off-by: Suneel Garapati <suneelglinux@gmail.com>
>>> ---
>>>
>>> Changes v1:
>>>  - add separate check on blk uclass
>>>  - modify description
>>>  - add separate check on parent uclass as usb
>>>
>>>  cmd/usb.c | 12 +++++++++---
>>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> 'usb tree' is seriously broken with this patch. See below.
>
> => usb tree
> USB device tree:
>   1  Hub (5 Gb/s, 0mA)
>   |  U-Boot XHCI Host Controller
>   |

Yeah, I see that too. Looks like the parent uclass could be
USB,USB_HUB/MASS_STORAGE/DEV_GENERIC.
Simon suggested this check for robustness although
usb_for_each_root_dev calls on only UCLASS_USB devices.

Well, either code can check on all of those or don't check at all.
                if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
                    (device_get_uclass_id(child) != UCLASS_BLK) &&
                    ((device_get_uclass_id(child->parent) == UCLASS_USB) ||
                    (device_get_uclass_id(child->parent) == UCLASS_USB_HUB) ||
                    (device_get_uclass_id(child->parent) ==
UCLASS_USB_DEV_GENERIC) ||
                    (device_get_uclass_id(child->parent) ==
UCLASS_MASS_STORAGE))) {

I would prefer someone to take a call.

Regards,
Suneel

>
> Regards,
> Bin