IronPort-SDR: AVUwnS3oZZh7vLD3iu65UD6twSY5iwrBrY6jLD2u2q5hGxQQpO2jPo9yiARUofFx8Ci4va1I92
 KIb8/2wf+cvDzZfwXpVBhV2GGLHRM9WPEpOo40z/7XEg6fpkIe5vDOUjJSp3ED6I+1gPqdPJ+F
 +d2yZ65Ay6gadn/XXr4CzjTH8ElSnQqNewRBT7jOaynBOLFoaF2Q3IaJg5jKaI18WoCjKIVM/b
 fD4D1QAomtoiO0GSS4Cq1H9wv519l0bFh20SLY/Azjya0gFy4rMjKPG8YyhLEPbyOmYoG4eaeq
 n60cRx6ohXCoyo9p33d5DEP5
Subject: Re: [PATCH] ARM: dts: at91: enable watchdog for SAMA5D3 Xplained
To:     Michael Opdenacker <michael.opdenacker@bootlin.com>,
        <robh+dt@kernel.org>, <alexandre.belloni@bootlin.com>,
        <ludovic.desroches@microchip.com>
CC:     <devicetree@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>
References: <20211209154540.6391-1-michael.opdenacker@bootlin.com>
From:   Nicolas Ferre <nicolas.ferre@microchip.com>
Organization: microchip
Message-ID: <9f01d0c7-b5c3-1353-8cc6-ed797e466706@microchip.com>
Date:   Mon, 13 Dec 2021 16:48:20 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <20211209154540.6391-1-michael.opdenacker@bootlin.com>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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