linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
@ 2022-02-25 17:53 Biju Das
  2022-02-25 17:53 ` [PATCH v5 1/7] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc

The first 4 patch in this series fixes the below issues
1) 32 bit overflow issue
2) Runtime PM usage count issue
3) BUG: Invalid context during reset.
4) Reset control imbalance

The later 3 patches are enhancements to the WDT driver.
1) Adding error check for reset_control_deassert() and fixing reset_control imbalance.
2) Generate Parity error for WDT reset
3) Add support for set_timeout callback

v4->v5:
 * Updated commit description of patch#4
 * Added Rb tag from Geert.
 * Separated reset control imbalance from patch#4

Biju Das (7):
  watchdog: rzg2l_wdt: Fix 32bit overflow issue
  watchdog: rzg2l_wdt: Fix Runtime PM usage
  watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'
  watchdog: rzg2l_wdt: Fix reset control imbalance
  watchdog: rzg2l_wdt: Add error check for reset_control_deassert
  watchdog: rzg2l_wdt: Use force reset for WDT reset
  watchdog: rzg2l_wdt: Add set_timeout callback

 drivers/watchdog/rzg2l_wdt.c | 83 ++++++++++++++++++++----------------
 1 file changed, 47 insertions(+), 36 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5 1/7] watchdog: rzg2l_wdt: Fix 32bit overflow issue
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
@ 2022-02-25 17:53 ` Biju Das
  2022-02-25 17:53 ` [PATCH v5 2/7] watchdog: rzg2l_wdt: Fix Runtime PM usage Biju Das
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 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.

This patch also fixes the warning message, 'watchdog: Invalid min and
max timeout values, resetting to 0!'.

Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
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>
---
v4->v5:
 * Updated commit description typo !"->!'.
v3->v4:
 * Updated commit description. Added Fixes tag
v2->v3:
 * No change
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] 21+ messages in thread

* [PATCH v5 2/7] watchdog: rzg2l_wdt: Fix Runtime PM usage
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
  2022-02-25 17:53 ` [PATCH v5 1/7] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
@ 2022-02-25 17:53 ` Biju Das
  2022-02-25 20:17   ` Guenter Roeck
  2022-02-25 17:53 ` [PATCH v5 3/7] watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context' Biju Das
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc

Both rzg2l_wdt_probe() and rzg2l_wdt_start() calls pm_runtime_get() which
results in a usage counter imbalance. This patch fixes this issue by
removing pm_runtime_get() call from probe.

Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V4->v5:
 * Added Rb tag from Geert.
V4:
 * New patch
---
 drivers/watchdog/rzg2l_wdt.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 96f2a018ab62..0fc73b8a9567 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -151,12 +151,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);
 }
@@ -206,11 +205,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
 
 	reset_control_deassert(priv->rstc);
 	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 +216,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 +229,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] 21+ messages in thread

* [PATCH v5 3/7] watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
  2022-02-25 17:53 ` [PATCH v5 1/7] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
  2022-02-25 17:53 ` [PATCH v5 2/7] watchdog: rzg2l_wdt: Fix Runtime PM usage Biju Das
@ 2022-02-25 17:53 ` Biju Das
  2022-02-25 20:18   ` Guenter Roeck
  2022-02-25 17:53 ` [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance Biju Das
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck
  Cc: Biju Das, linux-watchdog, Linux PM list, Geert Uytterhoeven,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad,
	linux-renesas-soc

This patch fixes the issue 'BUG: Invalid wait context' during restart()
callback by using clk_prepare_enable() instead of pm_runtime_get_sync()
for turning on the clocks during restart.

This issue is noticed when testing with renesas_defconfig.

[   42.213802] reboot: Restarting system
[   42.217860]
[   42.219364] =============================
[   42.223368] [ BUG: Invalid wait context ]
[   42.227372] 5.17.0-rc5-arm64-renesas-00002-g10393723e35e #522 Not tainted
[   42.234153] -----------------------------
[   42.238155] systemd-shutdow/1 is trying to lock:
[   42.242766] ffff00000a650828 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x14/0x20
[   42.250709] other info that might help us debug this:
[   42.255753] context-{4:4}
[   42.258368] 2 locks held by systemd-shutdow/1:
[   42.262806]  #0: ffff80000944e1c8 (system_transition_mutex#2){+.+.}-{3:3}, at: __do_sys_reboot+0xd0/0x250
[   42.272388]  #1: ffff8000094c4e40 (rcu_read_lock){....}-{1:2}, at: atomic_notifier_call_chain+0x0/0x150
[   42.281795] stack backtrace:
[   42.284672] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 5.17.0-rc5-arm64-renesas-00002-g10393723e35e #522
[   42.294577] Hardware name: Renesas SMARC EVK based on r9a07g044c2 (DT)
[   42.301096] Call trace:
[   42.303538]  dump_backtrace+0xcc/0xd8
[   42.307203]  show_stack+0x14/0x30
[   42.310517]  dump_stack_lvl+0x88/0xb0
[   42.314180]  dump_stack+0x14/0x2c
[   42.317492]  __lock_acquire+0x1b24/0x1b50
[   42.321502]  lock_acquire+0x120/0x3a8
[   42.325162]  __mutex_lock+0x84/0x8f8
[   42.328737]  mutex_lock_nested+0x30/0x58
[   42.332658]  genpd_lock_mtx+0x14/0x20
[   42.336319]  genpd_runtime_resume+0xc4/0x228
[   42.340587]  __rpm_callback+0x44/0x170
[   42.344337]  rpm_callback+0x64/0x70
[   42.347824]  rpm_resume+0x4e0/0x6b8
[   42.351310]  __pm_runtime_resume+0x50/0x78
[   42.355404]  rzg2l_wdt_restart+0x28/0x68
[   42.359329]  watchdog_restart_notifier+0x1c/0x30
[   42.363943]  atomic_notifier_call_chain+0x94/0x150
[   42.368732]  do_kernel_restart+0x24/0x30
[   42.372652]  machine_restart+0x44/0x70
[   42.376399]  kernel_restart+0x3c/0x60
[   42.380058]  __do_sys_reboot+0x228/0x250
[   42.383977]  __arm64_sys_reboot+0x20/0x28
[   42.387983]  invoke_syscall+0x40/0xf8

Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V4->V5:
 * Added Rb tag from Geert.
 * CC'ed Linux PM.
V4:
 * New patch
---
 drivers/watchdog/rzg2l_wdt.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 0fc73b8a9567..48dfe6e5e64f 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -43,6 +43,8 @@ struct rzg2l_wdt_priv {
 	struct reset_control *rstc;
 	unsigned long osc_clk_rate;
 	unsigned long delay;
+	struct clk *pclk;
+	struct clk *osc_clk;
 };
 
 static void rzg2l_wdt_wait_delay(struct rzg2l_wdt_priv *priv)
@@ -118,7 +120,9 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
 
 	/* Reset the module before we modify any register */
 	reset_control_reset(priv->rstc);
-	pm_runtime_get_sync(wdev->parent);
+
+	clk_prepare_enable(priv->pclk);
+	clk_prepare_enable(priv->osc_clk);
 
 	/* smallest counter value to reboot soon */
 	rzg2l_wdt_write(priv, WDTSET_COUNTER_VAL(1), WDTSET);
@@ -165,7 +169,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct rzg2l_wdt_priv *priv;
 	unsigned long pclk_rate;
-	struct clk *wdt_clk;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -177,22 +180,20 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->base);
 
 	/* Get watchdog main clock */
-	wdt_clk = clk_get(&pdev->dev, "oscclk");
-	if (IS_ERR(wdt_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(wdt_clk), "no oscclk");
+	priv->osc_clk = devm_clk_get(&pdev->dev, "oscclk");
+	if (IS_ERR(priv->osc_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->osc_clk), "no oscclk");
 
-	priv->osc_clk_rate = clk_get_rate(wdt_clk);
-	clk_put(wdt_clk);
+	priv->osc_clk_rate = clk_get_rate(priv->osc_clk);
 	if (!priv->osc_clk_rate)
 		return dev_err_probe(&pdev->dev, -EINVAL, "oscclk rate is 0");
 
 	/* Get Peripheral clock */
-	wdt_clk = clk_get(&pdev->dev, "pclk");
-	if (IS_ERR(wdt_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(wdt_clk), "no pclk");
+	priv->pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(priv->pclk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->pclk), "no pclk");
 
-	pclk_rate = clk_get_rate(wdt_clk);
-	clk_put(wdt_clk);
+	pclk_rate = clk_get_rate(priv->pclk);
 	if (!pclk_rate)
 		return dev_err_probe(&pdev->dev, -EINVAL, "pclk rate is 0");
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
                   ` (2 preceding siblings ...)
  2022-02-25 17:53 ` [PATCH v5 3/7] watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context' Biju Das
@ 2022-02-25 17:53 ` Biju Das
  2022-03-01 11:20   ` Geert Uytterhoeven
  2022-03-09 23:08   ` Guenter Roeck
  2022-02-25 17:53 ` [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert Biju Das
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc

Both rzg2l_wdt_probe() and rzg2l_wdt_start() calls reset_control_
deassert() which results in a reset control imbalance.

This patch fixes reset control imbalance by removing reset_control_
deassert() from rzg2l_wdt_start() and replaces reset_control_assert with
reset_control_reset in rzg2l_wdt_stop() as watchdog module can be stopped
only by a module reset. This change will allow us to restart WDT after
stop() by configuring WDT timeout and enable registers.

Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v5:
 * New patch
---
 drivers/watchdog/rzg2l_wdt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 48dfe6e5e64f..88274704b260 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -88,7 +88,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 */
@@ -108,7 +107,7 @@ 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);
+	reset_control_reset(priv->rstc);
 
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
                   ` (3 preceding siblings ...)
  2022-02-25 17:53 ` [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance Biju Das
@ 2022-02-25 17:53 ` Biju Das
  2022-02-26  9:15   ` Sergei Shtylyov
                     ` (2 more replies)
  2022-02-25 17:53 ` [PATCH v5 6/7] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
                   ` (2 subsequent siblings)
  7 siblings, 3 replies; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  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 bailout in case of error.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v4->v5:
 * Updated commit description.
 * Moved reset control imbalance to patch#4.
v3->v4:
 * Made reset usage counter balanced
 * Updated commit description
v2->v3:
 * Patch reordering from Patch 2 -> Patch 3
 * Updated commit description
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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 88274704b260..73b667ed3e99 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -203,7 +203,10 @@ 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);
 
 	priv->wdev.info = &rzg2l_wdt_ident;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 6/7] watchdog: rzg2l_wdt: Use force reset for WDT reset
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
                   ` (4 preceding siblings ...)
  2022-02-25 17:53 ` [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert Biju Das
@ 2022-02-25 17:53 ` Biju Das
  2022-02-25 20:18   ` Guenter Roeck
  2022-02-25 17:53 ` [PATCH v5 7/7] watchdog: rzg2l_wdt: Add set_timeout callback Biju Das
  2022-03-07  8:52 ` [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
  7 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  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 for
restart callback. This method(ie, Generate Reset (WDTRSTB) Signal on
parity error)is faster compared to the overflow method for triggering
watchdog reset.

Overflow method:
	reboot: Restarting system
	Reboot failed -- System halted
	NOTICE:  BL2: v2.5(release):v2.5/rzg2l-1.00-27-gf48f1440c

Parity error method:
	reboot: Restarting system
	NOTICE:  BL2: v2.5(release):v2.5/rzg2l-1.00-27-gf48f1440c

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V4->V5:
 * Added Rb tag from Geert.
V3->V4:
 * Renamed PEEN_FORCE_RST->PEEN_FORCE
 * Updated comments with parity error description
 * Updated commit description
V2->v3:
 * Patch reordering from patch 4->patch 2
 * Updated the commit description.
V1->V2:
 * Updated the commit description.
---
 drivers/watchdog/rzg2l_wdt.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 73b667ed3e99..4e7107655cc2 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	BIT(0)
 
 #define WDT_DEFAULT_TIMEOUT		60U
 
@@ -117,17 +120,14 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
 {
 	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
 
-	/* Reset the module before we modify any register */
-	reset_control_reset(priv->rstc);
-
 	clk_prepare_enable(priv->pclk);
 	clk_prepare_enable(priv->osc_clk);
 
-	/* smallest counter value to reboot soon */
-	rzg2l_wdt_write(priv, WDTSET_COUNTER_VAL(1), WDTSET);
+	/* Generate Reset (WDTRSTB) Signal on parity error */
+	rzg2l_wdt_write(priv, 0, PECR);
 
-	/* Enable watchdog timer*/
-	rzg2l_wdt_write(priv, WDTCNT_WDTEN, WDTCNT);
+	/* Force parity error */
+	rzg2l_wdt_write(priv, PEEN_FORCE, PEEN);
 
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 7/7] watchdog: rzg2l_wdt: Add set_timeout callback
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
                   ` (5 preceding siblings ...)
  2022-02-25 17:53 ` [PATCH v5 6/7] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
@ 2022-02-25 17:53 ` Biju Das
  2022-02-25 20:18   ` Guenter Roeck
  2022-03-07  8:52 ` [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
  7 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2022-02-25 17:53 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  Cc: Biju Das, linux-watchdog, Geert Uytterhoeven, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad, linux-renesas-soc

This patch adds support for set_timeout callback.

Once WDT is started, the WDT cycle setting register(WDTSET) can be updated
only after issuing a module reset. Otherwise, it will ignore the writes
and will hold the previous value. This patch updates the WDTSET register
if it is active.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V4->V5:
 * Added Rb tag from Geert.
V3->v4:
 * Updated commit description
 * Simplified the logic for updating timeout register, if wdt is active.
v2->v3:
 * Patch reodering Patch 3 -> patch 4
 * Updated commit description.
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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
index 4e7107655cc2..6eea0ee4af49 100644
--- a/drivers/watchdog/rzg2l_wdt.c
+++ b/drivers/watchdog/rzg2l_wdt.c
@@ -115,6 +115,25 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev)
 	return 0;
 }
 
+static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int timeout)
+{
+	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
+
+	wdev->timeout = timeout;
+
+	/*
+	 * If the watchdog is active, reset the module for updating the WDTSET
+	 * register so that it is updated with new timeout values.
+	 */
+	if (watchdog_active(wdev)) {
+		pm_runtime_put(wdev->parent);
+		reset_control_reset(priv->rstc);
+		rzg2l_wdt_start(wdev);
+	}
+
+	return 0;
+}
+
 static int rzg2l_wdt_restart(struct watchdog_device *wdev,
 			     unsigned long action, void *data)
 {
@@ -151,6 +170,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] 21+ messages in thread

* Re: [PATCH v5 2/7] watchdog: rzg2l_wdt: Fix Runtime PM usage
  2022-02-25 17:53 ` [PATCH v5 2/7] watchdog: rzg2l_wdt: Fix Runtime PM usage Biju Das
@ 2022-02-25 20:17   ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2022-02-25 20:17 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, Philipp Zabel, linux-watchdog,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

On Fri, Feb 25, 2022 at 05:53:15PM +0000, Biju Das wrote:
> Both rzg2l_wdt_probe() and rzg2l_wdt_start() calls pm_runtime_get() which
> results in a usage counter imbalance. This patch fixes this issue by
> removing pm_runtime_get() call from probe.
> 
> Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
> V4->v5:
>  * Added Rb tag from Geert.
> V4:
>  * New patch
> ---
>  drivers/watchdog/rzg2l_wdt.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 96f2a018ab62..0fc73b8a9567 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -151,12 +151,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);
>  }
> @@ -206,11 +205,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  
>  	reset_control_deassert(priv->rstc);
>  	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 +216,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 +229,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] 21+ messages in thread

* Re: [PATCH v5 3/7] watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'
  2022-02-25 17:53 ` [PATCH v5 3/7] watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context' Biju Das
@ 2022-02-25 20:18   ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2022-02-25 20:18 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, linux-watchdog, Linux PM list,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

On Fri, Feb 25, 2022 at 05:53:16PM +0000, Biju Das wrote:
> This patch fixes the issue 'BUG: Invalid wait context' during restart()
> callback by using clk_prepare_enable() instead of pm_runtime_get_sync()
> for turning on the clocks during restart.
> 
> This issue is noticed when testing with renesas_defconfig.
> 
> [   42.213802] reboot: Restarting system
> [   42.217860]
> [   42.219364] =============================
> [   42.223368] [ BUG: Invalid wait context ]
> [   42.227372] 5.17.0-rc5-arm64-renesas-00002-g10393723e35e #522 Not tainted
> [   42.234153] -----------------------------
> [   42.238155] systemd-shutdow/1 is trying to lock:
> [   42.242766] ffff00000a650828 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x14/0x20
> [   42.250709] other info that might help us debug this:
> [   42.255753] context-{4:4}
> [   42.258368] 2 locks held by systemd-shutdow/1:
> [   42.262806]  #0: ffff80000944e1c8 (system_transition_mutex#2){+.+.}-{3:3}, at: __do_sys_reboot+0xd0/0x250
> [   42.272388]  #1: ffff8000094c4e40 (rcu_read_lock){....}-{1:2}, at: atomic_notifier_call_chain+0x0/0x150
> [   42.281795] stack backtrace:
> [   42.284672] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 5.17.0-rc5-arm64-renesas-00002-g10393723e35e #522
> [   42.294577] Hardware name: Renesas SMARC EVK based on r9a07g044c2 (DT)
> [   42.301096] Call trace:
> [   42.303538]  dump_backtrace+0xcc/0xd8
> [   42.307203]  show_stack+0x14/0x30
> [   42.310517]  dump_stack_lvl+0x88/0xb0
> [   42.314180]  dump_stack+0x14/0x2c
> [   42.317492]  __lock_acquire+0x1b24/0x1b50
> [   42.321502]  lock_acquire+0x120/0x3a8
> [   42.325162]  __mutex_lock+0x84/0x8f8
> [   42.328737]  mutex_lock_nested+0x30/0x58
> [   42.332658]  genpd_lock_mtx+0x14/0x20
> [   42.336319]  genpd_runtime_resume+0xc4/0x228
> [   42.340587]  __rpm_callback+0x44/0x170
> [   42.344337]  rpm_callback+0x64/0x70
> [   42.347824]  rpm_resume+0x4e0/0x6b8
> [   42.351310]  __pm_runtime_resume+0x50/0x78
> [   42.355404]  rzg2l_wdt_restart+0x28/0x68
> [   42.359329]  watchdog_restart_notifier+0x1c/0x30
> [   42.363943]  atomic_notifier_call_chain+0x94/0x150
> [   42.368732]  do_kernel_restart+0x24/0x30
> [   42.372652]  machine_restart+0x44/0x70
> [   42.376399]  kernel_restart+0x3c/0x60
> [   42.380058]  __do_sys_reboot+0x228/0x250
> [   42.383977]  __arm64_sys_reboot+0x20/0x28
> [   42.387983]  invoke_syscall+0x40/0xf8
> 
> Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
> V4->V5:
>  * Added Rb tag from Geert.
>  * CC'ed Linux PM.
> V4:
>  * New patch
> ---
>  drivers/watchdog/rzg2l_wdt.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 0fc73b8a9567..48dfe6e5e64f 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -43,6 +43,8 @@ struct rzg2l_wdt_priv {
>  	struct reset_control *rstc;
>  	unsigned long osc_clk_rate;
>  	unsigned long delay;
> +	struct clk *pclk;
> +	struct clk *osc_clk;
>  };
>  
>  static void rzg2l_wdt_wait_delay(struct rzg2l_wdt_priv *priv)
> @@ -118,7 +120,9 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>  
>  	/* Reset the module before we modify any register */
>  	reset_control_reset(priv->rstc);
> -	pm_runtime_get_sync(wdev->parent);
> +
> +	clk_prepare_enable(priv->pclk);
> +	clk_prepare_enable(priv->osc_clk);
>  
>  	/* smallest counter value to reboot soon */
>  	rzg2l_wdt_write(priv, WDTSET_COUNTER_VAL(1), WDTSET);
> @@ -165,7 +169,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct rzg2l_wdt_priv *priv;
>  	unsigned long pclk_rate;
> -	struct clk *wdt_clk;
>  	int ret;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -177,22 +180,20 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>  		return PTR_ERR(priv->base);
>  
>  	/* Get watchdog main clock */
> -	wdt_clk = clk_get(&pdev->dev, "oscclk");
> -	if (IS_ERR(wdt_clk))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(wdt_clk), "no oscclk");
> +	priv->osc_clk = devm_clk_get(&pdev->dev, "oscclk");
> +	if (IS_ERR(priv->osc_clk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(priv->osc_clk), "no oscclk");
>  
> -	priv->osc_clk_rate = clk_get_rate(wdt_clk);
> -	clk_put(wdt_clk);
> +	priv->osc_clk_rate = clk_get_rate(priv->osc_clk);
>  	if (!priv->osc_clk_rate)
>  		return dev_err_probe(&pdev->dev, -EINVAL, "oscclk rate is 0");
>  
>  	/* Get Peripheral clock */
> -	wdt_clk = clk_get(&pdev->dev, "pclk");
> -	if (IS_ERR(wdt_clk))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(wdt_clk), "no pclk");
> +	priv->pclk = devm_clk_get(&pdev->dev, "pclk");
> +	if (IS_ERR(priv->pclk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(priv->pclk), "no pclk");
>  
> -	pclk_rate = clk_get_rate(wdt_clk);
> -	clk_put(wdt_clk);
> +	pclk_rate = clk_get_rate(priv->pclk);
>  	if (!pclk_rate)
>  		return dev_err_probe(&pdev->dev, -EINVAL, "pclk rate is 0");
>  
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 6/7] watchdog: rzg2l_wdt: Use force reset for WDT reset
  2022-02-25 17:53 ` [PATCH v5 6/7] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
@ 2022-02-25 20:18   ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2022-02-25 20:18 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, Philipp Zabel, linux-watchdog,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

On Fri, Feb 25, 2022 at 05:53:19PM +0000, Biju Das wrote:
> This patch uses the force reset(WDTRSTB) for triggering WDT reset for
> restart callback. This method(ie, Generate Reset (WDTRSTB) Signal on
> parity error)is faster compared to the overflow method for triggering
> watchdog reset.
> 
> Overflow method:
> 	reboot: Restarting system
> 	Reboot failed -- System halted
> 	NOTICE:  BL2: v2.5(release):v2.5/rzg2l-1.00-27-gf48f1440c
> 
> Parity error method:
> 	reboot: Restarting system
> 	NOTICE:  BL2: v2.5(release):v2.5/rzg2l-1.00-27-gf48f1440c
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
> V4->V5:
>  * Added Rb tag from Geert.
> V3->V4:
>  * Renamed PEEN_FORCE_RST->PEEN_FORCE
>  * Updated comments with parity error description
>  * Updated commit description
> V2->v3:
>  * Patch reordering from patch 4->patch 2
>  * Updated the commit description.
> V1->V2:
>  * Updated the commit description.
> ---
>  drivers/watchdog/rzg2l_wdt.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 73b667ed3e99..4e7107655cc2 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	BIT(0)
>  
>  #define WDT_DEFAULT_TIMEOUT		60U
>  
> @@ -117,17 +120,14 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>  {
>  	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
>  
> -	/* Reset the module before we modify any register */
> -	reset_control_reset(priv->rstc);
> -
>  	clk_prepare_enable(priv->pclk);
>  	clk_prepare_enable(priv->osc_clk);
>  
> -	/* smallest counter value to reboot soon */
> -	rzg2l_wdt_write(priv, WDTSET_COUNTER_VAL(1), WDTSET);
> +	/* Generate Reset (WDTRSTB) Signal on parity error */
> +	rzg2l_wdt_write(priv, 0, PECR);
>  
> -	/* Enable watchdog timer*/
> -	rzg2l_wdt_write(priv, WDTCNT_WDTEN, WDTCNT);
> +	/* Force parity error */
> +	rzg2l_wdt_write(priv, PEEN_FORCE, PEEN);
>  
>  	return 0;
>  }
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 7/7] watchdog: rzg2l_wdt: Add set_timeout callback
  2022-02-25 17:53 ` [PATCH v5 7/7] watchdog: rzg2l_wdt: Add set_timeout callback Biju Das
@ 2022-02-25 20:18   ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2022-02-25 20:18 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, Philipp Zabel, linux-watchdog,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

On Fri, Feb 25, 2022 at 05:53:20PM +0000, Biju Das wrote:
> This patch adds support for set_timeout callback.
> 
> Once WDT is started, the WDT cycle setting register(WDTSET) can be updated
> only after issuing a module reset. Otherwise, it will ignore the writes
> and will hold the previous value. This patch updates the WDTSET register
> if it is active.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
> V4->V5:
>  * Added Rb tag from Geert.
> V3->v4:
>  * Updated commit description
>  * Simplified the logic for updating timeout register, if wdt is active.
> v2->v3:
>  * Patch reodering Patch 3 -> patch 4
>  * Updated commit description.
> 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 | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 4e7107655cc2..6eea0ee4af49 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -115,6 +115,25 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev)
>  	return 0;
>  }
>  
> +static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int timeout)
> +{
> +	struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +
> +	wdev->timeout = timeout;
> +
> +	/*
> +	 * If the watchdog is active, reset the module for updating the WDTSET
> +	 * register so that it is updated with new timeout values.
> +	 */
> +	if (watchdog_active(wdev)) {
> +		pm_runtime_put(wdev->parent);
> +		reset_control_reset(priv->rstc);
> +		rzg2l_wdt_start(wdev);
> +	}
> +
> +	return 0;
> +}
> +
>  static int rzg2l_wdt_restart(struct watchdog_device *wdev,
>  			     unsigned long action, void *data)
>  {
> @@ -151,6 +170,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	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert
  2022-02-25 17:53 ` [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert Biju Das
@ 2022-02-26  9:15   ` Sergei Shtylyov
  2022-02-28 14:36     ` Biju Das
  2022-03-01 11:21   ` Geert Uytterhoeven
  2022-03-09 23:09   ` Guenter Roeck
  2 siblings, 1 reply; 21+ messages in thread
From: Sergei Shtylyov @ 2022-02-26  9:15 UTC (permalink / raw)
  To: Biju Das, Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  Cc: linux-watchdog, Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hello!

On 2/25/22 8:53 PM, Biju Das wrote:

> 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 bailout in case of error.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
[...]

> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 88274704b260..73b667ed3e99 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -203,7 +203,10 @@ 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");

   Deassert what? :-)

[...]

MBR, Sergey

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert
  2022-02-26  9:15   ` Sergei Shtylyov
@ 2022-02-28 14:36     ` Biju Das
  0 siblings, 0 replies; 21+ messages in thread
From: Biju Das @ 2022-02-28 14:36 UTC (permalink / raw)
  To: Sergei Shtylyov, Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  Cc: linux-watchdog, Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hello Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for
> reset_control_deassert
> 
> Hello!
> 
> On 2/25/22 8:53 PM, Biju Das wrote:
> 
> > 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 bailout in case of error.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> [...]
> 
> > diff --git a/drivers/watchdog/rzg2l_wdt.c
> > b/drivers/watchdog/rzg2l_wdt.c index 88274704b260..73b667ed3e99 100644
> > --- a/drivers/watchdog/rzg2l_wdt.c
> > +++ b/drivers/watchdog/rzg2l_wdt.c
> > @@ -203,7 +203,10 @@ 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");
> 
>    Deassert what? :-)

failed to deassert WDT_PRESETN reset signal.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance
  2022-02-25 17:53 ` [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance Biju Das
@ 2022-03-01 11:20   ` Geert Uytterhoeven
  2022-03-09 23:08   ` Guenter Roeck
  1 sibling, 0 replies; 21+ messages in thread
From: Geert Uytterhoeven @ 2022-03-01 11:20 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel,
	Linux Watchdog Mailing List, Geert Uytterhoeven, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad, Linux-Renesas

On Fri, Feb 25, 2022 at 6:53 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Both rzg2l_wdt_probe() and rzg2l_wdt_start() calls reset_control_
> deassert() which results in a reset control imbalance.
>
> This patch fixes reset control imbalance by removing reset_control_
> deassert() from rzg2l_wdt_start() and replaces reset_control_assert with
> reset_control_reset in rzg2l_wdt_stop() as watchdog module can be stopped
> only by a module reset. This change will allow us to restart WDT after
> stop() by configuring WDT timeout and enable registers.
>
> Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert
  2022-02-25 17:53 ` [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert Biju Das
  2022-02-26  9:15   ` Sergei Shtylyov
@ 2022-03-01 11:21   ` Geert Uytterhoeven
  2022-03-09 23:09   ` Guenter Roeck
  2 siblings, 0 replies; 21+ messages in thread
From: Geert Uytterhoeven @ 2022-03-01 11:21 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, Guenter Roeck, Philipp Zabel,
	Linux Watchdog Mailing List, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, Linux-Renesas

On Fri, Feb 25, 2022 at 6:53 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 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 bailout in case of error.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
  2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
                   ` (6 preceding siblings ...)
  2022-02-25 17:53 ` [PATCH v5 7/7] watchdog: rzg2l_wdt: Add set_timeout callback Biju Das
@ 2022-03-07  8:52 ` Biju Das
  2022-03-09 23:42   ` Guenter Roeck
  7 siblings, 1 reply; 21+ messages in thread
From: Biju Das @ 2022-03-07  8:52 UTC (permalink / raw)
  To: Biju Das, Wim Van Sebroeck, Guenter Roeck, Philipp Zabel
  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 series?

Please let me know.

Cheers,
Biju

> Subject: [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
> 
> The first 4 patch in this series fixes the below issues
> 1) 32 bit overflow issue
> 2) Runtime PM usage count issue
> 3) BUG: Invalid context during reset.
> 4) Reset control imbalance
> 
> The later 3 patches are enhancements to the WDT driver.
> 1) Adding error check for reset_control_deassert() and fixing
> reset_control imbalance.
> 2) Generate Parity error for WDT reset
> 3) Add support for set_timeout callback
> 
> v4->v5:
>  * Updated commit description of patch#4
>  * Added Rb tag from Geert.
>  * Separated reset control imbalance from patch#4
> 
> Biju Das (7):
>   watchdog: rzg2l_wdt: Fix 32bit overflow issue
>   watchdog: rzg2l_wdt: Fix Runtime PM usage
>   watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'
>   watchdog: rzg2l_wdt: Fix reset control imbalance
>   watchdog: rzg2l_wdt: Add error check for reset_control_deassert
>   watchdog: rzg2l_wdt: Use force reset for WDT reset
>   watchdog: rzg2l_wdt: Add set_timeout callback
> 
>  drivers/watchdog/rzg2l_wdt.c | 83 ++++++++++++++++++++----------------
>  1 file changed, 47 insertions(+), 36 deletions(-)
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance
  2022-02-25 17:53 ` [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance Biju Das
  2022-03-01 11:20   ` Geert Uytterhoeven
@ 2022-03-09 23:08   ` Guenter Roeck
  1 sibling, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2022-03-09 23:08 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, Philipp Zabel, linux-watchdog,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

On Fri, Feb 25, 2022 at 05:53:17PM +0000, Biju Das wrote:
> Both rzg2l_wdt_probe() and rzg2l_wdt_start() calls reset_control_
> deassert() which results in a reset control imbalance.
> 
> This patch fixes reset control imbalance by removing reset_control_
> deassert() from rzg2l_wdt_start() and replaces reset_control_assert with
> reset_control_reset in rzg2l_wdt_stop() as watchdog module can be stopped
> only by a module reset. This change will allow us to restart WDT after
> stop() by configuring WDT timeout and enable registers.
> 
> Fixes: 2cbc5cd0b55fa2 ("watchdog: Add Watchdog Timer driver for RZ/G2L")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
> v5:
>  * New patch
> ---
>  drivers/watchdog/rzg2l_wdt.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 48dfe6e5e64f..88274704b260 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -88,7 +88,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 */
> @@ -108,7 +107,7 @@ 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);
> +	reset_control_reset(priv->rstc);
>  
>  	return 0;
>  }

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert
  2022-02-25 17:53 ` [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert Biju Das
  2022-02-26  9:15   ` Sergei Shtylyov
  2022-03-01 11:21   ` Geert Uytterhoeven
@ 2022-03-09 23:09   ` Guenter Roeck
  2 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2022-03-09 23:09 UTC (permalink / raw)
  To: Biju Das
  Cc: Wim Van Sebroeck, Philipp Zabel, linux-watchdog,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

On Fri, Feb 25, 2022 at 05:53:18PM +0000, Biju Das wrote:
> 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 bailout in case of error.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
> v4->v5:
>  * Updated commit description.
>  * Moved reset control imbalance to patch#4.
> v3->v4:
>  * Made reset usage counter balanced
>  * Updated commit description
> v2->v3:
>  * Patch reordering from Patch 2 -> Patch 3
>  * Updated commit description
> 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 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 88274704b260..73b667ed3e99 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -203,7 +203,10 @@ 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);
>  
>  	priv->wdev.info = &rzg2l_wdt_ident;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
  2022-03-07  8:52 ` [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
@ 2022-03-09 23:42   ` Guenter Roeck
  2022-03-10  6:53     ` Biju Das
  0 siblings, 1 reply; 21+ messages in thread
From: Guenter Roeck @ 2022-03-09 23:42 UTC (permalink / raw)
  To: Biju Das, Wim Van Sebroeck, Philipp Zabel
  Cc: linux-watchdog, Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

On 3/7/22 00:52, Biju Das wrote:
> Hi All,
> 
> Gentle ping. Are we happy with this patch series?
> 
> Please let me know.
> 

Should be good to go. I added the series to my watchdog-next branch.
Usually Wim picks it up from there.

Guenter

> Cheers,
> Biju
> 
>> Subject: [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
>>
>> The first 4 patch in this series fixes the below issues
>> 1) 32 bit overflow issue
>> 2) Runtime PM usage count issue
>> 3) BUG: Invalid context during reset.
>> 4) Reset control imbalance
>>
>> The later 3 patches are enhancements to the WDT driver.
>> 1) Adding error check for reset_control_deassert() and fixing
>> reset_control imbalance.
>> 2) Generate Parity error for WDT reset
>> 3) Add support for set_timeout callback
>>
>> v4->v5:
>>   * Updated commit description of patch#4
>>   * Added Rb tag from Geert.
>>   * Separated reset control imbalance from patch#4
>>
>> Biju Das (7):
>>    watchdog: rzg2l_wdt: Fix 32bit overflow issue
>>    watchdog: rzg2l_wdt: Fix Runtime PM usage
>>    watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'
>>    watchdog: rzg2l_wdt: Fix reset control imbalance
>>    watchdog: rzg2l_wdt: Add error check for reset_control_deassert
>>    watchdog: rzg2l_wdt: Use force reset for WDT reset
>>    watchdog: rzg2l_wdt: Add set_timeout callback
>>
>>   drivers/watchdog/rzg2l_wdt.c | 83 ++++++++++++++++++++----------------
>>   1 file changed, 47 insertions(+), 36 deletions(-)
>>
>> --
>> 2.17.1
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
  2022-03-09 23:42   ` Guenter Roeck
@ 2022-03-10  6:53     ` Biju Das
  0 siblings, 0 replies; 21+ messages in thread
From: Biju Das @ 2022-03-10  6:53 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck, Philipp Zabel
  Cc: linux-watchdog, Geert Uytterhoeven, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Guenter,

> Subject: Re: [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
> 
> On 3/7/22 00:52, Biju Das wrote:
> > Hi All,
> >
> > Gentle ping. Are we happy with this patch series?
> >
> > Please let me know.
> >
> 
> Should be good to go. I added the series to my watchdog-next branch.
> Usually Wim picks it up from there.

Thanks,
Biju

> >> Subject: [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements
> >>
> >> The first 4 patch in this series fixes the below issues
> >> 1) 32 bit overflow issue
> >> 2) Runtime PM usage count issue
> >> 3) BUG: Invalid context during reset.
> >> 4) Reset control imbalance
> >>
> >> The later 3 patches are enhancements to the WDT driver.
> >> 1) Adding error check for reset_control_deassert() and fixing
> >> reset_control imbalance.
> >> 2) Generate Parity error for WDT reset
> >> 3) Add support for set_timeout callback
> >>
> >> v4->v5:
> >>   * Updated commit description of patch#4
> >>   * Added Rb tag from Geert.
> >>   * Separated reset control imbalance from patch#4
> >>
> >> Biju Das (7):
> >>    watchdog: rzg2l_wdt: Fix 32bit overflow issue
> >>    watchdog: rzg2l_wdt: Fix Runtime PM usage
> >>    watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'
> >>    watchdog: rzg2l_wdt: Fix reset control imbalance
> >>    watchdog: rzg2l_wdt: Add error check for reset_control_deassert
> >>    watchdog: rzg2l_wdt: Use force reset for WDT reset
> >>    watchdog: rzg2l_wdt: Add set_timeout callback
> >>
> >>   drivers/watchdog/rzg2l_wdt.c | 83 ++++++++++++++++++++---------------
> -
> >>   1 file changed, 47 insertions(+), 36 deletions(-)
> >>
> >> --
> >> 2.17.1
> >


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-03-10  6:53 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 17:53 [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
2022-02-25 17:53 ` [PATCH v5 1/7] watchdog: rzg2l_wdt: Fix 32bit overflow issue Biju Das
2022-02-25 17:53 ` [PATCH v5 2/7] watchdog: rzg2l_wdt: Fix Runtime PM usage Biju Das
2022-02-25 20:17   ` Guenter Roeck
2022-02-25 17:53 ` [PATCH v5 3/7] watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context' Biju Das
2022-02-25 20:18   ` Guenter Roeck
2022-02-25 17:53 ` [PATCH v5 4/7] watchdog: rzg2l_wdt: Fix reset control imbalance Biju Das
2022-03-01 11:20   ` Geert Uytterhoeven
2022-03-09 23:08   ` Guenter Roeck
2022-02-25 17:53 ` [PATCH v5 5/7] watchdog: rzg2l_wdt: Add error check for reset_control_deassert Biju Das
2022-02-26  9:15   ` Sergei Shtylyov
2022-02-28 14:36     ` Biju Das
2022-03-01 11:21   ` Geert Uytterhoeven
2022-03-09 23:09   ` Guenter Roeck
2022-02-25 17:53 ` [PATCH v5 6/7] watchdog: rzg2l_wdt: Use force reset for WDT reset Biju Das
2022-02-25 20:18   ` Guenter Roeck
2022-02-25 17:53 ` [PATCH v5 7/7] watchdog: rzg2l_wdt: Add set_timeout callback Biju Das
2022-02-25 20:18   ` Guenter Roeck
2022-03-07  8:52 ` [PATCH v5 0/7] RZG2L_WDT Fixes and Improvements Biju Das
2022-03-09 23:42   ` Guenter Roeck
2022-03-10  6:53     ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).