[PATCH v3 0/5] cmd: bind allow to bind driver with driver_data

[PATCH v3 0/5] cmd: bind allow to bind driver with driver_data

[Patrice Chotard]

   - fix the bind command
   - add a bind command test
   - add bind command documentation

Changes in v3:
 - fix typo
 - add bind/unbind parameters description and how to find them

Changes in v2:
   - add a bind command test
   - add bind command documentation in doc/driver/model/bind.rst
   - simplify patch 1 by using lists_bind_fdt()

Patrice Chotard (5):
  cmd: bind: allow to bind driver with driver data
  sandbox: phy: add driver_data for bind test cmd
  sandbox: dts: Add compatible string for bind-test node
  test/py: Update test_bind
  doc: add bind/unbind command documentation

 arch/sandbox/dts/test.dts  |  1 +
 cmd/bind.c                 |  5 ++--
 doc/driver-model/bind.rst  | 49 ++++++++++++++++++++++++++++++++++++++
 doc/driver-model/index.rst |  1 +
 drivers/phy/sandbox-phy.c  | 18 +++++++++++++-
 test/py/tests/test_bind.py | 18 +++++++-------
 6 files changed, 80 insertions(+), 12 deletions(-)
 create mode 100644 doc/driver-model/bind.rst

-- 
2.17.1

[PATCH v3 1/5] cmd: bind: allow to bind driver with driver data

[Patrice Chotard]

Initial implementation invokes device_bind_with_driver_data()
with driver_data parameter equal to 0.
For driver with driver data, the bind command can't bind
correctly this driver or even worse causes data abort as shown below:

As example, for debug purpose on STM32MP1 platform, ethernet (dwc_eth_qos.c)
driver needed to be unbinded/binded. This driver is using driver data:

static const struct udevice_id eqos_ids[] = {
    {
        .compatible = "nvidia,tegra186-eqos",
        .data = (ulong)&eqos_tegra186_config
    },
    {
        .compatible = "snps,dwmac-4.20a",
        .data = (ulong)&eqos_stm32_config
    },

    { }
};

After unbinding/binding this driver and probing it (with the dhcp command),
we got a prefetch abort as below:

STM32MP> unbind eth ethernet at 5800a000
STM32MP> bind /soc/ethernet at 5800a000 eth_eqos
STM32MP> dhcp
prefetch abort
pc : [<4310801c>]          lr : [<ffc8f4ad>]
reloc pc : [<035ba01c>]    lr : [<c01414ad>]
sp : fdaf19b0  ip : ffcea83c     fp : 00000001
r10: ffcfd4a0  r9 : fdaffed0     r8 : 00000000
r7 : ffcff304  r6 : fdc63220     r5 : 00000000  r4 : fdc5b108
r3 : 43108020  r2 : 00003d39     r1 : ffcea544  r0 : fdc63220
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Code: data abort
pc : [<ffc4f9c0>]          lr : [<ffc4f9ad>]
reloc pc : [<c01019c0>]    lr : [<c01019ad>]
sp : fdaf18b8  ip : 00000000     fp : 00000001
r10: ffcd69b2  r9 : fdaffed0     r8 : ffcd69aa
r7 : 00000000  r6 : 00000008     r5 : 4310801c  r4 : fffffffc
r3 : 00000001  r2 : 00000028     r1 : 00000000  r0 : 00000006
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32 (T)
Code: 2f00 d1e9 2c00 dce9 (f855) 2024
Resetting CPU ...

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

Changes in v3: None
Changes in v2:
   - add a bind command test
   - add bind command documentation in doc/driver/model/bind.rst
   - simplify patch 1 by using lists_bind_fdt()

 cmd/bind.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/bind.c b/cmd/bind.c
index 44a5f17f0d..0aefc531d8 100644
--- a/cmd/bind.c
+++ b/cmd/bind.c
@@ -7,6 +7,7 @@
 #include <dm.h>
 #include <dm/device-internal.h>
 #include <dm/lists.h>
+#include <dm/root.h>
 #include <dm/uclass-internal.h>
 
 static int bind_by_class_index(const char *uclass, int index,
@@ -150,8 +151,8 @@ static int bind_by_node_path(const char *path, const char *drv_name)
 	}
 
 	ofnode = ofnode_path(path);
-	ret = device_bind_with_driver_data(parent, drv, ofnode_get_name(ofnode),
-					   0, ofnode, &dev);
+	ret = lists_bind_fdt(parent, ofnode, &dev, false);
+
 	if (!dev || ret) {
 		printf("Unable to bind. err:%d\n", ret);
 		return ret;
-- 
2.17.1

[PATCH v3 2/5] sandbox: phy: add driver_data for bind test cmd

[Patrice Chotard]

Add driver data to existing compatible string "sandbox,phy".
Add an additional compatible string without driver_data

This will verify that bind command parses, finds and passes the
correct driver data to device_bind_with_driver_data() by using
driver_data in the second sandbox_phy_ids table entry.
In sandbox_phy_bind() a check is added to validate driver_data
content.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 drivers/phy/sandbox-phy.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/sandbox-phy.c b/drivers/phy/sandbox-phy.c
index 84ff5c6275..5f36da7692 100644
--- a/drivers/phy/sandbox-phy.c
+++ b/drivers/phy/sandbox-phy.c
@@ -8,6 +8,8 @@
 #include <dm.h>
 #include <generic-phy.h>
 
+#define DRIVER_DATA 0x12345678
+
 struct sandbox_phy_priv {
 	bool initialized;
 	bool on;
@@ -71,6 +73,14 @@ static int sandbox_phy_exit(struct phy *phy)
 	return 0;
 }
 
+static int sandbox_phy_bind(struct udevice *dev)
+{
+	if (dev_get_driver_data(dev) != DRIVER_DATA)
+		return -ENODATA;
+
+	return 0;
+}
+
 static int sandbox_phy_probe(struct udevice *dev)
 {
 	struct sandbox_phy_priv *priv = dev_get_priv(dev);
@@ -90,13 +100,19 @@ static struct phy_ops sandbox_phy_ops = {
 };
 
 static const struct udevice_id sandbox_phy_ids[] = {
-	{ .compatible = "sandbox,phy" },
+	{ .compatible = "sandbox,phy_no_driver_data",
+	},
+
+	{ .compatible = "sandbox,phy",
+	  .data = DRIVER_DATA
+	},
 	{ }
 };
 
 U_BOOT_DRIVER(phy_sandbox) = {
 	.name		= "phy_sandbox",
 	.id		= UCLASS_PHY,
+	.bind		= sandbox_phy_bind,
 	.of_match	= sandbox_phy_ids,
 	.ops		= &sandbox_phy_ops,
 	.probe		= sandbox_phy_probe,
-- 
2.17.1

[PATCH v3 3/5] sandbox: dts: Add compatible string for bind-test node

[Patrice Chotard]

Usage of lists_bind_fdt() in bind command imposes to add
a compatible string for bind-test node.
The other impact, is that bind-test node is binded at sandbox
start, so no need to bind it in test_bind_unbind_with_node() test

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 arch/sandbox/dts/test.dts  | 1 +
 test/py/tests/test_bind.py | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index df9f1835c9..7c6b14887f 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -131,6 +131,7 @@
 	};
 
 	bind-test {
+		compatible = "simple-bus";
 		bind-test-child1 {
 			compatible = "sandbox,phy";
 			#phy-cells = <1>;
diff --git a/test/py/tests/test_bind.py b/test/py/tests/test_bind.py
index 20c6050342..0b7cd9a808 100644
--- a/test/py/tests/test_bind.py
+++ b/test/py/tests/test_bind.py
@@ -25,9 +25,6 @@ def in_tree(response, name, uclass, drv, depth, last_child):
 @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_bus', 0, True)
 	assert in_tree(tree, 'bind-test-child1', 'phy', 'phy_sandbox', 1, False)
-- 
2.17.1

[PATCH v3 4/5] test/py: Update test_bind

[Patrice Chotard]

As bind-test is now binded at sandbox startup and no more by
test_bind.py, bind-test nodes are not located at the end of
"dm tree" output, but can be located everywhere in the tree, so
bind-test output could either be:

 simple_bus    0  [   ]   generic_simple_bus    |-- bind-test
 phy           0  [   ]   phy_sandbox           |   |-- bind-test-child1
 simple_bus    1  [   ]   generic_simple_bus    |   `-- bind-test-child2

or:

 simple_bus    5  [   ]   generic_simple_bus    `-- bind-test
 phy           2  [   ]   phy_sandbox               |-- bind-test-child1
 simple_bus    6  [   ]   generic_simple_bus        `-- bind-test-child2

in_tree() function need to be updated to take care of that change.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

Changes in v3:
 - fix typo

Changes in v2: None

 test/py/tests/test_bind.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/test/py/tests/test_bind.py b/test/py/tests/test_bind.py
index 0b7cd9a808..4753c7ea7b 100644
--- a/test/py/tests/test_bind.py
+++ b/test/py/tests/test_bind.py
@@ -7,13 +7,16 @@ 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 + r'\|'
-	else:
-		leaf = leaf + '`'
+	leaf = ''
+	if depth != 0:
+		leaf = '   ' + '    ' * (depth - 1) ;
+		if not last_child:
+			leaf = leaf + r'\|'
+		else:
+                        leaf = leaf + '`'
+
 	leaf = leaf + '-- ' + name
-	line = (r' *{:10.10}    [0-9]*  \[ [ +] \]   {:20.20}  {}$'
+	line = (r' *{:10.10}    [0-9]*  \[ [ +] \]   {:20.20}  [` |]{}$'
 	        .format(uclass, drv, leaf))
 	prog = re.compile(line)
 	for l in lines:
-- 
2.17.1

[PATCH v3 5/5] doc: add bind/unbind command documentation

[Patrice Chotard]

Add documentation in doc/drivel-model for the bind/unbind command.
Part of this documentation is extracted from original patch commit
message:
commit 49c752c93a78 ("cmd: Add bind/unbind commands to bind a device to a driver from the command line")

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

---

Changes in v3:
 - fix typo
 - add bind/unbind parameters description and how to find them

Changes in v2: None

 doc/driver-model/bind.rst  | 49 ++++++++++++++++++++++++++++++++++++++
 doc/driver-model/index.rst |  1 +
 2 files changed, 50 insertions(+)
 create mode 100644 doc/driver-model/bind.rst

diff --git a/doc/driver-model/bind.rst b/doc/driver-model/bind.rst
new file mode 100644
index 0000000000..dfe8fd57dd
--- /dev/null
+++ b/doc/driver-model/bind.rst
@@ -0,0 +1,49 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. sectionauthor:: Patrice Chotard <patrice.chotard@st.com>
+
+Binding/unbinding a driver
+=========================
+
+This document aims to describe the bind and unbind commands.
+
+For debugging purpose, it should be useful to bind or unbind a driver from
+the U-boot command line.
+
+The unbind command calls the remove device driver callback and unbind the
+device from its driver.
+
+The bind command binds a device to its driver.
+
+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:
+
+bind <node path> <driver>
+bind <class> <index> <driver>
+
+unbind <node path>
+unbind <class> <index>
+unbind <class> <index> <driver>
+
+Where:
+ - <node path> is the node's device tree path
+ - <class> is one of the class available in the list given by the "dm uclass"
+   command or first column of "dm tree" command.
+ - <index> is the index of the parent's node (second column of "dm tree" output).
+ - <driver> is the driver name to bind given by the "dm drivers" command or the by
+   the fourth column of "dm tree" output.
+
+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
diff --git a/doc/driver-model/index.rst b/doc/driver-model/index.rst
index b9df221627..37ef3721df 100644
--- a/doc/driver-model/index.rst
+++ b/doc/driver-model/index.rst
@@ -6,6 +6,7 @@ Driver Model
 .. toctree::
    :maxdepth: 2
 
+   bind
    debugging
    design
    ethernet
-- 
2.17.1

[PATCH v3 5/5] doc: add bind/unbind command documentation

[Heinrich Schuchardt]

On 30.04.20 10:52, Patrice Chotard wrote:
> Add documentation in doc/drivel-model for the bind/unbind command.
> Part of this documentation is extracted from original patch commit
> message:
> commit 49c752c93a78 ("cmd: Add bind/unbind commands to bind a device to a driver from the command line")
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>
> Changes in v3:
>  - fix typo
>  - add bind/unbind parameters description and how to find them
>
> Changes in v2: None
>
>  doc/driver-model/bind.rst  | 49 ++++++++++++++++++++++++++++++++++++++
>  doc/driver-model/index.rst |  1 +
>  2 files changed, 50 insertions(+)
>  create mode 100644 doc/driver-model/bind.rst
>
> diff --git a/doc/driver-model/bind.rst b/doc/driver-model/bind.rst
> new file mode 100644
> index 0000000000..dfe8fd57dd
> --- /dev/null
> +++ b/doc/driver-model/bind.rst
> @@ -0,0 +1,49 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +.. sectionauthor:: Patrice Chotard <patrice.chotard@st.com>
> +
> +Binding/unbinding a driver
> +=========================

The title underline is too short.

Please, use 'make htmldocs' for testing.

Best regards

Heinrich

> +
> +This document aims to describe the bind and unbind commands.
> +
> +For debugging purpose, it should be useful to bind or unbind a driver from
> +the U-boot command line.
> +
> +The unbind command calls the remove device driver callback and unbind the
> +device from its driver.
> +
> +The bind command binds a device to its driver.
> +
> +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:
> +
> +bind <node path> <driver>
> +bind <class> <index> <driver>
> +
> +unbind <node path>
> +unbind <class> <index>
> +unbind <class> <index> <driver>
> +
> +Where:
> + - <node path> is the node's device tree path
> + - <class> is one of the class available in the list given by the "dm uclass"
> +   command or first column of "dm tree" command.
> + - <index> is the index of the parent's node (second column of "dm tree" output).
> + - <driver> is the driver name to bind given by the "dm drivers" command or the by
> +   the fourth column of "dm tree" output.
> +
> +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
> diff --git a/doc/driver-model/index.rst b/doc/driver-model/index.rst
> index b9df221627..37ef3721df 100644
> --- a/doc/driver-model/index.rst
> +++ b/doc/driver-model/index.rst
> @@ -6,6 +6,7 @@ Driver Model
>  .. toctree::
>     :maxdepth: 2
>
> +   bind
>     debugging
>     design
>     ethernet
>

[PATCH v3 5/5] doc: add bind/unbind command documentation

[Patrice CHOTARD]

Hi Heinrich

On 4/30/20 11:31 AM, Heinrich Schuchardt wrote:
> On 30.04.20 10:52, Patrice Chotard wrote:
>> Add documentation in doc/drivel-model for the bind/unbind command.
>> Part of this documentation is extracted from original patch commit
>> message:
>> commit 49c752c93a78 ("cmd: Add bind/unbind commands to bind a device to a driver from the command line")
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>
>> Changes in v3:
>>  - fix typo
>>  - add bind/unbind parameters description and how to find them
>>
>> Changes in v2: None
>>
>>  doc/driver-model/bind.rst  | 49 ++++++++++++++++++++++++++++++++++++++
>>  doc/driver-model/index.rst |  1 +
>>  2 files changed, 50 insertions(+)
>>  create mode 100644 doc/driver-model/bind.rst
>>
>> diff --git a/doc/driver-model/bind.rst b/doc/driver-model/bind.rst
>> new file mode 100644
>> index 0000000000..dfe8fd57dd
>> --- /dev/null
>> +++ b/doc/driver-model/bind.rst
>> @@ -0,0 +1,49 @@
>> +.. SPDX-License-Identifier: GPL-2.0+
>> +.. sectionauthor:: Patrice Chotard <patrice.chotard@st.com>
>> +
>> +Binding/unbinding a driver
>> +=========================
> The title underline is too short.
>
> Please, use 'make htmldocs' for testing.

Thanks for the tips

Patrice


>
> Best regards
>
> Heinrich
>
>> +
>> +This document aims to describe the bind and unbind commands.
>> +
>> +For debugging purpose, it should be useful to bind or unbind a driver from
>> +the U-boot command line.
>> +
>> +The unbind command calls the remove device driver callback and unbind the
>> +device from its driver.
>> +
>> +The bind command binds a device to its driver.
>> +
>> +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:
>> +
>> +bind <node path> <driver>
>> +bind <class> <index> <driver>
>> +
>> +unbind <node path>
>> +unbind <class> <index>
>> +unbind <class> <index> <driver>
>> +
>> +Where:
>> + - <node path> is the node's device tree path
>> + - <class> is one of the class available in the list given by the "dm uclass"
>> +   command or first column of "dm tree" command.
>> + - <index> is the index of the parent's node (second column of "dm tree" output).
>> + - <driver> is the driver name to bind given by the "dm drivers" command or the by
>> +   the fourth column of "dm tree" output.
>> +
>> +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
>> diff --git a/doc/driver-model/index.rst b/doc/driver-model/index.rst
>> index b9df221627..37ef3721df 100644
>> --- a/doc/driver-model/index.rst
>> +++ b/doc/driver-model/index.rst
>> @@ -6,6 +6,7 @@ Driver Model
>>  .. toctree::
>>     :maxdepth: 2
>>
>> +   bind
>>     debugging
>>     design
>>     ethernet
>>