On Fri, Aug 04, 2023 at 06:00:12PM +0200, Miquel Raynal wrote: > Hi Marek, > > ma...@denx.de wrote on Fri, 4 Aug 2023 17:40:07 +0200: > > > On 8/4/23 17:12, Miquel Raynal wrote: > > > Hi, > > > > > > ma...@denx.de wrote on Fri, 4 Aug 2023 17:05:01 +0200: > > > > > >> On 8/4/23 17:01, Tom Rini wrote: > > >>> On Fri, Aug 04, 2023 at 04:42:13PM +0200, Marek Vasut wrote: > > >>>> On 8/4/23 09:00, Miquel Raynal wrote: > > >>>>> Hi Marek, > > >>>>> > > >>>>> ma...@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. > > >> > > >> Yes > > >> > > >>>>> 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. > > > > Did you test this yet ? > > Not yet, I'll send you the feedback once done. > > > >>>>> 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 ? > > > > > > At least I would appreciate: > > > - to select CMD_BIND "by default" when relevant > > > - to make the fastboot error more readable for the regular user > > > > Since with this 'unbind ethernet 0' this is orthogonal to this series, send > > separate patches, thanks. > > This is not orthogonal, I am sorry. > > version X: > - tftp works "out of the box" > - fastboot works "out of the box" > version X+1: > - tftp works "out of the box" > - fastboot returns an obscure error > > 1/ If we now *need* the bind/unbind commands, the series must take care > of it. > 2/ Without proper error message you just break fastboot for most > regular users (basically everyone but few U-Boot devs).
You're missing the class of users that will be impacted here. In order for there to be a change here, you have to already be in the case where you have CONFIG_USB_ETHER=y and gadget ethernet device isn't just enabled but also initialized by default by calling usb_ether_init(). That's a very small list. It's basically am33xx, two mediatek reference platforms and xilinx_zynqmp_virt. Given that am33xx defconfigs also setup DFU, I'm not really sure just how many people use gadget ethernet. The normal flow on modern devices is to be calling bind/unbind here already. -- Tom
signature.asc
Description: PGP signature
