[PATCH 4/4] drm/rockchip: use for_each_endpoint_of_node macro, drop endpoint reference on break
Daniel Kurtz
djkurtz at chromium.org
Mon Feb 23 19:13:05 PST 2015
Hi Philipp,
Thanks for this cleanup!
On Tue, Feb 24, 2015 at 12:34 AM, Philipp Zabel <p.zabel at pengutronix.de> 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 at pengutronix.de>
> ---
> 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 at chromium.org>
[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
>
More information about the dri-devel
mailing list