Hi,
On 08/04/2015 at 19:45:55 +0000, Harald Geyer wrote :
> When the watchdog is enabled, we set a persitent bit. After booting
> we query the bit and see if the system was reset by the watchdog.
>
> This is somewhat similiar to what the legacy driver from freescale
similar ^
> does. However we use STMP3XXX_RTC_PERSISTENT2 instead of
> STMP3XXX_RTC_PERSISTENT1. I tried that first, but it seems I can't
> clear the bit there once it is set. I didn't find any documentation
> what this register does - only vague hints that it is meant to
> control the boot ROM.
>
> Part of the code from the legacy driver touching this register
> is still included. Maybe this is stale, but this patch doesn't
> touch any of it because I don't know what it really does or is
> meant to do.
>
> Signed-off-by: Harald Geyer <harald@xxxxxxxxx>
The RTC part seems ok to me but the driver itself would benefit from a
few improvements:
- use STMP_OFFSET_REG_(SET|CLR) instead of defining _SET and _CLR for
STMP3XXX_RTC_CTRL and STMP3XXX_RTC_PERSISTENT0
- use syscon instead of passing callbacks to the watchdog driver
It would be greatly appreciated if you could do those changes too.
> ---
> drivers/rtc/rtc-stmp3xxx.c | 23 +++++++++++++++++++++++
> drivers/watchdog/stmp3xxx_rtc_wdt.c | 34 +++++++++++++++++++++++++++++++++-
> include/linux/stmp3xxx_rtc_wdt.h | 2 ++
> 3 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-stmp3xxx.c b/drivers/rtc/rtc-stmp3xxx.c
> index 2939cdc..55ab9ba 100644
> --- a/drivers/rtc/rtc-stmp3xxx.c
> +++ b/drivers/rtc/rtc-stmp3xxx.c
> @@ -30,6 +30,7 @@
> #include <linux/of.h>
> #include <linux/stmp_device.h>
> #include <linux/stmp3xxx_rtc_wdt.h>
> +#include <linux/watchdog.h>
>
> #define STMP3XXX_RTC_CTRL 0x0
> #define STMP3XXX_RTC_CTRL_SET 0x4
> @@ -60,6 +61,9 @@
> /* missing bitmask in headers */
> #define STMP3XXX_RTC_PERSISTENT1_FORCE_UPDATER 0x80000000
>
> +#define STMP3XXX_RTC_PERSISTENT2 0x80
> +#define STMP3XXX_RTC_PERSISTENT2_WDT_ACTIVE 0x00000001
> +
> struct stmp3xxx_rtc_data {
> struct rtc_device *rtc;
> void __iomem *io;
> @@ -81,6 +85,14 @@ struct stmp3xxx_rtc_data {
> * registers is atomic.
> */
>
> +static void stmp3xxx_wdt_clear_bootstatus(struct device *dev)
> +{
> + struct stmp3xxx_rtc_data *rtc_data = dev_get_drvdata(dev);
> +
> + writel(STMP3XXX_RTC_PERSISTENT2_WDT_ACTIVE,
> + STMP3XXX_RTC_PERSISTENT2 + STMP_OFFSET_REG_CLR + rtc_data->io);
> +}
> +
> static void stmp3xxx_wdt_set_timeout(struct device *dev, u32 timeout)
> {
> struct stmp3xxx_rtc_data *rtc_data = dev_get_drvdata(dev);
> @@ -91,16 +103,22 @@ static void stmp3xxx_wdt_set_timeout(struct device *dev, u32 timeout)
> rtc_data->io + STMP3XXX_RTC_CTRL + STMP_OFFSET_REG_SET);
> writel(STMP3XXX_RTC_PERSISTENT1_FORCE_UPDATER,
> rtc_data->io + STMP3XXX_RTC_PERSISTENT1 + STMP_OFFSET_REG_SET);
> + writel(STMP3XXX_RTC_PERSISTENT2_WDT_ACTIVE,
> + rtc_data->io + STMP3XXX_RTC_PERSISTENT2 + STMP_OFFSET_REG_SET);
> } else {
> writel(STMP3XXX_RTC_CTRL_WATCHDOGEN,
> rtc_data->io + STMP3XXX_RTC_CTRL + STMP_OFFSET_REG_CLR);
> writel(STMP3XXX_RTC_PERSISTENT1_FORCE_UPDATER,
> rtc_data->io + STMP3XXX_RTC_PERSISTENT1 + STMP_OFFSET_REG_CLR);
> + writel(STMP3XXX_RTC_PERSISTENT2_WDT_ACTIVE,
> + rtc_data->io + STMP3XXX_RTC_PERSISTENT2 + STMP_OFFSET_REG_CLR);
> }
> }
>
> static struct stmp3xxx_wdt_pdata wdt_pdata = {
> .wdt_set_timeout = stmp3xxx_wdt_set_timeout,
> + .wdt_clear_bootstatus = stmp3xxx_wdt_clear_bootstatus,
> + .bootstatus = 0,
> };
>
> static void stmp3xxx_wdt_register(struct platform_device *rtc_pdev)
> @@ -108,6 +126,8 @@ static void stmp3xxx_wdt_register(struct platform_device *rtc_pdev)
> struct platform_device *wdt_pdev =
> platform_device_alloc("stmp3xxx_rtc_wdt", rtc_pdev->id);
>
> + stmp3xxx_wdt_clear_bootstatus(&rtc_pdev->dev);
> +
> if (wdt_pdev) {
> wdt_pdev->dev.parent = &rtc_pdev->dev;
> wdt_pdev->dev.platform_data = &wdt_pdata;
> @@ -304,6 +324,9 @@ static int stmp3xxx_rtc_probe(struct platform_device *pdev)
> return err;
> }
>
> + if (readl(STMP3XXX_RTC_PERSISTENT2 + rtc_data->io) &
> + STMP3XXX_RTC_PERSISTENT2_WDT_ACTIVE)
> + wdt_pdata.bootstatus |= WDIOF_CARDRESET;
> stmp3xxx_wdt_register(pdev);
> return 0;
> }
> diff --git a/drivers/watchdog/stmp3xxx_rtc_wdt.c b/drivers/watchdog/stmp3xxx_rtc_wdt.c
> index a62b1b6..5b3f12a8 100644
> --- a/drivers/watchdog/stmp3xxx_rtc_wdt.c
> +++ b/drivers/watchdog/stmp3xxx_rtc_wdt.c
> @@ -14,6 +14,8 @@
> #include <linux/watchdog.h>
> #include <linux/platform_device.h>
> #include <linux/stmp3xxx_rtc_wdt.h>
> +#include <linux/notifier.h>
> +#include <linux/reboot.h>
>
> #define WDOG_TICK_RATE 1000 /* 1 kHz clock */
> #define STMP3XXX_DEFAULT_TIMEOUT 19
> @@ -50,7 +52,8 @@ static int wdt_set_timeout(struct watchdog_device *wdd, unsigned new_timeout)
> }
>
> static const struct watchdog_info stmp3xxx_wdt_ident = {
> - .options = WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
> + .options = WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> + WDIOF_CARDRESET,
> .identity = "STMP3XXX RTC Watchdog",
> };
>
> @@ -69,13 +72,38 @@ static struct watchdog_device stmp3xxx_wdd = {
> .status = WATCHDOG_NOWAYOUT_INIT_STATUS,
> };
>
> +static int wdt_notify_sys(struct notifier_block *nb, unsigned long code,
> + void *unused)
> +{
> + struct device *dev = watchdog_get_drvdata(&stmp3xxx_wdd);
> + struct stmp3xxx_wdt_pdata *pdata = dev_get_platdata(dev);
> +
> + switch (code) {
> + case SYS_DOWN: /* keep enabled, system might crash while going down */
> + pdata->wdt_clear_bootstatus(dev->parent);
> + break;
> + case SYS_HALT: /* allow the system to actually halt */
> + case SYS_POWER_OFF:
> + wdt_stop(&stmp3xxx_wdd);
> + break;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block wdt_notifier = {
> + .notifier_call = wdt_notify_sys,
> +};
> +
> static int stmp3xxx_wdt_probe(struct platform_device *pdev)
> {
> + struct stmp3xxx_wdt_pdata *pdata = dev_get_platdata(&pdev->dev);
> int ret;
>
> watchdog_set_drvdata(&stmp3xxx_wdd, &pdev->dev);
>
> stmp3xxx_wdd.timeout = clamp_t(unsigned, heartbeat, 1, STMP3XXX_MAX_TIMEOUT);
> + stmp3xxx_wdd.bootstatus = pdata->bootstatus;
>
> ret = watchdog_register_device(&stmp3xxx_wdd);
> if (ret < 0) {
> @@ -83,6 +111,9 @@ static int stmp3xxx_wdt_probe(struct platform_device *pdev)
> return ret;
> }
>
> + if (register_reboot_notifier(&wdt_notifier))
> + dev_warn(&pdev->dev, "cannot register reboot notifier\n");
> +
> dev_info(&pdev->dev, "initialized watchdog with heartbeat %ds\n",
> stmp3xxx_wdd.timeout);
> return 0;
> @@ -90,6 +121,7 @@ static int stmp3xxx_wdt_probe(struct platform_device *pdev)
>
> static int stmp3xxx_wdt_remove(struct platform_device *pdev)
> {
> + unregister_reboot_notifier(&wdt_notifier);
> watchdog_unregister_device(&stmp3xxx_wdd);
> return 0;
> }
> diff --git a/include/linux/stmp3xxx_rtc_wdt.h b/include/linux/stmp3xxx_rtc_wdt.h
> index 1dd12c9..62dd9e6 100644
> --- a/include/linux/stmp3xxx_rtc_wdt.h
> +++ b/include/linux/stmp3xxx_rtc_wdt.h
> @@ -10,6 +10,8 @@
>
> struct stmp3xxx_wdt_pdata {
> void (*wdt_set_timeout)(struct device *dev, u32 timeout);
> + void (*wdt_clear_bootstatus)(struct device *dev);
> + unsigned int bootstatus;
> };
>
> #endif /* __LINUX_STMP3XXX_RTC_WDT_H */
> --
> 1.7.10.4
>
> --
> --
> You received this message because you are subscribed to "rtc-linux".
> Membership options at http://groups.google.com/group/rtc-linux .
> Please read http://groups.google.com/group/rtc-linux/web/checklist
> before submitting a driver.
> ---
> You received this message because you are subscribed to the Google Groups "rtc-linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@xxxxxxxxxxxxxxxx.
> For more options, visit https://groups.google.com/d/optout.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
