Don't bother review this patch. It needs to be refactored better once I
know it works.
Geert and Herve,
This patch serious should hopefully fix both of your use cases. Can you
please check to make sure the device links created to/from the overlay
devices are to/from the right ones?
I've only compile tested it. If I made some obvious mistake, feel free
to fix it and give it a shot.
Cc: Rob Herring <[email protected]>
Thanks,
Saravana
Saravana Kannan (2):
Revert "treewide: Fix probing of devices in DT overlays"
of: dynamic: Fix probing of overlay devices
drivers/base/core.c | 10 ++++++++++
drivers/bus/imx-weim.c | 6 ------
drivers/i2c/i2c-core-of.c | 5 -----
drivers/of/dynamic.c | 9 ++++++++-
drivers/of/platform.c | 5 -----
drivers/spi/spi.c | 5 -----
include/linux/fwnode.h | 2 ++
7 files changed, 20 insertions(+), 22 deletions(-)
--
2.44.0.478.gd926399ef9-goog
This reverts commit 1a50d9403fb90cbe4dea0ec9fd0351d2ecbd8924.
---
drivers/bus/imx-weim.c | 6 ------
drivers/i2c/i2c-core-of.c | 5 -----
drivers/of/dynamic.c | 1 -
drivers/of/platform.c | 5 -----
drivers/spi/spi.c | 5 -----
5 files changed, 22 deletions(-)
diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 837bf9d51c6e..caaf887e0ccc 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -331,12 +331,6 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
"Failed to setup timing for '%pOF'\n", rd->dn);
if (!of_node_check_flag(rd->dn, OF_POPULATED)) {
- /*
- * Clear the flag before adding the device so that
- * fw_devlink doesn't skip adding consumers to this
- * device.
- */
- rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
dev_err(&pdev->dev,
"Failed to create child device '%pOF'\n",
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index a6c407d36800..a250921bbce0 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -178,11 +178,6 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
return NOTIFY_OK;
}
- /*
- * Clear the flag before adding the device so that fw_devlink
- * doesn't skip adding consumers to this device.
- */
- rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
client = of_i2c_register_device(adap, rd->dn);
if (IS_ERR(client)) {
dev_err(&adap->dev, "failed to create client for '%pOF'\n",
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 4d57a4e34105..19a1a38554f2 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -224,7 +224,6 @@ static void __of_attach_node(struct device_node *np)
np->sibling = np->parent->child;
np->parent->child = np;
of_node_clear_flag(np, OF_DETACHED);
- np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
raw_spin_unlock_irqrestore(&devtree_lock, flags);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 389d4ea6bfc1..efd861fa254f 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -743,11 +743,6 @@ static int of_platform_notify(struct notifier_block *nb,
if (of_node_check_flag(rd->dn, OF_POPULATED))
return NOTIFY_OK;
- /*
- * Clear the flag before adding the device so that fw_devlink
- * doesn't skip adding consumers to this device.
- */
- rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
/* pdev_parent may be NULL when no bus platform device */
pdev_parent = of_find_device_by_node(rd->dn->parent);
pdev = of_platform_device_create(rd->dn, NULL,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ff75838c1b5d..17cd417f7681 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4761,11 +4761,6 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
return NOTIFY_OK;
}
- /*
- * Clear the flag before adding the device so that fw_devlink
- * doesn't skip adding consumers to this device.
- */
- rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
spi = of_register_spi_device(ctlr, rd->dn);
put_device(&ctlr->dev);
--
2.44.0.478.gd926399ef9-goog
Get fw_devlink to work well with overlay devices.
Signed-off-by: Saravana Kannan <[email protected]>
---
drivers/base/core.c | 10 ++++++++++
drivers/of/dynamic.c | 8 ++++++++
include/linux/fwnode.h | 2 ++
3 files changed, 20 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5f4e03336e68..d856f9c5d601 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -237,6 +237,16 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
__fw_devlink_pickup_dangling_consumers(child, new_sup);
}
+
+void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
+ struct fwnode_handle *parent)
+{
+ mutex_lock(&fwnode_link_lock);
+ __fw_devlink_pickup_dangling_consumers(child, parent);
+ __fw_devlink_link_to_consumers(parent->dev);
+ mutex_unlock(&fwnode_link_lock);
+}
+
static DEFINE_MUTEX(device_links_lock);
DEFINE_STATIC_SRCU(device_links_srcu);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 19a1a38554f2..0a936f46820e 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -237,6 +237,7 @@ static void __of_attach_node(struct device_node *np)
int of_attach_node(struct device_node *np)
{
struct of_reconfig_data rd;
+ struct fwnode_handle *fwnode, *parent;
memset(&rd, 0, sizeof(rd));
rd.dn = np;
@@ -246,6 +247,13 @@ int of_attach_node(struct device_node *np)
mutex_unlock(&of_mutex);
of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, &rd);
+ fwnode = of_fwnode_handle(np);
+ fwnode_for_each_parent_node(fwnode, parent)
+ if (parent->dev) {
+ fw_devlink_pickup_dangling_consumers(fwnode, parent);
+ fwnode_handle_put(parent);
+ break;
+ }
return 0;
}
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 0d79070c5a70..4b3f697a90e8 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -220,6 +220,8 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
u8 flags);
void fwnode_links_purge(struct fwnode_handle *fwnode);
void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
+void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
+ struct fwnode_handle *parent);
bool fw_devlink_is_strict(void);
#endif
--
2.44.0.478.gd926399ef9-goog
On Mon, Apr 8, 2024 at 4:13 PM Saravana Kannan <[email protected]> wrote:
>
> Get fw_devlink to work well with overlay devices.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> drivers/base/core.c | 10 ++++++++++
> drivers/of/dynamic.c | 8 ++++++++
> include/linux/fwnode.h | 2 ++
> 3 files changed, 20 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 5f4e03336e68..d856f9c5d601 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -237,6 +237,16 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
> __fw_devlink_pickup_dangling_consumers(child, new_sup);
> }
>
> +
> +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> + struct fwnode_handle *parent)
> +{
> + mutex_lock(&fwnode_link_lock);
> + __fw_devlink_pickup_dangling_consumers(child, parent);
> + __fw_devlink_link_to_consumers(parent->dev);
> + mutex_unlock(&fwnode_link_lock);
> +}
> +
> static DEFINE_MUTEX(device_links_lock);
> DEFINE_STATIC_SRCU(device_links_srcu);
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 19a1a38554f2..0a936f46820e 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -237,6 +237,7 @@ static void __of_attach_node(struct device_node *np)
> int of_attach_node(struct device_node *np)
> {
> struct of_reconfig_data rd;
> + struct fwnode_handle *fwnode, *parent;
>
> memset(&rd, 0, sizeof(rd));
> rd.dn = np;
> @@ -246,6 +247,13 @@ int of_attach_node(struct device_node *np)
> mutex_unlock(&of_mutex);
>
> of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, &rd);
> + fwnode = of_fwnode_handle(np);
> + fwnode_for_each_parent_node(fwnode, parent)
> + if (parent->dev) {
> + fw_devlink_pickup_dangling_consumers(fwnode, parent);
> + fwnode_handle_put(parent);
> + break;
> + }
I'm clearly calling this in the wrong location. Please move this logic
to __of_changeset_entry_notify() and for the case
OF_RECONFIG_ATTACH_NODE. Haven't fully thought through the DETACH
case, but it should work correctly for that case too. If not, I'll
take care of that next.
-Saravana
>
> return 0;
> }
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 0d79070c5a70..4b3f697a90e8 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -220,6 +220,8 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
> u8 flags);
> void fwnode_links_purge(struct fwnode_handle *fwnode);
> void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> + struct fwnode_handle *parent);
> bool fw_devlink_is_strict(void);
>
> #endif
> --
> 2.44.0.478.gd926399ef9-goog
>
On Mon, Apr 8, 2024 at 6:40 PM Saravana Kannan <[email protected]> wrote:
>
> On Mon, Apr 8, 2024 at 4:13 PM Saravana Kannan <[email protected]> wrote:
> >
> > Get fw_devlink to work well with overlay devices.
> >
> > Signed-off-by: Saravana Kannan <[email protected]>
> > ---
> > drivers/base/core.c | 10 ++++++++++
> > drivers/of/dynamic.c | 8 ++++++++
> > include/linux/fwnode.h | 2 ++
> > 3 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 5f4e03336e68..d856f9c5d601 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -237,6 +237,16 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
> > __fw_devlink_pickup_dangling_consumers(child, new_sup);
> > }
> >
> > +
> > +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> > + struct fwnode_handle *parent)
> > +{
> > + mutex_lock(&fwnode_link_lock);
> > + __fw_devlink_pickup_dangling_consumers(child, parent);
> > + __fw_devlink_link_to_consumers(parent->dev);
> > + mutex_unlock(&fwnode_link_lock);
> > +}
> > +
> > static DEFINE_MUTEX(device_links_lock);
> > DEFINE_STATIC_SRCU(device_links_srcu);
> >
> > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> > index 19a1a38554f2..0a936f46820e 100644
> > --- a/drivers/of/dynamic.c
> > +++ b/drivers/of/dynamic.c
> > @@ -237,6 +237,7 @@ static void __of_attach_node(struct device_node *np)
> > int of_attach_node(struct device_node *np)
> > {
> > struct of_reconfig_data rd;
> > + struct fwnode_handle *fwnode, *parent;
> >
> > memset(&rd, 0, sizeof(rd));
> > rd.dn = np;
> > @@ -246,6 +247,13 @@ int of_attach_node(struct device_node *np)
> > mutex_unlock(&of_mutex);
> >
> > of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, &rd);
> > + fwnode = of_fwnode_handle(np);
> > + fwnode_for_each_parent_node(fwnode, parent)
> > + if (parent->dev) {
> > + fw_devlink_pickup_dangling_consumers(fwnode, parent);
> > + fwnode_handle_put(parent);
> > + break;
> > + }
>
> I'm clearly calling this in the wrong location. Please move this logic
> to __of_changeset_entry_notify() and for the case
> OF_RECONFIG_ATTACH_NODE. Haven't fully thought through the DETACH
> case, but it should work correctly for that case too. If not, I'll
> take care of that next.
>
I'll send out a RFC v2 with the code fixed up in a few minutes.
> -Saravana
>
> >
> > return 0;
> > }
> > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> > index 0d79070c5a70..4b3f697a90e8 100644
> > --- a/include/linux/fwnode.h
> > +++ b/include/linux/fwnode.h
> > @@ -220,6 +220,8 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
> > u8 flags);
> > void fwnode_links_purge(struct fwnode_handle *fwnode);
> > void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> > +void fw_devlink_pickup_dangling_consumers(struct fwnode_handle *child,
> > + struct fwnode_handle *parent);
> > bool fw_devlink_is_strict(void);
> >
> > #endif
> > --
> > 2.44.0.478.gd926399ef9-goog
> >