linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] watchdog: orion_wdt: add pretimeout support
@ 2019-03-05 20:19 Chris Packham
  2019-03-05 20:19 ` [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog Chris Packham
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chris Packham @ 2019-03-05 20:19 UTC (permalink / raw)
  To: jason, andrew, gregory.clement, linux
  Cc: linux-watchdog, linux-arm-kernel, devicetree, linux-kernel,
	Chris Packham

It appears that the kernel has only ever used timer0 on the armada platforms so
timer1 appears to be spare on these. Timers 2 and 3 are also free which could
be used if it is deemed that timer1 should be kept for some other purpose.

Chris Packham (3):
  ARM: dts: armada-38x: add interrupts for watchdog
  watchdog: orion: remove orion_wdt_set_timeout
  watchdog: orion_wdt: use timer1 as a pretimeout

Changes in v2:

I've reduced this to just touching the Armada-38x. It would probably
also work on Armada-XP just by adding the appropriate interrupt nodes
to the dts. I don't have access to the Armada-370 datasheet but it'd
likely work there as well..

 arch/arm/boot/dts/armada-38x.dtsi |  2 +
 drivers/watchdog/orion_wdt.c      | 67 +++++++++++++++++++++++--------
 2 files changed, 52 insertions(+), 17 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog
  2019-03-05 20:19 [PATCH v2 0/3] watchdog: orion_wdt: add pretimeout support Chris Packham
@ 2019-03-05 20:19 ` Chris Packham
  2019-03-05 21:30   ` Guenter Roeck
  2019-04-21 17:24   ` Gregory CLEMENT
  2019-03-05 20:19 ` [PATCH v2 2/3] watchdog: orion: remove orion_wdt_set_timeout Chris Packham
  2019-03-05 20:19 ` [PATCH v2 3/3] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham
  2 siblings, 2 replies; 6+ messages in thread
From: Chris Packham @ 2019-03-05 20:19 UTC (permalink / raw)
  To: jason, andrew, gregory.clement, linux
  Cc: linux-watchdog, linux-arm-kernel, devicetree, linux-kernel,
	Chris Packham, Sebastian Hesselbarth, Rob Herring, Mark Rutland

The first interrupt is for the regular watchdog timeout. Normally the
RSTOUT line will trigger a reset before this interrupt fires but on
systems with a non-standard reset it may still trigger.

The second interrupt is for a timer1 which is used as a pre-timeout for
the watchdog.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- new, split out from "watchdog: orion_wdt: use timer1 as a pretimeout"

 arch/arm/boot/dts/armada-38x.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 929459c42760..fc550c640ca8 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -376,6 +376,8 @@
 				reg = <0x20300 0x34>, <0x20704 0x4>, <0x18260 0x4>;
 				clocks = <&coreclk 2>, <&refclk>;
 				clock-names = "nbclk", "fixed";
+				interrupts-extended = <&gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
+						      <&gic GIC_SPI  9 IRQ_TYPE_LEVEL_HIGH>;
 			};
 
 			cpurst: cpurst@20800 {
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] watchdog: orion: remove orion_wdt_set_timeout
  2019-03-05 20:19 [PATCH v2 0/3] watchdog: orion_wdt: add pretimeout support Chris Packham
  2019-03-05 20:19 ` [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog Chris Packham
@ 2019-03-05 20:19 ` Chris Packham
  2019-03-05 20:19 ` [PATCH v2 3/3] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham
  2 siblings, 0 replies; 6+ messages in thread
From: Chris Packham @ 2019-03-05 20:19 UTC (permalink / raw)
  To: jason, andrew, gregory.clement, linux
  Cc: linux-watchdog, linux-arm-kernel, devicetree, linux-kernel,
	Chris Packham, Wim Van Sebroeck

The watchdog core will do the same thing if no set_timeout
is supplied so we can safely remove orion_wdt_set_timeout.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Changes in v2:
- Add Guenter's review

 drivers/watchdog/orion_wdt.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 9db3b09f7568..8b259c712c52 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -349,13 +349,6 @@ static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
 	return readl(dev->reg + dev->data->wdt_counter_offset) / dev->clk_rate;
 }
 
-static int orion_wdt_set_timeout(struct watchdog_device *wdt_dev,
-				 unsigned int timeout)
-{
-	wdt_dev->timeout = timeout;
-	return 0;
-}
-
 static const struct watchdog_info orion_wdt_info = {
 	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
 	.identity = "Orion Watchdog",
@@ -366,7 +359,6 @@ static const struct watchdog_ops orion_wdt_ops = {
 	.start = orion_wdt_start,
 	.stop = orion_wdt_stop,
 	.ping = orion_wdt_ping,
-	.set_timeout = orion_wdt_set_timeout,
 	.get_timeleft = orion_wdt_get_timeleft,
 };
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] watchdog: orion_wdt: use timer1 as a pretimeout
  2019-03-05 20:19 [PATCH v2 0/3] watchdog: orion_wdt: add pretimeout support Chris Packham
  2019-03-05 20:19 ` [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog Chris Packham
  2019-03-05 20:19 ` [PATCH v2 2/3] watchdog: orion: remove orion_wdt_set_timeout Chris Packham
@ 2019-03-05 20:19 ` Chris Packham
  2 siblings, 0 replies; 6+ messages in thread
From: Chris Packham @ 2019-03-05 20:19 UTC (permalink / raw)
  To: jason, andrew, gregory.clement, linux
  Cc: linux-watchdog, linux-arm-kernel, devicetree, linux-kernel,
	Chris Packham, Wim Van Sebroeck

The orion watchdog can either reset the CPU or generate an interrupt.
The interrupt would be useful for debugging as it provides panic()
output about the watchdog expiry, however if the interrupt is used the
watchdog can't reset the CPU in the event of being stuck in a loop with
interrupts disabled or if the CPU is prevented from accessing memory
(e.g. an unterminated DMA).

The Armada SoCs have spare timers that aren't currently used by the
Linux kernel. We can use timer1 to provide a pre-timeout ahead of the
watchdog timer and provide the possibility of gathering debug before the
reset triggers.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- apply changes to armada-38x only
- use watchdog_notify_pretimeout() as suggested by Andrew and Guenter

 drivers/watchdog/orion_wdt.c | 59 ++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 8b259c712c52..aa295f37e563 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -46,6 +46,11 @@
 #define WDT_AXP_FIXED_ENABLE_BIT BIT(10)
 #define WDT_A370_EXPIRED	BIT(31)
 
+#define TIMER1_VAL_OFF		0x001c
+#define TIMER1_ENABLE_BIT	BIT(2)
+#define TIMER1_FIXED_ENABLE_BIT	BIT(12)
+#define TIMER1_STATUS_BIT	BIT(8)
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static int heartbeat = -1;		/* module parameter (seconds) */
 
@@ -158,6 +163,7 @@ static int armadaxp_wdt_clock_init(struct platform_device *pdev,
 				   struct orion_watchdog *dev)
 {
 	int ret;
+	u32 val;
 
 	dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
 	if (IS_ERR(dev->clk))
@@ -169,38 +175,48 @@ static int armadaxp_wdt_clock_init(struct platform_device *pdev,
 	}
 
 	/* Enable the fixed watchdog clock input */
-	atomic_io_modify(dev->reg + TIMER_CTRL,
-			 WDT_AXP_FIXED_ENABLE_BIT,
-			 WDT_AXP_FIXED_ENABLE_BIT);
+	val = WDT_AXP_FIXED_ENABLE_BIT | TIMER1_FIXED_ENABLE_BIT;
+	atomic_io_modify(dev->reg + TIMER_CTRL, val, val);
 
 	dev->clk_rate = clk_get_rate(dev->clk);
+
 	return 0;
 }
 
 static int orion_wdt_ping(struct watchdog_device *wdt_dev)
 {
 	struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
+
 	/* Reload watchdog duration */
 	writel(dev->clk_rate * wdt_dev->timeout,
 	       dev->reg + dev->data->wdt_counter_offset);
+	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
+		writel(dev->clk_rate * (wdt_dev->timeout - wdt_dev->pretimeout),
+		       dev->reg + TIMER1_VAL_OFF);
+
 	return 0;
 }
 
 static int armada375_start(struct watchdog_device *wdt_dev)
 {
 	struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
-	u32 reg;
+	u32 reg, val;
 
 	/* Set watchdog duration */
 	writel(dev->clk_rate * wdt_dev->timeout,
 	       dev->reg + dev->data->wdt_counter_offset);
+	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
+		writel(dev->clk_rate * (wdt_dev->timeout - wdt_dev->pretimeout),
+		       dev->reg + TIMER1_VAL_OFF);
 
 	/* Clear the watchdog expiration bit */
 	atomic_io_modify(dev->reg + TIMER_A370_STATUS, WDT_A370_EXPIRED, 0);
 
 	/* Enable watchdog timer */
-	atomic_io_modify(dev->reg + TIMER_CTRL, dev->data->wdt_enable_bit,
-						dev->data->wdt_enable_bit);
+	val = dev->data->wdt_enable_bit;
+	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
+		val |= TIMER1_ENABLE_BIT;
+	atomic_io_modify(dev->reg + TIMER_CTRL, val, val);
 
 	/* Enable reset on watchdog */
 	reg = readl(dev->rstout);
@@ -277,7 +293,7 @@ static int orion_stop(struct watchdog_device *wdt_dev)
 static int armada375_stop(struct watchdog_device *wdt_dev)
 {
 	struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
-	u32 reg;
+	u32 reg, mask;
 
 	/* Disable reset on watchdog */
 	atomic_io_modify(dev->rstout_mask, dev->data->rstout_mask_bit,
@@ -287,7 +303,10 @@ static int armada375_stop(struct watchdog_device *wdt_dev)
 	writel(reg, dev->rstout);
 
 	/* Disable watchdog timer */
-	atomic_io_modify(dev->reg + TIMER_CTRL, dev->data->wdt_enable_bit, 0);
+	mask = dev->data->wdt_enable_bit;
+	if (wdt_dev->info->options & WDIOF_PRETIMEOUT)
+		mask += TIMER1_ENABLE_BIT;
+	atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
 
 	return 0;
 }
@@ -349,7 +368,7 @@ static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
 	return readl(dev->reg + dev->data->wdt_counter_offset) / dev->clk_rate;
 }
 
-static const struct watchdog_info orion_wdt_info = {
+static struct watchdog_info orion_wdt_info = {
 	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
 	.identity = "Orion Watchdog",
 };
@@ -368,6 +387,16 @@ static irqreturn_t orion_wdt_irq(int irq, void *devid)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t orion_wdt_pre_irq(int irq, void *devid)
+{
+	struct orion_watchdog *dev = devid;
+
+	atomic_io_modify(dev->reg + TIMER_A370_STATUS,
+			 TIMER1_STATUS_BIT, 0);
+	watchdog_notify_pretimeout(&dev->wdt);
+	return IRQ_HANDLED;
+}
+
 /*
  * The original devicetree binding for this driver specified only
  * one memory resource, so in order to keep DT backwards compatibility
@@ -591,6 +620,18 @@ static int orion_wdt_probe(struct platform_device *pdev)
 		}
 	}
 
+	irq = platform_get_irq(pdev, 1);
+	if (irq > 0) {
+		orion_wdt_info.options |= WDIOF_PRETIMEOUT;
+		ret = devm_request_irq(&pdev->dev, irq, orion_wdt_pre_irq,
+				       0, pdev->name, dev);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to request IRQ\n");
+			goto disable_clk;
+		}
+	}
+
+
 	watchdog_set_nowayout(&dev->wdt, nowayout);
 	ret = watchdog_register_device(&dev->wdt);
 	if (ret)
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog
  2019-03-05 20:19 ` [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog Chris Packham
@ 2019-03-05 21:30   ` Guenter Roeck
  2019-04-21 17:24   ` Gregory CLEMENT
  1 sibling, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2019-03-05 21:30 UTC (permalink / raw)
  To: Chris Packham
  Cc: jason, andrew, gregory.clement, linux-watchdog, linux-arm-kernel,
	devicetree, linux-kernel, Sebastian Hesselbarth, Rob Herring,
	Mark Rutland

On Wed, Mar 06, 2019 at 09:19:22AM +1300, Chris Packham wrote:
> The first interrupt is for the regular watchdog timeout. Normally the
> RSTOUT line will trigger a reset before this interrupt fires but on
> systems with a non-standard reset it may still trigger.
> 
> The second interrupt is for a timer1 which is used as a pre-timeout for
> the watchdog.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Changes in v2:
> - new, split out from "watchdog: orion_wdt: use timer1 as a pretimeout"
> 
>  arch/arm/boot/dts/armada-38x.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> index 929459c42760..fc550c640ca8 100644
> --- a/arch/arm/boot/dts/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> @@ -376,6 +376,8 @@
>  				reg = <0x20300 0x34>, <0x20704 0x4>, <0x18260 0x4>;
>  				clocks = <&coreclk 2>, <&refclk>;
>  				clock-names = "nbclk", "fixed";
> +				interrupts-extended = <&gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> +						      <&gic GIC_SPI  9 IRQ_TYPE_LEVEL_HIGH>;

This will need to be documented, possibly including the use of
interrupts-extended (currently only interrupts is documented in
Documentation/devicetree/bindings/watchdog/marvel.txt).

Also, how would it be handled if the primary interrupt is not specified ?
After all, it is optional.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog
  2019-03-05 20:19 ` [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog Chris Packham
  2019-03-05 21:30   ` Guenter Roeck
@ 2019-04-21 17:24   ` Gregory CLEMENT
  1 sibling, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2019-04-21 17:24 UTC (permalink / raw)
  To: Chris Packham, jason, andrew, linux
  Cc: linux-watchdog, linux-arm-kernel, devicetree, linux-kernel,
	Chris Packham, Sebastian Hesselbarth, Rob Herring, Mark Rutland

Hi Chris,

> The first interrupt is for the regular watchdog timeout. Normally the
> RSTOUT line will trigger a reset before this interrupt fires but on
> systems with a non-standard reset it may still trigger.
>
> The second interrupt is for a timer1 which is used as a pre-timeout for
> the watchdog.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Changes in v2:
> - new, split out from "watchdog: orion_wdt: use timer1 as a pretimeout"
>
>  arch/arm/boot/dts/armada-38x.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>

Applied on mvebu/dt

Thanks,

Gregory


> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> index 929459c42760..fc550c640ca8 100644
> --- a/arch/arm/boot/dts/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> @@ -376,6 +376,8 @@
>  				reg = <0x20300 0x34>, <0x20704 0x4>, <0x18260 0x4>;
>  				clocks = <&coreclk 2>, <&refclk>;
>  				clock-names = "nbclk", "fixed";
> +				interrupts-extended = <&gic GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> +						      <&gic GIC_SPI  9 IRQ_TYPE_LEVEL_HIGH>;
>  			};
>  
>  			cpurst: cpurst@20800 {
> -- 
> 2.21.0
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-04-21 17:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-05 20:19 [PATCH v2 0/3] watchdog: orion_wdt: add pretimeout support Chris Packham
2019-03-05 20:19 ` [PATCH v2 1/3] ARM: dts: armada-38x: add interrupts for watchdog Chris Packham
2019-03-05 21:30   ` Guenter Roeck
2019-04-21 17:24   ` Gregory CLEMENT
2019-03-05 20:19 ` [PATCH v2 2/3] watchdog: orion: remove orion_wdt_set_timeout Chris Packham
2019-03-05 20:19 ` [PATCH v2 3/3] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).