| Message ID | 1505960243-16704-1-git-send-email-suneelglinux@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [U-Boot,v4] cmd: usb: add blk devices to ignore list in tree graph | expand |
Hi Suneel, On Thu, Sep 21, 2017 at 10:17 AM, 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 v4: > - do not set preamble if child is block device for mass storage Thanks for working on this. > 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 | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/cmd/usb.c b/cmd/usb.c > index d95bcf5..d64561e 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,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)) { I hate this, but sorry I just managed to test this patch on Sandbox. This should also check UCLASS_USB_EMUL otherwise sandbox 'usb info' still crashes. diff --git a/cmd/usb.c b/cmd/usb.c index d64561e..907debe 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -620,6 +620,7 @@ static void usb_show_info(struct usb_device *udev) child; device_find_next_child(&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); > udev = dev_get_parent_priv(child); > usb_show_info(udev); > } > -- With the above changes, I tested on both Intel MinnowMax and Sandbox, it works fine. You can include my tags: Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Tested-by: Bin Meng <bmeng.cn@gmail.com> to the v5. Really sorry about that! Regards, Bin
Hi Bin, On Wed, Sep 20, 2017 at 8:35 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Suneel, > > On Thu, Sep 21, 2017 at 10:17 AM, 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 v4: >> - do not set preamble if child is block device for mass storage > > Thanks for working on this. > >> 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 | 21 ++++++++++++++++++--- >> 1 file changed, 18 insertions(+), 3 deletions(-) >> >> diff --git a/cmd/usb.c b/cmd/usb.c >> index d95bcf5..d64561e 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,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)) { > > I hate this, but sorry I just managed to test this patch on Sandbox. > This should also check UCLASS_USB_EMUL otherwise sandbox 'usb info' > still crashes. > > diff --git a/cmd/usb.c b/cmd/usb.c > index d64561e..907debe 100644 > --- a/cmd/usb.c > +++ b/cmd/usb.c > @@ -620,6 +620,7 @@ static void usb_show_info(struct usb_device *udev) > child; > device_find_next_child(&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); > >> udev = dev_get_parent_priv(child); >> usb_show_info(udev); >> } >> -- > > With the above changes, I tested on both Intel MinnowMax and Sandbox, > it works fine. Thanks for quick testing. > > You can include my tags: > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > Tested-by: Bin Meng <bmeng.cn@gmail.com> > > to the v5. Really sorry about that! Will send v5. Regards, Suneel > > Regards, > Bin
diff --git a/cmd/usb.c b/cmd/usb.c index d95bcf5..d64561e 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,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); }
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 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 | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)