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 D61BCC001DE for ; Fri, 4 Aug 2023 18:02:03 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 48FFA8693D; Fri, 4 Aug 2023 20:02:02 +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="l5ng8GG7"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id BD80C86945; Fri, 4 Aug 2023 20:02:00 +0200 (CEST) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) (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 A2C0B86930 for ; Fri, 4 Aug 2023 20:01:58 +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 B40541BF205; Fri, 4 Aug 2023 18:01:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1691172118; 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=Z9A1dJszSprD1BdH+1U23gMZD2JgdZn1FRuZXORwDdE=; b=l5ng8GG7iqQca0JrF/D3EOGOFgp7l9zfGMuyZCfm1ROh2s9gn4MvyRmdHqHTddhTHXOphq JFC/+m3ux9waoTJBRmfT6uOoJ7ljoGKhI56tAgh58aUVsJQEXSUJS5Phtj6h8z9bBClkzQ sYCigb5cvq6xie7Sl6EnqiOizYTORIeCgVqShBA0hnOW23NwsNp9ABjac1yLvAOgmqTmUc wub9xi5PRDWtRFMzukgd9b1rBg/coHsF2iJV6GOdhc2VSZ0DNbGSyC7zM/V939H3AoPGMi Xp3l/NEbJ+0jYoFY7qo2hkBFjuJQa7Yd/2VCs1uhLBjzBzdZQloLbicQr234gw== Date: Fri, 4 Aug 2023 20:01:56 +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: <20230804200156.04423128@xps-13> In-Reply-To: <20230804172029.GT3630934@bill-the-cat> References: <20230802124657.31184-1-marex@denx.de> <20230804090028.70aa780d@xps-13> <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> 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 13:20:29 -0400: > On Fri, Aug 04, 2023 at 07:01:46PM +0200, Miquel Raynal wrote: > > Hi Tom, > > =20 > > > > > >>> Well, what's needed / is it possible to get to the point wher= e we don't > > > > > >>> _need_ to call bind/unbind for each of these cases? Is there = 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, alwa= ys, with the 'unbind ethernet 0', you dont need the peripheral unbind/rebin= d part, so it should behave like before. =20 > > > > > >=20 > > > > > > And for my own understanding, why don't we need to bind a fastb= oot > > > > > > 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 diffe= rent > > > > between gadgets/commands. Why can't we have them both > > > > autoloaded/unloaded like before? I think I missed something which > > > > 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 starts. > > > 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 that 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 call) > "find usb gadget controller number X". That's doing the bind. Calling > usb_ether_init just takes the first controller and that's that (and so > could be / is wrong depending on the platform). This makes total sense, thanks for pointing it out. > > I also don't understand why I need to unbind the ethernet gadget but I > > cannot bind the fastboot gadget. =20 >=20 > You can't bind fastboot while ethernet is still configured. 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? > It's in > use. And we aren't a full blown operating system with the logic to have > 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 host > 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. Yes of course. > > 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. Sounds reasonably complex now, with the reasoning you made above. > 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 :) 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. Thanks, Miqu=C3=A8l