[PATCH 0/2] watchdog: Pretimeout support for Armada-XP

[PATCH 0/2] watchdog: Pretimeout support for Armada-XP

[Chris Packham]

This series extends and enables support for using timer1 as a pretimeout for
the watchdog which allows for getting some additional debug out before the
watchdog triggers a system reset.

Chris Packham (2):
  ARM: dts: armada-xp: add interrupts for watchdog
  watchdog: orion_wdt: support pretimeout on Armada-XP

 arch/arm/boot/dts/armada-xp-98dx3236.dtsi |  1 +
 arch/arm/boot/dts/armada-xp.dtsi          |  1 +
 drivers/watchdog/orion_wdt.c              | 13 +++++++++----
 3 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.35.1

[PATCH 1/2] ARM: dts: armada-xp: add interrupts for watchdog

[Chris Packham]

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>
---
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 1 +
 arch/arm/boot/dts/armada-xp.dtsi          | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index 38a052a0312d..0e561dfc0ca9 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -286,6 +286,7 @@ &watchdog {
 	compatible = "marvell,armada-xp-wdt";
 	clocks = <&coreclk 2>, <&refclk>;
 	clock-names = "nbclk", "fixed";
+	interrupts = <93>, <38>;
 };
 
 &cpurst {
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index 6c19984d668e..4297482da62f 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -260,6 +260,7 @@ &watchdog {
 	compatible = "marvell,armada-xp-wdt";
 	clocks = <&coreclk 2>, <&refclk>;
 	clock-names = "nbclk", "fixed";
+	interrupts = <93>, <38>;
 };
 
 &cpurst {
-- 
2.35.1

[PATCH 2/2] watchdog: orion_wdt: support pretimeout on Armada-XP

[Chris Packham]

Commit e07a4c79ca75 ("watchdog: orion_wdt: use timer1 as a pretimeout")
added support for a pretimeout on Armada-38x variants. Because the
Armada-XP variants use armada370_start/armada370_stop (due to missing an
explicit RSTOUT mask bit for the watchdog). Add the required pretimeout
support to armada370_start/armada370_stop for Armada-XP.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/watchdog/orion_wdt.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 127eefc9161d..e25e6bf4647f 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -238,8 +238,10 @@ 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);
+	reg = dev->data->wdt_enable_bit;
+	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
+		reg |= TIMER1_ENABLE_BIT;
+	atomic_io_modify(dev->reg + TIMER_CTRL, reg, reg);
 
 	/* Enable reset on watchdog */
 	reg = readl(dev->rstout);
@@ -312,7 +314,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);
@@ -320,7 +322,10 @@ 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;
+	if (wdt_dev->info->options & WDIOF_PRETIMEOUT)
+		mask |= TIMER1_ENABLE_BIT;
+	atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
 
 	return 0;
 }
-- 
2.35.1

Re: [PATCH 2/2] watchdog: orion_wdt: support pretimeout on Armada-XP

[Gregory CLEMENT]

Chris Packham <chris.packham@alliedtelesis.co.nz> writes:

> Commit e07a4c79ca75 ("watchdog: orion_wdt: use timer1 as a pretimeout")
> added support for a pretimeout on Armada-38x variants. Because the
> Armada-XP variants use armada370_start/armada370_stop (due to missing an
> explicit RSTOUT mask bit for the watchdog). Add the required pretimeout
> support to armada370_start/armada370_stop for Armada-XP.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Thanks,

Gregory

> ---
>  drivers/watchdog/orion_wdt.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 127eefc9161d..e25e6bf4647f 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -238,8 +238,10 @@ 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);
> +	reg = dev->data->wdt_enable_bit;
> +	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
> +		reg |= TIMER1_ENABLE_BIT;
> +	atomic_io_modify(dev->reg + TIMER_CTRL, reg, reg);
>  
>  	/* Enable reset on watchdog */
>  	reg = readl(dev->rstout);
> @@ -312,7 +314,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);
> @@ -320,7 +322,10 @@ 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;
> +	if (wdt_dev->info->options & WDIOF_PRETIMEOUT)
> +		mask |= TIMER1_ENABLE_BIT;
> +	atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
>  
>  	return 0;
>  }
> -- 
> 2.35.1
>

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

Re: [PATCH 1/2] ARM: dts: armada-xp: add interrupts for watchdog

[Gregory CLEMENT]

Hello 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>

Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>

To keep bisectability this patch should be merged after the driver
patch.

Thanks,

Gregory

> ---
>  arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 1 +
>  arch/arm/boot/dts/armada-xp.dtsi          | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> index 38a052a0312d..0e561dfc0ca9 100644
> --- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> +++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> @@ -286,6 +286,7 @@ &watchdog {
>  	compatible = "marvell,armada-xp-wdt";
>  	clocks = <&coreclk 2>, <&refclk>;
>  	clock-names = "nbclk", "fixed";
> +	interrupts = <93>, <38>;
>  };
>  
>  &cpurst {
> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
> index 6c19984d668e..4297482da62f 100644
> --- a/arch/arm/boot/dts/armada-xp.dtsi
> +++ b/arch/arm/boot/dts/armada-xp.dtsi
> @@ -260,6 +260,7 @@ &watchdog {
>  	compatible = "marvell,armada-xp-wdt";
>  	clocks = <&coreclk 2>, <&refclk>;
>  	clock-names = "nbclk", "fixed";
> +	interrupts = <93>, <38>;
>  };
>  
>  &cpurst {
> -- 
> 2.35.1
>

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

Re: [PATCH 2/2] watchdog: orion_wdt: support pretimeout on Armada-XP

[Guenter Roeck]

On Fri, Feb 11, 2022 at 01:32:57PM +1300, Chris Packham wrote:
> Commit e07a4c79ca75 ("watchdog: orion_wdt: use timer1 as a pretimeout")
> added support for a pretimeout on Armada-38x variants. Because the
> Armada-XP variants use armada370_start/armada370_stop (due to missing an
> explicit RSTOUT mask bit for the watchdog). Add the required pretimeout
> support to armada370_start/armada370_stop for Armada-XP.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/orion_wdt.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 127eefc9161d..e25e6bf4647f 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -238,8 +238,10 @@ 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);
> +	reg = dev->data->wdt_enable_bit;
> +	if (dev->wdt.info->options & WDIOF_PRETIMEOUT)
> +		reg |= TIMER1_ENABLE_BIT;
> +	atomic_io_modify(dev->reg + TIMER_CTRL, reg, reg);
>  
>  	/* Enable reset on watchdog */
>  	reg = readl(dev->rstout);
> @@ -312,7 +314,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);
> @@ -320,7 +322,10 @@ 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;
> +	if (wdt_dev->info->options & WDIOF_PRETIMEOUT)
> +		mask |= TIMER1_ENABLE_BIT;
> +	atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
>  
>  	return 0;
>  }

Re: [PATCH 1/2] ARM: dts: armada-xp: add interrupts for watchdog

[Guenter Roeck]

On Fri, Feb 11, 2022 at 01:32:56PM +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>
> Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 1 +
>  arch/arm/boot/dts/armada-xp.dtsi          | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> index 38a052a0312d..0e561dfc0ca9 100644
> --- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> +++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> @@ -286,6 +286,7 @@ &watchdog {
>  	compatible = "marvell,armada-xp-wdt";
>  	clocks = <&coreclk 2>, <&refclk>;
>  	clock-names = "nbclk", "fixed";
> +	interrupts = <93>, <38>;
>  };
>  
>  &cpurst {
> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
> index 6c19984d668e..4297482da62f 100644
> --- a/arch/arm/boot/dts/armada-xp.dtsi
> +++ b/arch/arm/boot/dts/armada-xp.dtsi
> @@ -260,6 +260,7 @@ &watchdog {
>  	compatible = "marvell,armada-xp-wdt";
>  	clocks = <&coreclk 2>, <&refclk>;
>  	clock-names = "nbclk", "fixed";
> +	interrupts = <93>, <38>;
>  };
>  
>  &cpurst {

Re: [PATCH 1/2] ARM: dts: armada-xp: add interrupts for watchdog

[Chris Packham]

Hi Gregory,

On 15/02/22 04:39, Gregory CLEMENT wrote:
> Hello 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>
> Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>
> To keep bisectability this patch should be merged after the driver
> patch.
>
> Thanks,
>
> Gregory

The driver changes were merged a while back. Looks like your intention 
was for this to go in via the watchdog tree but that never happened. 
Could you take it through your tree now? Probably won't be until 6.2 now 
but that's fine.

>
>> ---
>>   arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 1 +
>>   arch/arm/boot/dts/armada-xp.dtsi          | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
>> index 38a052a0312d..0e561dfc0ca9 100644
>> --- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
>> +++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
>> @@ -286,6 +286,7 @@ &watchdog {
>>   	compatible = "marvell,armada-xp-wdt";
>>   	clocks = <&coreclk 2>, <&refclk>;
>>   	clock-names = "nbclk", "fixed";
>> +	interrupts = <93>, <38>;
>>   };
>>   
>>   &cpurst {
>> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
>> index 6c19984d668e..4297482da62f 100644
>> --- a/arch/arm/boot/dts/armada-xp.dtsi
>> +++ b/arch/arm/boot/dts/armada-xp.dtsi
>> @@ -260,6 +260,7 @@ &watchdog {
>>   	compatible = "marvell,armada-xp-wdt";
>>   	clocks = <&coreclk 2>, <&refclk>;
>>   	clock-names = "nbclk", "fixed";
>> +	interrupts = <93>, <38>;
>>   };
>>   
>>   &cpurst {
>> -- 
>> 2.35.1
>>

Re: [PATCH 1/2] ARM: dts: armada-xp: add interrupts for watchdog

[Guenter Roeck]

On 10/16/22 15:39, Chris Packham wrote:
> Hi Gregory,
> 
> On 15/02/22 04:39, Gregory CLEMENT wrote:
>> Hello 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>
>> Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>>
>> To keep bisectability this patch should be merged after the driver
>> patch.
>>
>> Thanks,
>>
>> Gregory
> 
> The driver changes were merged a while back. Looks like your intention
> was for this to go in via the watchdog tree but that never happened.
> Could you take it through your tree now? Probably won't be until 6.2 now
> but that's fine.
> 

We don't take any actual devicetree changes. Those need to be pushed through
architecture/platform trees. Anything else would create never ending conflicts
(and I strongly suspect that various maintainers would complain).

Guenter

Re: [PATCH 1/2] ARM: dts: armada-xp: add interrupts for watchdog

[Gregory CLEMENT]

Chris Packham <Chris.Packham@alliedtelesis.co.nz> writes:

> Hi Gregory,
>
> On 15/02/22 04:39, Gregory CLEMENT wrote:
>> Hello 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>
>> Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>>
>> To keep bisectability this patch should be merged after the driver
>> patch.
>>
>> Thanks,
>>
>> Gregory
>
> The driver changes were merged a while back. Looks like your intention 
> was for this to go in via the watchdog tree but that never happened. 
> Could you take it through your tree now? Probably won't be until 6.2 now 
> but that's fine.


Applied on mvebu/dt64

Thanks and sorry for having missing the fact that it was not applied.

Gregory


>
>>
>>> ---
>>>   arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 1 +
>>>   arch/arm/boot/dts/armada-xp.dtsi          | 1 +
>>>   2 files changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
>>> index 38a052a0312d..0e561dfc0ca9 100644
>>> --- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
>>> +++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
>>> @@ -286,6 +286,7 @@ &watchdog {
>>>   	compatible = "marvell,armada-xp-wdt";
>>>   	clocks = <&coreclk 2>, <&refclk>;
>>>   	clock-names = "nbclk", "fixed";
>>> +	interrupts = <93>, <38>;
>>>   };
>>>   
>>>   &cpurst {
>>> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
>>> index 6c19984d668e..4297482da62f 100644
>>> --- a/arch/arm/boot/dts/armada-xp.dtsi
>>> +++ b/arch/arm/boot/dts/armada-xp.dtsi
>>> @@ -260,6 +260,7 @@ &watchdog {
>>>   	compatible = "marvell,armada-xp-wdt";
>>>   	clocks = <&coreclk 2>, <&refclk>;
>>>   	clock-names = "nbclk", "fixed";
>>> +	interrupts = <93>, <38>;
>>>   };
>>>   
>>>   &cpurst {
>>> -- 
>>> 2.35.1
>>>

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