2019-08-21 02:28:47

by Anson Huang

[permalink] [raw]
Subject: [PATCH V3 1/4] dt-bindings: watchdog: Add i.MX7ULP bindings

Add the watchdog bindings for Freescale i.MX7ULP.

Signed-off-by: Anson Huang <[email protected]>
---
No changes.
---
.../bindings/watchdog/fsl-imx7ulp-wdt.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/fsl-imx7ulp-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx7ulp-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx7ulp-wdt.txt
new file mode 100644
index 0000000..d83fc5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx7ulp-wdt.txt
@@ -0,0 +1,22 @@
+* Freescale i.MX7ULP Watchdog Timer (WDT) Controller
+
+Required properties:
+- compatible : Should be "fsl,imx7ulp-wdt"
+- reg : Should contain WDT registers location and length
+- interrupts : Should contain WDT interrupt
+- clocks: Should contain a phandle pointing to the gated peripheral clock.
+
+Optional properties:
+- timeout-sec : Contains the watchdog timeout in seconds
+
+Examples:
+
+wdog1: wdog@403d0000 {
+ compatible = "fsl,imx7ulp-wdt";
+ reg = <0x403d0000 0x10000>;
+ interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pcc2 IMX7ULP_CLK_WDG1>;
+ assigned-clocks = <&pcc2 IMX7ULP_CLK_WDG1>;
+ assigned-clocks-parents = <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>;
+ timeout-sec = <40>;
+};
--
2.7.4


2019-08-21 02:41:47

by Anson Huang

[permalink] [raw]
Subject: [PATCH V3 3/4] ARM: imx_v6_v7_defconfig: Enable CONFIG_IMX7ULP_WDT by default

Select CONFIG_IMX7ULP_WDT by default to support i.MX7ULP watchdog.

Signed-off-by: Anson Huang <[email protected]>
---
No changes.
---
arch/arm/configs/imx_v6_v7_defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 9bfffbe..be2a694 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -236,6 +236,7 @@ CONFIG_DA9062_WATCHDOG=y
CONFIG_DA9063_WATCHDOG=m
CONFIG_RN5T618_WATCHDOG=y
CONFIG_IMX2_WDT=y
+CONFIG_IMX7ULP_WDT=y
CONFIG_MFD_DA9052_I2C=y
CONFIG_MFD_DA9062=y
CONFIG_MFD_DA9063=y
--
2.7.4

2019-08-21 02:41:51

by Anson Huang

[permalink] [raw]
Subject: [PATCH V3 2/4] watchdog: Add i.MX7ULP watchdog support

The i.MX7ULP Watchdog Timer (WDOG) module is an independent timer
that is available for system use.
It provides a safety feature to ensure that software is executing
as planned and that the CPU is not stuck in an infinite loop or
executing unintended code. If the WDOG module is not serviced
(refreshed) within a certain period, it resets the MCU.

Add driver support for i.MX7ULP watchdog.

Signed-off-by: Anson Huang <[email protected]>
---
Changes since V2:
- add devm_add_action_or_reset to disable clk for remove action.
---
drivers/watchdog/Kconfig | 13 +++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/imx7ulp_wdt.c | 246 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 260 insertions(+)
create mode 100644 drivers/watchdog/imx7ulp_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index a8f5c81..d68e5b5 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -724,6 +724,19 @@ config IMX_SC_WDT
To compile this driver as a module, choose M here: the
module will be called imx_sc_wdt.

+config IMX7ULP_WDT
+ tristate "IMX7ULP Watchdog"
+ depends on ARCH_MXC || COMPILE_TEST
+ select WATCHDOG_CORE
+ help
+ This is the driver for the hardware watchdog on the Freescale
+ IMX7ULP and later processors. If you have one of these
+ processors and wish to have watchdog support enabled,
+ say Y, otherwise say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx7ulp_wdt.
+
config UX500_WATCHDOG
tristate "ST-Ericsson Ux500 watchdog"
depends on MFD_DB8500_PRCMU
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index b5a0aed..2ee352b 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
+obj-$(CONFIG_IMX7ULP_WDT) += imx7ulp_wdt.o
obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
new file mode 100644
index 0000000..5d37957
--- /dev/null
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 NXP.
+ */
+
+#include <linux/clk.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/watchdog.h>
+
+#define WDOG_CS 0x0
+#define WDOG_CS_CMD32EN BIT(13)
+#define WDOG_CS_ULK BIT(11)
+#define WDOG_CS_RCS BIT(10)
+#define WDOG_CS_EN BIT(7)
+#define WDOG_CS_UPDATE BIT(5)
+
+#define WDOG_CNT 0x4
+#define WDOG_TOVAL 0x8
+
+#define REFRESH_SEQ0 0xA602
+#define REFRESH_SEQ1 0xB480
+#define REFRESH ((REFRESH_SEQ1 << 16) | REFRESH_SEQ0)
+
+#define UNLOCK_SEQ0 0xC520
+#define UNLOCK_SEQ1 0xD928
+#define UNLOCK ((UNLOCK_SEQ1 << 16) | UNLOCK_SEQ0)
+
+#define DEFAULT_TIMEOUT 60
+#define MAX_TIMEOUT 128
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0000);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+ __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+struct imx7ulp_wdt_device {
+ struct notifier_block restart_handler;
+ struct watchdog_device wdd;
+ void __iomem *base;
+ struct clk *clk;
+ int rate;
+};
+
+static inline void imx7ulp_wdt_enable(void __iomem *base, bool enable)
+{
+ u32 val = readl(base + WDOG_CS);
+
+ writel(UNLOCK, base + WDOG_CNT);
+ if (enable)
+ writel(val | WDOG_CS_EN, base + WDOG_CS);
+ else
+ writel(val & ~WDOG_CS_EN, base + WDOG_CS);
+}
+
+static inline bool imx7ulp_wdt_is_enabled(void __iomem *base)
+{
+ u32 val = readl(base + WDOG_CS);
+
+ return val & WDOG_CS_EN;
+}
+
+static int imx7ulp_wdt_ping(struct watchdog_device *wdog)
+{
+ struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
+
+ writel(REFRESH, wdt->base + WDOG_CNT);
+
+ return 0;
+}
+
+static int imx7ulp_wdt_start(struct watchdog_device *wdog)
+{
+ struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
+
+ imx7ulp_wdt_enable(wdt->base, true);
+
+ return 0;
+}
+
+static int imx7ulp_wdt_stop(struct watchdog_device *wdog)
+{
+ struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
+
+ imx7ulp_wdt_enable(wdt->base, false);
+
+ return 0;
+}
+
+static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
+ unsigned int timeout)
+{
+ struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
+ u32 val = wdt->rate * timeout;
+
+ writel(UNLOCK, wdt->base + WDOG_CNT);
+ writel(val, wdt->base + WDOG_TOVAL);
+
+ wdog->timeout = timeout;
+
+ return 0;
+}
+
+static const struct watchdog_ops imx7ulp_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = imx7ulp_wdt_start,
+ .stop = imx7ulp_wdt_stop,
+ .ping = imx7ulp_wdt_ping,
+ .set_timeout = imx7ulp_wdt_set_timeout,
+};
+
+static const struct watchdog_info imx7ulp_wdt_info = {
+ .identity = "i.MX7ULP watchdog timer",
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+ WDIOF_MAGICCLOSE,
+};
+
+static inline void imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
+{
+ u32 val;
+
+ /* unlock the wdog for reconfiguration */
+ writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
+ writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
+
+ /* set an initial timeout value in TOVAL */
+ writel(timeout, base + WDOG_TOVAL);
+ /* enable 32bit command sequence and reconfigure */
+ val = BIT(13) | BIT(8) | BIT(5);
+ writel(val, base + WDOG_CS);
+}
+
+static void imx7ulp_wdt_action(void *data)
+{
+ struct imx7ulp_wdt_device *imx7ulp_wdt = data;
+
+ clk_disable_unprepare(imx7ulp_wdt->clk);
+}
+
+static int imx7ulp_wdt_probe(struct platform_device *pdev)
+{
+ struct imx7ulp_wdt_device *imx7ulp_wdt;
+ struct device *dev = &pdev->dev;
+ struct watchdog_device *wdog;
+ int ret;
+
+ imx7ulp_wdt = devm_kzalloc(dev, sizeof(*imx7ulp_wdt), GFP_KERNEL);
+ if (!imx7ulp_wdt)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, imx7ulp_wdt);
+
+ imx7ulp_wdt->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(imx7ulp_wdt->base))
+ return PTR_ERR(imx7ulp_wdt->base);
+
+ imx7ulp_wdt->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(imx7ulp_wdt->clk)) {
+ dev_err(dev, "Failed to get watchdog clock\n");
+ return PTR_ERR(imx7ulp_wdt->clk);
+ }
+
+ ret = clk_prepare_enable(imx7ulp_wdt->clk);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, imx7ulp_wdt_action, imx7ulp_wdt);
+ if (ret)
+ return ret;
+
+ imx7ulp_wdt->rate = 1000;
+ wdog = &imx7ulp_wdt->wdd;
+ wdog->info = &imx7ulp_wdt_info;
+ wdog->ops = &imx7ulp_wdt_ops;
+ wdog->min_timeout = 1;
+ wdog->max_timeout = MAX_TIMEOUT;
+ wdog->parent = dev;
+ wdog->timeout = DEFAULT_TIMEOUT;
+
+ watchdog_init_timeout(wdog, 0, dev);
+ watchdog_stop_on_reboot(wdog);
+ watchdog_stop_on_unregister(wdog);
+ watchdog_set_drvdata(wdog, imx7ulp_wdt);
+ imx7ulp_wdt_init(imx7ulp_wdt->base, wdog->timeout * imx7ulp_wdt->rate);
+
+ return devm_watchdog_register_device(dev, wdog);
+}
+
+static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev)
+{
+ struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
+
+ if (watchdog_active(&imx7ulp_wdt->wdd))
+ imx7ulp_wdt_stop(&imx7ulp_wdt->wdd);
+
+ clk_disable_unprepare(imx7ulp_wdt->clk);
+
+ return 0;
+}
+
+static int __maybe_unused imx7ulp_wdt_resume(struct device *dev)
+{
+ struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
+ u32 timeout = imx7ulp_wdt->wdd.timeout * imx7ulp_wdt->rate;
+ int ret;
+
+ ret = clk_prepare_enable(imx7ulp_wdt->clk);
+ if (ret)
+ return ret;
+
+ if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
+ imx7ulp_wdt_init(imx7ulp_wdt->base, timeout);
+
+ if (watchdog_active(&imx7ulp_wdt->wdd))
+ imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(imx7ulp_wdt_pm_ops, imx7ulp_wdt_suspend,
+ imx7ulp_wdt_resume);
+
+static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
+ { .compatible = "fsl,imx7ulp-wdt", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx7ulp_wdt_dt_ids);
+
+static struct platform_driver imx7ulp_wdt_driver = {
+ .probe = imx7ulp_wdt_probe,
+ .driver = {
+ .name = "imx7ulp-wdt",
+ .pm = &imx7ulp_wdt_pm_ops,
+ .of_match_table = imx7ulp_wdt_dt_ids,
+ },
+};
+module_platform_driver(imx7ulp_wdt_driver);
+
+MODULE_AUTHOR("Anson Huang <[email protected]>");
+MODULE_DESCRIPTION("Freescale i.MX7ULP watchdog driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2019-08-21 14:16:32

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH V3 2/4] watchdog: Add i.MX7ULP watchdog support

On Tue, Aug 20, 2019 at 10:07:56PM -0400, Anson Huang wrote:
> The i.MX7ULP Watchdog Timer (WDOG) module is an independent timer
> that is available for system use.
> It provides a safety feature to ensure that software is executing
> as planned and that the CPU is not stuck in an infinite loop or
> executing unintended code. If the WDOG module is not serviced
> (refreshed) within a certain period, it resets the MCU.
>
> Add driver support for i.MX7ULP watchdog.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> Changes since V2:
> - add devm_add_action_or_reset to disable clk for remove action.
> ---
> drivers/watchdog/Kconfig | 13 +++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/imx7ulp_wdt.c | 246 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 260 insertions(+)
> create mode 100644 drivers/watchdog/imx7ulp_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index a8f5c81..d68e5b5 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -724,6 +724,19 @@ config IMX_SC_WDT
> To compile this driver as a module, choose M here: the
> module will be called imx_sc_wdt.
>
> +config IMX7ULP_WDT
> + tristate "IMX7ULP Watchdog"
> + depends on ARCH_MXC || COMPILE_TEST
> + select WATCHDOG_CORE
> + help
> + This is the driver for the hardware watchdog on the Freescale
> + IMX7ULP and later processors. If you have one of these
> + processors and wish to have watchdog support enabled,
> + say Y, otherwise say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called imx7ulp_wdt.
> +
> config UX500_WATCHDOG
> tristate "ST-Ericsson Ux500 watchdog"
> depends on MFD_DB8500_PRCMU
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index b5a0aed..2ee352b 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
> obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
> obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
> obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
> +obj-$(CONFIG_IMX7ULP_WDT) += imx7ulp_wdt.o
> obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
> obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
> obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> new file mode 100644
> index 0000000..5d37957
> --- /dev/null
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 NXP.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/watchdog.h>
> +
> +#define WDOG_CS 0x0
> +#define WDOG_CS_CMD32EN BIT(13)
> +#define WDOG_CS_ULK BIT(11)
> +#define WDOG_CS_RCS BIT(10)
> +#define WDOG_CS_EN BIT(7)
> +#define WDOG_CS_UPDATE BIT(5)
> +
> +#define WDOG_CNT 0x4
> +#define WDOG_TOVAL 0x8
> +
> +#define REFRESH_SEQ0 0xA602
> +#define REFRESH_SEQ1 0xB480
> +#define REFRESH ((REFRESH_SEQ1 << 16) | REFRESH_SEQ0)
> +
> +#define UNLOCK_SEQ0 0xC520
> +#define UNLOCK_SEQ1 0xD928
> +#define UNLOCK ((UNLOCK_SEQ1 << 16) | UNLOCK_SEQ0)
> +
> +#define DEFAULT_TIMEOUT 60
> +#define MAX_TIMEOUT 128
> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0000);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +struct imx7ulp_wdt_device {
> + struct notifier_block restart_handler;
> + struct watchdog_device wdd;
> + void __iomem *base;
> + struct clk *clk;
> + int rate;
> +};
> +
> +static inline void imx7ulp_wdt_enable(void __iomem *base, bool enable)
> +{
> + u32 val = readl(base + WDOG_CS);
> +
> + writel(UNLOCK, base + WDOG_CNT);
> + if (enable)
> + writel(val | WDOG_CS_EN, base + WDOG_CS);
> + else
> + writel(val & ~WDOG_CS_EN, base + WDOG_CS);
> +}
> +
> +static inline bool imx7ulp_wdt_is_enabled(void __iomem *base)
> +{
> + u32 val = readl(base + WDOG_CS);
> +
> + return val & WDOG_CS_EN;
> +}
> +
> +static int imx7ulp_wdt_ping(struct watchdog_device *wdog)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +
> + writel(REFRESH, wdt->base + WDOG_CNT);
> +
> + return 0;
> +}
> +
> +static int imx7ulp_wdt_start(struct watchdog_device *wdog)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +
> + imx7ulp_wdt_enable(wdt->base, true);
> +
> + return 0;
> +}
> +
> +static int imx7ulp_wdt_stop(struct watchdog_device *wdog)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +
> + imx7ulp_wdt_enable(wdt->base, false);
> +
> + return 0;
> +}
> +
> +static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
> + unsigned int timeout)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> + u32 val = wdt->rate * timeout;
> +
> + writel(UNLOCK, wdt->base + WDOG_CNT);
> + writel(val, wdt->base + WDOG_TOVAL);
> +
> + wdog->timeout = timeout;
> +
> + return 0;
> +}
> +
> +static const struct watchdog_ops imx7ulp_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = imx7ulp_wdt_start,
> + .stop = imx7ulp_wdt_stop,
> + .ping = imx7ulp_wdt_ping,
> + .set_timeout = imx7ulp_wdt_set_timeout,
> +};
> +
> +static const struct watchdog_info imx7ulp_wdt_info = {
> + .identity = "i.MX7ULP watchdog timer",
> + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> + WDIOF_MAGICCLOSE,
> +};
> +
> +static inline void imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
> +{
> + u32 val;
> +
> + /* unlock the wdog for reconfiguration */
> + writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
> + writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> +
> + /* set an initial timeout value in TOVAL */
> + writel(timeout, base + WDOG_TOVAL);
> + /* enable 32bit command sequence and reconfigure */
> + val = BIT(13) | BIT(8) | BIT(5);
> + writel(val, base + WDOG_CS);
> +}
> +
> +static void imx7ulp_wdt_action(void *data)
> +{
> + struct imx7ulp_wdt_device *imx7ulp_wdt = data;
> +
> + clk_disable_unprepare(imx7ulp_wdt->clk);

If you had passed imx7ulp_wdt->clk as parameter, the dereference here
would not be necessary.

> +}
> +
> +static int imx7ulp_wdt_probe(struct platform_device *pdev)
> +{
> + struct imx7ulp_wdt_device *imx7ulp_wdt;
> + struct device *dev = &pdev->dev;
> + struct watchdog_device *wdog;
> + int ret;
> +
> + imx7ulp_wdt = devm_kzalloc(dev, sizeof(*imx7ulp_wdt), GFP_KERNEL);
> + if (!imx7ulp_wdt)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, imx7ulp_wdt);
> +
> + imx7ulp_wdt->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(imx7ulp_wdt->base))
> + return PTR_ERR(imx7ulp_wdt->base);
> +
> + imx7ulp_wdt->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(imx7ulp_wdt->clk)) {
> + dev_err(dev, "Failed to get watchdog clock\n");
> + return PTR_ERR(imx7ulp_wdt->clk);
> + }
> +
> + ret = clk_prepare_enable(imx7ulp_wdt->clk);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev, imx7ulp_wdt_action, imx7ulp_wdt);
> + if (ret)
> + return ret;
> +
> + imx7ulp_wdt->rate = 1000;

I forgot to ask (sorry, I had noticed, but I guess I forgot).

Why not clk_get_rate() ? If the clock rate is fixed, why bother with a 'rate'
variable ? You could use a constant instead whereever it is used.

Thanks
Guenter

> + wdog = &imx7ulp_wdt->wdd;
> + wdog->info = &imx7ulp_wdt_info;
> + wdog->ops = &imx7ulp_wdt_ops;
> + wdog->min_timeout = 1;
> + wdog->max_timeout = MAX_TIMEOUT;
> + wdog->parent = dev;
> + wdog->timeout = DEFAULT_TIMEOUT;
> +
> + watchdog_init_timeout(wdog, 0, dev);
> + watchdog_stop_on_reboot(wdog);
> + watchdog_stop_on_unregister(wdog);
> + watchdog_set_drvdata(wdog, imx7ulp_wdt);
> + imx7ulp_wdt_init(imx7ulp_wdt->base, wdog->timeout * imx7ulp_wdt->rate);
> +
> + return devm_watchdog_register_device(dev, wdog);
> +}
> +
> +static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev)
> +{
> + struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&imx7ulp_wdt->wdd))
> + imx7ulp_wdt_stop(&imx7ulp_wdt->wdd);
> +
> + clk_disable_unprepare(imx7ulp_wdt->clk);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx7ulp_wdt_resume(struct device *dev)
> +{
> + struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
> + u32 timeout = imx7ulp_wdt->wdd.timeout * imx7ulp_wdt->rate;
> + int ret;
> +
> + ret = clk_prepare_enable(imx7ulp_wdt->clk);
> + if (ret)
> + return ret;
> +
> + if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
> + imx7ulp_wdt_init(imx7ulp_wdt->base, timeout);
> +
> + if (watchdog_active(&imx7ulp_wdt->wdd))
> + imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx7ulp_wdt_pm_ops, imx7ulp_wdt_suspend,
> + imx7ulp_wdt_resume);
> +
> +static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
> + { .compatible = "fsl,imx7ulp-wdt", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx7ulp_wdt_dt_ids);
> +
> +static struct platform_driver imx7ulp_wdt_driver = {
> + .probe = imx7ulp_wdt_probe,
> + .driver = {
> + .name = "imx7ulp-wdt",
> + .pm = &imx7ulp_wdt_pm_ops,
> + .of_match_table = imx7ulp_wdt_dt_ids,
> + },
> +};
> +module_platform_driver(imx7ulp_wdt_driver);
> +
> +MODULE_AUTHOR("Anson Huang <[email protected]>");
> +MODULE_DESCRIPTION("Freescale i.MX7ULP watchdog driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>

2019-08-22 03:27:41

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V3 2/4] watchdog: Add i.MX7ULP watchdog support

Hi, Guenter

> On Tue, Aug 20, 2019 at 10:07:56PM -0400, Anson Huang wrote:
> > The i.MX7ULP Watchdog Timer (WDOG) module is an independent timer
> that
> > is available for system use.
> > It provides a safety feature to ensure that software is executing as
> > planned and that the CPU is not stuck in an infinite loop or executing
> > unintended code. If the WDOG module is not serviced
> > (refreshed) within a certain period, it resets the MCU.
> >
> > Add driver support for i.MX7ULP watchdog.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > Changes since V2:
> > - add devm_add_action_or_reset to disable clk for remove action.
> > ---
> > drivers/watchdog/Kconfig | 13 +++
> > drivers/watchdog/Makefile | 1 +
> > drivers/watchdog/imx7ulp_wdt.c | 246
> > +++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 260 insertions(+)
> > create mode 100644 drivers/watchdog/imx7ulp_wdt.c
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
> > a8f5c81..d68e5b5 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -724,6 +724,19 @@ config IMX_SC_WDT
> > To compile this driver as a module, choose M here: the
> > module will be called imx_sc_wdt.
> >
> > +config IMX7ULP_WDT
> > + tristate "IMX7ULP Watchdog"
> > + depends on ARCH_MXC || COMPILE_TEST
> > + select WATCHDOG_CORE
> > + help
> > + This is the driver for the hardware watchdog on the Freescale
> > + IMX7ULP and later processors. If you have one of these
> > + processors and wish to have watchdog support enabled,
> > + say Y, otherwise say N.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called imx7ulp_wdt.
> > +
> > config UX500_WATCHDOG
> > tristate "ST-Ericsson Ux500 watchdog"
> > depends on MFD_DB8500_PRCMU
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index b5a0aed..2ee352b 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -67,6 +67,7 @@ obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
> > obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
> > obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
> > obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
> > +obj-$(CONFIG_IMX7ULP_WDT) += imx7ulp_wdt.o
> > obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
> > obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
> > obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o diff --git
> > a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> new
> > file mode 100644 index 0000000..5d37957
> > --- /dev/null
> > +++ b/drivers/watchdog/imx7ulp_wdt.c
> > @@ -0,0 +1,246 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2019 NXP.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reboot.h>
> > +#include <linux/watchdog.h>
> > +
> > +#define WDOG_CS 0x0
> > +#define WDOG_CS_CMD32EN BIT(13)
> > +#define WDOG_CS_ULK BIT(11)
> > +#define WDOG_CS_RCS BIT(10)
> > +#define WDOG_CS_EN BIT(7)
> > +#define WDOG_CS_UPDATE BIT(5)
> > +
> > +#define WDOG_CNT 0x4
> > +#define WDOG_TOVAL 0x8
> > +
> > +#define REFRESH_SEQ0 0xA602
> > +#define REFRESH_SEQ1 0xB480
> > +#define REFRESH ((REFRESH_SEQ1 << 16) | REFRESH_SEQ0)
> > +
> > +#define UNLOCK_SEQ0 0xC520
> > +#define UNLOCK_SEQ1 0xD928
> > +#define UNLOCK ((UNLOCK_SEQ1 << 16) | UNLOCK_SEQ0)
> > +
> > +#define DEFAULT_TIMEOUT 60
> > +#define MAX_TIMEOUT 128
> > +
> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> module_param(nowayout,
> > +bool, 0000); MODULE_PARM_DESC(nowayout, "Watchdog cannot be
> stopped
> > +once started (default="
> > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +struct imx7ulp_wdt_device {
> > + struct notifier_block restart_handler;
> > + struct watchdog_device wdd;
> > + void __iomem *base;
> > + struct clk *clk;
> > + int rate;
> > +};
> > +
> > +static inline void imx7ulp_wdt_enable(void __iomem *base, bool
> > +enable) {
> > + u32 val = readl(base + WDOG_CS);
> > +
> > + writel(UNLOCK, base + WDOG_CNT);
> > + if (enable)
> > + writel(val | WDOG_CS_EN, base + WDOG_CS);
> > + else
> > + writel(val & ~WDOG_CS_EN, base + WDOG_CS); }
> > +
> > +static inline bool imx7ulp_wdt_is_enabled(void __iomem *base) {
> > + u32 val = readl(base + WDOG_CS);
> > +
> > + return val & WDOG_CS_EN;
> > +}
> > +
> > +static int imx7ulp_wdt_ping(struct watchdog_device *wdog) {
> > + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> > +
> > + writel(REFRESH, wdt->base + WDOG_CNT);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx7ulp_wdt_start(struct watchdog_device *wdog) {
> > + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> > +
> > + imx7ulp_wdt_enable(wdt->base, true);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx7ulp_wdt_stop(struct watchdog_device *wdog) {
> > + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> > +
> > + imx7ulp_wdt_enable(wdt->base, false);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
> > + unsigned int timeout)
> > +{
> > + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> > + u32 val = wdt->rate * timeout;
> > +
> > + writel(UNLOCK, wdt->base + WDOG_CNT);
> > + writel(val, wdt->base + WDOG_TOVAL);
> > +
> > + wdog->timeout = timeout;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct watchdog_ops imx7ulp_wdt_ops = {
> > + .owner = THIS_MODULE,
> > + .start = imx7ulp_wdt_start,
> > + .stop = imx7ulp_wdt_stop,
> > + .ping = imx7ulp_wdt_ping,
> > + .set_timeout = imx7ulp_wdt_set_timeout, };
> > +
> > +static const struct watchdog_info imx7ulp_wdt_info = {
> > + .identity = "i.MX7ULP watchdog timer",
> > + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> > + WDIOF_MAGICCLOSE,
> > +};
> > +
> > +static inline void imx7ulp_wdt_init(void __iomem *base, unsigned int
> > +timeout) {
> > + u32 val;
> > +
> > + /* unlock the wdog for reconfiguration */
> > + writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
> > + writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> > +
> > + /* set an initial timeout value in TOVAL */
> > + writel(timeout, base + WDOG_TOVAL);
> > + /* enable 32bit command sequence and reconfigure */
> > + val = BIT(13) | BIT(8) | BIT(5);
> > + writel(val, base + WDOG_CS);
> > +}
> > +
> > +static void imx7ulp_wdt_action(void *data) {
> > + struct imx7ulp_wdt_device *imx7ulp_wdt = data;
> > +
> > + clk_disable_unprepare(imx7ulp_wdt->clk);
>
> If you had passed imx7ulp_wdt->clk as parameter, the dereference here
> would not be necessary.

Yup, I thought we may need other data later, so I just passed whole structure.
I agree with your idea, for now, we ONLY need the clk, so I will ONLY pass the clk
to save the dereference operation. Will send out V4.

>
> > +}
> > +
> > +static int imx7ulp_wdt_probe(struct platform_device *pdev) {
> > + struct imx7ulp_wdt_device *imx7ulp_wdt;
> > + struct device *dev = &pdev->dev;
> > + struct watchdog_device *wdog;
> > + int ret;
> > +
> > + imx7ulp_wdt = devm_kzalloc(dev, sizeof(*imx7ulp_wdt),
> GFP_KERNEL);
> > + if (!imx7ulp_wdt)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, imx7ulp_wdt);
> > +
> > + imx7ulp_wdt->base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(imx7ulp_wdt->base))
> > + return PTR_ERR(imx7ulp_wdt->base);
> > +
> > + imx7ulp_wdt->clk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(imx7ulp_wdt->clk)) {
> > + dev_err(dev, "Failed to get watchdog clock\n");
> > + return PTR_ERR(imx7ulp_wdt->clk);
> > + }
> > +
> > + ret = clk_prepare_enable(imx7ulp_wdt->clk);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_add_action_or_reset(dev, imx7ulp_wdt_action,
> imx7ulp_wdt);
> > + if (ret)
> > + return ret;
> > +
> > + imx7ulp_wdt->rate = 1000;
>
> I forgot to ask (sorry, I had noticed, but I guess I forgot).
>
> Why not clk_get_rate() ? If the clock rate is fixed, why bother with a 'rate'
> variable ? You could use a constant instead whereever it is used.

OK, the clock rate is fixed on 7ULP, I will use a constant instead of the variable.

Thanks,
Anson

>
> Thanks
> Guenter
>
> > + wdog = &imx7ulp_wdt->wdd;
> > + wdog->info = &imx7ulp_wdt_info;
> > + wdog->ops = &imx7ulp_wdt_ops;
> > + wdog->min_timeout = 1;
> > + wdog->max_timeout = MAX_TIMEOUT;
> > + wdog->parent = dev;
> > + wdog->timeout = DEFAULT_TIMEOUT;
> > +
> > + watchdog_init_timeout(wdog, 0, dev);
> > + watchdog_stop_on_reboot(wdog);
> > + watchdog_stop_on_unregister(wdog);
> > + watchdog_set_drvdata(wdog, imx7ulp_wdt);
> > + imx7ulp_wdt_init(imx7ulp_wdt->base, wdog->timeout *
> > +imx7ulp_wdt->rate);
> > +
> > + return devm_watchdog_register_device(dev, wdog); }
> > +
> > +static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev) {
> > + struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
> > +
> > + if (watchdog_active(&imx7ulp_wdt->wdd))
> > + imx7ulp_wdt_stop(&imx7ulp_wdt->wdd);
> > +
> > + clk_disable_unprepare(imx7ulp_wdt->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused imx7ulp_wdt_resume(struct device *dev) {
> > + struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
> > + u32 timeout = imx7ulp_wdt->wdd.timeout * imx7ulp_wdt->rate;
> > + int ret;
> > +
> > + ret = clk_prepare_enable(imx7ulp_wdt->clk);
> > + if (ret)
> > + return ret;
> > +
> > + if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
> > + imx7ulp_wdt_init(imx7ulp_wdt->base, timeout);
> > +
> > + if (watchdog_active(&imx7ulp_wdt->wdd))
> > + imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
> > +
> > + return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(imx7ulp_wdt_pm_ops,
> imx7ulp_wdt_suspend,
> > + imx7ulp_wdt_resume);
> > +
> > +static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
> > + { .compatible = "fsl,imx7ulp-wdt", },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx7ulp_wdt_dt_ids);
> > +
> > +static struct platform_driver imx7ulp_wdt_driver = {
> > + .probe = imx7ulp_wdt_probe,
> > + .driver = {
> > + .name = "imx7ulp-wdt",
> > + .pm = &imx7ulp_wdt_pm_ops,
> > + .of_match_table = imx7ulp_wdt_dt_ids,
> > + },
> > +};
> > +module_platform_driver(imx7ulp_wdt_driver);
> > +
> > +MODULE_AUTHOR("Anson Huang <[email protected]>");
> > +MODULE_DESCRIPTION("Freescale i.MX7ULP watchdog driver");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.7.4
> >