On 09/18/2014 03:27 PM, Josh Cartwright wrote:
The WDT's BITE_TIME warm-reset behavior can be leveraged as a last
resort mechanism for triggering chip reset.  Usually, other restart
methods (such as PS_HOLD) are preferrable for issuing a more complete
reset of the chip.  As such, keep the priority of the watchdog notifier
low.

Signed-off-by: Josh Cartwright <[email protected]>
---
  drivers/watchdog/qcom-wdt.c | 31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index e9409f5..710ab43 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -15,6 +15,7 @@
  #include <linux/module.h>
  #include <linux/of.h>
  #include <linux/platform_device.h>
+#include <linux/reboot.h>
  #include <linux/watchdog.h>

  #define WDT_RST               0x0
@@ -24,6 +25,7 @@
  struct qcom_wdt {
        struct watchdog_device  wdd;
        unsigned long           freq;
+       struct notifier_block   restart_nb;
        void __iomem            *base;
  };

@@ -82,6 +84,24 @@ static const struct watchdog_info qcom_wdt_info = {
        .identity       = KBUILD_MODNAME,
  };

+static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action,
+                           void *data)
+{
+       struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb);
+
+       /*
+        * Trigger watchdog bite:
+        *    Setup BITE_TIME to be very low, and enable WDT.
+        */
+       mutex_lock(&wdt->wdd.lock);

At this time you don't need to worry about locks.

Actually, this might be dangerous if the lock happens to be taken,
as it won't be released (there is no other code running anymore
when this function is called).

+       writel_relaxed(0, wdt->base + WDT_EN);
+       writel_relaxed(1, wdt->base + WDT_RST);
+       writel_relaxed(0x31F3, wdt->base + WDT_BITE_TIME);

What is the magic here, ie what does 0x31F3 stand for ?

+       writel_relaxed(1, wdt->base + WDT_EN);
+       mutex_unlock(&wdt->wdd.lock);
+       return NOTIFY_DONE;
+}
+
  static int qcom_watchdog_probe(struct platform_device *pdev)
  {
        struct qcom_wdt *wdt;
@@ -121,6 +141,17 @@ static int qcom_watchdog_probe(struct platform_device 
*pdev)
                return ret;
        }

+       /*
+        * WDT restart notifier has priority 0 (use as a last resort)
+        */
+       wdt->restart_nb.notifier_call = qcom_wdt_restart;
+       ret = register_restart_handler(&wdt->restart_nb);
+       if (ret) {
+               dev_err(&pdev->dev, "failed to setup restart handler\n");
+               watchdog_unregister_device(&wdt->wdd);
+               return ret;

Sure you want to return an error here ? The watchdog itself is still working,
and this is supposed to be a restart method of last resort. Causing the driver
to fail loading because it can not register its restart handler seems to be
a bit aggressive.

Thanks,
Guenter

+       }
+
        return 0;
  }



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to