Re: [PATCHv2 3/4] watchdog: Add K3 RTI watchdog support

From: Tero Kristo
Date: Wed Mar 04 2020 - 05:45:18 EST

On 04/03/2020 11:23, Guenter Roeck wrote:
On 3/3/20 10:57 PM, Tero Kristo wrote:
On 03/03/2020 23:18, Guenter Roeck wrote:
On Mon, Mar 02, 2020 at 10:04:25PM +0200, Tero Kristo wrote:
Texas Instruments K3 SoCs contain an RTI (Real Time Interrupt) module
which can be used as a watchdog. This IP provides a support for
windowed watchdog mode, in which the watchdog must be petted within
a certain time window. If it is petted either too soon, or too late,
a watchdog error will be triggered.

Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
---
v2:
ÂÂ * Added better documentation within the driver code
ÂÂ * Dropped fck handle, instead get the fck rate during probe only
ÂÂ * Modified the max_hw_heartbeat calculation logic a bit

 drivers/watchdog/Kconfig | 8 ++
 drivers/watchdog/Makefile | 1 +
 drivers/watchdog/rti_wdt.c | 254 +++++++++++++++++++++++++++++++++++++
 3 files changed, 261 insertions(+)
 create mode 100644 drivers/watchdog/rti_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index cec868f8db3f..81faf47d44a6 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -583,6 +583,14 @@ config DAVINCI_WATCHDOG
ÂÂÂÂÂÂÂ NOTE: once enabled, this timer cannot be disabled.
ÂÂÂÂÂÂÂ Say N if you are unsure.
 +config K3_RTI_WATCHDOG
+ÂÂÂ tristate "Texas Instruments K3 RTI watchdog"
+ÂÂÂ depends on ARCH_K3 || COMPILE_TEST
+ÂÂÂ select WATCHDOG_CORE
+ÂÂÂ help
+ÂÂÂÂÂ Say Y here if you want to include support for the K3 watchdog
+ÂÂÂÂÂ timer (RTI module) available in the K3 generation of processors.
+
 config ORION_WATCHDOG
ÂÂÂÂÂ tristate "Orion watchdog"
ÂÂÂÂÂ depends on ARCH_ORION5X || ARCH_DOVE || MACH_DOVE || ARCH_MVEBU || (COMPILE_TEST && !ARCH_EBSA110)
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2ee352bf3372..6de2e4ceef19 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_EP93XX_WATCHDOG) += ep93xx_wdt.o
 obj-$(CONFIG_PNX4008_WATCHDOG) += pnx4008_wdt.o
 obj-$(CONFIG_IOP_WATCHDOG) += iop_wdt.o
 obj-$(CONFIG_DAVINCI_WATCHDOG) += davinci_wdt.o
+obj-$(CONFIG_K3_RTI_WATCHDOG) += rti_wdt.o
 obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o
 obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o
 obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
new file mode 100644
index 000000000000..7a46c40891e2
--- /dev/null
+++ b/drivers/watchdog/rti_wdt.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Watchdog driver for the K3 RTI module
+ *
+ * (c) Copyright 2019-2020 Texas Instruments Inc.
+ * All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#define DEFAULT_HEARTBEAT 60
+
+/* Max heartbeat is calculated at 32kHz source clock */
+#define MAX_HEARTBEATÂÂÂ 1000
+
+/* Timer register set definition */
+#define RTIDWDCTRLÂÂÂ 0x90
+#define RTIDWDPRLDÂÂÂ 0x94
+#define RTIWDSTATUSÂÂÂ 0x98
+#define RTIWDKEYÂÂÂ 0x9c
+#define RTIDWDCNTRÂÂÂ 0xa0
+#define RTIWWDRXCTRLÂÂÂ 0xa4
+#define RTIWWDSIZECTRLÂÂÂ 0xa8
+
+#define RTIWWDRX_NMIÂÂÂ 0xa
+
+#define RTIWWDSIZE_50PÂÂÂ 0x50
+
+#define WDENABLE_KEYÂÂÂ 0xa98559da
+
+#define WDKEY_SEQ0ÂÂÂÂÂÂÂ 0xe51a
+#define WDKEY_SEQ1ÂÂÂÂÂÂÂ 0xa35c
+
+#define WDT_PRELOAD_SHIFTÂÂÂ 13
+
+#define WDT_PRELOAD_MAXÂÂÂÂÂÂÂ 0xfff
+
+#define DWDSTÂÂÂÂÂÂÂÂÂÂÂ BIT(1)
+
+static int heartbeat;
+
+/*
+ * struct to hold data for each WDT device
+ * @base - base io address of WD device
+ * @freq - source clock frequency of WDT
+ * @wdd - hold watchdog device as is in WDT core
+ */
+struct rti_wdt_device {
+ÂÂÂ void __iomemÂÂÂÂÂÂÂ *base;
+ÂÂÂ unsigned longÂÂÂÂÂÂÂ freq;
+ÂÂÂ struct watchdog_deviceÂÂÂ wdd;
+};
+
+static int rti_wdt_start(struct watchdog_device *wdd)
+{
+ÂÂÂ u32 timer_margin;
+ÂÂÂ struct rti_wdt_device *wdt = watchdog_get_drvdata(wdd);
+
+ÂÂÂ /* set timeout period */
+ÂÂÂ timer_margin = (u64)wdd->timeout * wdt->freq;
+ÂÂÂ timer_margin >>= WDT_PRELOAD_SHIFT;
+ÂÂÂ if (timer_margin > WDT_PRELOAD_MAX)
+ÂÂÂÂÂÂÂ timer_margin = WDT_PRELOAD_MAX;
+ÂÂÂ writel_relaxed(timer_margin, wdt->base + RTIDWDPRLD);
+
+ÂÂÂ /*
+ÂÂÂÂ * RTI only supports a windowed mode, where the watchdog can only
+ÂÂÂÂ * be petted during the open window; not too early or not too late.
+ÂÂÂÂ * The HW configuration options only allow for the open window size
+ÂÂÂÂ * to be 50% or less than that; we obviouly want to configure the open
+ÂÂÂÂ * window as large as possible so we select the 50% option. To avoid
+ÂÂÂÂ * any glitches, we accommodate 5% safety margin also, so we setup
+ÂÂÂÂ * the min_hw_hearbeat at 55% of the timeout period.
+ÂÂÂÂ */
+ÂÂÂ wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+
+ÂÂÂ /* Generate NMI when wdt expires */
+ÂÂÂ writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
+
+ÂÂÂ /* Open window size 50%; this is the largest window size available */
+ÂÂÂ writel_relaxed(RTIWWDSIZE_50P, wdt->base + RTIWWDSIZECTRL);
+
+ÂÂÂ readl_relaxed(wdt->base + RTIWWDSIZECTRL);
+
+ÂÂÂ /* enable watchdog */
+ÂÂÂ writel_relaxed(WDENABLE_KEY, wdt->base + RTIDWDCTRL);
+ÂÂÂ return 0;
+}
+
+static int rti_wdt_ping(struct watchdog_device *wdd)
+{
+ÂÂÂ struct rti_wdt_device *wdt = watchdog_get_drvdata(wdd);
+
+ÂÂÂ /* put watchdog in service state */
+ÂÂÂ writel_relaxed(WDKEY_SEQ0, wdt->base + RTIWDKEY);
+ÂÂÂ /* put watchdog in active state */
+ÂÂÂ writel_relaxed(WDKEY_SEQ1, wdt->base + RTIWDKEY);
+
+ÂÂÂ return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+ÂÂÂ u64 timer_counter;
+ÂÂÂ u32 val;
+ÂÂÂ struct rti_wdt_device *wdt = watchdog_get_drvdata(wdd);
+
+ÂÂÂ /* if timeout has occurred then return 0 */
+ÂÂÂ val = readl_relaxed(wdt->base + RTIWDSTATUS);
+ÂÂÂ if (val & DWDST)
+ÂÂÂÂÂÂÂ return 0;
+
+ÂÂÂ timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
+
+ÂÂÂ do_div(timer_counter, wdt->freq);
+
+ÂÂÂ return timer_counter;
+}
+
+static const struct watchdog_info rti_wdt_info = {
+ÂÂÂ .options = WDIOF_KEEPALIVEPING,
+ÂÂÂ .identity = "K3 RTI Watchdog",
+};
+
+static const struct watchdog_ops rti_wdt_ops = {
+ÂÂÂ .ownerÂÂÂÂÂÂÂ = THIS_MODULE,
+ÂÂÂ .startÂÂÂÂÂÂÂ = rti_wdt_start,
+ÂÂÂ .pingÂÂÂÂÂÂÂ = rti_wdt_ping,
+ÂÂÂ .get_timeleftÂÂÂ = rti_wdt_get_timeleft,
+};
+
+static int rti_wdt_probe(struct platform_device *pdev)
+{
+ÂÂÂ int ret = 0;
+ÂÂÂ struct device *dev = &pdev->dev;
+ÂÂÂ struct resource *wdt_mem;
+ÂÂÂ struct watchdog_device *wdd;
+ÂÂÂ struct rti_wdt_device *wdt;
+ÂÂÂ struct clk *clk;
+
+ÂÂÂ wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
+ÂÂÂ if (!wdt)
+ÂÂÂÂÂÂÂ return -ENOMEM;
+
+ÂÂÂ clk = devm_clk_get(dev, NULL);
+ÂÂÂ if (IS_ERR(clk)) {
+ÂÂÂÂÂÂÂ if (PTR_ERR(clk) != -EPROBE_DEFER)
+ÂÂÂÂÂÂÂÂÂÂÂ dev_err(dev, "failed to get clock\n");
+ÂÂÂÂÂÂÂ return PTR_ERR(clk);
+ÂÂÂ }
+
+ÂÂÂ wdt->freq = clk_get_rate(clk);
+ÂÂÂ if (!wdt->freq) {
+ÂÂÂÂÂÂÂ dev_err(dev, "Failed to get fck rate.\n");
+ÂÂÂÂÂÂÂ return -EINVAL;
+ÂÂÂ }
+
+ÂÂÂ devm_clk_put(dev, clk);
+
Are you sure about this ? Doesn't this mean that the clock may be stopped ?
Normally the reason for using devm_ functions during probe is that release
functions are called automatically when the device is removed. Calling
the release function dorectly is unusual and most of the time wrong.

clk_get / clk_put does not enable / disable a clock by itself, this is just used to fetch a clock handle. But you are right, we are never calling clk_enable on it either, because at least the 32kHz clock we are interested in is of type always-on and can never be disabled.

I can keep the clock handle and call enable/disable on it in the probe/remove if you think that would be neater.


If it isn't needed to be held, you should use clk_get / clk_put,
and not the devm_ functions.

Ok, changed this and posted v3 of patch #3 only.

-Tero

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki