Re: [PATCH 5/5] Revert "drm: atmel-hlcdc: enable sys_clk during initalization."

[Sam Ravnborg]

Hi Cladiu

On Tue, Dec 10, 2019 at 03:24:47PM +0200, Claudiu Beznea wrote:
> This reverts commit d2c755e66617620b729041c625a6396c81d1231c.
> ("drm: atmel-hlcdc: enable sys_clk during initalization."). With
> commit "drm: atmel-hlcdc: enable clock before configuring timing engine"
> there is no need for this patch. Code is also simpler.
> 
> Cc: Sandeep Sheriker Mallikarjun <sandeepsheriker.mallikarjun@xxxxxxxxxxxxx>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>

Getting further in the patches tells me you looked at the
patch I referenced in previous mail.
Please squash the two patches together - that would make it
easier to follow what is done.

With the two patches applied sysclk is enabled only in mode_set_nofb()
and atomic_enable(). And disabled in atomic_disable().
This is simpler and we drop the conditionals. Also good.
So the end result looks OK.

	Sam

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 +------------------
>  1 file changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 8dc917a1270b..112aa5066cee 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -721,18 +721,10 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  	dc->hlcdc = dev_get_drvdata(dev->dev->parent);
>  	dev->dev_private = dc;
>  
> -	if (dc->desc->fixed_clksrc) {
> -		ret = clk_prepare_enable(dc->hlcdc->sys_clk);
> -		if (ret) {
> -			dev_err(dev->dev, "failed to enable sys_clk\n");
> -			goto err_destroy_wq;
> -		}
> -	}
> -
>  	ret = clk_prepare_enable(dc->hlcdc->periph_clk);
>  	if (ret) {
>  		dev_err(dev->dev, "failed to enable periph_clk\n");
> -		goto err_sys_clk_disable;
> +		goto err_destroy_wq;
>  	}
>  
>  	pm_runtime_enable(dev->dev);
> @@ -768,9 +760,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  err_periph_clk_disable:
>  	pm_runtime_disable(dev->dev);
>  	clk_disable_unprepare(dc->hlcdc->periph_clk);
> -err_sys_clk_disable:
> -	if (dc->desc->fixed_clksrc)
> -		clk_disable_unprepare(dc->hlcdc->sys_clk);
>  
>  err_destroy_wq:
>  	destroy_workqueue(dc->wq);
> @@ -795,8 +784,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
>  
>  	pm_runtime_disable(dev->dev);
>  	clk_disable_unprepare(dc->hlcdc->periph_clk);
> -	if (dc->desc->fixed_clksrc)
> -		clk_disable_unprepare(dc->hlcdc->sys_clk);
>  	destroy_workqueue(dc->wq);
>  }
>  
> @@ -910,8 +897,6 @@ static int atmel_hlcdc_dc_drm_suspend(struct device *dev)
>  	regmap_read(regmap, ATMEL_HLCDC_IMR, &dc->suspend.imr);
>  	regmap_write(regmap, ATMEL_HLCDC_IDR, dc->suspend.imr);
>  	clk_disable_unprepare(dc->hlcdc->periph_clk);
> -	if (dc->desc->fixed_clksrc)
> -		clk_disable_unprepare(dc->hlcdc->sys_clk);
>  
>  	return 0;
>  }
> @@ -921,8 +906,6 @@ static int atmel_hlcdc_dc_drm_resume(struct device *dev)
>  	struct drm_device *drm_dev = dev_get_drvdata(dev);
>  	struct atmel_hlcdc_dc *dc = drm_dev->dev_private;
>  
> -	if (dc->desc->fixed_clksrc)
> -		clk_prepare_enable(dc->hlcdc->sys_clk);
>  	clk_prepare_enable(dc->hlcdc->periph_clk);
>  	regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, dc->suspend.imr);
>  
> -- 
> 2.7.4