* [PATCH v2 1/4] platform_data/mlxreg: support new watchdog type with longer timeout period
2020-04-28 13:08 [PATCH v2 0/4] support watchdog with longer timeout period michaelsh
@ 2020-04-28 13:08 ` michaelsh
2020-04-28 13:08 ` [PATCH v2 2/4] platform/x86: mlx-platform: support new watchdog type with longer timeout michaelsh
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: michaelsh @ 2020-04-28 13:08 UTC (permalink / raw)
To: linux, wim, andy, dvhart
Cc: linux-watchdog, platform-driver-x86, vadimp, Michael Shych
From: Michael Shych <michaelsh@mellanox.com>
Add new watchdog type 3 with longer timeout period.
Extend size of health_cntr field that that can be used to init watchdog
timeout period.
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Reviewed-by: Vadim Pasternak <vadimp@mellanox.com>
---
include/linux/platform_data/mlxreg.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
index b8da8aef2446..2c5e58d1d77b 100644
--- a/include/linux/platform_data/mlxreg.h
+++ b/include/linux/platform_data/mlxreg.h
@@ -43,10 +43,13 @@
*
* TYPE1 HW watchdog implementation exist in old systems.
* All new systems have TYPE2 HW watchdog.
+ * TYPE3 HW watchdog can exist on all systems with new CPLD.
+ * TYPE3 is selected by WD capability bit.
*/
enum mlxreg_wdt_type {
MLX_WDT_TYPE1,
MLX_WDT_TYPE2,
+ MLX_WDT_TYPE3,
};
/**
@@ -90,7 +93,7 @@ struct mlxreg_core_data {
umode_t mode;
struct device_node *np;
struct mlxreg_hotplug_device hpdev;
- u8 health_cntr;
+ u32 health_cntr;
bool attached;
};
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] platform/x86: mlx-platform: support new watchdog type with longer timeout
2020-04-28 13:08 [PATCH v2 0/4] support watchdog with longer timeout period michaelsh
2020-04-28 13:08 ` [PATCH v2 1/4] platform_data/mlxreg: support new watchdog type " michaelsh
@ 2020-04-28 13:08 ` michaelsh
2020-04-28 13:08 ` [PATCH v2 3/4] watchdog: mlx-wdt: support new watchdog type with longer timeout period michaelsh
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: michaelsh @ 2020-04-28 13:08 UTC (permalink / raw)
To: linux, wim, andy, dvhart
Cc: linux-watchdog, platform-driver-x86, vadimp, Michael Shych
From: Michael Shych <michaelsh@mellanox.com>
Add verification of WD capability in order to distinguish between
the existing WD types and new type, implemented in CPLD.
Add configuration for a new WD type.
Change access mode for watchdog registers.
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Reviewed-by: Vadim Pasternak <vadimp@mellanox.com>
---
drivers/platform/x86/mlx-platform.c | 106 ++++++++++++++++++++++++++++++++++++
1 file changed, 106 insertions(+)
diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index c27548fd386a..9d3371cd58d5 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -178,7 +178,9 @@
#define MLXPLAT_CPLD_WD_RESET_ACT_MASK GENMASK(7, 1)
#define MLXPLAT_CPLD_WD_FAN_ACT_MASK (GENMASK(7, 0) & ~BIT(4))
#define MLXPLAT_CPLD_WD_COUNT_ACT_MASK (GENMASK(7, 0) & ~BIT(7))
+#define MLXPLAT_CPLD_WD_CPBLTY_MASK (GENMASK(7, 0) & ~BIT(6))
#define MLXPLAT_CPLD_WD_DFLT_TIMEOUT 30
+#define MLXPLAT_CPLD_WD3_DFLT_TIMEOUT 600
#define MLXPLAT_CPLD_WD_MAX_DEVS 2
/* mlxplat_priv - platform private data
@@ -1959,6 +1961,84 @@ static struct mlxreg_core_platform_data mlxplat_mlxcpld_wd_set_type2[] = {
},
};
+/* Watchdog type3: hardware implementation version 3
+ * Can be on all systems. It's differentiated by WD capability bit.
+ * Old systems (MSN2700, MSN2410, MSN2740, MSN2100 and MSN2140)
+ * still have only one main watchdog.
+ */
+static struct mlxreg_core_data mlxplat_mlxcpld_wd_main_regs_type3[] = {
+ {
+ .label = "action",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD2_ACT_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_RESET_ACT_MASK,
+ .bit = 0,
+ },
+ {
+ .label = "timeout",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD2_TMR_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_TYPE2_TO_MASK,
+ .health_cntr = MLXPLAT_CPLD_WD3_DFLT_TIMEOUT,
+ },
+ {
+ .label = "timeleft",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD2_TMR_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_TYPE2_TO_MASK,
+ },
+ {
+ .label = "ping",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD2_ACT_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_RESET_ACT_MASK,
+ .bit = 0,
+ },
+ {
+ .label = "reset",
+ .reg = MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET,
+ .mask = GENMASK(7, 0) & ~BIT(6),
+ .bit = 6,
+ },
+};
+
+static struct mlxreg_core_data mlxplat_mlxcpld_wd_aux_regs_type3[] = {
+ {
+ .label = "action",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD3_ACT_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_FAN_ACT_MASK,
+ .bit = 4,
+ },
+ {
+ .label = "timeout",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD3_TMR_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_TYPE2_TO_MASK,
+ .health_cntr = MLXPLAT_CPLD_WD3_DFLT_TIMEOUT,
+ },
+ {
+ .label = "timeleft",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD3_TMR_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_TYPE2_TO_MASK,
+ },
+ {
+ .label = "ping",
+ .reg = MLXPLAT_CPLD_LPC_REG_WD3_ACT_OFFSET,
+ .mask = MLXPLAT_CPLD_WD_FAN_ACT_MASK,
+ .bit = 4,
+ },
+};
+
+static struct mlxreg_core_platform_data mlxplat_mlxcpld_wd_set_type3[] = {
+ {
+ .data = mlxplat_mlxcpld_wd_main_regs_type3,
+ .counter = ARRAY_SIZE(mlxplat_mlxcpld_wd_main_regs_type3),
+ .version = MLX_WDT_TYPE3,
+ .identity = "mlx-wdt-main",
+ },
+ {
+ .data = mlxplat_mlxcpld_wd_aux_regs_type3,
+ .counter = ARRAY_SIZE(mlxplat_mlxcpld_wd_aux_regs_type3),
+ .version = MLX_WDT_TYPE3,
+ .identity = "mlx-wdt-aux",
+ },
+};
+
static bool mlxplat_mlxcpld_writeable_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
@@ -1989,8 +2069,10 @@ static bool mlxplat_mlxcpld_writeable_reg(struct device *dev, unsigned int reg)
case MLXPLAT_CPLD_LPC_REG_WD1_TMR_OFFSET:
case MLXPLAT_CPLD_LPC_REG_WD1_ACT_OFFSET:
case MLXPLAT_CPLD_LPC_REG_WD2_TMR_OFFSET:
+ case MLXPLAT_CPLD_LPC_REG_WD2_TLEFT_OFFSET:
case MLXPLAT_CPLD_LPC_REG_WD2_ACT_OFFSET:
case MLXPLAT_CPLD_LPC_REG_WD3_TMR_OFFSET:
+ case MLXPLAT_CPLD_LPC_REG_WD3_TLEFT_OFFSET:
case MLXPLAT_CPLD_LPC_REG_WD3_ACT_OFFSET:
case MLXPLAT_CPLD_LPC_REG_PWM1_OFFSET:
case MLXPLAT_CPLD_LPC_REG_PWM_CONTROL_OFFSET:
@@ -2601,6 +2683,27 @@ static int mlxplat_mlxcpld_verify_bus_topology(int *nr)
return 0;
}
+static int mlxplat_mlxcpld_check_wd_capability(void *regmap)
+{
+ u32 regval;
+ int i, rc;
+
+ rc = regmap_read(regmap, MLXPLAT_CPLD_LPC_REG_PSU_I2C_CAP_OFFSET,
+ ®val);
+ if (rc)
+ return rc;
+
+ if (!(regval & ~MLXPLAT_CPLD_WD_CPBLTY_MASK)) {
+ for (i = 0; i < ARRAY_SIZE(mlxplat_mlxcpld_wd_set_type3); i++) {
+ if (mlxplat_wd_data[i])
+ mlxplat_wd_data[i] =
+ &mlxplat_mlxcpld_wd_set_type3[i];
+ }
+ }
+
+ return 0;
+}
+
static int __init mlxplat_init(void)
{
struct mlxplat_priv *priv;
@@ -2733,6 +2836,9 @@ static int __init mlxplat_init(void)
}
/* Add WD drivers. */
+ err = mlxplat_mlxcpld_check_wd_capability(priv->regmap);
+ if (err)
+ goto fail_platform_wd_register;
for (j = 0; j < MLXPLAT_CPLD_WD_MAX_DEVS; j++) {
if (mlxplat_wd_data[j]) {
mlxplat_wd_data[j]->regmap = priv->regmap;
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/4] watchdog: mlx-wdt: support new watchdog type with longer timeout period
2020-04-28 13:08 [PATCH v2 0/4] support watchdog with longer timeout period michaelsh
2020-04-28 13:08 ` [PATCH v2 1/4] platform_data/mlxreg: support new watchdog type " michaelsh
2020-04-28 13:08 ` [PATCH v2 2/4] platform/x86: mlx-platform: support new watchdog type with longer timeout michaelsh
@ 2020-04-28 13:08 ` michaelsh
2020-04-29 12:29 ` Guenter Roeck
2020-04-28 13:08 ` [PATCH v2 4/4] docs: watchdog: mlx-wdt: Add description of new watchdog type 3 michaelsh
2020-04-28 15:36 ` [PATCH v2 0/4] support watchdog with longer timeout period Andy Shevchenko
4 siblings, 1 reply; 8+ messages in thread
From: michaelsh @ 2020-04-28 13:08 UTC (permalink / raw)
To: linux, wim, andy, dvhart
Cc: linux-watchdog, platform-driver-x86, vadimp, Michael Shych
From: Michael Shych <michaelsh@mellanox.com>
New programmable logic device can have watchdog type 3 implementation.
It's same as Type 2 with extended maximum timeout period.
Maximum timeout is up-to 65535 sec.
Type 3 HW watchdog implementation can exist on all Mellanox systems.
It is differentiated by WD capability bit.
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Reviewed-by: Vadim Pasternak <vadimp@mellanox.com>
---
v1-v2:
Make changes pointed out by Guenter:
-Simplify bit operations
-Consistency in registers access
-Don't check irrelevant return code
---
drivers/watchdog/mlx_wdt.c | 75 +++++++++++++++++++++++++++++++++++++++-------
1 file changed, 64 insertions(+), 11 deletions(-)
diff --git a/drivers/watchdog/mlx_wdt.c b/drivers/watchdog/mlx_wdt.c
index 03b9ac4b99af..4e76d4a1af5c 100644
--- a/drivers/watchdog/mlx_wdt.c
+++ b/drivers/watchdog/mlx_wdt.c
@@ -21,6 +21,7 @@
#define MLXREG_WDT_CLOCK_SCALE 1000
#define MLXREG_WDT_MAX_TIMEOUT_TYPE1 32
#define MLXREG_WDT_MAX_TIMEOUT_TYPE2 255
+#define MLXREG_WDT_MAX_TIMEOUT_TYPE3 65535
#define MLXREG_WDT_MIN_TIMEOUT 1
#define MLXREG_WDT_OPTIONS_BASE (WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | \
WDIOF_SETTIMEOUT)
@@ -49,6 +50,7 @@ struct mlxreg_wdt {
int tleft_idx;
int ping_idx;
int reset_idx;
+ int regmap_val_sz;
enum mlxreg_wdt_type wdt_type;
};
@@ -111,7 +113,8 @@ static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
u32 regval, set_time, hw_timeout;
int rc;
- if (wdt->wdt_type == MLX_WDT_TYPE1) {
+ switch (wdt->wdt_type) {
+ case MLX_WDT_TYPE1:
rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
if (rc)
return rc;
@@ -120,14 +123,33 @@ static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
regval = (regval & reg_data->mask) | hw_timeout;
/* Rowndown to actual closest number of sec. */
set_time = BIT(hw_timeout) / MLXREG_WDT_CLOCK_SCALE;
- } else {
+ rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+ break;
+ case MLX_WDT_TYPE2:
+ set_time = timeout;
+ rc = regmap_write(wdt->regmap, reg_data->reg, timeout);
+ break;
+ case MLX_WDT_TYPE3:
+ /* WD_TYPE3 has 2B set time register */
set_time = timeout;
- regval = timeout;
+ if (wdt->regmap_val_sz == 1) {
+ timeout = cpu_to_le16(timeout);
+ regval = timeout & 0xff;
+ rc = regmap_write(wdt->regmap, reg_data->reg, regval);
+ if (!rc) {
+ regval = (timeout & 0xff00) >> 8;
+ rc = regmap_write(wdt->regmap,
+ reg_data->reg + 1, regval);
+ }
+ } else {
+ rc = regmap_write(wdt->regmap, reg_data->reg, timeout);
+ }
+ break;
+ default:
+ return -EINVAL;
}
wdd->timeout = set_time;
- rc = regmap_write(wdt->regmap, reg_data->reg, regval);
-
if (!rc) {
/*
* Restart watchdog with new timeout period
@@ -147,10 +169,26 @@ 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;
+ u32 regval, msb, lsb;
int rc;
- rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
+ if (wdt->wdt_type == MLX_WDT_TYPE2) {
+ rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
+ } else {
+ /* WD_TYPE3 has 2 byte timeleft register */
+ if (wdt->regmap_val_sz == 1) {
+ rc = regmap_read(wdt->regmap, reg_data->reg, &lsb);
+ if (!rc) {
+ rc = regmap_read(wdt->regmap,
+ reg_data->reg + 1, &msb);
+ regval = (msb & 0xff) << 8 | (lsb & 0xff);
+ regval = le16_to_cpu(regval);
+ }
+ } else {
+ rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
+ }
+ }
+
/* Return 0 timeleft in case of failure register read. */
return rc == 0 ? regval : 0;
}
@@ -212,13 +250,23 @@ static void mlxreg_wdt_config(struct mlxreg_wdt *wdt,
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 {
+ switch (wdt->wdt_type) {
+ case MLX_WDT_TYPE1:
wdt->wdd.ops = &mlxreg_wdt_ops_type1;
wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE1;
+ break;
+ case MLX_WDT_TYPE2:
+ wdt->wdd.ops = &mlxreg_wdt_ops_type2;
+ wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE2;
+ break;
+ case MLX_WDT_TYPE3:
+ wdt->wdd.ops = &mlxreg_wdt_ops_type2;
+ wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE3;
+ break;
+ default:
+ break;
}
+
wdt->wdd.min_timeout = MLXREG_WDT_MIN_TIMEOUT;
}
@@ -249,6 +297,11 @@ static int mlxreg_wdt_probe(struct platform_device *pdev)
wdt->wdd.parent = dev;
wdt->regmap = pdata->regmap;
+ rc = regmap_get_val_bytes(wdt->regmap);
+ if (rc < 0)
+ return -EINVAL;
+
+ wdt->regmap_val_sz = rc;
mlxreg_wdt_config(wdt, pdata);
if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOWAYOUT))
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/4] watchdog: mlx-wdt: support new watchdog type with longer timeout period
2020-04-28 13:08 ` [PATCH v2 3/4] watchdog: mlx-wdt: support new watchdog type with longer timeout period michaelsh
@ 2020-04-29 12:29 ` Guenter Roeck
2020-05-03 5:32 ` Michael Shych
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2020-04-29 12:29 UTC (permalink / raw)
To: michaelsh, wim, andy, dvhart; +Cc: linux-watchdog, platform-driver-x86, vadimp
On 4/28/20 6:08 AM, michaelsh@mellanox.com wrote:
> From: Michael Shych <michaelsh@mellanox.com>
>
> New programmable logic device can have watchdog type 3 implementation.
> It's same as Type 2 with extended maximum timeout period.
> Maximum timeout is up-to 65535 sec.
> Type 3 HW watchdog implementation can exist on all Mellanox systems.
> It is differentiated by WD capability bit.
>
> Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> Reviewed-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
> v1-v2:
> Make changes pointed out by Guenter:
> -Simplify bit operations
> -Consistency in registers access
> -Don't check irrelevant return code
> ---
> drivers/watchdog/mlx_wdt.c | 75 +++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 64 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/watchdog/mlx_wdt.c b/drivers/watchdog/mlx_wdt.c
> index 03b9ac4b99af..4e76d4a1af5c 100644
> --- a/drivers/watchdog/mlx_wdt.c
> +++ b/drivers/watchdog/mlx_wdt.c
> @@ -21,6 +21,7 @@
> #define MLXREG_WDT_CLOCK_SCALE 1000
> #define MLXREG_WDT_MAX_TIMEOUT_TYPE1 32
> #define MLXREG_WDT_MAX_TIMEOUT_TYPE2 255
> +#define MLXREG_WDT_MAX_TIMEOUT_TYPE3 65535
> #define MLXREG_WDT_MIN_TIMEOUT 1
> #define MLXREG_WDT_OPTIONS_BASE (WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | \
> WDIOF_SETTIMEOUT)
> @@ -49,6 +50,7 @@ struct mlxreg_wdt {
> int tleft_idx;
> int ping_idx;
> int reset_idx;
> + int regmap_val_sz;
> enum mlxreg_wdt_type wdt_type;
> };
>
> @@ -111,7 +113,8 @@ static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
> u32 regval, set_time, hw_timeout;
> int rc;
>
> - if (wdt->wdt_type == MLX_WDT_TYPE1) {
> + switch (wdt->wdt_type) {
> + case MLX_WDT_TYPE1:
> rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
> if (rc)
> return rc;
> @@ -120,14 +123,33 @@ static int mlxreg_wdt_set_timeout(struct watchdog_device *wdd,
> regval = (regval & reg_data->mask) | hw_timeout;
> /* Rowndown to actual closest number of sec. */
> set_time = BIT(hw_timeout) / MLXREG_WDT_CLOCK_SCALE;
> - } else {
> + rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> + break;
> + case MLX_WDT_TYPE2:
> + set_time = timeout;
> + rc = regmap_write(wdt->regmap, reg_data->reg, timeout);
> + break;
> + case MLX_WDT_TYPE3:
> + /* WD_TYPE3 has 2B set time register */
> set_time = timeout;
> - regval = timeout;
> + if (wdt->regmap_val_sz == 1) {
> + timeout = cpu_to_le16(timeout);
> + regval = timeout & 0xff;
> + rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> + if (!rc) {
> + regval = (timeout & 0xff00) >> 8;
> + rc = regmap_write(wdt->regmap,
> + reg_data->reg + 1, regval);
> + }
> + } else {
> + rc = regmap_write(wdt->regmap, reg_data->reg, timeout);
> + }
> + break;
> + default:
> + return -EINVAL;
> }
>
> wdd->timeout = set_time;
> - rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> -
> if (!rc) {
> /*
> * Restart watchdog with new timeout period
> @@ -147,10 +169,26 @@ 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;
> + u32 regval, msb, lsb;
> int rc;
>
> - rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
> + if (wdt->wdt_type == MLX_WDT_TYPE2) {
> + rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
> + } else {
> + /* WD_TYPE3 has 2 byte timeleft register */
> + if (wdt->regmap_val_sz == 1) {
> + rc = regmap_read(wdt->regmap, reg_data->reg, &lsb);
> + if (!rc) {
> + rc = regmap_read(wdt->regmap,
> + reg_data->reg + 1, &msb);
> + regval = (msb & 0xff) << 8 | (lsb & 0xff);
> + regval = le16_to_cpu(regval);
I don't think my concerns regarding the use of le16_to_cpu() have been
addressed, and I still think this results in bad data on a big endian
system.
Guenter
> + }
> + } else {
> + rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
> + }
> + }
> +
> /* Return 0 timeleft in case of failure register read. */
> return rc == 0 ? regval : 0;
> }
> @@ -212,13 +250,23 @@ static void mlxreg_wdt_config(struct mlxreg_wdt *wdt,
> 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 {
> + switch (wdt->wdt_type) {
> + case MLX_WDT_TYPE1:
> wdt->wdd.ops = &mlxreg_wdt_ops_type1;
> wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE1;
> + break;
> + case MLX_WDT_TYPE2:
> + wdt->wdd.ops = &mlxreg_wdt_ops_type2;
> + wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE2;
> + break;
> + case MLX_WDT_TYPE3:
> + wdt->wdd.ops = &mlxreg_wdt_ops_type2;
> + wdt->wdd.max_timeout = MLXREG_WDT_MAX_TIMEOUT_TYPE3;
> + break;
> + default:
> + break;
> }
> +
> wdt->wdd.min_timeout = MLXREG_WDT_MIN_TIMEOUT;
> }
>
> @@ -249,6 +297,11 @@ static int mlxreg_wdt_probe(struct platform_device *pdev)
>
> wdt->wdd.parent = dev;
> wdt->regmap = pdata->regmap;
> + rc = regmap_get_val_bytes(wdt->regmap);
> + if (rc < 0)
> + return -EINVAL;
> +
> + wdt->regmap_val_sz = rc;
> mlxreg_wdt_config(wdt, pdata);
>
> if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOWAYOUT))
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 3/4] watchdog: mlx-wdt: support new watchdog type with longer timeout period
2020-04-29 12:29 ` Guenter Roeck
@ 2020-05-03 5:32 ` Michael Shych
0 siblings, 0 replies; 8+ messages in thread
From: Michael Shych @ 2020-05-03 5:32 UTC (permalink / raw)
To: Guenter Roeck, wim, andy, dvhart
Cc: linux-watchdog, platform-driver-x86, Vadim Pasternak
> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Wednesday, April 29, 2020 3:30 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 v2 3/4] watchdog: mlx-wdt: support new watchdog type
> with longer timeout period
>
> On 4/28/20 6:08 AM, michaelsh@mellanox.com wrote:
> > From: Michael Shych <michaelsh@mellanox.com>
> >
> > New programmable logic device can have watchdog type 3 implementation.
> > It's same as Type 2 with extended maximum timeout period.
> > Maximum timeout is up-to 65535 sec.
> > Type 3 HW watchdog implementation can exist on all Mellanox systems.
> > It is differentiated by WD capability bit.
> >
> > Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> > Reviewed-by: Vadim Pasternak <vadimp@mellanox.com>
> > ---
> > v1-v2:
> > Make changes pointed out by Guenter:
> > -Simplify bit operations
> > -Consistency in registers access
> > -Don't check irrelevant return code
> > ---
> > drivers/watchdog/mlx_wdt.c | 75
> +++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 64 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/watchdog/mlx_wdt.c b/drivers/watchdog/mlx_wdt.c
> > index 03b9ac4b99af..4e76d4a1af5c 100644
> > --- a/drivers/watchdog/mlx_wdt.c
> > +++ b/drivers/watchdog/mlx_wdt.c
> > @@ -21,6 +21,7 @@
> > #define MLXREG_WDT_CLOCK_SCALE 1000
> > #define MLXREG_WDT_MAX_TIMEOUT_TYPE1 32
> > #define MLXREG_WDT_MAX_TIMEOUT_TYPE2 255
> > +#define MLXREG_WDT_MAX_TIMEOUT_TYPE3 65535
> > #define MLXREG_WDT_MIN_TIMEOUT 1
> > #define MLXREG_WDT_OPTIONS_BASE (WDIOF_KEEPALIVEPING |
> WDIOF_MAGICCLOSE | \
> > WDIOF_SETTIMEOUT)
> > @@ -49,6 +50,7 @@ struct mlxreg_wdt {
> > int tleft_idx;
> > int ping_idx;
> > int reset_idx;
> > + int regmap_val_sz;
> > enum mlxreg_wdt_type wdt_type;
> > };
> >
> > @@ -111,7 +113,8 @@ static int mlxreg_wdt_set_timeout(struct
> watchdog_device *wdd,
> > u32 regval, set_time, hw_timeout;
> > int rc;
> >
> > - if (wdt->wdt_type == MLX_WDT_TYPE1) {
> > + switch (wdt->wdt_type) {
> > + case MLX_WDT_TYPE1:
> > rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
> > if (rc)
> > return rc;
> > @@ -120,14 +123,33 @@ static int mlxreg_wdt_set_timeout(struct
> watchdog_device *wdd,
> > regval = (regval & reg_data->mask) | hw_timeout;
> > /* Rowndown to actual closest number of sec. */
> > set_time = BIT(hw_timeout) / MLXREG_WDT_CLOCK_SCALE;
> > - } else {
> > + rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> > + break;
> > + case MLX_WDT_TYPE2:
> > + set_time = timeout;
> > + rc = regmap_write(wdt->regmap, reg_data->reg, timeout);
> > + break;
> > + case MLX_WDT_TYPE3:
> > + /* WD_TYPE3 has 2B set time register */
> > set_time = timeout;
> > - regval = timeout;
> > + if (wdt->regmap_val_sz == 1) {
> > + timeout = cpu_to_le16(timeout);
> > + regval = timeout & 0xff;
> > + rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> > + if (!rc) {
> > + regval = (timeout & 0xff00) >> 8;
> > + rc = regmap_write(wdt->regmap,
> > + reg_data->reg + 1, regval);
> > + }
> > + } else {
> > + rc = regmap_write(wdt->regmap, reg_data->reg,
> timeout);
> > + }
> > + break;
> > + default:
> > + return -EINVAL;
> > }
> >
> > wdd->timeout = set_time;
> > - rc = regmap_write(wdt->regmap, reg_data->reg, regval);
> > -
> > if (!rc) {
> > /*
> > * Restart watchdog with new timeout period
> > @@ -147,10 +169,26 @@ 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;
> > + u32 regval, msb, lsb;
> > int rc;
> >
> > - rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
> > + if (wdt->wdt_type == MLX_WDT_TYPE2) {
> > + rc = regmap_read(wdt->regmap, reg_data->reg, ®val);
> > + } else {
> > + /* WD_TYPE3 has 2 byte timeleft register */
> > + if (wdt->regmap_val_sz == 1) {
> > + rc = regmap_read(wdt->regmap, reg_data->reg, &lsb);
> > + if (!rc) {
> > + rc = regmap_read(wdt->regmap,
> > + reg_data->reg + 1, &msb);
> > + regval = (msb & 0xff) << 8 | (lsb & 0xff);
> > + regval = le16_to_cpu(regval);
>
> I don't think my concerns regarding the use of le16_to_cpu() have been
> addressed, and I still think this results in bad data on a big endian
> system.
>
> Guenter
You are right. These are unnecessary conversions. I removed them as well as reference
To them in the document. Resending patchset v3.
>
> > + }
> > + } else {
> > + rc = regmap_read(wdt->regmap, reg_data->reg,
> ®val);
> > + }
> > + }
> > +
> > /* Return 0 timeleft in case of failure register read. */
> > return rc == 0 ? regval : 0;
> > }
> > @@ -212,13 +250,23 @@ static void mlxreg_wdt_config(struct mlxreg_wdt
> *wdt,
> > 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 {
> > + switch (wdt->wdt_type) {
> > + case MLX_WDT_TYPE1:
> > wdt->wdd.ops = &mlxreg_wdt_ops_type1;
> > wdt->wdd.max_timeout =
> MLXREG_WDT_MAX_TIMEOUT_TYPE1;
> > + break;
> > + case MLX_WDT_TYPE2:
> > + wdt->wdd.ops = &mlxreg_wdt_ops_type2;
> > + wdt->wdd.max_timeout =
> MLXREG_WDT_MAX_TIMEOUT_TYPE2;
> > + break;
> > + case MLX_WDT_TYPE3:
> > + wdt->wdd.ops = &mlxreg_wdt_ops_type2;
> > + wdt->wdd.max_timeout =
> MLXREG_WDT_MAX_TIMEOUT_TYPE3;
> > + break;
> > + default:
> > + break;
> > }
> > +
> > wdt->wdd.min_timeout = MLXREG_WDT_MIN_TIMEOUT;
> > }
> >
> > @@ -249,6 +297,11 @@ static int mlxreg_wdt_probe(struct platform_device
> *pdev)
> >
> > wdt->wdd.parent = dev;
> > wdt->regmap = pdata->regmap;
> > + rc = regmap_get_val_bytes(wdt->regmap);
> > + if (rc < 0)
> > + return -EINVAL;
> > +
> > + wdt->regmap_val_sz = rc;
> > mlxreg_wdt_config(wdt, pdata);
> >
> > if ((pdata->features & MLXREG_CORE_WD_FEATURE_NOWAYOUT))
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 4/4] docs: watchdog: mlx-wdt: Add description of new watchdog type 3
2020-04-28 13:08 [PATCH v2 0/4] support watchdog with longer timeout period michaelsh
` (2 preceding siblings ...)
2020-04-28 13:08 ` [PATCH v2 3/4] watchdog: mlx-wdt: support new watchdog type with longer timeout period michaelsh
@ 2020-04-28 13:08 ` michaelsh
2020-04-28 15:36 ` [PATCH v2 0/4] support watchdog with longer timeout period Andy Shevchenko
4 siblings, 0 replies; 8+ messages in thread
From: michaelsh @ 2020-04-28 13:08 UTC (permalink / raw)
To: linux, wim, andy, dvhart
Cc: linux-watchdog, platform-driver-x86, vadimp, Michael Shych
From: Michael Shych <michaelsh@mellanox.com>
Add documentation with details of new type of Mellanox watchdog driver.
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Reviewed-by: Vadim Pasternak <vadimp@mellanox.com>
---
v1-v2:
Add explanation about device registers order
---
Documentation/watchdog/mlx-wdt.rst | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/watchdog/mlx-wdt.rst b/Documentation/watchdog/mlx-wdt.rst
index bf5bafac47f0..241768b885a5 100644
--- a/Documentation/watchdog/mlx-wdt.rst
+++ b/Documentation/watchdog/mlx-wdt.rst
@@ -24,10 +24,19 @@ Type 2:
Maximum timeout is 255 sec.
Get time-left is supported.
+Type 3:
+ Same as Type 2 with extended maximum timeout period.
+ Maximum timeout is 65535 sec.
+
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.
+Type 3 HW watchdog implementation can exist on all Mellanox systems
+with new programmer logic device.
+It's differentiated by WD capability bit.
+Old systems still have only one main watchdog.
+
Mellanox system can have 2 watchdogs: main and auxiliary.
Main and auxiliary watchdog devices can be enabled together
on the same system.
@@ -54,3 +63,6 @@ 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.
+Programmable logic device registers have little-endian order.
+Watchdog type 3, 2-byte width fields should be converted from LE to CPU order
+and vice versa.
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/4] support watchdog with longer timeout period
2020-04-28 13:08 [PATCH v2 0/4] support watchdog with longer timeout period michaelsh
` (3 preceding siblings ...)
2020-04-28 13:08 ` [PATCH v2 4/4] docs: watchdog: mlx-wdt: Add description of new watchdog type 3 michaelsh
@ 2020-04-28 15:36 ` Andy Shevchenko
4 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2020-04-28 15:36 UTC (permalink / raw)
To: michaelsh
Cc: Guenter Roeck, Wim Van Sebroeck, Andy Shevchenko, Darren Hart,
linux-watchdog, Platform Driver, Vadim Pasternak
On Tue, Apr 28, 2020 at 4:09 PM <michaelsh@mellanox.com> wrote:
>
> From: Michael Shych <michaelsh@mellanox.com>
>
> This patchset adds support of extended new watchdog type 3 of Mellanox
> Ethernet and Infiniband switch systems.
> This type of watchdog can have a timeout period longer than 255 or 32 sec.
> as it was before.
>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
for PDx86 bits.
> Michael Shych (4):
> platform_data/mlxreg: support new watchdog type with longer timeout
> period
> platform/x86: mlx-platform: support new watchdog type with longer
> timeout
> watchdog: mlx-wdt: support new watchdog type with longer timeout
> period
> docs: watchdog: mlx-wdt: Add description of new watchdog type 3
>
> Documentation/watchdog/mlx-wdt.rst | 12 ++++
> drivers/platform/x86/mlx-platform.c | 106 ++++++++++++++++++++++++++++++
> drivers/watchdog/mlx_wdt.c | 75 +++++++++++++++++++++----
> include/linux/platform_data/mlxreg.h | 5 +-
> 4 files changed, 186 insertions(+), 12 deletions(-)
>
> --
> 2.11.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread