2020-03-01 09:42:20

by Tomer Maimon

[permalink] [raw]
Subject: [PATCH v1 1/4] dt-binding: watchdog: add restart priority documentation

Add device tree restart priority documentation.

Signed-off-by: Tomer Maimon <[email protected]>
---
Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
index 6d593003c933..0a0f86a25eb0 100644
--- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
@@ -17,6 +17,7 @@ Required clocking property, have to be one of:

Optional properties:
- timeout-sec : Contains the watchdog timeout in seconds
+- nuvoton,restart-priority : Contains the card restart priority.

Example:

@@ -25,4 +26,5 @@ timer@f000801c {
interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
reg = <0xf000801c 0x4>;
clocks = <&clk NPCM7XX_CLK_TIMER>;
+ nuvoton,restart-priority = <155>;
};
--
2.22.0


2020-03-01 10:06:28

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v1 1/4] dt-binding: watchdog: add restart priority documentation

On 3/1/20 1:40 AM, Tomer Maimon wrote:
> Add device tree restart priority documentation.
>

I think this warrants an explanation _why_ this is needed.
What is the use case ? Not just theory, please.

Guenter

> Signed-off-by: Tomer Maimon <[email protected]>
> ---
> Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
> index 6d593003c933..0a0f86a25eb0 100644
> --- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
> @@ -17,6 +17,7 @@ Required clocking property, have to be one of:
>
> Optional properties:
> - timeout-sec : Contains the watchdog timeout in seconds
> +- nuvoton,restart-priority : Contains the card restart priority.
>
> Example:
>
> @@ -25,4 +26,5 @@ timer@f000801c {
> interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
> reg = <0xf000801c 0x4>;
> clocks = <&clk NPCM7XX_CLK_TIMER>;
> + nuvoton,restart-priority = <155>;
> };
>

2020-03-01 15:48:55

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v1 1/4] dt-binding: watchdog: add restart priority documentation

On 3/1/20 7:36 AM, Tomer Maimon wrote:
>
>
> On Sun, 1 Mar 2020 at 12:06, Guenter Roeck <[email protected] <mailto:[email protected]>> wrote:
>
> On 3/1/20 1:40 AM, Tomer Maimon wrote:
> > Add device tree restart priority documentation.
> >
>
> I think this warrants an explanation _why_ this is needed.
> What is the use case ? Not just theory, please.
>
>
> In the NPCM750 there is two initiated restarts:
>
> * Software reset
> * WD reset
>
> the Software restart found at NPCM reset driver
> https://github.com/torvalds/linux/blob/master/drivers/reset/reset-npcm.c
>
> In NPCM WD driver the restart is configure as well, I will like to add the priority so the user will have maximum flexibility if he using both restarts
>

This is not the intended use case for restart priority. It is not
intended to be user configurable. The idea is that the more thorough
restart gets higher priority. This is implied by the restart method,
not by user preferences.

Also, the idea behind supporting multiple means to reset the system
is to be able to support multiple means to restart, some of which
may not always be available. In that situation, the priority means,
and is supposed to mean, "pick the best restart method available".
Again, that is determined by system design, and not supposed to
be configurable by the user.

On top of that, a watchdog driver based reset is almost always
a reset of last resort, to be chosen only if nothing else is available
in a given system. The existence of the reset driver confirms that
this is not different for this driver/chip.

Guenter