# \[PATCH 0/9\] of: property: add of\_graph\_get\_next\_port/port\_endpoint()

- Subsystems: FRAMEBUFFER LAYER, OMAP DISPLAY SUBSYSTEM and FRAMEBUFFER SUPPORT (DSS2), DRM DRIVERS, DRM DRIVERS AND MISC GPU PATCHES, DRM DRIVERS FOR TI OMAP, MEDIA INPUT INFRASTRUCTURE (V4L/DVB), OPEN FIRMWARE AND FLATTENED DEVICE TREE, SOUND, SOUND - GENERIC SOUND CARD (Simple-Audio-Card, Audio-Graph-Card), SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC), THE REST, XILINX VIDEO IP CORES
- Maintainers: Helge Deller, David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Tomi Valkeinen, Mauro Carvalho Chehab, Rob Herring, Saravana Kannan, Jaroslav Kysela, Takashi Iwai, Mark Brown, Kuninori Morimoto, Liam Girdwood, Linus Torvalds, Laurent Pinchart
- Series: 9 patches touching 14 files, +244/-159
- Status: SUPERSEDED (Superseded by v8 posted 2024-10-24)
- Superseded by v8 (posted 2024-10-24).
- Revision v1 (8 revisions archived).
- List: linux-fbdev
- 22 messages, 3 authors
- Page: https://ratatoskr.run/linux-fbdev/2024/08/8236243/t

## \[PATCH 0/9\] of: property: add of\_graph\_get\_next\_port/port\_endpoint()

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:57:59 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236243.md

```text

Hi Rob, Saravana

I have been posting to add new port base for loop function
as below steps.

[o] done
[*] posting (A)
[@] this patch set

	[o] tidyup of_graph_get_endpoint_count()
	[o] replace endpoint func - use endpoint_by_regs()
	[*] replace endpoint func - use for_each()
	[@] add new port function

Because posting [*] patches are no update during this few month
(I'm keeping re-posting), and it and [@] patches are independent,
I try to post the patch.

Current Of-graph has "endpoint base" for loop, but doesn't have
"port base" loop. "endpoint base" loop only is not enough.
This patch-set add new "port base" for loop, and use it.


Kuninori Morimoto (9):
  of: property: add of_graph_get_next_port()
  of: property: add of_graph_get_next_port_endpoint()
  ASoC: test-component: use new of_graph functions
  ASoC: rcar_snd: use new of_graph functions
  ASoC: audio-graph-card: use new of_graph functions
  ASoC: audio-graph-card2: use new of_graph functions
  gpu: drm: omapdrm: use new of_graph functions
  fbdev: omapfb: use new of_graph functions
  media: xilinx-tpg: use new of_graph functions

 drivers/gpu/drm/omapdrm/dss/dpi.c             |   3 +-
 drivers/gpu/drm/omapdrm/dss/sdi.c             |   3 +-
 drivers/media/platform/xilinx/xilinx-tpg.c    |   3 +-
 drivers/of/property.c                         | 108 +++++++++++++++++
 drivers/video/fbdev/omap2/omapfb/dss/dpi.c    |   3 +-
 drivers/video/fbdev/omap2/omapfb/dss/dss-of.c |  66 -----------
 drivers/video/fbdev/omap2/omapfb/dss/dss.c    |   9 +-
 drivers/video/fbdev/omap2/omapfb/dss/sdi.c    |   3 +-
 include/linux/of_graph.h                      |  67 +++++++++++
 include/video/omapfb_dss.h                    |   8 --
 sound/soc/generic/audio-graph-card.c          |   5 +-
 sound/soc/generic/audio-graph-card2.c         | 111 ++++++++----------
 sound/soc/generic/test-component.c            |   2 +-
 sound/soc/sh/rcar/core.c                      |  12 +-
 14 files changed, 244 insertions(+), 159 deletions(-)

-- 
2.43.0

```

## \[PATCH 1/9\] of: property: add of\_graph\_get\_next\_port()

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:23 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236244.md

```text
We have endpoint base functions
	- of_graph_get_next_device_endpoint()
	- of_graph_get_device_endpoint_count()
	- for_each_of_graph_device_endpoint()

Here, for_each_of_graph_device_endpoint() loop finds each endpoints

	ports {
		port@0 {
(1)			endpoint {...};
		};
		port@1 {
(2)			endpoint {...};
		};
		...
	};

In above case, it finds endpoint as (1) -> (2) -> ...

Basically, user/driver knows which port is used for what, but not in
all cases. For example on flexible/generic driver case, how many ports
are used is not fixed.

For example Sound Generic Card driver which is used from many venders
can't know how many ports are used. Because the driver is very
flexible/generic, it is impossible to know how many ports are used,
it depends on each vender SoC and/or its used board.

And more, the port can have multi endpoints. For example Generic Sound
Card case, it supports many type of connection between CPU / Codec, and
some of them uses multi endpoint in one port.
Then, Generic Sound Card want to handle each connection via "port"
instead of "endpoint".
But, it is very difficult to handle each "port" via
for_each_of_graph_device_endpoint(). Getting "port" by using
of_get_parent() from "endpoint" doesn't work. see below.

	ports {
		port@0 {
(1)			endpoint@0 {...};
(2)			endpoint@1 {...};
		};
		port@1 {
(3)			endpoint {...};
		};
		...
	};

In the same time, same reason, we want to handle "ports" same as "port".

	node {
=>		ports@0 {
			port@0 {
				endpoint@0 {...};
				endpoint@1 {...};
				...
			};
			port@1 {
				endpoint@0 {...};
				endpoint@1 {...};
				...
			};
			...
		};
=>		ports@1 {
			...
		};
	};

Add "ports" / "port" base functions.
For above case, we can use

	for_each_of_graph_ports(node, ports) {
		for_each_of_graph_port(ports, port) {
			...
		}
	}

This loop works in case of "node" doesn't have "ports" also.

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

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 164d77cb9445..3b2d09c0376a 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -625,8 +625,76 @@ struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
 }
 EXPORT_SYMBOL(of_graph_get_port_by_id);
 
+/**
+ * of_graph_get_next_ports() - get next ports node.
+ * @parent: pointer to the parent device node
+ * @ports: current ports node, or NULL to get first
+ *
+ * Return: A 'ports' node pointer with refcount incremented. Refcount
+ * of the passed @prev node is decremented.
+ */
+struct device_node *of_graph_get_next_ports(struct device_node *parent,
+					    struct device_node *ports)
+{
+	if (!parent)
+		return NULL;
+
+	if (!ports) {
+		ports = of_get_child_by_name(parent, "ports");
+
+		/* use parent as its ports of this device if it not exist */
+		if (!ports) {
+			ports = parent;
+			of_node_get(ports);
+		}
+
+		return ports;
+	}
+
+	do {
+		ports = of_get_next_child(parent, ports);
+		if (!ports)
+			break;
+	} while (!of_node_name_eq(ports, "ports"));
+
+	return ports;
+}
+EXPORT_SYMBOL(of_graph_get_next_ports);
+
+/**
+ * of_graph_get_next_port() - get next port node.
+ * @parent: pointer to the parent device node
+ * @port: current port node, or NULL to get first
+ *
+ * Return: A 'port' node pointer with refcount incremented. Refcount
+ * of the passed @prev node is decremented.
+ */
+struct device_node *of_graph_get_next_port(struct device_node *parent,
+					   struct device_node *port)
+{
+	if (!parent)
+		return NULL;
+
+	if (!port) {
+		struct device_node *ports __free(device_node) =
+			of_graph_get_next_ports(parent, NULL);
+
+		return of_get_child_by_name(ports, "port");
+	}
+
+	do {
+		port = of_get_next_child(parent, port);
+		if (!port)
+			break;
+	} while (!of_node_name_eq(port, "port"));
+
+	return port;
+}
+EXPORT_SYMBOL(of_graph_get_next_port);
+
 /**
  * of_graph_get_next_endpoint() - get next endpoint node
+ *
  * @parent: pointer to the parent device node
  * @prev: previous endpoint node, or NULL to get first
  *
@@ -823,6 +891,24 @@ unsigned int of_graph_get_endpoint_count(const struct device_node *np)
 }
 EXPORT_SYMBOL(of_graph_get_endpoint_count);
 
+/**
+ * of_graph_get_port_count() - get count of port
+ * @np: pointer to the parent device node
+ *
+ * Return: count of port of this device node
+ */
+unsigned int of_graph_get_port_count(struct device_node *np)
+{
+	struct device_node *port = NULL;
+	int num = 0;
+
+	for_each_of_graph_port(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 a4bea62bfa29..30169b50b042 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -37,14 +37,42 @@ struct of_endpoint {
 	for (child = of_graph_get_next_endpoint(parent, NULL); child != NULL; \
 	     child = of_graph_get_next_endpoint(parent, child))
 
+/**
+ * for_each_of_graph_ports - iterate over every ports in a device node
+ * @parent: parent device node containing ports
+ * @child: loop variable pointing to the current ports node
+ *
+ * When breaking out of the loop, of_node_put(child) has to be called manually.
+ */
+#define for_each_of_graph_ports(parent, child)				\
+	for (child = of_graph_get_next_ports(parent, NULL); child != NULL; \
+	     child = of_graph_get_next_ports(parent, child))
+
+/**
+ * for_each_of_graph_port - iterate over every port in a device or ports node
+ * @parent: parent device or ports node containing 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_of_graph_port(parent, child)			\
+	for (child = of_graph_get_next_port(parent, NULL); child != NULL; \
+	     child = of_graph_get_next_port(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,
 				struct of_endpoint *endpoint);
+
 unsigned int of_graph_get_endpoint_count(const struct device_node *np);
+unsigned int of_graph_get_port_count(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_endpoint(const struct device_node *parent,
 					struct device_node *previous);
+struct device_node *of_graph_get_next_ports(struct device_node *parent,
+					    struct device_node *ports);
+struct device_node *of_graph_get_next_port(struct device_node *parent,
+					   struct device_node *port);
 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(
@@ -73,6 +101,11 @@ static inline unsigned int of_graph_get_endpoint_count(const struct device_node
 	return 0;
 }
 
+static inline unsigned int of_graph_get_port_count(struct device_node *np)
+{
+	return 0;
+}
+
 static inline struct device_node *of_graph_get_port_by_id(
 					struct device_node *node, u32 id)
 {
@@ -86,6 +119,20 @@ static inline struct device_node *of_graph_get_next_endpoint(
 	return NULL;
 }
 
+static inline struct device_node *of_graph_get_next_ports(
+					struct device_node *parent,
+					struct device_node *previous)
+{
+	return NULL;
+}
+
+static inline struct device_node *of_graph_get_next_port(
+					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

```

## \[PATCH 2/9\] of: property: add of\_graph\_get\_next\_port\_endpoint()

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:31 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236245.md

```text
We already have of_graph_get_next_endpoint(), but it is not
intuitive to use.

(X)	node {
(Y)		ports {
			port@0 { endpoint { remote-endpoint = ...; };};
(A1)			port@1 { endpoint { remote-endpoint = ...; };
(A2)				 endpoint { remote-endpoint = ...; };};
(B)			port@2 { endpoint { remote-endpoint = ...; };};
		};
	};

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

	A1 = of_graph_get_next_endpoint(port1, NULL);
	A2 = of_graph_get_next_endpoint(port1, A1);

But 1st one will be error, because of_graph_get_next_endpoint()
requested "parent" means "node" (X) or "ports" (Y), not "port".
Below are OK

	/* These will be node/ports/port@0/endpoint */
	of_graph_get_next_endpoint(node,  NULL);
	of_graph_get_next_endpoint(ports, NULL);

In other words, we can't handle A1/A2 directly via
of_graph_get_next_endpoint() so far.

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

	/*
	 * "ep" is now A1, and handle port1 things here,
	 * but we don't know how many endpoints port1 has.
	 *
	 * Because "ep" is non NULL, 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 A1 -> A2 -> NULL,
but      it will be A1 -> A2 -> B,    because
of_graph_get_next_endpoint() will fetch "endpoint" beyond the "port".

It is not useful on generic driver like Generic Sound Card.
It uses of_get_next_child() instead for now, but it is not intuitive.
And it doesn't check node name (= "endpoint").

To handle endpoint more intuitive, create of_graph_get_next_port_endpoint()

	of_graph_get_next_port_endpoint(port1, NULL); // A1
	of_graph_get_next_port_endpoint(port1, A1);   // A2
	of_graph_get_next_port_endpoint(port1, A2);   // NULL

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

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 3b2d09c0376a..de56795a7362 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -692,6 +692,28 @@ struct device_node *of_graph_get_next_port(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
+ * @endpoint: current 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 *endpoint)
+{
+	do {
+		endpoint = of_get_next_child(port, endpoint);
+		if (!endpoint)
+			break;
+	} while (!of_node_name_eq(endpoint, "endpoint"));
+
+	return endpoint;
+}
+EXPORT_SYMBOL(of_graph_get_next_port_endpoint);
+
 /**
  * of_graph_get_next_endpoint() - get next endpoint node
  *
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 30169b50b042..8b4777938c5e 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -59,6 +59,17 @@ struct of_endpoint {
 	for (child = 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 device or ports node
+ * @child: loop variable pointing to the current endpoint node
+ *
+ * When breaking out of the loop, of_node_put(child) has to be called manually.
+ */
+#define for_each_of_graph_port_endpoint(parent, child)			\
+		for (child = 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,
@@ -73,6 +84,8 @@ struct device_node *of_graph_get_next_ports(struct device_node *parent,
 					    struct device_node *ports);
 struct device_node *of_graph_get_next_port(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(
@@ -133,6 +146,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

```

## \[PATCH 3/9\] ASoC: test-component: use new of\_graph functions

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:36 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236246.md

```text
Current test-component.c is using for_each_endpoint_of_node()
for parsing, but it should use "port" base loop instead of "endpoint",
because properties are "port" base, instead of "endpoint".

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/test-component.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/generic/test-component.c b/sound/soc/generic/test-component.c
index e9e5e235a8a6..6f75da9f86e6 100644
--- a/sound/soc/generic/test-component.c
+++ b/sound/soc/generic/test-component.c
@@ -600,7 +600,7 @@ static int test_driver_probe(struct platform_device *pdev)
 	}
 
 	i = 0;
-	for_each_endpoint_of_node(node, ep) {
+	for_each_of_graph_port(node, ep) {
 		snprintf(dname[i].name, TEST_NAME_LEN, "%s.%d", node->name, i);
 		ddriv[i].name = dname[i].name;
 
-- 
2.43.0

```

## \[PATCH 4/9\] ASoC: rcar\_snd: use new of\_graph functions

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:40 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236247.md

```text
Now we can use new port related functions for port parsing. Use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/core.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 63b3c8bf0fde..eb146cf836eb 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1294,11 +1294,8 @@ static int rsnd_dai_of_node(struct rsnd_priv *priv, int *is_graph)
 	/*
 	 * Audio-Graph-Card
 	 */
-	for_each_child_of_node(np, ports) {
-		if (!of_node_name_eq(ports, "ports") &&
-		    !of_node_name_eq(ports, "port"))
-			continue;
-		priv->component_dais[i] = of_graph_get_endpoint_count(ports);
+	for_each_of_graph_ports(np, ports) {
+		priv->component_dais[i] = of_graph_get_port_count(ports);
 		nr += priv->component_dais[i];
 		i++;
 		if (i >= RSND_MAX_COMPONENT) {
@@ -1506,10 +1503,7 @@ static int rsnd_dai_probe(struct rsnd_priv *priv)
 		struct device_node *ports;
 		struct device_node *dai_np;
 
-		for_each_child_of_node(np, ports) {
-			if (!of_node_name_eq(ports, "ports") &&
-			    !of_node_name_eq(ports, "port"))
-				continue;
+		for_each_of_graph_ports(np, ports) {
 			for_each_endpoint_of_node(ports, dai_np) {
 				__rsnd_dai_probe(priv, dai_np, dai_np, 0, dai_i);
 				if (!rsnd_is_gen1(priv) && !rsnd_is_gen2(priv)) {
-- 
2.43.0

```

## \[PATCH 5/9\] ASoC: audio-graph-card: use new of\_graph functions

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:45 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236248.md

```text
Now we can use new port related functions for port parsing. Use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 3425fbbcbd7e..040c271e13c6 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -376,10 +376,7 @@ static int __graph_for_each_link(struct simple_util_priv *priv,
 		cpu_ep	 = NULL;
 
 		/* loop for all CPU endpoint */
-		while (1) {
-			cpu_ep = of_get_next_child(cpu_port, cpu_ep);
-			if (!cpu_ep)
-				break;
+		for_each_of_graph_port_endpoint(cpu_port, cpu_ep) {
 
 			/* get codec */
 			codec_ep = of_graph_get_remote_endpoint(cpu_ep);
-- 
2.43.0

```

## \[PATCH 6/9\] ASoC: audio-graph-card2: use new of\_graph functions

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:49 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236249.md

```text
Now we can use new port related functions for port parsing. Use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card2.c | 111 ++++++++++++--------------
 1 file changed, 49 insertions(+), 62 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index 56f7f946882e..e8ea201dbca3 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -234,8 +234,6 @@ enum graph_type {
 #define GRAPH_NODENAME_DPCM	"dpcm"
 #define GRAPH_NODENAME_C2C	"codec2codec"
 
-#define port_to_endpoint(port) of_get_child_by_name(port, "endpoint")
-
 #define ep_to_port(ep)	of_get_parent(ep)
 static struct device_node *port_to_ports(struct device_node *port)
 {
@@ -351,14 +349,9 @@ static struct device_node *graph_get_next_multi_ep(struct device_node **port)
 	 *	port@1 { rep1 };
 	 * };
 	 */
-	do {
-		*port = of_get_next_child(ports, *port);
-		if (!*port)
-			break;
-	} while (!of_node_name_eq(*port, "port"));
-
+	*port = of_graph_get_next_port(ports, *port);
 	if (*port) {
-		ep  = port_to_endpoint(*port);
+		ep  = of_graph_get_next_port_endpoint(*port, NULL);
 		rep = of_graph_get_remote_endpoint(ep);
 	}
 
@@ -530,67 +523,70 @@ static int graph_parse_node_multi_nm(struct snd_soc_dai_link *dai_link,
 	 *	};
 	 * };
 	 */
-	struct device_node *mcpu_ep		= port_to_endpoint(mcpu_port);
-	struct device_node *mcpu_ep_n		= mcpu_ep;
-	struct device_node *mcpu_port_top	= of_get_next_child(port_to_ports(mcpu_port), NULL);
-	struct device_node *mcpu_ep_top		= port_to_endpoint(mcpu_port_top);
+	struct device_node *mcpu_ep_n;
+	struct device_node *mcpu_ep		= of_graph_get_next_port_endpoint(mcpu_port, NULL);
+	struct device_node *mcpu_ports		= port_to_ports(mcpu_port);
+	struct device_node *mcpu_port_top	= of_graph_get_next_port(mcpu_ports, NULL);
+	struct device_node *mcpu_ep_top		= of_graph_get_next_port_endpoint(mcpu_port_top, NULL);
 	struct device_node *mcodec_ep_top	= of_graph_get_remote_endpoint(mcpu_ep_top);
 	struct device_node *mcodec_port_top	= ep_to_port(mcodec_ep_top);
 	struct device_node *mcodec_ports	= port_to_ports(mcodec_port_top);
 	int nm_max = max(dai_link->num_cpus, dai_link->num_codecs);
-	int ret = -EINVAL;
+	int ret = 0;
 
-	if (cpu_idx > dai_link->num_cpus)
+	if (cpu_idx > dai_link->num_cpus) {
+		ret = -EINVAL;
 		goto mcpu_err;
+	}
 
-	while (1) {
+	for_each_of_graph_port_endpoint(mcpu_port, mcpu_ep_n) {
 		struct device_node *mcodec_ep_n;
 		struct device_node *mcodec_port_i;
 		struct device_node *mcodec_port;
 		int codec_idx;
 
-		if (*nm_idx > nm_max)
-			break;
+		/* ignore 1st ep which is for element */
+		if (mcpu_ep_n == mcpu_ep)
+			continue;
 
-		mcpu_ep_n = of_get_next_child(mcpu_port, mcpu_ep_n);
-		if (!mcpu_ep_n) {
-			ret = 0;
+		if (*nm_idx > nm_max)
 			break;
-		}
 
 		mcodec_ep_n	= of_graph_get_remote_endpoint(mcpu_ep_n);
 		mcodec_port	= ep_to_port(mcodec_ep_n);
 
-		if (mcodec_ports != port_to_ports(mcodec_port))
+		if (mcodec_ports != port_to_ports(mcodec_port)) {
+			ret = -EINVAL;
 			goto mcpu_err;
+		}
 
 		codec_idx = 0;
-		mcodec_port_i = of_get_next_child(mcodec_ports, NULL);
-		while (1) {
-			if (codec_idx > dai_link->num_codecs)
-				goto mcodec_err;
-
-			mcodec_port_i = of_get_next_child(mcodec_ports, mcodec_port_i);
+		ret = -EINVAL;
+		for_each_of_graph_port(mcodec_ports, mcodec_port_i) {
 
-			if (!mcodec_port_i)
-				goto mcodec_err;
+			/* ignore 1st port which is for pair connection */
+			if (mcodec_port_top == mcodec_port_i)
+				continue;
 
-			if (mcodec_port_i == mcodec_port)
+			if (codec_idx > dai_link->num_codecs)
 				break;
 
+			if (mcodec_port_i == mcodec_port) {
+				dai_link->ch_maps[*nm_idx].cpu	 = cpu_idx;
+				dai_link->ch_maps[*nm_idx].codec = codec_idx;
+
+				(*nm_idx)++;
+				ret = 0;
+				break;
+			}
 			codec_idx++;
 		}
-
-		dai_link->ch_maps[*nm_idx].cpu		= cpu_idx;
-		dai_link->ch_maps[*nm_idx].codec	= codec_idx;
-
-		(*nm_idx)++;
-
 		of_node_put(mcodec_port_i);
-mcodec_err:
 		of_node_put(mcodec_port);
 		of_node_put(mcpu_ep_n);
 		of_node_put(mcodec_ep_n);
+		if (ret < 0)
+			break;
 	}
 mcpu_err:
 	of_node_put(mcpu_ep);
@@ -674,7 +670,7 @@ static int graph_parse_node_single(struct simple_util_priv *priv,
 				   struct device_node *port,
 				   struct link_info *li, int is_cpu)
 {
-	struct device_node *ep = port_to_endpoint(port);
+	struct device_node *ep = of_graph_get_next_port_endpoint(port, NULL);
 	int ret = __graph_parse_node(priv, gtype, ep, li, is_cpu, 0);
 
 	of_node_put(ep);
@@ -769,7 +765,7 @@ static void graph_link_init(struct simple_util_priv *priv,
 		of_node_put(port_cpu);
 		port_cpu = ep_to_port(ep_cpu);
 	} else {
-		ep_cpu = port_to_endpoint(port_cpu);
+		ep_cpu = of_graph_get_next_port_endpoint(port_cpu, NULL);
 	}
 	ports_cpu = port_to_ports(port_cpu);
 
@@ -779,7 +775,7 @@ static void graph_link_init(struct simple_util_priv *priv,
 		of_node_put(port_cpu);
 		port_codec = ep_to_port(ep_codec);
 	} else {
-		ep_codec = port_to_endpoint(port_codec);
+		ep_codec = of_graph_get_next_port_endpoint(port_codec, NULL);
 	}
 	ports_codec = port_to_ports(port_codec);
 
@@ -850,7 +846,7 @@ int audio_graph2_link_normal(struct simple_util_priv *priv,
 			     struct link_info *li)
 {
 	struct device_node *cpu_port = lnk;
-	struct device_node *cpu_ep = port_to_endpoint(cpu_port);
+	struct device_node *cpu_ep = of_graph_get_next_port_endpoint(cpu_port, NULL);
 	struct device_node *codec_port = of_graph_get_remote_port(cpu_ep);
 	int ret;
 
@@ -883,7 +879,7 @@ int audio_graph2_link_dpcm(struct simple_util_priv *priv,
 			   struct device_node *lnk,
 			   struct link_info *li)
 {
-	struct device_node *ep = port_to_endpoint(lnk);
+	struct device_node *ep = of_graph_get_next_port_endpoint(lnk, NULL);
 	struct device_node *rep = of_graph_get_remote_endpoint(ep);
 	struct device_node *cpu_port = NULL;
 	struct device_node *codec_port = NULL;
@@ -1009,7 +1005,7 @@ int audio_graph2_link_c2c(struct simple_util_priv *priv,
 	of_node_get(lnk);
 	port0 = lnk;
 	ports = port_to_ports(port0);
-	port1 = of_get_next_child(ports, lnk);
+	port1 = of_graph_get_next_port(ports, port0);
 
 	/*
 	 * Card2 can use original Codec2Codec settings if DT has.
@@ -1039,8 +1035,8 @@ int audio_graph2_link_c2c(struct simple_util_priv *priv,
 		dai_link->num_c2c_params	= 1;
 	}
 
-	ep0 = port_to_endpoint(port0);
-	ep1 = port_to_endpoint(port1);
+	ep0 = of_graph_get_next_port_endpoint(port0, NULL);
+	ep1 = of_graph_get_next_port_endpoint(port1, NULL);
 
 	codec0_port = of_graph_get_remote_port(ep0);
 	codec1_port = of_graph_get_remote_port(ep1);
@@ -1141,21 +1137,12 @@ static int graph_counter(struct device_node *lnk)
 	 */
 	if (graph_lnk_is_multi(lnk)) {
 		struct device_node *ports = port_to_ports(lnk);
-		struct device_node *port = NULL;
-		int cnt = 0;
 
 		/*
 		 * CPU/Codec = N:M case has many endpoints.
 		 * We can't use of_graph_get_endpoint_count() here
 		 */
-		while(1) {
-			port = of_get_next_child(ports, port);
-			if (!port)
-				break;
-			cnt++;
-		}
-
-		return cnt - 1;
+		return of_graph_get_port_count(ports) - 1;
 	}
 	/*
 	 * Single CPU / Codec
@@ -1169,7 +1156,7 @@ static int graph_count_normal(struct simple_util_priv *priv,
 			      struct link_info *li)
 {
 	struct device_node *cpu_port = lnk;
-	struct device_node *cpu_ep = port_to_endpoint(cpu_port);
+	struct device_node *cpu_ep = of_graph_get_next_port_endpoint(cpu_port, NULL);
 	struct device_node *codec_port = of_graph_get_remote_port(cpu_ep);
 
 	/*
@@ -1197,7 +1184,7 @@ static int graph_count_dpcm(struct simple_util_priv *priv,
 			    struct device_node *lnk,
 			    struct link_info *li)
 {
-	struct device_node *ep = port_to_endpoint(lnk);
+	struct device_node *ep = of_graph_get_next_port_endpoint(lnk, NULL);
 	struct device_node *rport = of_graph_get_remote_port(ep);
 
 	/*
@@ -1239,9 +1226,9 @@ static int graph_count_c2c(struct simple_util_priv *priv,
 {
 	struct device_node *ports = port_to_ports(lnk);
 	struct device_node *port0 = lnk;
-	struct device_node *port1 = of_get_next_child(ports, of_node_get(lnk));
-	struct device_node *ep0 = port_to_endpoint(port0);
-	struct device_node *ep1 = port_to_endpoint(port1);
+	struct device_node *port1 = of_graph_get_next_port(ports, of_node_get(port0));
+	struct device_node *ep0 = of_graph_get_next_port_endpoint(port0, NULL);
+	struct device_node *ep1 = of_graph_get_next_port_endpoint(port1, NULL);
 	struct device_node *codec0 = of_graph_get_remote_port(ep0);
 	struct device_node *codec1 = of_graph_get_remote_port(ep1);
 
-- 
2.43.0

```

## \[PATCH 7/9\] gpu: drm: omapdrm: use new of\_graph functions

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:53 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236250.md

```text
Now we can use new port related functions for port parsing. Use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/gpu/drm/omapdrm/dss/dpi.c | 3 ++-
 drivers/gpu/drm/omapdrm/dss/sdi.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
index 030f997eccd0..b17e77f700dd 100644
--- a/drivers/gpu/drm/omapdrm/dss/dpi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
@@ -16,6 +16,7 @@
 #include <linux/export.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/string.h>
@@ -709,7 +710,7 @@ int dpi_init_port(struct dss_device *dss, struct platform_device *pdev,
 	if (!dpi)
 		return -ENOMEM;
 
-	ep = of_get_next_child(port, NULL);
+	ep = of_graph_get_next_port_endpoint(port, NULL);
 	if (!ep)
 		return 0;
 
diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
index 91eaae3b9481..f9ae358e8e52 100644
--- a/drivers/gpu/drm/omapdrm/dss/sdi.c
+++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
@@ -11,6 +11,7 @@
 #include <linux/export.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/string.h>
@@ -346,7 +347,7 @@ int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
 	if (!sdi)
 		return -ENOMEM;
 
-	ep = of_get_next_child(port, NULL);
+	ep = of_graph_get_next_port_endpoint(port, NULL);
 	if (!ep) {
 		r = 0;
 		goto err_free;
-- 
2.43.0

```

## \[PATCH 8/9\] fbdev: omapfb: use new of\_graph functions

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:58:57 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236251.md

```text
Now we can use new port related functions for port parsing. Use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/video/fbdev/omap2/omapfb/dss/dpi.c    |  3 +-
 drivers/video/fbdev/omap2/omapfb/dss/dss-of.c | 66 -------------------
 drivers/video/fbdev/omap2/omapfb/dss/dss.c    |  9 +--
 drivers/video/fbdev/omap2/omapfb/dss/sdi.c    |  3 +-
 include/video/omapfb_dss.h                    |  8 ---
 5 files changed, 9 insertions(+), 80 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dpi.c b/drivers/video/fbdev/omap2/omapfb/dss/dpi.c
index 7c1b7d89389a..395b1139a5ae 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dpi.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dpi.c
@@ -20,6 +20,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/string.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/clk.h>
 #include <linux/component.h>
 
@@ -845,7 +846,7 @@ int dpi_init_port(struct platform_device *pdev, struct device_node *port)
 	if (!dpi)
 		return -ENOMEM;
 
-	ep = omapdss_of_get_next_endpoint(port, NULL);
+	ep = of_graph_get_next_port_endpoint(port, NULL);
 	if (!ep)
 		return 0;
 
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c b/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
index 4040e247e026..efb7d2e4ce85 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
@@ -15,72 +15,6 @@
 
 #include "dss.h"
 
-struct device_node *
-omapdss_of_get_next_port(const struct device_node *parent,
-			 struct device_node *prev)
-{
-	struct device_node *port = NULL;
-
-	if (!parent)
-		return NULL;
-
-	if (!prev) {
-		struct device_node *ports;
-		/*
-		 * It's the first call, we have to find a port subnode
-		 * within this node or within an optional 'ports' node.
-		 */
-		ports = of_get_child_by_name(parent, "ports");
-		if (ports)
-			parent = ports;
-
-		port = of_get_child_by_name(parent, "port");
-
-		/* release the 'ports' node */
-		of_node_put(ports);
-	} else {
-		struct device_node *ports;
-
-		ports = of_get_parent(prev);
-		if (!ports)
-			return NULL;
-
-		do {
-			port = of_get_next_child(ports, prev);
-			if (!port) {
-				of_node_put(ports);
-				return NULL;
-			}
-			prev = port;
-		} while (!of_node_name_eq(port, "port"));
-
-		of_node_put(ports);
-	}
-
-	return port;
-}
-EXPORT_SYMBOL_GPL(omapdss_of_get_next_port);
-
-struct device_node *
-omapdss_of_get_next_endpoint(const struct device_node *parent,
-			     struct device_node *prev)
-{
-	struct device_node *ep = NULL;
-
-	if (!parent)
-		return NULL;
-
-	do {
-		ep = of_get_next_child(parent, prev);
-		if (!ep)
-			return NULL;
-		prev = ep;
-	} while (!of_node_name_eq(ep, "endpoint"));
-
-	return ep;
-}
-EXPORT_SYMBOL_GPL(omapdss_of_get_next_endpoint);
-
 struct device_node *dss_of_port_get_parent_device(struct device_node *port)
 {
 	struct device_node *np;
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
index d814e4baa4b3..5cab317011ee 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
@@ -26,6 +26,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/regulator/consumer.h>
 #include <linux/suspend.h>
 #include <linux/component.h>
@@ -922,7 +923,7 @@ static int dss_init_ports(struct platform_device *pdev)
 	if (parent == NULL)
 		return 0;
 
-	port = omapdss_of_get_next_port(parent, NULL);
+	port = of_graph_get_next_port(parent, NULL);
 	if (!port)
 		return 0;
 
@@ -953,7 +954,7 @@ static int dss_init_ports(struct platform_device *pdev)
 			break;
 		}
 	} while (!ret &&
-		 (port = omapdss_of_get_next_port(parent, port)) != NULL);
+		 (port = of_graph_get_next_port(parent, port)) != NULL);
 
 	if (ret)
 		dss_uninit_ports(pdev);
@@ -969,7 +970,7 @@ static void dss_uninit_ports(struct platform_device *pdev)
 	if (parent == NULL)
 		return;
 
-	port = omapdss_of_get_next_port(parent, NULL);
+	port = of_graph_get_next_port(parent, NULL);
 	if (!port)
 		return;
 
@@ -1000,7 +1001,7 @@ static void dss_uninit_ports(struct platform_device *pdev)
 		default:
 			break;
 		}
-	} while ((port = omapdss_of_get_next_port(parent, port)) != NULL);
+	} while ((port = of_graph_get_next_port(parent, port)) != NULL);
 }
 
 static int dss_video_pll_probe(struct platform_device *pdev)
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/sdi.c b/drivers/video/fbdev/omap2/omapfb/dss/sdi.c
index d527931b2b16..22a6243d7abf 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/sdi.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/sdi.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/string.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/component.h>
 
 #include <video/omapfb_dss.h>
@@ -405,7 +406,7 @@ int sdi_init_port(struct platform_device *pdev, struct device_node *port)
 	u32 datapairs;
 	int r;
 
-	ep = omapdss_of_get_next_endpoint(port, NULL);
+	ep = of_graph_get_next_port_endpoint(port, NULL);
 	if (!ep)
 		return 0;
 
diff --git a/include/video/omapfb_dss.h b/include/video/omapfb_dss.h
index a8c0c3eeeb5b..d133190e3143 100644
--- a/include/video/omapfb_dss.h
+++ b/include/video/omapfb_dss.h
@@ -811,14 +811,6 @@ static inline bool omapdss_device_is_enabled(struct omap_dss_device *dssdev)
 	return dssdev->state == OMAP_DSS_DISPLAY_ACTIVE;
 }
 
-struct device_node *
-omapdss_of_get_next_port(const struct device_node *parent,
-			 struct device_node *prev);
-
-struct device_node *
-omapdss_of_get_next_endpoint(const struct device_node *parent,
-			     struct device_node *prev);
-
 struct omap_dss_device *
 omapdss_of_find_source_for_first_ep(struct device_node *node);
 #else
-- 
2.43.0

```

## \[PATCH 9/9\] media: xilinx-tpg: use new of\_graph functions

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-06 04:59:01 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236252.md

```text
Now we can use new port related functions for port parsing. Use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/media/platform/xilinx/xilinx-tpg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/xilinx/xilinx-tpg.c b/drivers/media/platform/xilinx/xilinx-tpg.c
index e05e528ffc6f..a25f216b2513 100644
--- a/drivers/media/platform/xilinx/xilinx-tpg.c
+++ b/drivers/media/platform/xilinx/xilinx-tpg.c
@@ -13,6 +13,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/xilinx-v4l2-controls.h>
 
@@ -744,7 +745,7 @@ static int xtpg_parse_of(struct xtpg_device *xtpg)
 		}
 
 		if (nports == 0) {
-			endpoint = of_get_next_child(port, NULL);
+			endpoint = of_graph_get_next_port_endpoint(port, NULL);
 			if (endpoint)
 				has_endpoint = true;
 			of_node_put(endpoint);
-- 
2.43.0

```

## Re: \[PATCH 4/9\] ASoC: rcar\_snd: use new of\_graph functions

- From: Mark Brown \<broonie@kernel.org\>
- Date: 2024-08-06 14:28:28 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236263.md

```text
On Tue, Aug 06, 2024 at 04:58:38AM +0000, Kuninori Morimoto wrote:
> Now we can use new port related functions for port parsing. Use it.

Acked-by: Mark Brown <broonie@kernel.org>
```

## Re: \[PATCH 5/9\] ASoC: audio-graph-card: use new of\_graph functions

- From: Mark Brown \<broonie@kernel.org\>
- Date: 2024-08-06 14:28:59 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236264.md

```text
On Tue, Aug 06, 2024 at 04:58:42AM +0000, Kuninori Morimoto wrote:
> Now we can use new port related functions for port parsing. Use it.

Acked-by: Mark Brown <broonie@kernel.org>
```

## Re: \[PATCH 6/9\] ASoC: audio-graph-card2: use new of\_graph functions

- From: Mark Brown \<broonie@kernel.org\>
- Date: 2024-08-06 14:31:08 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236265.md

```text
On Tue, Aug 06, 2024 at 04:58:46AM +0000, Kuninori Morimoto wrote:
> Now we can use new port related functions for port parsing. Use it.

Acked-by: Mark Brown <broonie@kernel.org>
```

## Re: \[PATCH 3/9\] ASoC: test-component: use new of\_graph functions

- From: Tomi Valkeinen \<tomi.valkeinen@ideasonboard.com\>
- Date: 2024-08-08 06:39:03 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236309.md

```text
On 06/08/2024 07:58, Kuninori Morimoto wrote:
> Current test-component.c is using for_each_endpoint_of_node()
> for parsing, but it should use "port" base loop instead of "endpoint",
> because properties are "port" base, instead of "endpoint".

The subject of the patch makes this sound like it's just using new helper functions, but the description makes me think this is a bug fix.

I think it would be good to mention what are the symptoms of the bug.

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   sound/soc/generic/test-component.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/generic/test-component.c b/sound/soc/generic/test-component.c
> index e9e5e235a8a6..6f75da9f86e6 100644
> --- a/sound/soc/generic/test-component.c
> +++ b/sound/soc/generic/test-component.c
> @@ -600,7 +600,7 @@ static int test_driver_probe(struct platform_device *pdev)
>   	}
>   
>   	i = 0;
> -	for_each_endpoint_of_node(node, ep) {
> +	for_each_of_graph_port(node, ep) {

"ep" is endpoint, so the variable name should also be changed (even if it's not used).

>   		snprintf(dname[i].name, TEST_NAME_LEN, "%s.%d", node->name, i);
>   		ddriv[i].name = dname[i].name;
>   

 Tomi

```

## Re: \[PATCH 1/9\] of: property: add of\_graph\_get\_next\_port()

- From: Tomi Valkeinen \<tomi.valkeinen@ideasonboard.com\>
- Date: 2024-08-08 08:06:33 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236313.md

```text
On 06/08/2024 07:58, Kuninori Morimoto wrote:
> We have endpoint base functions
> 	- of_graph_get_next_device_endpoint()
> 	- of_graph_get_device_endpoint_count()
> 	- for_each_of_graph_device_endpoint()
>
> Here, for_each_of_graph_device_endpoint() loop finds each endpoints
>
> 	ports {
> 		port@0 {
> (1)			endpoint {...};
> 		};
> 		port@1 {
> (2)			endpoint {...};
> 		};
> 		...
> 	};
>
> In above case, it finds endpoint as (1) -> (2) -> ...
>
> Basically, user/driver knows which port is used for what, but not in
> all cases. For example on flexible/generic driver case, how many ports
> are used is not fixed.
>
> For example Sound Generic Card driver which is used from many venders
> can't know how many ports are used. Because the driver is very
> flexible/generic, it is impossible to know how many ports are used,
> it depends on each vender SoC and/or its used board.
>
> And more, the port can have multi endpoints. For example Generic Sound
> Card case, it supports many type of connection between CPU / Codec, and
> some of them uses multi endpoint in one port.
> Then, Generic Sound Card want to handle each connection via "port"
> instead of "endpoint".
> But, it is very difficult to handle each "port" via
> for_each_of_graph_device_endpoint(). Getting "port" by using
> of_get_parent() from "endpoint" doesn't work. see below.
>
> 	ports {
> 		port@0 {
> (1)			endpoint@0 {...};
> (2)			endpoint@1 {...};
> 		};
> 		port@1 {
> (3)			endpoint {...};
> 		};
> 		...
> 	};
>
> In the same time, same reason, we want to handle "ports" same as "port".
>
> 	node {
> =>		ports@0 {
> 			port@0 {
> 				endpoint@0 {...};
> 				endpoint@1 {...};
> 				...
> 			};
> 			port@1 {
> 				endpoint@0 {...};
> 				endpoint@1 {...};
> 				...
> 			};
> 			...
> 		};
> =>		ports@1 {
> 			...
> 		};
> 	};
>
> Add "ports" / "port" base functions.
> For above case, we can use
>
> 	for_each_of_graph_ports(node, ports) {
> 		for_each_of_graph_port(ports, port) {
> 			...
> 		}
> 	}
>
> This loop works in case of "node" doesn't have "ports" also.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   drivers/of/property.c    | 86 ++++++++++++++++++++++++++++++++++++++++
>   include/linux/of_graph.h | 47 ++++++++++++++++++++++
>   2 files changed, 133 insertions(+)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 164d77cb9445..3b2d09c0376a 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -625,8 +625,76 @@ struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
>   }
>   EXPORT_SYMBOL(of_graph_get_port_by_id);
>   
> +/**
> + * of_graph_get_next_ports() - get next ports node.
> + * @parent: pointer to the parent device node
> + * @ports: current ports node, or NULL to get first
> + *
> + * Return: A 'ports' node pointer with refcount incremented. Refcount
> + * of the passed @prev node is decremented.

No "prev" argument in the code.

The of_graph_get_next_endpoint() function uses "previous" as the argument name (well, the function declaration uses "previous", the implementation uses "prev"...), and I would use the same naming here.

Also of_graph_get_next_endpoint() talks about "previous endpoint node", whereas here it's "current ports node". I'd use the same style here, so "previous ports node".

The same comments for the of_graph_get_next_port().

> + */
> +struct device_node *of_graph_get_next_ports(struct device_node *parent,
> +					    struct device_node *ports)
> +{
> +	if (!parent)
> +		return NULL;
> +
> +	if (!ports) {
> +		ports = of_get_child_by_name(parent, "ports");
> +
> +		/* use parent as its ports of this device if it not exist */

I think this needs to be described in the kernel doc. I understand the need for this, but it's somewhat counter-intuitive that this returns the parent node if there are no ports nodes, so it must be highlighted in the documentation.

I wonder if a bit more complexity here would be good... I think here we could:

- If there are no 'ports' nodes in the parent, but there is a 'port' node in the parent, return the parent node
- If there are no 'ports' nor 'port' nodes in the parent, return NULL

> +		if (!ports) {
> +			ports = parent;
> +			of_node_get(ports);
> +		}

You could just do "ports = of_node_get(parent);"

> +
> +		return ports;
> +	}
> +
> +	do {
> +		ports = of_get_next_child(parent, ports);
> +		if (!ports)
> +			break;
> +	} while (!of_node_name_eq(ports, "ports"));
> +
> +	return ports;
> +}
> +EXPORT_SYMBOL(of_graph_get_next_ports);
> +
> +/**
> + * of_graph_get_next_port() - get next port node.
> + * @parent: pointer to the parent device node
> + * @port: current port node, or NULL to get first
> + *
> + * Return: A 'port' node pointer with refcount incremented. Refcount
> + * of the passed @prev node is decremented.
> + */
> +struct device_node *of_graph_get_next_port(struct device_node *parent,
> +					   struct device_node *port)
> +{
> +	if (!parent)
> +		return NULL;
> +
> +	if (!port) {
> +		struct device_node *ports __free(device_node) =
> +			of_graph_get_next_ports(parent, NULL);
> +
> +		return of_get_child_by_name(ports, "port");
> +	}
> +
> +	do {
> +		port = of_get_next_child(parent, port);
> +		if (!port)
> +			break;
> +	} while (!of_node_name_eq(port, "port"));
> +
> +	return port;
> +}

Hmm... So if I call this with of_graph_get_next_port(dev_node, NULL) (dev_node being the device node of the device), it'll give me the first port in the first ports node, or the first port in the dev_node if there are no ports nodes?

And if I then continue iterating with of_graph_get_next_port(dev_node, prev_port)... The call will return NULL if the dev_node contains "ports" node (because the dev_node does not contain any "port" nodes)?

So if I understand right, of_graph_get_next_port() must always be called with a parent that contains port nodes. Sometimes that's the device's node (if there's just one port) and sometimes that's ports node. If it's called with a parent that contains ports node, it will not work correctly.

If the above is right, then should this just return "of_get_child_by_name(parent, "port")" if !port, instead of calling of_graph_get_next_ports()?

Or maybe I'm just getting confused here. But in any case, I think it would be very good to describe the behavior on the kernel doc for the different ports/port structure cases (also for of_graph_get_next_ports()), and be clear on what the parameters can be, i.e. what kind of device nodes can be given as parent, and how the function iterates over the ports.

> +EXPORT_SYMBOL(of_graph_get_next_port);
> +
>   /**
>    * of_graph_get_next_endpoint() - get next endpoint node
> + *

Extra change.

>    * @parent: pointer to the parent device node
>    * @prev: previous endpoint node, or NULL to get first
>    *
> @@ -823,6 +891,24 @@ unsigned int of_graph_get_endpoint_count(const struct device_node *np)
>   }
>   EXPORT_SYMBOL(of_graph_get_endpoint_count);
>   
> +/**
> + * of_graph_get_port_count() - get count of port

Perhaps "get the number of port nodes".

> + * @np: pointer to the parent device node
> + *
> + * Return: count of port of this device node
> + */
> +unsigned int of_graph_get_port_count(struct device_node *np)
> +{
> +	struct device_node *port = NULL;
> +	int num = 0;
> +
> +	for_each_of_graph_port(np, port)
> +		num++;
> +
> +	return num;
> +}

I my analysis above is right, calling of_graph_get_port_count(dev_node) will return 1, if the dev_node contains "ports" node which contains one or more "port" nodes.

> +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 a4bea62bfa29..30169b50b042 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -37,14 +37,42 @@ struct of_endpoint {
>   	for (child = of_graph_get_next_endpoint(parent, NULL); child != NULL; \
>   	     child = of_graph_get_next_endpoint(parent, child))
>   
> +/**
> + * for_each_of_graph_ports - iterate over every ports in a device node
> + * @parent: parent device node containing ports
> + * @child: loop variable pointing to the current ports node
> + *
> + * When breaking out of the loop, of_node_put(child) has to be called manually.
> + */
> +#define for_each_of_graph_ports(parent, child)				\
> +	for (child = of_graph_get_next_ports(parent, NULL); child != NULL; \
> +	     child = of_graph_get_next_ports(parent, child))
> +
> +/**
> + * for_each_of_graph_port - iterate over every port in a device or ports node
> + * @parent: parent device or ports node containing 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_of_graph_port(parent, child)			\
> +	for (child = of_graph_get_next_port(parent, NULL); child != NULL; \
> +	     child = of_graph_get_next_port(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,
>   				struct of_endpoint *endpoint);
> +

Extra change.

>   unsigned int of_graph_get_endpoint_count(const struct device_node *np);
> +unsigned int of_graph_get_port_count(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_endpoint(const struct device_node *parent,
>   					struct device_node *previous);
> +struct device_node *of_graph_get_next_ports(struct device_node *parent,
> +					    struct device_node *ports);
> +struct device_node *of_graph_get_next_port(struct device_node *parent,
> +					   struct device_node *port);
>   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(
> @@ -73,6 +101,11 @@ static inline unsigned int of_graph_get_endpoint_count(const struct device_node
>   	return 0;
>   }
>   
> +static inline unsigned int of_graph_get_port_count(struct device_node *np)
> +{
> +	return 0;
> +}
> +
>   static inline struct device_node *of_graph_get_port_by_id(
>   					struct device_node *node, u32 id)
>   {
> @@ -86,6 +119,20 @@ static inline struct device_node *of_graph_get_next_endpoint(
>   	return NULL;
>   }
>   
> +static inline struct device_node *of_graph_get_next_ports(
> +					struct device_node *parent,
> +					struct device_node *previous)
> +{
> +	return NULL;
> +}
> +
> +static inline struct device_node *of_graph_get_next_port(
> +					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)
>   {

 Tomi

```

## Re: \[PATCH 2/9\] of: property: add of\_graph\_get\_next\_port\_endpoint()

- From: Tomi Valkeinen \<tomi.valkeinen@ideasonboard.com\>
- Date: 2024-08-08 08:30:49 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236314.md

```text
On 06/08/2024 07:58, Kuninori Morimoto wrote:
> We already have of_graph_get_next_endpoint(), but it is not
> intuitive to use.
>
> (X)	node {
> (Y)		ports {
> 			port@0 { endpoint { remote-endpoint = ...; };};
> (A1)			port@1 { endpoint { remote-endpoint = ...; };
> (A2)				 endpoint { remote-endpoint = ...; };};
> (B)			port@2 { endpoint { remote-endpoint = ...; };};
> 		};
> 	};
>
> For example, if I want to handle port@1's 2 endpoints (= A1, A2),
> I want to use like below
>
> 	A1 = of_graph_get_next_endpoint(port1, NULL);
> 	A2 = of_graph_get_next_endpoint(port1, A1);
>
> But 1st one will be error, because of_graph_get_next_endpoint()
> requested "parent" means "node" (X) or "ports" (Y), not "port".
> Below are OK
>
> 	/* These will be node/ports/port@0/endpoint */
> 	of_graph_get_next_endpoint(node,  NULL);
> 	of_graph_get_next_endpoint(ports, NULL);
>
> In other words, we can't handle A1/A2 directly via
> of_graph_get_next_endpoint() so far.
>
> There is another non intuitive behavior on of_graph_get_next_endpoint().
> In case of if I could get A1 pointer for some way, and if I want to
> handle port@1 things, I would like use it like below
>
> 	/*
> 	 * "ep" is now A1, and handle port1 things here,
> 	 * but we don't know how many endpoints port1 has.
> 	 *
> 	 * Because "ep" is non NULL, 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 A1 -> A2 -> NULL,
> but      it will be A1 -> A2 -> B,    because
> of_graph_get_next_endpoint() will fetch "endpoint" beyond the "port".
>
> It is not useful on generic driver like Generic Sound Card.
> It uses of_get_next_child() instead for now, but it is not intuitive.
> And it doesn't check node name (= "endpoint").
>
> To handle endpoint more intuitive, create of_graph_get_next_port_endpoint()
>
> 	of_graph_get_next_port_endpoint(port1, NULL); // A1
> 	of_graph_get_next_port_endpoint(port1, A1);   // A2
> 	of_graph_get_next_port_endpoint(port1, A2);   // NULL
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   drivers/of/property.c    | 22 ++++++++++++++++++++++
>   include/linux/of_graph.h | 20 ++++++++++++++++++++
>   2 files changed, 42 insertions(+)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 3b2d09c0376a..de56795a7362 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -692,6 +692,28 @@ struct device_node *of_graph_get_next_port(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
> + * @endpoint: current endpoint node, or NULL to get first
> + *
> + * Return: An 'endpoint' node pointer with refcount incremented. Refcount
> + * of the passed @prev node is decremented.
> + */

Same issues here too. No "prev" parameter, and I suggest using "previous", not "current", to be consistent with of_graph_get_next_endpoint(). (or alternatively, change of_graph_get_next_endpoint()).

Oh, the declaration of the function uses "prev", but the implementation "endpoint". Please make the naming same.

> +struct device_node *of_graph_get_next_port_endpoint(const struct device_node *port,
> +						    struct device_node *endpoint)
> +{
> +	do {
> +		endpoint = of_get_next_child(port, endpoint);
> +		if (!endpoint)
> +			break;
> +	} while (!of_node_name_eq(endpoint, "endpoint"));
> +
> +	return endpoint;
> +}
> +EXPORT_SYMBOL(of_graph_get_next_port_endpoint);
> +
>   /**
>    * of_graph_get_next_endpoint() - get next endpoint node
>    *
> diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> index 30169b50b042..8b4777938c5e 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -59,6 +59,17 @@ struct of_endpoint {
>   	for (child = 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 device or ports node

Hmm, shouldn't the parent be a port node?

> + * @child: loop variable pointing to the current endpoint node
> + *
> + * When breaking out of the loop, of_node_put(child) has to be called manually.
> + */
> +#define for_each_of_graph_port_endpoint(parent, child)			\
> +		for (child = 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,
> @@ -73,6 +84,8 @@ struct device_node *of_graph_get_next_ports(struct device_node *parent,
>   					    struct device_node *ports);
>   struct device_node *of_graph_get_next_port(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(
> @@ -133,6 +146,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)
>   {

 Tomi

```

## Re: \[PATCH 7/9\] gpu: drm: omapdrm: use new of\_graph functions

- From: Tomi Valkeinen \<tomi.valkeinen@ideasonboard.com\>
- Date: 2024-08-08 10:04:49 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236328.md

```text
On 06/08/2024 07:58, Kuninori Morimoto wrote:
> Now we can use new port related functions for port parsing. Use it.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   drivers/gpu/drm/omapdrm/dss/dpi.c | 3 ++-
>   drivers/gpu/drm/omapdrm/dss/sdi.c | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
> index 030f997eccd0..b17e77f700dd 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -16,6 +16,7 @@
>   #include <linux/export.h>
>   #include <linux/kernel.h>
>   #include <linux/of.h>
> +#include <linux/of_graph.h>
>   #include <linux/platform_device.h>
>   #include <linux/regulator/consumer.h>
>   #include <linux/string.h>
> @@ -709,7 +710,7 @@ int dpi_init_port(struct dss_device *dss, struct platform_device *pdev,
>   	if (!dpi)
>   		return -ENOMEM;
>   
> -	ep = of_get_next_child(port, NULL);
> +	ep = of_graph_get_next_port_endpoint(port, NULL);
>   	if (!ep)
>   		return 0;
>   
> diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
> index 91eaae3b9481..f9ae358e8e52 100644
> --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
> @@ -11,6 +11,7 @@
>   #include <linux/export.h>
>   #include <linux/kernel.h>
>   #include <linux/of.h>
> +#include <linux/of_graph.h>
>   #include <linux/platform_device.h>
>   #include <linux/regulator/consumer.h>
>   #include <linux/string.h>
> @@ -346,7 +347,7 @@ int sdi_init_port(struct dss_device *dss, struct platform_device *pdev,
>   	if (!sdi)
>   		return -ENOMEM;
>   
> -	ep = of_get_next_child(port, NULL);
> +	ep = of_graph_get_next_port_endpoint(port, NULL);
>   	if (!ep) {
>   		r = 0;
>   		goto err_free;

Reviewed-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

 Tomi

```

## Re: \[PATCH 9/9\] media: xilinx-tpg: use new of\_graph functions

- From: Tomi Valkeinen \<tomi.valkeinen@ideasonboard.com\>
- Date: 2024-08-08 10:05:43 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236329.md

```text
On 06/08/2024 07:58, Kuninori Morimoto wrote:
> Now we can use new port related functions for port parsing. Use it.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   drivers/media/platform/xilinx/xilinx-tpg.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/xilinx/xilinx-tpg.c b/drivers/media/platform/xilinx/xilinx-tpg.c
> index e05e528ffc6f..a25f216b2513 100644
> --- a/drivers/media/platform/xilinx/xilinx-tpg.c
> +++ b/drivers/media/platform/xilinx/xilinx-tpg.c
> @@ -13,6 +13,7 @@
>   #include <linux/gpio/consumer.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> +#include <linux/of_graph.h>
>   #include <linux/platform_device.h>
>   #include <linux/xilinx-v4l2-controls.h>
>   
> @@ -744,7 +745,7 @@ static int xtpg_parse_of(struct xtpg_device *xtpg)
>   		}
>   
>   		if (nports == 0) {
> -			endpoint = of_get_next_child(port, NULL);
> +			endpoint = of_graph_get_next_port_endpoint(port, NULL);
>   			if (endpoint)
>   				has_endpoint = true;
>   			of_node_put(endpoint);

Reviewed-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

 Tomi

```

## Re: \[PATCH 1/9\] of: property: add of\_graph\_get\_next\_port()

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-09 02:10:07 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236353.md

```text

Hi Tomi

Thank you for your review


> > +/**
> > + * of_graph_get_next_ports() - get next ports node.
> > + * @parent: pointer to the parent device node
> > + * @ports: current ports node, or NULL to get first
> > + *
> > + * Return: A 'ports' node pointer with refcount incremented. Refcount
> > + * of the passed @prev node is decremented.
> 
> No "prev" argument in the code.
> 
> The of_graph_get_next_endpoint() function uses "previous" as the 
> argument name (well, the function declaration uses "previous", the 
> implementation uses "prev"...), and I would use the same naming here.
> 
> Also of_graph_get_next_endpoint() talks about "previous endpoint node", 
> whereas here it's "current ports node". I'd use the same style here, so 
> "previous ports node".
> 
> The same comments for the of_graph_get_next_port().

OK, thanks. Will fix in v2

> > +struct device_node *of_graph_get_next_ports(struct device_node *parent,
> > +					    struct device_node *ports)
> > +{
> > +	if (!parent)
> > +		return NULL;
> > +
> > +	if (!ports) {
> > +		ports = of_get_child_by_name(parent, "ports");
> > +
> > +		/* use parent as its ports of this device if it not exist */
> 
> I think this needs to be described in the kernel doc. I understand the 
> need for this, but it's somewhat counter-intuitive that this returns the 
> parent node if there are no ports nodes, so it must be highlighted in 
> the documentation.
> 
> I wonder if a bit more complexity here would be good... I think here we 
> could:
> 
> - If there are no 'ports' nodes in the parent, but there is a 'port' 
> node in the parent, return the parent node
> - If there are no 'ports' nor 'port' nodes in the parent, return NULL

Thanks, but unfortunately, get_next_ports() checks only "ports" and
doesn't check whether it has "port" node or not.
So correct comment here is maybe...

	If "parent" doesn't have "ports", it returns "parent" itself as "ports"

I will add it on v2

> > +/**
> > + * of_graph_get_next_port() - get next port node.
> > + * @parent: pointer to the parent device node
> > + * @port: current port node, or NULL to get first
> > + *
> > + * Return: A 'port' node pointer with refcount incremented. Refcount
> > + * of the passed @prev node is decremented.
> > + */
> > +struct device_node *of_graph_get_next_port(struct device_node *parent,
> > +					   struct device_node *port)
> > +{
> > +	if (!parent)
> > +		return NULL;
> > +
> > +	if (!port) {
> > +		struct device_node *ports __free(device_node) =
> > +			of_graph_get_next_ports(parent, NULL);
> > +
> > +		return of_get_child_by_name(ports, "port");
> > +	}
> > +
> > +	do {
> > +		port = of_get_next_child(parent, port);
> > +		if (!port)
> > +			break;
> > +	} while (!of_node_name_eq(port, "port"));
> > +
> > +	return port;
> > +}
> 
> Hmm... So if I call this with of_graph_get_next_port(dev_node, NULL) 
> (dev_node being the device node of the device), it'll give me the first 
> port in the first ports node, or the first port in the dev_node if there 
> are no ports nodes?

Yes

> And if I then continue iterating with of_graph_get_next_port(dev_node, 
> prev_port)... The call will return NULL if the dev_node contains "ports" 
> node (because the dev_node does not contain any "port" nodes)?
>
> So if I understand right, of_graph_get_next_port() must always be called 
> with a parent that contains port nodes. Sometimes that's the device's 
> node (if there's just one port) and sometimes that's ports node. If it's 
> called with a parent that contains ports node, it will not work correctly.
>
> If the above is right, then should this just return 
> "of_get_child_by_name(parent, "port")" if !port, instead of calling 
> of_graph_get_next_ports()?

Hmm ?  Do you mean you want access to ports@1 memeber port after ports@0 ?
I have tested below in my test environment

	parent {
(X)		ports@0 {
(A)			port@0 { };
(B)			port@1 { };
		};
(Y)		ports@1 {
(C)			port@0 { };
		};
	};

In this case, if you call get_next_port() with parent,
you can get ports@0 member port.

	/* use "paramet" and use "ports@0" are same result */

	// use parent
	port = of_graph_get_next_port(parent, NULL); // (A)
	port = of_graph_get_next_port(parent, port); // (B)
	port = of_graph_get_next_port(parent, port); // NULl

	// use ports@0
	ports = of_graph_get_next_ports(parent, NULL); // (X)
	port  = of_graph_get_next_port(ports, NULL);   // (A)
	port  = of_graph_get_next_port(ports, port);   // (B)
	port  = of_graph_get_next_port(ports, port);   // NULl

If you want to get ports@1 member port, you need to use ports@1.

	// use ports@1
	ports = of_graph_get_next_ports(parent, NULL);  // (X)
	ports = of_graph_get_next_ports(parent, ports); // (Y)
	port  = of_graph_get_next_port(ports, NULL);    // (C)

I have confirmed in my test environment.
But please double check it. Is this clear for you ?

> Or maybe I'm just getting confused here. But in any case, I think it 
> would be very good to describe the behavior on the kernel doc for the 
> different ports/port structure cases (also for 
> of_graph_get_next_ports()), and be clear on what the parameters can be, 
> i.e. what kind of device nodes can be given as parent, and how the 
> function iterates over the ports.

OK, will do in v2

> > + * @np: pointer to the parent device node
> > + *
> > + * Return: count of port of this device node
> > + */
> > +unsigned int of_graph_get_port_count(struct device_node *np)
> > +{
> > +	struct device_node *port = NULL;
> > +	int num = 0;
> > +
> > +	for_each_of_graph_port(np, port)
> > +		num++;
> > +
> > +	return num;
> > +}
> 
> I my analysis above is right, calling of_graph_get_port_count(dev_node) 
> will return 1, if the dev_node contains "ports" node which contains one 
> or more "port" nodes.

In my test, it will be..

	parent {
		ports@0 {
			port@0 { };
			port@1 { };
		};
		ports@1 {
			port@0 { };
		};
	};

	of_graph_get_port_count(parent); // 2 = number of ports@0
	of_graph_get_port_count(ports0); // 2 = number of ports@0
	of_graph_get_port_count(ports1); // 1 = number of ports@1


Thank you for your help !!

Best regards
---
Kuninori Morimoto
```

## Re: \[PATCH 2/9\] of: property: add of\_graph\_get\_next\_port\_endpoint()

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-09 02:29:45 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236354.md

```text

Hi Tomi

> > +/**
> > + * 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
> > + * @endpoint: current endpoint node, or NULL to get first
> > + *
> > + * Return: An 'endpoint' node pointer with refcount incremented. Refcount
> > + * of the passed @prev node is decremented.
> > + */
> 
> Same issues here too. No "prev" parameter, and I suggest using 
> "previous", not "current", to be consistent with 
> of_graph_get_next_endpoint(). (or alternatively, change 
> of_graph_get_next_endpoint()).
> 
> Oh, the declaration of the function uses "prev", but the implementation 
> "endpoint". Please make the naming same.

Will fix in v2.
But it will use "prev" for param same as of_graph_get_next_endpoint()

> > +/**
> > + * for_each_of_graph_port_endpoint - iterate over every endpoint in a port node
> > + * @parent: parent device or ports node
> 
> Hmm, shouldn't the parent be a port node?

Thanks, will fix in v2


Thank you for your help !!

Best regards
---
Kuninori Morimoto
```

## Re: \[PATCH 1/9\] of: property: add of\_graph\_get\_next\_port()

- From: Tomi Valkeinen \<tomi.valkeinen@ideasonboard.com\>
- Date: 2024-08-09 07:29:41 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236366.md

```text
Hi,

On 09/08/2024 05:10, Kuninori Morimoto wrote:
>
> Hi Tomi
>
> Thank you for your review
>
>
>>> +/**
>>> + * of_graph_get_next_ports() - get next ports node.
>>> + * @parent: pointer to the parent device node
>>> + * @ports: current ports node, or NULL to get first
>>> + *
>>> + * Return: A 'ports' node pointer with refcount incremented. Refcount
>>> + * of the passed @prev node is decremented.
>>
>> No "prev" argument in the code.
>>
>> The of_graph_get_next_endpoint() function uses "previous" as the
>> argument name (well, the function declaration uses "previous", the
>> implementation uses "prev"...), and I would use the same naming here.
>>
>> Also of_graph_get_next_endpoint() talks about "previous endpoint node",
>> whereas here it's "current ports node". I'd use the same style here, so
>> "previous ports node".
>>
>> The same comments for the of_graph_get_next_port().
>
> OK, thanks. Will fix in v2
>
>>> +struct device_node *of_graph_get_next_ports(struct device_node *parent,
>>> +					    struct device_node *ports)
>>> +{
>>> +	if (!parent)
>>> +		return NULL;
>>> +
>>> +	if (!ports) {
>>> +		ports = of_get_child_by_name(parent, "ports");
>>> +
>>> +		/* use parent as its ports of this device if it not exist */
>>
>> I think this needs to be described in the kernel doc. I understand the
>> need for this, but it's somewhat counter-intuitive that this returns the
>> parent node if there are no ports nodes, so it must be highlighted in
>> the documentation.
>>
>> I wonder if a bit more complexity here would be good... I think here we
>> could:
>>
>> - If there are no 'ports' nodes in the parent, but there is a 'port'
>> node in the parent, return the parent node
>> - If there are no 'ports' nor 'port' nodes in the parent, return NULL
>
> Thanks, but unfortunately, get_next_ports() checks only "ports" and
> doesn't check whether it has "port" node or not.

Yes, my point was if the check should be added. My reasoning is:

If we have this, of_graph_get_next_ports() returns the ports@0, and that makes sense:

parent {
	ports@0 {
		port@0 { };
	};
};

If we have this, of_graph_get_next_ports() returns the parent, and that's a bit surprising, but I can see the need, and it just needs to be documented:

parent {
	port { };
};

But if we have this, does it make sense that of_graph_get_next_ports() returns the parent, or should it return NULL:

parent {
	/* No ports or port */
};

> So correct comment here is maybe...
>
> 	If "parent" doesn't have "ports", it returns "parent" itself as "ports"
>
> I will add it on v2
>
>>> +/**
>>> + * of_graph_get_next_port() - get next port node.
>>> + * @parent: pointer to the parent device node
>>> + * @port: current port node, or NULL to get first
>>> + *
>>> + * Return: A 'port' node pointer with refcount incremented. Refcount
>>> + * of the passed @prev node is decremented.
>>> + */
>>> +struct device_node *of_graph_get_next_port(struct device_node *parent,
>>> +					   struct device_node *port)
>>> +{
>>> +	if (!parent)
>>> +		return NULL;
>>> +
>>> +	if (!port) {
>>> +		struct device_node *ports __free(device_node) =
>>> +			of_graph_get_next_ports(parent, NULL);
>>> +
>>> +		return of_get_child_by_name(ports, "port");
>>> +	}
>>> +
>>> +	do {
>>> +		port = of_get_next_child(parent, port);
>>> +		if (!port)
>>> +			break;
>>> +	} while (!of_node_name_eq(port, "port"));
>>> +
>>> +	return port;
>>> +}
>>
>> Hmm... So if I call this with of_graph_get_next_port(dev_node, NULL)
>> (dev_node being the device node of the device), it'll give me the first
>> port in the first ports node, or the first port in the dev_node if there
>> are no ports nodes?
>
> Yes
>
>> And if I then continue iterating with of_graph_get_next_port(dev_node,
>> prev_port)... The call will return NULL if the dev_node contains "ports"
>> node (because the dev_node does not contain any "port" nodes)?
>>
>> So if I understand right, of_graph_get_next_port() must always be called
>> with a parent that contains port nodes. Sometimes that's the device's
>> node (if there's just one port) and sometimes that's ports node. If it's
>> called with a parent that contains ports node, it will not work correctly.
>>
>> If the above is right, then should this just return
>> "of_get_child_by_name(parent, "port")" if !port, instead of calling
>> of_graph_get_next_ports()?
>
> Hmm ?  Do you mean you want access to ports@1 memeber port after ports@0 ?
> I have tested below in my test environment
>
> 	parent {
> (X)		ports@0 {
> (A)			port@0 { };
> (B)			port@1 { };
> 		};
> (Y)		ports@1 {
> (C)			port@0 { };
> 		};
> 	};
>
> In this case, if you call get_next_port() with parent,
> you can get ports@0 member port.
>
> 	/* use "paramet" and use "ports@0" are same result */
>
> 	// use parent
> 	port = of_graph_get_next_port(parent, NULL); // (A)
> 	port = of_graph_get_next_port(parent, port); // (B)
> 	port = of_graph_get_next_port(parent, port); // NULl
>
> 	// use ports@0
> 	ports = of_graph_get_next_ports(parent, NULL); // (X)
> 	port  = of_graph_get_next_port(ports, NULL);   // (A)
> 	port  = of_graph_get_next_port(ports, port);   // (B)
> 	port  = of_graph_get_next_port(ports, port);   // NULl
>
> If you want to get ports@1 member port, you need to use ports@1.
>
> 	// use ports@1
> 	ports = of_graph_get_next_ports(parent, NULL);  // (X)
> 	ports = of_graph_get_next_ports(parent, ports); // (Y)
> 	port  = of_graph_get_next_port(ports, NULL);    // (C)
>
> I have confirmed in my test environment.
> But please double check it. Is this clear for you ?

Ah, I see now. I was expecting of_get_next_child() to return children of 'parent', but that's actually not the case.

So when you call:

	port = of_graph_get_next_port(parent, NULL); // (A)

the function will call 'ports = of_graph_get_next_ports(parent, NULL)' (ports is (X)) and then return of_get_child_by_name(ports, "port") (which is (A)). This is fine.

	port = of_graph_get_next_port(parent, port); // (B)

Here the function will call 'port = of_get_next_child(parent, port)', where parent is the parent node, and port is (A). The problem is, (A) is not a child of the parent. This seems to work, as of_get_next_child() does not use the 'parent' for anything if 'port' is given, instead if just gives the next sibling of 'port'.

	port = of_graph_get_next_port(parent, port); // NULl

And now when the function calls of_get_next_child(parent, port), it does not give the next child of parent, but instead NULL because 'port' has no more siblings.

The documentation for of_get_next_child() doesn't mention if calling of_get_next_child(parent, child) is valid when the given child is actually not a child of the parent. The doc says that 'prev' parameter should be "previous child of the parent node", which is not the case here. So using it like this does not sound right to me.

And just looking at the behavior of:

> 	port = of_graph_get_next_port(parent, NULL); // (A)
> 	port = of_graph_get_next_port(parent, port); // (B)
> 	port = of_graph_get_next_port(parent, port); // NULl

it does not feel right. Why does of_graph_get_next_port() return only the ports of ports@0? I think it should either return nothing, as there are no 'port' nodes in the parent, or it should return all the port nodes from all the ports nodes.

Can we just drop the use of of_graph_get_next_ports() from of_graph_get_next_port()? In other words, make of_graph_get_next_port() iterate the 'port' nodes strictly only in the given parent node.

I think we have the same problem in of_graph_get_next_ports(). If we have:

parent {
	port { };
};

And we do:

ports = of_graph_get_next_ports(parent, NULL)

The returned 'ports' is actually the 'parent'. If we then call:

ports = of_graph_get_next_ports(parent, ports)

we are effectively calling of_graph_get_next_ports(parent, parent). This results in of_get_next_child(parent, parent). of_get_next_child() will return the next sibling of parent (so, perhaps, a node for some unrelated device). It then checks if the name of that node is 'ports'. So, while unlikely, if we have:

bus {
	/* our display device */
	display {
		port { };
	};

	/* some odd ports device */
	ports {
	};
};

and you use of_graph_get_next_ports() for display, you'll end up getting the 'ports' node.

I have to say, I feel that making the 'ports' node optional in the graph DT bindings was a mistake, but nothing we can do about that...

Can you try adding "WARN_ON(node && prev && node != prev->parent)" to of_get_next_child()?

>> Or maybe I'm just getting confused here. But in any case, I think it
>> would be very good to describe the behavior on the kernel doc for the
>> different ports/port structure cases (also for
>> of_graph_get_next_ports()), and be clear on what the parameters can be,
>> i.e. what kind of device nodes can be given as parent, and how the
>> function iterates over the ports.
>
> OK, will do in v2
>
>>> + * @np: pointer to the parent device node
>>> + *
>>> + * Return: count of port of this device node
>>> + */
>>> +unsigned int of_graph_get_port_count(struct device_node *np)
>>> +{
>>> +	struct device_node *port = NULL;
>>> +	int num = 0;
>>> +
>>> +	for_each_of_graph_port(np, port)
>>> +		num++;
>>> +
>>> +	return num;
>>> +}
>>
>> I my analysis above is right, calling of_graph_get_port_count(dev_node)
>> will return 1, if the dev_node contains "ports" node which contains one
>> or more "port" nodes.
>
> In my test, it will be..
>
> 	parent {
> 		ports@0 {
> 			port@0 { };
> 			port@1 { };
> 		};
> 		ports@1 {
> 			port@0 { };
> 		};
> 	};
>
> 	of_graph_get_port_count(parent); // 2 = number of ports@0

I think the above is a bit surprising, and in my opinion points that there is a problem. Why does using 'parent' equate to only using 'ports@0'? Again, I would expect either to get 0 (as there are no 'port' nodes in parent, or 3.

> 	of_graph_get_port_count(ports0); // 2 = number of ports@0
> 	of_graph_get_port_count(ports1); // 1 = number of ports@1
>
>
> Thank you for your help !!
>
> Best regards
> ---
> Kuninori Morimoto

 Tomi

```

## Re: \[PATCH 1/9\] of: property: add of\_graph\_get\_next\_port()

- From: Kuninori Morimoto \<kuninori.morimoto.gx@renesas.com\>
- Date: 2024-08-20 04:34:05 
- Message: https://ratatoskr.run/linux-fbdev/2024/08/8236631.md

```text

Hi Tomi

Thank you for the feedback, and sorry for my late responce.
I was under Summer Vacation

> If we have this, of_graph_get_next_ports() returns the ports@0, and that 
> makes sense:
> 
> parent {
> 	ports@0 {
> 		port@0 { };
> 	};
> };
> 
> If we have this, of_graph_get_next_ports() returns the parent, and 
> that's a bit surprising, but I can see the need, and it just needs to be 
> documented:
> 
> parent {
> 	port { };
> };

Thank you for understanding the situation

> But if we have this, does it make sense that of_graph_get_next_ports() 
> returns the parent, or should it return NULL:
> 
> parent {
> 	/* No ports or port */
> };

Yeah, it should return NULL in this case.

>  > 	port = of_graph_get_next_port(parent, NULL); // (A)
>  > 	port = of_graph_get_next_port(parent, port); // (B)
>  > 	port = of_graph_get_next_port(parent, port); // NULl
> 
> it does not feel right. Why does of_graph_get_next_port() return only 
> the ports of ports@0? I think it should either return nothing, as there 
> are no 'port' nodes in the parent, or it should return all the port 
> nodes from all the ports nodes.

As you also mentioned, having "ports" node is optional, and maybe this is
the reason you feel uncomfortable on current code, and I can agree about
it.

If the driver needs to care about multi "ports", it is obvious that the
driver cares "ports" node.

My opinion is because having "ports" is optional, we want to handle
"port" with/without "ports" by same function, especially if it doesn't
need to care about multi "ports".

I think your opinion (= "port" nodes strictly only in the given parent
node) means driver need to care both with/without "ports" node, but the
code will be pointlessly complex if it doesn't need to care multi "ports".

> bus {
> 	/* our display device */
> 	display {
> 		port { };
> 	};
> 
> 	/* some odd ports device */
> 	ports {
> 	};
> };
> 
> and you use of_graph_get_next_ports() for display, you'll end up getting 
> the 'ports' node.

This is interesting sample, but feels a little forced.
If you need to handle display::port, you call

	port = of_graph_get_next_port(display, NULL);

Indeed we will get "ports" node if it calls of_graph_get_next_ports(),
but it needs to use unrelated parameters, I think ?

	ports = of_graph_get_next_ports(bus, display);

> > 	parent {
> > 		ports@0 {
> > 			port@0 { };
> > 			port@1 { };
> > 		};
> > 		ports@1 {
> > 			port@0 { };
> > 		};
> > 	};
> > 
> > 	of_graph_get_port_count(parent); // 2 = number of ports@0
> 
> I think the above is a bit surprising, and in my opinion points that 
> there is a problem. Why does using 'parent' equate to only using 
> 'ports@0'? Again, I would expect either to get 0 (as there are no 'port' 
> nodes in parent, or 3.

This feature is for the driver which *doesn't* need to care about "ports".

	/* CASE1 */
	parent {
		port {...};
	};

	/* CASE2 */
	parent {
		ports {
			port {...};
		};
	};

both case (CASE1/2), we can use

	of_graph_get_port_count(parent);

If the driver need to care multi ports, it should use such code, IMO.

	nr = 0;
	for_each_of_graph_ports(parent, ports) {
		nr += of_graph_get_port_count(ports);

But of course we can handle it in v2 patch.

	/* of_graph_get_port_count() cares multi ports inside */
	of_graph_get_port_count(parent);

Thank you for your help !!

Best regards
---
Kuninori Morimoto
```