Re: [PATCH v2 01/10] drm/exynos/decon5433: use readl_poll_timeout helpers

From: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
To: Andrzej Hajda <a.hajda@samsung.com>, Inki Dae <inki.dae@samsung.com>
Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	dri-devel@lists.freedesktop.org,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH v2 01/10] drm/exynos/decon5433: use readl_poll_timeout helpers
Date: Fri, 25 Aug 2017 13:25:26 +0200	[thread overview]
Message-ID: <40d914b4-3584-8a14-1c2a-db1966bb6a2d@math.uni-bielefeld.de> (raw)
In-Reply-To: <2ae55d7a-3730-72f7-ae7e-8556b9e4e4a5@math.uni-bielefeld.de>

Tobias Jakobi wrote:
> Hello Andrzej,
> 
> 
> Andrzej Hajda wrote:
>> Linux core provide helpers for polling with timeout, lets use them.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 20 ++++++++------------
>>  1 file changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> index 5792ca88..237b4c9 100644
>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> @@ -13,6 +13,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/clk.h>
>>  #include <linux/component.h>
>> +#include <linux/iopoll.h>
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/of_device.h>
>>  #include <linux/of_gpio.h>
>> @@ -407,24 +408,19 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
>>  
>>  static void decon_swreset(struct decon_context *ctx)
>>  {
>> -	unsigned int tries;
>>  	unsigned long flags;
>> +	u32 val;
>> +	int ret;
>>  
>>  	writel(0, ctx->addr + DECON_VIDCON0);
>> -	for (tries = 2000; tries; --tries) {
>> -		if (~readl(ctx->addr + DECON_VIDCON0) & VIDCON0_STOP_STATUS)
>> -			break;
>> -		udelay(10);
>> -	}
>> +	readl_poll_timeout(ctx->addr + DECON_VIDCON0, val,
>> +			   ~val & VIDCON0_STOP_STATUS, 12, 20000);
> Wouldn't it be more consistent to also check for a timeout here?
I see! Thanks for the clarification.

- Tobias


> With best wishes,
> Tobias
> 
> 
> 
>>  	writel(VIDCON0_SWRESET, ctx->addr + DECON_VIDCON0);
>> -	for (tries = 2000; tries; --tries) {
>> -		if (~readl(ctx->addr + DECON_VIDCON0) & VIDCON0_SWRESET)
>> -			break;
>> -		udelay(10);
>> -	}
>> +	ret = readl_poll_timeout(ctx->addr + DECON_VIDCON0, val,
>> +				 ~val & VIDCON0_SWRESET, 12, 20000);
>>  
>> -	WARN(tries == 0, "failed to software reset DECON\n");
>> +	WARN(ret < 0, "failed to software reset DECON\n");
>>  
>>  	spin_lock_irqsave(&ctx->vblank_lock, flags);
>>  	ctx->frame_id = 0;
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel