Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1735523 > unrolled thread

[PATCH v2 1/4] watchdog: aspeed: Retain watchdog enabled state

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 1/4] watchdog: aspeed: Retain watchdog enabled state Andrew Jeffery <andrew@aj.id.au> - 2017-09-20 07:40 +0200
    Re: [PATCH v2 1/4] watchdog: aspeed: Retain watchdog enabled state Joel Stanley <joel@jms.id.au> - 2017-09-20 08:10 +0200

#1735523 — [PATCH v2 1/4] watchdog: aspeed: Retain watchdog enabled state

An unintended post-condition of probe() is that the watchdog is
disabled. This behaviour was introduced by an unnecessary write to the
control register to configure the hardware based on the devicetree. The
write is unnecessary because the cached control value that is
manipulated by the code parsing the devicetree is eventually written by
aspeed_wdt_enable(), which is when we care how the control register
should be configured.

Remove the write to restore expected behaviour.

Fixes: b7f0b8ad25f3 ("drivers/watchdog: ASPEED reference dev tree properties for config")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/watchdog/aspeed_wdt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index 79cc766cd30f..6c6dd3f4c48d 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -243,9 +243,13 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
 	if (of_property_read_bool(np, "aspeed,external-signal"))
 		wdt->ctrl |= WDT_CTRL_WDT_EXT;
 
-	writel(wdt->ctrl, wdt->base + WDT_CTRL);
-
 	if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE)  {
+		/*
+		 * The watchdog is running, but invoke aspeed_wdt_start() to
+		 * write wdt->ctrl to WDT_CTRL to ensure the watchdog's
+		 * configuration conforms to the driver's expectations.
+		 * Primarily, ensure we're using the 1MHz clock source.
+		 */
 		aspeed_wdt_start(&wdt->wdd);
 		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
 	}
-- 
2.11.0

[toc] | [next] | [standalone]

#1735541

On Wed, Sep 20, 2017 at 3:00 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> An unintended post-condition of probe() is that the watchdog is
> disabled. This behaviour was introduced by an unnecessary write to the
> control register to configure the hardware based on the devicetree. The
> write is unnecessary because the cached control value that is
> manipulated by the code parsing the devicetree is eventually written by
> aspeed_wdt_enable(), which is when we care how the control register
> should be configured.
>
> Remove the write to restore expected behaviour.
>
> Fixes: b7f0b8ad25f3 ("drivers/watchdog: ASPEED reference dev tree properties for config")
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

Reviewed-by: Joel Stanley <joel@jms.id.au>

Thanks,

Joel

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web