Add a driver for the hardware watchdogs in NVIDIA Tegra SoCs (Tegra30 and
later). This driver will configure one watchdog timer that will reset the
system in the case of a watchdog timeout.
This driver binds to the nvidia,tegra30-timer device node and gets its
register base from there.
Signed-off-by: Andrew Chew <[email protected]>
---
Changes from V2:
Applied all of Stephen Warren's comments.
Modified suspend callback by only stopping the watchdog timer if it was
currently active.
Added some logging during suspend/resume.
Documentation/watchdog/watchdog-parameters.txt | 5 +
drivers/watchdog/Kconfig | 11 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/tegra_wdt.c | 371 +++++++++++++++++++++++++
4 files changed, 388 insertions(+)
create mode 100644 drivers/watchdog/tegra_wdt.c
diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index f9492fe..b39f355 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -325,6 +325,11 @@ soft_noboot: Softdog action, set to 1 to ignore reboots, 0 to reboot
stmp3xxx_wdt:
heartbeat: Watchdog heartbeat period in seconds from 1 to 4194304, default 19
-------------------------------------------------
+tegra_wdt:
+heartbeat: Watchdog heartbeats in seconds. (default = 120)
+nowayout: Watchdog cannot be stopped once started
+ (default=kernel config parameter)
+-------------------------------------------------
ts72xx_wdt:
timeout: Watchdog timeout in seconds. (1 <= timeout <= 8, default=8)
nowayout: Disable watchdog shutdown on close
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 4c4c566..3aae2da 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -420,6 +420,17 @@ config SIRFSOC_WATCHDOG
Support for CSR SiRFprimaII and SiRFatlasVI watchdog. When
the watchdog triggers the system will be reset.
+config TEGRA_WATCHDOG
+ tristate "Tegra watchdog"
+ depends on ARCH_TEGRA || COMPILE_TEST
+ select WATCHDOG_CORE
+ help
+ Say Y here to include support for the watchdog timer
+ embedded in NVIDIA Tegra SoCs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called tegra_wdt.
+
# AVR32 Architecture
config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 985a66c..1b5f3d5 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o
obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
+obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o
# AVR32 Architecture
obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/tegra_wdt.c b/drivers/watchdog/tegra_wdt.c
new file mode 100644
index 0000000..1314475
--- /dev/null
+++ b/drivers/watchdog/tegra_wdt.c
@@ -0,0 +1,371 @@
+/*
+ * Copyright (c) 2013, NVIDIA CORPORATION. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+
+/* minimum and maximum watchdog trigger timeout, in seconds */
+#define MIN_WDT_TIMEOUT 1
+#define MAX_WDT_TIMEOUT 255
+
+/*
+ * Base of the WDT registers, from the timer base address. There are
+ * actually 5 watchdogs that can be configured (by pairing with an available
+ * timer), at bases 0x100 + (WDT ID) * 0x20, where WDT ID is 0 through 4.
+ * This driver only configures the first watchdog (WDT ID 0).
+ */
+#define WDT_BASE 0x100
+#define WDT_ID 0
+
+/*
+ * Register base of the timer that's selected for pairing with the watchdog.
+ * This driver arbitrarily uses timer 5, which is currently unused by
+ * other drivers (in particular, the Tegra clocksource driver). If this
+ * needs to change, take care that the new timer is not used by the
+ * clocksource driver.
+ */
+#define WDT_TIMER_BASE 0x60
+#define WDT_TIMER_ID 5
+
+/* WDT registers */
+#define WDT_CFG 0x0
+#define WDT_CFG_PERIOD_SHIFT 4
+#define WDT_CFG_PERIOD_MASK 0xff
+#define WDT_CFG_INT_EN (1 << 12)
+#define WDT_CFG_PMC2CAR_RST_EN (1 << 15)
+#define WDT_STS 0x4
+#define WDT_STS_COUNT_SHIFT 4
+#define WDT_STS_COUNT_MASK 0xff
+#define WDT_STS_EXP_SHIFT 12
+#define WDT_STS_EXP_MASK 0x3
+#define WDT_CMD 0x8
+#define WDT_CMD_START_COUNTER (1 << 0)
+#define WDT_CMD_DISABLE_COUNTER (1 << 1)
+#define WDT_UNLOCK (0xc)
+#define WDT_UNLOCK_PATTERN (0xc45a << 0)
+
+/* Timer registers */
+#define TIMER_PTV 0x0
+#define TIMER_EN (1 << 31)
+#define TIMER_PERIODIC (1 << 30)
+
+struct tegra_wdt {
+ struct watchdog_device wdd;
+ struct kref kref;
+ void __iomem *wdt_regs;
+ void __iomem *tmr_regs;
+};
+
+#define WDT_HEARTBEAT 120
+static int heartbeat = WDT_HEARTBEAT;
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
+ "(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
+
+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) ")");
+
+static void tegra_wdt_release_resources(struct kref *r)
+{
+}
+
+static int tegra_wdt_start(struct watchdog_device *wdd)
+{
+ struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
+ u32 val;
+
+ /* This thing has a fixed 1MHz clock. Normally, we would set the
+ * period to 1 second by writing 1000000ul, but the watchdog system
+ * reset actually occurs on the 4th expiration of this counter,
+ * so we set the period to 1/4 of this amount.
+ */
+ val = 1000000ul / 4;
+ val |= (TIMER_EN | TIMER_PERIODIC);
+ writel(val, wdt->tmr_regs + TIMER_PTV);
+
+ /*
+ * Set number of periods and start counter.
+ *
+ * Interrupt handler is not required for user space
+ * WDT accesses, since the caller is responsible to ping the
+ * WDT to reset the counter before expiration, through ioctls.
+ */
+ val = WDT_TIMER_ID |
+ (wdd->timeout << WDT_CFG_PERIOD_SHIFT) |
+ WDT_CFG_PMC2CAR_RST_EN;
+ writel(val, wdt->wdt_regs + WDT_CFG);
+
+ writel(WDT_CMD_START_COUNTER, wdt->wdt_regs + WDT_CMD);
+
+ return 0;
+}
+
+static int tegra_wdt_stop(struct watchdog_device *wdd)
+{
+ struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ writel(WDT_UNLOCK_PATTERN, wdt->wdt_regs + WDT_UNLOCK);
+ writel(WDT_CMD_DISABLE_COUNTER, wdt->wdt_regs + WDT_CMD);
+ writel(0, wdt->tmr_regs + TIMER_PTV);
+
+ return 0;
+}
+
+static int tegra_wdt_ping(struct watchdog_device *wdd)
+{
+ struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ writel(WDT_CMD_START_COUNTER, wdt->wdt_regs + WDT_CMD);
+
+ return 0;
+}
+
+static int tegra_wdt_set_timeout(struct watchdog_device *wdd,
+ unsigned int timeout)
+{
+ int ret;
+
+ ret = tegra_wdt_stop(wdd);
+ if (ret)
+ return ret;
+
+ wdd->timeout = clamp_t(int, timeout, MIN_WDT_TIMEOUT, MAX_WDT_TIMEOUT);
+
+ if (watchdog_active(wdd))
+ return tegra_wdt_start(wdd);
+
+ return 0;
+}
+
+static unsigned int tegra_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+ struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
+ u32 val;
+ int count;
+ int exp;
+
+ val = readl(wdt->wdt_regs + WDT_STS);
+
+ /* Current countdown (from timeout) */
+ count = (val >> WDT_STS_COUNT_SHIFT) & WDT_STS_COUNT_MASK;
+
+ /* Number of expirations (we are waiting for the 4th expiration) */
+ exp = (val >> WDT_STS_EXP_SHIFT) & WDT_STS_EXP_MASK;
+
+ /*
+ * The entire thing is divided by 4 because we are ticking down 4 times
+ * faster due to needing to wait for the 4th expiration.
+ */
+ return (((3 - exp) * wdd->timeout) + count) / 4;
+}
+
+static void tegra_wdt_ref(struct watchdog_device *wdd)
+{
+ struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ kref_get(&wdt->kref);
+}
+
+static void tegra_wdt_unref(struct watchdog_device *wdd)
+{
+ struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ kref_put(&wdt->kref, tegra_wdt_release_resources);
+}
+
+static const struct watchdog_info tegra_wdt_info = {
+ .options = WDIOF_SETTIMEOUT |
+ WDIOF_MAGICCLOSE |
+ WDIOF_KEEPALIVEPING,
+ .firmware_version = 0,
+ .identity = "Tegra Watchdog",
+};
+
+static struct watchdog_ops tegra_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = tegra_wdt_start,
+ .stop = tegra_wdt_stop,
+ .ping = tegra_wdt_ping,
+ .set_timeout = tegra_wdt_set_timeout,
+ .get_timeleft = tegra_wdt_get_timeleft,
+ .ref = tegra_wdt_ref,
+ .ref = tegra_wdt_unref,
+};
+
+static int tegra_wdt_probe(struct platform_device *pdev)
+{
+ struct watchdog_device *wdd;
+ struct tegra_wdt *wdt;
+ struct resource *res;
+ void __iomem *regs;
+ int ret;
+
+ /* This is the timer base. */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "incorrect resources\n");
+ return -ENOENT;
+ }
+
+ regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR_OR_NULL(regs)) {
+ dev_err(&pdev->dev, "unable to map registers\n");
+ return -ENOMEM;
+ }
+
+ /*
+ * Allocate our watchdog driver data, which has the
+ * struct watchdog_device nested within it.
+ */
+ wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+ if (!wdt) {
+ dev_err(&pdev->dev, "out of memory\n");
+ return -ENOMEM;
+ }
+
+ /* Initialize struct tegra_wdt. */
+ wdt->wdt_regs = regs + WDT_BASE;
+ wdt->tmr_regs = regs + WDT_TIMER_BASE;
+
+ /* Initialize struct watchdog_device. */
+ wdd = &wdt->wdd;
+ wdd->timeout = heartbeat;
+ wdd->info = &tegra_wdt_info;
+ wdd->ops = &tegra_wdt_ops;
+ wdd->min_timeout = MIN_WDT_TIMEOUT;
+ wdd->max_timeout = MAX_WDT_TIMEOUT;
+
+ watchdog_set_drvdata(wdd, wdt);
+
+ kref_init(&wdt->kref);
+
+ ret = tegra_wdt_stop(wdd);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed to stop watchdog device\n");
+ return ret;
+ }
+
+ watchdog_set_nowayout(wdd, nowayout);
+
+ ret = watchdog_register_device(wdd);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed to register watchdog device\n");
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, wdt);
+
+ dev_info(&pdev->dev,
+ "initialized wdt %d, using timer %d\n", WDT_ID, WDT_TIMER_ID);
+ dev_info(&pdev->dev,
+ "enabled wdt %d (heartbeat = %d sec, nowayout = %d)\n",
+ WDT_ID, heartbeat, nowayout);
+
+ return 0;
+}
+
+static int tegra_wdt_remove(struct platform_device *pdev)
+{
+ struct tegra_wdt *wdt = platform_get_drvdata(pdev);
+ int ret;
+
+ ret = tegra_wdt_stop(&wdt->wdd);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to stop watchdog %d\n", WDT_ID);
+ return ret;
+ }
+
+ watchdog_unregister_device(&wdt->wdd);
+
+ kref_put(&wdt->kref, tegra_wdt_release_resources);
+
+ dev_info(&pdev->dev, "removed wdt %d\n", WDT_ID);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra_wdt_runtime_suspend(struct device *dev)
+{
+ struct tegra_wdt *wdt = dev_get_drvdata(dev);
+ int ret;
+
+ if (watchdog_active(&wdt->wdd)) {
+ dev_info(dev, "suspend, stopping watchdog timer\n");
+
+ ret = tegra_wdt_stop(&wdt->wdd);
+ if (ret) {
+ dev_err(dev, "failed to stop watchdog %d\n", WDT_ID);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int tegra_wdt_runtime_resume(struct device *dev)
+{
+ struct tegra_wdt *wdt = dev_get_drvdata(dev);
+ int ret;
+
+ if (watchdog_active(&wdt->wdd)) {
+ dev_info(dev, "resume, restarting watchdog timer\n");
+
+ ret = tegra_wdt_start(&wdt->wdd);
+ if (ret) {
+ dev_err(dev, "failed to restart watchdog %d\n", WDT_ID);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+#endif
+
+static const struct of_device_id tegra_wdt_of_match[] = {
+ { .compatible = "nvidia,tegra30-timer", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tegra_wdt_of_match);
+
+static const struct dev_pm_ops tegra_wdt_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(tegra_wdt_runtime_suspend,
+ tegra_wdt_runtime_resume)
+};
+
+static struct platform_driver tegra_wdt_driver = {
+ .probe = tegra_wdt_probe,
+ .remove = tegra_wdt_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "tegra-wdt",
+ .pm = &tegra_wdt_pm_ops,
+ .of_match_table = tegra_wdt_of_match,
+ },
+};
+module_platform_driver(tegra_wdt_driver);
+
+MODULE_AUTHOR("NVIDIA Corporation");
+MODULE_DESCRIPTION("Tegra Watchdog Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
--
1.8.1.5
On 02/05/2014 07:38 PM, Andrew Chew wrote:
> Add a driver for the hardware watchdogs in NVIDIA Tegra SoCs (Tegra30 and
> later). This driver will configure one watchdog timer that will reset the
> system in the case of a watchdog timeout.
>
> This driver binds to the nvidia,tegra30-timer device node and gets its
> register base from there.
>
> Signed-off-by: Andrew Chew <[email protected]>
> ---
> Changes from V2:
>
> Applied all of Stephen Warren's comments.
> Modified suspend callback by only stopping the watchdog timer if it was
> currently active.
> Added some logging during suspend/resume.
>
> Documentation/watchdog/watchdog-parameters.txt | 5 +
> drivers/watchdog/Kconfig | 11 +
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/tegra_wdt.c | 371 +++++++++++++++++++++++++
> 4 files changed, 388 insertions(+)
> create mode 100644 drivers/watchdog/tegra_wdt.c
>
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index f9492fe..b39f355 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -325,6 +325,11 @@ soft_noboot: Softdog action, set to 1 to ignore reboots, 0 to reboot
> stmp3xxx_wdt:
> heartbeat: Watchdog heartbeat period in seconds from 1 to 4194304, default 19
> -------------------------------------------------
> +tegra_wdt:
> +heartbeat: Watchdog heartbeats in seconds. (default = 120)
> +nowayout: Watchdog cannot be stopped once started
> + (default=kernel config parameter)
> +-------------------------------------------------
> ts72xx_wdt:
> timeout: Watchdog timeout in seconds. (1 <= timeout <= 8, default=8)
> nowayout: Disable watchdog shutdown on close
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 4c4c566..3aae2da 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -420,6 +420,17 @@ config SIRFSOC_WATCHDOG
> Support for CSR SiRFprimaII and SiRFatlasVI watchdog. When
> the watchdog triggers the system will be reset.
>
> +config TEGRA_WATCHDOG
> + tristate "Tegra watchdog"
> + depends on ARCH_TEGRA || COMPILE_TEST
Hope you tested this on a couple of platforms ?
> + select WATCHDOG_CORE
> + help
> + Say Y here to include support for the watchdog timer
> + embedded in NVIDIA Tegra SoCs.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called tegra_wdt.
> +
> # AVR32 Architecture
>
> config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 985a66c..1b5f3d5 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -58,6 +58,7 @@ obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
> obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o
> obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
> +obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o
>
> # AVR32 Architecture
> obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/tegra_wdt.c b/drivers/watchdog/tegra_wdt.c
> new file mode 100644
> index 0000000..1314475
> --- /dev/null
> +++ b/drivers/watchdog/tegra_wdt.c
> @@ -0,0 +1,371 @@
> +/*
> + * Copyright (c) 2013, NVIDIA CORPORATION. All rights reserved.
> + *
2014 ?
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/miscdevice.h>
Not needed - also see below.
> +#include <linux/watchdog.h>
> +
> +/* minimum and maximum watchdog trigger timeout, in seconds */
> +#define MIN_WDT_TIMEOUT 1
> +#define MAX_WDT_TIMEOUT 255
> +
> +/*
> + * Base of the WDT registers, from the timer base address. There are
> + * actually 5 watchdogs that can be configured (by pairing with an available
> + * timer), at bases 0x100 + (WDT ID) * 0x20, where WDT ID is 0 through 4.
> + * This driver only configures the first watchdog (WDT ID 0).
> + */
> +#define WDT_BASE 0x100
> +#define WDT_ID 0
> +
> +/*
> + * Register base of the timer that's selected for pairing with the watchdog.
> + * This driver arbitrarily uses timer 5, which is currently unused by
> + * other drivers (in particular, the Tegra clocksource driver). If this
> + * needs to change, take care that the new timer is not used by the
> + * clocksource driver.
> + */
> +#define WDT_TIMER_BASE 0x60
> +#define WDT_TIMER_ID 5
> +
> +/* WDT registers */
> +#define WDT_CFG 0x0
> +#define WDT_CFG_PERIOD_SHIFT 4
> +#define WDT_CFG_PERIOD_MASK 0xff
> +#define WDT_CFG_INT_EN (1 << 12)
> +#define WDT_CFG_PMC2CAR_RST_EN (1 << 15)
> +#define WDT_STS 0x4
> +#define WDT_STS_COUNT_SHIFT 4
> +#define WDT_STS_COUNT_MASK 0xff
> +#define WDT_STS_EXP_SHIFT 12
> +#define WDT_STS_EXP_MASK 0x3
> +#define WDT_CMD 0x8
> +#define WDT_CMD_START_COUNTER (1 << 0)
> +#define WDT_CMD_DISABLE_COUNTER (1 << 1)
> +#define WDT_UNLOCK (0xc)
> +#define WDT_UNLOCK_PATTERN (0xc45a << 0)
> +
> +/* Timer registers */
> +#define TIMER_PTV 0x0
> +#define TIMER_EN (1 << 31)
> +#define TIMER_PERIODIC (1 << 30)
> +
> +struct tegra_wdt {
> + struct watchdog_device wdd;
> + struct kref kref;
> + void __iomem *wdt_regs;
> + void __iomem *tmr_regs;
> +};
> +
> +#define WDT_HEARTBEAT 120
> +static int heartbeat = WDT_HEARTBEAT;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
> + "(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
I'd suggest to split this such that the chackpatch warning goes away.
> +
> +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) ")");
Same here.
> +
> +static void tegra_wdt_release_resources(struct kref *r)
> +{
> +}
> +
> +static int tegra_wdt_start(struct watchdog_device *wdd)
> +{
> + struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
> + u32 val;
> +
> + /* This thing has a fixed 1MHz clock. Normally, we would set the
Multi-line comment coding style.
> + * period to 1 second by writing 1000000ul, but the watchdog system
> + * reset actually occurs on the 4th expiration of this counter,
> + * so we set the period to 1/4 of this amount.
> + */
> + val = 1000000ul / 4;
> + val |= (TIMER_EN | TIMER_PERIODIC);
> + writel(val, wdt->tmr_regs + TIMER_PTV);
> +
> + /*
> + * Set number of periods and start counter.
> + *
> + * Interrupt handler is not required for user space
> + * WDT accesses, since the caller is responsible to ping the
> + * WDT to reset the counter before expiration, through ioctls.
> + */
> + val = WDT_TIMER_ID |
> + (wdd->timeout << WDT_CFG_PERIOD_SHIFT) |
> + WDT_CFG_PMC2CAR_RST_EN;
> + writel(val, wdt->wdt_regs + WDT_CFG);
> +
> + writel(WDT_CMD_START_COUNTER, wdt->wdt_regs + WDT_CMD);
> +
> + return 0;
> +}
> +
> +static int tegra_wdt_stop(struct watchdog_device *wdd)
> +{
> + struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> + writel(WDT_UNLOCK_PATTERN, wdt->wdt_regs + WDT_UNLOCK);
> + writel(WDT_CMD_DISABLE_COUNTER, wdt->wdt_regs + WDT_CMD);
> + writel(0, wdt->tmr_regs + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static int tegra_wdt_ping(struct watchdog_device *wdd)
> +{
> + struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> + writel(WDT_CMD_START_COUNTER, wdt->wdt_regs + WDT_CMD);
> +
> + return 0;
> +}
> +
> +static int tegra_wdt_set_timeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + int ret;
> +
> + ret = tegra_wdt_stop(wdd);
> + if (ret)
> + return ret;
Unusual. Is it necessary to stop the wdt to change its timeout ?
If so, might be worth a comment.
> +
> + wdd->timeout = clamp_t(int, timeout, MIN_WDT_TIMEOUT, MAX_WDT_TIMEOUT);
> +
This is checked by the infrastructure.
> + if (watchdog_active(wdd))
> + return tegra_wdt_start(wdd);
> +
> + return 0;
> +}
> +
> +static unsigned int tegra_wdt_get_timeleft(struct watchdog_device *wdd)
> +{
> + struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
> + u32 val;
> + int count;
> + int exp;
> +
> + val = readl(wdt->wdt_regs + WDT_STS);
> +
> + /* Current countdown (from timeout) */
> + count = (val >> WDT_STS_COUNT_SHIFT) & WDT_STS_COUNT_MASK;
> +
> + /* Number of expirations (we are waiting for the 4th expiration) */
> + exp = (val >> WDT_STS_EXP_SHIFT) & WDT_STS_EXP_MASK;
> +
> + /*
> + * The entire thing is divided by 4 because we are ticking down 4 times
> + * faster due to needing to wait for the 4th expiration.
> + */
> + return (((3 - exp) * wdd->timeout) + count) / 4;
> +}
> +
> +static void tegra_wdt_ref(struct watchdog_device *wdd)
> +{
> + struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> + kref_get(&wdt->kref);
> +}
> +
> +static void tegra_wdt_unref(struct watchdog_device *wdd)
> +{
> + struct tegra_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> + kref_put(&wdt->kref, tegra_wdt_release_resources);
> +}
> +
> +static const struct watchdog_info tegra_wdt_info = {
> + .options = WDIOF_SETTIMEOUT |
> + WDIOF_MAGICCLOSE |
> + WDIOF_KEEPALIVEPING,
> + .firmware_version = 0,
> + .identity = "Tegra Watchdog",
> +};
> +
> +static struct watchdog_ops tegra_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = tegra_wdt_start,
> + .stop = tegra_wdt_stop,
> + .ping = tegra_wdt_ping,
> + .set_timeout = tegra_wdt_set_timeout,
> + .get_timeleft = tegra_wdt_get_timeleft,
> + .ref = tegra_wdt_ref,
> + .ref = tegra_wdt_unref,
I am quite at loss what the reference counting is for, but I guess
I don't have to understand it ;-).
Anyway, assigning the unref function to the ref variable definitely
doesn't make much sense.
> +};
> +
> +static int tegra_wdt_probe(struct platform_device *pdev)
> +{
> + struct watchdog_device *wdd;
> + struct tegra_wdt *wdt;
> + struct resource *res;
> + void __iomem *regs;
> + int ret;
> +
> + /* This is the timer base. */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "incorrect resources\n");
> + return -ENOENT;
"No such file or directory" ? -ENODEV or -EINVAL seems more appropriate.
Besides, devm_ioremap_resource() creates that same error message and
returns -EINVAL.
> + }
> +
> + regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR_OR_NULL(regs)) {
Does not return NULL on error.
> + dev_err(&pdev->dev, "unable to map registers\n");
devm_ioremap_resource() already creates an error message.
No need to repeat it.
> + return -ENOMEM;
return PTR_ERR(regs);
> + }
> +
> + /*
> + * Allocate our watchdog driver data, which has the
> + * struct watchdog_device nested within it.
> + */
> + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> + if (!wdt) {
> + dev_err(&pdev->dev, "out of memory\n");
The memory allocation code already dumps an error message.
No need to repeat it.
> + return -ENOMEM;
> + }
> +
> + /* Initialize struct tegra_wdt. */
> + wdt->wdt_regs = regs + WDT_BASE;
> + wdt->tmr_regs = regs + WDT_TIMER_BASE;
> +
> + /* Initialize struct watchdog_device. */
> + wdd = &wdt->wdd;
> + wdd->timeout = heartbeat;
> + wdd->info = &tegra_wdt_info;
> + wdd->ops = &tegra_wdt_ops;
> + wdd->min_timeout = MIN_WDT_TIMEOUT;
> + wdd->max_timeout = MAX_WDT_TIMEOUT;
> +
> + watchdog_set_drvdata(wdd, wdt);
> +
> + kref_init(&wdt->kref);
> +
> + ret = tegra_wdt_stop(wdd);
Sure you want to stop the watchdog if it is running ?
If that is the case, chances are that it is running on purpose,
eg if the watchdog is enabled in rommon/bios.
Besides, this really defeats nowayout. Just unload the driver,
load it again, and unload it again, and the watchdog is stopped.
> + if (ret) {
tegra_wdt_stop() always returns 0. I don't think it makes much sense
to have dummy error checking code each time you call it,
> + dev_err(&pdev->dev,
> + "failed to stop watchdog device\n");
much less an error message which will never be displayed
and just takes space in the code.
> + return ret;
> + }
> +
> + watchdog_set_nowayout(wdd, nowayout);
> +
> + ret = watchdog_register_device(wdd);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "failed to register watchdog device\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, wdt);
> +
> + dev_info(&pdev->dev,
> + "initialized wdt %d, using timer %d\n", WDT_ID, WDT_TIMER_ID);
I don't think writing those constants into the log provides much
if any value.
> + dev_info(&pdev->dev,
> + "enabled wdt %d (heartbeat = %d sec, nowayout = %d)\n",
> + WDT_ID, heartbeat, nowayout);
> +
Seems to me you stopped the watchdog above. Sure, it is initialized, but
I don't think you can claim that it is enabled.
> + return 0;
> +}
> +
> +static int tegra_wdt_remove(struct platform_device *pdev)
> +{
> + struct tegra_wdt *wdt = platform_get_drvdata(pdev);
> + int ret;
> +
> + ret = tegra_wdt_stop(&wdt->wdd);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to stop watchdog %d\n", WDT_ID);
> + return ret;
> + }
> +
> + watchdog_unregister_device(&wdt->wdd);
> +
> + kref_put(&wdt->kref, tegra_wdt_release_resources);
> +
> + dev_info(&pdev->dev, "removed wdt %d\n", WDT_ID);
> +
Same as above - %d is always 0 and doesn't provide any value.
If anything it is confising as it suggests that there might be
a wdt 1 as well. Same applies everywhere else where you refer
to wdt 0.
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra_wdt_runtime_suspend(struct device *dev)
> +{
> + struct tegra_wdt *wdt = dev_get_drvdata(dev);
> + int ret;
> +
> + if (watchdog_active(&wdt->wdd)) {
> + dev_info(dev, "suspend, stopping watchdog timer\n");
> +
I don't think that is worth clogging the log for.
> + ret = tegra_wdt_stop(&wdt->wdd);
> + if (ret) {
> + dev_err(dev, "failed to stop watchdog %d\n", WDT_ID);
> + return ret;
> + }
Same as above - this error checking and the message is really quite value-free.
> + }
> +
> + return 0;
> +}
> +
> +static int tegra_wdt_runtime_resume(struct device *dev)
> +{
> + struct tegra_wdt *wdt = dev_get_drvdata(dev);
> + int ret;
> +
> + if (watchdog_active(&wdt->wdd)) {
> + dev_info(dev, "resume, restarting watchdog timer\n");
> +
Same here.
> + ret = tegra_wdt_start(&wdt->wdd);
> + if (ret) {
> + dev_err(dev, "failed to restart watchdog %d\n", WDT_ID);
> + return ret;
> + }
Same here. Never happens.
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct of_device_id tegra_wdt_of_match[] = {
> + { .compatible = "nvidia,tegra30-timer", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, tegra_wdt_of_match);
> +
> +static const struct dev_pm_ops tegra_wdt_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(tegra_wdt_runtime_suspend,
> + tegra_wdt_runtime_resume)
> +};
> +
> +static struct platform_driver tegra_wdt_driver = {
> + .probe = tegra_wdt_probe,
> + .remove = tegra_wdt_remove,
> + .driver = {
> + .owner = THIS_MODULE,
> + .name = "tegra-wdt",
> + .pm = &tegra_wdt_pm_ops,
> + .of_match_table = tegra_wdt_of_match,
> + },
> +};
> +module_platform_driver(tegra_wdt_driver);
> +
> +MODULE_AUTHOR("NVIDIA Corporation");
> +MODULE_DESCRIPTION("Tegra Watchdog Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
>
The MISCDEV alias is not needed. We just got rid of all those.
Please don't re-introduce it.
Thanks,
Guenter
On 02/05/2014 08:38 PM, Andrew Chew wrote:
> Add a driver for the hardware watchdogs in NVIDIA Tegra SoCs (Tegra30 and
> later). This driver will configure one watchdog timer that will reset the
> system in the case of a watchdog timeout.
>
> This driver binds to the nvidia,tegra30-timer device node and gets its
> register base from there.
Can you provide some instructions on how to test this patch on Tegra
boards? Please make sure those instructions are on our internal upstream
testing wiki page too; I'll mail you the link internally.