Hi Tim,
On Wed, 16 Feb 2022 at 17:03, Tim Harvey tharvey@gateworks.com wrote:
On Wed, Feb 16, 2022 at 12:23 AM Patrice CHOTARD patrice.chotard@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@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@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@58005000.blk ethernet 0 [ + ] eth_eqos | |-- ethernet@5800a000 eth_phy_ge 0 [ + ] eth_phy_generic_drv | | `-- ethernet-phy@0 usb 0 [ ] ehci_generic | |-- usb@5800d000 video 0 [ ] stm32_display | |-- display-controller@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@58005000.blk usb 0 [ ] ehci_generic | |-- usb@5800d000 video 0 [ ] stm32_display | |-- display-controller@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@58005000.blk usb 0 [ ] ehci_generic | |-- usb@5800d000 ethernet 0 [ ] usb_ether | | `-- usb_ether video 0 [ ] stm32_display | |-- display-controller@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@2184000 usb 1 [ ] ehci_mx6 | | |-- usb@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@2188000 00:d0:12:f3:f2:f5 active Ventana > bind usb 0 usb_ether Ventana > net list eth0 : ethernet@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@2188000 flags=0x1043 eth0 : ethernet@2188000 00:d0:12:f3:f2:f5 active Ventana > bind usb 0 usb_ether Ventana > net list eth0 : ethernet@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@2188000 flags=0x1043 device_probe usb_ether flags=0x4a device_probe usb@2184000 flags=0x1042 device_probe bus@2100000 flags=0x1051 device_probe usbotggrp flags=0x40 device_probe iomuxc@20e0000 flags=0x1041 device_probe iomuxc@20e0000 flags=0x1041 device_probe regulator-usb-otg-vbus flags=0x52 device_probe gpio@20a4000 flags=0x1043 device_probe root_driver flags=0x1041 device_probe iomuxc@20e0000 flags=0x1041 usb_eth_probe usb_ether usb_eth_start usb_setup_ehci_gadget usb_setup_ehci_gadget removing old device 'usb@2184000' device_remove usb@2184000 device_remove usb_ether usb_eth_stop usb_setup_ehci_gadget probing 'usb@2184000' device_probe usb@2184000 flags=0x42 device_probe bus@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