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

[Tom Rini <trini@konsulko.com>]

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

On Sun, Jul 23, 2023 at 07:49:55PM +0200, Miquel Raynal wrote:
> Hi Marek,
> 
> marex@denx.de wrote on Mon, 17 Jul 2023 13:21:34 +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
> 
> I don't really get why this is needed? Yes, having proper bind and
> unbind methods and having them called internally is relevant, but when
> you have a single OTG controller, why is this needed? It basically
> breaks the CLI, making bisects more painful and all updates just fail.

I think part of the issue here is how usb_ether didn't act like the rest
of the gadget do, for example fastboot.

> > 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 <marex@denx.de>
> > ---
> > Cc: Kevin Hilman <khilman@baylibre.com>
> > Cc: Lukasz Majewski <lukma@denx.de>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Simon Glass <sjg@chromium.org>
> 
> I've tested the whole series, unfortunately is does not work on
> AM335x/BBBW:
> 
> * Any recovery attempted using the network will now fail in
>   the SPL, where, AFAIK, there is no way to manually bind:
> 
> U-Boot SPL 2023.07-00806-gac80e6de9cf (Jul 23 2023 - 19:45:51 +0200)
> Trying to boot from USB eth
> Could not get PHY for eth_cpsw: addr 0
> eth0: eth_cpswusing musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
> MAC de:ad:be:ef:00:01
> HOST MAC de:ad:be:ef:00:00
> RNDIS ready
> , eth1: usb_ether

For testing, what happens if you disable CPSW? Does it both bind (as it
shows here) and then use the expected device?

> * The bind command was not available on my default configuration,
>   making it even difficult for people unaware that this command is
>   now required to fix their common commands.

Yes, we'll need to make that default y if USB_ETHER.

> * Any command that expects the usb_ether driver will now fail badly
>   even after the bind call:
> 
> => bind /ocp/usb@47400000/usb@47401000 usb_ether
> => fastboot usb 0                                
> couldn't find an available UDC
> g_dnl_register: failed!, error: -19
> exit not allowed from main input shell.
> => tftp 0x81000000 zImage
> dev_get_priv: null device

Well, does it work if you do:
bind /ocp/usb@47400000/usb@47401000 usb_ether
tftp 0x81000000 zImage
?

-- 
Tom

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