On Mon, 26 Oct 2020 at 22:36, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On 10/26/20 1:09 AM, Chunyan Zhang wrote: > > From: Lingling Xu <ling_ling.xu@xxxxxxxxxx> > > > > Because cpu_relax() takes different time on different SoCs, for some rare > > cases, it would take more than 1000 cycles for waitting load operation > > waiting Ok. > > > finished. The result of many times testing verified that changing the > > timeout value to 2000 can solve the issue. > > > > This is just a kludge that doesn't address the underlying problem. > As the wait loop states, "Waiting the load value operation done, > it needs two or three RTC clock cycles". This means the loop > should wait for a maximum number of clock cycles, and not run > as hot loop. If we assume that clk_get_rate() returns the clock > frequency, that frequency can be used to determine how long this > needs to be retried. It might also make sense - depending on how > long this actually takes - to use usleep_range() instead of > cpu_relax() to avoid the hot loop. Agree, using usleep_range() instead makes more sense, I will look into that. Thanks for your review. Chunyan > > Guenter > > > Fixes: 477603467009 ("watchdog: Add Spreadtrum watchdog driver") > > Signed-off-by: Lingling Xu <ling_ling.xu@xxxxxxxxxx> > > Signed-off-by: Chunyan Zhang <chunyan.zhang@xxxxxxxxxx> > > --- > > drivers/watchdog/sprd_wdt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c > > index f3c90b4afead..4f2a8c6d6485 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 2000 > > > > struct sprd_wdt { > > void __iomem *base; > > >