[PATCH v1 0/3] introduce watchdog driver for Mellanox systems.

[PATCH v1 0/3] introduce watchdog driver for Mellanox systems.

[michaelsh]

From: Michael Shych <michaelsh@mellanox.com>

This patchset introduces watchdog driver for a various range of Mellanox
Ethernet and Infiniband switch systems.
Mellanox watchdog device is implemented in programmable logic device.
There are 2 types of HW watchdog implementations with diferent capabilities.
This mlx-wdt driver supports both HW watchdog implementations.
Mellanox system can have 2 watchdogs: main and auxiliary.
Main and auxiliary watchdog devices can be enabled together on the same system.
There are several actions that can be defined in the watchdog:
system reset, start fans on full speed and increase register counter.

Michael Shych (3):
  platform_data/mlxreg:	addittions for mellanox watchdog driver.
  watchdog: mlx-wdt: introduce watchdog driver for Mellanox systems.
  Documentation/watchdog: Add documentation mlx-wdt driver

 Documentation/watchdog/mlx-wdt.txt   |  48 +++++
 drivers/watchdog/Kconfig             |  16 ++
 drivers/watchdog/Makefile            |   1 +
 drivers/watchdog/mlx_wdt.c           | 406 +++++++++++++++++++++++++++++++++++
 include/linux/platform_data/mlxreg.h |   6 +
 5 files changed, 477 insertions(+)
 create mode 100644 Documentation/watchdog/mlx-wdt.txt
 create mode 100644 drivers/watchdog/mlx_wdt.c

-- 
2.11.0

[PATCH v1 1/3] platform_data/mlxreg: addittions for mellanox watchdog driver.

[michaelsh]

From: Michael Shych <michaelsh@mellanox.com>

There are two new fields added to mlxreg core structure:
features - supported features of device and
identity - device identity name.
Add new defines for watchdog features.

Signed-off-by: Michael Shych <michaelsh@mellanox.com>
---
 include/linux/platform_data/mlxreg.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
index 19f5cb618c55..e6bb0e2d6efc 100644
--- a/include/linux/platform_data/mlxreg.h
+++ b/include/linux/platform_data/mlxreg.h
@@ -35,6 +35,8 @@
 #define __LINUX_PLATFORM_DATA_MLXREG_H
 
 #define MLXREG_CORE_LABEL_MAX_SIZE	32
+#define MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START	BIT(0)
+#define MLXREG_CORE_WD_FEATURE_START_AT_BOOT		BIT(1)
 
 /**
  * struct mlxreg_hotplug_device - I2C device data:
@@ -110,11 +112,15 @@ struct mlxreg_core_item {
  * @led_data: led private data;
  * @regmap: register map of parent device;
  * @counter: number of led instances;
+ * @features: supported features of device;
+ * @identity: device identity name;
  */
 struct mlxreg_core_platform_data {
 	struct mlxreg_core_data *data;
 	void *regmap;
 	int counter;
+	u32 features;
+	char identity[MLXREG_CORE_LABEL_MAX_SIZE];
 };
 
 /**
-- 
2.11.0

[PATCH v1 2/3] watchdog: mlx-wdt: introduce watchdog driver for Mellanox systems.

[michaelsh]

From: Michael Shych <michaelsh@mellanox.com>

Introduce watchdog driver for variious rang of Mellanox Ethernet and
Infiniband switch systems.

Watchdog driver for Mellanox watchdog devices, implemented in
programmable logic device.

Main and auxiliary watchdog devices can exist on the same system.
There are several actions that can be defined in the watchdog:
system reset, start fans on full speed and increase a counter.
The last 2 actions are performed without a system reset.
Actions without reset are provided for auxiliary watchdog devices,
which is optional.
Access to HW registers is performed through generic
regmap interface.

There are 2 types of HW watchdog implementations.
Type 1: actual HW timeout can be defined as power of 2 msec.
e.g. timeout 20 sec will be rounded up to 32768 msec.;
maximum timeout period is 32 sec (32768 msec.);
get time-left isn't supported
Type 2: actual HW timeout is defined in sec. and it's the same as
user-defined timeout;
maximum timeout is 255 sec;
get time-left is supported;

Watchdog driver is probed from the common mlx_platform driver.

Signed-off-by: Michael Shych <michaelsh@mellanox.com>
---
 drivers/watchdog/Kconfig   |  16 ++
 drivers/watchdog/Makefile  |   1 +
 drivers/watchdog/mlx_wdt.c | 406 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 423 insertions(+)
 create mode 100644 drivers/watchdog/mlx_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 57f017d74a97..f1766eb081bb 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -241,6 +241,22 @@ config RAVE_SP_WATCHDOG
 	help
 	  Support for the watchdog on RAVE SP device.
 
+config MLX_WDT
+	tristate "Mellanox Watchdog"
+	depends on MELLANOX_PLATFORM
+	select WATCHDOG_CORE
+	select REGMAP
+	help
+	  This is the driver for the hardware watchdog on Mellanox systems.
+	  If you are going to use it, say Y here, otherwise N.
+	  This driver can be used together with the watchdog daemon.
+	  It can also watch your kernel to make sure it doesn't freeze,
+	  and if it does, it reboots your system after a certain amount of
+	  time.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mlx-wdt.
+
 # ALPHA Architecture
 
 # ARM Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a0917ef28e07..941b74185c9c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
 obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
 obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
 obj-$(CONFIG_NIC7018_WDT) += nic7018_wdt.o
+obj-$(CONFIG_MLX_WDT) += mlx_wdt.o
 
 # M68K Architecture
 obj-$(CONFIG_M54xx_WATCHDOG) += m54xx_wdt.o
diff --git a/drivers/watchdog/mlx_wdt.c b/drivers/watchdog/mlx_wdt.c
new file mode 100644
index 000000000000..be28aa165cf2
--- /dev/null
+++ b/drivers/watchdog/mlx_wdt.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Mellanox watchdog driver
+ *
+ * Copyright (C) 2019 Mellanox Technologies
+ * Copyright (C) 2019 Michael Shych <mshych@mellanox.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+#include <linux/platform_data/mlxreg.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#define MLXREG_WDT_CLOCK_SCALE		1000
+#define MLXREG_WDT_MAX_TIMEOUT_TYPE1	32
+#define MLXREG_WDT_MAX_TIMEOUT_TYPE2	255
+#define MLXREG_WDT_MIN_TIMEOUT		1
+#define MLXREG_WDT_HW_TIMEOUT_CONVERT(hw_timeout) ((1 << (hw_timeout)) \
+						   / MLXREG_WDT_CLOCK_SCALE)
+#define MLXREG_WDT_OPTIONS_BASE (WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | \
+				 WDIOF_SETTIMEOUT)
+
+/**
+ * enum mlxreg_wdt_type - type of HW watchdog
+ *
+ * TYPE1 can be differentiated by different register/mask
+ *	 for WD action set and ping.
+ */
+enum mlxreg_wdt_type {
+	MLX_WDT_TYPE1,
+	MLX_WDT_TYPE2,
+};
+
+/**
+ * struct mlxreg_wdt - wd private data:
+ *
+ * @wdd:	watchdog device;
+ * @device:	basic device;
+ * @pdata:	data received from platform driver;
+ * @regmap:	register map of parent device;
+ * @timeout:	defined timeout in sec.;
+ * @hw_timeout:	real timeout set in hw;
+ *		It will be roundup base of 2 in WD type 1,
+ *		in WD type 2 it will be same number of sec as timeout;
+ * @action_idx:	index for direct access to action register;
+ * @timeout_idx:index for direct access to TO register;
+ * @tleft_idx:	index for direct access to time left register;
+ * @ping_idx:	index for direct access to ping register;
+ * @reset_idx:	index for direct access to reset cause register;
+ * @wd_type:	watchdog HW type;
+ * @hw_timeout:	actual HW timeout;
+ * @io_lock:	spinlock for io access;
+ */
+struct mlxreg_wdt {
+	struct watchdog_device wdd;
+	struct mlxreg_core_platform_data *pdata;
+	void *regmap;
+	int action_idx;
+	int timeout_idx;
+	int tleft_idx;
+	int ping_idx;
+	int reset_idx;
+	enum mlxreg_wdt_type wdt_type;
+	u8 hw_timeout;
+	spinlock_t io_lock;	/* the lock for io operations */
+};
+
+static int mlxreg_wdt_roundup_to_base_2(struct mlxreg_wdt *wdt, int timeout)
+{
+	timeout *= MLXREG_WDT_CLOCK_SCALE;
+
+	wdt->hw_timeout = order_base_2(timeout);
+	dev_info(wdt->wdd.parent,
+		 "watchdog %s timeout %d was rounded up to %lu (msec)\n",
+		 wdt->wdd.info->identity, timeout, roundup_pow_of_two(timeout));
+
+	return 0;
+}
+
+static enum mlxreg_wdt_type
+mlxreg_wdt_check_watchdog_type(struct mlxreg_wdt *wdt,
+			       struct mlxreg_core_platform_data *pdata)
+{
+	if (pdata->data[wdt->action_idx].reg ==
+	    pdata->data[wdt->ping_idx].reg &&
+	    pdata->data[wdt->action_idx].mask ==
+	    pdata->data[wdt->ping_idx].mask)
+		return MLX_WDT_TYPE2;
+
+	return MLX_WDT_TYPE1;
+}
+
+static int mlxreg_wdt_check_card_reset(struct mlxreg_wdt *wdt)
+{
+	struct mlxreg_core_data *reg_data;
+	u32 regval;
+	int rc;
+
+	if (wdt->reset_idx == -EINVAL)
+		return -EINVAL;
+
+	if (!(wdt->wdd.info->options & WDIOF_CARDRESET))
+		return 0;
+
+	spin_lock(&wdt->io_lock);
+	reg_data = &wdt->pdata->data[wdt->reset_idx];
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	spin_unlock(&wdt->io_lock);
+	if (rc)
+		goto read_error;
+
+	if (regval & ~reg_data->mask) {
+		wdt->wdd.bootstatus = WDIOF_CARDRESET;
+		dev_info(wdt->wdd.parent,
+			 "watchdog previously reset the CPU\n");
+	}
+
+read_error:
+	return rc;
+}
+
+static int mlxreg_wdt_start(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->action_idx];
+	u32 regval;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	if (rc) {
+		spin_unlock(&wdt->io_lock);
+		goto read_error;
+	}
+
+	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+	spin_unlock(&wdt->io_lock);
+	if (!rc) {
+		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
+		dev_info(wdd->parent, "watchdog %s started\n",
+			 wdd->info->identity);
+	}
+
+read_error:
+	return rc;
+}
+
+static int mlxreg_wdt_stop(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->action_idx];
+	u32 regval;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	if (rc) {
+		spin_unlock(&wdt->io_lock);
+		goto read_error;
+	}
+
+	regval = (regval & reg_data->mask) & ~BIT(reg_data->bit);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+	spin_unlock(&wdt->io_lock);
+	if (!rc)
+		dev_info(wdd->parent, "watchdog %s stopped\n",
+			 wdd->info->identity);
+
+read_error:
+	return rc;
+}
+
+static int mlxreg_wdt_ping(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->ping_idx];
+	u32 regval;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	if (rc)
+		goto read_error;
+
+	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+
+read_error:
+	spin_unlock(&wdt->io_lock);
+
+	return rc;
+}
+
+static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
+				  unsigned int timeout)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->timeout_idx];
+	u32 regval, set_time;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+
+	if (wdt->wdt_type == MLX_WDT_TYPE1) {
+		rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+		if (rc)
+			goto read_error;
+		mlxreg_wdt_roundup_to_base_2(wdt, timeout);
+		regval = (regval & reg_data->mask) | wdt->hw_timeout;
+		/* Rowndown to actual closest number of sec. */
+		set_time = MLXREG_WDT_HW_TIMEOUT_CONVERT(wdt->hw_timeout);
+	} else {
+		wdt->hw_timeout = timeout;
+		set_time = timeout;
+		regval = timeout;
+	}
+
+	watchdog_init_timeout(&wdt->wdd, set_time, wdd->parent);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+
+read_error:
+	spin_unlock(&wdt->io_lock);
+	if (!rc) {
+		if (watchdog_active(wdd)) {
+			rc = mlxreg_wdt_stop(wdd);
+			if (!rc)
+				rc = mlxreg_wdt_start(wdd);
+		}
+	}
+
+	return rc;
+}
+
+static unsigned int mlxreg_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->tleft_idx];
+	u32 regval;
+	int rc;
+
+	if (wdt->wdt_type == MLX_WDT_TYPE1)
+		return 0;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	spin_unlock(&wdt->io_lock);
+
+	return rc == 0 ? regval : 0;
+}
+
+static const struct watchdog_ops mlxreg_wdt_ops_type1 = {
+	.start		= mlxreg_wdt_start,
+	.stop		= mlxreg_wdt_stop,
+	.ping		= mlxreg_wdt_ping,
+	.set_timeout	= mlxreg_wdt_set_timeout,
+	.owner		= THIS_MODULE,
+};
+
+static const struct watchdog_ops mlxreg_wdt_ops_type2 = {
+	.start		= mlxreg_wdt_start,
+	.stop		= mlxreg_wdt_stop,
+	.ping		= mlxreg_wdt_ping,
+	.set_timeout	= mlxreg_wdt_set_timeout,
+	.get_timeleft	= mlxreg_wdt_get_timeleft,
+	.owner		= THIS_MODULE,
+};
+
+static const struct watchdog_info mlxreg_wdt_main_info = {
+	.options	= MLXREG_WDT_OPTIONS_BASE
+			| WDIOF_CARDRESET,
+	.identity	= "mlx-wdt-main",
+};
+
+static const struct watchdog_info mlxreg_wdt_aux_info = {
+	.options	= MLXREG_WDT_OPTIONS_BASE
+			| WDIOF_ALARMONLY,
+	.identity	= "mlx-wdt-aux",
+};
+
+static int mlxreg_wdt_config(struct mlxreg_wdt *wdt,
+			     struct mlxreg_core_platform_data *pdata)
+{
+	struct mlxreg_core_data *data = pdata->data;
+	int i;
+
+	wdt->reset_idx = -EINVAL;
+	for (i = 0; i < pdata->counter; i++, data++) {
+		if (strnstr(data->label, "action", sizeof(data->label)))
+			wdt->action_idx = i;
+		else if (strnstr(data->label, "timeout", sizeof(data->label)))
+			wdt->timeout_idx = i;
+		else if (strnstr(data->label, "timeleft", sizeof(data->label)))
+			wdt->tleft_idx = i;
+		else if (strnstr(data->label, "ping", sizeof(data->label)))
+			wdt->ping_idx = i;
+		else if (strnstr(data->label, "reset", sizeof(data->label)))
+			wdt->reset_idx = i;
+	}
+
+	wdt->pdata = pdata;
+	if (strnstr(pdata->identity, mlxreg_wdt_main_info.identity,
+		    sizeof(mlxreg_wdt_main_info.identity)))
+		wdt->wdd.info = &mlxreg_wdt_main_info;
+	else
+		wdt->wdd.info = &mlxreg_wdt_aux_info;
+
+	wdt->wdt_type = mlxreg_wdt_check_watchdog_type(wdt, pdata);
+	if (wdt->wdt_type == MLX_WDT_TYPE2) {
+		wdt->wdd.ops = &mlxreg_wdt_ops_type2;
+		wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE2;
+	} else {
+		wdt->wdd.ops = &mlxreg_wdt_ops_type1;
+		wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE1;
+	}
+	wdt->wdd.min_timeout = MLXREG_WDT_MIN_TIMEOUT;
+
+	return 0;
+}
+
+static int mlxreg_wdt_init_timeout(struct mlxreg_wdt *wdt,
+				   struct mlxreg_core_platform_data *pdata)
+{
+	u32 timeout;
+
+	timeout = pdata->data[wdt->timeout_idx].health_cntr;
+	return mlxreg_wdt_set_timeout(&wdt->wdd, timeout);
+}
+
+static int mlxreg_wdt_probe(struct platform_device *pdev)
+{
+	struct mlxreg_core_platform_data *pdata;
+	struct mlxreg_wdt *wdt;
+	int rc;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		dev_err(&pdev->dev, "Failed to get platform data.\n");
+		return -EINVAL;
+	}
+	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	spin_lock_init(&wdt->io_lock);
+
+	wdt->wdd.parent = &pdev->dev;
+	wdt->regmap = pdata->regmap;
+	mlxreg_wdt_config(wdt, pdata);
+
+	if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START))
+		watchdog_set_nowayout(&wdt->wdd, WATCHDOG_NOWAYOUT);
+	watchdog_stop_on_reboot(&wdt->wdd);
+	watchdog_set_drvdata(&wdt->wdd, wdt);
+	mlxreg_wdt_check_card_reset(wdt);
+	rc = mlxreg_wdt_init_timeout(wdt, pdata);
+	if (rc) {
+		dev_err(&pdev->dev,
+			"Cannot init watchdog timeout (err=%d)\n", rc);
+		return rc;
+	}
+
+	rc = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
+	if (rc) {
+		dev_err(&pdev->dev,
+			"Cannot register watchdog device (err=%d)\n", rc);
+		return rc;
+	}
+
+	if ((pdata->features & MLXREG_CORE_WD_FEATURE_START_AT_BOOT))
+		mlxreg_wdt_start(&wdt->wdd);
+
+	return rc;
+}
+
+static int mlxreg_wdt_remove(struct platform_device *pdev)
+{
+	struct mlxreg_wdt *wdt = dev_get_platdata(&pdev->dev);
+
+	mlxreg_wdt_stop(&wdt->wdd);
+	watchdog_unregister_device(&wdt->wdd);
+
+	return 0;
+}
+
+static struct platform_driver mlxreg_wdt_driver = {
+	.probe	= mlxreg_wdt_probe,
+	.remove	= mlxreg_wdt_remove,
+	.driver	= {
+			.name = "mlx-wdt",
+	},
+};
+
+module_platform_driver(mlxreg_wdt_driver);
+
+MODULE_AUTHOR("Michael Shych <michaelsh@mellanox.com>");
+MODULE_DESCRIPTION("Mellanox watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mlx-wdt");
-- 
2.11.0

[PATCH v1 3/3] Documentation/watchdog: Add documentation mlx-wdt driver

[michaelsh]

From: Michael Shych <michaelsh@mellanox.com>

Add documentation with details of mellanox watchdog driver.

Signed-off-by: Michael Shych <michaelsh@mellanox.com>
---
 Documentation/watchdog/mlx-wdt.txt | 48 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/watchdog/mlx-wdt.txt

diff --git a/Documentation/watchdog/mlx-wdt.txt b/Documentation/watchdog/mlx-wdt.txt
new file mode 100644
index 000000000000..13123acc4195
--- /dev/null
+++ b/Documentation/watchdog/mlx-wdt.txt
@@ -0,0 +1,48 @@
+		Mellanox watchdog drivers
+		for x86 based system switches
+
+This driver provides watchdog functionality for various Mellanox
+Ethernet and Infiniband switch systems.
+
+Mellanox watchdog device is implemented in a programmable logic device.
+
+There are 2 types of HW watchdog implementations.
+
+Type 1:
+Actual HW timeout can be defined as a power of 2 msec.
+e.g. timeout 20 sec will be rounded up to 32768 msec.
+The maximum timeout period is 32 sec (32768 msec.),
+Get time-left isn't supported
+
+Type 2:
+Actual HW timeout is defined in sec. and it's the same as 
+a user-defined timeout.
+Maximum timeout is 255 sec.
+Get time-left is supported.
+
+Type 1 HW watchdog implementation exist in old systems and 
+all new systems have type 2 HW watchdog.
+Two types of HW implementation have also different register map.
+
+Mellanox system can have 2 watchdogs: main and auxiliary.
+Main and auxiliary watchdog devices can be enabled together 
+on the same system.
+There are several actions that can be defined in the watchdog:
+system reset, start fans on full speed and increase register counter.
+The last 2 actions are performed without a system reset.
+Actions without reset are provided for auxiliary watchdog device,
+which is optional.
+
+This mlx-wdt driver supports both HW watchdog implementations.
+
+Watchdog driver is probed from the common mlx_platform driver.
+Mlx_platform driver provides an appropriate set of registers for
+Mellanox watchdog device, identity name (mlx-wdt-main or mlx-wdt-aux),
+initial timeout, performed action in expiration and configuration flags.
+watchdog configuration flags: nostop_after_start i.e. nowayout and
+start_at_boot.
+The driver reports if watchdog set timeout is rounded in type1 HW watchdog.
+The driver checks during initialization if the previous system reset 
+was done by the watchdog. If yes, it makes a notification about this event.
+
+Access to HW registers is performed through a generic regmap interface.
-- 
2.11.0

RE: [PATCH v1 2/3] watchdog: mlx-wdt: introduce watchdog driver for Mellanox systems.

[Michael Shych]

Hi Guenter,

Thank you for your prompt review and comments.

Regards,
    Michael.

> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
> Roeck
> Sent: Wednesday, February 6, 2019 9:14 PM
> To: Michael Shych <michaelsh@mellanox.com>
> Cc: wim@linux-watchdog.org; andy@infradead.org; dvhart@infradead.org;
> linux-watchdog@vger.kernel.org; platform-driver-x86@vger.kernel.org;
> Vadim Pasternak <vadimp@mellanox.com>
> Subject: Re: [PATCH v1 2/3] watchdog: mlx-wdt: introduce watchdog driver
> for Mellanox systems.
> 
> On Wed, Feb 06, 2019 at 04:50:36PM +0000, michaelsh@mellanox.com
> wrote:
> > From: Michael Shych <michaelsh@mellanox.com>
> >
> > Introduce watchdog driver for variious rang of Mellanox Ethernet and
> 
> s/variious/various/
> 
> > Infiniband switch systems.
> >
> > Watchdog driver for Mellanox watchdog devices, implemented in
> > programmable logic device.
> >
> > Main and auxiliary watchdog devices can exist on the same system.
> > There are several actions that can be defined in the watchdog:
> > system reset, start fans on full speed and increase a counter.
> > The last 2 actions are performed without a system reset.
> > Actions without reset are provided for auxiliary watchdog devices,
> > which is optional.
> > Access to HW registers is performed through generic
> > regmap interface.
> >
> > There are 2 types of HW watchdog implementations.
> > Type 1: actual HW timeout can be defined as power of 2 msec.
> > e.g. timeout 20 sec will be rounded up to 32768 msec.;
> > maximum timeout period is 32 sec (32768 msec.);
> > get time-left isn't supported
> > Type 2: actual HW timeout is defined in sec. and it's the same as
> > user-defined timeout;
> > maximum timeout is 255 sec;
> > get time-left is supported;
> >
> > Watchdog driver is probed from the common mlx_platform driver.
> >
> > Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> > ---
> >  drivers/watchdog/Kconfig   |  16 ++
> >  drivers/watchdog/Makefile  |   1 +
> >  drivers/watchdog/mlx_wdt.c | 406
> +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 423 insertions(+)
> >  create mode 100644 drivers/watchdog/mlx_wdt.c
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 57f017d74a97..f1766eb081bb 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -241,6 +241,22 @@ config RAVE_SP_WATCHDOG
> >  	help
> >  	  Support for the watchdog on RAVE SP device.
> >
> > +config MLX_WDT
> > +	tristate "Mellanox Watchdog"
> > +	depends on MELLANOX_PLATFORM
> > +	select WATCHDOG_CORE
> > +	select REGMAP
> > +	help
> > +	  This is the driver for the hardware watchdog on Mellanox systems.
> > +	  If you are going to use it, say Y here, otherwise N.
> > +	  This driver can be used together with the watchdog daemon.
> > +	  It can also watch your kernel to make sure it doesn't freeze,
> > +	  and if it does, it reboots your system after a certain amount of
> > +	  time.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called mlx-wdt.
> > +
> >  # ALPHA Architecture
> >
> >  # ARM Architecture
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index a0917ef28e07..941b74185c9c 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -142,6 +142,7 @@ obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-
> mid_wdt.o
> >  obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
> >  obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
> >  obj-$(CONFIG_NIC7018_WDT) += nic7018_wdt.o
> > +obj-$(CONFIG_MLX_WDT) += mlx_wdt.o
> >
> >  # M68K Architecture
> >  obj-$(CONFIG_M54xx_WATCHDOG) += m54xx_wdt.o
> > diff --git a/drivers/watchdog/mlx_wdt.c b/drivers/watchdog/mlx_wdt.c
> > new file mode 100644
> > index 000000000000..be28aa165cf2
> > --- /dev/null
> > +++ b/drivers/watchdog/mlx_wdt.c
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Mellanox watchdog driver
> > + *
> > + * Copyright (C) 2019 Mellanox Technologies
> > + * Copyright (C) 2019 Michael Shych <mshych@mellanox.com>
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/log2.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_data/mlxreg.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/types.h>
> > +#include <linux/watchdog.h>
> > +
> > +#define MLXREG_WDT_CLOCK_SCALE		1000
> > +#define MLXREG_WDT_MAX_TIMEOUT_TYPE1	32
> > +#define MLXREG_WDT_MAX_TIMEOUT_TYPE2	255
> > +#define MLXREG_WDT_MIN_TIMEOUT		1
> > +#define MLXREG_WDT_HW_TIMEOUT_CONVERT(hw_timeout) ((1 <<
> (hw_timeout)) \
> > +						   /
> MLXREG_WDT_CLOCK_SCALE)
> > +#define MLXREG_WDT_OPTIONS_BASE (WDIOF_KEEPALIVEPING |
> WDIOF_MAGICCLOSE | \
> > +				 WDIOF_SETTIMEOUT)
> > +
> > +/**
> > + * enum mlxreg_wdt_type - type of HW watchdog
> > + *
> > + * TYPE1 can be differentiated by different register/mask
> > + *	 for WD action set and ping.
> > + */
> > +enum mlxreg_wdt_type {
> > +	MLX_WDT_TYPE1,
> > +	MLX_WDT_TYPE2,
> > +};
> > +
> > +/**
> > + * struct mlxreg_wdt - wd private data:
> > + *
> > + * @wdd:	watchdog device;
> > + * @device:	basic device;
> > + * @pdata:	data received from platform driver;
> > + * @regmap:	register map of parent device;
> > + * @timeout:	defined timeout in sec.;
> > + * @hw_timeout:	real timeout set in hw;
> > + *		It will be roundup base of 2 in WD type 1,
> > + *		in WD type 2 it will be same number of sec as timeout;
> > + * @action_idx:	index for direct access to action register;
> > + * @timeout_idx:index for direct access to TO register;
> > + * @tleft_idx:	index for direct access to time left register;
> > + * @ping_idx:	index for direct access to ping register;
> > + * @reset_idx:	index for direct access to reset cause register;
> > + * @wd_type:	watchdog HW type;
> > + * @hw_timeout:	actual HW timeout;
> > + * @io_lock:	spinlock for io access;
> 
> watchdog device access is already synchromized in the watchdog core.
> Why is this additional lock needed ?

Removed it.
> 
> > + */
> > +struct mlxreg_wdt {
> > +	struct watchdog_device wdd;
> > +	struct mlxreg_core_platform_data *pdata;
> > +	void *regmap;
> > +	int action_idx;
> > +	int timeout_idx;
> > +	int tleft_idx;
> > +	int ping_idx;
> > +	int reset_idx;
> > +	enum mlxreg_wdt_type wdt_type;
> > +	u8 hw_timeout;
> > +	spinlock_t io_lock;	/* the lock for io operations */
> > +};
> > +
> > +static int mlxreg_wdt_roundup_to_base_2(struct mlxreg_wdt *wdt, int
> timeout)
> > +{
> > +	timeout *= MLXREG_WDT_CLOCK_SCALE;
> > +
> > +	wdt->hw_timeout = order_base_2(timeout);
> > +	dev_info(wdt->wdd.parent,
> > +		 "watchdog %s timeout %d was rounded up to %lu
> (msec)\n",
> > +		 wdt->wdd.info->identity, timeout,
> roundup_pow_of_two(timeout));
> 
> Please no noise here. It is quite common for watchdog drivers to
> set a timeout value not exactly matching the real timeout.
> That is why the selected value is reported to userspace.
> 
> > +
> > +	return 0;
> 
> What exactly is the point of this return value ?

Just made as in other watchdog examples.
Changed function to void.
> 
> > +}
> > +
> > +static enum mlxreg_wdt_type
> > +mlxreg_wdt_check_watchdog_type(struct mlxreg_wdt *wdt,
> > +			       struct mlxreg_core_platform_data *pdata)
> > +{
> > +	if (pdata->data[wdt->action_idx].reg ==
> > +	    pdata->data[wdt->ping_idx].reg &&
> > +	    pdata->data[wdt->action_idx].mask ==
> > +	    pdata->data[wdt->ping_idx].mask)
> > +		return MLX_WDT_TYPE2;
> > +
> > +	return MLX_WDT_TYPE1;
> > +}
> > +
> > +static int mlxreg_wdt_check_card_reset(struct mlxreg_wdt *wdt)
> > +{
> > +	struct mlxreg_core_data *reg_data;
> > +	u32 regval;
> > +	int rc;
> > +
> > +	if (wdt->reset_idx == -EINVAL)
> > +		return -EINVAL;
> > +
> Return values from this function are not checked and are
> thus pointless.

Changed function to void.
> 
> > +	if (!(wdt->wdd.info->options & WDIOF_CARDRESET))
> > +		return 0;
> > +
> > +	spin_lock(&wdt->io_lock);
> > +	reg_data = &wdt->pdata->data[wdt->reset_idx];
> > +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> > +	spin_unlock(&wdt->io_lock);
> > +	if (rc)
> > +		goto read_error;
> > +
> > +	if (regval & ~reg_data->mask) {
> > +		wdt->wdd.bootstatus = WDIOF_CARDRESET;
> > +		dev_info(wdt->wdd.parent,
> > +			 "watchdog previously reset the CPU\n");
> > +	}
> > +
> > +read_error:
> > +	return rc;
> > +}
> > +
> > +static int mlxreg_wdt_start(struct watchdog_device *wdd)
> > +{
> > +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt-
> >action_idx];
> > +	u32 regval;
> > +	int rc;
> > +
> > +	spin_lock(&wdt->io_lock);
> > +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> > +	if (rc) {
> > +		spin_unlock(&wdt->io_lock);
> > +		goto read_error;
> > +	}
> > +
> > +	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
> > +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> 
> I don't immediately see why regmap_update_bits() would not work here.

Replaced it to regmap_update_bits()
> 
> > +	spin_unlock(&wdt->io_lock);
> > +	if (!rc) {
> > +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> 
> Setting WDOG_HW_RUNNING is only necessary if set during probe,
> or in the stop function if the watchdog can not be stopped.
> It should not be necessary here.
> 
> [ I see you are calling this function from probe.
>   Set the bit there. ]

Moved to probe.
> 
> > +		dev_info(wdd->parent, "watchdog %s started\n",
> > +			 wdd->info->identity);
> 
> Is all this noise really necessary ?

Removed it.
> 
> > +	}
> > +
> > +read_error:
> > +	return rc;
> > +}
> > +
> > +static int mlxreg_wdt_stop(struct watchdog_device *wdd)
> > +{
> > +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt-
> >action_idx];
> > +	u32 regval;
> > +	int rc;
> > +
> > +	spin_lock(&wdt->io_lock);
> > +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> > +	if (rc) {
> > +		spin_unlock(&wdt->io_lock);
> > +		goto read_error;
> > +	}
> > +
> > +	regval = (regval & reg_data->mask) & ~BIT(reg_data->bit);
> > +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> 
> I don't immediately see why regmap_update_bits() would not work here.

Replaced it to regmap_update_bits()
> 
> > +	spin_unlock(&wdt->io_lock);
> > +	if (!rc)
> > +		dev_info(wdd->parent, "watchdog %s stopped\n",
> > +			 wdd->info->identity);
> > +
> > +read_error:
> > +	return rc;
> > +}
> > +
> > +static int mlxreg_wdt_ping(struct watchdog_device *wdd)
> > +{
> > +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt-
> >ping_idx];
> > +	u32 regval;
> > +	int rc;
> > +
> > +	spin_lock(&wdt->io_lock);
> > +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> > +	if (rc)
> > +		goto read_error;
> > +
> > +	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
> > +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> 
> Same as above. Besides, the only difference to the start function is the
> logging noise there. I would suggest to drop the noise as well as this
> function.
> 

Different register is used in ping and start.
Removed unneeded log.
Replaced to regmap_update_bits_base(), because this bit should be
always refreshed in HW, i.e. perform force update.
> > +
> > +read_error:
> > +	spin_unlock(&wdt->io_lock);
> > +
> > +	return rc;
> > +}
> > +
> > +static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
> > +				  unsigned int timeout)
> > +{
> > +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt-
> >timeout_idx];
> > +	u32 regval, set_time;
> > +	int rc;
> > +
> > +	spin_lock(&wdt->io_lock);
> > +
> > +	if (wdt->wdt_type == MLX_WDT_TYPE1) {
> > +		rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> > +		if (rc)
> > +			goto read_error;
> > +		mlxreg_wdt_roundup_to_base_2(wdt, timeout);
> > +		regval = (regval & reg_data->mask) | wdt->hw_timeout;
> > +		/* Rowndown to actual closest number of sec. */
> > +		set_time = MLXREG_WDT_HW_TIMEOUT_CONVERT(wdt-
> >hw_timeout);
> 
> The code is expected to set the value of "wdd->timeout" to the actual
> timeout. This is why it is set here, and not in the calling code.

Moved it to this function.
> 
> > +	} else {
> > +		wdt->hw_timeout = timeout;
> > +		set_time = timeout;
> > +		regval = timeout;
> > +	}
> > +
> > +	watchdog_init_timeout(&wdt->wdd, set_time, wdd->parent);
> 
> This is wrong here. Just set wdd->timeout. watchdog_init_timeout()
> should only be called in the probe function.

Set directly wdd->timeout
> 
> > +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> > +
> > +read_error:
> > +	spin_unlock(&wdt->io_lock);
> > +	if (!rc) {
> > +		if (watchdog_active(wdd)) {
> 
> This warrants an explanation: Presumably updating the timeout
> requires it to be stopped and restarted. Overall, however,
> it would probably be simpler to call regmap_update_bits()
> twice directly here.

Added comments.
> 
> > +			rc = mlxreg_wdt_stop(wdd);
> > +			if (!rc)
> > +				rc = mlxreg_wdt_start(wdd);
> > +		}
> > +	}
> > +
> > +	return rc;
> > +}
> > +
> > +static unsigned int mlxreg_wdt_get_timeleft(struct watchdog_device
> *wdd)
> > +{
> > +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt-
> >tleft_idx];
> > +	u32 regval;
> > +	int rc;
> > +
> > +	if (wdt->wdt_type == MLX_WDT_TYPE1)
> > +		return 0;
> > +
> Should never get here, thus this check is unnecessary.
> 

Removed it.
> > +	spin_lock(&wdt->io_lock);
> > +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> > +	spin_unlock(&wdt->io_lock);
> > +
> > +	return rc == 0 ? regval : 0;
> > +}
> > +
> > +static const struct watchdog_ops mlxreg_wdt_ops_type1 = {
> > +	.start		= mlxreg_wdt_start,
> > +	.stop		= mlxreg_wdt_stop,
> > +	.ping		= mlxreg_wdt_ping,
> > +	.set_timeout	= mlxreg_wdt_set_timeout,
> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +static const struct watchdog_ops mlxreg_wdt_ops_type2 = {
> > +	.start		= mlxreg_wdt_start,
> > +	.stop		= mlxreg_wdt_stop,
> > +	.ping		= mlxreg_wdt_ping,
> > +	.set_timeout	= mlxreg_wdt_set_timeout,
> > +	.get_timeleft	= mlxreg_wdt_get_timeleft,
> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +static const struct watchdog_info mlxreg_wdt_main_info = {
> > +	.options	= MLXREG_WDT_OPTIONS_BASE
> > +			| WDIOF_CARDRESET,
> > +	.identity	= "mlx-wdt-main",
> > +};
> > +
> > +static const struct watchdog_info mlxreg_wdt_aux_info = {
> > +	.options	= MLXREG_WDT_OPTIONS_BASE
> > +			| WDIOF_ALARMONLY,
> > +	.identity	= "mlx-wdt-aux",
> > +};
> > +
> > +static int mlxreg_wdt_config(struct mlxreg_wdt *wdt,
> > +			     struct mlxreg_core_platform_data *pdata)
> > +{
> > +	struct mlxreg_core_data *data = pdata->data;
> > +	int i;
> > +
> > +	wdt->reset_idx = -EINVAL;
> > +	for (i = 0; i < pdata->counter; i++, data++) {
> > +		if (strnstr(data->label, "action", sizeof(data->label)))
> > +			wdt->action_idx = i;
> > +		else if (strnstr(data->label, "timeout", sizeof(data->label)))
> > +			wdt->timeout_idx = i;
> > +		else if (strnstr(data->label, "timeleft", sizeof(data->label)))
> > +			wdt->tleft_idx = i;
> > +		else if (strnstr(data->label, "ping", sizeof(data->label)))
> > +			wdt->ping_idx = i;
> > +		else if (strnstr(data->label, "reset", sizeof(data->label)))
> > +			wdt->reset_idx = i;
> > +	}
> > +
> > +	wdt->pdata = pdata;
> > +	if (strnstr(pdata->identity, mlxreg_wdt_main_info.identity,
> > +		    sizeof(mlxreg_wdt_main_info.identity)))
> > +		wdt->wdd.info = &mlxreg_wdt_main_info;
> > +	else
> > +		wdt->wdd.info = &mlxreg_wdt_aux_info;
> > +
> > +	wdt->wdt_type = mlxreg_wdt_check_watchdog_type(wdt, pdata);
> > +	if (wdt->wdt_type == MLX_WDT_TYPE2) {
> > +		wdt->wdd.ops = &mlxreg_wdt_ops_type2;
> > +		wdt->wdd.max_timeout =
> MLXREG_WDT_MAX_TIMEOUT_TYPE2;
> > +	} else {
> > +		wdt->wdd.ops = &mlxreg_wdt_ops_type1;
> > +		wdt->wdd.max_timeout =
> MLXREG_WDT_MAX_TIMEOUT_TYPE1;
> > +	}
> > +	wdt->wdd.min_timeout = MLXREG_WDT_MIN_TIMEOUT;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mlxreg_wdt_init_timeout(struct mlxreg_wdt *wdt,
> > +				   struct mlxreg_core_platform_data *pdata)
> > +{
> > +	u32 timeout;
> > +
> > +	timeout = pdata->data[wdt->timeout_idx].health_cntr;
> > +	return mlxreg_wdt_set_timeout(&wdt->wdd, timeout);
> > +}
> > +
> > +static int mlxreg_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct mlxreg_core_platform_data *pdata;
> > +	struct mlxreg_wdt *wdt;
> > +	int rc;
> > +
> > +	pdata = dev_get_platdata(&pdev->dev);
> > +	if (!pdata) {
> > +		dev_err(&pdev->dev, "Failed to get platform data.\n");
> > +		return -EINVAL;
> > +	}
> > +	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> > +	if (!wdt)
> > +		return -ENOMEM;
> > +
> > +	spin_lock_init(&wdt->io_lock);
> > +
> > +	wdt->wdd.parent = &pdev->dev;
> > +	wdt->regmap = pdata->regmap;
> > +	mlxreg_wdt_config(wdt, pdata);
> > +
> > +	if ((pdata->features &
> MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START))
> > +		watchdog_set_nowayout(&wdt->wdd,
> WATCHDOG_NOWAYOUT);
> 
> That is a bit different than the requested functionality, isn't it ?
>

Changed name MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START to
MLXREG_CORE_WD_FEATURE_NOWAYOUT.
WDOG_NO_WAY_OUT	bit will be set if it's provided by platform and
Also CONFIG_WATCHDOG_NOWAYOUT is set.
 
> > +	watchdog_stop_on_reboot(&wdt->wdd);
> > +	watchdog_set_drvdata(&wdt->wdd, wdt);
> > +	mlxreg_wdt_check_card_reset(wdt);
> > +	rc = mlxreg_wdt_init_timeout(wdt, pdata);
> > +	if (rc) {
> > +		dev_err(&pdev->dev,
> > +			"Cannot init watchdog timeout (err=%d)\n", rc);
> > +		return rc;
> > +	}
> > +
> > +	rc = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
> > +	if (rc) {
> > +		dev_err(&pdev->dev,
> > +			"Cannot register watchdog device (err=%d)\n", rc);
> > +		return rc;
> > +	}
> > +
> > +	if ((pdata->features &
> MLXREG_CORE_WD_FEATURE_START_AT_BOOT))
> > +		mlxreg_wdt_start(&wdt->wdd);
> 
> This should be called before registering the watchdog, and
> WDOG_HW_RUNNING
> should be set here to let the core know that the watchdog is running.

Moved it and changed set of WDOG_HW_RUNNING
> 
> > +
> > +	return rc;
> 
> rc is always 0 here.
> 
> > +}
> > +
> > +static int mlxreg_wdt_remove(struct platform_device *pdev)
> > +{
> > +	struct mlxreg_wdt *wdt = dev_get_platdata(&pdev->dev);
> > +
> > +	mlxreg_wdt_stop(&wdt->wdd);
> 
> Call watchdog_stop_on_unregister() before registering the watchdog.
> 
> > +	watchdog_unregister_device(&wdt->wdd);
> 
> This is wrong. The watchdog is registered with a devm_ function
> and is thus automatically unregistered.
> 

Removed it.
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver mlxreg_wdt_driver = {
> > +	.probe	= mlxreg_wdt_probe,
> > +	.remove	= mlxreg_wdt_remove,
> > +	.driver	= {
> > +			.name = "mlx-wdt",
> > +	},
> > +};
> > +
> > +module_platform_driver(mlxreg_wdt_driver);
> > +
> > +MODULE_AUTHOR("Michael Shych <michaelsh@mellanox.com>");
> > +MODULE_DESCRIPTION("Mellanox watchdog driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:mlx-wdt");
> > --
> > 2.11.0
> >

Re: [PATCH v1 2/3] watchdog: mlx-wdt: introduce watchdog driver for Mellanox systems.

[Guenter Roeck]

On Wed, Feb 06, 2019 at 04:50:36PM +0000, michaelsh@mellanox.com wrote:
> From: Michael Shych <michaelsh@mellanox.com>
> 
> Introduce watchdog driver for variious rang of Mellanox Ethernet and

s/variious/various/

> Infiniband switch systems.
> 
> Watchdog driver for Mellanox watchdog devices, implemented in
> programmable logic device.
> 
> Main and auxiliary watchdog devices can exist on the same system.
> There are several actions that can be defined in the watchdog:
> system reset, start fans on full speed and increase a counter.
> The last 2 actions are performed without a system reset.
> Actions without reset are provided for auxiliary watchdog devices,
> which is optional.
> Access to HW registers is performed through generic
> regmap interface.
> 
> There are 2 types of HW watchdog implementations.
> Type 1: actual HW timeout can be defined as power of 2 msec.
> e.g. timeout 20 sec will be rounded up to 32768 msec.;
> maximum timeout period is 32 sec (32768 msec.);
> get time-left isn't supported
> Type 2: actual HW timeout is defined in sec. and it's the same as
> user-defined timeout;
> maximum timeout is 255 sec;
> get time-left is supported;
> 
> Watchdog driver is probed from the common mlx_platform driver.
> 
> Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> ---
>  drivers/watchdog/Kconfig   |  16 ++
>  drivers/watchdog/Makefile  |   1 +
>  drivers/watchdog/mlx_wdt.c | 406 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 423 insertions(+)
>  create mode 100644 drivers/watchdog/mlx_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 57f017d74a97..f1766eb081bb 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -241,6 +241,22 @@ config RAVE_SP_WATCHDOG
>  	help
>  	  Support for the watchdog on RAVE SP device.
>  
> +config MLX_WDT
> +	tristate "Mellanox Watchdog"
> +	depends on MELLANOX_PLATFORM
> +	select WATCHDOG_CORE
> +	select REGMAP
> +	help
> +	  This is the driver for the hardware watchdog on Mellanox systems.
> +	  If you are going to use it, say Y here, otherwise N.
> +	  This driver can be used together with the watchdog daemon.
> +	  It can also watch your kernel to make sure it doesn't freeze,
> +	  and if it does, it reboots your system after a certain amount of
> +	  time.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called mlx-wdt.
> +
>  # ALPHA Architecture
>  
>  # ARM Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a0917ef28e07..941b74185c9c 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -142,6 +142,7 @@ obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
>  obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
>  obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
>  obj-$(CONFIG_NIC7018_WDT) += nic7018_wdt.o
> +obj-$(CONFIG_MLX_WDT) += mlx_wdt.o
>  
>  # M68K Architecture
>  obj-$(CONFIG_M54xx_WATCHDOG) += m54xx_wdt.o
> diff --git a/drivers/watchdog/mlx_wdt.c b/drivers/watchdog/mlx_wdt.c
> new file mode 100644
> index 000000000000..be28aa165cf2
> --- /dev/null
> +++ b/drivers/watchdog/mlx_wdt.c
> @@ -0,0 +1,406 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Mellanox watchdog driver
> + *
> + * Copyright (C) 2019 Mellanox Technologies
> + * Copyright (C) 2019 Michael Shych <mshych@mellanox.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/log2.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/mlxreg.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>
> +
> +#define MLXREG_WDT_CLOCK_SCALE		1000
> +#define MLXREG_WDT_MAX_TIMEOUT_TYPE1	32
> +#define MLXREG_WDT_MAX_TIMEOUT_TYPE2	255
> +#define MLXREG_WDT_MIN_TIMEOUT		1
> +#define MLXREG_WDT_HW_TIMEOUT_CONVERT(hw_timeout) ((1 << (hw_timeout)) \
> +						   / MLXREG_WDT_CLOCK_SCALE)
> +#define MLXREG_WDT_OPTIONS_BASE (WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | \
> +				 WDIOF_SETTIMEOUT)
> +
> +/**
> + * enum mlxreg_wdt_type - type of HW watchdog
> + *
> + * TYPE1 can be differentiated by different register/mask
> + *	 for WD action set and ping.
> + */
> +enum mlxreg_wdt_type {
> +	MLX_WDT_TYPE1,
> +	MLX_WDT_TYPE2,
> +};
> +
> +/**
> + * struct mlxreg_wdt - wd private data:
> + *
> + * @wdd:	watchdog device;
> + * @device:	basic device;
> + * @pdata:	data received from platform driver;
> + * @regmap:	register map of parent device;
> + * @timeout:	defined timeout in sec.;
> + * @hw_timeout:	real timeout set in hw;
> + *		It will be roundup base of 2 in WD type 1,
> + *		in WD type 2 it will be same number of sec as timeout;
> + * @action_idx:	index for direct access to action register;
> + * @timeout_idx:index for direct access to TO register;
> + * @tleft_idx:	index for direct access to time left register;
> + * @ping_idx:	index for direct access to ping register;
> + * @reset_idx:	index for direct access to reset cause register;
> + * @wd_type:	watchdog HW type;
> + * @hw_timeout:	actual HW timeout;
> + * @io_lock:	spinlock for io access;

watchdog device access is already synchromized in the watchdog core.
Why is this additional lock needed ?

> + */
> +struct mlxreg_wdt {
> +	struct watchdog_device wdd;
> +	struct mlxreg_core_platform_data *pdata;
> +	void *regmap;
> +	int action_idx;
> +	int timeout_idx;
> +	int tleft_idx;
> +	int ping_idx;
> +	int reset_idx;
> +	enum mlxreg_wdt_type wdt_type;
> +	u8 hw_timeout;
> +	spinlock_t io_lock;	/* the lock for io operations */
> +};
> +
> +static int mlxreg_wdt_roundup_to_base_2(struct mlxreg_wdt *wdt, int timeout)
> +{
> +	timeout *= MLXREG_WDT_CLOCK_SCALE;
> +
> +	wdt->hw_timeout = order_base_2(timeout);
> +	dev_info(wdt->wdd.parent,
> +		 "watchdog %s timeout %d was rounded up to %lu (msec)\n",
> +		 wdt->wdd.info->identity, timeout, roundup_pow_of_two(timeout));

Please no noise here. It is quite common for watchdog drivers to 
set a timeout value not exactly matching the real timeout.
That is why the selected value is reported to userspace.

> +
> +	return 0;

What exactly is the point of this return value ?

> +}
> +
> +static enum mlxreg_wdt_type
> +mlxreg_wdt_check_watchdog_type(struct mlxreg_wdt *wdt,
> +			       struct mlxreg_core_platform_data *pdata)
> +{
> +	if (pdata->data[wdt->action_idx].reg ==
> +	    pdata->data[wdt->ping_idx].reg &&
> +	    pdata->data[wdt->action_idx].mask ==
> +	    pdata->data[wdt->ping_idx].mask)
> +		return MLX_WDT_TYPE2;
> +
> +	return MLX_WDT_TYPE1;
> +}
> +
> +static int mlxreg_wdt_check_card_reset(struct mlxreg_wdt *wdt)
> +{
> +	struct mlxreg_core_data *reg_data;
> +	u32 regval;
> +	int rc;
> +
> +	if (wdt->reset_idx == -EINVAL)
> +		return -EINVAL;
> +
Return values from this function are not checked and are
thus pointless.

> +	if (!(wdt->wdd.info->options & WDIOF_CARDRESET))
> +		return 0;
> +
> +	spin_lock(&wdt->io_lock);
> +	reg_data = &wdt->pdata->data[wdt->reset_idx];
> +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +	spin_unlock(&wdt->io_lock);
> +	if (rc)
> +		goto read_error;
> +
> +	if (regval & ~reg_data->mask) {
> +		wdt->wdd.bootstatus = WDIOF_CARDRESET;
> +		dev_info(wdt->wdd.parent,
> +			 "watchdog previously reset the CPU\n");
> +	}
> +
> +read_error:
> +	return rc;
> +}
> +
> +static int mlxreg_wdt_start(struct watchdog_device *wdd)
> +{
> +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->action_idx];
> +	u32 regval;
> +	int rc;
> +
> +	spin_lock(&wdt->io_lock);
> +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +	if (rc) {
> +		spin_unlock(&wdt->io_lock);
> +		goto read_error;
> +	}
> +
> +	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
> +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);

I don't immediately see why regmap_update_bits() would not work here.

> +	spin_unlock(&wdt->io_lock);
> +	if (!rc) {
> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);

Setting WDOG_HW_RUNNING is only necessary if set during probe,
or in the stop function if the watchdog can not be stopped.
It should not be necessary here. 

[ I see you are calling this function from probe.
  Set the bit there. ]

> +		dev_info(wdd->parent, "watchdog %s started\n",
> +			 wdd->info->identity);

Is all this noise really necessary ?

> +	}
> +
> +read_error:
> +	return rc;
> +}
> +
> +static int mlxreg_wdt_stop(struct watchdog_device *wdd)
> +{
> +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->action_idx];
> +	u32 regval;
> +	int rc;
> +
> +	spin_lock(&wdt->io_lock);
> +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +	if (rc) {
> +		spin_unlock(&wdt->io_lock);
> +		goto read_error;
> +	}
> +
> +	regval = (regval & reg_data->mask) & ~BIT(reg_data->bit);
> +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);

I don't immediately see why regmap_update_bits() would not work here.

> +	spin_unlock(&wdt->io_lock);
> +	if (!rc)
> +		dev_info(wdd->parent, "watchdog %s stopped\n",
> +			 wdd->info->identity);
> +
> +read_error:
> +	return rc;
> +}
> +
> +static int mlxreg_wdt_ping(struct watchdog_device *wdd)
> +{
> +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->ping_idx];
> +	u32 regval;
> +	int rc;
> +
> +	spin_lock(&wdt->io_lock);
> +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +	if (rc)
> +		goto read_error;
> +
> +	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
> +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);

Same as above. Besides, the only difference to the start function is the
logging noise there. I would suggest to drop the noise as well as this
function.

> +
> +read_error:
> +	spin_unlock(&wdt->io_lock);
> +
> +	return rc;
> +}
> +
> +static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
> +				  unsigned int timeout)
> +{
> +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->timeout_idx];
> +	u32 regval, set_time;
> +	int rc;
> +
> +	spin_lock(&wdt->io_lock);
> +
> +	if (wdt->wdt_type == MLX_WDT_TYPE1) {
> +		rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +		if (rc)
> +			goto read_error;
> +		mlxreg_wdt_roundup_to_base_2(wdt, timeout);
> +		regval = (regval & reg_data->mask) | wdt->hw_timeout;
> +		/* Rowndown to actual closest number of sec. */
> +		set_time = MLXREG_WDT_HW_TIMEOUT_CONVERT(wdt->hw_timeout);

The code is expected to set the value of "wdd->timeout" to the actual
timeout. This is why it is set here, and not in the calling code.

> +	} else {
> +		wdt->hw_timeout = timeout;
> +		set_time = timeout;
> +		regval = timeout;
> +	}
> +
> +	watchdog_init_timeout(&wdt->wdd, set_time, wdd->parent);

This is wrong here. Just set wdd->timeout. watchdog_init_timeout()
should only be called in the probe function.

> +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> +
> +read_error:
> +	spin_unlock(&wdt->io_lock);
> +	if (!rc) {
> +		if (watchdog_active(wdd)) {

This warrants an explanation: Presumably updating the timeout
requires it to be stopped and restarted. Overall, however,
it would probably be simpler to call regmap_update_bits()
twice directly here.

> +			rc = mlxreg_wdt_stop(wdd);
> +			if (!rc)
> +				rc = mlxreg_wdt_start(wdd);
> +		}
> +	}
> +
> +	return rc;
> +}
> +
> +static unsigned int mlxreg_wdt_get_timeleft(struct watchdog_device *wdd)
> +{
> +	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->tleft_idx];
> +	u32 regval;
> +	int rc;
> +
> +	if (wdt->wdt_type == MLX_WDT_TYPE1)
> +		return 0;
> +
Should never get here, thus this check is unnecessary.

> +	spin_lock(&wdt->io_lock);
> +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +	spin_unlock(&wdt->io_lock);
> +
> +	return rc == 0 ? regval : 0;
> +}
> +
> +static const struct watchdog_ops mlxreg_wdt_ops_type1 = {
> +	.start		= mlxreg_wdt_start,
> +	.stop		= mlxreg_wdt_stop,
> +	.ping		= mlxreg_wdt_ping,
> +	.set_timeout	= mlxreg_wdt_set_timeout,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct watchdog_ops mlxreg_wdt_ops_type2 = {
> +	.start		= mlxreg_wdt_start,
> +	.stop		= mlxreg_wdt_stop,
> +	.ping		= mlxreg_wdt_ping,
> +	.set_timeout	= mlxreg_wdt_set_timeout,
> +	.get_timeleft	= mlxreg_wdt_get_timeleft,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct watchdog_info mlxreg_wdt_main_info = {
> +	.options	= MLXREG_WDT_OPTIONS_BASE
> +			| WDIOF_CARDRESET,
> +	.identity	= "mlx-wdt-main",
> +};
> +
> +static const struct watchdog_info mlxreg_wdt_aux_info = {
> +	.options	= MLXREG_WDT_OPTIONS_BASE
> +			| WDIOF_ALARMONLY,
> +	.identity	= "mlx-wdt-aux",
> +};
> +
> +static int mlxreg_wdt_config(struct mlxreg_wdt *wdt,
> +			     struct mlxreg_core_platform_data *pdata)
> +{
> +	struct mlxreg_core_data *data = pdata->data;
> +	int i;
> +
> +	wdt->reset_idx = -EINVAL;
> +	for (i = 0; i < pdata->counter; i++, data++) {
> +		if (strnstr(data->label, "action", sizeof(data->label)))
> +			wdt->action_idx = i;
> +		else if (strnstr(data->label, "timeout", sizeof(data->label)))
> +			wdt->timeout_idx = i;
> +		else if (strnstr(data->label, "timeleft", sizeof(data->label)))
> +			wdt->tleft_idx = i;
> +		else if (strnstr(data->label, "ping", sizeof(data->label)))
> +			wdt->ping_idx = i;
> +		else if (strnstr(data->label, "reset", sizeof(data->label)))
> +			wdt->reset_idx = i;
> +	}
> +
> +	wdt->pdata = pdata;
> +	if (strnstr(pdata->identity, mlxreg_wdt_main_info.identity,
> +		    sizeof(mlxreg_wdt_main_info.identity)))
> +		wdt->wdd.info = &mlxreg_wdt_main_info;
> +	else
> +		wdt->wdd.info = &mlxreg_wdt_aux_info;
> +
> +	wdt->wdt_type = mlxreg_wdt_check_watchdog_type(wdt, pdata);
> +	if (wdt->wdt_type == MLX_WDT_TYPE2) {
> +		wdt->wdd.ops = &mlxreg_wdt_ops_type2;
> +		wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE2;
> +	} else {
> +		wdt->wdd.ops = &mlxreg_wdt_ops_type1;
> +		wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE1;
> +	}
> +	wdt->wdd.min_timeout = MLXREG_WDT_MIN_TIMEOUT;
> +
> +	return 0;
> +}
> +
> +static int mlxreg_wdt_init_timeout(struct mlxreg_wdt *wdt,
> +				   struct mlxreg_core_platform_data *pdata)
> +{
> +	u32 timeout;
> +
> +	timeout = pdata->data[wdt->timeout_idx].health_cntr;
> +	return mlxreg_wdt_set_timeout(&wdt->wdd, timeout);
> +}
> +
> +static int mlxreg_wdt_probe(struct platform_device *pdev)
> +{
> +	struct mlxreg_core_platform_data *pdata;
> +	struct mlxreg_wdt *wdt;
> +	int rc;
> +
> +	pdata = dev_get_platdata(&pdev->dev);
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "Failed to get platform data.\n");
> +		return -EINVAL;
> +	}
> +	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&wdt->io_lock);
> +
> +	wdt->wdd.parent = &pdev->dev;
> +	wdt->regmap = pdata->regmap;
> +	mlxreg_wdt_config(wdt, pdata);
> +
> +	if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START))
> +		watchdog_set_nowayout(&wdt->wdd, WATCHDOG_NOWAYOUT);

That is a bit different than the requested functionality, isn't it ?

> +	watchdog_stop_on_reboot(&wdt->wdd);
> +	watchdog_set_drvdata(&wdt->wdd, wdt);
> +	mlxreg_wdt_check_card_reset(wdt);
> +	rc = mlxreg_wdt_init_timeout(wdt, pdata);
> +	if (rc) {
> +		dev_err(&pdev->dev,
> +			"Cannot init watchdog timeout (err=%d)\n", rc);
> +		return rc;
> +	}
> +
> +	rc = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
> +	if (rc) {
> +		dev_err(&pdev->dev,
> +			"Cannot register watchdog device (err=%d)\n", rc);
> +		return rc;
> +	}
> +
> +	if ((pdata->features & MLXREG_CORE_WD_FEATURE_START_AT_BOOT))
> +		mlxreg_wdt_start(&wdt->wdd);

This should be called before registering the watchdog, and WDOG_HW_RUNNING
should be set here to let the core know that the watchdog is running.

> +
> +	return rc;

rc is always 0 here.

> +}
> +
> +static int mlxreg_wdt_remove(struct platform_device *pdev)
> +{
> +	struct mlxreg_wdt *wdt = dev_get_platdata(&pdev->dev);
> +
> +	mlxreg_wdt_stop(&wdt->wdd);

Call watchdog_stop_on_unregister() before registering the watchdog.

> +	watchdog_unregister_device(&wdt->wdd);

This is wrong. The watchdog is registered with a devm_ function
and is thus automatically unregistered. 

> +
> +	return 0;
> +}
> +
> +static struct platform_driver mlxreg_wdt_driver = {
> +	.probe	= mlxreg_wdt_probe,
> +	.remove	= mlxreg_wdt_remove,
> +	.driver	= {
> +			.name = "mlx-wdt",
> +	},
> +};
> +
> +module_platform_driver(mlxreg_wdt_driver);
> +
> +MODULE_AUTHOR("Michael Shych <michaelsh@mellanox.com>");
> +MODULE_DESCRIPTION("Mellanox watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:mlx-wdt");
> -- 
> 2.11.0
> 

[PATCH v1 2/3] watchdog: mlx-wdt: introduce watchdog driver for Mellanox systems.

[michaelsh]

From: Michael Shych <michaelsh@mellanox.com>

Introduce watchdog driver for variious rang of Mellanox Ethernet and
Infiniband switch systems.

Watchdog driver for Mellanox watchdog devices, implemented in
programmable logic device.

Main and auxiliary watchdog devices can exist on the same system.
There are several actions that can be defined in the watchdog:
system reset, start fans on full speed and increase a counter.
The last 2 actions are performed without a system reset.
Actions without reset are provided for auxiliary watchdog devices,
which is optional.
Access to HW registers is performed through generic
regmap interface.

There are 2 types of HW watchdog implementations.
Type 1: actual HW timeout can be defined as power of 2 msec.
e.g. timeout 20 sec will be rounded up to 32768 msec.;
maximum timeout period is 32 sec (32768 msec.);
get time-left isn't supported
Type 2: actual HW timeout is defined in sec. and it's the same as
user-defined timeout;
maximum timeout is 255 sec;
get time-left is supported;

Watchdog driver is probed from the common mlx_platform driver.

Signed-off-by: Michael Shych <michaelsh@mellanox.com>
---
 drivers/watchdog/Kconfig   |  16 ++
 drivers/watchdog/Makefile  |   1 +
 drivers/watchdog/mlx_wdt.c | 406 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 423 insertions(+)
 create mode 100644 drivers/watchdog/mlx_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 57f017d74a97..f1766eb081bb 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -241,6 +241,22 @@ config RAVE_SP_WATCHDOG
 	help
 	  Support for the watchdog on RAVE SP device.
 
+config MLX_WDT
+	tristate "Mellanox Watchdog"
+	depends on MELLANOX_PLATFORM
+	select WATCHDOG_CORE
+	select REGMAP
+	help
+	  This is the driver for the hardware watchdog on Mellanox systems.
+	  If you are going to use it, say Y here, otherwise N.
+	  This driver can be used together with the watchdog daemon.
+	  It can also watch your kernel to make sure it doesn't freeze,
+	  and if it does, it reboots your system after a certain amount of
+	  time.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mlx-wdt.
+
 # ALPHA Architecture
 
 # ARM Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a0917ef28e07..941b74185c9c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
 obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
 obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
 obj-$(CONFIG_NIC7018_WDT) += nic7018_wdt.o
+obj-$(CONFIG_MLX_WDT) += mlx_wdt.o
 
 # M68K Architecture
 obj-$(CONFIG_M54xx_WATCHDOG) += m54xx_wdt.o
diff --git a/drivers/watchdog/mlx_wdt.c b/drivers/watchdog/mlx_wdt.c
new file mode 100644
index 000000000000..be28aa165cf2
--- /dev/null
+++ b/drivers/watchdog/mlx_wdt.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Mellanox watchdog driver
+ *
+ * Copyright (C) 2019 Mellanox Technologies
+ * Copyright (C) 2019 Michael Shych <mshych@mellanox.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+#include <linux/platform_data/mlxreg.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#define MLXREG_WDT_CLOCK_SCALE		1000
+#define MLXREG_WDT_MAX_TIMEOUT_TYPE1	32
+#define MLXREG_WDT_MAX_TIMEOUT_TYPE2	255
+#define MLXREG_WDT_MIN_TIMEOUT		1
+#define MLXREG_WDT_HW_TIMEOUT_CONVERT(hw_timeout) ((1 << (hw_timeout)) \
+						   / MLXREG_WDT_CLOCK_SCALE)
+#define MLXREG_WDT_OPTIONS_BASE (WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | \
+				 WDIOF_SETTIMEOUT)
+
+/**
+ * enum mlxreg_wdt_type - type of HW watchdog
+ *
+ * TYPE1 can be differentiated by different register/mask
+ *	 for WD action set and ping.
+ */
+enum mlxreg_wdt_type {
+	MLX_WDT_TYPE1,
+	MLX_WDT_TYPE2,
+};
+
+/**
+ * struct mlxreg_wdt - wd private data:
+ *
+ * @wdd:	watchdog device;
+ * @device:	basic device;
+ * @pdata:	data received from platform driver;
+ * @regmap:	register map of parent device;
+ * @timeout:	defined timeout in sec.;
+ * @hw_timeout:	real timeout set in hw;
+ *		It will be roundup base of 2 in WD type 1,
+ *		in WD type 2 it will be same number of sec as timeout;
+ * @action_idx:	index for direct access to action register;
+ * @timeout_idx:index for direct access to TO register;
+ * @tleft_idx:	index for direct access to time left register;
+ * @ping_idx:	index for direct access to ping register;
+ * @reset_idx:	index for direct access to reset cause register;
+ * @wd_type:	watchdog HW type;
+ * @hw_timeout:	actual HW timeout;
+ * @io_lock:	spinlock for io access;
+ */
+struct mlxreg_wdt {
+	struct watchdog_device wdd;
+	struct mlxreg_core_platform_data *pdata;
+	void *regmap;
+	int action_idx;
+	int timeout_idx;
+	int tleft_idx;
+	int ping_idx;
+	int reset_idx;
+	enum mlxreg_wdt_type wdt_type;
+	u8 hw_timeout;
+	spinlock_t io_lock;	/* the lock for io operations */
+};
+
+static int mlxreg_wdt_roundup_to_base_2(struct mlxreg_wdt *wdt, int timeout)
+{
+	timeout *= MLXREG_WDT_CLOCK_SCALE;
+
+	wdt->hw_timeout = order_base_2(timeout);
+	dev_info(wdt->wdd.parent,
+		 "watchdog %s timeout %d was rounded up to %lu (msec)\n",
+		 wdt->wdd.info->identity, timeout, roundup_pow_of_two(timeout));
+
+	return 0;
+}
+
+static enum mlxreg_wdt_type
+mlxreg_wdt_check_watchdog_type(struct mlxreg_wdt *wdt,
+			       struct mlxreg_core_platform_data *pdata)
+{
+	if (pdata->data[wdt->action_idx].reg ==
+	    pdata->data[wdt->ping_idx].reg &&
+	    pdata->data[wdt->action_idx].mask ==
+	    pdata->data[wdt->ping_idx].mask)
+		return MLX_WDT_TYPE2;
+
+	return MLX_WDT_TYPE1;
+}
+
+static int mlxreg_wdt_check_card_reset(struct mlxreg_wdt *wdt)
+{
+	struct mlxreg_core_data *reg_data;
+	u32 regval;
+	int rc;
+
+	if (wdt->reset_idx == -EINVAL)
+		return -EINVAL;
+
+	if (!(wdt->wdd.info->options & WDIOF_CARDRESET))
+		return 0;
+
+	spin_lock(&wdt->io_lock);
+	reg_data = &wdt->pdata->data[wdt->reset_idx];
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	spin_unlock(&wdt->io_lock);
+	if (rc)
+		goto read_error;
+
+	if (regval & ~reg_data->mask) {
+		wdt->wdd.bootstatus = WDIOF_CARDRESET;
+		dev_info(wdt->wdd.parent,
+			 "watchdog previously reset the CPU\n");
+	}
+
+read_error:
+	return rc;
+}
+
+static int mlxreg_wdt_start(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->action_idx];
+	u32 regval;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	if (rc) {
+		spin_unlock(&wdt->io_lock);
+		goto read_error;
+	}
+
+	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+	spin_unlock(&wdt->io_lock);
+	if (!rc) {
+		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
+		dev_info(wdd->parent, "watchdog %s started\n",
+			 wdd->info->identity);
+	}
+
+read_error:
+	return rc;
+}
+
+static int mlxreg_wdt_stop(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->action_idx];
+	u32 regval;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	if (rc) {
+		spin_unlock(&wdt->io_lock);
+		goto read_error;
+	}
+
+	regval = (regval & reg_data->mask) & ~BIT(reg_data->bit);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+	spin_unlock(&wdt->io_lock);
+	if (!rc)
+		dev_info(wdd->parent, "watchdog %s stopped\n",
+			 wdd->info->identity);
+
+read_error:
+	return rc;
+}
+
+static int mlxreg_wdt_ping(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->ping_idx];
+	u32 regval;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	if (rc)
+		goto read_error;
+
+	regval = (regval & reg_data->mask) | BIT(reg_data->bit);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+
+read_error:
+	spin_unlock(&wdt->io_lock);
+
+	return rc;
+}
+
+static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
+				  unsigned int timeout)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->timeout_idx];
+	u32 regval, set_time;
+	int rc;
+
+	spin_lock(&wdt->io_lock);
+
+	if (wdt->wdt_type == MLX_WDT_TYPE1) {
+		rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+		if (rc)
+			goto read_error;
+		mlxreg_wdt_roundup_to_base_2(wdt, timeout);
+		regval = (regval & reg_data->mask) | wdt->hw_timeout;
+		/* Rowndown to actual closest number of sec. */
+		set_time = MLXREG_WDT_HW_TIMEOUT_CONVERT(wdt->hw_timeout);
+	} else {
+		wdt->hw_timeout = timeout;
+		set_time = timeout;
+		regval = timeout;
+	}
+
+	watchdog_init_timeout(&wdt->wdd, set_time, wdd->parent);
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+
+read_error:
+	spin_unlock(&wdt->io_lock);
+	if (!rc) {
+		if (watchdog_active(wdd)) {
+			rc = mlxreg_wdt_stop(wdd);
+			if (!rc)
+				rc = mlxreg_wdt_start(wdd);
+		}
+	}
+
+	return rc;
+}
+
+static unsigned int mlxreg_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct mlxreg_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct mlxreg_core_data *reg_data = &wdt->pdata->data[wdt->tleft_idx];
+	u32 regval;
+	int rc;
+
+	if (wdt->wdt_type == MLX_WDT_TYPE1)
+		return 0;
+
+	spin_lock(&wdt->io_lock);
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	spin_unlock(&wdt->io_lock);
+
+	return rc == 0 ? regval : 0;
+}
+
+static const struct watchdog_ops mlxreg_wdt_ops_type1 = {
+	.start		= mlxreg_wdt_start,
+	.stop		= mlxreg_wdt_stop,
+	.ping		= mlxreg_wdt_ping,
+	.set_timeout	= mlxreg_wdt_set_timeout,
+	.owner		= THIS_MODULE,
+};
+
+static const struct watchdog_ops mlxreg_wdt_ops_type2 = {
+	.start		= mlxreg_wdt_start,
+	.stop		= mlxreg_wdt_stop,
+	.ping		= mlxreg_wdt_ping,
+	.set_timeout	= mlxreg_wdt_set_timeout,
+	.get_timeleft	= mlxreg_wdt_get_timeleft,
+	.owner		= THIS_MODULE,
+};
+
+static const struct watchdog_info mlxreg_wdt_main_info = {
+	.options	= MLXREG_WDT_OPTIONS_BASE
+			| WDIOF_CARDRESET,
+	.identity	= "mlx-wdt-main",
+};
+
+static const struct watchdog_info mlxreg_wdt_aux_info = {
+	.options	= MLXREG_WDT_OPTIONS_BASE
+			| WDIOF_ALARMONLY,
+	.identity	= "mlx-wdt-aux",
+};
+
+static int mlxreg_wdt_config(struct mlxreg_wdt *wdt,
+			     struct mlxreg_core_platform_data *pdata)
+{
+	struct mlxreg_core_data *data = pdata->data;
+	int i;
+
+	wdt->reset_idx = -EINVAL;
+	for (i = 0; i < pdata->counter; i++, data++) {
+		if (strnstr(data->label, "action", sizeof(data->label)))
+			wdt->action_idx = i;
+		else if (strnstr(data->label, "timeout", sizeof(data->label)))
+			wdt->timeout_idx = i;
+		else if (strnstr(data->label, "timeleft", sizeof(data->label)))
+			wdt->tleft_idx = i;
+		else if (strnstr(data->label, "ping", sizeof(data->label)))
+			wdt->ping_idx = i;
+		else if (strnstr(data->label, "reset", sizeof(data->label)))
+			wdt->reset_idx = i;
+	}
+
+	wdt->pdata = pdata;
+	if (strnstr(pdata->identity, mlxreg_wdt_main_info.identity,
+		    sizeof(mlxreg_wdt_main_info.identity)))
+		wdt->wdd.info = &mlxreg_wdt_main_info;
+	else
+		wdt->wdd.info = &mlxreg_wdt_aux_info;
+
+	wdt->wdt_type = mlxreg_wdt_check_watchdog_type(wdt, pdata);
+	if (wdt->wdt_type == MLX_WDT_TYPE2) {
+		wdt->wdd.ops = &mlxreg_wdt_ops_type2;
+		wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE2;
+	} else {
+		wdt->wdd.ops = &mlxreg_wdt_ops_type1;
+		wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE1;
+	}
+	wdt->wdd.min_timeout = MLXREG_WDT_MIN_TIMEOUT;
+
+	return 0;
+}
+
+static int mlxreg_wdt_init_timeout(struct mlxreg_wdt *wdt,
+				   struct mlxreg_core_platform_data *pdata)
+{
+	u32 timeout;
+
+	timeout = pdata->data[wdt->timeout_idx].health_cntr;
+	return mlxreg_wdt_set_timeout(&wdt->wdd, timeout);
+}
+
+static int mlxreg_wdt_probe(struct platform_device *pdev)
+{
+	struct mlxreg_core_platform_data *pdata;
+	struct mlxreg_wdt *wdt;
+	int rc;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		dev_err(&pdev->dev, "Failed to get platform data.\n");
+		return -EINVAL;
+	}
+	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	spin_lock_init(&wdt->io_lock);
+
+	wdt->wdd.parent = &pdev->dev;
+	wdt->regmap = pdata->regmap;
+	mlxreg_wdt_config(wdt, pdata);
+
+	if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START))
+		watchdog_set_nowayout(&wdt->wdd, WATCHDOG_NOWAYOUT);
+	watchdog_stop_on_reboot(&wdt->wdd);
+	watchdog_set_drvdata(&wdt->wdd, wdt);
+	mlxreg_wdt_check_card_reset(wdt);
+	rc = mlxreg_wdt_init_timeout(wdt, pdata);
+	if (rc) {
+		dev_err(&pdev->dev,
+			"Cannot init watchdog timeout (err=%d)\n", rc);
+		return rc;
+	}
+
+	rc = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
+	if (rc) {
+		dev_err(&pdev->dev,
+			"Cannot register watchdog device (err=%d)\n", rc);
+		return rc;
+	}
+
+	if ((pdata->features & MLXREG_CORE_WD_FEATURE_START_AT_BOOT))
+		mlxreg_wdt_start(&wdt->wdd);
+
+	return rc;
+}
+
+static int mlxreg_wdt_remove(struct platform_device *pdev)
+{
+	struct mlxreg_wdt *wdt = dev_get_platdata(&pdev->dev);
+
+	mlxreg_wdt_stop(&wdt->wdd);
+	watchdog_unregister_device(&wdt->wdd);
+
+	return 0;
+}
+
+static struct platform_driver mlxreg_wdt_driver = {
+	.probe	= mlxreg_wdt_probe,
+	.remove	= mlxreg_wdt_remove,
+	.driver	= {
+			.name = "mlx-wdt",
+	},
+};
+
+module_platform_driver(mlxreg_wdt_driver);
+
+MODULE_AUTHOR("Michael Shych <michaelsh@mellanox.com>");
+MODULE_DESCRIPTION("Mellanox watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mlx-wdt");
-- 
2.11.0