From patchwork Wed Jun 21 07:13:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xavier Drudis Ferran X-Patchwork-Id: 1797669 X-Patchwork-Delegate: marek.vasut@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QmF8v08ycz20Wk for ; Wed, 21 Jun 2023 17:13:33 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B162A8476D; Wed, 21 Jun 2023 09:13:22 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 9FDB184659; Wed, 21 Jun 2023 09:13:20 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.2 Received: from mx1.tinet.cat (mx1.dipta.cat [195.76.233.59]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2A51A84659 for ; Wed, 21 Jun 2023 09:13:17 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xdrudis@tinet.cat X-ASG-Debug-ID: 1687331595-12aaf2530a1e13c0001-4l7tJC Received: from smtp01.tinet.cat (smtp.tinet.cat [195.77.216.131]) by mx1.tinet.cat with ESMTP id qt2UrvhLMD7fPyCY; Wed, 21 Jun 2023 09:13:15 +0200 (CEST) X-Barracuda-Envelope-From: xdrudis@tinet.cat X-Barracuda-Effective-Source-IP: smtp.tinet.cat[195.77.216.131] X-Barracuda-Apparent-Source-IP: 195.77.216.131 Received: from xdrudis.tinet.cat (180.red-79-152-181.dynamicip.rima-tde.net [79.152.181.180]) by smtp01.tinet.cat (Postfix) with ESMTPSA id A4232605DE45; Wed, 21 Jun 2023 09:13:15 +0200 (CEST) Date: Wed, 21 Jun 2023 09:13:07 +0200 From: Xavier Drudis Ferran To: u-boot@lists.denx.de Cc: Marek Vasut , Simon Glass , Lukasz Majewski , Sean Anderson , Suneel Garapati , Bin Meng Subject: [PATCH v2] cmd: usb: Prevent reset in usb tree/info command Message-ID: X-ASG-Orig-Subj: [PATCH v2] cmd: usb: Prevent reset in usb tree/info command MIME-Version: 1.0 Content-Disposition: inline X-Barracuda-Connect: smtp.tinet.cat[195.77.216.131] X-Barracuda-Start-Time: 1687331595 X-Barracuda-URL: https://webmail.tinet.cat:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 3831 X-Barracuda-BRTS-Status: 1 X-Barracuda-Bayes: INNOCENT GLOBAL 0.5204 1.0000 0.7500 X-Barracuda-Spam-Score: 0.75 X-Barracuda-Spam-Status: No, SCORE=0.75 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=6.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.110333 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean Commands causing reset in some configs: When bootflow scan is run, this will cause a UCLASS_BOOTDEV device to be added as sibling of those UCLASS_BLK devices found in the search chain defined in environment variable "boot_targets", until boot succeeds from some device. This can happen automatically as part of the default boot process on some boards (example: Rock Pi 4) depending on the board configuration (DISTRO_DEFAULTS, BOOTSTD, BOOTCOMMAND, etc.) because they have bootcmd=bootflow scan. If boot doesn't succeed from any device, 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 device and pass a null usb_device pointer to usb_show_tree_graph() or usb_show_info() (because it has no parent_priv_). This causes a reset. The expected behaviour would be to ignore the UCLASS_BOOTDEV device, continue listing the usb information and return to the prompt. Minimal test: Another way to trigger this reset as a minimal test or on boards with a different bootcmd would be: - make sure "usb" is in environment variable boot_targets (might need setenv boot_targets usb; and/or saveenv and reset), then, with a usb storage device plugged to a usb port, run: => usb reset ; bootflow scan ; usb info Solution: 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 Reviewed-by: Simon Glass Reviewed-by: Marek Vasut Tested-by: Marek Vasut --- 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/ ] Improved the commit message trying to follow Marek Vasut's advice. --- 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); } } }