Hi Andy

On 4/9/21 11:16 AM, Andy Shevchenko wrote:

On Fri, Apr 9, 2021 at 10:37 AM Patrice Chotard patrice.chotard@foss.st.com wrote:

...

Fix a regression brings by commit 84f8e36f03fa ("cmd: bind: allow to bind driver with driver data")

As example, the following bind command doesn't work:

bind /soc/usb-otg@49000000 usb_ether

As usb_ether driver has no compatible string, it can't be find by lists_bind_fdt(). In bind_by_node_path(), which called lists_bind_fdt(), the driver entry is known, pass it to lists_bind_fdt() to force the driver entry selection.

For this, add a new parameter struct *driver to lists_bind_fdt(). Fix also all lists_bind_fdt() callers.

With or without comments addressed Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com

...

Fixes: 84f8e36f03fa ("cmd: bind: allow to bind driver with driver data")

...

No blank line in the tag block.

ok will remove it

...

Signed-off-by: Patrice Chotard patrice.chotard@foss.st.com Reported-by: Herbert Poetzl herbert@13thfloor.at Cc: Marek Vasut marex@denx.de Cc: Herbert Poetzl herbert@13thfloor.at

---

cmd/bind.c | 2 +- drivers/core/device.c | 2 +- drivers/core/lists.c | 11 ++++++++--- drivers/core/root.c | 2 +- drivers/misc/imx8/scu.c | 2 +- drivers/serial/serial-uclass.c | 2 +- drivers/timer/timer-uclass.c | 2 +- include/dm/lists.h | 3 ++- test/dm/nop.c | 2 +- test/dm/test-fdt.c | 2 +- 10 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/cmd/bind.c b/cmd/bind.c index af2f22cc4c..d8f610943c 100644 --- a/cmd/bind.c +++ b/cmd/bind.c @@ -152,7 +152,7 @@ static int bind_by_node_path(const char *path, const char *drv_name) }

    ofnode = ofnode_path(path);

•   ret = lists_bind_fdt(parent, ofnode, &dev, false);
  

•   ret = lists_bind_fdt(parent, ofnode, &dev, drv, false);
  
    if (!dev || ret) {
            printf("Unable to bind. err:%d\n", ret);
  

diff --git a/drivers/core/device.c b/drivers/core/device.c index 81f6880eac..3abd89aca6 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -1133,6 +1133,6 @@ int dev_enable_by_path(const char *path) if (ret) return ret;

•   return lists_bind_fdt(parent, node, NULL, false);
  

•   return lists_bind_fdt(parent, node, NULL, NULL, false);
  

} #endif diff --git a/drivers/core/lists.c b/drivers/core/lists.c index e214306b90..2eb808ce2d 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -182,7 +182,7 @@ static int driver_check_compatible(const struct udevice_id *of_match, }

int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp,

•              bool pre_reloc_only)
  

•              struct driver *drv, bool pre_reloc_only)
  

{ struct driver *driver = ll_entry_start(struct driver, driver); const int n_ents = ll_entry_count(struct driver, driver); @@ -225,8 +225,13 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, for (entry = driver; entry != driver + n_ents; entry++) { ret = driver_check_compatible(entry->of_match, &id, compat);

•                   if (!ret)
  

•                           break;
  

•                   if (drv) {
  

•                           if (drv == entry)
  

•                                   break;
  

...

•                   } else {
  

•                           if (!ret)
  

•                                   break;
  

•                   }
  

This can be simplified to } else if (!ret) break;

I know but checkpatch.pl requested it ;-)

Thanks Patrice

...

            }
            if (entry == driver + n_ents)
                    continue;

diff --git a/drivers/core/root.c b/drivers/core/root.c index 9bc682cffe..3c6fa3838d 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -236,7 +236,7 @@ static int dm_scan_fdt_node(struct udevice *parent, ofnode parent_node, pr_debug(" - ignoring disabled device\n"); continue; }

•           err = lists_bind_fdt(parent, node, NULL, pre_reloc_only);
  

•           err = lists_bind_fdt(parent, node, NULL, NULL, pre_reloc_only);
            if (err && !ret) {
                    ret = err;
                    debug("%s: ret=%d\n", node_name, ret);
  

diff --git a/drivers/misc/imx8/scu.c b/drivers/misc/imx8/scu.c index 035a600f71..4ab5cb4bf1 100644 --- a/drivers/misc/imx8/scu.c +++ b/drivers/misc/imx8/scu.c @@ -219,7 +219,7 @@ static int imx8_scu_bind(struct udevice *dev)

    debug("%s(dev=%p)\n", __func__, dev);
    ofnode_for_each_subnode(node, dev_ofnode(dev)) {

•           ret = lists_bind_fdt(dev, node, &child, true);
  

•           ret = lists_bind_fdt(dev, node, &child, NULL, true);
            if (ret)
                    return ret;
            debug("bind child dev %s\n", child->name);
  

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 8a87eed683..6d1c671efc 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -67,7 +67,7 @@ static int serial_check_stdout(const void *blob, struct udevice **devp) * anyway. */ if (node > 0 && !lists_bind_fdt(gd->dm_root, offset_to_ofnode(node),

•                                   devp, false)) {
  

•                                   devp, NULL, false)) {
            if (!device_probe(*devp))
                    return 0;
    }
  

diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index 6f00a5d0db..b1ac604b5b 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -148,7 +148,7 @@ int notrace dm_timer_init(void) * If the timer is not marked to be bound before * relocation, bind it anyway. */

•           if (!lists_bind_fdt(dm_root(), node, &dev, false)) {
  

•           if (!lists_bind_fdt(dm_root(), node, &dev, NULL, false)) {
                    ret = device_probe(dev);
                    if (ret)
                            return ret;
  

diff --git a/include/dm/lists.h b/include/dm/lists.h index 1a86552546..5896ae3658 100644 --- a/include/dm/lists.h +++ b/include/dm/lists.h @@ -53,13 +53,14 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only);

• @parent: parent device (root)
• @node: device tree node to bind
• @devp: if non-NULL, returns a pointer to the bound device

• • @drv: if non-NULL, force this driver to be bound
  • @pre_reloc_only: If true, bind only nodes with special devicetree properties,
  • or drivers with the DM_FLAG_PRE_RELOC flag. If false bind all drivers.
  • @return 0 if device was bound, -EINVAL if the device tree is invalid,
  • other -ve value on error
  */

int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp,

•              bool pre_reloc_only);
  

•              struct driver *drv, bool pre_reloc_only);
  
  

/**

• device_bind_driver() - bind a device to a driver

diff --git a/test/dm/nop.c b/test/dm/nop.c index 2cd92c5240..75b9e7b6cc 100644 --- a/test/dm/nop.c +++ b/test/dm/nop.c @@ -25,7 +25,7 @@ static int noptest_bind(struct udevice *parent) const char *bind_flag = ofnode_read_string(ofnode, "bind");

            if (bind_flag && (strcmp(bind_flag, "True") == 0))

•                   lists_bind_fdt(parent, ofnode, &dev, false);
  

•                   lists_bind_fdt(parent, ofnode, &dev, NULL, false);
            ofnode = dev_read_next_subnode(ofnode);
    }
  
  

diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 6e83aeecd9..c6968b0d5f 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -592,7 +592,7 @@ static int zero_size_cells_bus_child_bind(struct udevice *dev) int err;

    ofnode_for_each_subnode(child, dev_ofnode(dev)) {

•           err = lists_bind_fdt(dev, child, NULL, false);
  

•           err = lists_bind_fdt(dev, child, NULL, NULL, false);
            if (err) {
                    dev_err(dev, "%s: lists_bind_fdt, err=%d\n",
                            __func__, err);
  

-- 2.17.1