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 5097AC001DF for ; Fri, 4 Aug 2023 19:38:22 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9288A86970; Fri, 4 Aug 2023 21:38:20 +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="N9R4nAdK"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id A5FA48694D; Fri, 4 Aug 2023 21:38:19 +0200 (CEST) Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) (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 CB51C86970 for ; Fri, 4 Aug 2023 21:38:16 +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 B3E90C0005; Fri, 4 Aug 2023 19:38:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1691177896; 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=dKAoZJlojLDOruKreonQ6MODeutYpq3XFJ8wSaLdJrU=; b=N9R4nAdK2teHd1wi0NGoov46zNvBfy18yx2+3snDMi8AUyWDvdQEJcaLG4XfGKw5rWVpKQ MKW1vXmBJMVf5G1q6qGAzg3bq1Kh8vBQpkRK7RJ7InWUh6UoVculaNr2lnkuhkE8p7RuGj SONan1IMe1bbl1UUQ1bFUSx5m82vKfYNcS7/jULfmhA5sEedwuFfZkkXoV/wjlR7W9lQ9z gwNj8hLo9RtEnSnJOE1rFyqEXHIJ0Wdqk9K/EKbDLs9IbB/2YvcJ7hPJvPrEYHArpV5Xtr /rRjrMYlfQ6leE0mi2+zaDnhU/2c9GIY9kThEV1v8Lcs5ItS0qR4qHd3QQ714A== Date: Fri, 4 Aug 2023 21:38:13 +0200 From: Miquel Raynal To: Tom Rini Cc: Marek Vasut , u-boot@lists.denx.de, Kevin Hilman , Lukasz Majewski , Simon Glass Subject: Re: [PATCH v4 1/4] cmd: bind: Add unbind command with driver filter Message-ID: <20230804213813.37f47528@xps-13> In-Reply-To: <20230804185107.GZ3630934@bill-the-cat> References: <4833722f-18ab-e885-291a-988115fe39f8@denx.de> <20230804150100.GN3630934@bill-the-cat> <8902cf54-acba-cfb4-571b-09ba807e7a89@denx.de> <20230804171206.07faade4@xps-13> <0eb7f3fe-42da-6252-3b77-50eb3276f2ee@denx.de> <20230804180012.79c4f9ec@xps-13> <20230804161515.GR3630934@bill-the-cat> <20230804190146.2595e241@xps-13> <20230804172029.GT3630934@bill-the-cat> <20230804200156.04423128@xps-13> <20230804185107.GZ3630934@bill-the-cat> 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 Tom, trini@konsulko.com wrote on Fri, 4 Aug 2023 14:51:07 -0400: > On Fri, Aug 04, 2023 at 08:01:56PM +0200, Miquel Raynal wrote: > > Hi Tom, > >=20 > > trini@konsulko.com wrote on Fri, 4 Aug 2023 13:20:29 -0400: > > =20 > > > On Fri, Aug 04, 2023 at 07:01:46PM +0200, Miquel Raynal wrote: =20 > > > > Hi Tom, > > > > =20 > > > > > > > >>> Well, what's needed / is it possible to get to the point = where we don't > > > > > > > >>> _need_ to call bind/unbind for each of these cases? Is th= ere something > > > > > > > >>> we're supposed to be setting in the DT that we aren't? = =20 > > > > > > > >> > > > > > > > >> You do need to unbind the ethernet before using fastboot, = always, with the 'unbind ethernet 0', you dont need the peripheral unbind/r= ebind part, so it should behave like before. =20 > > > > > > > >=20 > > > > > > > > And for my own understanding, why don't we need to bind a f= astboot > > > > > > > > gadget? =20 > > > > > > >=20 > > > > > > > The fastboot command does that internally . =20 > > > > > >=20 > > > > > > This is not visible with dm tree and I did not find how to bind= the > > > > > > fastboot gadget manually. > > > > > >=20 > > > > > > This makes the CLI clearly uneven and the internal code badly d= ifferent > > > > > > between gadgets/commands. Why can't we have them both > > > > > > autoloaded/unloaded like before? I think I missed something whi= ch > > > > > > explains the rationale behind this series. =20 > > > > >=20 > > > > > They aren't both auto-loaded currently. We have a legacy call, > > > > > usb_ether_init(), in a few cases, so that gadget mode ethernet st= arts. > > > > > But this leads to the ref counting problems you encountered and > > > > > re-posted the rejected series for. =20 > > > >=20 > > > > Ok, thanks for the additional details. > > > >=20 > > > > I don't understand why fastboot autoloads the correct gadget driver= if > > > > there is none bound, while all network commands just fail to do tha= t if > > > > we don't make the usb_ether_init() call manually. =20 > > >=20 > > > Because "fastboot" or "dfu" are both being told (as part of their cal= l) > > > "find usb gadget controller number X". That's doing the bind. Calli= ng > > > usb_ether_init just takes the first controller and that's that (and so > > > could be / is wrong depending on the platform). =20 > >=20 > > This makes total sense, thanks for pointing it out. > > =20 > > > > I also don't understand why I need to unbind the ethernet gadget bu= t I > > > > cannot bind the fastboot gadget. =20 > > >=20 > > > You can't bind fastboot while ethernet is still configured. =20 > >=20 > > I guess "before this series", the ethernet would not be kept > > configured, because I could use both fastboot and ethernet without > > caring about which driver had to be bound. And that's maybe what lead to > > the bug reports also. So you want to get rid of this. Do I get the > > situation right now? =20 >=20 > We're getting rid of this path because it leads to failures, yes. >=20 > > > It's in > > > use. And we aren't a full blown operating system with the logic to h= ave > > > multiple end points and devices configured and exposed. I mean heck,= we > > > don't keep the gadget interface up between network calls so on the ho= st > > > side you need to re-configure the interface (or have something that's > > > bringing it up there again each time). Which is why DFU or fastboot = or > > > other "treat USB like USB" options tend to be more popular than "treat > > > USB like ethernet" work flows. =20 > >=20 > > Yes of course. > > =20 > > > > My underlying question is: can we have a single approach for all > > > > drivers, or is it too complex today? Could it be possible, when we > > > > perform these autoloads, to look up the registered gadget and > > > > potentially unbind the one already in place before? =20 > > >=20 > > > I would invite you to look in to this, yes. =20 > >=20 > > Sounds reasonably complex now, with the reasoning you made above. > > =20 > > > No one objects to enhancing > > > the code, but the "functionality" you see on am33xx is as you've also > > > seen very broken in other ways, which is why it's not used virtually > > > anywhere else and instead the "bind" command is. For example, if you > > > wanted to do this workflow on the new beagle devices, that's DWC3 and > > > where the "bind" command came from :) =20 > >=20 > > Again to be very clear, while I felt misunderstood at the beginning and > > did not accept Marek's series because it was replacing a data abort > > with a non-functional setup, I now get a better understanding of the > > problem (I believe) and, as said before, I am always in favor of > > writing better code, easier to maintain, clearer to review. I am in > > favor of such change. I just want the user life to be eased when this > > happens if we break the CLI. So if you think the right approach is to > > get rid of the usb_ether_init() call, alright, let's drop it off. But > > we should not let the users in the dark by providing proper doc or > > error messages which should compensate the extra step now required. =20 >=20 > I think it would be good if you posted a patch to update > doc/board/ti/am335x_evm.rst to explain how to use the various gadget > device cases. Good idea, I've tried something. Thanks, Miqu=C3=A8l