[U-Boot] [PATCH v4 0/7] Fixes/Addition to use the USB Ethernet gadget with the DWC3 gadget controller

This series implements 2 fixes to be able to use USB Ethernet gadget with the dwc3 driver. It also adds new commands to bind/unbind a device to/from a driver and update the 'dm tree' command to make it easier to use those new commands. The bind/unbind commands can be used to bind the DWC3 USB gadget to the usb_ether driver from the command line instead of relying on platform code.
Changes in v4: - Fixed compiler warning: "cmd/bind.c:236:5: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized]"
Changes in v3: - update commit log - fixed problem with the function name - update commit log - new commit - new - factorize code based on comments from ML - remove the devices before unbinding them - use device_find_global_by_ofnode() to get a device by its node.
Changes in v2: - Make the bind/unbind command generic, not specific to usb device. - Update the API to be able to bind/unbind based on DTS node path - Add a Kconfig option to select the bind/unbind commands
Jean-Jacques Hiblot (7): usb: gadget: Add bcdDevice for the DWC3 USB Gadget Controller net: eth-uclass: Fix for DM USB ethernet support uclass: Add dev_get_uclass_index() to get the uclass/index of a device dm: print the index of the device when dumping the dm tree dm: convert device_get_global_by_of_offset() to device_get_global_by_ofnode() device: expose the functions used to remove and unbind children of a device cmd: Add bind/unbind commands to bind a device to a driver from the command line
arch/arm/mach-rockchip/rk3188-board-spl.c | 2 +- arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +- arch/sandbox/dts/test.dts | 11 ++ cmd/Kconfig | 9 ++ cmd/Makefile | 1 + cmd/bind.c | 255 ++++++++++++++++++++++++++++++ configs/sandbox_defconfig | 1 + drivers/core/device-remove.c | 30 ++-- drivers/core/device.c | 19 ++- drivers/core/dump.c | 16 +- drivers/core/uclass.c | 21 +++ drivers/usb/gadget/gadget_chips.h | 2 + include/dm/device-internal.h | 38 +++++ include/dm/device.h | 23 ++- include/dm/uclass-internal.h | 11 ++ net/eth-uclass.c | 3 +- test/py/tests/test_bind.py | 178 +++++++++++++++++++++ 17 files changed, 584 insertions(+), 38 deletions(-) create mode 100644 cmd/bind.c create mode 100644 test/py/tests/test_bind.py

Add an entry in usb_gadget_controller_number() for the DWC3 gadget controller. Without it, it is not possible to bind the USB Ethernet driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
Changes in v4: None Changes in v3: None Changes in v2: None
drivers/usb/gadget/gadget_chips.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/usb/gadget/gadget_chips.h b/drivers/usb/gadget/gadget_chips.h index f320708..9b0ad2e 100644 --- a/drivers/usb/gadget/gadget_chips.h +++ b/drivers/usb/gadget/gadget_chips.h @@ -214,5 +214,7 @@ static inline int usb_gadget_controller_number(struct usb_gadget *gadget) return 0x21; else if (gadget_is_fotg210(gadget)) return 0x22; + else if (gadget_is_dwc3(gadget)) + return 0x23; return -ENOENT; }

When a USB ethernet device is halted, the device driver is removed. When this happens the uclass private memory is freed and uclass_priv is set to NULL. This causes a data abort when uclass_priv->state is then set to ETH_STATE_PASSIVE.
Fix it by checking if uclass_priv is NULL before setting uclass_priv->state
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com Acked-by: Joe Hershberger joe.hershberger@ni.com ---
Changes in v4: None Changes in v3: None Changes in v2: None
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index fa3f549..91d861b 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -312,7 +312,8 @@ void eth_halt(void)
eth_get_ops(current)->stop(current); priv = current->uclass_priv; - priv->state = ETH_STATE_PASSIVE; + if (priv) + priv->state = ETH_STATE_PASSIVE; }
int eth_is_active(struct udevice *dev)

This function is the reciprocal of uclass_find_device(). It will be used to print the index information in dm tree dump.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v4: None Changes in v3: - update commit log - fixed problem with the function name
Changes in v2: None
drivers/core/uclass.c | 21 +++++++++++++++++++++ include/dm/uclass-internal.h | 11 +++++++++++ 2 files changed, 32 insertions(+)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index d609b17..3113d6a 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -171,6 +171,27 @@ enum uclass_id uclass_get_by_name(const char *name) return UCLASS_INVALID; }
+int dev_get_uclass_index(struct udevice *dev, struct uclass **ucp) +{ + struct udevice *iter; + struct uclass *uc = dev->uclass; + int i = 0; + + if (list_empty(&uc->dev_head)) + return -ENODEV; + + list_for_each_entry(iter, &uc->dev_head, uclass_node) { + if (iter == dev) { + if (ucp) + *ucp = uc; + return i; + } + i++; + } + + return -ENODEV; +} + int uclass_find_device(enum uclass_id id, int index, struct udevice **devp) { struct uclass *uc; diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h index 7ba064b..30d5a4f 100644 --- a/include/dm/uclass-internal.h +++ b/include/dm/uclass-internal.h @@ -24,6 +24,17 @@ int uclass_get_device_tail(struct udevice *dev, int ret, struct udevice **devp);
/** + * dev_get_uclass_index() - Get uclass and index of device + * @dev: - in - Device that we want the uclass/index of + * @ucp: - out - A pointer to the uclass the device belongs to + * + * The device is not prepared for use - this is an internal function. + * + * @return the index of the device in the uclass list or -ENODEV if not found. + */ +int dev_get_uclass_index(struct udevice *dev, struct uclass **ucp); + +/** * uclass_find_device() - Return n-th child of uclass * @id: Id number of the uclass * @index: Position of the child in uclass's list

On 9 August 2018 at 08:17, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This function is the reciprocal of uclass_find_device(). It will be used to print the index information in dm tree dump.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v4: None Changes in v3:
- update commit log
- fixed problem with the function name
Changes in v2: None
drivers/core/uclass.c | 21 +++++++++++++++++++++ include/dm/uclass-internal.h | 11 +++++++++++ 2 files changed, 32 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

Command "dm tree" dumps the devices with class, driver, name information. Add the index of the device in the class too, because the information is useful for the bind/unbind commands.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com Reviewed-by: Simon Glass sjg@chromium.org
---
Changes in v4: None Changes in v3: - update commit log
Changes in v2: None
drivers/core/dump.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/core/dump.c b/drivers/core/dump.c index 58820cd..d7cdb14 100644 --- a/drivers/core/dump.c +++ b/drivers/core/dump.c @@ -8,6 +8,7 @@ #include <mapmem.h> #include <dm/root.h> #include <dm/util.h> +#include <dm/uclass-internal.h>
static void show_devices(struct udevice *dev, int depth, int last_flag) { @@ -15,7 +16,8 @@ static void show_devices(struct udevice *dev, int depth, int last_flag) struct udevice *child;
/* print the first 11 characters to not break the tree-format. */ - printf(" %-10.10s [ %c ] %-10.10s ", dev->uclass->uc_drv->name, + printf(" %-10.10s %d [ %c ] %-10.10s ", dev->uclass->uc_drv->name, + dev_get_uclass_index(dev, NULL), dev->flags & DM_FLAG_ACTIVATED ? '+' : ' ', dev->driver->name);
for (i = depth; i >= 0; i--) { @@ -47,8 +49,8 @@ void dm_dump_all(void)
root = dm_root(); if (root) { - printf(" Class Probed Driver Name\n"); - printf("----------------------------------------\n"); + printf(" Class index Probed Driver Name\n"); + printf("-----------------------------------------\n"); show_devices(root, -1, 0); } } @@ -60,9 +62,9 @@ void dm_dump_all(void) * * @dev: Device to display */ -static void dm_display_line(struct udevice *dev) +static void dm_display_line(struct udevice *dev, int index) { - printf("- %c %s @ %08lx", + printf("%i %c %s @ %08lx", index, dev->flags & DM_FLAG_ACTIVATED ? '*' : ' ', dev->name, (ulong)map_to_sysmem(dev)); if (dev->seq != -1 || dev->req_seq != -1) @@ -78,6 +80,7 @@ void dm_dump_uclass(void)
for (id = 0; id < UCLASS_COUNT; id++) { struct udevice *dev; + int i = 0;
ret = uclass_get(id, &uc); if (ret) @@ -87,7 +90,8 @@ void dm_dump_uclass(void) if (list_empty(&uc->dev_head)) continue; list_for_each_entry(dev, &uc->dev_head, uclass_node) { - dm_display_line(dev); + dm_display_line(dev, i); + i++; } puts("\n"); }

Also add device_find_global_by_ofnode() that also find a device based on the OF node, but doesn't probe the device.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v4: None Changes in v3: - new commit
Changes in v2: None
arch/arm/mach-rockchip/rk3188-board-spl.c | 2 +- arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +- drivers/core/device.c | 19 +++++++++++++------ include/dm/device.h | 23 +++++++++++++++++++---- 4 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-rockchip/rk3188-board-spl.c b/arch/arm/mach-rockchip/rk3188-board-spl.c index 59c7e4d..98ca971 100644 --- a/arch/arm/mach-rockchip/rk3188-board-spl.c +++ b/arch/arm/mach-rockchip/rk3188-board-spl.c @@ -49,7 +49,7 @@ u32 spl_boot_device(void) debug("node=%d\n", node); goto fallback; } - ret = device_get_global_by_of_offset(node, &dev); + ret = device_get_global_by_ofnode(offset_to_ofnode(node), &dev); if (ret) { debug("device at node %s/%d not found: %d\n", bootdev, node, ret); diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c index ea6a14a..abd62e5 100644 --- a/arch/arm/mach-rockchip/rk3288-board-spl.c +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c @@ -51,7 +51,7 @@ u32 spl_boot_device(void) debug("node=%d\n", node); goto fallback; } - ret = device_get_global_by_of_offset(node, &dev); + ret = device_get_global_by_ofnode(offset_to_ofnode(node), &dev); if (ret) { debug("device at node %s/%d not found: %d\n", bootdev, node, ret); diff --git a/drivers/core/device.c b/drivers/core/device.c index 207d566..fd59fe1 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -594,16 +594,16 @@ int device_get_child_by_of_offset(struct udevice *parent, int node, return device_get_device_tail(dev, ret, devp); }
-static struct udevice *_device_find_global_by_of_offset(struct udevice *parent, - int of_offset) +static struct udevice *_device_find_global_by_ofnode(struct udevice *parent, + ofnode ofnode) { struct udevice *dev, *found;
- if (dev_of_offset(parent) == of_offset) + if (ofnode_equal(dev_ofnode(parent), ofnode)) return parent;
list_for_each_entry(dev, &parent->child_head, sibling_node) { - found = _device_find_global_by_of_offset(dev, of_offset); + found = _device_find_global_by_ofnode(dev, ofnode); if (found) return found; } @@ -611,11 +611,18 @@ static struct udevice *_device_find_global_by_of_offset(struct udevice *parent, return NULL; }
-int device_get_global_by_of_offset(int of_offset, struct udevice **devp) +int device_find_global_by_ofnode(ofnode ofnode, struct udevice **devp) +{ + *devp = _device_find_global_by_ofnode(gd->dm_root, ofnode); + + return *devp ? 0 : -ENOENT; +} + +int device_get_global_by_ofnode(ofnode ofnode, struct udevice **devp) { struct udevice *dev;
- dev = _device_find_global_by_of_offset(gd->dm_root, of_offset); + dev = _device_find_global_by_ofnode(gd->dm_root, ofnode); return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp); }
diff --git a/include/dm/device.h b/include/dm/device.h index 49078bc..3120b68 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -473,18 +473,33 @@ int device_get_child_by_of_offset(struct udevice *parent, int of_offset, struct udevice **devp);
/** - * device_get_global_by_of_offset() - Get a device based on FDT offset + * device_find_global_by_ofnode() - Get a device based on ofnode * - * Locates a device by its device tree offset, searching globally throughout + * Locates a device by its device tree ofnode, searching globally throughout + * the all driver model devices. + * + * The device is NOT probed + * + * @node: Device tree ofnode to find + * @devp: Returns pointer to device if found, otherwise this is set to NULL + * @return 0 if OK, -ve on error + */ + +int device_find_global_by_ofnode(ofnode node, struct udevice **devp); + +/** + * device_get_global_by_ofnode() - Get a device based on ofnode + * + * Locates a device by its device tree ofnode, searching globally throughout * the all driver model devices. * * The device is probed to activate it ready for use. * - * @of_offset: Device tree offset to find + * @node: Device tree ofnode to find * @devp: Returns pointer to device if found, otherwise this is set to NULL * @return 0 if OK, -ve on error */ -int device_get_global_by_of_offset(int of_offset, struct udevice **devp); +int device_get_global_by_ofnode(ofnode node, struct udevice **devp);
/** * device_find_first_child() - Find the first child of a device

Hi Jean-Jacques,
On 9 August 2018 at 08:17, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Also add device_find_global_by_ofnode() that also find a device based on the OF node, but doesn't probe the device.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v4: None Changes in v3:
- new commit
Changes in v2: None
arch/arm/mach-rockchip/rk3188-board-spl.c | 2 +- arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +- drivers/core/device.c | 19 +++++++++++++------ include/dm/device.h | 23 +++++++++++++++++++---- 4 files changed, 34 insertions(+), 12 deletions(-)
Can you please add a test for this? See dm/test/ofnode.c
Regards, Simon

Also add a 'drv' parameter to filter the children to remove/unbind. Exporting those functions is a preparatory work for the addition of the bind/unbind commands.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v4: None Changes in v3: - new
Changes in v2: None
drivers/core/device-remove.c | 30 +++++++++++------------------- include/dm/device-internal.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 1cf2278..586fade 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -17,16 +17,7 @@ #include <dm/uclass-internal.h> #include <dm/util.h>
-/** - * device_chld_unbind() - Unbind all device's children from the device - * - * On error, the function continues to unbind all children, and reports the - * first error. - * - * @dev: The device that is to be stripped of its children - * @return 0 on success, -ve on error - */ -static int device_chld_unbind(struct udevice *dev) +int device_chld_unbind(struct udevice *dev, struct driver *drv) { struct udevice *pos, *n; int ret, saved_ret = 0; @@ -34,6 +25,9 @@ static int device_chld_unbind(struct udevice *dev) assert(dev);
list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) { + if (drv && (pos->driver != drv)) + continue; + ret = device_unbind(pos); if (ret && !saved_ret) saved_ret = ret; @@ -42,13 +36,8 @@ static int device_chld_unbind(struct udevice *dev) return saved_ret; }
-/** - * device_chld_remove() - Stop all device's children - * @dev: The device whose children are to be removed - * @pre_os_remove: Flag, if this functions is called in the pre-OS stage - * @return 0 on success, -ve on error - */ -static int device_chld_remove(struct udevice *dev, uint flags) +int device_chld_remove(struct udevice *dev, struct driver *drv, + uint flags) { struct udevice *pos, *n; int ret; @@ -56,6 +45,9 @@ static int device_chld_remove(struct udevice *dev, uint flags) assert(dev);
list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) { + if (drv && (pos->driver != drv)) + continue; + ret = device_remove(pos, flags); if (ret) return ret; @@ -87,7 +79,7 @@ int device_unbind(struct udevice *dev) return ret; }
- ret = device_chld_unbind(dev); + ret = device_chld_unbind(dev, NULL); if (ret) return ret;
@@ -178,7 +170,7 @@ int device_remove(struct udevice *dev, uint flags) if (ret) return ret;
- ret = device_chld_remove(dev, flags); + ret = device_chld_remove(dev, NULL, flags); if (ret) goto err;
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index f4af154..02ac4c7 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -131,6 +131,44 @@ static inline void device_free(struct udevice *dev) {} #endif
/** + * device_chld_unbind() - Unbind all device's children from the device if bound + * to drv + * + * On error, the function continues to unbind all children, and reports the + * first error. + * + * @dev: The device that is to be stripped of its children + * @drv: The targeted driver + * @return 0 on success, -ve on error + */ +#if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE) +int device_chld_unbind(struct udevice *dev, struct driver *drv); +#else +static inline int device_chld_unbind(struct udevice *dev, struct driver *drv) +{ + return 0; +} +#endif + +/** + * device_chld_remove() - Stop all device's children + * @dev: The device whose children are to be removed + * @drv: The targeted driver + * @flags: Flag, if this functions is called in the pre-OS stage + * @return 0 on success, -ve on error + */ +#if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE) +int device_chld_remove(struct udevice *dev, struct driver *drv, + uint flags); +#else +static inline int device_chld_remove(struct udevice *dev, struct driver *drv, + uint flags) +{ + return 0; +} +#endif + +/** * simple_bus_translate() - translate a bus address to a system address * * This handles the 'ranges' property in a simple bus. It translates the

On 9 August 2018 at 08:17, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Also add a 'drv' parameter to filter the children to remove/unbind. Exporting those functions is a preparatory work for the addition of the bind/unbind commands.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v4: None Changes in v3:
- new
Changes in v2: None
drivers/core/device-remove.c | 30 +++++++++++------------------- include/dm/device-internal.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 19 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

In some cases it can be useful to be able to bind a device to a driver from the command line. The obvious example is for versatile devices such as USB gadget. Another use case is when the devices are not yet ready at startup and require some setup before the drivers are bound (ex: FPGA which bitsream is fetched from a mass storage or ethernet)
usage example:
bind usb_dev_generic 0 usb_ether unbind usb_dev_generic 0 usb_ether or unbind eth 1
bind /ocp/omap_dwc3@48380000/usb@48390000 usb_ether unbind /ocp/omap_dwc3@48380000/usb@48390000
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v4: - Fixed compiler warning: "cmd/bind.c:236:5: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized]"
Changes in v3: - factorize code based on comments from ML - remove the devices before unbinding them - use device_find_global_by_ofnode() to get a device by its node.
Changes in v2: - Make the bind/unbind command generic, not specific to usb device. - Update the API to be able to bind/unbind based on DTS node path - Add a Kconfig option to select the bind/unbind commands
arch/sandbox/dts/test.dts | 11 ++ cmd/Kconfig | 9 ++ cmd/Makefile | 1 + cmd/bind.c | 255 +++++++++++++++++++++++++++++++++++++++++++++ configs/sandbox_defconfig | 1 + test/py/tests/test_bind.py | 178 +++++++++++++++++++++++++++++++ 6 files changed, 455 insertions(+) create mode 100644 cmd/bind.c create mode 100644 test/py/tests/test_bind.py
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 137679a..0d7f5ab 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -58,6 +58,17 @@ reg = <2 1>; };
+ bind-test { + bind-test-child1 { + compatible = "sandbox,phy"; + #phy-cells = <1>; + }; + + bind-test-child2 { + compatible = "simple-bus"; + }; + }; + b-test { reg = <3 1>; compatible = "denx,u-boot-fdt-test"; diff --git a/cmd/Kconfig b/cmd/Kconfig index ef43ed8..b26f625 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -607,6 +607,15 @@ config CMD_ADC Shows ADC device info and permit printing one-shot analog converted data from a named Analog to Digital Converter.
+config CMD_BIND + bool "bind/unbind - Bind or unbind a device to/from a driver" + depends on DM + help + Bind or unbind a device to/from a driver from the command line. + This is useful in situations where a device may be handled by several + drivers. For example, this can be used to bind a UDC to the usb ether + gadget driver from the command line. + config CMD_CLK bool "clk - Show clock frequencies" help diff --git a/cmd/Makefile b/cmd/Makefile index 323f1fd..4429f6b 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_SOURCE) += source.o obj-$(CONFIG_CMD_SOURCE) += source.o obj-$(CONFIG_CMD_BDI) += bdinfo.o obj-$(CONFIG_CMD_BEDBUG) += bedbug.o +obj-$(CONFIG_CMD_BIND) += bind.o obj-$(CONFIG_CMD_BINOP) += binop.o obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o obj-$(CONFIG_CMD_BMP) += bmp.o diff --git a/cmd/bind.c b/cmd/bind.c new file mode 100644 index 0000000..44a5f17 --- /dev/null +++ b/cmd/bind.c @@ -0,0 +1,255 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2018 JJ Hiblot jjhiblot@ti.com + */ + +#include <common.h> +#include <dm.h> +#include <dm/device-internal.h> +#include <dm/lists.h> +#include <dm/uclass-internal.h> + +static int bind_by_class_index(const char *uclass, int index, + const char *drv_name) +{ + static enum uclass_id uclass_id; + struct udevice *dev; + struct udevice *parent; + int ret; + struct driver *drv; + + drv = lists_driver_lookup_name(drv_name); + if (!drv) { + printf("Cannot find driver '%s'\n", drv_name); + return -ENOENT; + } + + uclass_id = uclass_get_by_name(uclass); + if (uclass_id == UCLASS_INVALID) { + printf("%s is not a valid uclass\n", uclass); + return -EINVAL; + } + + ret = uclass_find_device(uclass_id, index, &parent); + if (!parent || ret) { + printf("Cannot find device %d of class %s\n", index, uclass); + return ret; + } + + ret = device_bind_with_driver_data(parent, drv, drv->name, 0, + ofnode_null(), &dev); + if (!dev || ret) { + printf("Unable to bind. err:%d\n", ret); + return ret; + } + + return 0; +} + +static int find_dev(const char *uclass, int index, struct udevice **devp) +{ + static enum uclass_id uclass_id; + int rc; + + uclass_id = uclass_get_by_name(uclass); + if (uclass_id == UCLASS_INVALID) { + printf("%s is not a valid uclass\n", uclass); + return -EINVAL; + } + + rc = uclass_find_device(uclass_id, index, devp); + if (!*devp || rc) { + printf("Cannot find device %d of class %s\n", index, uclass); + return rc; + } + + return 0; +} + +static int unbind_by_class_index(const char *uclass, int index) +{ + int ret; + struct udevice *dev; + + ret = find_dev(uclass, index, &dev); + if (ret) + return ret; + + ret = device_remove(dev, DM_REMOVE_NORMAL); + if (ret) { + printf("Unable to remove. err:%d\n", ret); + return ret; + } + + ret = device_unbind(dev); + if (ret) { + printf("Unable to unbind. err:%d\n", ret); + return ret; + } + + return 0; +} + +static int unbind_child_by_class_index(const char *uclass, int index, + const char *drv_name) +{ + struct udevice *parent; + int ret; + struct driver *drv; + + drv = lists_driver_lookup_name(drv_name); + if (!drv) { + printf("Cannot find driver '%s'\n", drv_name); + return -ENOENT; + } + + ret = find_dev(uclass, index, &parent); + if (ret) + return ret; + + ret = device_chld_remove(parent, drv, DM_REMOVE_NORMAL); + if (ret) + printf("Unable to remove all. err:%d\n", ret); + + ret = device_chld_unbind(parent, drv); + if (ret) + printf("Unable to unbind all. err:%d\n", ret); + + return ret; +} + +static int bind_by_node_path(const char *path, const char *drv_name) +{ + struct udevice *dev; + struct udevice *parent = NULL; + int ret; + ofnode ofnode; + struct driver *drv; + + drv = lists_driver_lookup_name(drv_name); + if (!drv) { + printf("%s is not a valid driver name\n", drv_name); + return -ENOENT; + } + + ofnode = ofnode_path(path); + if (!ofnode_valid(ofnode)) { + printf("%s is not a valid node path\n", path); + return -EINVAL; + } + + while (ofnode_valid(ofnode)) { + if (!device_find_global_by_ofnode(ofnode, &parent)) + break; + ofnode = ofnode_get_parent(ofnode); + } + + if (!parent) { + printf("Cannot find a parent device for node path %s\n", path); + return -ENODEV; + } + + ofnode = ofnode_path(path); + ret = device_bind_with_driver_data(parent, drv, ofnode_get_name(ofnode), + 0, ofnode, &dev); + if (!dev || ret) { + printf("Unable to bind. err:%d\n", ret); + return ret; + } + + return 0; +} + +static int unbind_by_node_path(const char *path) +{ + struct udevice *dev; + int ret; + ofnode ofnode; + + ofnode = ofnode_path(path); + if (!ofnode_valid(ofnode)) { + printf("%s is not a valid node path\n", path); + return -EINVAL; + } + + ret = device_find_global_by_ofnode(ofnode, &dev); + + if (!dev || ret) { + printf("Cannot find a device with path %s\n", path); + return -ENODEV; + } + + ret = device_remove(dev, DM_REMOVE_NORMAL); + if (ret) { + printf("Unable to remove. err:%d\n", ret); + return ret; + } + + ret = device_unbind(dev); + if (ret) { + printf("Unable to unbind. err:%d\n", ret); + return ret; + } + + return 0; +} + +static int do_bind_unbind(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + int ret = 0; + bool bind; + bool by_node; + + if (argc < 2) + return CMD_RET_USAGE; + + bind = (argv[0][0] == 'b'); + by_node = (argv[1][0] == '/'); + + if (by_node && bind) { + if (argc != 3) + return CMD_RET_USAGE; + ret = bind_by_node_path(argv[1], argv[2]); + } else if (by_node && !bind) { + if (argc != 2) + return CMD_RET_USAGE; + ret = unbind_by_node_path(argv[1]); + } else if (!by_node && bind) { + int index = (argc > 2) ? simple_strtoul(argv[2], NULL, 10) : 0; + + if (argc != 4) + return CMD_RET_USAGE; + ret = bind_by_class_index(argv[1], index, argv[3]); + } else if (!by_node && !bind) { + int index = (argc > 2) ? simple_strtoul(argv[2], NULL, 10) : 0; + + if (argc == 3) + ret = unbind_by_class_index(argv[1], index); + else if (argc == 4) + ret = unbind_child_by_class_index(argv[1], index, + argv[3]); + else + return CMD_RET_USAGE; + } + + if (ret) + return CMD_RET_FAILURE; + else + return CMD_RET_SUCCESS; +} + +U_BOOT_CMD( + bind, 4, 0, do_bind_unbind, + "Bind a device to a driver", + "<node path> <driver>\n" + "bind <class> <index> <driver>\n" +); + +U_BOOT_CMD( + unbind, 4, 0, do_bind_unbind, + "Unbind a device from a driver", + "<node path>\n" + "unbind <class> <index>\n" + "unbind <class> <index> <driver>\n" +); diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index afc3429..2613368 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -33,6 +33,7 @@ CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y CONFIG_CMD_MEMTEST=y CONFIG_CMD_MX_CYCLIC=y +CONFIG_CMD_BIND=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPT=y diff --git a/test/py/tests/test_bind.py b/test/py/tests/test_bind.py new file mode 100644 index 0000000..f21b705 --- /dev/null +++ b/test/py/tests/test_bind.py @@ -0,0 +1,178 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + +import os.path +import pytest +import re + +def in_tree(response, name, uclass, drv, depth, last_child): + lines = [x.strip() for x in response.splitlines()] + leaf = ' ' * 4 * depth; + if not last_child: + leaf = leaf + '|' + else: + leaf = leaf + '`' + leaf = leaf + '-- ' + name + line = ' *{:10.10} [0-9]* [ [ +] ] {:10.10} {}$'.format(uclass, drv,leaf) + prog = re.compile(line) + for l in lines: + if prog.match(l): + return True + return False + + +@pytest.mark.buildconfigspec('cmd_bind') +def test_bind_unbind_with_node(u_boot_console): + + #bind /bind-test. Device should come up as well as its children + response = u_boot_console.run_command("bind /bind-test generic_simple_bus") + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test", "simple_bus", "generic_simple", 0, True) + assert in_tree(tree, "bind-test-child1", "phy", "phy_sandbox", 1, False) + assert in_tree(tree, "bind-test-child2", "simple_bus", "generic_simple", 1, True) + + #Unbind child #1. No error expected and all devices should be there except for bind-test-child1 + response = u_boot_console.run_command("unbind /bind-test/bind-test-child1") + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test", "simple_bus", "generic_simple", 0, True) + assert "bind-test-child1" not in tree + assert in_tree(tree, "bind-test-child2", "simple_bus", "generic_simple", 1, True) + + #bind child #1. No error expected and all devices should be there + response = u_boot_console.run_command("bind /bind-test/bind-test-child1 phy_sandbox") + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test", "simple_bus", "generic_simple", 0, True) + assert in_tree(tree, "bind-test-child1", "phy", "phy_sandbox", 1, True) + assert in_tree(tree, "bind-test-child2", "simple_bus", "generic_simple", 1, False) + + #Unbind child #2. No error expected and all devices should be there except for bind-test-child2 + response = u_boot_console.run_command("unbind /bind-test/bind-test-child2") + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test", "simple_bus", "generic_simple", 0, True) + assert in_tree(tree, "bind-test-child1", "phy", "phy_sandbox", 1, True) + assert "bind-test-child2" not in tree + + + #Bind child #2. No error expected and all devices should be there + response = u_boot_console.run_command("bind /bind-test/bind-test-child2 generic_simple_bus") + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test", "simple_bus", "generic_simple", 0, True) + assert in_tree(tree, "bind-test-child1", "phy", "phy_sandbox", 1, False) + assert in_tree(tree, "bind-test-child2", "simple_bus", "generic_simple", 1, True) + + #Unbind parent. No error expected. All devices should be removed and unbound + response = u_boot_console.run_command("unbind /bind-test") + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert "bind-test" not in tree + assert "bind-test-child1" not in tree + assert "bind-test-child2" not in tree + + #try binding invalid node with valid driver + response = u_boot_console.run_command("bind /not-a-valid-node generic_simple_bus") + assert response != '' + tree = u_boot_console.run_command("dm tree") + assert "not-a-valid-node" not in tree + + #try binding valid node with invalid driver + response = u_boot_console.run_command("bind /bind-test not_a_driver") + assert response != '' + tree = u_boot_console.run_command("dm tree") + assert "bind-test" not in tree + + #bind /bind-test. Device should come up as well as its children + response = u_boot_console.run_command("bind /bind-test generic_simple_bus") + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test", "simple_bus", "generic_simple", 0, True) + assert in_tree(tree, "bind-test-child1", "phy", "phy_sandbox", 1, False) + assert in_tree(tree, "bind-test-child2", "simple_bus", "generic_simple", 1, True) + + response = u_boot_console.run_command("unbind /bind-test") + assert response == '' + +def get_next_line(tree, name): + treelines = [x.strip() for x in tree.splitlines() if x.strip()] + child_line = "" + for idx, line in enumerate(treelines): + if ("-- " + name) in line: + try: + child_line = treelines[idx+1] + except: + pass + break + return child_line + +@pytest.mark.buildconfigspec('cmd_bind') +def test_bind_unbind_with_uclass(u_boot_console): + #bind /bind-test + response = u_boot_console.run_command("bind /bind-test generic_simple_bus") + assert response == '' + + #make sure bind-test-child2 is there and get its uclass/index pair + tree = u_boot_console.run_command("dm tree") + child2_line = [x.strip() for x in tree.splitlines() if "-- bind-test-child2" in x] + assert len(child2_line) == 1 + + child2_uclass = child2_line[0].split()[0] + child2_index = int(child2_line[0].split()[1]) + + #bind generic_simple_bus as a child of bind-test-child2 + response = u_boot_console.run_command("bind {} {} generic_simple_bus".format(child2_uclass, child2_index, "generic_simple_bus")) + + #check that the child is there and its uclass/index pair is right + tree = u_boot_console.run_command("dm tree") + + child_of_child2_line = get_next_line(tree, "bind-test-child2") + assert child_of_child2_line + child_of_child2_index = int(child_of_child2_line.split()[1]) + assert in_tree(tree, "generic_simple_bus", "simple_bus", "generic_simple_bus", 2, True) + assert child_of_child2_index == child2_index + 1 + + #unbind the child and check it has been removed + response = u_boot_console.run_command("unbind simple_bus {}".format(child_of_child2_index)) + assert response == '' + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test-child2", "simple_bus", "generic_simple", 1, True) + assert not in_tree(tree, "generic_simple_bus", "simple_bus", "generic_simple_bus", 2, True) + child_of_child2_line = get_next_line(tree, "bind-test-child2") + assert child_of_child2_line == "" + + #bind generic_simple_bus as a child of bind-test-child2 + response = u_boot_console.run_command("bind {} {} generic_simple_bus".format(child2_uclass, child2_index, "generic_simple_bus")) + + #check that the child is there and its uclass/index pair is right + tree = u_boot_console.run_command("dm tree") + treelines = [x.strip() for x in tree.splitlines() if x.strip()] + + child_of_child2_line = get_next_line(tree, "bind-test-child2") + assert child_of_child2_line + child_of_child2_index = int(child_of_child2_line.split()[1]) + assert in_tree(tree, "generic_simple_bus", "simple_bus", "generic_simple_bus", 2, True) + assert child_of_child2_index == child2_index + 1 + + #unbind the child and check it has been removed + response = u_boot_console.run_command("unbind {} {} generic_simple_bus".format(child2_uclass, child2_index, "generic_simple_bus")) + assert response == '' + + tree = u_boot_console.run_command("dm tree") + assert in_tree(tree, "bind-test-child2", "simple_bus", "generic_simple", 1, True) + + child_of_child2_line = get_next_line(tree, "bind-test-child2") + assert child_of_child2_line == "" + + #unbind the child again and check it doesn't change the tree + tree_old = u_boot_console.run_command("dm tree") + response = u_boot_console.run_command("unbind {} {} generic_simple_bus".format(child2_uclass, child2_index, "generic_simple_bus")) + tree_new = u_boot_console.run_command("dm tree") + + assert response == '' + assert tree_old == tree_new + + response = u_boot_console.run_command("unbind /bind-test") + assert response == ''

On 9 August 2018 at 08:17, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
In some cases it can be useful to be able to bind a device to a driver from the command line. The obvious example is for versatile devices such as USB gadget. Another use case is when the devices are not yet ready at startup and require some setup before the drivers are bound (ex: FPGA which bitsream is fetched from a mass storage or ethernet)
usage example:
bind usb_dev_generic 0 usb_ether unbind usb_dev_generic 0 usb_ether or unbind eth 1
bind /ocp/omap_dwc3@48380000/usb@48390000 usb_ether unbind /ocp/omap_dwc3@48380000/usb@48390000
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v4:
- Fixed compiler warning: "cmd/bind.c:236:5: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized]"
Changes in v3:
- factorize code based on comments from ML
- remove the devices before unbinding them
- use device_find_global_by_ofnode() to get a device by its node.
Changes in v2:
- Make the bind/unbind command generic, not specific to usb device.
- Update the API to be able to bind/unbind based on DTS node path
- Add a Kconfig option to select the bind/unbind commands
arch/sandbox/dts/test.dts | 11 ++ cmd/Kconfig | 9 ++ cmd/Makefile | 1 + cmd/bind.c | 255 +++++++++++++++++++++++++++++++++++++++++++++ configs/sandbox_defconfig | 1 + test/py/tests/test_bind.py | 178 +++++++++++++++++++++++++++++++ 6 files changed, 455 insertions(+) create mode 100644 cmd/bind.c create mode 100644 test/py/tests/test_bind.py
Reviewed-by: Simon Glass sjg@chromium.org

On 08/09/2018 08:17 AM, Jean-Jacques Hiblot wrote:
In some cases it can be useful to be able to bind a device to a driver from the command line. The obvious example is for versatile devices such as USB gadget. Another use case is when the devices are not yet ready at startup and require some setup before the drivers are bound (ex: FPGA which bitsream is fetched from a mass storage or ethernet)
usage example:
bind usb_dev_generic 0 usb_ether unbind usb_dev_generic 0 usb_ether or unbind eth 1
bind /ocp/omap_dwc3@48380000/usb@48390000 usb_ether unbind /ocp/omap_dwc3@48380000/usb@48390000
diff --git a/test/py/tests/test_bind.py b/test/py/tests/test_bind.py
+# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
That line looks wrong.
+def in_tree(response, name, uclass, drv, depth, last_child):
- lines = [x.strip() for x in response.splitlines()]
- leaf = ' ' * 4 * depth;
- if not last_child:
leaf = leaf + '\|'
- else:
leaf = leaf + '`'
- leaf = leaf + '-- ' + name
- line = ' *{:10.10} [0-9]* [ [ +] ] {:10.10} {}$'.format(uclass, drv,leaf)
Does Python 2 support that interpolation format? test/py should support both Python 2 and 3.
+@pytest.mark.buildconfigspec('cmd_bind') +def test_bind_unbind_with_node(u_boot_console):
- #bind /bind-test. Device should come up as well as its children
- response = u_boot_console.run_command("bind /bind-test generic_simple_bus")
Nit: There are 2 spaces after "bind".
- #Unbind child #2. No error expected and all devices should be there except for bind-test-child2
...
- assert "bind-test-child2" not in tree
- #Bind child #2. No error expected and all devices should be there
Nit: No need for 2 blank lines there.
- #try binding invalid node with valid driver
- response = u_boot_console.run_command("bind /not-a-valid-node generic_simple_bus")
- assert response != ''
Should this check for a specific (partial) error message or format of message?
- #bind /bind-test. Device should come up as well as its children
...
- response = u_boot_console.run_command("unbind /bind-test")
- assert response == ''
Shouldn't this validate the dm tree output right at the end too?
+def get_next_line(tree, name):
- treelines = [x.strip() for x in tree.splitlines() if x.strip()]
- child_line = ""
- for idx, line in enumerate(treelines):
if ("-- " + name) in line:
try:
child_line = treelines[idx+1]
except:
pass
Squashing all exceptions seems a little drastic. What exceptions happen and why?
+@pytest.mark.buildconfigspec('cmd_bind') +def test_bind_unbind_with_uclass(u_boot_console):
...
- #check that the child is there and its uclass/index pair is right
- tree = u_boot_console.run_command("dm tree")
- treelines = [x.strip() for x in tree.splitlines() if x.strip()]
I don't /think/ treelines is used anywhere in this function?

Hi Jean-Jacques,
This series implements 2 fixes to be able to use USB Ethernet gadget with the dwc3 driver. It also adds new commands to bind/unbind a device to/from a driver and update the 'dm tree' command to make it easier to use those new commands. The bind/unbind commands can be used to bind the DWC3 USB gadget to the usb_ether driver from the command line instead of relying on platform code.
Changes in v4:
- Fixed compiler warning: "cmd/bind.c:236:5: warning: ‘ret’ may be
used uninitialized in this function [-Wmaybe-uninitialized]"
With the v3 Michal Simek asked (I was out of the office) if we shall wait for sandbox tests or not?
Is there any follow up plan for adding tests to sandbox?
Changes in v3:
- update commit log
- fixed problem with the function name
- update commit log
- new commit
- new
- factorize code based on comments from ML
- remove the devices before unbinding them
- use device_find_global_by_ofnode() to get a device by its node.
Changes in v2:
- Make the bind/unbind command generic, not specific to usb device.
- Update the API to be able to bind/unbind based on DTS node path
- Add a Kconfig option to select the bind/unbind commands
Jean-Jacques Hiblot (7): usb: gadget: Add bcdDevice for the DWC3 USB Gadget Controller net: eth-uclass: Fix for DM USB ethernet support uclass: Add dev_get_uclass_index() to get the uclass/index of a device dm: print the index of the device when dumping the dm tree dm: convert device_get_global_by_of_offset() to device_get_global_by_ofnode() device: expose the functions used to remove and unbind children of a device cmd: Add bind/unbind commands to bind a device to a driver from the command line
arch/arm/mach-rockchip/rk3188-board-spl.c | 2 +- arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +- arch/sandbox/dts/test.dts | 11 ++ cmd/Kconfig | 9 ++ cmd/Makefile | 1 + cmd/bind.c | 255 ++++++++++++++++++++++++++++++ configs/sandbox_defconfig | 1 + drivers/core/device-remove.c | 30 ++-- drivers/core/device.c | 19 ++- drivers/core/dump.c | 16 +- drivers/core/uclass.c | 21 +++ drivers/usb/gadget/gadget_chips.h | 2 + include/dm/device-internal.h | 38 +++++ include/dm/device.h | 23 ++- include/dm/uclass-internal.h | 11 ++ net/eth-uclass.c | 3 +- test/py/tests/test_bind.py | 178 +++++++++++++++++++++ 17 files changed, 584 insertions(+), 38 deletions(-) create mode 100644 cmd/bind.c create mode 100644 test/py/tests/test_bind.py
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

On 11/08/2018 14:04, Lukasz Majewski wrote:
Hi Jean-Jacques,
This series implements 2 fixes to be able to use USB Ethernet gadget with the dwc3 driver. It also adds new commands to bind/unbind a device to/from a driver and update the 'dm tree' command to make it easier to use those new commands. The bind/unbind commands can be used to bind the DWC3 USB gadget to the usb_ether driver from the command line instead of relying on platform code.
Changes in v4:
- Fixed compiler warning: "cmd/bind.c:236:5: warning: ‘ret’ may be
used uninitialized in this function [-Wmaybe-uninitialized]"
With the v3 Michal Simek asked (I was out of the office) if we shall wait for sandbox tests or not?
There are sandbox tests for the bind/unbind commands provided in test/py/tests/test_bind.py.
JJ
Is there any follow up plan for adding tests to sandbox?
Changes in v3:
- update commit log
- fixed problem with the function name
- update commit log
- new commit
- new
- factorize code based on comments from ML
- remove the devices before unbinding them
- use device_find_global_by_ofnode() to get a device by its node.
Changes in v2:
- Make the bind/unbind command generic, not specific to usb device.
- Update the API to be able to bind/unbind based on DTS node path
- Add a Kconfig option to select the bind/unbind commands
Jean-Jacques Hiblot (7): usb: gadget: Add bcdDevice for the DWC3 USB Gadget Controller net: eth-uclass: Fix for DM USB ethernet support uclass: Add dev_get_uclass_index() to get the uclass/index of a device dm: print the index of the device when dumping the dm tree dm: convert device_get_global_by_of_offset() to device_get_global_by_ofnode() device: expose the functions used to remove and unbind children of a device cmd: Add bind/unbind commands to bind a device to a driver from the command line
arch/arm/mach-rockchip/rk3188-board-spl.c | 2 +- arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +- arch/sandbox/dts/test.dts | 11 ++ cmd/Kconfig | 9 ++ cmd/Makefile | 1 + cmd/bind.c | 255 ++++++++++++++++++++++++++++++ configs/sandbox_defconfig | 1 + drivers/core/device-remove.c | 30 ++-- drivers/core/device.c | 19 ++- drivers/core/dump.c | 16 +- drivers/core/uclass.c | 21 +++ drivers/usb/gadget/gadget_chips.h | 2 + include/dm/device-internal.h | 38 +++++ include/dm/device.h | 23 ++- include/dm/uclass-internal.h | 11 ++ net/eth-uclass.c | 3 +- test/py/tests/test_bind.py | 178 +++++++++++++++++++++ 17 files changed, 584 insertions(+), 38 deletions(-) create mode 100644 cmd/bind.c create mode 100644 test/py/tests/test_bind.py
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
participants (4)
-
Jean-Jacques Hiblot
-
Lukasz Majewski
-
Simon Glass
-
Stephen Warren