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

[PATCH v4 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 different 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:	additions for Mellanox watchdog driver.
  watchdog: mlx-wdt: introduce a watchdog driver for Mellanox systems.
  Documentation/watchdog: Add documentation mlx-wdt driver

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

-- 
2.11.0

[PATCH v4 1/3] platform_data/mlxreg: additions 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>

---
v1->v2
Change feature name MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START
to MLXREG_CORE_WD_FEATURE_NOWAYOUT
---
v2->v3
1. Move enum enum mlxreg_wdt_type from mlx_wdt driver to common mlxreg.h
2. Add version field to mlxreg_core_platform_data struct.
---
 include/linux/platform_data/mlxreg.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
index 19f5cb618c55..31f7c25a44da 100644
--- a/include/linux/platform_data/mlxreg.h
+++ b/include/linux/platform_data/mlxreg.h
@@ -35,6 +35,19 @@
 #define __LINUX_PLATFORM_DATA_MLXREG_H
 
 #define MLXREG_CORE_LABEL_MAX_SIZE	32
+#define MLXREG_CORE_WD_FEATURE_NOWAYOUT		BIT(0)
+#define MLXREG_CORE_WD_FEATURE_START_AT_BOOT	BIT(1)
+
+/**
+ * enum mlxreg_wdt_type - type of HW watchdog
+ *
+ * TYPE1 HW watchdog implementation exist in old systems.
+ * All new systems have TYPE2 HW watchdog.
+ */
+enum mlxreg_wdt_type {
+	MLX_WDT_TYPE1,
+	MLX_WDT_TYPE2,
+};
 
 /**
  * struct mlxreg_hotplug_device - I2C device data:
@@ -110,11 +123,17 @@ 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;
+ * @version: implementation version;
+ * @identity: device identity name;
  */
 struct mlxreg_core_platform_data {
 	struct mlxreg_core_data *data;
 	void *regmap;
 	int counter;
+	u32 features;
+	u32 version;
+	char identity[MLXREG_CORE_LABEL_MAX_SIZE];
 };
 
 /**
-- 
2.11.0

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

[michaelsh]

From: Michael Shych <michaelsh@mellanox.com>

Introduce watchdog driver for a various range 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>

---
v1->v2
Comments pointed out by Guenter:
1. Remove unneeded lock in access functions.
2. Change return  int to void in functions with constant 0 return value
   or with unchecked returned code.
3. Use regmap_update_bits() or regmap_update_bits_base() functions.
4. Remove unneeded dev_info messages.
5. Change set_timeout function.
6. Changes in probe
7. Delete unneeded remove function.
---
v2->v3
Comments pointed out by Guenter:
1. Remove unneedeed define.
2. Remove structure field hw_timeout. Use local var instead it.
3. Remove unnecesarry label.
4. Move enum mlxreg_wdt_type to common mlxreh.h file.
5. Watchdog HW type will be provided by parent platform driver,
   no need to find this in mlx-wdt driver.
---
v3->v4
Comment pointed out by Guenter:
Add cooment about return value of get_timeleft function.
---
 drivers/watchdog/Kconfig   |  16 +++
 drivers/watchdog/Makefile  |   1 +
 drivers/watchdog/mlx_wdt.c | 290 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 307 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..70c2cbf9c993
--- /dev/null
+++ b/drivers/watchdog/mlx_wdt.c
@@ -0,0 +1,290 @@
+// 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_OPTIONS_BASE (WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | \
+				 WDIOF_SETTIMEOUT)
+
+/**
+ * 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.;
+ * @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;
+ */
+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;
+};
+
+static void 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;
+
+	if (!(wdt->wdd.info->options & WDIOF_CARDRESET))
+		return;
+
+	reg_data = &wdt->pdata->data[wdt->reset_idx];
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	if (!rc) {
+		if (regval & ~reg_data->mask) {
+			wdt->wdd.bootstatus = WDIOF_CARDRESET;
+			dev_info(wdt->wdd.parent,
+				 "watchdog previously reset the CPU\n");
+		}
+	}
+}
+
+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];
+
+	return regmap_update_bits(wdt->regmap, reg_data->reg, ~reg_data->mask,
+				  BIT(reg_data->bit));
+}
+
+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];
+
+	return regmap_update_bits(wdt->regmap, reg_data->reg, ~reg_data->mask,
+				  ~BIT(reg_data->bit));
+}
+
+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];
+
+	return regmap_update_bits_base(wdt->regmap, reg_data->reg,
+				       ~reg_data->mask, BIT(reg_data->bit),
+				       NULL, false, true);
+}
+
+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, hw_timeout;
+	int rc;
+
+	if (wdt->wdt_type == MLX_WDT_TYPE1) {
+		rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+		if (rc)
+			return rc;
+
+		hw_timeout = order_base_2(timeout * MLXREG_WDT_CLOCK_SCALE);
+		regval = (regval & reg_data->mask) | hw_timeout;
+		/* Rowndown to actual closest number of sec. */
+		set_time = BIT(hw_timeout) / MLXREG_WDT_CLOCK_SCALE;
+	} else {
+		set_time = timeout;
+		regval = timeout;
+	}
+
+	wdd->timeout = set_time;
+	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+
+	if (!rc) {
+		/*
+		 * Restart watchdog with new timeout period
+		 * if watchdog is already started.
+		 */
+		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;
+
+	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
+	/* Return 0 timeleft in case of failure register read. */
+	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 void 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 = pdata->version;
+	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;
+}
+
+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;
+
+	wdt->wdd.parent = &pdev->dev;
+	wdt->regmap = pdata->regmap;
+	mlxreg_wdt_config(wdt, pdata);
+
+	if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOWAYOUT))
+		watchdog_set_nowayout(&wdt->wdd, WATCHDOG_NOWAYOUT);
+	watchdog_stop_on_reboot(&wdt->wdd);
+	watchdog_stop_on_unregister(&wdt->wdd);
+	watchdog_set_drvdata(&wdt->wdd, wdt);
+	rc = mlxreg_wdt_init_timeout(wdt, pdata);
+	if (rc)
+		goto register_error;
+
+	if ((pdata->features & MLXREG_CORE_WD_FEATURE_START_AT_BOOT)) {
+		rc = mlxreg_wdt_start(&wdt->wdd);
+		if (rc)
+			goto register_error;
+		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
+	}
+	mlxreg_wdt_check_card_reset(wdt);
+	rc = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
+
+register_error:
+	if (rc)
+		dev_err(&pdev->dev,
+			"Cannot register watchdog device (err=%d)\n", rc);
+	return rc;
+}
+
+static struct platform_driver mlxreg_wdt_driver = {
+	.probe	= mlxreg_wdt_probe,
+	.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 v4 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>
---
v1->v4
1. Remove sentence that now doesn't correspond to code.
2. Add explanations about nowayout and start_at_boot options.
---
 Documentation/watchdog/mlx-wdt.txt | 52 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 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..434eadece41b
--- /dev/null
+++ b/Documentation/watchdog/mlx-wdt.txt
@@ -0,0 +1,52 @@
+		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.
+Watchdog can be started during a probe, in this case it will be
+pinged by watchdog core before watchdog device will be opened by
+user space application.
+Watchdog can be initialised in nowayout way, i.e. oncse started
+it can be stopped.
+
+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: nowayout and start_at_boot, hw watchdog
+version - type1 or type2.
+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 v4 1/3] platform_data/mlxreg: additions for Mellanox watchdog driver.

[Guenter Roeck]

On 2/20/19 1:34 AM, michaelsh@mellanox.com wrote:
> 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>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> 
> ---
> v1->v2
> Change feature name MLXREG_CORE_WD_FEATURE_NOSTOP_AFTER_START
> to MLXREG_CORE_WD_FEATURE_NOWAYOUT
> ---
> v2->v3
> 1. Move enum enum mlxreg_wdt_type from mlx_wdt driver to common mlxreg.h
> 2. Add version field to mlxreg_core_platform_data struct.
> ---
>   include/linux/platform_data/mlxreg.h | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
> index 19f5cb618c55..31f7c25a44da 100644
> --- a/include/linux/platform_data/mlxreg.h
> +++ b/include/linux/platform_data/mlxreg.h
> @@ -35,6 +35,19 @@
>   #define __LINUX_PLATFORM_DATA_MLXREG_H
>   
>   #define MLXREG_CORE_LABEL_MAX_SIZE	32
> +#define MLXREG_CORE_WD_FEATURE_NOWAYOUT		BIT(0)
> +#define MLXREG_CORE_WD_FEATURE_START_AT_BOOT	BIT(1)
> +
> +/**
> + * enum mlxreg_wdt_type - type of HW watchdog
> + *
> + * TYPE1 HW watchdog implementation exist in old systems.
> + * All new systems have TYPE2 HW watchdog.
> + */
> +enum mlxreg_wdt_type {
> +	MLX_WDT_TYPE1,
> +	MLX_WDT_TYPE2,
> +};
>   
>   /**
>    * struct mlxreg_hotplug_device - I2C device data:
> @@ -110,11 +123,17 @@ 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;
> + * @version: implementation version;
> + * @identity: device identity name;
>    */
>   struct mlxreg_core_platform_data {
>   	struct mlxreg_core_data *data;
>   	void *regmap;
>   	int counter;
> +	u32 features;
> +	u32 version;
> +	char identity[MLXREG_CORE_LABEL_MAX_SIZE];
>   };
>   
>   /**
> 

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

[Guenter Roeck]

On 2/20/19 1:34 AM, michaelsh@mellanox.com wrote:
> From: Michael Shych <michaelsh@mellanox.com>
> 
> Introduce watchdog driver for a various range 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>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> 
> ---
> v1->v2
> Comments pointed out by Guenter:
> 1. Remove unneeded lock in access functions.
> 2. Change return  int to void in functions with constant 0 return value
>     or with unchecked returned code.
> 3. Use regmap_update_bits() or regmap_update_bits_base() functions.
> 4. Remove unneeded dev_info messages.
> 5. Change set_timeout function.
> 6. Changes in probe
> 7. Delete unneeded remove function.
> ---
> v2->v3
> Comments pointed out by Guenter:
> 1. Remove unneedeed define.
> 2. Remove structure field hw_timeout. Use local var instead it.
> 3. Remove unnecesarry label.
> 4. Move enum mlxreg_wdt_type to common mlxreh.h file.
> 5. Watchdog HW type will be provided by parent platform driver,
>     no need to find this in mlx-wdt driver.
> ---
> v3->v4
> Comment pointed out by Guenter:
> Add cooment about return value of get_timeleft function.
> ---
>   drivers/watchdog/Kconfig   |  16 +++
>   drivers/watchdog/Makefile  |   1 +
>   drivers/watchdog/mlx_wdt.c | 290 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 307 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..70c2cbf9c993
> --- /dev/null
> +++ b/drivers/watchdog/mlx_wdt.c
> @@ -0,0 +1,290 @@
> +// 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_OPTIONS_BASE (WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | \
> +				 WDIOF_SETTIMEOUT)
> +
> +/**
> + * 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.;
> + * @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;
> + */
> +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;
> +};
> +
> +static void 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;
> +
> +	if (!(wdt->wdd.info->options & WDIOF_CARDRESET))
> +		return;
> +
> +	reg_data = &wdt->pdata->data[wdt->reset_idx];
> +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +	if (!rc) {
> +		if (regval & ~reg_data->mask) {
> +			wdt->wdd.bootstatus = WDIOF_CARDRESET;
> +			dev_info(wdt->wdd.parent,
> +				 "watchdog previously reset the CPU\n");
> +		}
> +	}
> +}
> +
> +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];
> +
> +	return regmap_update_bits(wdt->regmap, reg_data->reg, ~reg_data->mask,
> +				  BIT(reg_data->bit));
> +}
> +
> +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];
> +
> +	return regmap_update_bits(wdt->regmap, reg_data->reg, ~reg_data->mask,
> +				  ~BIT(reg_data->bit));
> +}
> +
> +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];
> +
> +	return regmap_update_bits_base(wdt->regmap, reg_data->reg,
> +				       ~reg_data->mask, BIT(reg_data->bit),
> +				       NULL, false, true);
> +}
> +
> +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, hw_timeout;
> +	int rc;
> +
> +	if (wdt->wdt_type == MLX_WDT_TYPE1) {
> +		rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +		if (rc)
> +			return rc;
> +
> +		hw_timeout = order_base_2(timeout * MLXREG_WDT_CLOCK_SCALE);
> +		regval = (regval & reg_data->mask) | hw_timeout;
> +		/* Rowndown to actual closest number of sec. */
> +		set_time = BIT(hw_timeout) / MLXREG_WDT_CLOCK_SCALE;
> +	} else {
> +		set_time = timeout;
> +		regval = timeout;
> +	}
> +
> +	wdd->timeout = set_time;
> +	rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> +
> +	if (!rc) {
> +		/*
> +		 * Restart watchdog with new timeout period
> +		 * if watchdog is already started.
> +		 */
> +		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;
> +
> +	rc = regmap_read(wdt->regmap, reg_data->reg, &regval);
> +	/* Return 0 timeleft in case of failure register read. */
> +	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 void 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 = pdata->version;
> +	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;
> +}
> +
> +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;
> +
> +	wdt->wdd.parent = &pdev->dev;
> +	wdt->regmap = pdata->regmap;
> +	mlxreg_wdt_config(wdt, pdata);
> +
> +	if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOWAYOUT))
> +		watchdog_set_nowayout(&wdt->wdd, WATCHDOG_NOWAYOUT);
> +	watchdog_stop_on_reboot(&wdt->wdd);
> +	watchdog_stop_on_unregister(&wdt->wdd);
> +	watchdog_set_drvdata(&wdt->wdd, wdt);
> +	rc = mlxreg_wdt_init_timeout(wdt, pdata);
> +	if (rc)
> +		goto register_error;
> +
> +	if ((pdata->features & MLXREG_CORE_WD_FEATURE_START_AT_BOOT)) {
> +		rc = mlxreg_wdt_start(&wdt->wdd);
> +		if (rc)
> +			goto register_error;
> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> +	}
> +	mlxreg_wdt_check_card_reset(wdt);
> +	rc = devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
> +
> +register_error:
> +	if (rc)
> +		dev_err(&pdev->dev,
> +			"Cannot register watchdog device (err=%d)\n", rc);
> +	return rc;
> +}
> +
> +static struct platform_driver mlxreg_wdt_driver = {
> +	.probe	= mlxreg_wdt_probe,
> +	.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");
> 

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

[Guenter Roeck]

On 2/20/19 1:34 AM, michaelsh@mellanox.com wrote:
> From: Michael Shych <michaelsh@mellanox.com>
> 
> Add documentation with details of Mellanox watchdog driver.
> 
> Signed-off-by: Michael Shych <michaelsh@mellanox.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> v1->v4
> 1. Remove sentence that now doesn't correspond to code.
> 2. Add explanations about nowayout and start_at_boot options.
> ---
>   Documentation/watchdog/mlx-wdt.txt | 52 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 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..434eadece41b
> --- /dev/null
> +++ b/Documentation/watchdog/mlx-wdt.txt
> @@ -0,0 +1,52 @@
> +		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.
> +Watchdog can be started during a probe, in this case it will be
> +pinged by watchdog core before watchdog device will be opened by
> +user space application.
> +Watchdog can be initialised in nowayout way, i.e. oncse started
> +it can be stopped.
> +
> +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: nowayout and start_at_boot, hw watchdog
> +version - type1 or type2.
> +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.
> 

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

[Vadim Pasternak]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Saturday, February 23, 2019 7:13 PM
> To: Michael Shych <michaelsh@mellanox.com>; wim@linux-watchdog.org;
> andy@infradead.org; dvhart@infradead.org
> Cc: linux-watchdog@vger.kernel.org; platform-driver-x86@vger.kernel.org;
> Vadim Pasternak <vadimp@mellanox.com>
> Subject: Re: [PATCH v4 2/3] watchdog: mlx-wdt: introduce a watchdog driver for
> Mellanox systems.
> 
> On 2/20/19 1:34 AM, michaelsh@mellanox.com wrote:
> > From: Michael Shych <michaelsh@mellanox.com>
> >
> > Introduce watchdog driver for a various range 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>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> >

Hi Guenter,

Thank you very much for your reviews.

How we should proceed for this patchset?

Who should take it to the next, watchdog or platform mainatainer?

Thanks,
Vadim.
 

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

[Guenter Roeck]

On 2/24/19 3:36 AM, Vadim Pasternak wrote:
> 
> 
>> -----Original Message-----
>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>> Sent: Saturday, February 23, 2019 7:13 PM
>> To: Michael Shych <michaelsh@mellanox.com>; wim@linux-watchdog.org;
>> andy@infradead.org; dvhart@infradead.org
>> Cc: linux-watchdog@vger.kernel.org; platform-driver-x86@vger.kernel.org;
>> Vadim Pasternak <vadimp@mellanox.com>
>> Subject: Re: [PATCH v4 2/3] watchdog: mlx-wdt: introduce a watchdog driver for
>> Mellanox systems.
>>
>> On 2/20/19 1:34 AM, michaelsh@mellanox.com wrote:
>>> From: Michael Shych <michaelsh@mellanox.com>
>>>
>>> Introduce watchdog driver for a various range 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>
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>
>>>
> 
> Hi Guenter,
> 
> Thank you very much for your reviews.
> 
> How we should proceed for this patchset?
> 
> Who should take it to the next, watchdog or platform mainatainer?
> 
Good question. For now I added it to my watchdog-next branch. But then strictly
speaking (and as far as I know) we don't have an Ack from the platform maintainer,
so maybe I should not have done that. Maybe we should wait for the platform part
to be merged by the platform maintainer. Thoughts, anyone ?

Guenter

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

[Michael Shych]

> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
> Roeck
> Sent: Saturday, February 23, 2019 7:13 PM
> To: Michael Shych <michaelsh@mellanox.com>; wim@linux-watchdog.org;
> andy@infradead.org; dvhart@infradead.org
> Cc: linux-watchdog@vger.kernel.org; platform-driver-x86@vger.kernel.org;
> Vadim Pasternak <vadimp@mellanox.com>
> Subject: Re: [PATCH v4 3/3] Documentation/watchdog: Add documentation
> mlx-wdt driver
> 
> On 2/20/19 1:34 AM, michaelsh@mellanox.com wrote:
> > From: Michael Shych <michaelsh@mellanox.com>
> >
> > Add documentation with details of Mellanox watchdog driver.
> >
> > Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>

Hi Guenter,

Thank you for your reviews.
Sorry, I looked today over the document and noted one typo 
that I made in patchset V4.
It's, however, changed explanation of nowayout.
In this explanation must be can't instead incorrect can.
I'll resend just this document as patch V5.

Thank you,
   Michael.
 
> > ---
> > v1->v4
> > 1. Remove sentence that now doesn't correspond to code.
> > 2. Add explanations about nowayout and start_at_boot options.
> > ---
> >   Documentation/watchdog/mlx-wdt.txt | 52
> ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 52 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..434eadece41b
> > --- /dev/null
> > +++ b/Documentation/watchdog/mlx-wdt.txt
> > @@ -0,0 +1,52 @@
> > +		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.
> > +Watchdog can be started during a probe, in this case it will be
> > +pinged by watchdog core before watchdog device will be opened by
> > +user space application.
> > +Watchdog can be initialised in nowayout way, i.e. oncse started
> > +it can be stopped.
> > +
> > +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: nowayout and start_at_boot, hw watchdog
> > +version - type1 or type2.
> > +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.
> >

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

[Vadim Pasternak]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Sunday, February 24, 2019 6:39 PM
> To: Vadim Pasternak <vadimp@mellanox.com>; Michael Shych
> <michaelsh@mellanox.com>; wim@linux-watchdog.org; andy@infradead.org;
> dvhart@infradead.org
> Cc: linux-watchdog@vger.kernel.org; platform-driver-x86@vger.kernel.org
> Subject: Re: [PATCH v4 2/3] watchdog: mlx-wdt: introduce a watchdog driver for
> Mellanox systems.
> 
> On 2/24/19 3:36 AM, Vadim Pasternak wrote:
> >
> >
> >> -----Original Message-----
> >> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> >> Sent: Saturday, February 23, 2019 7:13 PM
> >> To: Michael Shych <michaelsh@mellanox.com>; wim@linux-watchdog.org;
> >> andy@infradead.org; dvhart@infradead.org
> >> Cc: linux-watchdog@vger.kernel.org;
> >> platform-driver-x86@vger.kernel.org;
> >> Vadim Pasternak <vadimp@mellanox.com>
> >> Subject: Re: [PATCH v4 2/3] watchdog: mlx-wdt: introduce a watchdog
> >> driver for Mellanox systems.
> >>
> >> On 2/20/19 1:34 AM, michaelsh@mellanox.com wrote:
> >>> From: Michael Shych <michaelsh@mellanox.com>
> >>>
> >>> Introduce watchdog driver for a various range 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>
> >>
> >> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >>
> >>>
> >
> > Hi Guenter,
> >
> > Thank you very much for your reviews.
> >
> > How we should proceed for this patchset?
> >
> > Who should take it to the next, watchdog or platform mainatainer?
> >
> Good question. For now I added it to my watchdog-next branch. But then strictly
> speaking (and as far as I know) we don't have an Ack from the platform
> maintainer, so maybe I should not have done that. Maybe we should wait for
> the platform part to be merged by the platform maintainer. Thoughts, anyone ?
> 

Right.

Darren, Andy,
In case you are OK with the changes in mlxreg.h from
[PATCH v4 1/3] platform_data/mlxreg: additions for Mellanox watchdog driver.
from this patchset, could you please ack?

Thanks,
Vadim.

> Guenter

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

[Darren Hart]

On Sun, Feb 24, 2019 at 06:01:46PM +0000, Vadim Pasternak wrote:
...
> > > Who should take it to the next, watchdog or platform mainatainer?
> > >
> > Good question. For now I added it to my watchdog-next branch. But then strictly
> > speaking (and as far as I know) we don't have an Ack from the platform
> > maintainer, so maybe I should not have done that. Maybe we should wait for
> > the platform part to be merged by the platform maintainer. Thoughts, anyone ?
> > 
> 
> Right.
> 
> Darren, Andy,
> In case you are OK with the changes in mlxreg.h from
> [PATCH v4 1/3] platform_data/mlxreg: additions for Mellanox watchdog driver.
> from this patchset, could you please ack?

I don't have any problem with the change as proposed. We do appear to
have a conflict in next now though. So we either have to provide merge
instructions to Linus, or rebase the wdt next branch and have us pull it
in.

I'd prefer to leave things alone, keep the series together in the wdt
next tree, and just provide Linus with an explanation of the merge
conflict (standard operating procedure).

So, a bit late, but for the record, for the mlxreg.h changes:

Acked-by: Darren Hart (VMware) <dvhart@infradead.org>


-- 
Darren Hart
VMware Open Source Technology Center