cmd: usb: add blk devices to ignore list in tree graph

PATCH New Suneel Garapati 3 participants 4 messages Sep 6, 2017 – Sep 15, 2017

Share & tools

Suneel Garapati

author

2017-09-06 17:01 UTC #0 lore raw

PATCH 1/1 cmd: usb: add blk devices to ignore list in tree graph

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 <[email protected]>

---

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(-)

cmd/usb.c +9 -3

index d95bcf5..cb0e183 100644
@@ -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);
 		}

Signature · 1 line

2.7.4

Simon Glass

2017-09-11 06:17 UTC #1 lore raw

PATCH 1/1 cmd: usb: add blk devices to ignore list in tree graph

Quoted 7 lines · 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 <[email protected]>
---

Quoted 7 lines

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 <[email protected]>

Bin Meng

2017-09-15 06:27 UTC #2 lore raw

PATCH 1/1 cmd: usb: add blk devices to ignore list in tree graph

Hi Suneel,

Quoted 13 lines · Simon Glass

Quoted 12 lines · 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 <[email protected]>
---
Changes v1:
- add separate check on blk uclass
- modify description
- add separate check on parent uclass as usb

Quoted 4 lines

Quoted 2 lines
 cmd/usb.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
Reviewed-by Simon Glass <[email protected]>

'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

Suneel Garapati

author

2017-09-15 19:27 UTC #3 lore raw

PATCH 1/1 cmd: usb: add blk devices to ignore list in tree graph

Hi Bin,

Quoted 21 lines · Bin Meng

Hi Suneel,
Quoted 13 lines · Simon Glass
Quoted 12 lines · 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 <[email protected]>
---
Changes v1:
- add separate check on blk uclass
- modify description
- add separate check on parent uclass as usb
Quoted 4 lines
Quoted 2 lines
 cmd/usb.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Quoted 7 lines

'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

Quoted 3 lines

Regards,
Bin