diff mbox series

[v2,2/7] watchdog: imx7ulp: Add explict memory barrier for unlock sequence

Message ID 20220825083256.14565-3-alice.guo@oss.nxp.com
State New
Headers show
Series watchdog: imx7ulp_wdt: update i.MX7ULP WDOG timer driver | expand

Commit Message

Alice Guo (OSS) Aug. 25, 2022, 8:32 a.m. UTC 
From: Jacky Bai <ping.bai@nxp.com>

When reconfiguring the WDOG Timer of i.MX7ULP, there is a certain
probability causes it to reset. The reason is that the CMD32EN of the
WDOG Timer of i.MX7ULP is disabled in bootloader. The unlock sequence
are two 16-bit writes to the CNT register within 16 bus clocks. Adding
mb() is to guarantee that two 16-bit writes are finished within 16 bus
clocks. Memory barriers cannot be added between these two 16-bit writes
so that writel_relaxed is used.

Suggested-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
---

Changes for v2:
 - add the reason why memory barriers are added for unlock sequence in commit log

 drivers/watchdog/imx7ulp_wdt.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Guenter Roeck Sept. 25, 2022, 5:37 p.m. UTC | #1 
On Thu, Aug 25, 2022 at 04:32:51PM +0800, Alice Guo (OSS) wrote:
> From: Jacky Bai <ping.bai@nxp.com>
> 
> When reconfiguring the WDOG Timer of i.MX7ULP, there is a certain
> probability causes it to reset. The reason is that the CMD32EN of the
> WDOG Timer of i.MX7ULP is disabled in bootloader. The unlock sequence
> are two 16-bit writes to the CNT register within 16 bus clocks. Adding
> mb() is to guarantee that two 16-bit writes are finished within 16 bus
> clocks. Memory barriers cannot be added between these two 16-bit writes
> so that writel_relaxed is used.
> 
> Suggested-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Ye Li <ye.li@nxp.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changes for v2:
>  - add the reason why memory barriers are added for unlock sequence in commit log
> 
>  drivers/watchdog/imx7ulp_wdt.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index 014f497ea0dc..b8ac0cb04d2f 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -179,9 +179,13 @@ static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
>  	int ret;
>  
>  	local_irq_disable();
> +
> +	mb();
>  	/* unlock the wdog for reconfiguration */
>  	writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
>  	writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> +	mb();
> +
>  	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
>  	if (ret)
>  		goto init_out;
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index 014f497ea0dc..b8ac0cb04d2f 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -179,9 +179,13 @@  static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
 	int ret;
 
 	local_irq_disable();
+
+	mb();
 	/* unlock the wdog for reconfiguration */
 	writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
 	writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
+	mb();
+
 	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
 	if (ret)
 		goto init_out;