[PATCH] drm/rockchip: fix clk enable disable mismatch in vop_crtc_mode_set

[Daniel Kurtz]

Hi Heiko,

On Sat, Jan 31, 2015 at 3:28 AM, Heiko Stübner <heiko at sntech.de> wrote:
> The function disables the dclk at the beginning, so don't simply return
> when an error happens, but instead enable the clock again, so that
> enable and disable calls are balanced.

This function is a bit of a disaster, and needs fixing some day.
AFAICT, the dclk_rst is actually ignored if dclk is disabled, so that
part doesn't even work.

>
> ret_clk is introduced to hold the clk_enable result and not mangle the
> original error code.
>
> Signed-off-by: Heiko Stuebner <heiko at sntech.de>

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 04b619a..c0387f7 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -852,7 +852,7 @@ static int vop_crtc_mode_set(struct drm_crtc *crtc,
>         u16 vsync_len = adjusted_mode->vsync_end - adjusted_mode->vsync_start;
>         u16 vact_st = adjusted_mode->vtotal - adjusted_mode->vsync_start;
>         u16 vact_end = vact_st + vdisplay;
> -       int ret;
> +       int ret, ret_clk;
>         uint32_t val;
>
>         /*
> @@ -874,7 +874,8 @@ static int vop_crtc_mode_set(struct drm_crtc *crtc,
>         default:
>                 DRM_ERROR("unsupport connector_type[%d]\n",
>                           vop->connector_type);
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto out;
>         };
>         VOP_CTRL_SET(vop, out_mode, vop->connector_out_mode);
>
> @@ -897,7 +898,7 @@ static int vop_crtc_mode_set(struct drm_crtc *crtc,
>
>         ret = vop_crtc_mode_set_base(crtc, x, y, fb);
>         if (ret)
> -               return ret;
> +               goto out;
>
>         /*
>          * reset dclk, take all mode config affect, so the clk would run in
> @@ -908,13 +909,14 @@ static int vop_crtc_mode_set(struct drm_crtc *crtc,
>         reset_control_deassert(vop->dclk_rst);
>
>         clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
> -       ret = clk_enable(vop->dclk);
> -       if (ret < 0) {
> -               dev_err(vop->dev, "failed to enable dclk - %d\n", ret);
> -               return ret;
> +out:
> +       ret_clk = clk_enable(vop->dclk);
> +       if (ret_clk < 0) {
> +               dev_err(vop->dev, "failed to enable dclk - %d\n", ret_clk);
> +               return ret_clk;

Doesn't this swallow ret ?  I thought the point was to still return
the original error?


>         }
>
> -       return 0;
> +       return ret;
>  }
>
>  static void vop_crtc_commit(struct drm_crtc *crtc)
> --
> 2.1.1