Re: [PATCH v4 1/4] cmd: bind: Add unbind command with driver filter

[Miquel Raynal <miquel.raynal@bootlin.com>]

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,
> >   
> > > > > >>> 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 there something
> > > > > >>> we're supposed to be setting in the DT that we aren't?      
> > > > > >>
> > > > > >> You do need to unbind the ethernet before using fastboot, always, with the 'unbind ethernet 0', you dont need the peripheral unbind/rebind part, so it should behave like before.      
> > > > > > 
> > > > > > And for my own understanding, why don't we need to bind a fastboot
> > > > > > gadget?      
> > > > > 
> > > > > The fastboot command does that internally .    
> > > > 
> > > > This is not visible with dm tree and I did not find how to bind the
> > > > fastboot gadget manually.
> > > > 
> > > > This makes the CLI clearly uneven and the internal code badly different
> > > > 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.    
> > > 
> > > 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.  
> > 
> > Ok, thanks for the additional details.
> > 
> > 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.  
> 
> 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.  
> 
> 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?  
> 
> 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èl