diff mbox series

[v2,01/13] drm/sun4i: backend: Simplify the get_id logic

Message ID 1a9bf911b0a40475da8025859032514131d5397b.1552594551.git-series.maxime.ripard@bootlin.com (mailing list archive)
State New, archived
Headers show
Series ARM: dts: sunxi: Cleanup DTC warnings | expand

Commit Message

Maxime Ripard March 14, 2019, 8:16 p.m. UTC
Using the new helpers introduced since we wrote that code, we can simplify
the code to retrieve the backend ID significantly.

The new code will also allow us to deal nicely with endpoints that don't
have a reg property, as expected in the case where there's a single
endpoint for a given port.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 34 +++++++++-------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

Comments

Chen-Yu Tsai March 15, 2019, 2:43 a.m. UTC | #1
On Fri, Mar 15, 2019 at 4:16 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Using the new helpers introduced since we wrote that code, we can simplify
> the code to retrieve the backend ID significantly.
>
> The new code will also allow us to deal nicely with endpoints that don't
> have a reg property, as expected in the case where there's a single
> endpoint for a given port.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 34 +++++++++-------------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index 4c0d51f73237..02ef8e455db8 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -720,33 +720,21 @@ static int sun4i_backend_free_sat(struct device *dev) {
>   */
>  static int sun4i_backend_of_get_id(struct device_node *node)
>  {
> -       struct device_node *port, *ep;
> -       int ret = -EINVAL;
> +       struct device_node *ep, *remote;
> +       struct of_endpoint of_ep;
>
> -       /* input is port 0 */
> -       port = of_graph_get_port_by_id(node, 0);
> -       if (!port)
> +       /* Input port is 0, and we want the first endpoint. */
> +       ep = of_graph_get_endpoint_by_regs(node, 0, -1);
> +       if (!ep)
>                 return -EINVAL;
>
> -       /* try finding an upstream endpoint */
> -       for_each_available_child_of_node(port, ep) {
> -               struct device_node *remote;
> -               u32 reg;
> -
> -               remote = of_graph_get_remote_endpoint(ep);
> -               if (!remote)
> -                       continue;
> -
> -               ret = of_property_read_u32(remote, "reg", &reg);
> -               if (ret)
> -                       continue;
> -
> -               ret = reg;
> -       }
> -
> -       of_node_put(port);
> +       remote = of_graph_get_remote_endpoint(ep);

I believe you also need to call of_node_put for ep?

Otherwise,

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

> +       if (!remote)
> +               return -EINVAL;
>
> -       return ret;
> +       of_graph_parse_endpoint(remote, &of_ep);
> +       of_node_put(remote);
> +       return of_ep.id;
>  }
>
>  /* TODO: This needs to take multiple pipelines into account */
> --
> git-series 0.9.1
Maxime Ripard March 15, 2019, 8:38 a.m. UTC | #2
On Fri, Mar 15, 2019 at 10:43:48AM +0800, Chen-Yu Tsai wrote:
> On Fri, Mar 15, 2019 at 4:16 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > Using the new helpers introduced since we wrote that code, we can simplify
> > the code to retrieve the backend ID significantly.
> >
> > The new code will also allow us to deal nicely with endpoints that don't
> > have a reg property, as expected in the case where there's a single
> > endpoint for a given port.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_backend.c | 34 +++++++++-------------------
> >  1 file changed, 11 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > index 4c0d51f73237..02ef8e455db8 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > @@ -720,33 +720,21 @@ static int sun4i_backend_free_sat(struct device *dev) {
> >   */
> >  static int sun4i_backend_of_get_id(struct device_node *node)
> >  {
> > -       struct device_node *port, *ep;
> > -       int ret = -EINVAL;
> > +       struct device_node *ep, *remote;
> > +       struct of_endpoint of_ep;
> >
> > -       /* input is port 0 */
> > -       port = of_graph_get_port_by_id(node, 0);
> > -       if (!port)
> > +       /* Input port is 0, and we want the first endpoint. */
> > +       ep = of_graph_get_endpoint_by_regs(node, 0, -1);
> > +       if (!ep)
> >                 return -EINVAL;
> >
> > -       /* try finding an upstream endpoint */
> > -       for_each_available_child_of_node(port, ep) {
> > -               struct device_node *remote;
> > -               u32 reg;
> > -
> > -               remote = of_graph_get_remote_endpoint(ep);
> > -               if (!remote)
> > -                       continue;
> > -
> > -               ret = of_property_read_u32(remote, "reg", &reg);
> > -               if (ret)
> > -                       continue;
> > -
> > -               ret = reg;
> > -       }
> > -
> > -       of_node_put(port);
> > +       remote = of_graph_get_remote_endpoint(ep);
> 
> I believe you also need to call of_node_put for ep?

Ah, right. It wasn't mentionned in the doc, but you definitely need
it. I'll send a patch for that.

> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Thanks!

I've applied this on and the next and added the of_node_put in the
process.

Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 4c0d51f73237..02ef8e455db8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -720,33 +720,21 @@  static int sun4i_backend_free_sat(struct device *dev) {
  */
 static int sun4i_backend_of_get_id(struct device_node *node)
 {
-	struct device_node *port, *ep;
-	int ret = -EINVAL;
+	struct device_node *ep, *remote;
+	struct of_endpoint of_ep;
 
-	/* input is port 0 */
-	port = of_graph_get_port_by_id(node, 0);
-	if (!port)
+	/* Input port is 0, and we want the first endpoint. */
+	ep = of_graph_get_endpoint_by_regs(node, 0, -1);
+	if (!ep)
 		return -EINVAL;
 
-	/* try finding an upstream endpoint */
-	for_each_available_child_of_node(port, ep) {
-		struct device_node *remote;
-		u32 reg;
-
-		remote = of_graph_get_remote_endpoint(ep);
-		if (!remote)
-			continue;
-
-		ret = of_property_read_u32(remote, "reg", &reg);
-		if (ret)
-			continue;
-
-		ret = reg;
-	}
-
-	of_node_put(port);
+	remote = of_graph_get_remote_endpoint(ep);
+	if (!remote)
+		return -EINVAL;
 
-	return ret;
+	of_graph_parse_endpoint(remote, &of_ep);
+	of_node_put(remote);
+	return of_ep.id;
 }
 
 /* TODO: This needs to take multiple pipelines into account */