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

- fix the bind command - add a bind command test - add bind command documentation
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 | 32 ++++++++++++++++++++++++++++++++ doc/driver-model/index.rst | 1 + drivers/phy/sandbox-phy.c | 18 +++++++++++++++++- test/py/tests/test_bind.py | 18 +++++++++--------- 6 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 doc/driver-model/bind.rst

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@5800a000 STM32MP> bind /soc/ethernet@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
---
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;

On Wed, 29 Apr 2020 at 06:20, Patrice Chotard patrice.chotard@st.com wrote:
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@5800a000 STM32MP> bind /soc/ethernet@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
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(-)
Reviewed-by: Simon Glass sjg@chromium.org

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 ---
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,

On Wed, 29 Apr 2020 at 06:20, Patrice Chotard patrice.chotard@st.com wrote:
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
Changes in v2: None
drivers/phy/sandbox-phy.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

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 ---
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)

On Wed, 29 Apr 2020 at 06:20, Patrice Chotard patrice.chotard@st.com wrote:
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
Changes in v2: None
arch/sandbox/dts/test.dts | 1 + test/py/tests/test_bind.py | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

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 everywwhere 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 ---
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:

On Wed, 29 Apr 2020 at 06:20, Patrice Chotard patrice.chotard@st.com wrote:
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 everywwhere in the tree, so
everywhere
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
Changes in v2: None
test/py/tests/test_bind.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Simon
On 4/29/20 8:04 PM, Simon Glass wrote:
On Wed, 29 Apr 2020 at 06:20, Patrice Chotard patrice.chotard@st.com wrote:
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 everywwhere in the tree, so
everywhere
will be fixed
Thanks
Patrice
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
Changes in v2: None
test/py/tests/test_bind.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

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 v2: None
doc/driver-model/bind.rst | 32 ++++++++++++++++++++++++++++++++ doc/driver-model/index.rst | 1 + 2 files changed, 33 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..df6b5f143b --- /dev/null +++ b/doc/driver-model/bind.rst @@ -0,0 +1,32 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. sectionauthor:: Patrice Chotard patrice.chotard@st.com + +Binding/unbiding a driver +========================= + +This documents aims to describe the bind and unbind commands. + +For debug 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 example: + +bind usb_dev_generic 0 usb_ether +unbind usb_dev_generic 0 usb_ether +or +unbind eth 1 + +bind /ocp/omap_dwc3@48380000/usb@48390000 usb_ether +unbind /ocp/omap_dwc3@48380000/usb@48390000 diff --git a/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

Hi Patrice,
On Wed, 29 Apr 2020 at 06:20, Patrice Chotard patrice.chotard@st.com 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 v2: None
doc/driver-model/bind.rst | 32 ++++++++++++++++++++++++++++++++ doc/driver-model/index.rst | 1 + 2 files changed, 33 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..df6b5f143b --- /dev/null +++ b/doc/driver-model/bind.rst @@ -0,0 +1,32 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. sectionauthor:: Patrice Chotard patrice.chotard@st.com
+Binding/unbiding a driver
unbinding
+=========================
+This documents aims to describe the bind and unbind commands.
document
+For debug purpose, it should be useful to bind or unbind a driver from
For debugging
+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 example:
+bind usb_dev_generic 0 usb_ether +unbind usb_dev_generic 0 usb_ether
can you mention what the two parameters are and how to find them?
+or +unbind eth 1
+bind /ocp/omap_dwc3@48380000/usb@48390000 usb_ether +unbind /ocp/omap_dwc3@48380000/usb@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
Regards, Simon

On 4/29/20 8:04 PM, Simon Glass wrote:
Hi Patrice,
On Wed, 29 Apr 2020 at 06:20, Patrice Chotard patrice.chotard@st.com 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 v2: None
doc/driver-model/bind.rst | 32 ++++++++++++++++++++++++++++++++ doc/driver-model/index.rst | 1 + 2 files changed, 33 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..df6b5f143b --- /dev/null +++ b/doc/driver-model/bind.rst @@ -0,0 +1,32 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. sectionauthor:: Patrice Chotard patrice.chotard@st.com
+Binding/unbiding a driver
unbinding
+=========================
+This documents aims to describe the bind and unbind commands.
document
+For debug purpose, it should be useful to bind or unbind a driver from
For debugging
+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 example:
+bind usb_dev_generic 0 usb_ether +unbind usb_dev_generic 0 usb_ether
can you mention what the two parameters are and how to find them?
I will fix typos and update the documentation with these informations.
Thanks
Patrice
+or +unbind eth 1
+bind /ocp/omap_dwc3@48380000/usb@48390000 usb_ether +unbind /ocp/omap_dwc3@48380000/usb@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
Regards, Simon
participants (3)
-
Patrice CHOTARD
-
Patrice Chotard
-
Simon Glass