diff mbox series

[4/4] watchdog: rzg2l_wdt: Fix Reboot failed message

Message ID 20211211212617.19639-4-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Headers show
Series [1/4] watchdog: rzg2l_wdt: Fix 32bit overflow issue | expand

Commit Message

Biju Das Dec. 11, 2021, 9:26 p.m. UTC
This patch fixes the message "Reboot failed -- System halted"
by triggering WDT reset by enabling force reset(WDTRSTB).

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/watchdog/rzg2l_wdt.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

Comments

Guenter Roeck Dec. 11, 2021, 9:40 p.m. UTC | #1
On 12/11/21 1:26 PM, Biju Das wrote:
> This patch fixes the message "Reboot failed -- System halted"
> by triggering WDT reset by enabling force reset(WDTRSTB).
> 

That is really misleading. The patch does not fix the message - it fixes
the reboot handler to make it actually execute the reboot.

Guenter

> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>   drivers/watchdog/rzg2l_wdt.c | 21 +++++++--------------
>   1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index c81b9dd05e63..497c86129f20 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -21,8 +21,11 @@
>   #define WDTSET		0x04
>   #define WDTTIM		0x08
>   #define WDTINT		0x0C
> +#define PECR		0x10
> +#define PEEN		0x14
>   #define WDTCNT_WDTEN	BIT(0)
>   #define WDTINT_INTDISP	BIT(0)
> +#define PEEN_FORCE_RST	BIT(0)
>   
>   #define WDT_DEFAULT_TIMEOUT		60U
>   
> @@ -130,22 +133,12 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>   			     unsigned long action, void *data)
>   {
>   	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> -	int ret;
>   
> -	/* Reset the module before we modify any register */
> -	ret = reset_control_reset(priv->rstc);
> -	if (ret) {
> -		dev_err(wdev->parent, "failed to reset");
> -		return ret;
> -	}
> +	/* Generate Reset (WDTRSTB) Signal */
> +	rzg2l_wdt_write(priv, 0, PECR);
>   
> -	pm_runtime_get_sync(wdev->parent);
> -
> -	/* smallest counter value to reboot soon */
> -	rzg2l_wdt_write(priv, WDTSET_COUNTER_VAL(1), WDTSET);
> -
> -	/* Enable watchdog timer*/
> -	rzg2l_wdt_write(priv, WDTCNT_WDTEN, WDTCNT);
> +	/* Force reset (WDTRSTB) */
> +	rzg2l_wdt_write(priv, PEEN_FORCE_RST, PEEN);
>   
>   	return 0;
>   }
>
Biju Das Dec. 12, 2021, 8:16 a.m. UTC | #2
Hi Guenter,

Thanks for the feedback.

> Subject: Re: [PATCH 4/4] watchdog: rzg2l_wdt: Fix Reboot failed message
> 
> On 12/11/21 1:26 PM, Biju Das wrote:
> > This patch fixes the message "Reboot failed -- System halted"
> > by triggering WDT reset by enabling force reset(WDTRSTB).
> >
> 
> That is really misleading. The patch does not fix the message - it fixes
> the reboot handler to make it actually execute the reboot.

There is some delay(~34 msec)in reboot handler, make this noisy message to appear in the kernel logs,
before the reboot handler to actually executing the reboot due to watchdog reset.

This issue is vanished by using new force reset method.

May be I will update the commit header and description as Use force reset method
for WDT reset.

Regards,
Biju


> 
> Guenter
> 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >   drivers/watchdog/rzg2l_wdt.c | 21 +++++++--------------
> >   1 file changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/watchdog/rzg2l_wdt.c
> > b/drivers/watchdog/rzg2l_wdt.c index c81b9dd05e63..497c86129f20 100644
> > --- a/drivers/watchdog/rzg2l_wdt.c
> > +++ b/drivers/watchdog/rzg2l_wdt.c
> > @@ -21,8 +21,11 @@
> >   #define WDTSET		0x04
> >   #define WDTTIM		0x08
> >   #define WDTINT		0x0C
> > +#define PECR		0x10
> > +#define PEEN		0x14
> >   #define WDTCNT_WDTEN	BIT(0)
> >   #define WDTINT_INTDISP	BIT(0)
> > +#define PEEN_FORCE_RST	BIT(0)
> >
> >   #define WDT_DEFAULT_TIMEOUT		60U
> >
> > @@ -130,22 +133,12 @@ static int rzg2l_wdt_restart(struct
> watchdog_device *wdev,
> >   			     unsigned long action, void *data)
> >   {
> >   	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > -	int ret;
> >
> > -	/* Reset the module before we modify any register */
> > -	ret = reset_control_reset(priv->rstc);
> > -	if (ret) {
> > -		dev_err(wdev->parent, "failed to reset");
> > -		return ret;
> > -	}
> > +	/* Generate Reset (WDTRSTB) Signal */
> > +	rzg2l_wdt_write(priv, 0, PECR);
> >
> > -	pm_runtime_get_sync(wdev->parent);
> > -
> > -	/* smallest counter value to reboot soon */
> > -	rzg2l_wdt_write(priv, WDTSET_COUNTER_VAL(1), WDTSET);
> > -
> > -	/* Enable watchdog timer*/
> > -	rzg2l_wdt_write(priv, WDTCNT_WDTEN, WDTCNT);
> > +	/* Force reset (WDTRSTB) */
> > +	rzg2l_wdt_write(priv, PEEN_FORCE_RST, PEEN);
> >
> >   	return 0;
> >   }
> >
diff mbox series

Patch

diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index c81b9dd05e63..497c86129f20 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -21,8 +21,11 @@ 
 #define WDTSET		0x04
 #define WDTTIM		0x08
 #define WDTINT		0x0C
+#define PECR		0x10
+#define PEEN		0x14
 #define WDTCNT_WDTEN	BIT(0)
 #define WDTINT_INTDISP	BIT(0)
+#define PEEN_FORCE_RST	BIT(0)
 
 #define WDT_DEFAULT_TIMEOUT		60U
 
@@ -130,22 +133,12 @@  static int rzg2l_wdt_restart(struct watchdog_device *wdev,
 			     unsigned long action, void *data)
 {
 	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
-	int ret;
 
-	/* Reset the module before we modify any register */
-	ret = reset_control_reset(priv->rstc);
-	if (ret) {
-		dev_err(wdev->parent, "failed to reset");
-		return ret;
-	}
+	/* Generate Reset (WDTRSTB) Signal */
+	rzg2l_wdt_write(priv, 0, PECR);
 
-	pm_runtime_get_sync(wdev->parent);
-
-	/* smallest counter value to reboot soon */
-	rzg2l_wdt_write(priv, WDTSET_COUNTER_VAL(1), WDTSET);
-
-	/* Enable watchdog timer*/
-	rzg2l_wdt_write(priv, WDTCNT_WDTEN, WDTCNT);
+	/* Force reset (WDTRSTB) */
+	rzg2l_wdt_write(priv, PEEN_FORCE_RST, PEEN);
 
 	return 0;
 }