On Wed, Oct 18, 2017 at 6:39 PM, Marek Vasut <[email protected]> wrote: > On 10/19/2017 03:22 AM, Suneel Garapati wrote: >> usb tree/info commands iterate over all usb uclass devices >> recursively. blk uclass child devices are created for mass storage, >> treating these as usb uclass devices and referencing usb config >> interface descriptors cause crash. To fix, ignore blk and usb_emul >> uclass devices(sandbox) > ^^^^^^^ what's this about ? USB_EMUL devices can be > enables elsewhere too, right ? Only disabled during the tree/info dump.
> > Anyway, shouldn't you rather filter for positive matches (usb uclass > devices etc) , rather than filtering out a few negative matches (blk > etc) which might break in the future ? usb_for_each_root_dev does that but we dont have uclass_find_first_child_device to call on UCLASS_USB like uclass_find_first_device. So, device_find_first_child and check on uclass id is performed. > >> in usb_show_info and usb_tree_graph. >> also avoid addition of preamble for blk uclass child devices, >> otherwise tree dump gets messed up. > > Also, sentences start with capital letter. This should be in a separate > patch if it's a separate change ... Ignoring preamble and device should go together, hence cannot be separate change. Regards, Suneel > >> Signed-off-by: Suneel Garapati <[email protected]> >> Reviewed-by: Bin Meng <[email protected]> >> Tested-by: Bin Meng <[email protected]> >> Reviewed-by: Simon Glass <[email protected]> >> --- >> Changes v6: >> - re-write commit message with detail >> 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); >> } >> > > > -- > Best regards, > Marek Vasut _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
