mbox series

[0/3] watchdog: sbsa_gwdt: add support for Marvell ac5

Message ID 20231214150414.1849058-1-enachman@marvell.com
Headers show
Series watchdog: sbsa_gwdt: add support for Marvell ac5 | expand

Message

Elad Nachman Dec. 14, 2023, 3:04 p.m. UTC
From: Elad Nachman <enachman@marvell.com>

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(-)

Comments

Krzysztof Kozlowski Dec. 14, 2023, 3:15 p.m. UTC | #1
On 14/12/2023 16:04, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add watchdog nodes to ac5 and ac5x device tree files
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 14 ++++++++++++++
>  arch/arm64/boot/dts/marvell/ac5-98dx35xx.dtsi |  8 ++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> index b5e042b8e929..e898c6bd31f0 100644
> --- a/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> @@ -307,6 +307,20 @@ nand: nand-controller@805b0000 {
>  			status = "disabled";
>  		};
>  
> +/*
> + * Global Watchdog:
> + */

Messed indentation. Also unnecessary line breaks around comment, unless
you have some KPI per lines of code. If it is the only watchdog, why
even commenting on it?

> +		watchdog: watchdog@80216000 {
> +			compatible = "marvell,ac5-wd";
> +			reg = <0x0 0x80216000 0 0x1000>,
> +			      <0x0 0x80215000 0 0x1000>,
> +			      <0x0 0x80210000 0 0x1000>,
> +			      <0x0 0x7f900000 0 0x1000>,
> +			      <0x0 0x840F8000 0 0x1000>;

Lowercase hex.



Best regards,
Krzysztof
Chris Packham Dec. 15, 2023, 4:21 a.m. UTC | #2
On 15/12/23 04:04, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
>
> 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(-)
>
Rob Herring (Arm) Dec. 15, 2023, 5:48 p.m. UTC | #3
On Thu, Dec 14, 2023 at 05:04:11PM +0200, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> 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
>