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

PATCH Superseded Suneel Garapati 2 participants 7 messages Sep 19, 2017 – Sep 21, 2017

Share & tools

Suneel Garapati

author

2017-09-19 05:55 UTC #0 lore raw

PATCH v3 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 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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

cmd/usb.c +8 -3

index d95bcf5..3889994 100644
@@ -414,8 +414,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 +609,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

Bin Meng

2017-09-19 07:32 UTC #1 lore raw

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

Hi Suneel,

Quoted 9 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 v3:
- remove 'check on parent uclass' in description

thanks for making the changes.

Quoted 30 lines

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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

cmd/usb.c +6 -2

index d95bcf5..3889994 100644
@@ -414,8 +414,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;
                }

cmd/usb.c static void usb_show_info(struct usb_device *udev)

Quoted 10 lines

cmd/usb.c +2 -2

@@ -605,7 +609,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);
                }
--

My testing of 'usb info' looks OK, however 'usb tree' still has some issues below:

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

  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | |
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | |

As you see, we just don't print out the BLK device, but we still print out the | here.

  | +-7  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard
  |
  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |

And here.

Regards,
Bin

Suneel Garapati

author

2017-09-19 18:31 UTC #2 lore raw

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

Hi Bin,

Quoted 64 lines · Bin Meng

Hi Suneel,

Quoted 9 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 v3:
- remove 'check on parent uclass' in description

thanks for making the changes.

Quoted 30 lines

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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

cmd/usb.c +6 -2

index d95bcf5..3889994 100644
@@ -414,8 +414,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;
                }

cmd/usb.c static void usb_show_info(struct usb_device *udev)

Quoted 10 lines

cmd/usb.c +2 -2

@@ -605,7 +609,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);
                }
--

My testing of 'usb info' looks OK, however 'usb tree' still has some issues below:

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

Quoted 6 lines

  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |

Leaving block devices, why the extra print here for lan port?

Quoted 3 lines

  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | |

And here?

Quoted 6 lines

  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | |
As you see, we just don't print out the BLK device, but we still print out the | here.

I believe if the extra print for other devices is correct, then this tree is fine.
Also, I believe this is not related to the fix this patch aims at. Let me know if otherwise.

Regards,
Suneel

Quoted 12 lines

  | +-7  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard
  |
  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |

And here.

Regards,
Bin

Bin Meng

2017-09-20 03:32 UTC #3 lore raw

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

Hi Suneel,

Quoted 73 lines · Suneel Garapati

Hi Bin,

Quoted 64 lines · Bin Meng

Hi Suneel,

Quoted 9 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 v3:
- remove 'check on parent uclass' in description

thanks for making the changes.

Quoted 30 lines

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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

cmd/usb.c +6 -2

index d95bcf5..3889994 100644
@@ -414,8 +414,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;
                }

cmd/usb.c static void usb_show_info(struct usb_device *udev)

Quoted 10 lines

cmd/usb.c +2 -2

@@ -605,7 +609,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);
                }
--

My testing of 'usb info' looks OK, however 'usb tree' still has some issues below:

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

Quoted 6 lines

  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |

Quoted 1 line

Leaving block devices, why the extra print here for lan port?

There is nothing wrong here. Every device has a separation line.

Quoted 6 lines

Quoted 4 lines

  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | |

And here?

Again, nothing wrong here.

Quoted 8 lines

Quoted 6 lines
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | |
As you see, we just don't print out the BLK device, but we still print out the | here.
I believe if the extra print for other devices is correct, then this tree is fine.

It's not correct. The tree graphic does not look correct now.

Quoted 2 lines

Also, I believe this is not related to the fix this patch aims at. Let me know if otherwise.

No, you should not fix one thing but introduce another thing.

Quoted 12 lines

Regards,
Suneel

Quoted 9 lines

  | +-7  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard
  |
  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |

And here.

Regards,
Bin

Suneel Garapati

author

2017-09-20 06:32 UTC #4 lore raw

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

Hi Bin,

Quoted 98 lines · Bin Meng

Hi Suneel,
Quoted 73 lines · Suneel Garapati
Hi Bin,
Quoted 64 lines · Bin Meng
Hi Suneel,
Quoted 9 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 v3:
- remove 'check on parent uclass' in description
thanks for making the changes.
Quoted 30 lines
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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
cmd/usb.c +6 -2
index d95bcf5..3889994 100644
@@ -414,8 +414,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;
                }
cmd/usb.c static void usb_show_info(struct usb_device *udev)
Quoted 10 lines
cmd/usb.c +2 -2
@@ -605,7 +609,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);
                }
--
My testing of 'usb info' looks OK, however 'usb tree' still has some issues below:
=> usb tree
USB device tree:
1 Hub (5 Gb/s, 0mA)
| U-Boot XHCI Host Controller
|
Quoted 6 lines
  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |
Quoted 1 line
Leaving block devices, why the extra print here for lan port?
There is nothing wrong here. Every device has a separation line.
Quoted 6 lines
Quoted 4 lines
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | |
And here?
Again, nothing wrong here.
Quoted 8 lines
Quoted 6 lines
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | |
As you see, we just don't print out the BLK device, but we still print out the | here.
I believe if the extra print for other devices is correct, then this tree is fine.

Quoted 6 lines

It's not correct. The tree graphic does not look correct now.
Quoted 2 lines
Also, I believe this is not related to the fix this patch aims at. Let me know if otherwise.
No, you should not fix one thing but introduce another thing.

Ok. Let me be explicit here, to understand where I am going wrong.

Each usb_show_tree_graph call on a device can be broken down like below into three line print sets

<old-pre><devnum> <class description>
<new-pre><device description>
<repeat new-pre> (This is unconditional print, even before this fix)

USB device tree:

1 Hub (5 Gb/s, 0mA)
| U-Boot XHCI Host Controller
|

  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |

  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |

  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | | (You confirmed this as device separator)

  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | | (so is this unconditional print as device separator)

Call to block child is ignored here, so is the set of prints as described above and continues with next device below.
It is not like only device description is cancelled but preamble is being printed.

  | +-7  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard
  |

  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |

Also, request you to let me know the correct tree graph you have in mind.

Regards,
Suneel

Quoted 16 lines

Quoted 13 lines

Regards,
Suneel

Quoted 9 lines

  | +-7  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard
  |
  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |

And here.

Regards,
Bin

Bin Meng

2017-09-20 07:42 UTC #5 lore raw

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

Hi Suneel,

Quoted 148 lines · Suneel Garapati

Hi Bin,
Quoted 98 lines · Bin Meng
Hi Suneel,
Quoted 73 lines · Suneel Garapati
Hi Bin,
Quoted 64 lines · Bin Meng
Hi Suneel,
Quoted 9 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 v3:
- remove 'check on parent uclass' in description
thanks for making the changes.
Quoted 30 lines
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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
cmd/usb.c +6 -2
index d95bcf5..3889994 100644
@@ -414,8 +414,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;
                }
cmd/usb.c static void usb_show_info(struct usb_device *udev)
Quoted 10 lines
cmd/usb.c +2 -2
@@ -605,7 +609,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);
                }
--
My testing of 'usb info' looks OK, however 'usb tree' still has some issues below:
=> usb tree
USB device tree:
1 Hub (5 Gb/s, 0mA)
| U-Boot XHCI Host Controller
|
Quoted 6 lines
  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |
Quoted 1 line
Leaving block devices, why the extra print here for lan port?
There is nothing wrong here. Every device has a separation line.
Quoted 6 lines
Quoted 4 lines
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | |
And here?
Again, nothing wrong here.
Quoted 8 lines
Quoted 6 lines
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | |
As you see, we just don't print out the BLK device, but we still print out the | here.
I believe if the extra print for other devices is correct, then this tree is fine.
Quoted 6 lines
It's not correct. The tree graphic does not look correct now.
Quoted 2 lines
Also, I believe this is not related to the fix this patch aims at. Let me know if otherwise.
No, you should not fix one thing but introduce another thing.
Ok. Let me be explicit here, to understand where I am going wrong.
Each usb_show_tree_graph call on a device can be broken down like below into three line print sets
<old-pre><devnum> <class description>
<new-pre><device description>
<repeat new-pre> (This is unconditional print, even before this fix)
USB device tree:
1 Hub (5 Gb/s, 0mA)
| U-Boot XHCI Host Controller
|
  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | | (You confirmed this as device separator)
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | | (so is this unconditional print as device separator)
Call to block child is ignored here, so is the set of prints as described above and continues with next device below.
It is not like only device description is cancelled but preamble is being printed.
  | +-7  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard
  |

Quoted 6 lines

  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |
Also, request you to let me know the correct tree graph you have in mind.

Please check the attached 'usb tree' logs, and I believe that explains where the problem is.

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

  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub 
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |    
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub 
  | |
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | |    USBest Technology USB Mass Storage Device 101111c452b7c0
  | |  
  | +-7  Human Interface (1.5 Mb/s, 98mA)
  | |     USB Optical Mouse 
  | |  
  | +-8  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard 
  |    
  +-4  Mass Storage (480 Mb/s, 300mA)

JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
-------------- next part --------------
=> usb tree
USB device tree:
1 Hub (5 Gb/s, 0mA)
| U-Boot XHCI Host Controller
|

  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub 
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |    
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub 
  | |
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | |
  | +-7  Human Interface (1.5 Mb/s, 98mA)
  | |     USB Optical Mouse 
  | |  
  | +-8  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard 
  |    
  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |

Suneel Garapati

author

2017-09-21 02:18 UTC #6 lore raw

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

Hi Bin,

Quoted 155 lines · Bin Meng

Hi Suneel,
Quoted 152 lines · Suneel Garapati
Hi Bin,
Quoted 98 lines · Bin Meng
Hi Suneel,
Quoted 73 lines · Suneel Garapati
Hi Bin,
Quoted 64 lines · Bin Meng
Hi Suneel,
Quoted 9 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 v3:
- remove 'check on parent uclass' in description
thanks for making the changes.
Quoted 30 lines
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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
cmd/usb.c +6 -2
index d95bcf5..3889994 100644
@@ -414,8 +414,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;
                }
cmd/usb.c static void usb_show_info(struct usb_device *udev)
Quoted 10 lines
cmd/usb.c +2 -2
@@ -605,7 +609,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);
                }
--
My testing of 'usb info' looks OK, however 'usb tree' still has some issues below:
=> usb tree
USB device tree:
1 Hub (5 Gb/s, 0mA)
| U-Boot XHCI Host Controller
|
Quoted 6 lines
  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |
Quoted 1 line
Leaving block devices, why the extra print here for lan port?
There is nothing wrong here. Every device has a separation line.
Quoted 6 lines
Quoted 4 lines
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | |
And here?
Again, nothing wrong here.
Quoted 8 lines
Quoted 6 lines
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | |
As you see, we just don't print out the BLK device, but we still print out the | here.
I believe if the extra print for other devices is correct, then this tree is fine.
Quoted 6 lines
It's not correct. The tree graphic does not look correct now.
Quoted 2 lines
Also, I believe this is not related to the fix this patch aims at. Let me know if otherwise.
No, you should not fix one thing but introduce another thing.
Ok. Let me be explicit here, to understand where I am going wrong.
Each usb_show_tree_graph call on a device can be broken down like below into three line print sets
<old-pre><devnum> <class description>
<new-pre><device description>
<repeat new-pre> (This is unconditional print, even before this fix)
USB device tree:
1 Hub (5 Gb/s, 0mA)
| U-Boot XHCI Host Controller
|
  +-2  Hub (5 Gb/s, 0mA)
  | |  GenesysLogic USB3.0 Hub
  | |
  | +-5  Vendor specific (5 Gb/s, 36mA)
  |      Realtek USB 10/100/1000 LAN 00E04C680977
  |
  +-3  Hub (480 Mb/s, 100mA)
  | |  GenesysLogic USB2.0 Hub
  | | (You confirmed this as device separator)
  | +-6  Mass Storage (480 Mb/s, 98mA)
  | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
  | | | (so is this unconditional print as device separator)
Call to block child is ignored here, so is the set of prints as described above and continues with next device below.
It is not like only device description is cancelled but preamble is being printed.
  | +-7  Human Interface (1.5 Mb/s, 70mA)
  |      Dell Dell USB Keyboard
  |
  +-4  Mass Storage (480 Mb/s, 300mA)
    |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
    |

Quoted 5 lines

Quoted 2 lines
Also, request you to let me know the correct tree graph you have in mind.
Please check the attached 'usb tree' logs, and I believe that explains where the problem is.

Thanks for the inputs. I have sent v4 including the fix for unwanted preamble for child blk devices.

Regards,
Suneel

Quoted 3 lines

Regards,
Bin