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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3667 bytes --]

On Fri, Aug 04, 2023 at 04:42:13PM +0200, Marek Vasut wrote:
> On 8/4/23 09:00, Miquel Raynal wrote:
> > Hi Marek,
> > 
> > marex@denx.de wrote on Wed,  2 Aug 2023 14:46:54 +0200:
> > 
> > > Extend the driver core to perform lookup by both OF node and driver
> > > bound to the node. Use this to look up specific device instances to
> > > unbind from nodes in the unbind command. One example where this is
> > > needed is USB peripheral controller, which may have multiple gadget
> > > drivers bound to it. The unbind command has to select that specific
> > > gadget driver instance to unbind from the controller, not unbind the
> > > controller driver itself from the controller.
> > > 
> > > USB ethernet gadget usage looks as follows with this change. Notice
> > > 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
> > 
> > This extra parameter does not seem to work on the BBBW. I cannot select
> > the "usb_ether" driver like you do.
> > 
> > Good news though, I am now able to use fastboot, but it is not
> > straightforward:
> > 
> > Here is my sequence right after the boot (reducing the dm tree output
> > to the usb nodes for clarity):
> > 
> > => dm tree
> >   misc          0  [ + ]   ti-musb-wrapper       |   |-- usb@47400000
> >   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
> > => fastboot usb 0
> > couldn't find an available UDC
> > g_dnl_register: failed!, error: -19
> 
> That is expected and not a bug, since the beagle explicitly binds USB
> ethernet to MUSB gadget in board file, which is legacy deprecated way.

So, we should do away with, probably all of arch_misc_init() in
arch/arm/mach-omap2/am33xx/board.c for the non-SPL case.

> > exit not allowed from main input shell.
> > => unbind /ocp/usb@47400000/usb@47401000 usb_ether
> 
> Does
> 
> => unbind ethernet 0
> 
> work ?
> 
> If so, 1/4 in this series can be skipped altogether.
> 
> You likely won't even need the rebinding of ti-musb-peripheral anymore.
> 
> > Cannot find a device with path /ocp/usb@47400000/usb@47401000
> > => unbind /ocp/usb@47400000/usb@47401000
> > => dm tree
> >   misc          0  [ + ]   ti-musb-wrapper       |   |-- usb@47400000
> >   usb           0  [   ]   ti-musb-host          |   |   `-- usb@47401800
> > => fastboot usb 0
> > => bind /ocp/usb@47400000/usb@47401000 ti-musb-peripheral
> > => dm tree
> >   misc          0  [ + ]   ti-musb-wrapper       |   |-- usb@47400000
> >   usb           0  [   ]   ti-musb-host          |   |   |-- usb@47401800
> >   usb           0  [   ]   ti-musb-peripheral    |   |   `-- usb@47401000
> > => fastboot usb 0
> > musb-hdrc: peripheral reset irq lost!
> > # works! (the irq-related line above as always been there)
> > 
> > So now, how do we make this process easy/understandable?
> 
> What would be your proposal ?

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?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]