[PATCH v6 0/3] Add of-graph helpers to loop over endpoints and find ports by id

[PATCH v6 0/3] Add of-graph helpers to loop over endpoints and find ports by id

[Philipp Zabel]

Hi,

next try for v3.20. After the merge window we have a few new users of
of_graph_get_next_endpoint, could I please get some acks from the respective
maintainers for this to go in through Grant's tree?

This series converts all existing users of of_graph_get_next_endpoint that pass
a non-NULL prev argument to the function and decrement its refcount themselves
to stop doing that. The of_node_put is moved into of_graph_get_next_endpoint
instead.
This allows to add a for_each_endpoint_of_node helper macro to loop over all
endpoints in a device tree node.

Changes since v5:
 - Rebased onto v3.19-rc1
 - Added new users of of_graph_get_next_endpoint, coresight and
   drm/rcar-du. There's also rockchip, but the driver already doesn't
   decrement the prev node's reference count.
 - Dropped the "use for_each_endpoint_of_node macro" patches,
   I'll add the new users and resend them separately.

The previous version can be found here: https://lkml.org/lkml/2014/9/29/529

regards
Philipp

Philipp Zabel (3):
  of: Decrement refcount of previous endpoint in
    of_graph_get_next_endpoint
  of: Add for_each_endpoint_of_node helper macro
  of: Add of_graph_get_port_by_id function

 drivers/coresight/of_coresight.c               | 13 ++--------
 drivers/gpu/drm/imx/imx-drm-core.c             | 13 ++--------
 drivers/gpu/drm/rcar-du/rcar_du_kms.c          | 15 +++--------
 drivers/media/platform/soc_camera/soc_camera.c |  3 ++-
 drivers/of/base.c                              | 35 ++++++++++++++++++++------
 include/linux/of_graph.h                       | 18 +++++++++++++
 6 files changed, 55 insertions(+), 42 deletions(-)

-- 
2.1.4

[PATCH v6 1/3] of: Decrement refcount of previous endpoint in of_graph_get_next_endpoint

[Philipp Zabel]

Decrementing the reference count of the previous endpoint node allows to
use the of_graph_get_next_endpoint function in a for_each_... style macro.
All current users of this function that pass a non-NULL prev parameter
(that is, soc_camera and imx-drm) are changed to not decrement the passed
prev argument's refcount themselves.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
Changes since v5:
 - Rebased onto v3.19-rc1
 - Added coresight and rcar-du
---
 drivers/coresight/of_coresight.c               | 13 ++-----------
 drivers/gpu/drm/imx/imx-drm-core.c             | 13 ++-----------
 drivers/gpu/drm/rcar-du/rcar_du_kms.c          | 15 ++++-----------
 drivers/media/platform/soc_camera/soc_camera.c |  3 ++-
 drivers/of/base.c                              |  9 +--------
 5 files changed, 11 insertions(+), 42 deletions(-)

diff --git a/drivers/coresight/of_coresight.c b/drivers/coresight/of_coresight.c
index 5030c07..349c88b 100644
--- a/drivers/coresight/of_coresight.c
+++ b/drivers/coresight/of_coresight.c
@@ -52,15 +52,6 @@ of_coresight_get_endpoint_device(struct device_node *endpoint)
 			       endpoint, of_dev_node_match);
 }
 
-static struct device_node *of_get_coresight_endpoint(
-		const struct device_node *parent, struct device_node *prev)
-{
-	struct device_node *node = of_graph_get_next_endpoint(parent, prev);
-
-	of_node_put(prev);
-	return node;
-}
-
 static void of_coresight_get_ports(struct device_node *node,
 				   int *nr_inport, int *nr_outport)
 {
@@ -68,7 +59,7 @@ static void of_coresight_get_ports(struct device_node *node,
 	int in = 0, out = 0;
 
 	do {
-		ep = of_get_coresight_endpoint(node, ep);
+		ep = of_graph_get_next_endpoint(node, ep);
 		if (!ep)
 			break;
 
@@ -140,7 +131,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
 		/* Iterate through each port to discover topology */
 		do {
 			/* Get a handle on a port */
-			ep = of_get_coresight_endpoint(node, ep);
+			ep = of_graph_get_next_endpoint(node, ep);
 			if (!ep)
 				break;
 
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index b250130..fed627d 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -436,15 +436,6 @@ static uint32_t imx_drm_find_crtc_mask(struct imx_drm_device *imxdrm,
 	return 0;
 }
 
-static struct device_node *imx_drm_of_get_next_endpoint(
-		const struct device_node *parent, struct device_node *prev)
-{
-	struct device_node *node = of_graph_get_next_endpoint(parent, prev);
-
-	of_node_put(prev);
-	return node;
-}
-
 int imx_drm_encoder_parse_of(struct drm_device *drm,
 	struct drm_encoder *encoder, struct device_node *np)
 {
@@ -456,7 +447,7 @@ int imx_drm_encoder_parse_of(struct drm_device *drm,
 	for (i = 0; ; i++) {
 		u32 mask;
 
-		ep = imx_drm_of_get_next_endpoint(np, ep);
+		ep = of_graph_get_next_endpoint(np, ep);
 		if (!ep)
 			break;
 
@@ -504,7 +495,7 @@ int imx_drm_encoder_get_mux_id(struct device_node *node,
 		return -EINVAL;
 
 	do {
-		ep = imx_drm_of_get_next_endpoint(node, ep);
+		ep = of_graph_get_next_endpoint(node, ep);
 		if (!ep)
 			break;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 0c5ee61..480c4d9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -206,7 +206,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu,
 	enum rcar_du_encoder_type enc_type = RCAR_DU_ENCODER_NONE;
 	struct device_node *connector = NULL;
 	struct device_node *encoder = NULL;
-	struct device_node *prev = NULL;
+	struct device_node *ep_node = NULL;
 	struct device_node *entity_ep_node;
 	struct device_node *entity;
 	int ret;
@@ -225,11 +225,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu,
 	entity_ep_node = of_parse_phandle(ep->local_node, "remote-endpoint", 0);
 
 	while (1) {
-		struct device_node *ep_node;
-
-		ep_node = of_graph_get_next_endpoint(entity, prev);
-		of_node_put(prev);
-		prev = ep_node;
+		ep_node = of_graph_get_next_endpoint(entity, ep_node);
 
 		if (!ep_node)
 			break;
@@ -300,7 +296,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu,
 static int rcar_du_encoders_init(struct rcar_du_device *rcdu)
 {
 	struct device_node *np = rcdu->dev->of_node;
-	struct device_node *prev = NULL;
+	struct device_node *ep_node = NULL;
 	unsigned int num_encoders = 0;
 
 	/*
@@ -308,15 +304,12 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu)
 	 * pipeline.
 	 */
 	while (1) {
-		struct device_node *ep_node;
 		enum rcar_du_output output;
 		struct of_endpoint ep;
 		unsigned int i;
 		int ret;
 
-		ep_node = of_graph_get_next_endpoint(np, prev);
-		of_node_put(prev);
-		prev = ep_node;
+		ep_node = of_graph_get_next_endpoint(np, ep_node);
 
 		if (ep_node == NULL)
 			break;
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index b3db51c..289b637 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1710,7 +1710,6 @@ static void scan_of_host(struct soc_camera_host *ici)
 		if (!i)
 			soc_of_bind(ici, epn, ren->parent);
 
-		of_node_put(epn);
 		of_node_put(ren);
 
 		if (i) {
@@ -1718,6 +1717,8 @@ static void scan_of_host(struct soc_camera_host *ici)
 			break;
 		}
 	}
+
+	of_node_put(epn);
 }
 
 #else
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 36536b6..aac66df 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2085,8 +2085,7 @@ EXPORT_SYMBOL(of_graph_parse_endpoint);
  * @prev: previous endpoint node, or NULL to get first
  *
  * Return: An 'endpoint' node pointer with refcount incremented. Refcount
- * of the passed @prev node is not decremented, the caller have to use
- * of_node_put() on it when done.
+ * of the passed @prev node is decremented.
  */
 struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 					struct device_node *prev)
@@ -2122,12 +2121,6 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 		if (WARN_ONCE(!port, "%s(): endpoint %s has no parent node\n",
 			      __func__, prev->full_name))
 			return NULL;
-
-		/*
-		 * Avoid dropping prev node refcount to 0 when getting the next
-		 * child below.
-		 */
-		of_node_get(prev);
 	}
 
 	while (1) {
-- 
2.1.4

Re: [PATCH v6 1/3] of: Decrement refcount of previous endpoint in of_graph_get_next_endpoint

[Mathieu Poirier]

On 22 December 2014 at 08:11, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Decrementing the reference count of the previous endpoint node allows to
> use the of_graph_get_next_endpoint function in a for_each_... style macro.
> All current users of this function that pass a non-NULL prev parameter
> (that is, soc_camera and imx-drm) are changed to not decrement the passed
> prev argument's refcount themselves.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
> Changes since v5:
>  - Rebased onto v3.19-rc1
>  - Added coresight and rcar-du
> ---
>  drivers/coresight/of_coresight.c               | 13 ++-----------
>  drivers/gpu/drm/imx/imx-drm-core.c             | 13 ++-----------
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c          | 15 ++++-----------
>  drivers/media/platform/soc_camera/soc_camera.c |  3 ++-
>  drivers/of/base.c                              |  9 +--------
>  5 files changed, 11 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/coresight/of_coresight.c b/drivers/coresight/of_coresight.c
> index 5030c07..349c88b 100644
> --- a/drivers/coresight/of_coresight.c
> +++ b/drivers/coresight/of_coresight.c
> @@ -52,15 +52,6 @@ of_coresight_get_endpoint_device(struct device_node *endpoint)
>                                endpoint, of_dev_node_match);
>  }
>
> -static struct device_node *of_get_coresight_endpoint(
> -               const struct device_node *parent, struct device_node *prev)
> -{
> -       struct device_node *node = of_graph_get_next_endpoint(parent, prev);
> -
> -       of_node_put(prev);
> -       return node;
> -}
> -
>  static void of_coresight_get_ports(struct device_node *node,
>                                    int *nr_inport, int *nr_outport)
>  {
> @@ -68,7 +59,7 @@ static void of_coresight_get_ports(struct device_node *node,
>         int in = 0, out = 0;
>
>         do {
> -               ep = of_get_coresight_endpoint(node, ep);
> +               ep = of_graph_get_next_endpoint(node, ep);
>                 if (!ep)
>                         break;
>
> @@ -140,7 +131,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
>                 /* Iterate through each port to discover topology */
>                 do {
>                         /* Get a handle on a port */
> -                       ep = of_get_coresight_endpoint(node, ep);
> +                       ep = of_graph_get_next_endpoint(node, ep);
>                         if (!ep)
>                                 break;
>

I tested this in my tree - ack.

Re: [PATCH v6 1/3] of: Decrement refcount of previous endpoint in of_graph_get_next_endpoint

[Philipp Zabel]

On Mon, Dec 22, 2014 at 02:09:46PM -0700, Mathieu Poirier wrote:
> On 22 December 2014 at 08:11, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Decrementing the reference count of the previous endpoint node allows to
> > use the of_graph_get_next_endpoint function in a for_each_... style macro.
> > All current users of this function that pass a non-NULL prev parameter
> > (that is, soc_camera and imx-drm) are changed to not decrement the passed
> > prev argument's refcount themselves.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
[...]
> I tested this in my tree - ack.

Thanks!

regards
Philipp

[PATCH v6 2/3] of: Add for_each_endpoint_of_node helper macro

[Philipp Zabel]

Note that while of_graph_get_next_endpoint decrements the reference count
of the child node passed to it, of_node_put(child) still has to be called
manually when breaking out of the loop.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/linux/of_graph.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index befef42..e43442e 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_endpoint_of_node - iterate over every endpoint in a device node
+ * @parent: parent device node containing ports and endpoints
+ * @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_endpoint_of_node(parent, child) \
+	for (child = of_graph_get_next_endpoint(parent, NULL); child != NULL; \
+	     child = of_graph_get_next_endpoint(parent, child))
+
 #ifdef CONFIG_OF
 int of_graph_parse_endpoint(const struct device_node *node,
 				struct of_endpoint *endpoint);
-- 
2.1.4

[PATCH v6 3/3] of: Add of_graph_get_port_by_id function

[Philipp Zabel]

This patch adds a function to get a port device tree node by port id,
or reg property value.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/of/base.c        | 26 ++++++++++++++++++++++++++
 include/linux/of_graph.h |  7 +++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index aac66df..c816299 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2080,6 +2080,32 @@ int of_graph_parse_endpoint(const struct device_node *node,
 EXPORT_SYMBOL(of_graph_parse_endpoint);
 
 /**
+ * of_graph_get_port_by_id() - get the port matching a given id
+ * @parent: pointer to the parent device node
+ * @id: id of the port
+ *
+ * Return: A 'port' node pointer with refcount incremented. The caller
+ * has to use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id)
+{
+	struct device_node *port;
+
+	for_each_child_of_node(node, port) {
+		u32 port_id = 0;
+
+		if (of_node_cmp(port->name, "port") != 0)
+			continue;
+		of_property_read_u32(port, "reg", &port_id);
+		if (id == port_id)
+			return port;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL(of_graph_get_port_by_id);
+
+/**
  * 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
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index e43442e..3c1c95a 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -40,6 +40,7 @@ struct of_endpoint {
 #ifdef CONFIG_OF
 int of_graph_parse_endpoint(const struct device_node *node,
 				struct of_endpoint *endpoint);
+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_remote_port_parent(
@@ -53,6 +54,12 @@ static inline int of_graph_parse_endpoint(const struct device_node *node,
 	return -ENOSYS;
 }
 
+static inline struct device_node *of_graph_get_port_by_id(
+					struct device_node *node, u32 id)
+{
+	return NULL;
+}
+
 static inline struct device_node *of_graph_get_next_endpoint(
 					const struct device_node *parent,
 					struct device_node *previous)
-- 
2.1.4

Re: [PATCH v6 3/3] of: Add of_graph_get_port_by_id function

[Andrzej Hajda]

Hi Philipp,

On 12/22/2014 04:11 PM, Philipp Zabel wrote:
> This patch adds a function to get a port device tree node by port id,
> or reg property value.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/of/base.c        | 26 ++++++++++++++++++++++++++
>  include/linux/of_graph.h |  7 +++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index aac66df..c816299 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2080,6 +2080,32 @@ int of_graph_parse_endpoint(const struct device_node *node,
>  EXPORT_SYMBOL(of_graph_parse_endpoint);
>  
>  /**
> + * of_graph_get_port_by_id() - get the port matching a given id
> + * @parent: pointer to the parent device node

Here you have 'parent' and 'node' in the code.

> + * @id: id of the port
> + *
> + * Return: A 'port' node pointer with refcount incremented. The caller
> + * has to use of_node_put() on it when done.
> + */
> +struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id)
> +{
> +	struct device_node *port;
> +
> +	for_each_child_of_node(node, port) {
> +		u32 port_id = 0;
> +
> +		if (of_node_cmp(port->name, "port") != 0)
> +			continue;
> +		of_property_read_u32(port, "reg", &port_id);
> +		if (id == port_id)
> +			return port;
> +	}
> +
> +	return NULL;
> +}


Maybe I miss something but it does not handle optional 'ports' node.

Regards
Andrzej


> +EXPORT_SYMBOL(of_graph_get_port_by_id);
> +
> +/**
>   * 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
> diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> index e43442e..3c1c95a 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -40,6 +40,7 @@ struct of_endpoint {
>  #ifdef CONFIG_OF
>  int of_graph_parse_endpoint(const struct device_node *node,
>  				struct of_endpoint *endpoint);
> +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_remote_port_parent(
> @@ -53,6 +54,12 @@ static inline int of_graph_parse_endpoint(const struct device_node *node,
>  	return -ENOSYS;
>  }
>  
> +static inline struct device_node *of_graph_get_port_by_id(
> +					struct device_node *node, u32 id)
> +{
> +	return NULL;
> +}
> +
>  static inline struct device_node *of_graph_get_next_endpoint(
>  					const struct device_node *parent,
>  					struct device_node *previous)
> 

Re: [PATCH v6 3/3] of: Add of_graph_get_port_by_id function

[Philipp Zabel]

Hi Andrzej,

On Tue, Dec 23, 2014 at 12:10:00PM +0100, Andrzej Hajda wrote:
[...]
> >  /**
> > + * of_graph_get_port_by_id() - get the port matching a given id
> > + * @parent: pointer to the parent device node
> 
> Here you have 'parent' and 'node' in the code.
[...]
> Maybe I miss something but it does not handle optional 'ports' node.

You missed nothing, thank you for the comments! I'll fix both issues
like this:

struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
{
	struct device_node *node, *port;

	node = of_get_child_by_name(parent, "ports");
	if (node)
		parent = node;

	for_each_child_of_node(parent, port) {
		u32 port_id = 0;

		if (of_node_cmp(port->name, "port") != 0)
			continue;
		of_property_read_u32(port, "reg", &port_id);
		if (id == port_id)
			break;
	}

	of_node_put(node);

	return port;
}

regards
Philipp