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

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

[Jean-Jacques Hiblot]

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 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 (5):
  usb: gadget: Add bcdDevice for the DWC3 USB Gadget Controller
  net: eth-uclass: Fix for DM USB ethernet support
  cmd: Add bind/unbind commands to bind a device to a driver from the
    command line
  drivers: 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

 cmd/Kconfig                       |   9 ++
 cmd/Makefile                      |   1 +
 cmd/bind.c                        | 256 ++++++++++++++++++++++++++++++++++++++
 drivers/core/dump.c               |  16 ++-
 drivers/core/uclass.c             |  21 ++++
 drivers/usb/gadget/gadget_chips.h |   2 +
 include/dm/uclass-internal.h      |  11 ++
 net/eth-uclass.c                  |   3 +-
 8 files changed, 312 insertions(+), 7 deletions(-)
 create mode 100644 cmd/bind.c

-- 
2.7.4

[U-Boot] [PATCH v2 1/5] usb: gadget: Add bcdDevice for the DWC3 USB Gadget Controller

[Jean-Jacques Hiblot]

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 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;
 }
-- 
2.7.4

[U-Boot] [PATCH v2 2/5] net: eth-uclass: Fix for DM USB ethernet support

[Jean-Jacques Hiblot]

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 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 e4b4922..3a5217a 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)
-- 
2.7.4

[U-Boot] [PATCH v2 3/5] cmd: Add bind/unbind commands to bind a device to a driver from the command line

[Jean-Jacques Hiblot]

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 at 48380000/usb at 48390000 usb_ether
unbind /ocp/omap_dwc3 at 48380000/usb at 48390000

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

---

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

 cmd/Kconfig  |   9 +++
 cmd/Makefile |   1 +
 cmd/bind.c   | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 266 insertions(+)
 create mode 100644 cmd/bind.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 1eb55e5..d6bbfba 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 e0088df..b12aca3 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..60d84d6
--- /dev/null
+++ b/cmd/bind.c
@@ -0,0 +1,256 @@
+// 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 unbind_by_class_index(const char *uclass, int index)
+{
+	static enum uclass_id uclass_id;
+	struct udevice *dev;
+	int ret;
+
+	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, &dev);
+	if (!dev || ret) {
+		printf("Cannot find device %d of class %s\n", index, uclass);
+		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)
+{
+	static enum uclass_id uclass_id;
+	struct udevice *parent;
+	struct udevice *dev, *n;
+	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;
+	}
+
+	list_for_each_entry_safe(dev, n, &parent->child_head, sibling_node) {
+		int rc;
+
+		if (dev->driver != drv)
+			continue;
+
+		rc = device_unbind(dev);
+		if (rc && !ret) {
+			printf("failed to unbind %s/%s\n", dev->name,
+			       drv->name);
+			ret = rc;
+		}
+	}
+
+	return ret;
+}
+
+static int bind_by_node_path(const char *path, const char *drv_name)
+{
+	struct udevice *dev;
+	struct udevice *parent = NULL;
+	int ret;
+	int id;
+	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)) {
+		for (id = 0; id < UCLASS_COUNT; id++) {
+			if (!uclass_find_device_by_ofnode(id, ofnode, &parent))
+				break;
+		}
+		if (parent)
+			break;
+		ofnode = ofnode_get_parent(ofnode);
+	}
+
+	if (!parent) {
+		printf("Cannot find a parent device for node path %s\n", path);
+		return -ENODEV;
+	}
+
+	ret = device_bind_with_driver_data(parent, drv, drv->name, 0,
+					   ofnode_path(path), &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;
+	int id;
+	ofnode ofnode;
+
+	ofnode = ofnode_path(path);
+	if (!ofnode_valid(ofnode)) {
+		printf("%s is not a valid node path\n", path);
+		return -EINVAL;
+	}
+
+	for (id = 0; id < UCLASS_COUNT; id++) {
+		if (uclass_find_device_by_ofnode(id, ofnode, &dev) == 0)
+			break;
+	}
+
+	if (!dev) {
+		printf("Cannot find a device with path %s\n", path);
+		return -ENODEV;
+	}
+
+	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;
+	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"
+);
-- 
2.7.4

[U-Boot] [PATCH v2 4/5] drivers: uclass: Add dev_get_uclass_index() to get the uclass/index of a device

[Jean-Jacques Hiblot]

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---

Changes in v2: New

 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 0085d3f..6efce20 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_find_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
-- 
2.7.4

[U-Boot] [PATCH v2 5/5] dm: print the index of the device when dumping the dm tree

[Jean-Jacques Hiblot]

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>

---

Changes in v2: New

 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");
 	}
-- 
2.7.4

[U-Boot] [PATCH v2 3/5] cmd: Add bind/unbind commands to bind a device to a driver from the command line

[Joe Hershberger]

On Mon, Jun 18, 2018 at 8:56 AM, 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 at 48380000/usb at 48390000 usb_ether
> unbind /ocp/omap_dwc3 at 48380000/usb at 48390000
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>

Looks great!

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH v2 3/5] cmd: Add bind/unbind commands to bind a device to a driver from the command line

[Simon Glass]

Hi Jean-Jacques,

On 18 June 2018 at 07:56, 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 at 48380000/usb at 48390000 usb_ether
> unbind /ocp/omap_dwc3 at 48380000/usb at 48390000
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>
> ---
>
> 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
>
>  cmd/Kconfig  |   9 +++
>  cmd/Makefile |   1 +
>  cmd/bind.c   | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 266 insertions(+)
>  create mode 100644 cmd/bind.c

This looks good to me, with a few comments below. I think it is great
to get this sort of functionality in U-Boot.

Please can you add a test which calls issues a few of these commands?
You might find test_shell_basics.py useful as a starting point.

>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 1eb55e5..d6bbfba 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 e0088df..b12aca3 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..60d84d6
> --- /dev/null
> +++ b/cmd/bind.c
> @@ -0,0 +1,256 @@
> +// 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 unbind_by_class_index(const char *uclass, int index)
> +{
> +       static enum uclass_id uclass_id;
> +       struct udevice *dev;
> +       int ret;
> +
> +       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, &dev);
> +       if (!dev || ret) {
> +               printf("Cannot find device %d of class %s\n", index, uclass);
> +               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)
> +{
> +       static enum uclass_id uclass_id;
> +       struct udevice *parent;
> +       struct udevice *dev, *n;
> +       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;
> +       }

The above two blocks duplicate code in the function above. Can you put
then in a separate function can call it here?

> +
> +       list_for_each_entry_safe(dev, n, &parent->child_head, sibling_node) {

This feels like code that should be in drivers/core since it is
messing with the internal lists and driver names. But I suppose for
now we can keep it here. Let's see how things expand later.

> +               int rc;
> +
> +               if (dev->driver != drv)
> +                       continue;
> +
> +               rc = device_unbind(dev);
> +               if (rc && !ret) {
> +                       printf("failed to unbind %s/%s\n", dev->name,
> +                              drv->name);
> +                       ret = rc;
> +               }
> +       }
> +
> +       return ret;
> +}
> +
> +static int bind_by_node_path(const char *path, const char *drv_name)
> +{
> +       struct udevice *dev;
> +       struct udevice *parent = NULL;
> +       int ret;
> +       int id;
> +       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)) {
> +               for (id = 0; id < UCLASS_COUNT; id++) {
> +                       if (!uclass_find_device_by_ofnode(id, ofnode, &parent))
> +                               break;
> +               }

Can you create a function in drivers/core that finds an ofnode
globally? I don't like having this logic here.

You could convert device_get_global_by_of_offset() to use an ofnode.

Remember that any printf() calls should be here, not in drivers/core.

> +               if (parent)
> +                       break;
> +               ofnode = ofnode_get_parent(ofnode);
> +       }
> +
> +       if (!parent) {
> +               printf("Cannot find a parent device for node path %s\n", path);
> +               return -ENODEV;
> +       }
> +
> +       ret = device_bind_with_driver_data(parent, drv, drv->name, 0,
> +                                          ofnode_path(path), &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;
> +       int id;
> +       ofnode ofnode;
> +
> +       ofnode = ofnode_path(path);
> +       if (!ofnode_valid(ofnode)) {
> +               printf("%s is not a valid node path\n", path);
> +               return -EINVAL;
> +       }
> +
> +       for (id = 0; id < UCLASS_COUNT; id++) {
> +               if (uclass_find_device_by_ofnode(id, ofnode, &dev) == 0)
> +                       break;
> +       }

Same comment as above - move this logic to drivers/core

> +
> +       if (!dev) {
> +               printf("Cannot find a device with path %s\n", path);
> +               return -ENODEV;
> +       }
> +
> +       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;
> +       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"
> +);
> --
> 2.7.4
>

Regards,
Simon

[U-Boot] [PATCH v2 4/5] drivers: uclass: Add dev_get_uclass_index() to get the uclass/index of a device

[Simon Glass]

Hi Jean-Jacques,

On 18 June 2018 at 07:56, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---

Please add a commit message with motivation and purpose.

Also please add a test for this (e.g. in test/dm/core.c

>
> Changes in v2: New
>
>  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 0085d3f..6efce20 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_find_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);

find or get? It should be find, since it doesn't probe anything.

> +
> +/**
>   * uclass_find_device() - Return n-th child of uclass
>   * @id:                Id number of the uclass
>   * @index:     Position of the child in uclass's list
> --
> 2.7.4
>


Regards,
Simon

[U-Boot] [PATCH v2 5/5] dm: print the index of the device when dumping the dm tree

[Simon Glass]

On 18 June 2018 at 07:56, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> 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>
>
> ---
>
> Changes in v2: New
>
>  drivers/core/dump.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 4/5] drivers: uclass: Add dev_get_uclass_index() to get the uclass/index of a device

[Michal Simek]

On 18.6.2018 15:56, Jean-Jacques Hiblot wrote:
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
> 
> Changes in v2: New
> 
>  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 0085d3f..6efce20 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_find_uclass_index(struct udevice *dev, struct uclass **ucp)

This function doesn't fit with description or even with declaration below.

Thanks,
Michal

> +{
> +	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
> 


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180621/f94168c8/attachment.sig>

[U-Boot] [PATCH v2 3/5] cmd: Add bind/unbind commands to bind a device to a driver from the command line

[Michal Simek]

On 18.6.2018 15:56, 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 at 48380000/usb at 48390000 usb_ether
> unbind /ocp/omap_dwc3 at 48380000/usb at 48390000
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> 
> ---
> 
> 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
> 
>  cmd/Kconfig  |   9 +++
>  cmd/Makefile |   1 +
>  cmd/bind.c   | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 266 insertions(+)
>  create mode 100644 cmd/bind.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 1eb55e5..d6bbfba 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 e0088df..b12aca3 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..60d84d6
> --- /dev/null
> +++ b/cmd/bind.c
> @@ -0,0 +1,256 @@
> +// 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 unbind_by_class_index(const char *uclass, int index)
> +{
> +	static enum uclass_id uclass_id;
> +	struct udevice *dev;
> +	int ret;
> +
> +	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, &dev);
> +	if (!dev || ret) {
> +		printf("Cannot find device %d of class %s\n", index, uclass);
> +		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)
> +{
> +	static enum uclass_id uclass_id;
> +	struct udevice *parent;
> +	struct udevice *dev, *n;
> +	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;
> +	}
> +
> +	list_for_each_entry_safe(dev, n, &parent->child_head, sibling_node) {
> +		int rc;
> +
> +		if (dev->driver != drv)
> +			continue;
> +
> +		rc = device_unbind(dev);
> +		if (rc && !ret) {
> +			printf("failed to unbind %s/%s\n", dev->name,
> +			       drv->name);
> +			ret = rc;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int bind_by_node_path(const char *path, const char *drv_name)
> +{
> +	struct udevice *dev;
> +	struct udevice *parent = NULL;
> +	int ret;
> +	int id;
> +	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)) {
> +		for (id = 0; id < UCLASS_COUNT; id++) {
> +			if (!uclass_find_device_by_ofnode(id, ofnode, &parent))
> +				break;
> +		}
> +		if (parent)
> +			break;
> +		ofnode = ofnode_get_parent(ofnode);
> +	}
> +
> +	if (!parent) {
> +		printf("Cannot find a parent device for node path %s\n", path);
> +		return -ENODEV;
> +	}
> +
> +	ret = device_bind_with_driver_data(parent, drv, drv->name, 0,
> +					   ofnode_path(path), &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;
> +	int id;
> +	ofnode ofnode;
> +
> +	ofnode = ofnode_path(path);
> +	if (!ofnode_valid(ofnode)) {
> +		printf("%s is not a valid node path\n", path);
> +		return -EINVAL;
> +	}
> +
> +	for (id = 0; id < UCLASS_COUNT; id++) {
> +		if (uclass_find_device_by_ofnode(id, ofnode, &dev) == 0)
> +			break;
> +	}
> +
> +	if (!dev) {
> +		printf("Cannot find a device with path %s\n", path);
> +		return -ENODEV;
> +	}
> +
> +	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;
> +	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;

I am getting compilation warnings bout this ret handling.

cmd/bind.c: In function ‘do_bind_unbind’:
cmd/bind.c:237:5: warning: ‘ret’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
  if (ret)
     ^



> +}
> +
> +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"
> +);
> 

I have played with this series with zynq_gpio driver on zcu100.
It can be only one instance of this driver but I can call bind multiple
times and instance is listed with new index. Log below.

ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [   ]   gpio_zynq   |   |-- gpio at ff0a0000
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP> bind /amba/gpio at ff0a0000 gpio_zynq
EFI: Initializing UCLASS_EFI
Calling zynq_gpio_bind
ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [   ]   gpio_zynq   |   |-- gpio at ff0a0000
 gpio        1  [   ]   gpio_zynq   |   |   `-- gpio_zynq
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP>

I can read dev->seq in probe and fail if dev->seq is not 0 but not sure
if this should be checked in probe.

The second thing is that when I call unbind for probed device that it is
failing because it hasn't been removed.

ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [   ]   gpio_zynq   |   |-- gpio at ff0a0000
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP> gpio status 0
Calling zynq_gpio_ofdata_to_platdata
Calling zynq_gpio_probe 0
ZynqMP> dm tree
 Class    index  Probed  Driver      Name
-----------------------------------------
 root        0  [ + ]   root_drive  root_driver
 serial      0  [   ]   arm_dcc     |-- dcc
 simple_bus  0  [   ]   generic_si  |-- amba_apu at 0
 simple_bus  1  [ + ]   generic_si  |-- amba
 gpio        0  [ + ]   gpio_zynq   |   |-- gpio at ff0a0000
 mmc         0  [ + ]   arasan_sdh  |   |-- sdhci at ff160000
...
ZynqMP> unbind /amba/gpio at ff0a0000
EFI: Initializing UCLASS_EFI
activated
Unable to unbind. err:-22

That's why I am curious if this command enable to unbind device then
there should be also an option to remove that device too.

More generic note. I have been taking information about structure from
dm tree command and I would consider to move test/dm/cmd_dm.c to cmd
folder and extend this file by these new functionality.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180621/e152deb0/attachment.sig>