On 10/19/2017 05:24 AM, Suneel Garapati wrote: > 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.
I don't understand this answer. Can USB_EMUL devices be enabled on any other machine than sandbox or not ? I presume it can ... >> 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. I mean, rather than doing (device_get_uclass_id(child) != UCLASS_USB_xxx && device_get_uclass_id(child) != UCLASS_USB_yyy) dump do (device_get_uclass_id(child) == UCLASS_USB_nnn) dump for nnn being only the relevant USB classes for which we actually want to dump. Does that work ? >>> 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 -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
