* [PATCH 0/2] watchdog: orion_wdt: add pretimeout support
[not found] <20190227230707.GA28635@roeck-us.net>
@ 2019-03-04 22:51 ` Chris Packham
2019-03-04 22:51 ` [PATCH 1/2] watchdog: orion_wdt: remove orion_wdt_set_timeout Chris Packham
2019-03-04 22:51 ` [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham
0 siblings, 2 replies; 8+ messages in thread
From: Chris Packham @ 2019-03-04 22:51 UTC (permalink / raw)
To: linux, andrew, gregory.clement, jason
Cc: Chris Packham, linux-watchdog, linux-arm-kernel, linux-kernel
This is a follow up from https://lore.kernel.org/lkml/20190227230707.GA28635@roeck-us.net/
It appears that the kernel has only ever used timer0 on the
orion/kirkwood/armada platforms so timer1 appears to be spare on all of
these. Armada-XP and Armada-385 also have timers 2 and 3 which could be
used in a similar way if it is deemed that timer1 should be kept for
some other purpose.
Chris Packham (2):
watchdog: orion: remove orion_wdt_set_timeout
watchdog: orion_wdt: use timer1 as a pretimeout
arch/arm/boot/dts/armada-38x.dtsi | 1 +
drivers/watchdog/orion_wdt.c | 66 ++++++++++++++++++-------------
2 files changed, 39 insertions(+), 28 deletions(-)
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] watchdog: orion_wdt: remove orion_wdt_set_timeout
2019-03-04 22:51 ` [PATCH 0/2] watchdog: orion_wdt: add pretimeout support Chris Packham
@ 2019-03-04 22:51 ` Chris Packham
2019-03-05 17:17 ` Guenter Roeck
2019-03-04 22:51 ` [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham
1 sibling, 1 reply; 8+ messages in thread
From: Chris Packham @ 2019-03-04 22:51 UTC (permalink / raw)
To: linux, andrew, gregory.clement, jason
Cc: Wim Van Sebroeck, Chris Packham, linux-watchdog, linux-arm-kernel,
linux-kernel
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>
---
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout
2019-03-04 22:51 ` [PATCH 0/2] watchdog: orion_wdt: add pretimeout support Chris Packham
2019-03-04 22:51 ` [PATCH 1/2] watchdog: orion_wdt: remove orion_wdt_set_timeout Chris Packham
@ 2019-03-04 22:51 ` Chris Packham
2019-03-05 0:57 ` Andrew Lunn
1 sibling, 1 reply; 8+ messages in thread
From: Chris Packham @ 2019-03-04 22:51 UTC (permalink / raw)
To: linux, andrew, gregory.clement, jason
Cc: Mark Rutland, devicetree, linux-watchdog, linux-kernel,
Rob Herring, Chris Packham, Wim Van Sebroeck, linux-arm-kernel,
Sebastian Hesselbarth
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).
All of the orion based CPU cores (at least back as far as Kirkwood) 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>
---
arch/arm/boot/dts/armada-38x.dtsi | 1 +
drivers/watchdog/orion_wdt.c | 58 ++++++++++++++++++++-----------
2 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 929459c42760..fd0caa9714f2 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -376,6 +376,7 @@
reg = <0x20300 0x34>, <0x20704 0x4>, <0x18260 0x4>;
clocks = <&coreclk 2>, <&refclk>;
clock-names = "nbclk", "fixed";
+ interrupts-extended = <&gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
};
cpurst: cpurst@20800 {
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 8b259c712c52..bf1dc75c2045 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -46,6 +46,10 @@
#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)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int heartbeat = -1; /* module parameter (seconds) */
@@ -118,6 +122,7 @@ static int armada375_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)) {
@@ -127,9 +132,8 @@ static int armada375_wdt_clock_init(struct platform_device *pdev,
return ret;
}
- 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;
@@ -158,6 +162,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 +174,46 @@ 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);
+ 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 (wdt_dev->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 | TIMER1_ENABLE_BIT;
+ atomic_io_modify(dev->reg + TIMER_CTRL, val, val);
/* Enable reset on watchdog */
reg = readl(dev->rstout);
@@ -214,7 +227,7 @@ static int armada375_start(struct watchdog_device *wdt_dev)
static int armada370_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,
@@ -224,8 +237,8 @@ static int armada370_start(struct watchdog_device *wdt_dev)
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 | TIMER1_ENABLE_BIT;
+ atomic_io_modify(dev->reg + TIMER_CTRL, val, val);
/* Enable reset on watchdog */
reg = readl(dev->rstout);
@@ -237,14 +250,15 @@ static int armada370_start(struct watchdog_device *wdt_dev)
static int orion_start(struct watchdog_device *wdt_dev)
{
struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
+ u32 val;
/* Set watchdog duration */
writel(dev->clk_rate * wdt_dev->timeout,
dev->reg + dev->data->wdt_counter_offset);
/* 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 | TIMER1_ENABLE_BIT;
+ atomic_io_modify(dev->reg + TIMER_CTRL, val, val);
/* Enable reset on watchdog */
atomic_io_modify(dev->rstout, dev->data->rstout_enable_bit,
@@ -264,12 +278,14 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
static int orion_stop(struct watchdog_device *wdt_dev)
{
struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
+ u32 mask;
/* Disable reset on watchdog */
atomic_io_modify(dev->rstout, dev->data->rstout_enable_bit, 0);
/* Disable watchdog timer */
- atomic_io_modify(dev->reg + TIMER_CTRL, dev->data->wdt_enable_bit, 0);
+ mask = dev->data->wdt_enable_bit | TIMER1_ENABLE_BIT;
+ atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
return 0;
}
@@ -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,8 @@ 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 | TIMER1_ENABLE_BIT;
+ atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
return 0;
}
@@ -295,7 +312,7 @@ static int armada375_stop(struct watchdog_device *wdt_dev)
static int armada370_stop(struct watchdog_device *wdt_dev)
{
struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
- u32 reg;
+ u32 reg, mask;
/* Disable reset on watchdog */
reg = readl(dev->rstout);
@@ -303,7 +320,8 @@ static int armada370_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 | TIMER1_ENABLE_BIT;
+ atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
return 0;
}
@@ -350,7 +368,7 @@ static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
}
static const struct watchdog_info orion_wdt_info = {
- .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | WDIOF_PRETIMEOUT,
.identity = "Orion Watchdog",
};
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout
2019-03-04 22:51 ` [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham
@ 2019-03-05 0:57 ` Andrew Lunn
2019-03-05 1:26 ` Chris Packham
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2019-03-05 0:57 UTC (permalink / raw)
To: Chris Packham
Cc: Mark Rutland, devicetree, Wim Van Sebroeck, jason,
gregory.clement, linux-kernel, Rob Herring, linux,
Sebastian Hesselbarth, linux-arm-kernel, linux-watchdog
On Tue, Mar 05, 2019 at 11:51:52AM +1300, Chris Packham wrote:
> 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).
>
> All of the orion based CPU cores (at least back as far as Kirkwood) 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.
Hi Chris
I had a quick look at other drivers implementing pre-timeout. They
seem to call watchdog_notify_pretimeout(). I don't see that here? What
happens when timer1 fires?
> @@ -169,38 +174,46 @@ 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);
> +
> +
One blank line is sufficient,
> return 0;
> }
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout
2019-03-05 0:57 ` Andrew Lunn
@ 2019-03-05 1:26 ` Chris Packham
2019-03-05 2:09 ` Andrew Lunn
2019-03-05 17:27 ` Guenter Roeck
0 siblings, 2 replies; 8+ messages in thread
From: Chris Packham @ 2019-03-05 1:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Mark Rutland, devicetree@vger.kernel.org, Wim Van Sebroeck,
jason@lakedaemon.net, gregory.clement@bootlin.com,
linux-kernel@vger.kernel.org, Rob Herring, linux@roeck-us.net,
Sebastian Hesselbarth, linux-arm-kernel@lists.infradead.org,
linux-watchdog@vger.kernel.org
On 5/03/19 1:57 PM, Andrew Lunn wrote:
> On Tue, Mar 05, 2019 at 11:51:52AM +1300, Chris Packham wrote:
>> 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).
>>
>> All of the orion based CPU cores (at least back as far as Kirkwood) have
>> spare timers that aren't currently used by the Linux kernel.
Actually this appears to be incorrect Kirkwood does configure timer1 as
a clockevent timer. So I can't just grab timer1 for all platforms.
>> 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.
>
> Hi Chris
>
> I had a quick look at other drivers implementing pre-timeout. They
> seem to call watchdog_notify_pretimeout(). I don't see that here? What
> happens when timer1 fires?
>
It invokes the regular orion_wdt_irq(). On Armada-385 prior to this
change the irq was not specified because the reset always kicked in so
there was no point.
For correctness I could make the devicetree binding specify 2
interrupts. One for the regular watchdog interrupt (which would never
usually get hit because the reset would kick in) and one for the
pretimeout/timer1.
>> @@ -169,38 +174,46 @@ 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);
>> +
>> +
>
> One blank line is sufficient,
>
>
>> return 0;
>> }
>
> Andrew
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout
2019-03-05 1:26 ` Chris Packham
@ 2019-03-05 2:09 ` Andrew Lunn
2019-03-05 17:27 ` Guenter Roeck
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2019-03-05 2:09 UTC (permalink / raw)
To: Chris Packham
Cc: Mark Rutland, devicetree@vger.kernel.org, Wim Van Sebroeck,
jason@lakedaemon.net, gregory.clement@bootlin.com,
linux-kernel@vger.kernel.org, Rob Herring, linux@roeck-us.net,
Sebastian Hesselbarth, linux-arm-kernel@lists.infradead.org,
linux-watchdog@vger.kernel.org
> > Hi Chris
> >
> > I had a quick look at other drivers implementing pre-timeout. They
> > seem to call watchdog_notify_pretimeout(). I don't see that here? What
> > happens when timer1 fires?
> >
>
> It invokes the regular orion_wdt_irq(). On Armada-385 prior to this
> change the irq was not specified because the reset always kicked in so
> there was no point.
>
> For correctness I could make the devicetree binding specify 2
> interrupts. One for the regular watchdog interrupt (which would never
> usually get hit because the reset would kick in) and one for the
> pretimeout/timer1.
Hi Chris
If the regular watchdog interrupt would never actually fire because
the SoC gets reset, maybe make the IRQ handler call
watchdog_notify_pretimeout()?
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] watchdog: orion_wdt: remove orion_wdt_set_timeout
2019-03-04 22:51 ` [PATCH 1/2] watchdog: orion_wdt: remove orion_wdt_set_timeout Chris Packham
@ 2019-03-05 17:17 ` Guenter Roeck
0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2019-03-05 17:17 UTC (permalink / raw)
To: Chris Packham
Cc: andrew, jason, gregory.clement, linux-kernel, Wim Van Sebroeck,
linux-arm-kernel, linux-watchdog
On Tue, Mar 05, 2019 at 11:51:51AM +1300, Chris Packham wrote:
> 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>
> ---
> 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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout
2019-03-05 1:26 ` Chris Packham
2019-03-05 2:09 ` Andrew Lunn
@ 2019-03-05 17:27 ` Guenter Roeck
1 sibling, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2019-03-05 17:27 UTC (permalink / raw)
To: Chris Packham
Cc: Mark Rutland, Andrew Lunn, Wim Van Sebroeck, jason@lakedaemon.net,
devicetree@vger.kernel.org, gregory.clement@bootlin.com,
linux-kernel@vger.kernel.org, Rob Herring, Sebastian Hesselbarth,
linux-arm-kernel@lists.infradead.org,
linux-watchdog@vger.kernel.org
On Tue, Mar 05, 2019 at 01:26:08AM +0000, Chris Packham wrote:
> On 5/03/19 1:57 PM, Andrew Lunn wrote:
> > On Tue, Mar 05, 2019 at 11:51:52AM +1300, Chris Packham wrote:
> >> 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).
> >>
> >> All of the orion based CPU cores (at least back as far as Kirkwood) have
> >> spare timers that aren't currently used by the Linux kernel.
>
> Actually this appears to be incorrect Kirkwood does configure timer1 as
> a clockevent timer. So I can't just grab timer1 for all platforms.
>
If you can't use it unconditionally, can you specify it (and use it)
as clock ?
> >> 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.
> >
> > Hi Chris
> >
> > I had a quick look at other drivers implementing pre-timeout. They
> > seem to call watchdog_notify_pretimeout(). I don't see that here? What
> > happens when timer1 fires?
> >
>
> It invokes the regular orion_wdt_irq(). On Armada-385 prior to this
> change the irq was not specified because the reset always kicked in so
> there was no point.
>
I would suggest to update that function to actually call
watchdog_notify_pretimeout() if a pretimeout is configured configured.
After all, we do want to support the infrastructure, and that includes
support for the various pretimeout governors (if enabled).
> For correctness I could make the devicetree binding specify 2
> interrupts. One for the regular watchdog interrupt (which would never
> usually get hit because the reset would kick in) and one for the
> pretimeout/timer1.
>
Yes, if they are different interrupts and orion_wdt_irq() is only supposed
to handle the real timeout.
Thanks,
Guenter
> >> @@ -169,38 +174,46 @@ 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);
> >> +
> >> +
> >
> > One blank line is sufficient,
> >
> >
> >> return 0;
> >> }
> >
> > Andrew
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-05 17:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190227230707.GA28635@roeck-us.net>
2019-03-04 22:51 ` [PATCH 0/2] watchdog: orion_wdt: add pretimeout support Chris Packham
2019-03-04 22:51 ` [PATCH 1/2] watchdog: orion_wdt: remove orion_wdt_set_timeout Chris Packham
2019-03-05 17:17 ` Guenter Roeck
2019-03-04 22:51 ` [PATCH 2/2] watchdog: orion_wdt: use timer1 as a pretimeout Chris Packham
2019-03-05 0:57 ` Andrew Lunn
2019-03-05 1:26 ` Chris Packham
2019-03-05 2:09 ` Andrew Lunn
2019-03-05 17:27 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox