On 8/18/19 11:00 AM, Wolfram Sang wrote:
From: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
Support an already running watchdog by checking its enable bit and set
up the status accordingly before registering the device. Introduce a new
flag to remember all this to keep RPM calls balanced.
Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
---
Changes since RFC:
* Geert ensured that the module clock for the RWDT will stay active
during the boot process because clock will only be stopped at the end
of init if there is no refcnt for this clk.
* So, we make sure to have a refcnt when FW enabled the wdog. Once the
first call to open comes, we "transfer" the refcnt to that call.
(Is that the correct behaviour? I think it is a tad better than to
place the balancing RPM call in remove, but I am open here)
* Tested with "open_timeout" kernel parameter. System can now reboot
if userspace hasn't taken over the watchdog with <n> seconds.
drivers/watchdog/renesas_wdt.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
index 00662a8e039c..11cef69f329b 100644
--- a/drivers/watchdog/renesas_wdt.c
+++ b/drivers/watchdog/renesas_wdt.c
@@ -50,6 +50,7 @@ struct rwdt_priv {
struct watchdog_device wdev;
unsigned long clk_rate;
u8 cks;
+ bool started_by_fw;
};
static void rwdt_write(struct rwdt_priv *priv, u32 val, unsigned int reg)
@@ -85,7 +86,11 @@ static int rwdt_start(struct watchdog_device *wdev)
struct rwdt_priv *priv = watchdog_get_drvdata(wdev);
u8 val;
- pm_runtime_get_sync(wdev->parent);
+ if (priv->started_by_fw)
+ /* we already called this function and RPM is active */
+ priv->started_by_fw = false;
If the HW watchdog is running, the start function should not be called
on open. It would be called after the watchdog was stopped and is then
started again. With that, opening the watchdog the first time would not
call this function, and started_by_fw would remain true. Closing it would
then stopp the watchdog and call pm_runtime_put(). The next open() would
hit the above case, and not call pm_runtime_get_sync(). pm would then be
out of sync.
What am I missing ?
Guenter
+ else
+ pm_runtime_get_sync(wdev->parent);
/* Stop the timer before we modify any register */
val = readb_relaxed(priv->base + RWTCSRA) & ~RWTCSRA_TME;
@@ -194,6 +199,7 @@ static int rwdt_probe(struct platform_device *pdev)
struct clk *clk;
unsigned long clks_per_sec;
int ret, i;
+ u8 csra;
if (rwdt_blacklisted(dev))
return -ENODEV;
@@ -213,8 +219,8 @@ static int rwdt_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
priv->clk_rate = clk_get_rate(clk);
- priv->wdev.bootstatus = (readb_relaxed(priv->base + RWTCSRA) &
- RWTCSRA_WOVF) ? WDIOF_CARDRESET : 0;
+ csra = readb_relaxed(priv->base + RWTCSRA);
+ priv->wdev.bootstatus = csra & RWTCSRA_WOVF ? WDIOF_CARDRESET : 0;
pm_runtime_put(dev);
if (!priv->clk_rate) {
@@ -252,6 +258,14 @@ static int rwdt_probe(struct platform_device *pdev)
/* This overrides the default timeout only if DT configuration was found */
watchdog_init_timeout(&priv->wdev, 0, dev);
+ /* Check if FW enabled the watchdog */
+ if (csra & RWTCSRA_TME) {
+ /* Ensure properly initialized dividers */
+ rwdt_start(&priv->wdev);
+ set_bit(WDOG_HW_RUNNING, &priv->wdev.status);
+ priv->started_by_fw = true;
+ }
+
ret = watchdog_register_device(&priv->wdev);
if (ret < 0)
goto out_pm_disable;
