cmd: usb: add blk, emulation devices to ignore list as needed

PATCH Superseded Suneel Garapati 4 participants 9 messages Sep 21, 2017 – Oct 18, 2017

Share & tools

Suneel Garapati

author

2017-09-21 05:09 UTC #0 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

add blk child devices to ignore list while displaying usb tree graph, also preamble should not be set for blk child devices.
add usb_emul to ignore list in usb_show_info. otherwise usb tree and info commands may cause crash treating blk as usb device.

Signed-off-by Suneel Garapati <[email protected]>

Reviewed-by Bin Meng <[email protected]>

Tested-by Bin Meng <[email protected]>

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

cmd/usb.c +19 -3

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

Signature · 1 line

2.7.4

Simon Glass

2017-09-25 02:13 UTC #1 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

Quoted 23 lines · Suneel Garapati

add blk child devices to ignore list while displaying usb tree graph, also preamble should not be set for blk child devices.
add usb_emul to ignore list in usb_show_info. otherwise usb tree and info commands may cause crash treating blk as usb device.
Signed-off-by Suneel Garapati <[email protected]>
Reviewed-by Bin Meng <[email protected]>
Tested-by Bin Meng <[email protected]>
---
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

Quoted 2 lines

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

Reviewed-by Simon Glass <[email protected]>

Suneel Garapati

author

2017-10-17 03:14 UTC #2 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

Hi Bin/Simon,

Request to know if this will be applied for v2017.11

Regards,
Suneel

Quoted 28 lines · Simon Glass

Quoted 23 lines · Suneel Garapati
add blk child devices to ignore list while displaying usb tree graph, also preamble should not be set for blk child devices.
add usb_emul to ignore list in usb_show_info. otherwise usb tree and info commands may cause crash treating blk as usb device.
Signed-off-by Suneel Garapati <[email protected]>
Reviewed-by Bin Meng <[email protected]>
Tested-by Bin Meng <[email protected]>
---
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
Quoted 2 lines
 cmd/usb.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)
Reviewed-by Simon Glass <[email protected]>

Bin Meng

2017-10-17 03:21 UTC #3 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

+Marek, Tom,

Quoted 4 lines · Suneel Garapati

Hi Bin/Simon,
Request to know if this will be applied for v2017.11

Not sure which of you should pick this up for v2017.11.

Quoted 3 lines

Regards,
Suneel

Suneel, please avoid top-posting next time. thanks!

Quoted 25 lines · Simon Glass

Quoted 24 lines · Suneel Garapati
Quoted 23 lines
add blk child devices to ignore list while displaying usb tree graph, also preamble should not be set for blk child devices.
add usb_emul to ignore list in usb_show_info. otherwise usb tree and info commands may cause crash treating blk as usb device.
Signed-off-by Suneel Garapati <[email protected]>
Reviewed-by Bin Meng <[email protected]>
Tested-by Bin Meng <[email protected]>
---
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

Quoted 4 lines

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

Regards,
Bin

Marek Vasut

2017-10-17 12:43 UTC #4 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

Quoted 1 line · Bin Meng

+Marek, Tom,

Well, if the author CCed me in the first place, that'd be useful.

Quoted 8 lines

Quoted 4 lines · Suneel Garapati
Hi Bin/Simon,
Request to know if this will be applied for v2017.11
Not sure which of you should pick this up for v2017.11.

The commit message makes no sense, can you explain to me what this patch tries to achieve ?

Quoted 39 lines

Quoted 3 lines
Regards,
Suneel
Suneel, please avoid top-posting next time. thanks!
Quoted 25 lines · Simon Glass
Quoted 24 lines · Suneel Garapati
Quoted 23 lines
add blk child devices to ignore list while displaying usb tree graph, also preamble should not be set for blk child devices.
add usb_emul to ignore list in usb_show_info. otherwise usb tree and info commands may cause crash treating blk as usb device.
Signed-off-by Suneel Garapati <[email protected]>
Reviewed-by Bin Meng <[email protected]>
Tested-by Bin Meng <[email protected]>
---
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
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
 cmd/usb.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)
Reviewed-by Simon Glass <[email protected]>
Regards,
Bin

Signature · 2 lines

Best regards,
Marek Vasut

Suneel Garapati

author

2017-10-17 23:50 UTC #5 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

Quoted 9 lines · Marek Vasut

Quoted 1 line · Bin Meng
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
Quoted 4 lines
Quoted 2 lines · Suneel Garapati
Hi Bin/Simon,

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Request to know if this will be applied for v2017.11
Not sure which of you should pick this up for v2017.11.
The commit message makes no sense, can you explain to me what this patch tries to achieve ?

usb tree/info commands for DM_USB should ignore blk child under mass storage and usb_emul devices
otherwise u-boot crashes.

Regards,
Suneel

Quoted 45 lines

Quoted 32 lines
Quoted 4 lines
Regards,
Suneel
Suneel, please avoid top-posting next time. thanks!
Quoted 25 lines · Simon Glass
Quoted 24 lines · Suneel Garapati
Quoted 23 lines
add blk child devices to ignore list while displaying usb tree graph, also preamble should not be set for blk child devices.
add usb_emul to ignore list in usb_show_info. otherwise usb tree and info commands may cause crash treating blk as usb device.
Signed-off-by Suneel Garapati <[email protected]>
Reviewed-by Bin Meng <[email protected]>
Tested-by Bin Meng <[email protected]>
---
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
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
 cmd/usb.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)
Reviewed-by Simon Glass <[email protected]>
Regards,
Bin
--
Best regards,
Marek Vasut

Marek Vasut

2017-10-18 01:22 UTC #6 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

Quoted 13 lines · Suneel Garapati

Quoted 12 lines · Marek Vasut
Quoted 1 line · Bin Meng
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
Quoted 7 lines
Quoted 4 lines · Suneel Garapati
Hi Bin/Simon,
Request to know if this will be applied for v2017.11

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Not sure which of you should pick this up for v2017.11.
The commit message makes no sense, can you explain to me what this patch tries to achieve ?
usb tree/info commands for DM_USB should ignore blk child under mass storage and usb_emul devices
otherwise u-boot crashes.

And it crashes because ... why ? I am sure you analyzed the problem and that analysis/explanation should be in the commit message too.

Signature · 2 lines

Best regards,
Marek Vasut

Suneel Garapati

author

2017-10-18 01:30 UTC #7 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

Quoted 16 lines · Marek Vasut

Quoted 15 lines · Suneel Garapati
Quoted 14 lines · Marek Vasut
Quoted 1 line · Bin Meng
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
Quoted 8 lines
Quoted 4 lines · Suneel Garapati
Hi Bin/Simon,
Request to know if this will be applied for v2017.11
Not sure which of you should pick this up for v2017.11.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
The commit message makes no sense, can you explain to me what this patch tries to achieve ?
usb tree/info commands for DM_USB should ignore blk child under mass storage and usb_emul devices
otherwise u-boot crashes.
And it crashes because ... why ? I am sure you analyzed the problem and that analysis/explanation should be in the commit message too.

It crashes because the next call of tree graph on blk device will try to fetch config interface class descriptors
assuming it as usb_device structure.

I will add the above to commit message and send v6.

Regards,
Suneel

Quoted 4 lines

--
Best regards,
Marek Vasut

Marek Vasut

2017-10-18 10:12 UTC #8 lore raw

PATCH v5 1/1 cmd: usb: add blk, emulation devices to ignore list as needed

Quoted 23 lines · Suneel Garapati

Quoted 22 lines · Marek Vasut
Quoted 20 lines · Suneel Garapati
Quoted 9 lines · Marek Vasut
Quoted 1 line · Bin Meng
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
Quoted 4 lines
Quoted 2 lines · Suneel Garapati
Hi Bin/Simon,
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Request to know if this will be applied for v2017.11
Not sure which of you should pick this up for v2017.11.
The commit message makes no sense, can you explain to me what this patch tries to achieve ?
usb tree/info commands for DM_USB should ignore blk child under mass storage and usb_emul devices
otherwise u-boot crashes.

Quoted 6 lines

Quoted 2 lines
And it crashes because ... why ? I am sure you analyzed the problem and that analysis/explanation should be in the commit message too.
It crashes because the next call of tree graph on blk device will try to fetch config interface class descriptors
assuming it as usb_device structure.

Assume I know nothing about the subsystem, expand on this a bit and then it's gonna be good. This is the kind of description that should be in the commit message, to make it obvious what the problem is and how it got fixed, without having to think too much about what the author meant.

Quoted 8 lines

I will add the above to commit message and send v6.
Regards,
Suneel
Quoted 4 lines
--
Best regards,
Marek Vasut

Signature · 2 lines

Best regards,
Marek Vasut