Re: [PATCH 4/4] drm/rockchip: use for_each_endpoint_of_node macro, drop endpoint reference on break

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Philipp,

Thanks for this cleanup!

On Tue, Feb 24, 2015 at 12:34 AM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
>
> Using the for_each_... macro should make the code a bit shorter and
> easier to read. Also, when breaking out of the loop, the endpoint node
> reference count needs to be decremented.
>
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 21a481b..9bb4fd2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -366,7 +366,7 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>  int rockchip_drm_encoder_get_mux_id(struct device_node *node,
>                                     struct drm_encoder *encoder)
>  {
> -       struct device_node *ep = NULL;
> +       struct device_node *ep;
>         struct drm_crtc *crtc = encoder->crtc;
>         struct of_endpoint endpoint;
>         struct device_node *port;
> @@ -375,18 +375,15 @@ int rockchip_drm_encoder_get_mux_id(struct device_node *node,
>         if (!node || !crtc)
>                 return -EINVAL;
>
> -       do {
> -               ep = of_graph_get_next_endpoint(node, ep);
> -               if (!ep)
> -                       break;
> -
> +       for_each_endpoint_of_node(node, ep) {
>                 port = of_graph_get_remote_port(ep);
>                 of_node_put(port);

Shouldn't we put port after comparing it to crtc->port?

This looks like an existing issue, though so this patch is (assuming
the series [0] is merged):

Reviewed-by: Daniel Kurtz <djkurtz@xxxxxxxxxxxx>

[0] https://lkml.org/lkml/2015/2/23/115

>
>                 if (port == crtc->port) {
>                         ret = of_graph_parse_endpoint(ep, &endpoint);
> +                       of_node_put(ep);
>                         return ret ?: endpoint.id;

This function looks really similar to imx_drm_encoder_get_mux_id().
The only difference is that it looks up endpoint.id rather than endpoint.port.

Is there any way we can resolve this slight difference so that both
can use a single common helper?

Or, at least move both of them to somewhere more generic?

>                 }
> -       } while (ep);
> +       }
>
>         return -EINVAL;
>  }
> --
> 2.1.4
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux