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

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

Hi Tom,

trini@konsulko.com wrote on Mon, 24 Jul 2023 14:13:45 -0400:

> 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.

It definitely is. "before it was working, now it does not anymore".
That's the gut feeling many people get when the community breaks common
habits. If we break something like this, we must at least be very clear
on what is the new behavior.

> > > 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?

Disabling CPSW breaks the link, I had to ensure ft_board_setup in the
am335x arch folder was still available (and returned 0). But once I
managed to start I did not observe any improvements.

U-Boot SPL 2023.07-00806-gac80e6de9cf-dirty (Jul 28 2023 - 14:49:48 +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

> > * 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.

Agreed.

> > * 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
> ?

No, I already tried both:

U-Boot SPL 2023.07-00806-gac80e6de9cf (Jul 23 2023 - 19:45:51 +0200)
Trying to boot from MMC2


U-Boot 2023.07-00806-gac80e6de9cf (Jul 23 2023 - 19:45:51 +0200)

CPU  : AM335X-GP rev 2.1
Model: TI AM335x BeagleBone Black
DRAM:  512 MiB
Core:  160 devices, 18 uclasses, devicetree: separate
WDT:   Started wdt@44e35000 with servicing every 1000ms (60s timeout)
NAND:  0 MiB
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
Loading Environment from FAT... Unable to read "uboot.env" from mmc1:1... 
<ethaddr> not set. Validating first E-fuse MAC
Net:   Could not get PHY for ethernet@4a100000: addr 0
eth2: ethernet@4a100000using 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
, eth3: usb_ether
=> bind /ocp/usb@47400000/usb@47401000 usb_ether
=> tftp 0x81000000 zImage
dev_get_priv: null device
data abort
pc : [<9ffa04ba>]          lr : [<9ff86d5f>]
reloc pc : [<8083b4ba>]    lr : [<80821d5f>]
sp : 9df2f920  ip : 00000000     fp : 00000003
r10: 9df4cf48  r9 : 9df44ea0     r8 : 9ffec33c
r7 : 0000538c  r6 : 00000000     r5 : 00000bb8  r4 : 9df4d3c0
r3 : 9ff97653  r2 : fff69249     r1 : 00000000  r0 : 00000000
Flags: nzcv  IRQs off  FIQs on  Mode SVC_32 (T)
Code: 9ffd b508 f7e6 fc4b (6801) 2000 
Resetting CPU ...

resetting ...

Thanks,
Miquèl