[PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set

Subsystems: open firmware and flattened device tree, the rest

STALE1718d LANDED

Landed in mainline as 3cd6bab2f81d on 2021-11-18.

4 messages, 3 authors, 2021-11-18 · open the first message on its own page

[PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set

From: Ulf Hansson <hidden>
Date: 2021-09-02 09:02:37

In the struct supplier_bindings the member 'node_not_dev' is described as
"The consumer node containing the property is never a device.", but that is
inconsistent with the behaviour of the code in of_link_property(), as it
calls of_get_compat_node() that starts parsing for a compatible property
from the node it gets passed to it. The proper behaviour is to start at the
node's parent, so let's do that.

While at it, let's take the opportunity to update the description of the
'node_not_dev' flag, as to clarify its purpose.

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/of/property.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 6c028632f425..a94d007be416 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c

@@ -1075,6 +1075,17 @@ static struct device_node *of_get_compat_node(struct device_node *np)
 	return np;
 }
 
+static struct device_node *of_get_compat_node_parent(struct device_node *np)
+{
+	struct device_node *parent, *node;
+
+	parent = of_get_parent(np);
+	node = of_get_compat_node(parent);
+	of_node_put(parent);
+
+	return node;
+}
+
 /**
  * of_link_to_phandle - Add fwnode link to supplier from supplier phandle
  * @con_np: consumer device tree node

@@ -1249,7 +1260,9 @@ static struct device_node *parse_##fname(struct device_node *np,	     \
  * @parse_prop.index: For properties holding a list of phandles, this is the
  *		      index into the list
  * @optional: Describes whether a supplier is mandatory or not
- * @node_not_dev: The consumer node containing the property is never a device.
+ * @node_not_dev: The consumer node containing the property is never converted
+ *		  to a struct device. Instead, parse ancestor nodes for the
+ *		  compatible property to find a node corresponding to a device.
  *
  * Returns:
  * parse_prop() return values are

@@ -1416,7 +1429,7 @@ static int of_link_property(struct device_node *con_np, const char *prop_name)
 			struct device_node *con_dev_np;
 
 			con_dev_np = s->node_not_dev
-					? of_get_compat_node(con_np)
+					? of_get_compat_node_parent(con_np)
 					: of_node_get(con_np);
 			matched = true;
 			i++;

-- 
2.25.1

Re: [PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set

From: Saravana Kannan <hidden>
Date: 2021-09-02 17:10:14

On Thu, Sep 2, 2021 at 2:02 AM Ulf Hansson [off-list ref] wrote:

In the struct supplier_bindings the member 'node_not_dev' is described as
"The consumer node containing the property is never a device.", but that is
inconsistent with the behaviour of the code in of_link_property(), as it
calls of_get_compat_node() that starts parsing for a compatible property
from the node it gets passed to it. The proper behaviour is to start at the
node's parent, so let's do that.

While at it, let's take the opportunity to update the description of the
'node_not_dev' flag, as to clarify its purpose.

Signed-off-by: Ulf Hansson <redacted>

Reviewed-by: Saravana Kannan <redacted>

-Saravana

quoted hunk

---
 drivers/of/property.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 6c028632f425..a94d007be416 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c

@@ -1075,6 +1075,17 @@ static struct device_node *of_get_compat_node(struct device_node *np)
        return np;
 }

+static struct device_node *of_get_compat_node_parent(struct device_node *np)
+{
+       struct device_node *parent, *node;
+
+       parent = of_get_parent(np);
+       node = of_get_compat_node(parent);
+       of_node_put(parent);
+
+       return node;
+}
+
 /**
  * of_link_to_phandle - Add fwnode link to supplier from supplier phandle
  * @con_np: consumer device tree node

@@ -1249,7 +1260,9 @@ static struct device_node *parse_##fname(struct device_node *np,       \
  * @parse_prop.index: For properties holding a list of phandles, this is the
  *                   index into the list
  * @optional: Describes whether a supplier is mandatory or not
- * @node_not_dev: The consumer node containing the property is never a device.
+ * @node_not_dev: The consumer node containing the property is never converted
+ *               to a struct device. Instead, parse ancestor nodes for the
+ *               compatible property to find a node corresponding to a device.
  *
  * Returns:
  * parse_prop() return values are

@@ -1416,7 +1429,7 @@ static int of_link_property(struct device_node *con_np, const char *prop_name)
                        struct device_node *con_dev_np;

                        con_dev_np = s->node_not_dev
-                                       ? of_get_compat_node(con_np)
+                                       ? of_get_compat_node_parent(con_np)
                                        : of_node_get(con_np);
                        matched = true;
                        i++;
--

2.25.1

Re: [PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set

From: Ulf Hansson <hidden>
Date: 2021-11-18 14:02:47

On Thu, 2 Sept 2021 at 11:02, Ulf Hansson [off-list ref] wrote:

In the struct supplier_bindings the member 'node_not_dev' is described as
"The consumer node containing the property is never a device.", but that is
inconsistent with the behaviour of the code in of_link_property(), as it
calls of_get_compat_node() that starts parsing for a compatible property
from the node it gets passed to it. The proper behaviour is to start at the
node's parent, so let's do that.

While at it, let's take the opportunity to update the description of the
'node_not_dev' flag, as to clarify its purpose.

Signed-off-by: Ulf Hansson <redacted>

Rob, just a gentle reminder about this one. Should I perhaps resend?

Kind regards
Uffe

quoted hunk

---
 drivers/of/property.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 6c028632f425..a94d007be416 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c

@@ -1075,6 +1075,17 @@ static struct device_node *of_get_compat_node(struct device_node *np)
        return np;
 }

+static struct device_node *of_get_compat_node_parent(struct device_node *np)
+{
+       struct device_node *parent, *node;
+
+       parent = of_get_parent(np);
+       node = of_get_compat_node(parent);
+       of_node_put(parent);
+
+       return node;
+}
+
 /**
  * of_link_to_phandle - Add fwnode link to supplier from supplier phandle
  * @con_np: consumer device tree node

@@ -1249,7 +1260,9 @@ static struct device_node *parse_##fname(struct device_node *np,       \
  * @parse_prop.index: For properties holding a list of phandles, this is the
  *                   index into the list
  * @optional: Describes whether a supplier is mandatory or not
- * @node_not_dev: The consumer node containing the property is never a device.
+ * @node_not_dev: The consumer node containing the property is never converted
+ *               to a struct device. Instead, parse ancestor nodes for the
+ *               compatible property to find a node corresponding to a device.
  *
  * Returns:
  * parse_prop() return values are

@@ -1416,7 +1429,7 @@ static int of_link_property(struct device_node *con_np, const char *prop_name)
                        struct device_node *con_dev_np;

                        con_dev_np = s->node_not_dev
-                                       ? of_get_compat_node(con_np)
+                                       ? of_get_compat_node_parent(con_np)
                                        : of_node_get(con_np);
                        matched = true;
                        i++;
--

2.25.1

Re: [PATCH] of: property: fw_devlink: Fixup behaviour when 'node_not_dev' is set

From: Rob Herring <robh@kernel.org>
Date: 2021-11-18 14:14:26

On Thu, 02 Sep 2021 11:02:21 +0200, Ulf Hansson wrote:

In the struct supplier_bindings the member 'node_not_dev' is described as
"The consumer node containing the property is never a device.", but that is
inconsistent with the behaviour of the code in of_link_property(), as it
calls of_get_compat_node() that starts parsing for a compatible property
from the node it gets passed to it. The proper behaviour is to start at the
node's parent, so let's do that.

While at it, let's take the opportunity to update the description of the
'node_not_dev' flag, as to clarify its purpose.

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/of/property.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Applied, thanks!

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help