Re: [PATCH v4] drm/exynos: prepare FIMD clocks

Viresh Kumar
2013-04-08T11:11:55Z

On 8 April 2013 16:37, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> While migrating to common clock framework (CCF), I found that the FIMD clocks
> were pulled down by the CCF.
> If CCF finds any clock(s) which has NOT been claimed by any of the
> drivers, then such clock(s) are PULLed low by CCF.
>
> Calling clk_prepare() for FIMD clocks fixes the issue.
>
> This patch also replaces clk_disable() with clk_unprepare() during exit, since
> clk_prepare() is called in fimd_probe().

I asked you about fixing your commit log too.. It still looks incorrect to me.

This patch doesn't have anything to do with CCF pulling clocks down, but
calling clk_prepare() before clk_enable() is must now.. that's it..
nothing more.

> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> ---
> Changes since v3:
> - added clk_prepare() in fimd_probe() and clk_unprepare() in fimd_remove()
> as suggested by Viresh Kumar <viresh.kumar@linaro.org>
> Changes since v2:
> - moved clk_prepare_enable() and clk_disable_unprepare() from
> fimd_probe() to fimd_clock() as suggested by Inki Dae <inki.dae@samsung.com>
> Changes since v1:
> - added error checking for clk_prepare_enable() and also replaced
> clk_disable() with clk_disable_unprepare() during exit.
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 9537761..aa22370 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -934,6 +934,16 @@ static int fimd_probe(struct platform_device *pdev)
> return ret;
> }
>
> + ret = clk_prepare(ctx->bus_clk);
> + if (ret < 0)
> + return ret;
> +
> + ret = clk_prepare(ctx->lcd_clk);
> + if (ret < 0) {
> + clk_unprepare(ctx->bus_clk);
> + return ret;
> + }
> +
> ctx->vidcon0 = pdata->vidcon0;
> ctx->vidcon1 = pdata->vidcon1;
> ctx->default_win = pdata->default_win;
> @@ -981,8 +991,8 @@ static int fimd_remove(struct platform_device *pdev)
> if (ctx->suspended)
> goto out;
>
> - clk_disable(ctx->lcd_clk);
> - clk_disable(ctx->bus_clk);
> + clk_unprepare(ctx->lcd_clk);
> + clk_unprepare(ctx->bus_clk);

This looks wrong again.. You still need to call clk_disable() to make
clk enabled
count zero...