mbox series

[v2,0/3] A few fixes to sprd watchdog driver

Message ID 20201029023933.24548-1-zhang.lyra@gmail.com
Headers show
Series A few fixes to sprd watchdog driver | expand

Message

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

A few issues about sprd watchdog driver were found recently, this
patchset would fix them.

Changes since v1:
* Added Reviewed-by from Guenter Roeck;
* Abandon original patch 2, add a new patch to use usleep_range() instead of busy loop;
* Revised the max times of loop, also revised the comments for checking busy bit;
* Revised commit message for the whole patchset;

Chunyan Zhang (1):
  watchdog: sprd: change to use usleep_range() instead of busy loop

Lingling Xu (2):
  watchdog: sprd: remove watchdog disable from resume fail path
  watchdog: sprd: check busy bit before new loading rather than after
    that

 drivers/watchdog/sprd_wdt.c | 42 ++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 22 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)

>