* [PATCH v2 1/4] watchdog: rzg2l_wdt: Fix 32bit overflow issue
@ 2021-12-13 15:26 Biju Das
2021-12-13 15:26 ` [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for reset_control_{deassert/reset} Biju Das
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Biju Das @ 2021-12-13 15:26 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc
The value of timer_cycle_us can be 0 due to 32bit overflow.
For eg:- If we assign the counter value "0xfff" for computing
maxval.
This patch fixes this issue by appending ULL to 1024, so that
it is promoted to 64bit.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V1->V2:
* Added Rb tag from Guenter and Geert.
---
drivers/watchdog/rzg2l_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 6b426df34fd6..96f2a018ab62 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -53,7 +53,7 @@ static void rzg2l_wdt_wait_delay(struct rzg2l_wdt_priv *priv)
static u32 rzg2l_wdt_get_cycle_usec(unsigned long cycle, u32 wdttime)
{
- u64 timer_cycle_us = 1024 * 1024 * (wdttime + 1) * MICRO;
+ u64 timer_cycle_us = 1024 * 1024ULL * (wdttime + 1) * MICRO;
return div64_ul(timer_cycle_us, cycle);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for reset_control_{deassert/reset}
2021-12-13 15:26 [PATCH v2 1/4] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
@ 2021-12-13 15:26 ` Biju Das
2022-01-04 16:36 ` Biju Das
2021-12-13 15:26 ` [PATCH v2 3/4] watchdog: rzg2l_wdt: Add set_timeout callback Biju Das
2021-12-13 15:26 ` [PATCH v2 4/4] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
2 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2021-12-13 15:26 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc
If reset_control_deassert() fails, then we won't be able to
access the device registers. Therefore check the return code of
reset_control_deassert() and bail out in case of error.
While at it remove the unnecessary pm_runtime_resume_and_get()
from probe(), as it turns on the clocks.
Replace reset_control_assert()->reset_control_reset() in rzg2l_wdt
_stop() and remove the unnecessary reset_control_deassert() from
rzg2l_wdt_start(). Also add error check for reset_control_reset()
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* Updated commit description and removed Rb tag from Guenter,
since there is code change
* Replaced reset_control_assert with reset_control_reset in stop
and removed reset_control_deassert() from start.
*
---
drivers/watchdog/rzg2l_wdt.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 96f2a018ab62..0e62d7be153c 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -86,7 +86,6 @@ static int rzg2l_wdt_start(struct watchdog_device *wdev)
{
struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
- reset_control_deassert(priv->rstc);
pm_runtime_get_sync(wdev->parent);
/* Initialize time out */
@@ -106,18 +105,24 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev)
struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
pm_runtime_put(wdev->parent);
- reset_control_assert(priv->rstc);
- return 0;
+ /* Reset the module for stopping watchdog */
+ return reset_control_reset(priv->rstc);
}
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 */
- reset_control_reset(priv->rstc);
+ ret = reset_control_reset(priv->rstc);
+ if (ret) {
+ dev_err(wdev->parent, "failed to reset");
+ return ret;
+ }
+
pm_runtime_get_sync(wdev->parent);
/* smallest counter value to reboot soon */
@@ -151,12 +156,11 @@ static const struct watchdog_ops rzg2l_wdt_ops = {
.restart = rzg2l_wdt_restart,
};
-static void rzg2l_wdt_reset_assert_pm_disable_put(void *data)
+static void rzg2l_wdt_reset_assert_pm_disable(void *data)
{
struct watchdog_device *wdev = data;
struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
- pm_runtime_put(wdev->parent);
pm_runtime_disable(wdev->parent);
reset_control_assert(priv->rstc);
}
@@ -204,13 +208,11 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc),
"failed to get cpg reset");
- reset_control_deassert(priv->rstc);
+ ret = reset_control_deassert(priv->rstc);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to deassert");
+
pm_runtime_enable(&pdev->dev);
- ret = pm_runtime_resume_and_get(&pdev->dev);
- if (ret < 0) {
- dev_err(dev, "pm_runtime_resume_and_get failed ret=%pe", ERR_PTR(ret));
- goto out_pm_get;
- }
priv->wdev.info = &rzg2l_wdt_ident;
priv->wdev.ops = &rzg2l_wdt_ops;
@@ -222,7 +224,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
watchdog_set_drvdata(&priv->wdev, priv);
ret = devm_add_action_or_reset(&pdev->dev,
- rzg2l_wdt_reset_assert_pm_disable_put,
+ rzg2l_wdt_reset_assert_pm_disable,
&priv->wdev);
if (ret < 0)
return ret;
@@ -235,12 +237,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
dev_warn(dev, "Specified timeout invalid, using default");
return devm_watchdog_register_device(&pdev->dev, &priv->wdev);
-
-out_pm_get:
- pm_runtime_disable(dev);
- reset_control_assert(priv->rstc);
-
- return ret;
}
static const struct of_device_id rzg2l_wdt_ids[] = {
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] watchdog: rzg2l_wdt: Add set_timeout callback
2021-12-13 15:26 [PATCH v2 1/4] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
2021-12-13 15:26 ` [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for reset_control_{deassert/reset} Biju Das
@ 2021-12-13 15:26 ` Biju Das
2021-12-13 15:26 ` [PATCH v2 4/4] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
2 siblings, 0 replies; 6+ messages in thread
From: Biju Das @ 2021-12-13 15:26 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc
Once watchdog is started, WDT cycle setting register(WDTSET) cannot be
changed. However we can reconfigure the new value for WDSET, after a
module reset. Otherwise it will ignore the writes and will hold the
previous value instead of the updated one.
This patch add support for set_timeout callback by doing module
reset, which allows us to update WDTSET register. Based on the
watchdog timer state, it may restart WDT with the modified values.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
V1->V2:
* Updated commit description
* Removed stop/start and started using reset() instead.
* After reset, Start WDT based on watchdog timer state.
---
drivers/watchdog/rzg2l_wdt.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 0e62d7be153c..d1b5cb70d56c 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -110,6 +110,33 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev)
return reset_control_reset(priv->rstc);
}
+static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int timeout)
+{
+ struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
+ int ret;
+
+ wdev->timeout = timeout;
+
+ /*
+ * We need to reset the module for updating WDTSET register
+ * If watchdog is active, then decrement the PM counter to make
+ * it balanced, after reset operation.
+ */
+ if (watchdog_active(wdev))
+ pm_runtime_put(wdev->parent);
+
+ /* Reset the module for updating WDTSET register */
+ ret = reset_control_reset(priv->rstc);
+ if (watchdog_active(wdev)) {
+ if (ret)
+ pm_runtime_get_sync(wdev->parent);
+ else
+ rzg2l_wdt_start(wdev);
+ }
+
+ return ret;
+}
+
static int rzg2l_wdt_restart(struct watchdog_device *wdev,
unsigned long action, void *data)
{
@@ -153,6 +180,7 @@ static const struct watchdog_ops rzg2l_wdt_ops = {
.start = rzg2l_wdt_start,
.stop = rzg2l_wdt_stop,
.ping = rzg2l_wdt_ping,
+ .set_timeout = rzg2l_wdt_set_timeout,
.restart = rzg2l_wdt_restart,
};
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] watchdog: rzg2l_wdt: Use force reset for WDT reset
2021-12-13 15:26 [PATCH v2 1/4] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
2021-12-13 15:26 ` [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for reset_control_{deassert/reset} Biju Das
2021-12-13 15:26 ` [PATCH v2 3/4] watchdog: rzg2l_wdt: Add set_timeout callback Biju Das
@ 2021-12-13 15:26 ` Biju Das
2022-01-04 16:38 ` Biju Das
2 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2021-12-13 15:26 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc
This patch uses the force reset(WDTRSTB) for triggering WDT reset
(WDTRSTB). This method is faster compared to the overflow method
for triggering watchdog reset.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
V1->V2:
* Updated the commit description.
---
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 d1b5cb70d56c..94f98825ab8d 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
@@ -141,22 +144,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;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for reset_control_{deassert/reset}
2021-12-13 15:26 ` [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for reset_control_{deassert/reset} Biju Das
@ 2022-01-04 16:36 ` Biju Das
0 siblings, 0 replies; 6+ messages in thread
From: Biju Das @ 2022-01-04 16:36 UTC (permalink / raw)
To: Biju Das, Wim Van Sebroeck, Guenter Roeck
Cc: linux-watchdog, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi All,
Gentle ping. Are we happy with this patch? Please let me know.
Regards,
Biju
> Subject: [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for
> reset_control_{deassert/reset}
>
> If reset_control_deassert() fails, then we won't be able to access the
> device registers. Therefore check the return code of
> reset_control_deassert() and bail out in case of error.
>
> While at it remove the unnecessary pm_runtime_resume_and_get() from
> probe(), as it turns on the clocks.
>
> Replace reset_control_assert()->reset_control_reset() in rzg2l_wdt
> _stop() and remove the unnecessary reset_control_deassert() from
> rzg2l_wdt_start(). Also add error check for reset_control_reset()
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
> * Updated commit description and removed Rb tag from Guenter,
> since there is code change
> * Replaced reset_control_assert with reset_control_reset in stop
> and removed reset_control_deassert() from start.
> *
> ---
> drivers/watchdog/rzg2l_wdt.c | 34 +++++++++++++++-------------------
> 1 file changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 96f2a018ab62..0e62d7be153c 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -86,7 +86,6 @@ static int rzg2l_wdt_start(struct watchdog_device *wdev)
> {
> struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
>
> - reset_control_deassert(priv->rstc);
> pm_runtime_get_sync(wdev->parent);
>
> /* Initialize time out */
> @@ -106,18 +105,24 @@ static int rzg2l_wdt_stop(struct watchdog_device
> *wdev)
> struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
>
> pm_runtime_put(wdev->parent);
> - reset_control_assert(priv->rstc);
>
> - return 0;
> + /* Reset the module for stopping watchdog */
> + return reset_control_reset(priv->rstc);
> }
>
> 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 */
> - reset_control_reset(priv->rstc);
> + ret = reset_control_reset(priv->rstc);
> + if (ret) {
> + dev_err(wdev->parent, "failed to reset");
> + return ret;
> + }
> +
> pm_runtime_get_sync(wdev->parent);
>
> /* smallest counter value to reboot soon */ @@ -151,12 +156,11 @@
> static const struct watchdog_ops rzg2l_wdt_ops = {
> .restart = rzg2l_wdt_restart,
> };
>
> -static void rzg2l_wdt_reset_assert_pm_disable_put(void *data)
> +static void rzg2l_wdt_reset_assert_pm_disable(void *data)
> {
> struct watchdog_device *wdev = data;
> struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
>
> - pm_runtime_put(wdev->parent);
> pm_runtime_disable(wdev->parent);
> reset_control_assert(priv->rstc);
> }
> @@ -204,13 +208,11 @@ static int rzg2l_wdt_probe(struct platform_device
> *pdev)
> return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc),
> "failed to get cpg reset");
>
> - reset_control_deassert(priv->rstc);
> + ret = reset_control_deassert(priv->rstc);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to deassert");
> +
> pm_runtime_enable(&pdev->dev);
> - ret = pm_runtime_resume_and_get(&pdev->dev);
> - if (ret < 0) {
> - dev_err(dev, "pm_runtime_resume_and_get failed ret=%pe",
> ERR_PTR(ret));
> - goto out_pm_get;
> - }
>
> priv->wdev.info = &rzg2l_wdt_ident;
> priv->wdev.ops = &rzg2l_wdt_ops;
> @@ -222,7 +224,7 @@ static int rzg2l_wdt_probe(struct platform_device
> *pdev)
>
> watchdog_set_drvdata(&priv->wdev, priv);
> ret = devm_add_action_or_reset(&pdev->dev,
> - rzg2l_wdt_reset_assert_pm_disable_put,
> + rzg2l_wdt_reset_assert_pm_disable,
> &priv->wdev);
> if (ret < 0)
> return ret;
> @@ -235,12 +237,6 @@ static int rzg2l_wdt_probe(struct platform_device
> *pdev)
> dev_warn(dev, "Specified timeout invalid, using default");
>
> return devm_watchdog_register_device(&pdev->dev, &priv->wdev);
> -
> -out_pm_get:
> - pm_runtime_disable(dev);
> - reset_control_assert(priv->rstc);
> -
> - return ret;
> }
>
> static const struct of_device_id rzg2l_wdt_ids[] = {
> --
> 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2 4/4] watchdog: rzg2l_wdt: Use force reset for WDT reset
2021-12-13 15:26 ` [PATCH v2 4/4] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
@ 2022-01-04 16:38 ` Biju Das
0 siblings, 0 replies; 6+ messages in thread
From: Biju Das @ 2022-01-04 16:38 UTC (permalink / raw)
To: Biju Das, Wim Van Sebroeck, Guenter Roeck
Cc: linux-watchdog, Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad, linux-renesas-soc
Hi All,
Gentle ping. Are we happy with this patch? Please let me know.
Regards,
Biju
> Subject: [PATCH v2 4/4] watchdog: rzg2l_wdt: Use force reset for WDT reset
>
> This patch uses the force reset(WDTRSTB) for triggering WDT reset
> (WDTRSTB). This method is faster compared to the overflow method for
> triggering watchdog reset.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> V1->V2:
> * Updated the commit description.
> ---
> 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 d1b5cb70d56c..94f98825ab8d 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
>
> @@ -141,22 +144,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;
> }
> --
> 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-04 16:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-13 15:26 [PATCH v2 1/4] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
2021-12-13 15:26 ` [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for reset_control_{deassert/reset} Biju Das
2022-01-04 16:36 ` Biju Das
2021-12-13 15:26 ` [PATCH v2 3/4] watchdog: rzg2l_wdt: Add set_timeout callback Biju Das
2021-12-13 15:26 ` [PATCH v2 4/4] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
2022-01-04 16:38 ` Biju Das
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.