[PATCH] ARM: dts: at91: enable watchdog for SAMA5D3 Xplained

[PATCH] ARM: dts: at91: enable watchdog for SAMA5D3 Xplained

[Michael Opdenacker]

Like on the SAMA5D2 and SAMA5D4 Xplained boards,
enable the watchdog device on the SAMA5D3 Xplained board.

As explained on drivers/watchdog/at91sam9_wdt.c,
for the watchdog driver to work in Linux, you need to make sure
that the bootstrap / bootloader doesn't disable the watchdog,
as the Watchdog Timer Mode Register can be only written to once.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
---
 arch/arm/boot/dts/at91-sama5d3_xplained.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
index d72c042f2850..440eccc9eb38 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -79,6 +79,10 @@ timer1: timer@1 {
 				};
 			};
 
+			watchdog: watchdog@fffffe40 {
+				status = "okay";
+			};
+
 			i2c0: i2c@f0014000 {
 				pinctrl-0 = <&pinctrl_i2c0_pu>;
 				status = "okay";
-- 
2.25.1

Re: [PATCH] ARM: dts: at91: enable watchdog for SAMA5D3 Xplained

[Nicolas Ferre]

On 09/12/2021 at 16:45, Michael Opdenacker wrote:
> Like on the SAMA5D2 and SAMA5D4 Xplained boards,
> enable the watchdog device on the SAMA5D3 Xplained board.
> 
> As explained on drivers/watchdog/at91sam9_wdt.c,
> for the watchdog driver to work in Linux, you need to make sure
> that the bootstrap / bootloader doesn't disable the watchdog,
> as the Watchdog Timer Mode Register can be only written to once.
> 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>

Hi Michael,

Thanks for your patch.

Even if I understand the need for alignment with other sama5 SoCs, I'm 
not planning to take this patch to enable the watchdog by default.

As you highlight, this older watchdog, compared to the ones on sama5d4 
or sama5d2, cannot be re-enabled.
On our usual prebuilt demos and configurations, that don't have watchdog 
support by default, enabling it triggers the following errors:
at91_wdt fffffe40.watchdog: watchdog is disabled
at91_wdt: probe of fffffe40.watchdog failed with error -22

I prefer to let the user enable the watchdog explicitly, on the whole 
chain of components for its use-case and make sure to "pet" it correctly.

Best regards,
   Nicolas

> ---
>   arch/arm/boot/dts/at91-sama5d3_xplained.dts | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
> index d72c042f2850..440eccc9eb38 100644
> --- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
> +++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
> @@ -79,6 +79,10 @@ timer1: timer@1 {
>                                  };
>                          };
> 
> +                       watchdog: watchdog@fffffe40 {
> +                               status = "okay";
> +                       };
> +
>                          i2c0: i2c@f0014000 {
>                                  pinctrl-0 = <&pinctrl_i2c0_pu>;
>                                  status = "okay";
> --
> 2.25.1
> 


-- 
Nicolas Ferre

Re: [PATCH] ARM: dts: at91: enable watchdog for SAMA5D3 Xplained

[Michael Opdenacker]

Hi Nicolas,

On 12/13/21 4:48 PM, Nicolas Ferre wrote:
> On 09/12/2021 at 16:45, Michael Opdenacker wrote:
>> Like on the SAMA5D2 and SAMA5D4 Xplained boards,
>> enable the watchdog device on the SAMA5D3 Xplained board.
>>
>> As explained on drivers/watchdog/at91sam9_wdt.c,
>> for the watchdog driver to work in Linux, you need to make sure
>> that the bootstrap / bootloader doesn't disable the watchdog,
>> as the Watchdog Timer Mode Register can be only written to once.
>>
>> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
>
> Hi Michael,
>
> Thanks for your patch.
>
> Even if I understand the need for alignment with other sama5 SoCs, I'm
> not planning to take this patch to enable the watchdog by default.
>
> As you highlight, this older watchdog, compared to the ones on sama5d4
> or sama5d2, cannot be re-enabled.
> On our usual prebuilt demos and configurations, that don't have
> watchdog support by default, enabling it triggers the following errors:
> at91_wdt fffffe40.watchdog: watchdog is disabled
> at91_wdt: probe of fffffe40.watchdog failed with error -22
>
> I prefer to let the user enable the watchdog explicitly, on the whole
> chain of components for its use-case and make sure to "pet" it correctly.


Thanks for your review. I understand. It's your call anyway.
At least, this is documented through this discussion in the mailing list
archives :)
Cheers
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com