From: Kuninori Morimoto <[email protected]>
We have endpoint base functions
- of_graph_get_next_endpoint()
- of_graph_get_endpoint_count()
- for_each_endpoint_of_node()
Here, for_each_endpoint_of_node() loop finds endpoint
ports {
port@0 {
(1) endpoint {...};
};
port@1 {
(2) endpoint {...};
};
...
};
In above case, for_each_endpoint_of_node() loop
finds endpoint as (1) -> (2) -> ...
We can check endpoint parent to get its port
if user want to do something to it.
But port can have multi endpoints.
In such case, it is difficult to find
port@0 -> port@1 -> ...
ports {
port@0 {
(1) endpoint@0 {...};
(2) endpoint@1 {...};
};
port@1 {
(3) endpoint {...};
};
...
};
In such case, people want to have port base loop
instead of endpoints base loop.
This patch adds such functions/macros.
Signed-off-by: Kuninori Morimoto <[email protected]>
---
drivers/of/property.c | 69 ++++++++++++++++++++++++++++++++++------
include/linux/of_graph.h | 14 ++++++++
2 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 5f9eed79a8aa..9b511cfe97b3 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -631,15 +631,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
* parent port node.
*/
if (!prev) {
- struct device_node *node;
-
- node = of_get_child_by_name(parent, "ports");
- if (node)
- parent = node;
-
- port = of_get_child_by_name(parent, "port");
- of_node_put(node);
-
+ port = of_graph_get_next_port(parent, NULL);
if (!port) {
pr_err("graph: no port node found in %pOF\n", parent);
return NULL;
@@ -666,14 +658,59 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
/* No more endpoints under this port, try the next one. */
prev = NULL;
+ port = of_graph_get_next_port(parent, port);
+ if (!port)
+ return NULL;
+ }
+}
+EXPORT_SYMBOL(of_graph_get_next_endpoint);
+
+/**
+ * of_graph_get_next_port() - get next port node
+ * @parent: pointer to the parent device node
+ * @prev: previous port node, or NULL to get first
+ *
+ * Return: An 'port' node pointer with refcount incremented. Refcount
+ * of the passed @prev node is decremented.
+ */
+struct device_node *of_graph_get_next_port(const struct device_node *parent,
+ struct device_node *prev)
+{
+ struct device_node *port = prev;
+
+ if (!parent)
+ return NULL;
+
+ /*
+ * Start by locating the port node. If no previous endpoint is specified
+ * search for the first port node, otherwise get the previous endpoint
+ * parent port node.
+ */
+ if (!port) {
+ struct device_node *node;
+
+ node = of_get_child_by_name(parent, "ports");
+ if (node)
+ parent = node;
+
+ port = of_get_child_by_name(parent, "port");
+ of_node_put(node);
+
+ if (!port) {
+ pr_err("graph: no port node found in %pOF\n", parent);
+ return NULL;
+ }
+ } else {
do {
port = of_get_next_child(parent, port);
if (!port)
return NULL;
} while (!of_node_name_eq(port, "port"));
}
+
+ return port;
}
-EXPORT_SYMBOL(of_graph_get_next_endpoint);
+EXPORT_SYMBOL(of_graph_get_next_port);
/**
* of_graph_get_endpoint_by_regs() - get endpoint node of specific identifiers
@@ -800,6 +837,18 @@ int of_graph_get_endpoint_count(const struct device_node *np)
}
EXPORT_SYMBOL(of_graph_get_endpoint_count);
+int of_graph_get_port_count(const struct device_node *np)
+{
+ struct device_node *port;
+ int num = 0;
+
+ for_each_port_of_node(np, port)
+ num++;
+
+ return num;
+}
+EXPORT_SYMBOL(of_graph_get_port_count);
+
/**
* of_graph_get_remote_node() - get remote parent device_node for given port/endpoint
* @node: pointer to parent device_node containing graph port/endpoint
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 4d7756087b6b..8cd3bd674ebd 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -26,6 +26,17 @@ struct of_endpoint {
const struct device_node *local_node;
};
+/**
+ * for_each_port_of_node - iterate over every port in a device node
+ * @parent: parent device node containing ports and port
+ * @child: loop variable pointing to the current port node
+ *
+ * When breaking out of the loop, of_node_put(child) has to be called manually.
+ */
+#define for_each_port_of_node(parent, child) \
+ for (child = of_graph_get_next_port(parent, NULL); child != NULL; \
+ child = of_graph_get_next_port(parent, child))
+
/**
* for_each_endpoint_of_node - iterate over every endpoint in a device node
* @parent: parent device node containing ports and endpoints
@@ -41,8 +52,11 @@ struct of_endpoint {
bool of_graph_is_present(const struct device_node *node);
int of_graph_parse_endpoint(const struct device_node *node,
struct of_endpoint *endpoint);
+int of_graph_get_port_count(const struct device_node *np);
int of_graph_get_endpoint_count(const struct device_node *np);
struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id);
+struct device_node *of_graph_get_next_port(const struct device_node *parent,
+ struct device_node *previous);
struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
struct device_node *previous);
struct device_node *of_graph_get_endpoint_by_regs(
--
2.25.1
On Wed, Jan 13, 2021 at 12:53 AM Kuninori Morimoto
<[email protected]> wrote:
>
>
> From: Kuninori Morimoto <[email protected]>
>
> We have endpoint base functions
> - of_graph_get_next_endpoint()
> - of_graph_get_endpoint_count()
> - for_each_endpoint_of_node()
I actually think these functions don't make sense. Iterating over
endpoints for a port make sense, but not all endpoints.
> Here, for_each_endpoint_of_node() loop finds endpoint
>
> ports {
> port@0 {
> (1) endpoint {...};
> };
> port@1 {
> (2) endpoint {...};
> };
> ...
> };
>
> In above case, for_each_endpoint_of_node() loop
> finds endpoint as (1) -> (2) -> ...
> We can check endpoint parent to get its port
> if user want to do something to it.
>
> But port can have multi endpoints.
> In such case, it is difficult to find
> port@0 -> port@1 -> ...
>
> ports {
> port@0 {
> (1) endpoint@0 {...};
> (2) endpoint@1 {...};
> };
> port@1 {
> (3) endpoint {...};
> };
> ...
> };
>
> In such case, people want to have port base loop
> instead of endpoints base loop.
> This patch adds such functions/macros.
I'm a bit hesitant on these too. A driver should generally know what
each port # is (since the binding has to define them), and it should
just request the port (or its connection) it wants. At least that was
the premise behind of_graph_get_remote_node() and the cleanups (mostly
DRM drivers) I did to use it. I'd rather see things move in that
direction.
In any case, this needs a user before merging.
>
> Signed-off-by: Kuninori Morimoto <[email protected]>
> ---
> drivers/of/property.c | 69 ++++++++++++++++++++++++++++++++++------
> include/linux/of_graph.h | 14 ++++++++
> 2 files changed, 73 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 5f9eed79a8aa..9b511cfe97b3 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -631,15 +631,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> * parent port node.
> */
> if (!prev) {
> - struct device_node *node;
> -
> - node = of_get_child_by_name(parent, "ports");
> - if (node)
> - parent = node;
> -
> - port = of_get_child_by_name(parent, "port");
> - of_node_put(node);
> -
> + port = of_graph_get_next_port(parent, NULL);
> if (!port) {
> pr_err("graph: no port node found in %pOF\n", parent);
> return NULL;
> @@ -666,14 +658,59 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> /* No more endpoints under this port, try the next one. */
> prev = NULL;
>
> + port = of_graph_get_next_port(parent, port);
> + if (!port)
> + return NULL;
> + }
> +}
> +EXPORT_SYMBOL(of_graph_get_next_endpoint);
> +
> +/**
> + * of_graph_get_next_port() - get next port node
> + * @parent: pointer to the parent device node
> + * @prev: previous port node, or NULL to get first
> + *
> + * Return: An 'port' node pointer with refcount incremented. Refcount
> + * of the passed @prev node is decremented.
> + */
> +struct device_node *of_graph_get_next_port(const struct device_node *parent,
> + struct device_node *prev)
> +{
> + struct device_node *port = prev;
> +
> + if (!parent)
> + return NULL;
> +
> + /*
> + * Start by locating the port node. If no previous endpoint is specified
> + * search for the first port node, otherwise get the previous endpoint
> + * parent port node.
> + */
> + if (!port) {
> + struct device_node *node;
> +
> + node = of_get_child_by_name(parent, "ports");
> + if (node)
> + parent = node;
> +
> + port = of_get_child_by_name(parent, "port");
> + of_node_put(node);
> +
> + if (!port) {
> + pr_err("graph: no port node found in %pOF\n", parent);
> + return NULL;
> + }
> + } else {
> do {
> port = of_get_next_child(parent, port);
> if (!port)
> return NULL;
> } while (!of_node_name_eq(port, "port"));
> }
> +
> + return port;
> }
> -EXPORT_SYMBOL(of_graph_get_next_endpoint);
> +EXPORT_SYMBOL(of_graph_get_next_port);
>
> /**
> * of_graph_get_endpoint_by_regs() - get endpoint node of specific identifiers
> @@ -800,6 +837,18 @@ int of_graph_get_endpoint_count(const struct device_node *np)
> }
> EXPORT_SYMBOL(of_graph_get_endpoint_count);
>
> +int of_graph_get_port_count(const struct device_node *np)
> +{
> + struct device_node *port;
> + int num = 0;
> +
> + for_each_port_of_node(np, port)
> + num++;
> +
> + return num;
> +}
> +EXPORT_SYMBOL(of_graph_get_port_count);
> +
> /**
> * of_graph_get_remote_node() - get remote parent device_node for given port/endpoint
> * @node: pointer to parent device_node containing graph port/endpoint
> diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> index 4d7756087b6b..8cd3bd674ebd 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -26,6 +26,17 @@ struct of_endpoint {
> const struct device_node *local_node;
> };
>
> +/**
> + * for_each_port_of_node - iterate over every port in a device node
> + * @parent: parent device node containing ports and port
> + * @child: loop variable pointing to the current port node
> + *
> + * When breaking out of the loop, of_node_put(child) has to be called manually.
> + */
> +#define for_each_port_of_node(parent, child) \
> + for (child = of_graph_get_next_port(parent, NULL); child != NULL; \
> + child = of_graph_get_next_port(parent, child))
> +
> /**
> * for_each_endpoint_of_node - iterate over every endpoint in a device node
> * @parent: parent device node containing ports and endpoints
> @@ -41,8 +52,11 @@ struct of_endpoint {
> bool of_graph_is_present(const struct device_node *node);
> int of_graph_parse_endpoint(const struct device_node *node,
> struct of_endpoint *endpoint);
> +int of_graph_get_port_count(const struct device_node *np);
> int of_graph_get_endpoint_count(const struct device_node *np);
> struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id);
> +struct device_node *of_graph_get_next_port(const struct device_node *parent,
> + struct device_node *previous);
> struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> struct device_node *previous);
> struct device_node *of_graph_get_endpoint_by_regs(
> --
> 2.25.1
>
Hi Rob
Thank you for your feedback
> > In such case, people want to have port base loop
> > instead of endpoints base loop.
> > This patch adds such functions/macros.
>
> I'm a bit hesitant on these too. A driver should generally know what
> each port # is (since the binding has to define them), and it should
> just request the port (or its connection) it wants. At least that was
> the premise behind of_graph_get_remote_node() and the cleanups (mostly
> DRM drivers) I did to use it. I'd rather see things move in that
> direction.
I'm working for ALSA SoC now.
To use ALSA SoC sound, it needs at least 3 drivers.
The purpose of "Card" here is for glueing SoC and Codec.
SoC <-> Card <-> Codec
If board/platform needs special handling,
user needs to create specific Card driver.
In such case, it needs to know detail of each ports.
But many cases, we want to use generic Card driver.
Because it is generic, it don't know how many ports are used,
and endpoint base loop is not enough for it.
This is my motivation for this patch.
> In any case, this needs a user before merging.
OK.
I will repost it again as part of above patch-set.
Thank you for your help !!
Best regards
---
Kuninori Morimoto