diff mbox series

[v2,3/3] watchdog: sprd: change to use usleep_range() instead of busy loop

Message ID 20201029023933.24548-4-zhang.lyra@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series A few fixes to sprd watchdog driver | expand

Commit Message

Chunyan Zhang Oct. 29, 2020, 2:39 a.m. UTC
From: Chunyan Zhang <chunyan.zhang@unisoc.com>

After changing to check busy bit for the previous loading operation instead
of the current one, for most of cases, the busy bit is not set for the
first time of read, so there's no need to check so frequently, so this
patch use usleep_range() to replace cpu_relax() to avoid busy loop.

Also this patch change the max times to 11 which would be enough, since
according to the specification, the busy bit would be set after a new
loading operation and last 2 or 3 RTC clock cycles (about 60us~92us).

Fixes: 477603467009 ("watchdog: Add Spreadtrum watchdog driver")
Original-by: Lingling Xu <ling_ling.xu@unisoc.com>
Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
---
 drivers/watchdog/sprd_wdt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Guenter Roeck Nov. 8, 2020, 4:30 p.m. UTC | #1
On 10/28/20 7:39 PM, Chunyan Zhang wrote:
> From: Chunyan Zhang <chunyan.zhang@unisoc.com>
> 
> After changing to check busy bit for the previous loading operation instead
> of the current one, for most of cases, the busy bit is not set for the
> first time of read, so there's no need to check so frequently, so this
> patch use usleep_range() to replace cpu_relax() to avoid busy loop.
> 
> Also this patch change the max times to 11 which would be enough, since
> according to the specification, the busy bit would be set after a new
> loading operation and last 2 or 3 RTC clock cycles (about 60us~92us).
> 
> Fixes: 477603467009 ("watchdog: Add Spreadtrum watchdog driver")
> Original-by: Lingling Xu <ling_ling.xu@unisoc.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/sprd_wdt.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c
> index b9b1daa9e2a4..e8097551dfcd 100644
> --- a/drivers/watchdog/sprd_wdt.c
> +++ b/drivers/watchdog/sprd_wdt.c
> @@ -53,7 +53,7 @@
>  
>  #define SPRD_WDT_CNT_HIGH_SHIFT		16
>  #define SPRD_WDT_LOW_VALUE_MASK		GENMASK(15, 0)
> -#define SPRD_WDT_LOAD_TIMEOUT		1000
> +#define SPRD_WDT_LOAD_TIMEOUT		11
>  
>  struct sprd_wdt {
>  	void __iomem *base;
> @@ -109,15 +109,17 @@ static int sprd_wdt_load_value(struct sprd_wdt *wdt, u32 timeout,
>  	u32 prtmr_step = pretimeout * SPRD_WDT_CNT_STEP;
>  
>  	/*
> -	 * Waiting the load value operation done,
> -	 * it needs two or three RTC clock cycles.
> +	 * Checking busy bit to make sure the previous loading operation is
> +	 * done. According to the specification, the busy bit would be set
> +	 * after a new loading operation and last 2 or 3 RTC clock
> +	 * cycles (about 60us~92us).
>  	 */
>  	do {
>  		val = readl_relaxed(wdt->base + SPRD_WDT_INT_RAW);
>  		if (!(val & SPRD_WDT_LD_BUSY_BIT))
>  			break;
>  
> -		cpu_relax();
> +		usleep_range(10, 100);
>  	} while (delay_cnt++ < SPRD_WDT_LOAD_TIMEOUT);
>  
>  	if (delay_cnt >= SPRD_WDT_LOAD_TIMEOUT)
>
diff mbox series

Patch

diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c
index b9b1daa9e2a4..e8097551dfcd 100644
--- a/drivers/watchdog/sprd_wdt.c
+++ b/drivers/watchdog/sprd_wdt.c
@@ -53,7 +53,7 @@ 
 
 #define SPRD_WDT_CNT_HIGH_SHIFT		16
 #define SPRD_WDT_LOW_VALUE_MASK		GENMASK(15, 0)
-#define SPRD_WDT_LOAD_TIMEOUT		1000
+#define SPRD_WDT_LOAD_TIMEOUT		11
 
 struct sprd_wdt {
 	void __iomem *base;
@@ -109,15 +109,17 @@  static int sprd_wdt_load_value(struct sprd_wdt *wdt, u32 timeout,
 	u32 prtmr_step = pretimeout * SPRD_WDT_CNT_STEP;
 
 	/*
-	 * Waiting the load value operation done,
-	 * it needs two or three RTC clock cycles.
+	 * Checking busy bit to make sure the previous loading operation is
+	 * done. According to the specification, the busy bit would be set
+	 * after a new loading operation and last 2 or 3 RTC clock
+	 * cycles (about 60us~92us).
 	 */
 	do {
 		val = readl_relaxed(wdt->base + SPRD_WDT_INT_RAW);
 		if (!(val & SPRD_WDT_LD_BUSY_BIT))
 			break;
 
-		cpu_relax();
+		usleep_range(10, 100);
 	} while (delay_cnt++ < SPRD_WDT_LOAD_TIMEOUT);
 
 	if (delay_cnt >= SPRD_WDT_LOAD_TIMEOUT)