[U-Boot] [PATCH v5] 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 suneelglinux@gmail.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com --- 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); }

On 20 September 2017 at 23:09, Suneel Garapati suneelglinux@gmail.com wrote:
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 suneelglinux@gmail.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Bin/Simon,
Request to know if this will be applied for v2017.11
Regards, Suneel
On Sun, Sep 24, 2017 at 7:13 PM, Simon Glass sjg@chromium.org wrote:
On 20 September 2017 at 23:09, Suneel Garapati suneelglinux@gmail.com wrote:
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 suneelglinux@gmail.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org

+Marek, Tom,
On Tue, Oct 17, 2017 at 11:14 AM, Suneel Garapati suneelglinux@gmail.com wrote:
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.
Regards, Suneel
Suneel, please avoid top-posting next time. thanks!
On Sun, Sep 24, 2017 at 7:13 PM, Simon Glass sjg@chromium.org wrote:
On 20 September 2017 at 23:09, Suneel Garapati suneelglinux@gmail.com wrote:
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 suneelglinux@gmail.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Bin

On 10/17/2017 05:21 AM, Bin Meng wrote:
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
On Tue, Oct 17, 2017 at 11:14 AM, Suneel Garapati suneelglinux@gmail.com wrote:
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 ?
Regards, Suneel
Suneel, please avoid top-posting next time. thanks!
On Sun, Sep 24, 2017 at 7:13 PM, Simon Glass sjg@chromium.org wrote:
On 20 September 2017 at 23:09, Suneel Garapati suneelglinux@gmail.com wrote:
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 suneelglinux@gmail.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Bin

On Tue, Oct 17, 2017 at 5:43 AM, Marek Vasut marex@denx.de wrote:
On 10/17/2017 05:21 AM, Bin Meng wrote:
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
On Tue, Oct 17, 2017 at 11:14 AM, Suneel Garapati suneelglinux@gmail.com wrote:
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 ?
usb tree/info commands for DM_USB should ignore blk child under mass storage and usb_emul devices otherwise u-boot crashes.
Regards, Suneel
Regards, Suneel
Suneel, please avoid top-posting next time. thanks!
On Sun, Sep 24, 2017 at 7:13 PM, Simon Glass sjg@chromium.org wrote:
On 20 September 2017 at 23:09, Suneel Garapati suneelglinux@gmail.com wrote:
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 suneelglinux@gmail.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Bin
-- Best regards, Marek Vasut

On 10/18/2017 01:50 AM, Suneel Garapati wrote:
On Tue, Oct 17, 2017 at 5:43 AM, Marek Vasut marex@denx.de wrote:
On 10/17/2017 05:21 AM, Bin Meng wrote:
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
On Tue, Oct 17, 2017 at 11:14 AM, Suneel Garapati suneelglinux@gmail.com wrote:
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 ?
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.

On Tue, Oct 17, 2017 at 6:22 PM, Marek Vasut marex@denx.de wrote:
On 10/18/2017 01:50 AM, Suneel Garapati wrote:
On Tue, Oct 17, 2017 at 5:43 AM, Marek Vasut marex@denx.de wrote:
On 10/17/2017 05:21 AM, Bin Meng wrote:
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
On Tue, Oct 17, 2017 at 11:14 AM, Suneel Garapati suneelglinux@gmail.com wrote:
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 ?
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
-- Best regards, Marek Vasut

On 10/18/2017 03:30 AM, Suneel Garapati wrote:
On Tue, Oct 17, 2017 at 6:22 PM, Marek Vasut marex@denx.de wrote:
On 10/18/2017 01:50 AM, Suneel Garapati wrote:
On Tue, Oct 17, 2017 at 5:43 AM, Marek Vasut marex@denx.de wrote:
On 10/17/2017 05:21 AM, Bin Meng wrote:
+Marek, Tom,
Well, if the author CCed me in the first place, that'd be useful.
On Tue, Oct 17, 2017 at 11:14 AM, Suneel Garapati suneelglinux@gmail.com wrote:
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 ?
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.
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.
I will add the above to commit message and send v6.
Regards, Suneel
-- Best regards, Marek Vasut
participants (4)
-
Bin Meng
-
Marek Vasut
-
Simon Glass
-
Suneel Garapati