bind/unbind command not working for usb_ether
Simon Glass
sjg at chromium.org
Sat Aug 13 16:59:00 CEST 2022
Hi Tim,
On Wed, 16 Feb 2022 at 17:03, Tim Harvey <tharvey at gateworks.com> wrote:
>
> On Wed, Feb 16, 2022 at 12:23 AM Patrice CHOTARD
> <patrice.chotard at foss.st.com> wrote:
> >
> > Hi Tim
> >
> > On 2/15/22 19:28, Tim Harvey wrote:
> > > On Tue, Feb 15, 2022 at 9:36 AM Patrice CHOTARD
> > > <patrice.chotard at foss.st.com> wrote:
> > >>
> > >> Hi Tim
> > >>
> > >> On 2/15/22 17:09, Tim Harvey wrote:
> > >>> On Tue, Feb 15, 2022 at 5:29 AM Patrice CHOTARD
> > >>> <patrice.chotard at foss.st.com> wrote:
> > >>>>
> > >>>> Hi Tim
> > >>>>
> > >>>> On 2/10/22 18:04, Tim Harvey wrote:
> > >>>>> Greetings,
> > >>>>>
> > >>>>> I'm trying to understand how to use the U-Boot bind command to bind
> > >>>>> the usb_ether driver to the usb class to register a USB ethernet
> > >>>>> gadget network device as referenced in:
> > >>>>> commit 02291d83fdaaf ("doc: add bind/unbind command documentation")
> > >>>>> commit 49c752c93a78 ("cmd: Add bind/unbind commands to bind a device
> > >>>>> to a driver from the command line")
> > >>>>>
> > >>>>
> > >>>> For example, i made some trial on STM32MP1 platform:
> > >>>>
> > >>>> At boot, we got :
> > >>>>
> > >>>> STM32MP> dm tree
> > >>>> Class Index Probed Driver Name
> > >>>> -----------------------------------------------------------
> > >>>> root 0 [ + ] root_driver root_driver
> > >>>> firmware 0 [ ] psci |-- psci
> > >>>> sysreset 0 [ ] psci-sysreset | `-- psci-sysreset
> > >>>> .....
> > >>>> blk 0 [ + ] mmc_blk | | `-- mmc at 58005000.blk
> > >>>> ethernet 0 [ + ] eth_eqos | |-- ethernet at 5800a000
> > >>>> eth_phy_ge 0 [ + ] eth_phy_generic_drv | | `-- ethernet-phy at 0
> > >>>> usb 0 [ ] ehci_generic | |-- usb at 5800d000
> > >>>> video 0 [ ] stm32_display | |-- display-controller at 5a001000
> > >>>> .....
> > >>>>
> > >>>>
> > >>>> As you can see, there is already an ethernet interface used.
> > >>>> We unbind the ethernet interface before binding the usb_ether gadget to the usb class.
> > >>>> First unbind the generic ethernet phy (eth_phy_generic_drv) and the ethernet driver
> > >>>> (eth_eqos).
> > >>>>
> > >>>>
> > >>>> STM32MP> unbind eth_phy_generic 0
> > >>>> STM32MP> unbind ethernet 0
> > >>>> STM32MP> dm tree
> > >>>> Class Index Probed Driver Name
> > >>>> -----------------------------------------------------------
> > >>>> root 0 [ + ] root_driver root_driver
> > >>>> firmware 0 [ ] psci |-- psci
> > >>>> sysreset 0 [ ] psci-sysreset | `-- psci-sysreset
> > >>>> ....
> > >>>> blk 0 [ + ] mmc_blk | | `-- mmc at 58005000.blk
> > >>>> usb 0 [ ] ehci_generic | |-- usb at 5800d000
> > >>>> video 0 [ ] stm32_display | |-- display-controller at 5a001000
> > >>>> ....
> > >>>>
> > >>>> Ethernet and phy driver are both unbinded.
> > >>>> Now we can bind the usb_eher to the usb class
> > >>>>
> > >>>> STM32MP> bind usb 0 usb_ether
> > >>>> STM32MP> dm tree
> > >>>> Class Index Probed Driver Name
> > >>>> -----------------------------------------------------------
> > >>>> root 0 [ + ] root_driver root_driver
> > >>>> firmware 0 [ ] psci |-- psci
> > >>>> sysreset 0 [ ] psci-sysreset | `-- psci-sysreset
> > >>>> ....
> > >>>> blk 0 [ + ] mmc_blk | | `-- mmc at 58005000.blk
> > >>>> usb 0 [ ] ehci_generic | |-- usb at 5800d000
> > >>>> ethernet 0 [ ] usb_ether | | `-- usb_ether
> > >>>> video 0 [ ] stm32_display | |-- display-controller at 5a001000
> > >>>> ....
> > >>>>
> > >>>> usb_ether is now binded.
> > >>>> As example, if you can then use some ethernet command as dhcp or ping :
> > >>>>
> > >>>> STM32MP> dhcp
> > >>>> using dwc2-udc, OUT ep2out-bulk IN ep1in-bulk STATUS ep3in-int
> > >>>> MAC de:ad:be:ef:00:01
> > >>>> HOST MAC de:ad:be:ef:00:00
> > >>>> RNDIS ready
> > >>>> high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
> > >>>> USB RNDIS network up!
> > >>>> BOOTP broadcast 1
> > >>>>
> > >>>>> I have enabled:
> > >>>>> CONFIG_DM_USB=y
> > >>>>> CONFIG_USB_GADGET=y
> > >>>>> CONFIG_USB_ETHER=y
> > >>>>>
> > >>>> In my case i enabled also CONFIG_USB_ETH_RNDIS=y
> > >>>>
> > >>>
> > >>> Patrice,
> > >>>
> > >>> In my case when I try to bind to usb_ether the device can not be found
> > >>> (as it is never registered in the first place):
> > >>> Ventana > unbind ethernet 0
> > >>> Ventana > bind usb 0 usb_ether
> > >>> Cannot find device 0 of class usb
> > >>
> > >> weird, because below, in the dm tree output, we can see :
> > >>
> > >>> usb 0 [ ] ehci_mx6 | | |-- usb at 2184000
> > >>> usb 1 [ ] ehci_mx6 | | |-- usb at 2184200
> > >>
> > >> so it should find a usb class device .....
> > >>
> > >
> > > Patrice,
> > >
> > > I added some debugging and found that 'bind usb 0 usb_ether' does the following:
> > > bind_by_class_index(class="usb" index=0 drv="usb_ether")
> > > uclass_get_by_name(name="usb")
> > > uclass_get_by_name_len(name="usb" len=3)
> > > ^^^ does a strncmp with name=usb and len=3 so it matches the first
> > > driver with a class name starting with 'usb' and in this case matches
> > > usb_mass_storage instead of 'usb'
> >
> > Good catch ;-)
> >
> > >
> > > Simon, this behavior comes from commit 4b030177b660 ("dm: core: Allow
> > > finding children / uclasses by partial name"). Why would
> > > device_find_child_by_name() want to use a substring match? I suppose
> > > if there is a valid reason for this such as your commit logs describe
> > > then those functions should directly call uclass_get_by_name_len() and
> > > uclass_get_by_name() should be doing a full name match correct?
> > >
> > > Patrice, if I make uclass_get_by_name match the full string with the
> > > below patch then 'bind usb 0 usb_ether' indeed works as planned:
> > >
> > > diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> > > index 2578803b7a4d..33dbca544574 100644
> > > --- a/drivers/core/uclass.c
> > > +++ b/drivers/core/uclass.c
> > > @@ -196,7 +196,16 @@ enum uclass_id uclass_get_by_name_len(const char
> > > *name, int len)
> > >
> > > enum uclass_id uclass_get_by_name(const char *name)
> > > {
> > > - return uclass_get_by_name_len(name, strlen(name));
> > > + int i;
> > > +
> > > + for (i = 0; i < UCLASS_COUNT; i++) {
> > > + struct uclass_driver *uc_drv = lists_uclass_lookup(i);
> > > +
> > > + if (uc_drv && !strcmp(uc_drv->name, name))
> > > + return i;
> > > + }
> > > +
> > > + return UCLASS_INVALID;
> > > }
> > >
> > > int dev_get_uclass_index(struct udevice *dev, struct uclass **ucp)
> > >
> > > I don't see the need to unbind 'ethernet' as you can select the active
> > > network device with 'ethact':
> > >
> > > Ventana > net list
> > > eth0 : ethernet at 2188000 00:d0:12:f3:f2:f5 active
> > > Ventana > bind usb 0 usb_ether
> > > Ventana > net list
> > > eth0 : ethernet at 2188000 00:d0:12:f3:f2:f5 active
> > > eth1 : usb_ether 00:00:00:00:00:00
> > > Ventana > setenv ethact eth1
> >
> >
> > Thanks for the tips, i didn't know it.
> >
> > > Ventana > ping 192.168.1.146
> > > ^^^ but now we run into the issue Heiko discovered where the
> > > usb_gadget_register_driver() call from ether.c ends up removing usb
> > > and usb_ether and without his hack to return from device-remove if
> > > device is usb_ether we hang:
> > >
> > > @@ -201,12 +202,16 @@ int device_remove(struct udevice *dev, uint flags)
> > > const struct driver *drv;
> > > int ret;
> > >
> > > +printf("%s %s\n", __func__, dev->name);
> > > if (!dev)
> > > return -EINVAL;
> > >
> > > if (!(dev_get_flags(dev) & DM_FLAG_ACTIVATED))
> > > return 0;
> > >
> > > + if (!strncmp(dev->name, "usb_ether", 8))
> > > + return 0;
> > > +
> > > /*
> > > * If the child returns EKEYREJECTED, continue. It just means that it
> > > * didn't match the flags.
> > >
> > >
>
> Simon,
>
> Now that we have identified a fix needed to uclass_get_by_name to fix
> the dm bind command, can you comment on the other issue we have run
> into trying to use usb_ether?
>
> Here's an example with some debugging:
> Ventana > net list
> device_probe ethernet at 2188000 flags=0x1043
> eth0 : ethernet at 2188000 00:d0:12:f3:f2:f5 active
> Ventana > bind usb 0 usb_ether
> Ventana > net list
> eth0 : ethernet at 2188000 00:d0:12:f3:f2:f5 active
> eth1 : usb_ether 00:00:00:00:00:00
> Ventana > setenv ethact eth1
> Ventana > ping 192.168.1.146
> device_probe ethernet at 2188000 flags=0x1043
> device_probe usb_ether flags=0x4a
> device_probe usb at 2184000 flags=0x1042
> device_probe bus at 2100000 flags=0x1051
> device_probe usbotggrp flags=0x40
> device_probe iomuxc at 20e0000 flags=0x1041
> device_probe iomuxc at 20e0000 flags=0x1041
> device_probe regulator-usb-otg-vbus flags=0x52
> device_probe gpio at 20a4000 flags=0x1043
> device_probe root_driver flags=0x1041
> device_probe iomuxc at 20e0000 flags=0x1041
> usb_eth_probe usb_ether
> usb_eth_start
> usb_setup_ehci_gadget
> usb_setup_ehci_gadget removing old device 'usb at 2184000'
> device_remove usb at 2184000
> device_remove usb_ether
> usb_eth_stop
> usb_setup_ehci_gadget probing 'usb at 2184000'
> device_probe usb at 2184000 flags=0x42
> device_probe bus at 2100000 flags=0x1051
> device_probe usbotggrp flags=0x1041
> device_probe regulator-usb-otg-vbus flags=0x1053
> usb_setup_ehci_gadget done
> ^^^ hangs here - notice usb controller and the usb_ether child were
> removed then usb controller probed again (but not usb_ether)
>
> fbeceb260232 ("dm: usb: Allow setting up a USB controller as a
> device/gadget") adds the usb_setup_ehci_gadget() function which
> removes the old USB controller device (and its usb_ether child) then
> probes only the USB controller leaving the usb_ether device un-probed.
> The commit log makes it sound like something isn't complete:
>
> Some controllers support OTG (on-the-go) where they can operate as either
> host or device. The gadget layer in U-Boot supports this.
>
> While this layer does not interact with driver model, we can provide a
> function which sets up the controller in the correct way. This way the code
> at least builds (although it likely will not work).
>
> I'm not clear why the USB controller (and children) need to be removed
> here. If I comment out the removal and re-probe of the controller
> usb_ether then works fine:
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 27e2fc6fcd36..0882641b51b0 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -399,6 +399,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
> ret = uclass_find_first_device(UCLASS_USB, &dev);
> if (ret)
> return ret;
> +#if 0
> ret = device_remove(dev, DM_REMOVE_NORMAL);
> if (ret)
> return ret;
> @@ -408,6 +409,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
> ret = device_probe(dev);
> if (ret)
> return ret;
> +#endif
> *ctlrp = dev_get_priv(dev);
>
> return 0;
>
> Why was it necessary to remove the USB controller and children and reprobe?
+Marek Vasut who may know more
I suspect that this is a bit of a hack to get the device running after
it is swtiched from host to device mode?
The gadget side of things should really move to driver model.
Regards,
SImon
More information about the U-Boot
mailing list