2014-12-22 15:11:42

by Philipp Zabel

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

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


2014-12-22 15:11:47

by Philipp Zabel

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

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 <[email protected]>
Acked-by: Mauro Carvalho Chehab <[email protected]>
---
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

2014-12-22 15:11:53

by Philipp Zabel

[permalink] [raw]
Subject: [PATCH v6 3/3] of: Add of_graph_get_port_by_id function

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

Signed-off-by: Philipp Zabel <[email protected]>
Acked-by: Laurent Pinchart <[email protected]>
---
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

2014-12-22 15:12:29

by Philipp Zabel

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

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 <[email protected]>
Acked-by: Laurent Pinchart <[email protected]>
---
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

2014-12-22 21:09:48

by Mathieu Poirier

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

On 22 December 2014 at 08:11, Philipp Zabel <[email protected]> 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 <[email protected]>
> Acked-by: Mauro Carvalho Chehab <[email protected]>
> ---
> 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.

2014-12-23 11:10:22

by Andrzej Hajda

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

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 <[email protected]>
> Acked-by: Laurent Pinchart <[email protected]>
> ---
> 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)
>

2014-12-23 12:02:21

by Philipp Zabel

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

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

2014-12-23 12:02:26

by Philipp Zabel

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

On Mon, Dec 22, 2014 at 02:09:46PM -0700, Mathieu Poirier wrote:
> On 22 December 2014 at 08:11, Philipp Zabel <[email protected]> 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 <[email protected]>
> > Acked-by: Mauro Carvalho Chehab <[email protected]>
[...]
> I tested this in my tree - ack.

Thanks!

regards
Philipp