From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 70AC8C001DE for ; Wed, 2 Aug 2023 07:48:12 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 60D5686C80; Wed, 2 Aug 2023 09:48:10 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=bootlin.com header.i=@bootlin.com header.b="pU52TcXF"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8B58386849; Wed, 2 Aug 2023 09:48:08 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::221]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id ABFA686A8C for ; Wed, 2 Aug 2023 09:48:04 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=miquel.raynal@bootlin.com Received: by mail.gandi.net (Postfix) with ESMTPSA id 3496224000C; Wed, 2 Aug 2023 07:48:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1690962484; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=cfXWgJCpnQ7Rk1xsMi2xgF29HS79kGxskbzHzA3AsPE=; b=pU52TcXF2lQbafPr9RZIg4sFxWTe2LNYvoW1lndQBIjSoG+ymHELWWI59It5Ol9RPEOBVg bU3TE+h//8gOo2BqXiWOsjz5XYSTKMl7ElOpKoaGKImQWeUnTuKK+q5Ydx9zZZkooT5cc7 QcOsYWl/dIZ7/EAd0PCSp8W/R3vOe/uWhXG+pOGvCB4ZrPq3tIsV5CPt5tVsycEuqmAi2U 1YDihp+XdoE7M084I9fMh7dU+R1DESMecsQV76fShVWhmxDaP98b7ER2P2Edb8hhplxK6R uUpf9jfKWwVP19a8+XNmm16XmtRjG+WrlZomc3XUkNgQoCneKByjKpzTKTp3uA== Date: Wed, 2 Aug 2023 09:48:02 +0200 From: Miquel Raynal To: Marek Vasut Cc: u-boot@lists.denx.de, Kevin Hilman , Lukasz Majewski , Simon Glass Subject: Re: [PATCH v3 1/4] cmd: bind: Add unbind command with driver filter Message-ID: <20230802094802.736eb4dc@xps-13> In-Reply-To: <85b8d9a7-db07-429b-e880-677f575f05c7@denx.de> References: <20230729145712.213945-1-marex@denx.de> <20230731113159.26e710aa@xps-13> <20230731153607.6210e92f@xps-13> <8c31ee42-36c4-beab-853b-45f7828908a6@denx.de> <20230731155857.1575f5a4@xps-13> <830897a6-d061-f7c8-8f30-7ccc906b53a4@denx.de> <20230731162520.3897ba88@xps-13> <20230801205342.2fdbffc8@xps-13> <85b8d9a7-db07-429b-e880-677f575f05c7@denx.de> Organization: Bootlin X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Sasl: miquel.raynal@bootlin.com 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 Hi Marek, marex@denx.de wrote on Wed, 2 Aug 2023 01:07:46 +0200: > On 8/1/23 20:53, Miquel Raynal wrote: > > Hi Marek, > >=20 > > marex@denx.de wrote on Mon, 31 Jul 2023 16:40:04 +0200: > > =20 > >> On 7/31/23 16:25, Miquel Raynal wrote: =20 > >>> Hi Marek, > >>> > >>> marex@denx.de wrote on Mon, 31 Jul 2023 16:08:19 +0200: =20 > >>> >>>> On 7/31/23 15:58, Miquel Raynal wrote: =20 > >>>>> Hi Marek, > >>>>> > >>>>> marex@denx.de wrote on Mon, 31 Jul 2023 15:50:58 +0200: =20 > >>>>> >>>> On 7/31/23 15:36, Miquel Raynal wrote: =20 > >>>>>>> Hi Marek, > >>>>>>> > >>>>>>> marex@denx.de wrote on Mon, 31 Jul 2023 13:44:25 +0200: =20 > >>>>>>> >>>> On 7/31/23 11:31, Miquel Raynal wrote: =20 > >>>>>>>>> Hi Marek, > >>>>>>>>> > >>>>>>>>> marex@denx.de wrote on Sat, 29 Jul 2023 16:57:09 +0200: =20 > >>>>>>>>> >>>> Extend the driver core to perform lookup by both OF = node and driver =20 > >>>>>>>>>> bound to the node. Use this to look up specific device instanc= es to > >>>>>>>>>> unbind from nodes in the unbind command. One example where thi= s is > >>>>>>>>>> needed is USB peripheral controller, which may have multiple g= adget > >>>>>>>>>> drivers bound to it. The unbind command has to select that spe= cific > >>>>>>>>>> gadget driver instance to unbind from the controller, not unbi= nd the > >>>>>>>>>> controller driver itself from the controller. > >>>>>>>>>> > >>>>>>>>>> USB ethernet gadget usage looks as follows with this change. N= otice > >>>>>>>>>> the extra 'usb_ether' addition in the 'unbind' command at the = end. > >>>>>>>>>> " > >>>>>>>>>> bind /soc/usb-otg@49000000 usb_ether > >>>>>>>>>> setenv ethact usb_ether > >>>>>>>>>> setenv loadaddr 0xc2000000 > >>>>>>>>>> setenv ipaddr 10.0.0.2 > >>>>>>>>>> setenv serverip 10.0.0.1 > >>>>>>>>>> setenv netmask 255.255.255.0 > >>>>>>>>>> tftpboot 0xc2000000 10.0.0.1:test.file > >>>>>>>>>> unbind /soc/usb-otg@49000000 usb_ether > >>>>>>>>>> " > >>>>>>>>>> > >>>>>>>>>> Signed-off-by: Marek Vasut > >>>>>>>>>> --- =20 > >>>>>>>>> > >>>>>>>>> I am no longer getting wrong pointer dereferences, the SPL is w= orking in > >>>>>>>>> recovery mode, TFTP "File not found" errors are no longer a pro= blem and > >>>>>>>>> I did not experience any reset while tftp'ing regular files. > >>>>>>>>> > >>>>>>>>> One last remaining request on my side is the need for using fas= tboot as > >>>>>>>>> well which does no longer work as-is: =20 > >>>>>>>>> >>> =3D> fastboot usb 0 =20 > >>>>>>>>> couldn't find an available UDC > >>>>>>>>> g_dnl_register: failed!, error: -19 > >>>>>>>>> exit not allowed from main input shell. > >>>>>>>>> > >>>>>>>>> Can you advise what bind/unbind command would be necessary here= ? =20 > >>>>>>>> > >>>>>>>> Either 'unbind usb_ether' or run 'dm tree' -> look up the path t= o usb_ether in the tree (it will be hanging under usb_peripheral or some su= ch), and then use 'unbind '. =20 > >>>>>>> > >>>>>>> Nice `dm tree` command, never used it before. > >>>>>>> > >>>>>>> Even when I unbind usb_ether I still get the same error: =20 > >>>>>>> >>> =3D> unbind /ocp/usb@47400000/usb@47401000 =20 > >>>>>>> =3D> fastboot usb 0 =20 > >>>>>>> couldn't find an available UDC > >>>>>>> g_dnl_register: failed!, error: -19 > >>>>>>> exit not allowed from main input shell. > >>>>>>> > >>>>>>> Is there a specific gadget driver which I should bind again manua= lly? =20 > >>>>>> > >>>>>> Can you share the output of dm tree before/after unbind ? > >>>>>> > >>>>>> fastboot should auto-bind to the right thing. =20 > >>>>> > >>>>> Ok. Apparently it does not, but I don't have any clue why. If you w= ant > >>>>> me to check something else I will. Here is the output: > >>>>> > >>>>> U-Boot 2023.07-00806-g979e7443428 (Jul 31 2023 - 11:17:06 +0200) =20 > >>>> > >>>> [...] =20 > >>>> >>>>> watchdog 0 [ + ] omap3_wdt | |-- w= dt@44e35000 =20 > >>>>> misc 0 [ + ] ti-musb-wrapper | |-- usb@4740= 0000 > >>>>> usb 0 [ + ] ti-musb-peripheral | | |-- usb@= 47401000 > >>>>> ethernet 1 [ + ] usb_ether | | | `-- = usb_ether > >>>>> bootdev 3 [ ] eth_bootdev | | | = `-- usb_ether.bootdev > >>>>> usb 0 [ ] ti-musb-host | | `-- usb@= 47401800 > >>>>> ethernet 0 [ + ] eth_cpsw | |-- ethernet= @4a100000 > >>>>> bootdev 2 [ ] eth_bootdev | | `-- ethe= rnet@4a100000.bootdev > >>>>> simple_bus 67 [ ] ti_sysc | |-- target-m= odule@53100000 > >>>>> simple_bus 68 [ ] ti_sysc | |-- target-m= odule@53500000 > >>>>> simple_bus 69 [ ] ti_sysc | `-- target-m= odule@56000000 > >>>>> clk 62 [ ] fixed_clock |-- clk_mcasp0_f= ixed > >>>>> bootstd 0 [ ] bootstd_drv |-- bootstd > >>>>> bootmeth 0 [ ] bootmeth_efi | |-- efi > >>>>> bootmeth 1 [ ] bootmeth_extlinux | |-- extlinux > >>>>> bootmeth 2 [ ] bootmeth_pxe | |-- pxe > >>>>> bootmeth 3 [ ] vbe_simple | `-- vbe_simp= le > >>>>> timer 0 [ + ] omap_timer `-- timer@0 =20 > >>>>> =3D> unbind /ocp/usb@47400000/usb@47401000 =20 > >>>> > >>>> The commit message of this patch contains the following example: > >>>> unbind /soc/usb-otg@49000000 usb_ether > >>>> so in your case, try > >>>> unbind /ocp/usb@47400000/usb@47401000 usb_ether =20 > >>> > >>> Does not work, unfortunately: =20 > >>> =3D> unbind /ocp/usb@47400000/usb@47401000 usb_ether =20 > >>> Cannot find a device with path /ocp/usb@47400000/usb@47401000 =20 > >>> =3D> unbind /ocp/usb@47400000/usb@47401000 =20 > >> > >> Did this even work before , i.e. without this series, if you were to u= se USB ethernet first and then fastboot second (that sequence is important)= ? =20 > >=20 > > Yes it did. > > =20 > >> Can you debug this ? Basically the problem that happens is that if you= run: > >> unbind /ocp/usb@47400000/usb@47401000 > >> this unbind ti-musb-peripheral usb@47401000 instead of just unbinding > >> usb_ether usb_ether =20 > >=20 > > True. > > =20 > >> Maybe just try > >> unbind usb_ether =20 > > =20 > > =3D> unbind usb_ether =20 > > unbind - Unbind a device from a driver > >=20 > > Usage: > > unbind [] > > unbind > > unbind > > =20 > >> or > >> unbind usb_ether usb_ether =20 > > =20 > > =3D> unbind usb_ether usb_ether =20 > > usb_ether is not a valid uclass > > =20 > >> ti-musb-peripheral usb@47401000 has to stay in the dm tree output afte= r the unbind. Or maybe try > >> > >> bind /ocp/usb@47400000/usb@47401000 ti-musb-peripheral =20 > >=20 > > Indeed when we perform the unbind, ti-musb-peripheral is removed. Yes, > > binding it again like you advise makes it come back. > > =20 > >> and then the fastboot thing =20 > >=20 > > Unfortunately after binding it again, so with the following dm tree > > snippet: > >=20 > > misc 0 [ + ] ti-musb-wrapper | |-- usb@47400000 > > usb 0 [ ] ti-musb-host | | |-- usb@474018= 00 > > usb 0 [ ] ti-musb-peripheral | | `-- usb@474010= 00 > >=20 > > It still fails: > > =20 > > =3D> fastboot usb 0 =20 > > couldn't find an available UDC > > g_dnl_register: failed!, error: -19 > > exit not allowed from main input shell. > >=20 > > Without this series, this works: =20 > > =3D> unbind /ocp/usb@47400000/usb@47401000 > > =3D> bind /ocp/usb@47400000/usb@47401000 ti-musb-peripheral > > =3D> fastboot usb 0 =20 >=20 > OK, so fastboot did not work even without this series without obscure wor= karounds ? I'm happy you call the unbind/bind sequence an "obscure workaround" ;-) The command "fastboot usb 0" works without your series, before tftp or after tftp, it does not matter, it used to work. Applying this series makes it always fail. I was just reporting that, before your series, calling unbind would also remove ti-musb-peripheral, but calling bind again with the above line makes the usb_ether gadget and fastboot work again. Whereas with your series the unbind behaves identically but the bind command does not "fix" fastboot (tftp works fine in v3). > Can you try and debug this ? The breakage is not part of the existing code base, it was introduced by the series. Maybe you can try with any other board with USB peripheral and see if the behavior is identical? > Does 'fastboot usb 1' work with this series by any chance ? When fastboot works, starting it "blocks" the CLI. When I unbind the peripheral driver on USB 0, the fastboot command does not block any more and yet does not print anything, it just returns "immediately" without any log. When I run 'fastboot usb 1' it acts like that as well, nothing happens, without error. It acts identically before and after this series. Thanks, Miqu=C3=A8l