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

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

[Philipp Zabel]

Hi,

Since there now is a merge conflict in imx-drm-core, I've rebased the series
onto v4.0-rc1. Also a new driver touched by this change appeared, so the first
patch now includes a fix for am437x-vfpe, too. I'd be happy to get an ack for
that.

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 v8:
 - Rebased onto v4.0-rc1
 - Added fix to am437x-vpfe

The previous version can be found here: https://lkml.org/lkml/2014/12/23/219

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                | 11 +-----
 drivers/gpu/drm/rcar-du/rcar_du_kms.c             | 15 +++------
 drivers/media/platform/am437x/am437x-vpfe.c       |  1 -
 drivers/media/platform/soc_camera/soc_camera.c    |  3 +-
 drivers/of/base.c                                 | 41 ++++++++++++++++++-----
 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c |  7 +---
 include/linux/of_graph.h                          | 18 ++++++++++
 8 files changed, 61 insertions(+), 48 deletions(-)

-- 
2.1.4

[PATCH v8 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>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
Changes since v7:
 - Rebased onto v4.0-rc1
 - Added fix for am437x-vpfe
---
 drivers/coresight/of_coresight.c                  | 13 ++-----------
 drivers/gpu/drm/imx/imx-drm-core.c                | 11 +----------
 drivers/gpu/drm/rcar-du/rcar_du_kms.c             | 15 ++++-----------
 drivers/media/platform/am437x/am437x-vpfe.c       |  1 -
 drivers/media/platform/soc_camera/soc_camera.c    |  3 ++-
 drivers/of/base.c                                 |  9 +--------
 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c |  7 +------
 7 files changed, 11 insertions(+), 48 deletions(-)

diff --git a/drivers/coresight/of_coresight.c b/drivers/coresight/of_coresight.c
index c3efa41..6f75e9d 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 a002f53..84cf99f 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -431,15 +431,6 @@ int imx_drm_encoder_parse_of(struct drm_device *drm,
 }
 EXPORT_SYMBOL_GPL(imx_drm_encoder_parse_of);
 
-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;
-}
-
 /*
  * @node: device tree node containing encoder input ports
  * @encoder: drm_encoder
@@ -457,7 +448,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 cc9136e..68dab26 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/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index 56a5cb0..0d07fca 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2504,7 +2504,6 @@ vpfe_get_pdata(struct platform_device *pdev)
 					     GFP_KERNEL);
 		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
 		pdata->asd[i]->match.of.node = rem;
-		of_node_put(endpoint);
 		of_node_put(rem);
 	}
 
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index cee7b56..f2a3d96 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1694,7 +1694,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) {
@@ -1702,6 +1701,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 0a8aeb8..05b20f1 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2086,8 +2086,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)
@@ -2123,12 +2122,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) {
diff --git a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
index 42b87f9..8b6f6d5 100644
--- a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
+++ b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
@@ -164,20 +164,15 @@ static void __init omapdss_walk_device(struct device_node *node, bool root)
 
 		pn = of_graph_get_remote_port_parent(n);
 
-		if (!pn) {
-			of_node_put(n);
+		if (!pn)
 			continue;
-		}
 
 		if (!of_device_is_available(pn) || omapdss_list_contains(pn)) {
 			of_node_put(pn);
-			of_node_put(n);
 			continue;
 		}
 
 		omapdss_walk_device(pn, false);
-
-		of_node_put(n);
 	}
 }
 
-- 
2.1.4

[PATCH v8 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 v8 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        | 32 ++++++++++++++++++++++++++++++++
 include/linux/of_graph.h |  7 +++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 05b20f1..6398b9c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2081,6 +2081,38 @@ 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 *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;
+}
+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 v8 1/3] of: Decrement refcount of previous endpoint in of_graph_get_next_endpoint

[Laurent Pinchart]

Hi Philipp,

Thank you for the patch.

Benoit, please see below for a possible issue in the am437x-vpfe driver.

On Monday 23 February 2015 11:54:04 Philipp Zabel 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>
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> Changes since v7:
>  - Rebased onto v4.0-rc1
>  - Added fix for am437x-vpfe
> ---
>  drivers/coresight/of_coresight.c                  | 13 ++-----------
>  drivers/gpu/drm/imx/imx-drm-core.c                | 11 +----------
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c             | 15 ++++-----------
>  drivers/media/platform/am437x/am437x-vpfe.c       |  1 -
>  drivers/media/platform/soc_camera/soc_camera.c    |  3 ++-
>  drivers/of/base.c                                 |  9 +--------
>  drivers/video/fbdev/omap2/dss/omapdss-boot-init.c |  7 +------
>  7 files changed, 11 insertions(+), 48 deletions(-)

[snip]

> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c
> b/drivers/media/platform/am437x/am437x-vpfe.c index 56a5cb0..0d07fca 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2504,7 +2504,6 @@ vpfe_get_pdata(struct platform_device *pdev)
>  					     GFP_KERNEL);
>  		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
>  		pdata->asd[i]->match.of.node = rem;
> -		of_node_put(endpoint);
>  		of_node_put(rem);
>  	}

For the am47x-vpfe driver,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Benoit, there seems to be a refcount issue with rem. The node pointer is 
assigned to pdata->asd[i]->match.of.node, which should require a reference, 
but you then call of_node_put(rem), releasing the only reference held. Isn't 
that a problem ?

Furthermore, on the next iteration, if an error occurs the goto done will 
result in of_node_put(rem) being called again, releasing a reference that you 
don't hold. I've sent a patch to fix that.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Philipp and all,

This series has been around for a long time, it seems to be time to get it 
merged.

What's the plan ? If we wait for the v4.1 merge window there's a chance we'll 
get more conflicts, especially on patch 1/3. Could it be merged in v4.0-rc to 
avoid that ?

On Monday 23 February 2015 11:54:03 Philipp Zabel wrote:
> Hi,
> 
> Since there now is a merge conflict in imx-drm-core, I've rebased the series
> onto v4.0-rc1. Also a new driver touched by this change appeared, so the
> first patch now includes a fix for am437x-vfpe, too. I'd be happy to get an
> ack for that.
> 
> 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 v8:
>  - Rebased onto v4.0-rc1
>  - Added fix to am437x-vpfe
> 
> The previous version can be found here: https://lkml.org/lkml/2014/12/23/219
> 
> 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                | 11 +-----
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c             | 15 +++------
>  drivers/media/platform/am437x/am437x-vpfe.c       |  1 -
>  drivers/media/platform/soc_camera/soc_camera.c    |  3 +-
>  drivers/of/base.c                                 | 41 +++++++++++++++-----
>  drivers/video/fbdev/omap2/dss/omapdss-boot-init.c |  7 +---
>  include/linux/of_graph.h                          | 18 ++++++++++
>  8 files changed, 61 insertions(+), 48 deletions(-)

-- 
Regards,

Laurent Pinchart

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

[Philipp Zabel]

Hi Laurent,

Am Sonntag, den 01.03.2015, 18:20 +0200 schrieb Laurent Pinchart:
> Hi Philipp and all,
> 
> This series has been around for a long time, it seems to be time to get it 
> merged.
> 
> What's the plan ? If we wait for the v4.1 merge window there's a chance we'll 
> get more conflicts, especially on patch 1/3. Could it be merged in v4.0-rc to 
> avoid that ?

I've just sent another pull request to Grant and Rob. My favorite
outcome would be for the tag to be merged in time for v4.0 via the
device tree branch. My second most favorite outcome would be maintainer
approval to merge the same tag into both drm, media, etc. for v4.1.

regards
Philipp

> On Monday 23 February 2015 11:54:03 Philipp Zabel wrote:
> > Hi,
> > 
> > Since there now is a merge conflict in imx-drm-core, I've rebased the series
> > onto v4.0-rc1. Also a new driver touched by this change appeared, so the
> > first patch now includes a fix for am437x-vfpe, too. I'd be happy to get an
> > ack for that.
> > 
> > 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 v8:
> >  - Rebased onto v4.0-rc1
> >  - Added fix to am437x-vpfe
> > 
> > The previous version can be found here: https://lkml.org/lkml/2014/12/23/219
> > 
> > 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                | 11 +-----
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c             | 15 +++------
> >  drivers/media/platform/am437x/am437x-vpfe.c       |  1 -
> >  drivers/media/platform/soc_camera/soc_camera.c    |  3 +-
> >  drivers/of/base.c                                 | 41 +++++++++++++++-----
> >  drivers/video/fbdev/omap2/dss/omapdss-boot-init.c |  7 +---
> >  include/linux/of_graph.h                          | 18 ++++++++++
> >  8 files changed, 61 insertions(+), 48 deletions(-)
>