Improve remote-endpoint parsing

We have a more accurate function to find the right consumer of a remote-endpoint property instead of searching for a parent with compatible string property. So, use that instead. While at it, make the code to find the consumer a bit more flexible and based on the property being parsed.

Fixes f7514a663016 ("of: property: fw_devlink: Add support for remote-endpoint")

Signed-off-by Saravana Kannan <[email protected]>

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

drivers/of/property.c +4 -34

index 641a40cf5cf3..ba374a1f2072 100644
@@ -1063,36 +1063,6 @@ of_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 	return of_device_get_match_data(dev);
 }
 
-static struct device_node *of_get_compat_node(struct device_node *np)
-{
-	of_node_get(np);
-
-	while (np) {
-		if (!of_device_is_available(np)) {
-			of_node_put(np);
-			np = NULL;
-		}
-
-		if (of_property_present(np, "compatible"))
-			break;
-
-		np = of_get_next_parent(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;
-}
-
 static void of_link_to_phandle(struct device_node *con_np,
 			      struct device_node *sup_np)
 {

@@ -1222,10 +1192,10 @@ static struct device_node *parse_##fname(struct device_node *np,	     \
  *  parse_prop.prop_name: Name of property holding a phandle value
  *  parse_prop.index: For properties holding a list of phandles, this is the
  *		      index into the list
+ * @get_con_dev: If the consumer node containing the property is never converted
+ *		 to a struct device, implement this ops so fw_devlink can use it
+ *		 to find the true consumer.
  * @optional: Describes whether a supplier is mandatory or not
- * @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

@@ -1236,8 +1206,8 @@ static struct device_node *parse_##fname(struct device_node *np,	     \
 struct supplier_bindings {
 	struct device_node *(*parse_prop)(struct device_node *np,
 					  const char *prop_name, int index);
+	struct device_node *(*get_con_dev)(struct device_node *np);
 	bool optional;
-	bool node_not_dev;
 };
 
 DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells")

Quoted 81 lines · Saravana Kannan

We have a more accurate function to find the right consumer of a remote-endpoint property instead of searching for a parent with compatible string property. So, use that instead. While at it, make the code to find the consumer a bit more flexible and based on the property being parsed.

Fixes f7514a663016 ("of: property: fw_devlink: Add support for remote-endpoint")

Signed-off-by Saravana Kannan <[email protected]>

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

drivers/of/property.c +4 -34

index 641a40cf5cf3..ba374a1f2072 100644
@@ -1063,36 +1063,6 @@ of_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
      return of_device_get_match_data(dev);
 }
 
-static struct device_node *of_get_compat_node(struct device_node *np)
-{
-     of_node_get(np);
-
-     while (np) {
-             if (!of_device_is_available(np)) {
-                     of_node_put(np);
-                     np = NULL;
-             }
-
-             if (of_property_present(np, "compatible"))
-                     break;
-
-             np = of_get_next_parent(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;
-}
-
 static void of_link_to_phandle(struct device_node *con_np,
                            struct device_node *sup_np)
 {

@@ -1222,10 +1192,10 @@ static struct device_node *parse_##fname(struct device_node *np,           \
  *  parse_prop.prop_name: Name of property holding a phandle value
  *  parse_prop.index: For properties holding a list of phandles, this is the
  *                 index into the list
+ * @get_con_dev: If the consumer node containing the property is never converted
+ *            to a struct device, implement this ops so fw_devlink can use it
+ *            to find the true consumer.
  * @optional: Describes whether a supplier is mandatory or not
- * @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

@@ -1236,8 +1206,8 @@ static struct device_node *parse_##fname(struct device_node *np,             \
 struct supplier_bindings {
      struct device_node *(*parse_prop)(struct device_node *np,
                                        const char *prop_name, int index);
+     struct device_node *(*get_con_dev)(struct device_node *np);
      bool optional;
-     bool node_not_dev;
 };
 
 DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells")

@@ -1328,6 +1298,11 @@ static struct device_node *parse_interrupts(struct device_node *np,
      return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np;
 }
 

drivers/of/property.c static struct device_node *parse_interrupts(struct device_node *np, · line 1303

Quoted 3 lines

drivers/of/property.c +3 -0

+static struct device_node *get_remote_endpoint_dev(struct device_node *np)
+{
+     return to_of_node(fwnode_graph_get_port_parent(of_fwnode_handle(np)));

DT APIs shouldn't depend on fwnode APIs.

Otherwise, this series looks good.

Quoted 83 lines · Rob Herring

Quoted 81 lines · Saravana Kannan

We have a more accurate function to find the right consumer of a remote-endpoint property instead of searching for a parent with compatible string property. So, use that instead. While at it, make the code to find the consumer a bit more flexible and based on the property being parsed.

Fixes f7514a663016 ("of: property: fw_devlink: Add support for remote-endpoint")

Signed-off-by Saravana Kannan <[email protected]>

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

drivers/of/property.c +4 -34

index 641a40cf5cf3..ba374a1f2072 100644
@@ -1063,36 +1063,6 @@ of_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
      return of_device_get_match_data(dev);
 }
 
-static struct device_node *of_get_compat_node(struct device_node *np)
-{
-     of_node_get(np);
-
-     while (np) {
-             if (!of_device_is_available(np)) {
-                     of_node_put(np);
-                     np = NULL;
-             }
-
-             if (of_property_present(np, "compatible"))
-                     break;
-
-             np = of_get_next_parent(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;
-}
-
 static void of_link_to_phandle(struct device_node *con_np,
                            struct device_node *sup_np)
 {

@@ -1222,10 +1192,10 @@ static struct device_node *parse_##fname(struct device_node *np,           \
  *  parse_prop.prop_name: Name of property holding a phandle value
  *  parse_prop.index: For properties holding a list of phandles, this is the
  *                 index into the list
+ * @get_con_dev: If the consumer node containing the property is never converted
+ *            to a struct device, implement this ops so fw_devlink can use it
+ *            to find the true consumer.
  * @optional: Describes whether a supplier is mandatory or not
- * @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

@@ -1236,8 +1206,8 @@ static struct device_node *parse_##fname(struct device_node *np,             \
 struct supplier_bindings {
      struct device_node *(*parse_prop)(struct device_node *np,
                                        const char *prop_name, int index);
+     struct device_node *(*get_con_dev)(struct device_node *np);
      bool optional;
-     bool node_not_dev;
 };
 
 DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells")

@@ -1328,6 +1298,11 @@ static struct device_node *parse_interrupts(struct device_node *np,
      return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np;
 }
 

Quoted 5 lines

drivers/of/property.c static struct device_node *parse_interrupts(struct device_node *np, · line 1303

Quoted 3 lines

drivers/of/property.c +3 -0

+static struct device_node *get_remote_endpoint_dev(struct device_node *np)
+{
+     return to_of_node(fwnode_graph_get_port_parent(of_fwnode_handle(np)));

DT APIs shouldn't depend on fwnode APIs.

I get what you are saying, but is it an API though? It's a static internal function that's eventually used by .add_link to add fwnode links. Would it be better if I rolled this into two separate inline code where this function is getting called from? I'm trying to avoid duplicating the function that's already present in fwnode_graph_get_port_parent(), but it also feels too specific an op to introduce into drivers/of/property.c

How would you like me to handle this? I don't see a good option.