From: Elad Nachman <[email protected]>
Add support for Marvell ac5/x variant of the ARM
sbsa global watchdog. This watchdog deviates from
the standard driver by the following items:
1. Registers reside in secure register section.
hence access is only possible via SMC calls to ATF.
2. There are couple more registers which reside in
other register areas, which needs to be configured
in order for the watchdog to properly generate
reset through the SOC.
The new Marvell compatibility string differentiates between
the original sbsa mode of operation and the Marvell mode of
operation.
Elad Nachman (3):
dt-bindings: watchdog: add Marvell AC5 watchdog
arm64: dts: ac5: add watchdog nodes
watchdog: sbsa_gwdt: add support for Marvell ac5
.../bindings/watchdog/arm,sbsa-gwdt.yaml | 52 +++-
arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 +
arch/arm64/boot/dts/marvell/ac5-98dx35xx.dtsi | 8 +
drivers/watchdog/sbsa_gwdt.c | 247 ++++++++++++++++--
4 files changed, 298 insertions(+), 23 deletions(-)
--
2.25.1
From: Elad Nachman <[email protected]>
Add definitions and examples for Marvell AC5 variant
of the sbsa watchdog.
Marvell variant requires more memory definitions,
since the initialization is more complex, and involves
several register sets.
Signed-off-by: Elad Nachman <[email protected]>
---
.../bindings/watchdog/arm,sbsa-gwdt.yaml | 52 ++++++++++++++++++-
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml b/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml
index aa804f96acba..331e9aa7c2f7 100644
--- a/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml
@@ -20,12 +20,17 @@ allOf:
properties:
compatible:
- const: arm,sbsa-gwdt
+ enum:
+ - arm,sbsa-gwdt
+ - marvell,ac5-wd
reg:
items:
- description: Watchdog control frame
- description: Refresh frame
+ - description: Marvell CPU control frame
+ - description: Marvell Management frame
+ - description: Marvell reset control unit frame
interrupts:
description: The Watchdog Signal 0 (WS0) SPI (Shared Peripheral Interrupt)
@@ -39,12 +44,55 @@ required:
unevaluatedProperties: false
examples:
+ # First example is for generic ARM one
+ # Next examples are for Marvell.
+ # They are organized as three sets:
+ # first set is for global watchdog, then CPU core #0 private watchdog,
+ # and finally CPU core #1 private watchdog
+ # Examples are given for AC5 or Ironman. For AC5X SOC, the last
+ # reg item's low address (0x840F8000) should be replaced with 0x944F8000
- |
watchdog@2a440000 {
compatible = "arm,sbsa-gwdt";
reg = <0x2a440000 0x1000>,
- <0x2a450000 0x1000>;
+ <0x2a450000 0x1000>,
+ <0x0 0x0>,
+ <0x0 0x0>,
+ <0x0 0x0>;
interrupts = <0 27 4>;
timeout-sec = <30>;
};
+ - |
+ watchdog@80216000 {
+ compatible = "marvell,ac5-wd";
+ reg = <0x80216000 0x1000>,
+ <0x80215000 0x1000>,
+ <0x80210000 0x1000>,
+ <0x7f900000 0x1000>,
+ <0x840F8000 0x1000>;
+ interrupts = <0 124 4>;
+ timeout-sec = <30>;
+ };
+ - |
+ watchdog@80212000 {
+ compatible = "marvell,ac5-wd";
+ reg = <0x80212000 0x1000>,
+ <0x80211000 0x1000>,
+ <0x80210000 0x1000>,
+ <0x7f900000 0x1000>,
+ <0x840F8000 0x1000>;
+ interrupts = <0 122 4>;
+ timeout-sec = <30>;
+ };
+ - |
+ watchdog@80214000 {
+ compatible = "marvell,ac5-wd";
+ reg = <0x80214000 0x1000>,
+ <0x80213000 0x1000>,
+ <0x80210000 0x1000>,
+ <0x7f900000 0x1000>,
+ <0x840F8000 0x1000>;
+ interrupts = <0 122 4>;
+ timeout-sec = <30>;
+ };
...
--
2.25.1
On 14/12/2023 16:04, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> Add definitions and examples for Marvell AC5 variant
> of the sbsa watchdog.
> Marvell variant requires more memory definitions,
> since the initialization is more complex, and involves
> several register sets.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
> Signed-off-by: Elad Nachman <[email protected]>
> ---
> .../bindings/watchdog/arm,sbsa-gwdt.yaml | 52 ++++++++++++++++++-
> 1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml b/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml
> index aa804f96acba..331e9aa7c2f7 100644
> --- a/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/arm,sbsa-gwdt.yaml
> @@ -20,12 +20,17 @@ allOf:
>
> properties:
> compatible:
> - const: arm,sbsa-gwdt
> + enum:
> + - arm,sbsa-gwdt
> + - marvell,ac5-wd
>
> reg:
> items:
> - description: Watchdog control frame
> - description: Refresh frame
> + - description: Marvell CPU control frame
> + - description: Marvell Management frame
> + - description: Marvell reset control unit frame
You just broke all the users... I doubt this was tested on ARM platforms.
>
> interrupts:
> description: The Watchdog Signal 0 (WS0) SPI (Shared Peripheral Interrupt)
> @@ -39,12 +44,55 @@ required:
> unevaluatedProperties: false
>
> examples:
> + # First example is for generic ARM one
> + # Next examples are for Marvell.
One new example could be enough... but if it differs with one property,
also not that much of benefit.
> + # They are organized as three sets:
> + # first set is for global watchdog, then CPU core #0 private watchdog,
> + # and finally CPU core #1 private watchdog
> + # Examples are given for AC5 or Ironman. For AC5X SOC, the last
> + # reg item's low address (0x840F8000) should be replaced with 0x944F8000
> - |
> watchdog@2a440000 {
> compatible = "arm,sbsa-gwdt";
> reg = <0x2a440000 0x1000>,
> - <0x2a450000 0x1000>;
> + <0x2a450000 0x1000>,
> + <0x0 0x0>,
> + <0x0 0x0>,
> + <0x0 0x0>;
No, drop.
> interrupts = <0 27 4>;
> timeout-sec = <30>;
> };
> + - |
> + watchdog@80216000 {
> + compatible = "marvell,ac5-wd";
> + reg = <0x80216000 0x1000>,
> + <0x80215000 0x1000>,
> + <0x80210000 0x1000>,
> + <0x7f900000 0x1000>,
> + <0x840F8000 0x1000>;
> + interrupts = <0 124 4>;
Use proper defines.
> + timeout-sec = <30>;
> + };
> + - |
> + watchdog@80212000 {
Drop example.
> + compatible = "marvell,ac5-wd";
> + reg = <0x80212000 0x1000>,
> + <0x80211000 0x1000>,
> + <0x80210000 0x1000>,
> + <0x7f900000 0x1000>,
> + <0x840F8000 0x1000>;
> + interrupts = <0 122 4>;
> + timeout-sec = <30>;
> + };
> + - |
> + watchdog@80214000 {
Drop example.
Best regards,
Krzysztof
On 15/12/23 04:04, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> Add support for Marvell ac5/x variant of the ARM
> sbsa global watchdog. This watchdog deviates from
> the standard driver by the following items:
>
> 1. Registers reside in secure register section.
> hence access is only possible via SMC calls to ATF.
>
> 2. There are couple more registers which reside in
> other register areas, which needs to be configured
> in order for the watchdog to properly generate
> reset through the SOC.
>
> The new Marvell compatibility string differentiates between
> the original sbsa mode of operation and the Marvell mode of
> operation.
I gave this a quick try on our AC5X based board and it worked well with
both action=0/action=1
> Elad Nachman (3):
> dt-bindings: watchdog: add Marvell AC5 watchdog
> arm64: dts: ac5: add watchdog nodes
> watchdog: sbsa_gwdt: add support for Marvell ac5
>
> .../bindings/watchdog/arm,sbsa-gwdt.yaml | 52 +++-
> arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 +
> arch/arm64/boot/dts/marvell/ac5-98dx35xx.dtsi | 8 +
> drivers/watchdog/sbsa_gwdt.c | 247 ++++++++++++++++--
> 4 files changed, 298 insertions(+), 23 deletions(-)
>
On Thu, Dec 14, 2023 at 05:04:11PM +0200, Elad Nachman wrote:
> From: Elad Nachman <[email protected]>
>
> Add support for Marvell ac5/x variant of the ARM
> sbsa global watchdog. This watchdog deviates from
> the standard driver by the following items:
>
> 1. Registers reside in secure register section.
> hence access is only possible via SMC calls to ATF.
Oops.
> 2. There are couple more registers which reside in
> other register areas, which needs to be configured
> in order for the watchdog to properly generate
> reset through the SOC.
Your firmware should configure these.
>
> The new Marvell compatibility string differentiates between
> the original sbsa mode of operation and the Marvell mode of
> operation.
>
>
> Elad Nachman (3):
> dt-bindings: watchdog: add Marvell AC5 watchdog
> arm64: dts: ac5: add watchdog nodes
> watchdog: sbsa_gwdt: add support for Marvell ac5
>
> .../bindings/watchdog/arm,sbsa-gwdt.yaml | 52 +++-
> arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 +
> arch/arm64/boot/dts/marvell/ac5-98dx35xx.dtsi | 8 +
> drivers/watchdog/sbsa_gwdt.c | 247 ++++++++++++++++--
> 4 files changed, 298 insertions(+), 23 deletions(-)
>
> --
> 2.25.1
>