We already have of_graph_get_next_endpoint(), but it is not
intuitive to use in some case.

(X)     node {
(Y)             ports {
(P0)                    port@0 { endpoint { remote-endpoint = ...; };};
(P10)                   port@1 { endpoint { remote-endpoint = ...; };
(P11)                            endpoint { remote-endpoint = ...; };};
(P2)                    port@2 { endpoint { remote-endpoint = ...; };};
                };
        };

For example, if I want to handle port@1's 2 endpoints (= P10, P11),
I want to use like below

        P10 = of_graph_get_next_endpoint(port1, NULL);
        P11 = of_graph_get_next_endpoint(port1, P10);

But 1st one will be error, because of_graph_get_next_endpoint()
requested 1st parameter is "node" (X) or "ports" (Y), not but "port".
Below works well, but it will get P0

        P0 = of_graph_get_next_endpoint(node,  NULL);
        P0 = of_graph_get_next_endpoint(ports, NULL);

In other words, we can't handle P10/P11 directly via
of_graph_get_next_endpoint().

There is another non intuitive behavior on of_graph_get_next_endpoint().
In case of if I could get P10 pointer for some way, and if I want to
handle port@1 things by loop, I would like use it like below

        /*
         * "ep" is now P10, and handle port1 things here,
         * but we don't know how many endpoints port1 have.
         *
         * Because "ep" is non NULL now, we can use port1
         * as of_graph_get_next_endpoint(port1, xxx)
         */
        do {
                /* do something for port1 specific things here */
        } while (ep = of_graph_get_next_endpoint(port1, ep))

But it also not worked as I expected.
I expect it will be P10 -> P11 -> NULL,
but      it will be P10 -> P11 -> P2,    because
of_graph_get_next_endpoint() will fetch "endpoint" beyond the "port".
It is not useful for generic driver.

To handle endpoint more intuitive, create of_graph_get_next_port_endpoint()

        of_graph_get_next_port_endpoint(port1, NULL); // P10
        of_graph_get_next_port_endpoint(port1, P10);  // P11
        of_graph_get_next_port_endpoint(port1, P11);  // NULL

Signed-off-by: Kuninori Morimoto <kuninori.morimoto...@renesas.com>
---
 drivers/of/property.c    | 27 +++++++++++++++++++++++++++
 include/linux/of_graph.h | 21 +++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 375d81fad8bf8..fcc609a17fe35 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -667,6 +667,33 @@ struct device_node *of_graph_get_next_port(const struct 
device_node *parent,
 }
 EXPORT_SYMBOL(of_graph_get_next_port);
 
+/**
+ * of_graph_get_next_port_endpoint() - get next endpoint node in port.
+ * If it reached to end of the port, it will return NULL.
+ * @port: pointer to the target port node
+ * @prev: previous endpoint node, or NULL to get first
+ *
+ * Return: An 'endpoint' node pointer with refcount incremented. Refcount
+ * of the passed @prev node is decremented.
+ */
+struct device_node *of_graph_get_next_port_endpoint(const struct device_node 
*port,
+                                                   struct device_node *prev)
+{
+       while (1) {
+               prev = of_get_next_child(port, prev);
+               if (!prev)
+                       break;
+               if (WARN(!of_node_name_eq(prev, "endpoint"),
+                        "non endpoint node is used (%pOF)", prev))
+                       continue;
+
+               break;
+       }
+
+       return prev;
+}
+EXPORT_SYMBOL(of_graph_get_next_port_endpoint);
+
 /**
  * of_graph_get_next_endpoint() - get next endpoint node
  * @parent: pointer to the parent device node
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 44518f3583a44..a692d9d979a64 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -50,6 +50,18 @@ struct of_endpoint {
        for (struct device_node *child __free(device_node) = 
of_graph_get_next_port(parent, NULL);\
             child != NULL; child = of_graph_get_next_port(parent, child))
 
+/**
+ * for_each_of_graph_port_endpoint - iterate over every endpoint in a port node
+ * @parent: parent port node
+ * @child: loop variable pointing to the current endpoint node
+ *
+ * When breaking out of the loop, and continue to use the @child, you need to
+ * use return_ptr(@child) or no_free_ptr(@child) not to call __free() for it.
+ */
+#define for_each_of_graph_port_endpoint(parent, child)                 \
+       for (struct device_node *child __free(device_node) = 
of_graph_get_next_port_endpoint(parent, NULL);\
+            child != NULL; child = of_graph_get_next_port_endpoint(parent, 
child))
+
 #ifdef CONFIG_OF
 bool of_graph_is_present(const struct device_node *node);
 int of_graph_parse_endpoint(const struct device_node *node,
@@ -61,6 +73,8 @@ struct device_node *of_graph_get_next_endpoint(const struct 
device_node *parent,
                                        struct device_node *previous);
 struct device_node *of_graph_get_next_port(const struct device_node *parent,
                                           struct device_node *port);
+struct device_node *of_graph_get_next_port_endpoint(const struct device_node 
*port,
+                                                   struct device_node *prev);
 struct device_node *of_graph_get_endpoint_by_regs(
                const struct device_node *parent, int port_reg, int reg);
 struct device_node *of_graph_get_remote_endpoint(
@@ -114,6 +128,13 @@ static inline struct device_node *of_graph_get_next_port(
        return NULL;
 }
 
+static inline struct device_node *of_graph_get_next_port_endpoint(
+                                       const struct device_node *parent,
+                                       struct device_node *previous)
+{
+       return NULL;
+}
+
 static inline struct device_node *of_graph_get_endpoint_by_regs(
                const struct device_node *parent, int port_reg, int reg)
 {
-- 
2.43.0

Reply via email to