lkml.org 
[lkml]   [2020]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 2/3] watchdog: sprd: change timeout value from 1000 to 2000
    On Mon, 26 Oct 2020 at 22:36, Guenter Roeck <linux@roeck-us.net> wrote:
    >
    > On 10/26/20 1:09 AM, Chunyan Zhang wrote:
    > > From: Lingling Xu <ling_ling.xu@unisoc.com>
    > >
    > > 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@unisoc.com>
    > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
    > > ---
    > > 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;
    > >
    >

    \
     
     \ /
      Last update: 2020-10-27 10:22    [W:3.551 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site