Hi Ariel,
On Fri, Jun 26, 2015 at 03:24:31PM -0300, Ariel D'Alessandro wrote:
> This commit adds support for the watchdog timer found in NXP LPC SoCs
> family, which includes LPC18xx/LPC43xx. Other SoCs in that family may
> share the same watchdog hardware.
>
> Watchdog driver registers a restart handler that will restart the system
> by performing an incorrect feed after ensuring the watchdog is enabled in
> reset mode.
>
> As watchdog cannot be disabled in hardware, driver's stop routine will
> regularly send a keepalive ping using a timer.
>
> Signed-off-by: Ariel D'Alessandro <ariel@xxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/watchdog/Kconfig | 11 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/lpc18xx_wdt.c | 340 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 352 insertions(+)
> create mode 100644 drivers/watchdog/lpc18xx_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 742fbbc..31100c2 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -547,6 +547,17 @@ config DIGICOLOR_WATCHDOG
> To compile this driver as a module, choose M here: the
> module will be called digicolor_wdt.
>
> +config LPC18XX_WATCHDOG
> + tristate "LCP18XX Watchdog"
> + depends on ARCH_LPC18XX
> + select WATCHDOG_CORE
> + help
> + Say Y here if to include support for the watchdog timer
> + in NXP LPC SoCs family, which includes LPC18xx/LPC43xx
> + processors.
> + To compile this driver as a module, choose M here: the
> + module will be called lpc18xx_wdt.
> +
> # AVR32 Architecture
>
> config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 59ea9a1..1b0ef48 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o
> obj-$(CONFIG_MESON_WATCHDOG) += meson_wdt.o
> obj-$(CONFIG_MEDIATEK_WATCHDOG) += mtk_wdt.o
> obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o
> +obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o
>
> # AVR32 Architecture
> obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/lpc18xx_wdt.c b/drivers/watchdog/lpc18xx_wdt.c
> new file mode 100644
> index 0000000..80d68eb
> --- /dev/null
> +++ b/drivers/watchdog/lpc18xx_wdt.c
> @@ -0,0 +1,340 @@
> +/*
> + * NXP LPC18xx Windowed Watchdog Timer (WWDT)
> + *
What does "Windowed" stand for ? That term doesn't really mean anything
to me. How does it differ from a non-windowed watchdog driver ?
> + * Copyright (c) 2015 Ariel D'Alessandro <ariel@xxxxxxxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * Notes
> + * -----
> + * The Watchdog consists of a fixed divide-by-4 clock pre-scaler and a 24-bit counter
> + * which decrements on every clock cycle.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/watchdog.h>
> +
> +/* Registers */
> +#define LPC_WDT_MOD 0x00
> +#define LPC_WDT_MOD_WDEN BIT(0)
> +#define LPC_WDT_MOD_WDRESET BIT(1)
> +#define LPC_WDT_MOD_WDTOF_MASK 0x04
> +
> +#define LPC_WDT_TC 0x04
> +#define LPC_WDT_TC_MIN 0xff
> +#define LPC_WDT_TC_MAX 0xffffff
> +
> +#define LPC_WDT_FEED 0x08
> +#define LPC_WDT_FEED_MAGIC1 0xaa
> +#define LPC_WDT_FEED_MAGIC2 0x55
> +
> +#define LPC_WDT_TV 0x0c
> +
> +/* Clock pre-scaler */
> +#define LPC_WDT_CLK_DIV 4
> +
> +/* Timeout values in seconds */
> +#define LPC_WDT_DEF_TIMEOUT 1
> +
One second ? This is highly unusual. 30 or 60 seconds is more common,
and one second would be very challenging for user space.
Any special reason for using such a tight default ?
> +static int heartbeat;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds "
> + "(default=" __MODULE_STRING(LPC_WDT_DEF_TIMEOUT) ")");
> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
> + "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +struct lpc_wdt_dev {
> + struct watchdog_device wdt_dev;
> + struct clk *wdt_clk;
> + struct clk *sys_clk;
> + void __iomem *base;
> + struct timer_list timer;
> + struct notifier_block restart_handler;
> +};
> +
> +static int lpc_wdt_feed(struct watchdog_device *wdt_dev)
> +{
> + unsigned long flags;
> + struct lpc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
> +
Please order variables length wise where possible.
> + raw_local_irq_save(flags);
> + writel(LPC_WDT_FEED_MAGIC1, wdt->base + LPC_WDT_FEED);
> + writel(LPC_WDT_FEED_MAGIC2, wdt->base + LPC_WDT_FEED);
> + raw_local_irq_restore(flags);
> +
> + return 0;
> +}
> +
> +static void lpc_wdt_timer_feed(unsigned long data)
> +{
> + unsigned long flags;
> + struct watchdog_device *wdt_dev = (struct watchdog_device *) data;
> + struct lpc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
> +
> + /*
> + * An abort condition will occur if an interrupt happens during the feed
> + * sequence.
> + */
> + raw_local_irq_save(flags);
> + writel(LPC_WDT_FEED_MAGIC1, wdt->base + LPC_WDT_FEED);
> + writel(LPC_WDT_FEED_MAGIC2, wdt->base + LPC_WDT_FEED);
> + raw_local_irq_restore(flags);
> +
Unless I am missing something,
lpc_wdt_feed(wdt_dev);
should do the same and reduce code duplication.
> + /* Use safe value (1/2 of real timeout) */
> + mod_timer(&wdt->timer, jiffies + msecs_to_jiffies((wdt_dev->timeout *
> + MSEC_PER_SEC) / 2));
> +}
> +
> +/*
> + * Since LPC18xx Watchdog cannot be disabled in hardware, we must keep feeding
> + * it with a timer until userspace watchdog software takes over.
> + */
> +static int lpc_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> + lpc_wdt_timer_feed((unsigned long) wdt_dev);
> +
> + return 0;
> +}
> +
> +static void __lpc_wdt_set_timeout(struct lpc_wdt_dev *wdt)
> +{
> + unsigned long clk_rate = clk_get_rate(wdt->wdt_clk);
> + unsigned int val;
> +
> + val = DIV_ROUND_UP(wdt->wdt_dev.timeout * clk_rate, LPC_WDT_CLK_DIV);
> + writel(val, wdt->base + LPC_WDT_TC);
> +}
> +
> +static int lpc_wdt_set_timeout(struct watchdog_device *wdt_dev,
> + unsigned int new_timeout)
> +{
> + struct lpc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
> +
> + wdt->wdt_dev.timeout = new_timeout;
> + __lpc_wdt_set_timeout(wdt);
> +
> + return 0;
> +}
> +
> +
> +unsigned int lpc_wdt_get_timeleft(struct watchdog_device *wdt_dev)
> +{
> + struct lpc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
> + unsigned long clk_rate = clk_get_rate(wdt->wdt_clk);
> + unsigned int val;
> +
> + val = readl(wdt->base + LPC_WDT_TV);
> + return ((val * LPC_WDT_CLK_DIV) / clk_rate);
> +}
> +
> +static int lpc_wdt_start(struct watchdog_device *wdt_dev)
> +{
> + unsigned int val;
> + struct lpc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
> +
> + if (timer_pending(&wdt->timer))
> + del_timer(&wdt->timer);
> +
> + val = readl(wdt->base + LPC_WDT_MOD);
> + val |= LPC_WDT_MOD_WDEN;
> + val |= LPC_WDT_MOD_WDRESET;
> + writel(val, wdt->base + LPC_WDT_MOD);
> +
> + /*
> + * Setting the WDEN bit in the WDMOD register is not sufficient to
> + * enable the Watchdog. A valid feed sequence must be completed after
> + * setting WDEN before the Watchdog is capable of generating a reset.
> + */
> + lpc_wdt_feed(wdt_dev);
> +
> + return 0;
> +}
> +
> +static struct watchdog_info lpc_wdt_info = {
> + .identity = "LPC 18XX Watchdog",
> + .options = WDIOF_SETTIMEOUT |
> + WDIOF_KEEPALIVEPING |
> + WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops lpc_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = lpc_wdt_start,
> + .stop = lpc_wdt_stop,
> + .ping = lpc_wdt_feed,
> + .set_timeout = lpc_wdt_set_timeout,
> + .get_timeleft = lpc_wdt_get_timeleft,
> +};
> +
> +static int lpc_wdt_restart(struct notifier_block *this, unsigned long mode,
> + void *cmd)
> +{
> + int val;
> + unsigned long flags;
> + struct lpc_wdt_dev *wdt = container_of(this, struct lpc_wdt_dev,
> + restart_handler);
> +
> + /*
> + * Incorrect feed sequence causes immediate watchdog reset if enabled.
> + */
> + raw_local_irq_save(flags);
> +
> + val = readl(wdt->base + LPC_WDT_MOD);
> + val |= LPC_WDT_MOD_WDEN;
> + val |= LPC_WDT_MOD_WDRESET;
> + writel(val, wdt->base + LPC_WDT_MOD);
> +
> + writel(LPC_WDT_FEED_MAGIC1, wdt->base + LPC_WDT_FEED);
> + writel(LPC_WDT_FEED_MAGIC2, wdt->base + LPC_WDT_FEED);
> +
> + writel(LPC_WDT_FEED_MAGIC1, wdt->base + LPC_WDT_FEED);
> + writel(LPC_WDT_FEED_MAGIC1, wdt->base + LPC_WDT_FEED);
> +
> + raw_local_irq_restore(flags);
> +
> + return NOTIFY_OK;
> +}
> +
> +static int lpc_wdt_probe(struct platform_device *pdev)
> +{
> + int ret;
> + unsigned long clk_rate;
> + struct resource *res;
> + struct lpc_wdt_dev *lpc_wdt;
> +
> + lpc_wdt = devm_kzalloc(&pdev->dev, sizeof(*lpc_wdt), GFP_KERNEL);
> + if (!lpc_wdt)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + lpc_wdt->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(lpc_wdt->base))
> + return PTR_ERR(lpc_wdt->base);
> +
> + lpc_wdt->sys_clk = devm_clk_get(&pdev->dev, "sys");
> + if (IS_ERR(lpc_wdt->sys_clk)) {
> + dev_err(&pdev->dev, "failed to get the sys clock\n");
> + return PTR_ERR(lpc_wdt->sys_clk);
> + }
> +
> + lpc_wdt->wdt_clk = devm_clk_get(&pdev->dev, "wdt");
> + if (IS_ERR(lpc_wdt->wdt_clk)) {
> + dev_err(&pdev->dev, "failed to get the wdt clock\n");
> + return PTR_ERR(lpc_wdt->wdt_clk);
> + }
> +
> + ret = clk_prepare_enable(lpc_wdt->sys_clk);
> + if (ret) {
> + dev_err(&pdev->dev, "could not prepare or enable sys clock\n");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(lpc_wdt->wdt_clk);
> + if (ret) {
> + dev_err(&pdev->dev, "could not prepare or enable wdt clock\n");
> + goto disable_sys_clk;
> + }
> +
> + /* We use the clock rate to calculate timeouts */
> + clk_rate = clk_get_rate(lpc_wdt->wdt_clk);
> + if (clk_rate == 0) {
> + dev_err(&pdev->dev, "failed to get clock rate\n");
> + ret = -EINVAL;
> + goto disable_wdt_clk;
> + }
> +
> + lpc_wdt->wdt_dev.info = &lpc_wdt_info;
> + lpc_wdt->wdt_dev.ops = &lpc_wdt_ops;
> +
> + lpc_wdt->wdt_dev.min_timeout =
> + DIV_ROUND_UP(LPC_WDT_TC_MIN * LPC_WDT_CLK_DIV, clk_rate);
> +
> + lpc_wdt->wdt_dev.max_timeout =
> + (LPC_WDT_TC_MAX * LPC_WDT_CLK_DIV) / clk_rate;
> +
> + lpc_wdt->wdt_dev.parent = &pdev->dev;
> + watchdog_set_drvdata(&lpc_wdt->wdt_dev, lpc_wdt);
> +
> + ret = watchdog_init_timeout(&lpc_wdt->wdt_dev, heartbeat, &pdev->dev);
> + if (ret < 0)
> + lpc_wdt->wdt_dev.timeout = LPC_WDT_DEF_TIMEOUT;
> +
> + __lpc_wdt_set_timeout(lpc_wdt);
> +
> + setup_timer(&lpc_wdt->timer, lpc_wdt_timer_feed,
> + (unsigned long)&lpc_wdt->wdt_dev);
> +
> + watchdog_set_nowayout(&lpc_wdt->wdt_dev, nowayout);
> +
> + platform_set_drvdata(pdev, lpc_wdt);
> +
> + ret = watchdog_register_device(&lpc_wdt->wdt_dev);
> + if (ret)
> + goto disable_wdt_clk;
> +
> + lpc_wdt->restart_handler.notifier_call = lpc_wdt_restart;
> + lpc_wdt->restart_handler.priority = 128;
> + ret = register_restart_handler(&lpc_wdt->restart_handler);
> + if (ret)
> + dev_warn(&pdev->dev, "failed to register restart handler: %d\n",
> + ret);
> +
> + return 0;
> +
> +disable_wdt_clk:
> + clk_disable_unprepare(lpc_wdt->wdt_clk);
> +disable_sys_clk:
> + clk_disable_unprepare(lpc_wdt->sys_clk);
> + return ret;
> +}
> +
> +static void lpc_wdt_shutdown(struct platform_device *pdev)
> +{
> + struct lpc_wdt_dev *lpc_wdt = platform_get_drvdata(pdev);
> +
> + lpc_wdt_stop(&lpc_wdt->wdt_dev);
> +}
> +
> +static int lpc_wdt_remove(struct platform_device *pdev)
> +{
> + struct lpc_wdt_dev *lpc_wdt = platform_get_drvdata(pdev);
> +
> + lpc_wdt_stop(&lpc_wdt->wdt_dev);
This will keep the timer enabled. It would be interesting to see what
happens if you build the driver as module and unload it. I think
it will crash. Can you try ?
> + watchdog_unregister_device(&lpc_wdt->wdt_dev);
> + clk_disable_unprepare(lpc_wdt->wdt_clk);
> + clk_disable_unprepare(lpc_wdt->sys_clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id lpc_wdt_match[] = {
> + { .compatible = "nxp,lpc18xx-wdt" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, lpc_wdt_match);
> +
> +static struct platform_driver lpc_wdt_driver = {
> + .driver = {
> + .name = "lpc18xx-wdt",
> + .of_match_table = lpc_wdt_match,
> + },
> + .probe = lpc_wdt_probe,
> + .remove = lpc_wdt_remove,
> + .shutdown = lpc_wdt_shutdown,
> +};
> +module_platform_driver(lpc_wdt_driver);
> +
> +MODULE_AUTHOR("Ariel D'Alessandro <ariel@xxxxxxxxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("NXP LPC18xx Windowed Watchdog Timer Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
--
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
