linux-kernel - Re: [PATCH 5/5] Revert "drm: atmel-hlcdc: enable sys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191210203419.GB24756@ravnborg.org>
Date:   Tue, 10 Dec 2019 21:34:19 +0100
From:   Sam Ravnborg <sam@...nborg.org>
To:     Claudiu Beznea <claudiu.beznea@...rochip.com>
Cc:     bbrezillon@...nel.org, airlied@...ux.ie, daniel@...ll.ch,
        nicolas.ferre@...rochip.com, alexandre.belloni@...tlin.com,
        ludovic.desroches@...rochip.com, lee.jones@...aro.org,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Sandeep Sheriker Mallikarjun 
        <sandeepsheriker.mallikarjun@...rochip.com>
Subject: Re: [PATCH 5/5] Revert "drm: atmel-hlcdc: enable sys_clk during
 initalization."

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@...rochip.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@...rochip.com>

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