| Message ID | 1467570125-8790-2-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | e6e188f5623e3db7fcba785f4246d16425dd2104 |
| Delegated to: | Marek Vasut |
| Headers | show |
On 3 July 2016 at 12:22, Hans de Goede <hdegoede@redhat.com> wrote: > The old dm "usb info" implementation has several issues: > > 1) NULL pointer deref when a bus has no children > 2) Not showing usb devices on busses without an emulated root-hub (otg host) > 3) Attempting to show devices on inactive busses > 4) "usb info" Would cause some hosts to get re-probed something which only > "usb reset" should do > > TL;DR: proper iterating over usb bus root devs is hard, use the helper > for it. > > Reported-by: Bernhard Nortmann <bernhard.nortmann@web.de> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > cmd/usb.c | 31 ++++++------------------------- > 1 file changed, 6 insertions(+), 25 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On 07/03/2016 08:22 PM, Hans de Goede wrote: > The old dm "usb info" implementation has several issues: > > 1) NULL pointer deref when a bus has no children > 2) Not showing usb devices on busses without an emulated root-hub (otg host) > 3) Attempting to show devices on inactive busses > 4) "usb info" Would cause some hosts to get re-probed something which only > "usb reset" should do > > TL;DR: proper iterating over usb bus root devs is hard, use the helper > for it. > > Reported-by: Bernhard Nortmann <bernhard.nortmann@web.de> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Applied, thanks > --- > cmd/usb.c | 31 ++++++------------------------- > 1 file changed, 6 insertions(+), 25 deletions(-) > > diff --git a/cmd/usb.c b/cmd/usb.c > index 5453c0d..455127c 100644 > --- a/cmd/usb.c > +++ b/cmd/usb.c > @@ -593,39 +593,20 @@ static void do_usb_start(void) > } > > #ifdef CONFIG_DM_USB > -static void show_info(struct udevice *dev) > +static void usb_show_info(struct usb_device *udev) > { > struct udevice *child; > - struct usb_device *udev; > > - udev = dev_get_parent_priv(dev); > usb_display_desc(udev); > usb_display_config(udev); > - for (device_find_first_child(dev, &child); > + for (device_find_first_child(udev->dev, &child); > child; > device_find_next_child(&child)) { > - if (device_active(child)) > - show_info(child); > - } > -} > - > -static int usb_device_info(void) > -{ > - struct udevice *bus; > - > - for (uclass_first_device(UCLASS_USB, &bus); > - bus; > - uclass_next_device(&bus)) { > - struct udevice *hub; > - > - device_find_first_child(bus, &hub); > - if (device_get_uclass_id(hub) == UCLASS_USB_HUB && > - device_active(hub)) { > - show_info(hub); > + if (device_active(child)) { > + udev = dev_get_parent_priv(child); > + usb_show_info(udev); > } > } > - > - return 0; > } > #endif > > @@ -681,7 +662,7 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > if (strncmp(argv[1], "inf", 3) == 0) { > if (argc == 2) { > #ifdef CONFIG_DM_USB > - usb_device_info(); > + usb_for_each_root_dev(usb_show_info); > #else > int d; > for (d = 0; d < USB_MAX_DEVICE; d++) { >
diff --git a/cmd/usb.c b/cmd/usb.c index 5453c0d..455127c 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -593,39 +593,20 @@ static void do_usb_start(void) } #ifdef CONFIG_DM_USB -static void show_info(struct udevice *dev) +static void usb_show_info(struct usb_device *udev) { struct udevice *child; - struct usb_device *udev; - udev = dev_get_parent_priv(dev); usb_display_desc(udev); usb_display_config(udev); - for (device_find_first_child(dev, &child); + for (device_find_first_child(udev->dev, &child); child; device_find_next_child(&child)) { - if (device_active(child)) - show_info(child); - } -} - -static int usb_device_info(void) -{ - struct udevice *bus; - - for (uclass_first_device(UCLASS_USB, &bus); - bus; - uclass_next_device(&bus)) { - struct udevice *hub; - - device_find_first_child(bus, &hub); - if (device_get_uclass_id(hub) == UCLASS_USB_HUB && - device_active(hub)) { - show_info(hub); + if (device_active(child)) { + udev = dev_get_parent_priv(child); + usb_show_info(udev); } } - - return 0; } #endif @@ -681,7 +662,7 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (strncmp(argv[1], "inf", 3) == 0) { if (argc == 2) { #ifdef CONFIG_DM_USB - usb_device_info(); + usb_for_each_root_dev(usb_show_info); #else int d; for (d = 0; d < USB_MAX_DEVICE; d++) {
The old dm "usb info" implementation has several issues: 1) NULL pointer deref when a bus has no children 2) Not showing usb devices on busses without an emulated root-hub (otg host) 3) Attempting to show devices on inactive busses 4) "usb info" Would cause some hosts to get re-probed something which only "usb reset" should do TL;DR: proper iterating over usb bus root devs is hard, use the helper for it. Reported-by: Bernhard Nortmann <bernhard.nortmann@web.de> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- cmd/usb.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-)