Re: [PATCH v1 1/4] ulp_wdog: Update ulp wdog driver for 32bits command

[Stefan Roese]

On 09.10.22 10:00, Alice Guo (OSS) wrote:

From: Ye Li <ye...@nxp.com>

To use 32bits refresh and unlock command as default, check the CMD32EN
bit to select the corresponding commands.

Signed-off-by: Ye Li <ye...@nxp.com>
Signed-off-by: Alice Guo <alice....@nxp.com>
Reviewed-by: Peng Fan <peng....@nxp.com>
---
  drivers/watchdog/ulp_wdog.c | 52 +++++++++++++++++++++++++------------
  1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/ulp_wdog.c b/drivers/watchdog/ulp_wdog.c
index ecd35ef22a..c40e785d0a 100644
--- a/drivers/watchdog/ulp_wdog.c
+++ b/drivers/watchdog/ulp_wdog.c
@@ -28,11 +28,15 @@ struct wdog_regs {
  #define UNLOCK_WORD0 0xC520 /* 1st unlock word */
  #define UNLOCK_WORD1 0xD928 /* 2nd unlock word */

+#define UNLOCK_WORD 0xD928C520 /* unlock word */

+#define REFRESH_WORD 0xB480A602 /* refresh word */
+
  #define WDGCS_WDGE                      BIT(7)
  #define WDGCS_WDGUPDATE                 BIT(5)

#define WDGCS_RCS BIT(10)

  #define WDGCS_ULK                       BIT(11)
+#define WDGCS_CMD32EN                   BIT(13)
  #define WDGCS_FLG                       BIT(14)

#define WDG_BUS_CLK (0x0)

@@ -52,20 +56,30 @@ void hw_watchdog_reset(void)
  {
        struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE_ADDR;

- dmb();

-       __raw_writel(REFRESH_WORD0, &wdog->cnt);
-       __raw_writel(REFRESH_WORD1, &wdog->cnt);
-       dmb();
+       if (readl(&wdog->cs) & WDGCS_CMD32EN) {
+               writel(REFRESH_WORD, &wdog->cnt);
+       } else {
+               dmb();
+               __raw_writel(REFRESH_WORD0, &wdog->cnt);
+               __raw_writel(REFRESH_WORD1, &wdog->cnt);
+               dmb();
+       }
  }

void hw_watchdog_init(void)

  {
        struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE_ADDR;
-
-       dmb();
-       __raw_writel(UNLOCK_WORD0, &wdog->cnt);
-       __raw_writel(UNLOCK_WORD1, &wdog->cnt);
-       dmb();
+       u32 cmd32 = 0;
+
+       if (readl(&wdog->cs) & WDGCS_CMD32EN) {
+               writel(UNLOCK_WORD, &wdog->cnt);
+               cmd32 = WDGCS_CMD32EN;
+       } else {
+               dmb();
+               __raw_writel(UNLOCK_WORD0, &wdog->cnt);
+               __raw_writel(UNLOCK_WORD1, &wdog->cnt);
+               dmb();
+       }

/* Wait WDOG Unlock */

        while (!(readl(&wdog->cs) & WDGCS_ULK))
@@ -75,7 +89,7 @@ void hw_watchdog_init(void)
        writel(0, &wdog->win);

/* setting 1-kHz clock source, enable counter running, and clear interrupt */

-       writel((WDGCS_WDGE | WDGCS_WDGUPDATE |(WDG_LPO_CLK << 8) | WDGCS_FLG), 
&wdog->cs);
+       writel((cmd32 | WDGCS_WDGE | WDGCS_WDGUPDATE |(WDG_LPO_CLK << 8) | WDGCS_FLG), 
&wdog->cs);

I know you're just copying / re-structuring old code here, but could
you please also change this minor coding style issue:

WDGCS_WDGUPDATE |(WDG_LPO_CLK << 8)
->
WDGCS_WDGUPDATE | (WDG_LPO_CLK << 8)

Other than this:

Reviewed-by: Stefan Roese <s...@denx.de>

Thanks,
Stefan

/* Wait WDOG reconfiguration */

        while (!(readl(&wdog->cs) & WDGCS_RCS))
@@ -87,11 +101,17 @@ void hw_watchdog_init(void)
  void reset_cpu(void)
  {
        struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE_ADDR;
-
-       dmb();
-       __raw_writel(UNLOCK_WORD0, &wdog->cnt);
-       __raw_writel(UNLOCK_WORD1, &wdog->cnt);
-       dmb();
+       u32 cmd32 = 0;
+
+       if (readl(&wdog->cs) & WDGCS_CMD32EN) {
+               writel(UNLOCK_WORD, &wdog->cnt);
+               cmd32 = WDGCS_CMD32EN;
+       } else {
+               dmb();
+               __raw_writel(UNLOCK_WORD0, &wdog->cnt);
+               __raw_writel(UNLOCK_WORD1, &wdog->cnt);
+               dmb();
+       }

/* Wait WDOG Unlock */

        while (!(readl(&wdog->cs) & WDGCS_ULK))
@@ -101,7 +121,7 @@ void reset_cpu(void)
        writel(0, &wdog->win);

/* enable counter running */

-       writel((WDGCS_WDGE | (WDG_LPO_CLK << 8)), &wdog->cs);
+       writel((cmd32 | WDGCS_WDGE | (WDG_LPO_CLK << 8)), &wdog->cs);

/* Wait WDOG reconfiguration */

        while (!(readl(&wdog->cs) & WDGCS_RCS))

Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de