[PATCH] of: add helper for getting endpoint node with specific identifiers

[PATCH] of: add helper for getting endpoint node with specific identifiers

[Hyungwon Hwang]

When there are multiple ports or multiple endpoints in a port, they have to be
distinguished by the value of reg property. It is common. The drivers can get
the specific endpoint in the specific port via this function. Now the drivers
have to implement this code in themselves or have to force the order of dt nodes
to get the right node.

Signed-off-by: Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/of/base.c        | 34 ++++++++++++++++++++++++++++++++++
 include/linux/of_graph.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 36536b6..95ffe1c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2155,6 +2155,40 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 EXPORT_SYMBOL(of_graph_get_next_endpoint);
 
 /**
+ * of_graph_get_endpoint_by_regs() - get endpoint node of specific identifiers
+ * @parent: pointer to the parent device node
+ * @port_reg: identifier (value of reg property) of the parent port node
+ * @reg: identifier (value of reg property) of the endpoint node
+ *
+ * Return: An 'endpoint' node pointer which is identified by reg and at the same
+ * is the child of a port node identified by port_reg. reg and port_reg are
+ * ignored when they are -1.
+ */
+struct device_node *of_graph_get_endpoint_by_regs(
+	const struct device_node *parent, int port_reg, int reg)
+{
+	struct of_endpoint endpoint;
+	struct device_node *node, *prev_node = NULL;
+
+	while (1) {
+		node = of_graph_get_next_endpoint(parent, prev_node);
+		of_node_put(prev_node);
+		if (!node)
+			break;
+
+		of_graph_parse_endpoint(node, &endpoint);
+		if (((port_reg == -1) ||
+			(endpoint.port == (unsigned int) port_reg)) &&
+			((reg == -1) || (endpoint.id == (unsigned int) reg)))
+			return node;
+
+		prev_node = node;
+	}
+
+	return NULL;
+}
+
+/**
  * of_graph_get_remote_port_parent() - get remote port's parent node
  * @node: pointer to a local endpoint device_node
  *
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index befef42..c119728 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -31,6 +31,8 @@ int of_graph_parse_endpoint(const struct device_node *node,
 				struct of_endpoint *endpoint);
 struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 					struct device_node *previous);
+struct device_node *of_graph_get_endpoint_by_regs(
+		const struct device_node *parent, int port_reg, int reg);
 struct device_node *of_graph_get_remote_port_parent(
 					const struct device_node *node);
 struct device_node *of_graph_get_remote_port(const struct device_node *node);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: add helper for getting endpoint node with specific identifiers

[Rob Herring]

On Mon, Feb 16, 2015 at 4:37 AM, Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> When there are multiple ports or multiple endpoints in a port, they have to be
> distinguished by the value of reg property. It is common. The drivers can get
> the specific endpoint in the specific port via this function. Now the drivers
> have to implement this code in themselves or have to force the order of dt nodes
> to get the right node.
>
> Signed-off-by: Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/of/base.c        | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/of_graph.h |  2 ++
>  2 files changed, 36 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 36536b6..95ffe1c 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2155,6 +2155,40 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
>  EXPORT_SYMBOL(of_graph_get_next_endpoint);
>
>  /**
> + * of_graph_get_endpoint_by_regs() - get endpoint node of specific identifiers
> + * @parent: pointer to the parent device node
> + * @port_reg: identifier (value of reg property) of the parent port node
> + * @reg: identifier (value of reg property) of the endpoint node
> + *
> + * Return: An 'endpoint' node pointer which is identified by reg and at the same
> + * is the child of a port node identified by port_reg. reg and port_reg are
> + * ignored when they are -1.
> + */
> +struct device_node *of_graph_get_endpoint_by_regs(
> +       const struct device_node *parent, int port_reg, int reg)
> +{
> +       struct of_endpoint endpoint;
> +       struct device_node *node, *prev_node = NULL;
> +
> +       while (1) {
> +               node = of_graph_get_next_endpoint(parent, prev_node);
> +               of_node_put(prev_node);
> +               if (!node)
> +                       break;
> +
> +               of_graph_parse_endpoint(node, &endpoint);

Seems like this all could be in a "for_each_of_graph_child_endpoint"
iterator. I've not looked, but I'd guess there are other similar
loops?

> +               if (((port_reg == -1) ||

What if of_graph_parse_endpoint failed?

> +                       (endpoint.port == (unsigned int) port_reg)) &&
> +                       ((reg == -1) || (endpoint.id == (unsigned int) reg)))

Why the casts?

> +                       return node;
> +
> +               prev_node = node;
> +       }
> +
> +       return NULL;
> +}
> +
> +/**
>   * of_graph_get_remote_port_parent() - get remote port's parent node
>   * @node: pointer to a local endpoint device_node
>   *
> diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> index befef42..c119728 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -31,6 +31,8 @@ int of_graph_parse_endpoint(const struct device_node *node,
>                                 struct of_endpoint *endpoint);
>  struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
>                                         struct device_node *previous);
> +struct device_node *of_graph_get_endpoint_by_regs(
> +               const struct device_node *parent, int port_reg, int reg);

Empty version is needed.

>  struct device_node *of_graph_get_remote_port_parent(
>                                         const struct device_node *node);
>  struct device_node *of_graph_get_remote_port(const struct device_node *node);
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of: add helper for getting endpoint node with specific identifiers

[Hyungwon Hwang]

Dear Rob Herring,

On Mon, 16 Feb 2015 11:47:24 -0600
Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, Feb 16, 2015 at 4:37 AM, Hyungwon Hwang
> <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > When there are multiple ports or multiple endpoints in a port, they
> > have to be distinguished by the value of reg property. It is
> > common. The drivers can get the specific endpoint in the specific
> > port via this function. Now the drivers have to implement this code
> > in themselves or have to force the order of dt nodes to get the
> > right node.
> >
> > Signed-off-by: Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  drivers/of/base.c        | 34 ++++++++++++++++++++++++++++++++++
> >  include/linux/of_graph.h |  2 ++
> >  2 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 36536b6..95ffe1c 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -2155,6 +2155,40 @@ struct device_node
> > *of_graph_get_next_endpoint(const struct device_node *parent,
> > EXPORT_SYMBOL(of_graph_get_next_endpoint);
> >
> >  /**
> > + * of_graph_get_endpoint_by_regs() - get endpoint node of specific
> > identifiers
> > + * @parent: pointer to the parent device node
> > + * @port_reg: identifier (value of reg property) of the parent
> > port node
> > + * @reg: identifier (value of reg property) of the endpoint node
> > + *
> > + * Return: An 'endpoint' node pointer which is identified by reg
> > and at the same
> > + * is the child of a port node identified by port_reg. reg and
> > port_reg are
> > + * ignored when they are -1.
> > + */
> > +struct device_node *of_graph_get_endpoint_by_regs(
> > +       const struct device_node *parent, int port_reg, int reg)
> > +{
> > +       struct of_endpoint endpoint;
> > +       struct device_node *node, *prev_node = NULL;
> > +
> > +       while (1) {
> > +               node = of_graph_get_next_endpoint(parent,
> > prev_node);
> > +               of_node_put(prev_node);
> > +               if (!node)
> > +                       break;
> > +
> > +               of_graph_parse_endpoint(node, &endpoint);
> 
> Seems like this all could be in a "for_each_of_graph_child_endpoint"
> iterator. I've not looked, but I'd guess there are other similar
> loops?
> 

There is no other iterator which can be used here. So other codes which
call of_graph_get_next_endpoint() are also using loop. I thought
about adding that kinds of iterator newly. IMHO, I feel it is a little
excessive. So I prefer to using the loop.

> > +               if (((port_reg == -1) ||
> 
> What if of_graph_parse_endpoint failed?
> 

It never fails and never return except 0. So there is no need to check
the return value of it.

> > +                       (endpoint.port == (unsigned int) port_reg))
> > &&
> > +                       ((reg == -1) || (endpoint.id == (unsigned
> > int) reg)))
> 
> Why the casts?

I just clarify that endpoint.{port/id} is unsigned int. But now it
seems unnecessary, and make harder to read the code. I will remove it.

> 
> > +                       return node;
> > +
> > +               prev_node = node;
> > +       }
> > +
> > +       return NULL;
> > +}
> > +
> > +/**
> >   * of_graph_get_remote_port_parent() - get remote port's parent
> > node
> >   * @node: pointer to a local endpoint device_node
> >   *
> > diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> > index befef42..c119728 100644
> > --- a/include/linux/of_graph.h
> > +++ b/include/linux/of_graph.h
> > @@ -31,6 +31,8 @@ int of_graph_parse_endpoint(const struct
> > device_node *node, struct of_endpoint *endpoint);
> >  struct device_node *of_graph_get_next_endpoint(const struct
> > device_node *parent, struct device_node *previous);
> > +struct device_node *of_graph_get_endpoint_by_regs(
> > +               const struct device_node *parent, int port_reg, int
> > reg);
> 
> Empty version is needed.

Sorry. I forgot.

> 
> >  struct device_node *of_graph_get_remote_port_parent(
> >                                         const struct device_node
> > *node); struct device_node *of_graph_get_remote_port(const struct
> > device_node *node); --
> > 1.9.1
> >

Best regards,
Hyungwon Hwang
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html