All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xavier Drudis Ferran <xdrudis@tinet.cat>
To: u-boot@lists.denx.de
Cc: Marek Vasut <marex@denx.de>, Simon Glass <sjg@chromium.org>,
	e@xdrudis.tinet.cat, Lukasz Majewski <lukma@denx.de>,
	Sean Anderson <seanga2@gmail.com>,
	Suneel Garapati <suneelglinux@gmail.com>,
	Bin Meng <bmeng.cn@gmail.com>
Subject: [PATCH] cmd: usb: Prevent reset in usb tree/info  command
Date: Mon, 19 Jun 2023 12:26:30 +0200	[thread overview]
Message-ID: <ZJAtVokXPsj7ooJQ@xdrudis.tinet.cat> (raw)

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(-)

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);
 		}
 	}
 }
-- 
2.20.1


             reply	other threads:[~2023-06-19 11:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-19 10:26 Xavier Drudis Ferran [this message]
2023-06-19 11:54 ` [PATCH] cmd: usb: Prevent reset in usb tree/info command Marek Vasut
     [not found] <ZJAqKxrO7qa8r6Kq@xdrudis.tinet.cat>
2023-06-19 21:49 ` Marek Vasut
2023-06-20  9:17   ` Xavier Drudis Ferran
2023-06-20  9:49     ` Marek Vasut
  -- strict thread matches above, loose matches on Subject: below --
2023-06-05 15:20 Xavier Drudis Ferran
2023-06-07 22:05 ` Marek Vasut
2023-06-08  7:39   ` Xavier Drudis Ferran
2023-06-09  1:20     ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZJAtVokXPsj7ooJQ@xdrudis.tinet.cat \
    --to=xdrudis@tinet.cat \
    --cc=bmeng.cn@gmail.com \
    --cc=e@xdrudis.tinet.cat \
    --cc=lukma@denx.de \
    --cc=marex@denx.de \
    --cc=seanga2@gmail.com \
    --cc=sjg@chromium.org \
    --cc=suneelglinux@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.