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

[U-Boot] [PATCH v3 0/7] 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 v3:
- update some commit logs
- 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

-- 
2.7.4

[U-Boot] [PATCH v3 1/7] 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 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;
 }
-- 
2.7.4

[U-Boot] [PATCH v3 2/7] 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 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 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 v3 3/7] uclass: Add dev_get_uclass_index() to get the uclass/index of a device

[Jean-Jacques Hiblot]

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 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 0085d3f..3af973c 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
-- 
2.7.4

[U-Boot] [PATCH v3 3/7] uclass: Add dev_get_uclass_index() to get the uclass/index of a device

[Simon Glass]

Hi Jean-Jacques,

On 22 June 2018 at 05:25, 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 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>

But this does need a sandbox test in test/dm/ofnode.c.

[U-Boot] [PATCH v3 3/7] uclass: Add dev_get_uclass_index() to get the uclass/index of a device

[Jean-Jacques Hiblot]

On 30/06/2018 06:19, Simon Glass wrote:
> Hi Jean-Jacques,
>
> On 22 June 2018 at 05:25, 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 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>
>
> But this does need a sandbox test in test/dm/ofnode.c.
I will not be able to add one before a few weeks.

JJ
>

[U-Boot] [PATCH v3 4/7] 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>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

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

[U-Boot] [PATCH v3 5/7] dm: convert device_get_global_by_of_offset() to device_get_global_by_ofnode()

[Jean-Jacques Hiblot]

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 v3:
- New

 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 e048e1a..9db3ffd 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -579,16 +579,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;
 	}
@@ -596,11 +596,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
-- 
2.7.4

[U-Boot] [PATCH v3 5/7] dm: convert device_get_global_by_of_offset() to device_get_global_by_ofnode()

[Simon Glass]

On 22 June 2018 at 05:25, 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 v3:
> - New
>
>  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(-)

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

[U-Boot] [PATCH v3 6/7] device: expose the functions used to remove and unbind children of a device

[Jean-Jacques Hiblot]

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 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 5a4d50c..14b5c1b 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -127,6 +127,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
-- 
2.7.4

[U-Boot] [PATCH v3 6/7] device: expose the functions used to remove and unbind children of a device

[Simon Glass]

On 22 June 2018 at 05:25, 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 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>

[U-Boot] [PATCH v3 7/7] 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 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.
- Added tests

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 5752bf5..8c789b3 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 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..a213d26
--- /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;
+	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 2fc84a1..cb4fdf4 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
+
+
+ at 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
+
+ at 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 == ''
-- 
2.7.4

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

[Michal Simek]

On 22.6.2018 14:25, 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 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.
> - Added tests
> 
> 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 5752bf5..8c789b3 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 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..a213d26
> --- /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;
> +	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 2fc84a1..cb4fdf4 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
> +
> +
> + at 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
> +
> + at 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 == ''
> 


I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
is working fine for me.
I have also tried to use bind/unbind for gpio zynqmp driver and it is
also working fine.

It means
Tested-by: Michal Simek <michal.simek@xilinx.com>

I would prefer if these commands are called as dm bind and dm unbind
instead of simply bind/unbind which should also fit to dm command
description "dm - Driver model low level access".


Thanks,
Michal

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

[Simon Glass]

On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
> On 22.6.2018 14:25, 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 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.
>> - Added tests
>>
>> 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>

Nice work

[...]

>
> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
> is working fine for me.
> I have also tried to use bind/unbind for gpio zynqmp driver and it is
> also working fine.
>
> It means
> Tested-by: Michal Simek <michal.simek@xilinx.com>
>
> I would prefer if these commands are called as dm bind and dm unbind
> instead of simply bind/unbind which should also fit to dm command
> description "dm - Driver model low level access".

Yes i can see the point. But after thinking about it, maybe it is best
as it is? After all driver model is fundamental to U-Boot and it's not
clear what else we might bind/unbind.

I'd like to get other opinions here, too.

Regards,
Simon

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

[Michal Simek]

On 30.6.2018 06:19, Simon Glass wrote:
> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 22.6.2018 14:25, 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 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.
>>> - Added tests
>>>
>>> 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>
> 
> Nice work
> 
> [...]
> 
>>
>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
>> is working fine for me.
>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
>> also working fine.
>>
>> It means
>> Tested-by: Michal Simek <michal.simek@xilinx.com>
>>
>> I would prefer if these commands are called as dm bind and dm unbind
>> instead of simply bind/unbind which should also fit to dm command
>> description "dm - Driver model low level access".
> 
> Yes i can see the point. But after thinking about it, maybe it is best
> as it is? After all driver model is fundamental to U-Boot and it's not
> clear what else we might bind/unbind.
> 
> I'd like to get other opinions here, too.

Tom/Marek: Any opinion?

Thanks,
Michal

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

[Tom Rini]

On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
> On 30.6.2018 06:19, Simon Glass wrote:
> > On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
> >> On 22.6.2018 14:25, 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 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.
> >>> - Added tests
> >>>
> >>> 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>
> > 
> > Nice work
> > 
> > [...]
> > 
> >>
> >> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
> >> is working fine for me.
> >> I have also tried to use bind/unbind for gpio zynqmp driver and it is
> >> also working fine.
> >>
> >> It means
> >> Tested-by: Michal Simek <michal.simek@xilinx.com>
> >>
> >> I would prefer if these commands are called as dm bind and dm unbind
> >> instead of simply bind/unbind which should also fit to dm command
> >> description "dm - Driver model low level access".
> > 
> > Yes i can see the point. But after thinking about it, maybe it is best
> > as it is? After all driver model is fundamental to U-Boot and it's not
> > clear what else we might bind/unbind.
> > 
> > I'd like to get other opinions here, too.
> 
> Tom/Marek: Any opinion?

I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
generic terms and making it clear it's part of working "inside" of DM to
hook/unhook things by making it a sub-command of dm sounds good.
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180709/6e961586/attachment.sig>

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

[Joe Hershberger]

On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
>> On 30.6.2018 06:19, Simon Glass wrote:
>> > On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
>> >> On 22.6.2018 14:25, 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 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.
>> >>> - Added tests
>> >>>
>> >>> 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>
>> >
>> > Nice work
>> >
>> > [...]
>> >
>> >>
>> >> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
>> >> is working fine for me.
>> >> I have also tried to use bind/unbind for gpio zynqmp driver and it is
>> >> also working fine.
>> >>
>> >> It means
>> >> Tested-by: Michal Simek <michal.simek@xilinx.com>
>> >>
>> >> I would prefer if these commands are called as dm bind and dm unbind
>> >> instead of simply bind/unbind which should also fit to dm command
>> >> description "dm - Driver model low level access".
>> >
>> > Yes i can see the point. But after thinking about it, maybe it is best
>> > as it is? After all driver model is fundamental to U-Boot and it's not
>> > clear what else we might bind/unbind.
>> >
>> > I'd like to get other opinions here, too.
>>
>> Tom/Marek: Any opinion?
>
> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
> generic terms and making it clear it's part of working "inside" of DM to
> hook/unhook things by making it a sub-command of dm sounds good.
> Thanks!

I agree with Simon here. I think bind and unbind won't have any
plausible other meaning in U-Boot and DM is core to U-Boot
functionality in the new world. I think it would be OK to have "dm
bind" alias to "bind" if that's a point of confusion, but having it
top-level seems good to me.

-Joe

> --
> Tom
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>

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

[Tom Rini]

On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
> > On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
> >> On 30.6.2018 06:19, Simon Glass wrote:
> >> > On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
> >> >> On 22.6.2018 14:25, 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 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.
> >> >>> - Added tests
> >> >>>
> >> >>> 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>
> >> >
> >> > Nice work
> >> >
> >> > [...]
> >> >
> >> >>
> >> >> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
> >> >> is working fine for me.
> >> >> I have also tried to use bind/unbind for gpio zynqmp driver and it is
> >> >> also working fine.
> >> >>
> >> >> It means
> >> >> Tested-by: Michal Simek <michal.simek@xilinx.com>
> >> >>
> >> >> I would prefer if these commands are called as dm bind and dm unbind
> >> >> instead of simply bind/unbind which should also fit to dm command
> >> >> description "dm - Driver model low level access".
> >> >
> >> > Yes i can see the point. But after thinking about it, maybe it is best
> >> > as it is? After all driver model is fundamental to U-Boot and it's not
> >> > clear what else we might bind/unbind.
> >> >
> >> > I'd like to get other opinions here, too.
> >>
> >> Tom/Marek: Any opinion?
> >
> > I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
> > generic terms and making it clear it's part of working "inside" of DM to
> > hook/unhook things by making it a sub-command of dm sounds good.
> > Thanks!
> 
> I agree with Simon here. I think bind and unbind won't have any
> plausible other meaning in U-Boot and DM is core to U-Boot
> functionality in the new world. I think it would be OK to have "dm
> bind" alias to "bind" if that's a point of confusion, but having it
> top-level seems good to me.

They're still very generic words for something that's part of working
under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
it's supposed to be only for test/debug type work, yes, OK, I'll change
my mind.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180710/be8550fe/attachment.sig>

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

[Michal Simek]

On 10.7.2018 18:40, Tom Rini wrote:
> On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
>> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
>>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
>>>> On 30.6.2018 06:19, Simon Glass wrote:
>>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>> On 22.6.2018 14:25, 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 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.
>>>>>>> - Added tests
>>>>>>>
>>>>>>> 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>
>>>>>
>>>>> Nice work
>>>>>
>>>>> [...]
>>>>>
>>>>>>
>>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
>>>>>> is working fine for me.
>>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
>>>>>> also working fine.
>>>>>>
>>>>>> It means
>>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>
>>>>>> I would prefer if these commands are called as dm bind and dm unbind
>>>>>> instead of simply bind/unbind which should also fit to dm command
>>>>>> description "dm - Driver model low level access".
>>>>>
>>>>> Yes i can see the point. But after thinking about it, maybe it is best
>>>>> as it is? After all driver model is fundamental to U-Boot and it's not
>>>>> clear what else we might bind/unbind.
>>>>>
>>>>> I'd like to get other opinions here, too.
>>>>
>>>> Tom/Marek: Any opinion?
>>>
>>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
>>> generic terms and making it clear it's part of working "inside" of DM to
>>> hook/unhook things by making it a sub-command of dm sounds good.
>>> Thanks!
>>
>> I agree with Simon here. I think bind and unbind won't have any
>> plausible other meaning in U-Boot and DM is core to U-Boot
>> functionality in the new world. I think it would be OK to have "dm
>> bind" alias to "bind" if that's a point of confusion, but having it
>> top-level seems good to me.
> 
> They're still very generic words for something that's part of working
> under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
> it's supposed to be only for test/debug type work, yes, OK, I'll change
> my mind.

I would expect that almost everybody will enable CMD_DM where symbol is
in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
if this is the right place for this file).
When solution with bind and unbind is designed because it is giving you
clear picture what has been binded and probed. You can find paths from
DT but it is not giving you run time information.
At least for xilinx platforms when this functionality is added (which is
great) I am going to make sure that CMD_DM is also enabled because
that's the way how to check status at run time.

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/20180711/c7fe5640/attachment.sig>

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

[Tom Rini]

On Wed, Jul 11, 2018 at 07:57:13AM +0200, Michal Simek wrote:
> On 10.7.2018 18:40, Tom Rini wrote:
> > On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
> >> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
> >>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
> >>>> On 30.6.2018 06:19, Simon Glass wrote:
> >>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
> >>>>>> On 22.6.2018 14:25, 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 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.
> >>>>>>> - Added tests
> >>>>>>>
> >>>>>>> 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>
> >>>>>
> >>>>> Nice work
> >>>>>
> >>>>> [...]
> >>>>>
> >>>>>>
> >>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
> >>>>>> is working fine for me.
> >>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
> >>>>>> also working fine.
> >>>>>>
> >>>>>> It means
> >>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
> >>>>>>
> >>>>>> I would prefer if these commands are called as dm bind and dm unbind
> >>>>>> instead of simply bind/unbind which should also fit to dm command
> >>>>>> description "dm - Driver model low level access".
> >>>>>
> >>>>> Yes i can see the point. But after thinking about it, maybe it is best
> >>>>> as it is? After all driver model is fundamental to U-Boot and it's not
> >>>>> clear what else we might bind/unbind.
> >>>>>
> >>>>> I'd like to get other opinions here, too.
> >>>>
> >>>> Tom/Marek: Any opinion?
> >>>
> >>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
> >>> generic terms and making it clear it's part of working "inside" of DM to
> >>> hook/unhook things by making it a sub-command of dm sounds good.
> >>> Thanks!
> >>
> >> I agree with Simon here. I think bind and unbind won't have any
> >> plausible other meaning in U-Boot and DM is core to U-Boot
> >> functionality in the new world. I think it would be OK to have "dm
> >> bind" alias to "bind" if that's a point of confusion, but having it
> >> top-level seems good to me.
> > 
> > They're still very generic words for something that's part of working
> > under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
> > it's supposed to be only for test/debug type work, yes, OK, I'll change
> > my mind.
> 
> I would expect that almost everybody will enable CMD_DM where symbol is
> in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
> if this is the right place for this file).

It might well really belong as cmd/dm.c, but content wise it's debug
information that is also useful in the bind/unbind case (so you know
where U-Boot sees things as being at).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180711/3269d210/attachment.sig>

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

[Michal Simek]

On 11.7.2018 14:46, Tom Rini wrote:
> On Wed, Jul 11, 2018 at 07:57:13AM +0200, Michal Simek wrote:
>> On 10.7.2018 18:40, Tom Rini wrote:
>>> On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
>>>> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
>>>>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
>>>>>> On 30.6.2018 06:19, Simon Glass wrote:
>>>>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>> On 22.6.2018 14:25, 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 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.
>>>>>>>>> - Added tests
>>>>>>>>>
>>>>>>>>> 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>
>>>>>>>
>>>>>>> Nice work
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>>
>>>>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
>>>>>>>> is working fine for me.
>>>>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
>>>>>>>> also working fine.
>>>>>>>>
>>>>>>>> It means
>>>>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>
>>>>>>>> I would prefer if these commands are called as dm bind and dm unbind
>>>>>>>> instead of simply bind/unbind which should also fit to dm command
>>>>>>>> description "dm - Driver model low level access".
>>>>>>>
>>>>>>> Yes i can see the point. But after thinking about it, maybe it is best
>>>>>>> as it is? After all driver model is fundamental to U-Boot and it's not
>>>>>>> clear what else we might bind/unbind.
>>>>>>>
>>>>>>> I'd like to get other opinions here, too.
>>>>>>
>>>>>> Tom/Marek: Any opinion?
>>>>>
>>>>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
>>>>> generic terms and making it clear it's part of working "inside" of DM to
>>>>> hook/unhook things by making it a sub-command of dm sounds good.
>>>>> Thanks!
>>>>
>>>> I agree with Simon here. I think bind and unbind won't have any
>>>> plausible other meaning in U-Boot and DM is core to U-Boot
>>>> functionality in the new world. I think it would be OK to have "dm
>>>> bind" alias to "bind" if that's a point of confusion, but having it
>>>> top-level seems good to me.
>>>
>>> They're still very generic words for something that's part of working
>>> under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
>>> it's supposed to be only for test/debug type work, yes, OK, I'll change
>>> my mind.
>>
>> I would expect that almost everybody will enable CMD_DM where symbol is
>> in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
>> if this is the right place for this file).
> 
> It might well really belong as cmd/dm.c, but content wise it's debug
> information that is also useful in the bind/unbind case (so you know
> where U-Boot sees things as being at).

Then we should really not enable it by default for all boards with DM on.

 640 config CMD_DM
 641         bool "dm - Access to driver model information"
 642         depends on DM
 643         default y

Thanks,
Michal

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

[Tom Rini]

On Wed, Jul 11, 2018 at 03:31:39PM +0200, Michal Simek wrote:
> On 11.7.2018 14:46, Tom Rini wrote:
> > On Wed, Jul 11, 2018 at 07:57:13AM +0200, Michal Simek wrote:
> >> On 10.7.2018 18:40, Tom Rini wrote:
> >>> On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
> >>>> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
> >>>>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
> >>>>>> On 30.6.2018 06:19, Simon Glass wrote:
> >>>>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
> >>>>>>>> On 22.6.2018 14:25, 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 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.
> >>>>>>>>> - Added tests
> >>>>>>>>>
> >>>>>>>>> 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>
> >>>>>>>
> >>>>>>> Nice work
> >>>>>>>
> >>>>>>> [...]
> >>>>>>>
> >>>>>>>>
> >>>>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
> >>>>>>>> is working fine for me.
> >>>>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
> >>>>>>>> also working fine.
> >>>>>>>>
> >>>>>>>> It means
> >>>>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
> >>>>>>>>
> >>>>>>>> I would prefer if these commands are called as dm bind and dm unbind
> >>>>>>>> instead of simply bind/unbind which should also fit to dm command
> >>>>>>>> description "dm - Driver model low level access".
> >>>>>>>
> >>>>>>> Yes i can see the point. But after thinking about it, maybe it is best
> >>>>>>> as it is? After all driver model is fundamental to U-Boot and it's not
> >>>>>>> clear what else we might bind/unbind.
> >>>>>>>
> >>>>>>> I'd like to get other opinions here, too.
> >>>>>>
> >>>>>> Tom/Marek: Any opinion?
> >>>>>
> >>>>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
> >>>>> generic terms and making it clear it's part of working "inside" of DM to
> >>>>> hook/unhook things by making it a sub-command of dm sounds good.
> >>>>> Thanks!
> >>>>
> >>>> I agree with Simon here. I think bind and unbind won't have any
> >>>> plausible other meaning in U-Boot and DM is core to U-Boot
> >>>> functionality in the new world. I think it would be OK to have "dm
> >>>> bind" alias to "bind" if that's a point of confusion, but having it
> >>>> top-level seems good to me.
> >>>
> >>> They're still very generic words for something that's part of working
> >>> under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
> >>> it's supposed to be only for test/debug type work, yes, OK, I'll change
> >>> my mind.
> >>
> >> I would expect that almost everybody will enable CMD_DM where symbol is
> >> in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
> >> if this is the right place for this file).
> > 
> > It might well really belong as cmd/dm.c, but content wise it's debug
> > information that is also useful in the bind/unbind case (so you know
> > where U-Boot sees things as being at).
> 
> Then we should really not enable it by default for all boards with DM on.
> 
>  640 config CMD_DM
>  641         bool "dm - Access to driver model information"
>  642         depends on DM
>  643         default y

Simon?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180711/9a298a66/attachment.sig>

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

[Simon Glass]

Hi,

On 11 July 2018 at 07:40, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Jul 11, 2018 at 03:31:39PM +0200, Michal Simek wrote:
> > On 11.7.2018 14:46, Tom Rini wrote:
> > > On Wed, Jul 11, 2018 at 07:57:13AM +0200, Michal Simek wrote:
> > >> On 10.7.2018 18:40, Tom Rini wrote:
> > >>> On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
> > >>>> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
> > >>>>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
> > >>>>>> On 30.6.2018 06:19, Simon Glass wrote:
> > >>>>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
> > >>>>>>>> On 22.6.2018 14:25, 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 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.
> > >>>>>>>>> - Added tests
> > >>>>>>>>>
> > >>>>>>>>> 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>
> > >>>>>>>
> > >>>>>>> Nice work
> > >>>>>>>
> > >>>>>>> [...]
> > >>>>>>>
> > >>>>>>>>
> > >>>>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
> > >>>>>>>> is working fine for me.
> > >>>>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
> > >>>>>>>> also working fine.
> > >>>>>>>>
> > >>>>>>>> It means
> > >>>>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
> > >>>>>>>>
> > >>>>>>>> I would prefer if these commands are called as dm bind and dm unbind
> > >>>>>>>> instead of simply bind/unbind which should also fit to dm command
> > >>>>>>>> description "dm - Driver model low level access".
> > >>>>>>>
> > >>>>>>> Yes i can see the point. But after thinking about it, maybe it is best
> > >>>>>>> as it is? After all driver model is fundamental to U-Boot and it's not
> > >>>>>>> clear what else we might bind/unbind.
> > >>>>>>>
> > >>>>>>> I'd like to get other opinions here, too.
> > >>>>>>
> > >>>>>> Tom/Marek: Any opinion?
> > >>>>>
> > >>>>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
> > >>>>> generic terms and making it clear it's part of working "inside" of DM to
> > >>>>> hook/unhook things by making it a sub-command of dm sounds good.
> > >>>>> Thanks!
> > >>>>
> > >>>> I agree with Simon here. I think bind and unbind won't have any
> > >>>> plausible other meaning in U-Boot and DM is core to U-Boot
> > >>>> functionality in the new world. I think it would be OK to have "dm
> > >>>> bind" alias to "bind" if that's a point of confusion, but having it
> > >>>> top-level seems good to me.
> > >>>
> > >>> They're still very generic words for something that's part of working
> > >>> under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
> > >>> it's supposed to be only for test/debug type work, yes, OK, I'll change
> > >>> my mind.
> > >>
> > >> I would expect that almost everybody will enable CMD_DM where symbol is
> > >> in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
> > >> if this is the right place for this file).
> > >
> > > It might well really belong as cmd/dm.c, but content wise it's debug
> > > information that is also useful in the bind/unbind case (so you know
> > > where U-Boot sees things as being at).
> >
> > Then we should really not enable it by default for all boards with DM on.
> >
> >  640 config CMD_DM
> >  641         bool "dm - Access to driver model information"
> >  642         depends on DM
> >  643         default y
>
> Simon?

I'm OK with having it disabled by default - it is for debugging after all.

Regards,
Simon

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

[Michal Simek]

On 11.7.2018 22:13, Simon Glass wrote:
> Hi,
> 
> On 11 July 2018 at 07:40, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Wed, Jul 11, 2018 at 03:31:39PM +0200, Michal Simek wrote:
>>> On 11.7.2018 14:46, Tom Rini wrote:
>>>> On Wed, Jul 11, 2018 at 07:57:13AM +0200, Michal Simek wrote:
>>>>> On 10.7.2018 18:40, Tom Rini wrote:
>>>>>> On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
>>>>>>> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
>>>>>>>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
>>>>>>>>> On 30.6.2018 06:19, Simon Glass wrote:
>>>>>>>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>>>>> On 22.6.2018 14:25, 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 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.
>>>>>>>>>>>> - Added tests
>>>>>>>>>>>>
>>>>>>>>>>>> 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>
>>>>>>>>>>
>>>>>>>>>> Nice work
>>>>>>>>>>
>>>>>>>>>> [...]
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
>>>>>>>>>>> is working fine for me.
>>>>>>>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
>>>>>>>>>>> also working fine.
>>>>>>>>>>>
>>>>>>>>>>> It means
>>>>>>>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>>>
>>>>>>>>>>> I would prefer if these commands are called as dm bind and dm unbind
>>>>>>>>>>> instead of simply bind/unbind which should also fit to dm command
>>>>>>>>>>> description "dm - Driver model low level access".
>>>>>>>>>>
>>>>>>>>>> Yes i can see the point. But after thinking about it, maybe it is best
>>>>>>>>>> as it is? After all driver model is fundamental to U-Boot and it's not
>>>>>>>>>> clear what else we might bind/unbind.
>>>>>>>>>>
>>>>>>>>>> I'd like to get other opinions here, too.
>>>>>>>>>
>>>>>>>>> Tom/Marek: Any opinion?
>>>>>>>>
>>>>>>>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
>>>>>>>> generic terms and making it clear it's part of working "inside" of DM to
>>>>>>>> hook/unhook things by making it a sub-command of dm sounds good.
>>>>>>>> Thanks!
>>>>>>>
>>>>>>> I agree with Simon here. I think bind and unbind won't have any
>>>>>>> plausible other meaning in U-Boot and DM is core to U-Boot
>>>>>>> functionality in the new world. I think it would be OK to have "dm
>>>>>>> bind" alias to "bind" if that's a point of confusion, but having it
>>>>>>> top-level seems good to me.
>>>>>>
>>>>>> They're still very generic words for something that's part of working
>>>>>> under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
>>>>>> it's supposed to be only for test/debug type work, yes, OK, I'll change
>>>>>> my mind.
>>>>>
>>>>> I would expect that almost everybody will enable CMD_DM where symbol is
>>>>> in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
>>>>> if this is the right place for this file).
>>>>
>>>> It might well really belong as cmd/dm.c, but content wise it's debug
>>>> information that is also useful in the bind/unbind case (so you know
>>>> where U-Boot sees things as being at).
>>>
>>> Then we should really not enable it by default for all boards with DM on.
>>>
>>>  640 config CMD_DM
>>>  641         bool "dm - Access to driver model information"
>>>  642         depends on DM
>>>  643         default y
>>
>> Simon?
> 
> I'm OK with having it disabled by default - it is for debugging after all.

Does it make sense to simply remove that line? At least I would like to
keep it enable for microblaze/zynq/zynqmp boards but not sure about
others. Or simply update every defconfig and enable it there to have
zero diff?

Thanks,
Michal

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

[Simon Glass]

Hi Michal,

On 16 July 2018 at 02:33, Michal Simek <michal.simek@xilinx.com> wrote:
> On 11.7.2018 22:13, Simon Glass wrote:
>> Hi,
>>
>> On 11 July 2018 at 07:40, Tom Rini <trini@konsulko.com> wrote:
>>>
>>> On Wed, Jul 11, 2018 at 03:31:39PM +0200, Michal Simek wrote:
>>>> On 11.7.2018 14:46, Tom Rini wrote:
>>>>> On Wed, Jul 11, 2018 at 07:57:13AM +0200, Michal Simek wrote:
>>>>>> On 10.7.2018 18:40, Tom Rini wrote:
>>>>>>> On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
>>>>>>>> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
>>>>>>>>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
>>>>>>>>>> On 30.6.2018 06:19, Simon Glass wrote:
>>>>>>>>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>>>>>> On 22.6.2018 14:25, 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 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.
>>>>>>>>>>>>> - Added tests
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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>
>>>>>>>>>>>
>>>>>>>>>>> Nice work
>>>>>>>>>>>
>>>>>>>>>>> [...]
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
>>>>>>>>>>>> is working fine for me.
>>>>>>>>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
>>>>>>>>>>>> also working fine.
>>>>>>>>>>>>
>>>>>>>>>>>> It means
>>>>>>>>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>>>>
>>>>>>>>>>>> I would prefer if these commands are called as dm bind and dm unbind
>>>>>>>>>>>> instead of simply bind/unbind which should also fit to dm command
>>>>>>>>>>>> description "dm - Driver model low level access".
>>>>>>>>>>>
>>>>>>>>>>> Yes i can see the point. But after thinking about it, maybe it is best
>>>>>>>>>>> as it is? After all driver model is fundamental to U-Boot and it's not
>>>>>>>>>>> clear what else we might bind/unbind.
>>>>>>>>>>>
>>>>>>>>>>> I'd like to get other opinions here, too.
>>>>>>>>>>
>>>>>>>>>> Tom/Marek: Any opinion?
>>>>>>>>>
>>>>>>>>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
>>>>>>>>> generic terms and making it clear it's part of working "inside" of DM to
>>>>>>>>> hook/unhook things by making it a sub-command of dm sounds good.
>>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>> I agree with Simon here. I think bind and unbind won't have any
>>>>>>>> plausible other meaning in U-Boot and DM is core to U-Boot
>>>>>>>> functionality in the new world. I think it would be OK to have "dm
>>>>>>>> bind" alias to "bind" if that's a point of confusion, but having it
>>>>>>>> top-level seems good to me.
>>>>>>>
>>>>>>> They're still very generic words for something that's part of working
>>>>>>> under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
>>>>>>> it's supposed to be only for test/debug type work, yes, OK, I'll change
>>>>>>> my mind.
>>>>>>
>>>>>> I would expect that almost everybody will enable CMD_DM where symbol is
>>>>>> in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
>>>>>> if this is the right place for this file).
>>>>>
>>>>> It might well really belong as cmd/dm.c, but content wise it's debug
>>>>> information that is also useful in the bind/unbind case (so you know
>>>>> where U-Boot sees things as being at).
>>>>
>>>> Then we should really not enable it by default for all boards with DM on.
>>>>
>>>>  640 config CMD_DM
>>>>  641         bool "dm - Access to driver model information"
>>>>  642         depends on DM
>>>>  643         default y
>>>
>>> Simon?
>>
>> I'm OK with having it disabled by default - it is for debugging after all.
>
> Does it make sense to simply remove that line? At least I would like to
> keep it enable for microblaze/zynq/zynqmp boards but not sure about
> others. Or simply update every defconfig and enable it there to have
> zero diff?

That seems like a hassle.

One option is to change each arch to 'imply CMD_DM' and then people
can change that down the track as they want to disable it?

Regards,
Simon

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

[Michal Simek]

On 17.7.2018 05:44, Simon Glass wrote:
> Hi Michal,
> 
> On 16 July 2018 at 02:33, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 11.7.2018 22:13, Simon Glass wrote:
>>> Hi,
>>>
>>> On 11 July 2018 at 07:40, Tom Rini <trini@konsulko.com> wrote:
>>>>
>>>> On Wed, Jul 11, 2018 at 03:31:39PM +0200, Michal Simek wrote:
>>>>> On 11.7.2018 14:46, Tom Rini wrote:
>>>>>> On Wed, Jul 11, 2018 at 07:57:13AM +0200, Michal Simek wrote:
>>>>>>> On 10.7.2018 18:40, Tom Rini wrote:
>>>>>>>> On Mon, Jul 09, 2018 at 11:59:57AM -0500, Joe Hershberger wrote:
>>>>>>>>> On Mon, Jul 9, 2018 at 9:43 AM, Tom Rini <trini@konsulko.com> wrote:
>>>>>>>>>> On Mon, Jul 09, 2018 at 08:19:44AM +0200, Michal Simek wrote:
>>>>>>>>>>> On 30.6.2018 06:19, Simon Glass wrote:
>>>>>>>>>>>> On 27 June 2018 at 07:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>>>>>>> On 22.6.2018 14:25, 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 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.
>>>>>>>>>>>>>> - Added tests
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 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>
>>>>>>>>>>>>
>>>>>>>>>>>> Nice work
>>>>>>>>>>>>
>>>>>>>>>>>> [...]
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I have tested bind/unbind with dwc3 on zynqmp for ethernet gadget and it
>>>>>>>>>>>>> is working fine for me.
>>>>>>>>>>>>> I have also tried to use bind/unbind for gpio zynqmp driver and it is
>>>>>>>>>>>>> also working fine.
>>>>>>>>>>>>>
>>>>>>>>>>>>> It means
>>>>>>>>>>>>> Tested-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I would prefer if these commands are called as dm bind and dm unbind
>>>>>>>>>>>>> instead of simply bind/unbind which should also fit to dm command
>>>>>>>>>>>>> description "dm - Driver model low level access".
>>>>>>>>>>>>
>>>>>>>>>>>> Yes i can see the point. But after thinking about it, maybe it is best
>>>>>>>>>>>> as it is? After all driver model is fundamental to U-Boot and it's not
>>>>>>>>>>>> clear what else we might bind/unbind.
>>>>>>>>>>>>
>>>>>>>>>>>> I'd like to get other opinions here, too.
>>>>>>>>>>>
>>>>>>>>>>> Tom/Marek: Any opinion?
>>>>>>>>>>
>>>>>>>>>> I think dm bind/unbind makes sense, yes.  "bind" and "unbind" are pretty
>>>>>>>>>> generic terms and making it clear it's part of working "inside" of DM to
>>>>>>>>>> hook/unhook things by making it a sub-command of dm sounds good.
>>>>>>>>>> Thanks!
>>>>>>>>>
>>>>>>>>> I agree with Simon here. I think bind and unbind won't have any
>>>>>>>>> plausible other meaning in U-Boot and DM is core to U-Boot
>>>>>>>>> functionality in the new world. I think it would be OK to have "dm
>>>>>>>>> bind" alias to "bind" if that's a point of confusion, but having it
>>>>>>>>> top-level seems good to me.
>>>>>>>>
>>>>>>>> They're still very generic words for something that's part of working
>>>>>>>> under the dm framework.  That said, looking at test/dm/cmd_dm.c and that
>>>>>>>> it's supposed to be only for test/debug type work, yes, OK, I'll change
>>>>>>>> my mind.
>>>>>>>
>>>>>>> I would expect that almost everybody will enable CMD_DM where symbol is
>>>>>>> in cmd/Kconfig but implementation in test/dm/cmd_dm.c (it is a question
>>>>>>> if this is the right place for this file).
>>>>>>
>>>>>> It might well really belong as cmd/dm.c, but content wise it's debug
>>>>>> information that is also useful in the bind/unbind case (so you know
>>>>>> where U-Boot sees things as being at).
>>>>>
>>>>> Then we should really not enable it by default for all boards with DM on.
>>>>>
>>>>>  640 config CMD_DM
>>>>>  641         bool "dm - Access to driver model information"
>>>>>  642         depends on DM
>>>>>  643         default y
>>>>
>>>> Simon?
>>>
>>> I'm OK with having it disabled by default - it is for debugging after all.
>>
>> Does it make sense to simply remove that line? At least I would like to
>> keep it enable for microblaze/zynq/zynqmp boards but not sure about
>> others. Or simply update every defconfig and enable it there to have
>> zero diff?
> 
> That seems like a hassle.
> 
> One option is to change each arch to 'imply CMD_DM' and then people
> can change that down the track as they want to disable it?

I have sent patches for that. Please check.

Thanks,
Michal

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

[Jagan Teki]

On Fri, Jun 22, 2018 at 5:55 PM, 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

Apart from having separate dm command, can't we add this as a part of
'usb reset'  I'm thinking usb reset can show the all possible
controllers shutdown and probe it again so-that gadget can show it
like others. This is what I tried [1]

[1] https://patchwork.ozlabs.org/patch/941597/

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

[Eugeniu Rosca]

Hi,

On Fri, Jun 22, 2018 at 02:25:34PM +0200, Jean-Jacques Hiblot wrote:
> +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)

FWIW, gcc 7.3.0 complains at this line:

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

> +		return CMD_RET_FAILURE;
> +	else
> +		return CMD_RET_SUCCESS;
> +}

Best regards,
Eugeniu.

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

[Michal Simek]

Hi Lukasz,

On 22.6.2018 14:25, Jean-Jacques Hiblot wrote:
> 
> 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 v3:
> - update some commit logs
> - 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
> 

Lukasz: I see your name in patchwork.
Are you going to take this series? Or we are waiting for sandbox testing?

Thanks,
Michal

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

[Eugeniu Rosca]

Hello,

FWIW, patch "[7/7] cmd: Add bind/unbind commands to bind a device to a
driver from the command line" contributes with a compiler warning, as
described in https://patchwork.ozlabs.org/patch/933310/#1952127 .

Best regards,
Eugeniu.
On Wed, Aug 8, 2018 at 8:36 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Hi Lukasz,
>
> On 22.6.2018 14:25, Jean-Jacques Hiblot wrote:
> >
> > 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 v3:
> > - update some commit logs
> > - 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
> >
>
> Lukasz: I see your name in patchwork.
> Are you going to take this series? Or we are waiting for sandbox testing?
>
> Thanks,
> Michal
>

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

[Michal Simek]

On 8.8.2018 11:17, Eugeniu Rosca wrote:
> Hello,
> 
> FWIW, patch "[7/7] cmd: Add bind/unbind commands to bind a device to a
> driver from the command line" contributes with a compiler warning, as
> described in https://patchwork.ozlabs.org/patch/933310/#1952127 .

any other problem?

M

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

[Eugeniu Rosca]

On Wed, Aug 8, 2018 at 11:58 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 8.8.2018 11:17, Eugeniu Rosca wrote:
> > Hello,
> >
> > FWIW, patch "[7/7] cmd: Add bind/unbind commands to bind a device to a
> > driver from the command line" contributes with a compiler warning, as
> > described in https://patchwork.ozlabs.org/patch/933310/#1952127 .
>
> any other problem?

Not from my side.

Best regards,
Eugeniu.

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

[Jean-Jacques Hiblot]

Hi all,


On 08/08/2018 11:17, Eugeniu Rosca wrote:
> Hello,
>
> FWIW, patch "[7/7] cmd: Add bind/unbind commands to bind a device to a
> driver from the command line" contributes with a compiler warning, as
> described in https://patchwork.ozlabs.org/patch/933310/#1952127 .
This looks like a false positive to me.
It can be fixed by assigning an initialization value to ret.

Does that require a v4 ?

JJ
>
> Best regards,
> Eugeniu.
> On Wed, Aug 8, 2018 at 8:36 AM Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi Lukasz,
>>
>> On 22.6.2018 14:25, Jean-Jacques Hiblot wrote:
>>> 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 v3:
>>> - update some commit logs
>>> - 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
>>>
>> Lukasz: I see your name in patchwork.
>> Are you going to take this series? Or we are waiting for sandbox testing?
>>
>> Thanks,
>> Michal
>>

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

[Eugeniu Rosca]

On Wed, Aug 8, 2018 at 1:36 PM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>
> Hi all,
>
>
> On 08/08/2018 11:17, Eugeniu Rosca wrote:
> > Hello,
> >
> > FWIW, patch "[7/7] cmd: Add bind/unbind commands to bind a device to a
> > driver from the command line" contributes with a compiler warning, as
> > described in https://patchwork.ozlabs.org/patch/933310/#1952127 .
> This looks like a false positive to me.
> It can be fixed by assigning an initialization value to ret.
>
> Does that require a v4 ?
>
> JJ

I think having a warning-free build is a basic policy everybody is
expected to comply with.
The only right way to fix the gcc warning is to submit a v4.

Best regards,
Eugeniu.

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

[Jean-Jacques Hiblot]

On 08/08/2018 15:08, Eugeniu Rosca wrote:
> On Wed, Aug 8, 2018 at 1:36 PM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>> Hi all,
>>
>>
>> On 08/08/2018 11:17, Eugeniu Rosca wrote:
>>> Hello,
>>>
>>> FWIW, patch "[7/7] cmd: Add bind/unbind commands to bind a device to a
>>> driver from the command line" contributes with a compiler warning, as
>>> described in https://patchwork.ozlabs.org/patch/933310/#1952127 .
>> This looks like a false positive to me.
>> It can be fixed by assigning an initialization value to ret.
>>
>> Does that require a v4 ?
>>
>> JJ
> I think having a warning-free build is a basic policy everybody is
> expected to comply with.
I agree. I meant that it could be fixed at the time of the commit by the 
maintainer.
Thanks,
JJ
> The only right way to fix the gcc warning is to submit a v4.
>
> Best regards,
> Eugeniu.
>

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

[Eugeniu Rosca]

On Wed, Aug 8, 2018 at 6:18 PM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> On 08/08/2018 15:08, Eugeniu Rosca wrote:
[snip]
> > I think having a warning-free build is a basic policy everybody is
> > expected to comply with.
> I agree. I meant that it could be fixed at the time of the commit by the
> maintainer.

I'm not sure how well this plays with https://developercertificate.org/.
At least for my contributions, I would expect that they reach the main
tree unmodified, unless there is some trivial change in commit
metadata (which I expect to be documented). But I'll let others to
comment, who went through such kind of questions before.

Best regards,
Eugeniu.

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

[Tom Rini]

On Wed, Aug 08, 2018 at 06:44:07PM +0200, Eugeniu Rosca wrote:
> On Wed, Aug 8, 2018 at 6:18 PM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> > On 08/08/2018 15:08, Eugeniu Rosca wrote:
> [snip]
> > > I think having a warning-free build is a basic policy everybody is
> > > expected to comply with.
> > I agree. I meant that it could be fixed at the time of the commit by the
> > maintainer.
> 
> I'm not sure how well this plays with https://developercertificate.org/.
> At least for my contributions, I would expect that they reach the main
> tree unmodified, unless there is some trivial change in commit
> metadata (which I expect to be documented). But I'll let others to
> comment, who went through such kind of questions before.

In DCO terms, it is my belief that a maintainer can make additional
changes, so long as they too add a S-o-B line.  I also like it if they
do:
[me: Did stuff]
so it's clear what changed.  But I don't think that's a conflict with
DCOs.

In custodian terms, it's a whole lot quicker if someone can directly
apply a series rather than apply, rebase (fixup a patch) and continue.
So, if you do another round and fix the problem it'll be one less thing
for the custodian to deal with.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180808/a24a6e88/attachment.sig>

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

[Eugeniu Rosca]

On Wed, Aug 8, 2018 at 6:54 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Aug 08, 2018 at 06:44:07PM +0200, Eugeniu Rosca wrote:
> > On Wed, Aug 8, 2018 at 6:18 PM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
> > > On 08/08/2018 15:08, Eugeniu Rosca wrote:
> > [snip]
> > > > I think having a warning-free build is a basic policy everybody is
> > > > expected to comply with.
> > > I agree. I meant that it could be fixed at the time of the commit by the
> > > maintainer.
> >
> > I'm not sure how well this plays with https://developercertificate.org/.
> > At least for my contributions, I would expect that they reach the main
> > tree unmodified, unless there is some trivial change in commit
> > metadata (which I expect to be documented). But I'll let others to
> > comment, who went through such kind of questions before.
>
> In DCO terms, it is my belief that a maintainer can make additional
> changes, so long as they too add a S-o-B line.  I also like it if they
> do:
> [me: Did stuff]
> so it's clear what changed.  But I don't think that's a conflict with
> DCOs.

One interesting aspect is whether the community has a chance to review
the changes occurred in the timeframe between the author's and the
maintainer's S-o-B. This seems to be elided in DCO, probably because
DCO treats all contributors equally. But obviously, the maintainer has
an outstanding status of committing the work into the tree.

It is my assumption that the maintainer should give enough time to
community to reach some agreement that the contents is ready for
merge. With this assumption in mind, it should probably be an
exception for the maintainer to perform any changes without prior
community review. This is why I noted that I expect a v4 to fix the
compiler warning (especially given that this is not a metadata
change). But don't get me wrong. My main interest is fixing the gcc
warning occurring in sandbox build with this patch-set. I am open
minded about the rest.

> In custodian terms, it's a whole lot quicker if someone can directly
> apply a series rather than apply, rebase (fixup a patch) and continue.
> So, if you do another round and fix the problem it'll be one less thing
> for the custodian to deal with.
>
> --
> Tom

Thanks,
Eugeniu.

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

[Jean-Jacques Hiblot]

On 08/08/2018 18:54, Tom Rini wrote:
> On Wed, Aug 08, 2018 at 06:44:07PM +0200, Eugeniu Rosca wrote:
>> On Wed, Aug 8, 2018 at 6:18 PM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>>> On 08/08/2018 15:08, Eugeniu Rosca wrote:
>> [snip]
>>>> I think having a warning-free build is a basic policy everybody is
>>>> expected to comply with.
>>> I agree. I meant that it could be fixed at the time of the commit by the
>>> maintainer.
>> I'm not sure how well this plays with https://developercertificate.org/.
>> At least for my contributions, I would expect that they reach the main
>> tree unmodified, unless there is some trivial change in commit
>> metadata (which I expect to be documented). But I'll let others to
>> comment, who went through such kind of questions before.
> In DCO terms, it is my belief that a maintainer can make additional
> changes, so long as they too add a S-o-B line.  I also like it if they
> do:
> [me: Did stuff]
> so it's clear what changed.  But I don't think that's a conflict with
> DCOs.
>
> In custodian terms, it's a whole lot quicker if someone can directly
> apply a series rather than apply, rebase (fixup a patch) and continue.
> So, if you do another round and fix the problem it'll be one less thing
> for the custodian to deal with.

For the sake of the custodian, I posted the v4 with the warning fix.
Thanks,
JJ

>