| Message ID | 20160119065313.GA22443@vireshk |
|---|---|
| State | New |
| Headers | show |
On 18-01-16, 23:11, Guenter Roeck wrote: > On 01/18/2016 10:53 PM, Viresh Kumar wrote: > >On 19-01-16, 11:34, Sandeep Tripathy wrote: > >> sp805 wdt asserts interrupt for the first expiry and > > > >Why an extra space before sp805 ? > > > >>reloads the counter. If wdt interrupt is set and count > >>reaches zero then wdt reset event is generated.To get > > > >Add space after full-stop. > > > >>wdt reset at t timeout the driver loads wdt counter > > > >Keep t and t/2 in single quotes, its hardly readable otherwise. > > > >>with t/2. A ping before t second *should* prevent wdt > >>reset. Currently if ping is done after t/2 sec then > >>wdt interrupt condition gets set. On the next countdown > >>of loadval wdt reset event occurs eventhough wdt was > >>reloaded before the set timeout t. > >> > >> This patch clears the interrupt condition on ping. > > > >Why extra spaces before 'This'? > > > >> > >>Signed-off-by: Sandeep Tripathy <tripathy@broadcom.com> > >>--- > >> drivers/watchdog/sp805_wdt.c | 5 ++--- > >> 1 file changed, 2 insertions(+), 3 deletions(-) > >> > >>diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c > >>index 01d8162..e7a715e 100644 > >>--- a/drivers/watchdog/sp805_wdt.c > >>+++ b/drivers/watchdog/sp805_wdt.c > >>@@ -139,12 +139,11 @@ static int wdt_config(struct watchdog_device *wdd, bool ping) > >> > >> writel_relaxed(UNLOCK, wdt->base + WDTLOCK); > >> writel_relaxed(wdt->load_val, wdt->base + WDTLOAD); > >>+ writel_relaxed(INT_MASK, wdt->base + WDTINTCLR); > >> > >>- if (!ping) { > >>- writel_relaxed(INT_MASK, wdt->base + WDTINTCLR); > >>+ if (!ping) > >> writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + > >> WDTCONTROL); > >>- } > >> > >> writel_relaxed(LOCK, wdt->base + WDTLOCK); > > > >If I read the TRM properly, then you don't need to load the WDTLOAD > >register on ping and so your change can be written as: > > > >diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c > >index 01d816251302..42e1ec86532d 100644 > >--- a/drivers/watchdog/sp805_wdt.c > >+++ b/drivers/watchdog/sp805_wdt.c > >@@ -138,10 +138,10 @@ static int wdt_config(struct watchdog_device *wdd, bool ping) > > spin_lock(&wdt->lock); > > > > writel_relaxed(UNLOCK, wdt->base + WDTLOCK); > >- writel_relaxed(wdt->load_val, wdt->base + WDTLOAD); > >+ writel_relaxed(INT_MASK, wdt->base + WDTINTCLR); > > > > if (!ping) { > >- writel_relaxed(INT_MASK, wdt->base + WDTINTCLR); > >+ writel_relaxed(wdt->load_val, wdt->base + WDTLOAD); > > writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + > > WDTCONTROL); > > } > > > > That would require additional changes. Reason is that the load value is not immediately > written after a timeout change, but only with the next ping (which happens immediately > afterwards). With your proposed change, the load value would never be updated. > > Besides, that would be a separate change, and would require a separate patch. Hmm, maybe then we can keep the original patch as is :) @Sandeep: Please resend your patch after fixing the commit log and not the diff. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index 01d816251302..42e1ec86532d 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -138,10 +138,10 @@ static int wdt_config(struct watchdog_device *wdd, bool ping) spin_lock(&wdt->lock); writel_relaxed(UNLOCK, wdt->base + WDTLOCK); - writel_relaxed(wdt->load_val, wdt->base + WDTLOAD); + writel_relaxed(INT_MASK, wdt->base + WDTINTCLR); if (!ping) { - writel_relaxed(INT_MASK, wdt->base + WDTINTCLR); + writel_relaxed(wdt->load_val, wdt->base + WDTLOAD); writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL); }