| Message ID | ZJAtVokXPsj7ooJQ@xdrudis.tinet.cat |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | cmd: usb: Prevent reset in usb tree/info command | expand |
On 6/19/23 12:26, Xavier Drudis Ferran wrote: > When DISTRO_DEFAULTS is not set, the default environment has > bootcmd=bootflow and this will cause a UCLASS_BOOTDEV device to be > added as sibling of those UCLASS_BLK devices in boot_targets, until > boot succeeds from some device. If none succeeds, and usb is in > boot_targets, and an usb storage device is plugged to some usb port at > boot time, its UCLASS_MASS_STORAGE device will have a UCLASS_BOOTDEV > device as child, besides a UCLASS_BLK child. > > If once the boot fails the user enters at the U-Boot shell prompt: > > usb info > > or > > usb tree > > The code in cmd/usb.c will eventually recurse into the UCLASS_BOOTDEV > and pass a null pointer to usb_device (because it has no parent_priv_). > This causes a reset. > > Fix it (twice) by checking for null parent_priv_ and adding > UCLASS_BOOTDEV to the list of ignored class ids before the recursive > call. > > This prevents the current particular problem with UCLASS_BOOTDEV, even > in case it ever gets some parent_priv_ struct which is not an > usb_device, despite being the child of a usb_device->dev. And it also > prevents possible future problems if other children are added to usb > devices that don't have parent_priv_ because they are not part of the > usb tree, just abstractions of functionality (like UCLASS_BLK and > UCLASS_BOOTDEV are now). > > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> > --- > > v2: added UCLASS_BOOTDEV check (discussion of v1 dried up without much > evident consensus, so hopefully Simon Glass likes it better now) > [ https://patchwork.ozlabs.org/project/uboot/patch/ZH39SVA1vbZR3+Gk@xdrudis.tinet.cat/ ] > --- > > Apologies to the people in Cc: for resending this. I had a typo in the > email list address. git send-email -v2 would help Also you can add a CC list into the commit message below --- , then git send-email will automatically pick the CC list up. To get a list of people to CC , use get-maintainer script.
diff --git a/cmd/usb.c b/cmd/usb.c index 6193728384..23253f2223 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -421,7 +421,9 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre) * Ignore emulators and block child devices, we only want * real devices */ - if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) && + if (udev && + (device_get_uclass_id(child) != UCLASS_BOOTDEV) && + (device_get_uclass_id(child) != UCLASS_USB_EMUL) && (device_get_uclass_id(child) != UCLASS_BLK)) { usb_show_tree_graph(udev, pre); pre[index] = 0; @@ -604,10 +606,12 @@ 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_BOOTDEV) && (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); + if (udev) + usb_show_info(udev); } } }
When DISTRO_DEFAULTS is not set, the default environment has bootcmd=bootflow and this will cause a UCLASS_BOOTDEV device to be added as sibling of those UCLASS_BLK devices in boot_targets, until boot succeeds from some device. If none succeeds, and usb is in boot_targets, and an usb storage device is plugged to some usb port at boot time, its UCLASS_MASS_STORAGE device will have a UCLASS_BOOTDEV device as child, besides a UCLASS_BLK child. If once the boot fails the user enters at the U-Boot shell prompt: usb info or usb tree The code in cmd/usb.c will eventually recurse into the UCLASS_BOOTDEV and pass a null pointer to usb_device (because it has no parent_priv_). This causes a reset. Fix it (twice) by checking for null parent_priv_ and adding UCLASS_BOOTDEV to the list of ignored class ids before the recursive call. This prevents the current particular problem with UCLASS_BOOTDEV, even in case it ever gets some parent_priv_ struct which is not an usb_device, despite being the child of a usb_device->dev. And it also prevents possible future problems if other children are added to usb devices that don't have parent_priv_ because they are not part of the usb tree, just abstractions of functionality (like UCLASS_BLK and UCLASS_BOOTDEV are now). Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat> --- v2: added UCLASS_BOOTDEV check (discussion of v1 dried up without much evident consensus, so hopefully Simon Glass likes it better now) [ https://patchwork.ozlabs.org/project/uboot/patch/ZH39SVA1vbZR3+Gk@xdrudis.tinet.cat/ ] --- Apologies to the people in Cc: for resending this. I had a typo in the email list address. --- cmd/usb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)