linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Primoz Fiser <primoz.fiser@norik.com>
To: linux-watchdog@vger.kernel.org,
	Guenter Roeck <linux@roeck-us.net>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Support Opensource <support.opensource@diasemi.com>
Cc: Tero Kristo <t-kristo@ti.com>,
	Stefan Riedmueller <s.riedmueller@phytec.de>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>
Subject: [PATCH v2] watchdog: da9062: da9063: prevent pings ahead of machine reset
Date: Thu,  8 Jul 2021 10:21:28 +0200	[thread overview]
Message-ID: <20210708082128.2832904-1-primoz.fiser@norik.com> (raw)
In-Reply-To: <20210706112103.1687587-1-primoz.fiser@norik.com>

Proper machine resets via da9062/da9063 PMICs are very tricky as they
require special i2c atomic transfers when interrupts are not available
anymore. This is also a reason why both PMIC's restart handlers do not
use regmap but instead opt for i2c_smbus_write_byte_data() which does
i2c transfer in atomic manner. Under the hood, this function tries to
obtain i2c bus lock with call to i2c_adapter_trylock_bus() which will
return -EAGAIN (-11) if lock is not available.

Since commit 982bb70517aef ("watchdog: reset last_hw_keepalive time at
start") occasional restart handler failures with "Failed to shutdown
(err = -11)" error messages were observed, indicating that some
process is holding the i2c bus lock. Investigation into the matter
uncovered that sometimes during reboot sequence watchdog ping is issued
late into poweroff/reboot phase which did not happen before mentioned
commit (usually the watchdog ping happened immediately as commit message
suggests). As of now, when watchdog ping usually happens late into
poweroff/reboot stage when interrupts are not available anymore, i2c bus
lock cannot be released anymore and pending restart handler in turn
fails.

Thus, to prevent such late watchdog pings from happening ahead of
pending machine restart and consequently locking up the i2c bus, check
for system_state in watchdog ping handler and consequently do not send
pings anymore in case system_state > SYSTEM_RUNNING.

Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
---
Implemented proposal suggested by Guenter Roeck.

Re-spun boards in boot/reboot loop tests and done 5000 cycles on each
one with flying colors. 

Changes in v2:
- reduce code complexity by removing reboot notifiers and use
  system_state variable instead
- minor commit message rewording

 drivers/watchdog/da9062_wdt.c | 7 +++++++
 drivers/watchdog/da9063_wdt.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
index 706fb09c2f24..f02cbd530538 100644
--- a/drivers/watchdog/da9062_wdt.c
+++ b/drivers/watchdog/da9062_wdt.c
@@ -117,6 +117,13 @@ static int da9062_wdt_ping(struct watchdog_device *wdd)
 	struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd);
 	int ret;
 
+	/*
+	 * Prevent pings from occurring late in system poweroff/reboot sequence
+	 * and possibly locking out restart handler from accessing i2c bus.
+	 */
+	if (system_state > SYSTEM_RUNNING)
+		return 0;
+
 	ret = da9062_reset_watchdog_timer(wdt);
 	if (ret)
 		dev_err(wdt->hw->dev, "Failed to ping the watchdog (err = %d)\n",
diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
index 423584252606..d79ce64e26a9 100644
--- a/drivers/watchdog/da9063_wdt.c
+++ b/drivers/watchdog/da9063_wdt.c
@@ -121,6 +121,13 @@ static int da9063_wdt_ping(struct watchdog_device *wdd)
 	struct da9063 *da9063 = watchdog_get_drvdata(wdd);
 	int ret;
 
+	/*
+	 * Prevent pings from occurring late in system poweroff/reboot sequence
+	 * and possibly locking out restart handler from accessing i2c bus.
+	 */
+	if (system_state > SYSTEM_RUNNING)
+		return 0;
+
 	ret = regmap_write(da9063->regmap, DA9063_REG_CONTROL_F,
 			   DA9063_WATCHDOG);
 	if (ret)
-- 
2.25.1


  parent reply	other threads:[~2021-07-08  8:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-06 11:21 [PATCH] watchdog: da9062: da9063: prevent watchdog pings ahead of reboot Primoz Fiser
2021-07-06 13:28 ` Guenter Roeck
2021-07-07  4:39   ` Primoz Fiser
2021-07-08  8:21 ` Primoz Fiser [this message]
2021-07-08 14:00   ` [PATCH v2] watchdog: da9062: da9063: prevent pings ahead of machine reset Guenter Roeck
2021-07-08 14:15   ` Adam Thomson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210708082128.2832904-1-primoz.fiser@norik.com \
    --to=primoz.fiser@norik.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=s.riedmueller@phytec.de \
    --cc=support.opensource@diasemi.com \
    --cc=t-kristo@ti.com \
    --cc=wim@linux-watchdog.org \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).