[PATCH 0/5] MTX-1 watchdog driver removal

[PATCH 0/5] MTX-1 watchdog driver removal

[Florian Fainelli]

This patch series removes the bespoke mtx-1_wdt driver and migrates it
to the generic gpio_wdt driver which it should have done ever since
gpio_wdt existed.

Assuming the patches are satisfactory, I suppose it would be easier to
merge those via the watchdog tree than the MIPS tree?

Florian Fainelli (5):
  watchdog: gpio: Allow devices described by software nodes
  dt-bindings: watchdog: gpio: Describe minimum heartbeat intervals
  watchdog: gpio: Support watchdogs with a minimum heartbeat interval
  MIPS: Alchemy: Use the GPIO watchdog driver on MTX-1
  watchdog: Remove the MTX-1 watchdog driver

 .../bindings/watchdog/linux,wdt-gpio.yaml     |  11 +-
 arch/mips/alchemy/board-mtx1.c                |   8 +-
 arch/mips/configs/mtx1_defconfig              |   2 +-
 drivers/watchdog/Kconfig                      |   9 +-
 drivers/watchdog/Makefile                     |   1 -
 drivers/watchdog/gpio_wdt.c                   |  15 +-
 drivers/watchdog/mtx-1_wdt.c                  | 245 ------------------
 7 files changed, 31 insertions(+), 260 deletions(-)
 delete mode 100644 drivers/watchdog/mtx-1_wdt.c

-- 
2.53.0

[PATCH 1/5] watchdog: gpio: Allow devices described by software nodes

[Florian Fainelli]

The driver reads its configuration through device properties and obtains
its GPIO with the descriptor API. Neither operation requires ACPI or OF,
so allow the driver to be selected on platforms using software nodes.

Add a platform module alias so a non-OF platform device can also load the
driver when it is built as a module.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/watchdog/Kconfig    | 2 +-
 drivers/watchdog/gpio_wdt.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f7d0a0c2c0ef..f50709c635e5 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -267,7 +267,7 @@ config DA9062_WATCHDOG
 
 config GPIO_WATCHDOG
 	tristate "Watchdog device controlled through GPIO-line"
-	depends on GPIOLIB && (ACPI || OF)
+	depends on GPIOLIB
 	select WATCHDOG_CORE
 	help
 	  If you say yes here you get support for watchdog device
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
index 1abc7d0b78a6..ff323d6e692e 100644
--- a/drivers/watchdog/gpio_wdt.c
+++ b/drivers/watchdog/gpio_wdt.c
@@ -191,3 +191,4 @@ module_platform_driver(gpio_wdt_driver);
 MODULE_AUTHOR("Alexander Shiyan <shc_work@mail.ru>");
 MODULE_DESCRIPTION("GPIO Watchdog");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio-wdt");
-- 
2.53.0

[PATCH 2/5] dt-bindings: watchdog: gpio: Describe minimum heartbeat intervals

[Florian Fainelli]

Some GPIO-controlled watchdogs require a minimum interval between input
transitions. Add an optional hw-min-interval-ms property to describe this
constraint, allowing the watchdog core to defer early keepalives.

Also remove the 16-bit limit on hw_margin_ms. The generic GPIO driver has
no hardware register imposing this limit, and its heartbeat interval is
stored in an unsigned int. The MTX-1 watchdog has a nominal 100-second
hardware timeout and requires at least 1.6 seconds between transitions.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../devicetree/bindings/watchdog/linux,wdt-gpio.yaml  | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/linux,wdt-gpio.yaml b/Documentation/devicetree/bindings/watchdog/linux,wdt-gpio.yaml
index 499f1b7e03f9..1adbfe5e5461 100644
--- a/Documentation/devicetree/bindings/watchdog/linux,wdt-gpio.yaml
+++ b/Documentation/devicetree/bindings/watchdog/linux,wdt-gpio.yaml
@@ -36,7 +36,12 @@ properties:
     description: Maximum time to reset watchdog circuit (milliseconds).
     $ref: /schemas/types.yaml#/definitions/uint32
     minimum: 2
-    maximum: 65535
+
+  hw-min-interval-ms:
+    description:
+      Minimum time between watchdog resets in milliseconds. If omitted, the
+      watchdog can be reset at any time. Must be less than hw_margin_ms.
+    minimum: 1
 
   always-running:
     type: boolean
@@ -64,5 +69,7 @@ examples:
         compatible = "linux,wdt-gpio";
         gpios = <&gpio3 9 GPIO_ACTIVE_LOW>;
         hw_algo = "toggle";
-        hw_margin_ms = <1600>;
+        hw_margin_ms = <100000>;
+        hw-min-interval-ms = <1600>;
+        always-running;
     };
-- 
2.53.0

[PATCH 3/5] watchdog: gpio: Support watchdogs with a minimum heartbeat interval

[Florian Fainelli]

Read the optional hw-min-interval-ms property into min_hw_heartbeat_ms so
the watchdog core defers keepalives that would toggle the GPIO too soon.
Reject zero intervals and intervals at or above the hardware margin.
Devices without the property retain their existing behavior.

Remove the arbitrary 65535 ms upper bound on hw_margin_ms as well. There
is no register width to constrain this software interval, and the MTX-1
watchdog needs a larger margin with a 1600 ms minimum ping interval.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/watchdog/gpio_wdt.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
index ff323d6e692e..a431c77fa179 100644
--- a/drivers/watchdog/gpio_wdt.c
+++ b/drivers/watchdog/gpio_wdt.c
@@ -138,10 +138,20 @@ static int gpio_wdt_probe(struct platform_device *pdev)
 	ret = device_property_read_u32(dev, "hw_margin_ms", &hw_margin);
 	if (ret)
 		return ret;
-	/* Disallow values lower than 2 and higher than 65535 ms */
-	if (hw_margin < 2 || hw_margin > 65535)
+	/* The core feeds the watchdog at half the hardware margin. */
+	if (hw_margin < 2)
 		return -EINVAL;
 
+	if (device_property_present(dev, "hw-min-interval-ms")) {
+		ret = device_property_read_u32(dev, "hw-min-interval-ms",
+					      &priv->wdd.min_hw_heartbeat_ms);
+		if (ret)
+			return ret;
+		if (!priv->wdd.min_hw_heartbeat_ms ||
+		    priv->wdd.min_hw_heartbeat_ms >= hw_margin)
+			return -EINVAL;
+	}
+
 	priv->always_running = device_property_read_bool(dev, "always-running");
 
 	watchdog_set_drvdata(&priv->wdd, priv);
-- 
2.53.0

[PATCH 4/5] MIPS: Alchemy: Use the GPIO watchdog driver on MTX-1

[Florian Fainelli]

The MTX-1 watchdog is serviced by toggling GPIO 215. Describe its toggle
algorithm and 1.6-second minimum interval through the existing software
node properties, and register gpio-wdt instead of mtx1-wdt.

The nominal 100-second circuit reset the test board about 82 seconds after
starting an unclean-close test over the serial console. Allow for circuit
tolerance and console transmission time with a conservative 70-second
hardware service margin. Keep a 100-second initial userspace timeout and
let the core schedule keepalives for the longer userspace timeout.

Mark the watchdog always-running so the core services it before userspace
opens the device and after a clean stop, without trying to disable the
hardware by tristating the GPIO. Enable GPIO_WATCHDOG in the MTX-1
defconfig.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/alchemy/board-mtx1.c   | 8 +++++++-
 arch/mips/configs/mtx1_defconfig | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/mips/alchemy/board-mtx1.c b/arch/mips/alchemy/board-mtx1.c
index 88c20c0ca96d..8543ae42af92 100644
--- a/arch/mips/alchemy/board-mtx1.c
+++ b/arch/mips/alchemy/board-mtx1.c
@@ -125,13 +125,19 @@ static void __init mtx1_keys_init(void)
 }
 
 static const struct property_entry mtx1_wdt_props[] = {
+	PROPERTY_ENTRY_STRING("hw_algo", "toggle"),
+	/* Allow for tolerance in the nominal 100-second timeout. */
+	PROPERTY_ENTRY_U32("hw_margin_ms", 70000),
+	PROPERTY_ENTRY_U32("hw-min-interval-ms", 1600),
+	PROPERTY_ENTRY_U32("timeout-sec", 100),
+	PROPERTY_ENTRY_BOOL("always-running"),
 	/* Global number 215 is offset 15 on Alchemy GPIO 2 */
 	PROPERTY_ENTRY_GPIO("gpios", &alchemy_gpio2_node, 15, GPIO_ACTIVE_HIGH),
 	{ }
 };
 
 static const struct platform_device_info mtx1_wdt_info __initconst = {
-	.name = "mtx1-wdt",
+	.name = "gpio-wdt",
 	.id = 0,
 	.properties = mtx1_wdt_props,
 };
diff --git a/arch/mips/configs/mtx1_defconfig b/arch/mips/configs/mtx1_defconfig
index e28a0b1e6d52..5e060daded9d 100644
--- a/arch/mips/configs/mtx1_defconfig
+++ b/arch/mips/configs/mtx1_defconfig
@@ -377,7 +377,7 @@ CONFIG_SENSORS_W83627HF=m
 CONFIG_SENSORS_W83627EHF=m
 CONFIG_WATCHDOG=y
 CONFIG_WATCHDOG_NOWAYOUT=y
-CONFIG_WDT_MTX1=y
+CONFIG_GPIO_WATCHDOG=y
 # CONFIG_VGA_ARB is not set
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-- 
2.53.0

[PATCH 5/5] watchdog: Remove the MTX-1 watchdog driver

[Florian Fainelli]

MTX-1 now uses the generic GPIO watchdog driver with board properties
describing its GPIO and heartbeat constraints. Remove the unused custom
driver and its Kconfig and Makefile entries.

The watchdog core now supplies the character device, keepalive scheduling,
timeout handling and close semantics previously implemented by this driver.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/watchdog/Kconfig     |   7 -
 drivers/watchdog/Makefile    |   1 -
 drivers/watchdog/mtx-1_wdt.c | 245 -----------------------------------
 3 files changed, 253 deletions(-)
 delete mode 100644 drivers/watchdog/mtx-1_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f50709c635e5..c884f86be4e3 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1865,13 +1865,6 @@ config JZ4740_WDT
 	help
 	  Hardware driver for the built-in watchdog timer on Ingenic jz4740 SoCs.
 
-config WDT_MTX1
-	tristate "MTX-1 Hardware Watchdog"
-	depends on MIPS_MTX1 || (MIPS && COMPILE_TEST)
-	help
-	  Hardware driver for the MTX-1 boards. This is a watchdog timer that
-	  will reboot the machine after a 100 seconds timer expired.
-
 config SIBYTE_WDOG
 	tristate "Sibyte SoC hardware watchdog"
 	depends on CPU_SB1
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 9ac406677aba..19cb25f51624 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -172,7 +172,6 @@ obj-$(CONFIG_BCM47XX_WDT) += bcm47xx_wdt.o
 obj-$(CONFIG_RC32434_WDT) += rc32434_wdt.o
 obj-$(CONFIG_INDYDOG) += indydog.o
 obj-$(CONFIG_JZ4740_WDT) += jz4740_wdt.o
-obj-$(CONFIG_WDT_MTX1) += mtx-1_wdt.o
 obj-$(CONFIG_SIBYTE_WDOG) += sb_wdog.o
 obj-$(CONFIG_TXX9_WDT) += txx9wdt.o
 obj-$(CONFIG_OCTEON_WDT) += octeon-wdt.o
diff --git a/drivers/watchdog/mtx-1_wdt.c b/drivers/watchdog/mtx-1_wdt.c
deleted file mode 100644
index f75426cfa425..000000000000
--- a/drivers/watchdog/mtx-1_wdt.c
+++ /dev/null
@@ -1,245 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- *      Driver for the MTX-1 Watchdog.
- *
- *      (C) Copyright 2005 4G Systems <info@4g-systems.biz>,
- *							All Rights Reserved.
- *                              http://www.4g-systems.biz
- *
- *	(C) Copyright 2007 OpenWrt.org, Florian Fainelli <florian@openwrt.org>
- *      (c) Copyright 2005    4G Systems <info@4g-systems.biz>
- *
- *      Release 0.01.
- *      Author: Michael Stickel  michael.stickel@4g-systems.biz
- *
- *      Release 0.02.
- *	Author: Florian Fainelli florian@openwrt.org
- *		use the Linux watchdog/timer APIs
- *
- *      The Watchdog is configured to reset the MTX-1
- *      if it is not triggered for 100 seconds.
- *      It should not be triggered more often than 1.6 seconds.
- *
- *      A timer triggers the watchdog every 5 seconds, until
- *      it is opened for the first time. After the first open
- *      it MUST be triggered every 2..95 seconds.
- */
-
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/types.h>
-#include <linux/errno.h>
-#include <linux/miscdevice.h>
-#include <linux/fs.h>
-#include <linux/ioport.h>
-#include <linux/timer.h>
-#include <linux/completion.h>
-#include <linux/jiffies.h>
-#include <linux/watchdog.h>
-#include <linux/platform_device.h>
-#include <linux/io.h>
-#include <linux/uaccess.h>
-#include <linux/gpio/consumer.h>
-
-#define MTX1_WDT_INTERVAL	(5 * HZ)
-
-static int ticks = 100 * HZ;
-
-static struct {
-	struct completion stop;
-	spinlock_t lock;
-	int running;
-	struct timer_list timer;
-	int queue;
-	int default_ticks;
-	unsigned long inuse;
-	struct gpio_desc *gpiod;
-	unsigned int gstate;
-} mtx1_wdt_device;
-
-static void mtx1_wdt_trigger(struct timer_list *unused)
-{
-	spin_lock(&mtx1_wdt_device.lock);
-	if (mtx1_wdt_device.running)
-		ticks--;
-
-	/* toggle wdt gpio */
-	mtx1_wdt_device.gstate = !mtx1_wdt_device.gstate;
-	gpiod_set_value(mtx1_wdt_device.gpiod, mtx1_wdt_device.gstate);
-
-	if (mtx1_wdt_device.queue && ticks)
-		mod_timer(&mtx1_wdt_device.timer, jiffies + MTX1_WDT_INTERVAL);
-	else
-		complete(&mtx1_wdt_device.stop);
-	spin_unlock(&mtx1_wdt_device.lock);
-}
-
-static void mtx1_wdt_reset(void)
-{
-	ticks = mtx1_wdt_device.default_ticks;
-}
-
-
-static void mtx1_wdt_start(void)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&mtx1_wdt_device.lock, flags);
-	if (!mtx1_wdt_device.queue) {
-		mtx1_wdt_device.queue = 1;
-		mtx1_wdt_device.gstate = 1;
-		gpiod_set_value(mtx1_wdt_device.gpiod, 1);
-		mod_timer(&mtx1_wdt_device.timer, jiffies + MTX1_WDT_INTERVAL);
-	}
-	mtx1_wdt_device.running++;
-	spin_unlock_irqrestore(&mtx1_wdt_device.lock, flags);
-}
-
-static int mtx1_wdt_stop(void)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&mtx1_wdt_device.lock, flags);
-	if (mtx1_wdt_device.queue) {
-		mtx1_wdt_device.queue = 0;
-		mtx1_wdt_device.gstate = 0;
-		gpiod_set_value(mtx1_wdt_device.gpiod, 0);
-	}
-	ticks = mtx1_wdt_device.default_ticks;
-	spin_unlock_irqrestore(&mtx1_wdt_device.lock, flags);
-	return 0;
-}
-
-/* Filesystem functions */
-
-static int mtx1_wdt_open(struct inode *inode, struct file *file)
-{
-	if (test_and_set_bit(0, &mtx1_wdt_device.inuse))
-		return -EBUSY;
-	return stream_open(inode, file);
-}
-
-
-static int mtx1_wdt_release(struct inode *inode, struct file *file)
-{
-	clear_bit(0, &mtx1_wdt_device.inuse);
-	return 0;
-}
-
-static long mtx1_wdt_ioctl(struct file *file, unsigned int cmd,
-							unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-	int __user *p = (int __user *)argp;
-	unsigned int value;
-	static const struct watchdog_info ident = {
-		.options = WDIOF_CARDRESET,
-		.identity = "MTX-1 WDT",
-	};
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		if (copy_to_user(argp, &ident, sizeof(ident)))
-			return -EFAULT;
-		break;
-	case WDIOC_GETSTATUS:
-	case WDIOC_GETBOOTSTATUS:
-		put_user(0, p);
-		break;
-	case WDIOC_SETOPTIONS:
-		if (get_user(value, p))
-			return -EFAULT;
-		if (value & WDIOS_ENABLECARD)
-			mtx1_wdt_start();
-		else if (value & WDIOS_DISABLECARD)
-			mtx1_wdt_stop();
-		else
-			return -EINVAL;
-		return 0;
-	case WDIOC_KEEPALIVE:
-		mtx1_wdt_reset();
-		break;
-	default:
-		return -ENOTTY;
-	}
-	return 0;
-}
-
-
-static ssize_t mtx1_wdt_write(struct file *file, const char *buf,
-						size_t count, loff_t *ppos)
-{
-	if (!count)
-		return -EIO;
-	mtx1_wdt_reset();
-	return count;
-}
-
-static const struct file_operations mtx1_wdt_fops = {
-	.owner		= THIS_MODULE,
-	.unlocked_ioctl	= mtx1_wdt_ioctl,
-	.compat_ioctl	= compat_ptr_ioctl,
-	.open		= mtx1_wdt_open,
-	.write		= mtx1_wdt_write,
-	.release	= mtx1_wdt_release,
-};
-
-
-static struct miscdevice mtx1_wdt_misc = {
-	.minor	= WATCHDOG_MINOR,
-	.name	= "watchdog",
-	.fops	= &mtx1_wdt_fops,
-};
-
-
-static int mtx1_wdt_probe(struct platform_device *pdev)
-{
-	int ret;
-
-	mtx1_wdt_device.gpiod = devm_gpiod_get(&pdev->dev,
-					       NULL, GPIOD_OUT_HIGH);
-	if (IS_ERR(mtx1_wdt_device.gpiod)) {
-		dev_err(&pdev->dev, "failed to request gpio");
-		return PTR_ERR(mtx1_wdt_device.gpiod);
-	}
-
-	spin_lock_init(&mtx1_wdt_device.lock);
-	init_completion(&mtx1_wdt_device.stop);
-	mtx1_wdt_device.queue = 0;
-	clear_bit(0, &mtx1_wdt_device.inuse);
-	timer_setup(&mtx1_wdt_device.timer, mtx1_wdt_trigger, 0);
-	mtx1_wdt_device.default_ticks = ticks;
-
-	ret = misc_register(&mtx1_wdt_misc);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to register\n");
-		return ret;
-	}
-	mtx1_wdt_start();
-	dev_info(&pdev->dev, "MTX-1 Watchdog driver\n");
-	return 0;
-}
-
-static void mtx1_wdt_remove(struct platform_device *pdev)
-{
-	/* FIXME: do we need to lock this test ? */
-	if (mtx1_wdt_device.queue) {
-		mtx1_wdt_device.queue = 0;
-		wait_for_completion(&mtx1_wdt_device.stop);
-	}
-
-	misc_deregister(&mtx1_wdt_misc);
-}
-
-static struct platform_driver mtx1_wdt_driver = {
-	.probe = mtx1_wdt_probe,
-	.remove = mtx1_wdt_remove,
-	.driver.name = "mtx1-wdt",
-};
-
-module_platform_driver(mtx1_wdt_driver);
-
-MODULE_AUTHOR("Michael Stickel, Florian Fainelli");
-MODULE_DESCRIPTION("Driver for the MTX-1 watchdog");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:mtx1-wdt");
-- 
2.53.0