[1/2] of: Add helper to lookup non port child node

Message ID 20211207054747.461029-2-jagan@amarulasolutions.com
State New
Headers show
Series
  • drm: of: Lookup child panel_or_bridge
Related show

Commit Message

Jagan Teki Dec. 7, 2021, 5:47 a.m. UTC
Add of_get_non_port_child() helper that can be used to lookup
non port child nodes.

Some OF graphs don't require 'ports' to represent the next output
instead, it simply adds a child node on a given parent node. This
helper lookup that child node, however that child node is not a
'port' on given parent as 'port' based nodes are looked up via
of_graph_get_remote_node().

Example OF graph representation of DSI host, which doesn't
have 'ports'.

dsi {
	#address-cells = <1>;
	#size-cells = <0>;

	port {
		dsi_in_tcon0: endpoint {
			remote-endpoint = <tcon0_out_dsi>;
	};

	panel@0 {
		reg = <0>;
	};
};

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/of/base.c  | 29 +++++++++++++++++++++++++++++
 include/linux/of.h |  6 ++++++
 2 files changed, 35 insertions(+)

Comments

Rob Herring Dec. 7, 2021, 8:50 p.m. UTC | #1
On Mon, Dec 6, 2021 at 11:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Add of_get_non_port_child() helper that can be used to lookup
> non port child nodes.
>
> Some OF graphs don't require 'ports' to represent the next output
> instead, it simply adds a child node on a given parent node. This
> helper lookup that child node, however that child node is not a
> 'port' on given parent as 'port' based nodes are looked up via
> of_graph_get_remote_node().
>
> Example OF graph representation of DSI host, which doesn't
> have 'ports'.

This seems pretty specific to DSI and also can't handle there being
more than 1 non-port node. That's allowed for DSI too, but I don't
think I've ever seen a case. Anyways, I'd just move this to DRM rather
than common DT code. One comment on the implementation that will
shrink it.

>
> dsi {
>         #address-cells = <1>;
>         #size-cells = <0>;
>
>         port {
>                 dsi_in_tcon0: endpoint {
>                         remote-endpoint = <tcon0_out_dsi>;
>         };
>
>         panel@0 {
>                 reg = <0>;
>         };
> };
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/of/base.c  | 29 +++++++++++++++++++++++++++++
>  include/linux/of.h |  6 ++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 61de453b885c..31bbf885b0f8 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -854,6 +854,35 @@ struct device_node *of_get_compatible_child(const struct device_node *parent,
>  }
>  EXPORT_SYMBOL(of_get_compatible_child);
>
> +/**
> + * of_get_non_port_child - Find the non port child node for a given parent
> + * @node:      parent node
> + *
> + * This function looks for child node which is not port child for given parent.
> + *
> + * Return: A node pointer if found, with refcount incremented, use
> + * of_node_put() on it when done.
> + * Returns NULL if node is not found.
> + */
> +struct device_node *of_get_non_port_child(const struct device_node *parent)
> +{
> +       struct device_node *child;
> +
> +       for_each_child_of_node(parent, child) {

for_each_available_child_of_node

> +               if (of_node_name_eq(child, "port"))
> +                       continue;
> +
> +               if (!of_device_is_available(child)) {
> +                       of_node_put(child);
> +                       continue;
> +               }
> +               break;
> +       }
> +
> +       return child;
> +}
> +EXPORT_SYMBOL(of_get_non_port_child);
> +
>  /**
>   * of_get_child_by_name - Find the child node by name for a given parent
>   * @node:      parent node
> diff --git a/include/linux/of.h b/include/linux/of.h
> index ff143a027abc..3e699becef82 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -290,6 +290,7 @@ extern struct device_node *of_get_next_child(const struct device_node *node,
>  extern struct device_node *of_get_next_available_child(
>         const struct device_node *node, struct device_node *prev);
>
> +extern struct device_node *of_get_non_port_child(const struct device_node *parent);
>  extern struct device_node *of_get_compatible_child(const struct device_node *parent,
>                                         const char *compatible);
>  extern struct device_node *of_get_child_by_name(const struct device_node *node,
> @@ -678,6 +679,11 @@ static inline bool of_have_populated_dt(void)
>         return false;
>  }
>
> +static inline struct device_node *of_get_non_port_child(const struct device_node *parent)
> +{
> +       return NULL;
> +}
> +
>  static inline struct device_node *of_get_compatible_child(const struct device_node *parent,
>                                         const char *compatible)
>  {
> --
> 2.25.1
>
Jagan Teki Dec. 8, 2021, 6:26 a.m. UTC | #2
On Wed, Dec 8, 2021 at 2:20 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Mon, Dec 6, 2021 at 11:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Add of_get_non_port_child() helper that can be used to lookup
> > non port child nodes.
> >
> > Some OF graphs don't require 'ports' to represent the next output
> > instead, it simply adds a child node on a given parent node. This
> > helper lookup that child node, however that child node is not a
> > 'port' on given parent as 'port' based nodes are looked up via
> > of_graph_get_remote_node().
> >
> > Example OF graph representation of DSI host, which doesn't
> > have 'ports'.
>
> This seems pretty specific to DSI and also can't handle there being
> more than 1 non-port node. That's allowed for DSI too, but I don't
> think I've ever seen a case. Anyways, I'd just move this to DRM rather
> than common DT code. One comment on the implementation that will
> shrink it.

I think it can be possible to OF graph even for non-DSI, however if
the end-node outputs to a panel or bridge. At the moment, I can one
use case on the non-DSI side is rcar du encoder.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rcar-du/rcar_du_encoder.c#n68

I'm not sure this can be done existing of_graph_get_remote_node, but an example.

>
> >
> > dsi {
> >         #address-cells = <1>;
> >         #size-cells = <0>;
> >
> >         port {
> >                 dsi_in_tcon0: endpoint {
> >                         remote-endpoint = <tcon0_out_dsi>;
> >         };
> >
> >         panel@0 {
> >                 reg = <0>;
> >         };
> > };
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/of/base.c  | 29 +++++++++++++++++++++++++++++
> >  include/linux/of.h |  6 ++++++
> >  2 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 61de453b885c..31bbf885b0f8 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -854,6 +854,35 @@ struct device_node *of_get_compatible_child(const struct device_node *parent,
> >  }
> >  EXPORT_SYMBOL(of_get_compatible_child);
> >
> > +/**
> > + * of_get_non_port_child - Find the non port child node for a given parent
> > + * @node:      parent node
> > + *
> > + * This function looks for child node which is not port child for given parent.
> > + *
> > + * Return: A node pointer if found, with refcount incremented, use
> > + * of_node_put() on it when done.
> > + * Returns NULL if node is not found.
> > + */
> > +struct device_node *of_get_non_port_child(const struct device_node *parent)
> > +{
> > +       struct device_node *child;
> > +
> > +       for_each_child_of_node(parent, child) {
>
> for_each_available_child_of_node

Look like this simplifies not required to use of_device_is_available,
I will use it in v2.

Thanks,
Jagan.
Jagan Teki Dec. 9, 2021, 5:35 p.m. UTC | #3
Hi Rob and Laurent,

On Wed, Dec 8, 2021 at 11:56 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Wed, Dec 8, 2021 at 2:20 AM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Mon, Dec 6, 2021 at 11:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Add of_get_non_port_child() helper that can be used to lookup
> > > non port child nodes.
> > >
> > > Some OF graphs don't require 'ports' to represent the next output
> > > instead, it simply adds a child node on a given parent node. This
> > > helper lookup that child node, however that child node is not a
> > > 'port' on given parent as 'port' based nodes are looked up via
> > > of_graph_get_remote_node().
> > >
> > > Example OF graph representation of DSI host, which doesn't
> > > have 'ports'.
> >
> > This seems pretty specific to DSI and also can't handle there being
> > more than 1 non-port node. That's allowed for DSI too, but I don't
> > think I've ever seen a case. Anyways, I'd just move this to DRM rather
> > than common DT code. One comment on the implementation that will
> > shrink it.
>
> I think it can be possible to OF graph even for non-DSI, however if
> the end-node outputs to a panel or bridge. At the moment, I can one
> use case on the non-DSI side is rcar du encoder.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rcar-du/rcar_du_encoder.c#n68

Do you see any point to make this helper in of/base based on above
rcar_du_encoder usage? if not i can directly use this functionality in
panel_or_bridge finding code itself.

Please let me know.

Thanks,
Jagan.

Patch

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 61de453b885c..31bbf885b0f8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -854,6 +854,35 @@  struct device_node *of_get_compatible_child(const struct device_node *parent,
 }
 EXPORT_SYMBOL(of_get_compatible_child);
 
+/**
+ * of_get_non_port_child - Find the non port child node for a given parent
+ * @node:	parent node
+ *
+ * This function looks for child node which is not port child for given parent.
+ *
+ * Return: A node pointer if found, with refcount incremented, use
+ * of_node_put() on it when done.
+ * Returns NULL if node is not found.
+ */
+struct device_node *of_get_non_port_child(const struct device_node *parent)
+{
+	struct device_node *child;
+
+	for_each_child_of_node(parent, child) {
+		if (of_node_name_eq(child, "port"))
+			continue;
+
+		if (!of_device_is_available(child)) {
+			of_node_put(child);
+			continue;
+		}
+		break;
+	}
+
+	return child;
+}
+EXPORT_SYMBOL(of_get_non_port_child);
+
 /**
  * of_get_child_by_name - Find the child node by name for a given parent
  * @node:	parent node
diff --git a/include/linux/of.h b/include/linux/of.h
index ff143a027abc..3e699becef82 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -290,6 +290,7 @@  extern struct device_node *of_get_next_child(const struct device_node *node,
 extern struct device_node *of_get_next_available_child(
 	const struct device_node *node, struct device_node *prev);
 
+extern struct device_node *of_get_non_port_child(const struct device_node *parent);
 extern struct device_node *of_get_compatible_child(const struct device_node *parent,
 					const char *compatible);
 extern struct device_node *of_get_child_by_name(const struct device_node *node,
@@ -678,6 +679,11 @@  static inline bool of_have_populated_dt(void)
 	return false;
 }
 
+static inline struct device_node *of_get_non_port_child(const struct device_node *parent)
+{
+	return NULL;
+}
+
 static inline struct device_node *of_get_compatible_child(const struct device_node *parent,
 					const char *compatible)
 {