Changes for v2
1) Patch 0001: Simplify initialization of pretimeout removing #ifdef.
2) Patch 0002: Loosen check on mynmi to accommodate potential FW issue.
3) Patch 0003: Split dev_info into mulitple calls as output was getting long.
4) Patch 0004: New: Add alias to module parameter soft_margin.
-----------------------------
Two defect fixes and one enhancement.
0001: Initialize pretimeout from module parameter.
Bug Fix.
Pretimeout was getting programmed correctly in the hardware,
but this issue caused it to not be displayed correctly
in sysfs.
0002: Claim NMI from iLO
Bug Fix.
Default configuration worked, but if user were to disable the
pretimeout for the watchdog, then an explicit request to NMI
the system from the iLO virutal button would fail.
0003: Display module parameters
Enhancement.
Display all the module parameters when loading the driver.
Makes it easier to diagnose problems.
Jerry
Jerry Hoemann (5):
watchdog: hpwdt: Initialize pretimeout from module parameter.
watchdog: hpwdt: Claim NMI from iLO
watchdog: hpwdt: Display module parameters.
watchdog: hpwdt: Module paramerter alias.
watchdog: hpwdt: Update version number.
drivers/watchdog/hpwdt.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
--
1.8.3.1
Add module parameter "timeout" as an alias to "soft_margin."
This aligns hpwdt usage more closely with other WDT while
retaining backwards compatibility.
Signed-off-by: Jerry Hoemann <[email protected]>
---
drivers/watchdog/hpwdt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 69a88b1..eb947bc 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -367,6 +367,9 @@ static void hpwdt_exit(struct pci_dev *dev)
module_param(soft_margin, int, 0);
MODULE_PARM_DESC(soft_margin, "Watchdog timeout in seconds");
+module_param_named(timeout, soft_margin, int, 0);
+MODULE_PARM_DESC(timeout, "Alias of soft_margin");
+
module_param(nowayout, bool, 0);
MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
--
1.8.3.1
Print module parameters when the driver is loaded.
Signed-off-by: Jerry Hoemann <[email protected]>
---
drivers/watchdog/hpwdt.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index bb41714..69a88b1 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -320,9 +320,12 @@ static int hpwdt_init_one(struct pci_dev *dev,
goto error_wd_register;
}
- dev_info(&dev->dev, "HPE Watchdog Timer Driver: %s"
- ", timer margin: %d seconds (nowayout=%d).\n",
- HPWDT_VERSION, hpwdt_dev.timeout, nowayout);
+ dev_info(&dev->dev, "HPE Watchdog Timer Driver: Version: %s\n",
+ HPWDT_VERSION);
+ dev_info(&dev->dev, "timeout: %d seconds (nowayout=%d)\n",
+ hpwdt_dev.timeout, nowayout);
+ dev_info(&dev->dev, "pretimeout: %s.\n",
+ pretimeout ? "on" : "off");
if (dev->subsystem_vendor == PCI_VENDOR_ID_HP_3PAR)
ilo5 = true;
--
1.8.3.1
When the pretimeout is specified as a module parameter, the
value should be reflected in hpwdt_dev.pretimeout. The default
(on) case is correct. But, when disabling pretimeout, the value
should be set to zero in hpwdt_dev.
When compiling w/o CONFIG_HPWDT_NMI_DECODING defined, the pretimeout
module parameter is ignored and the value internally will be 0.
Signed-off-by: Jerry Hoemann <[email protected]>
---
drivers/watchdog/hpwdt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 9dc62a4..fae9364 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -205,9 +205,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
.min_timeout = 1,
.max_timeout = HPWDT_MAX_TIMER,
.timeout = DEFAULT_MARGIN,
-#ifdef CONFIG_HPWDT_NMI_DECODING
.pretimeout = PRETIMEOUT_SEC,
-#endif
};
@@ -313,6 +311,8 @@ static int hpwdt_init_one(struct pci_dev *dev,
if (watchdog_init_timeout(&hpwdt_dev, soft_margin, NULL))
dev_warn(&dev->dev, "Invalid soft_margin: %d.\n", soft_margin);
+ hpwdt_dev.pretimeout = pretimeout ? PRETIMEOUT_SEC : 0;
+
hpwdt_dev.parent = &dev->dev;
retval = watchdog_register_device(&hpwdt_dev);
if (retval < 0) {
--
1.8.3.1
The hwpdt driver is overloaded for handling both the iLO
watchdog and the explicit "Generate NMI to System" virutal
button. Hence NMI handler needs to claim NMI resulting
from the virutal button.
Claim if iLO generated accommodating firmware that might
set wrong bit.
Signed-off-by: Jerry Hoemann <[email protected]>
---
drivers/watchdog/hpwdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index fae9364..bb41714 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -162,7 +162,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
if (ilo5 && ulReason == NMI_UNKNOWN && !mynmi)
return NMI_DONE;
- if (ilo5 && !pretimeout)
+ if (ilo5 && !pretimeout && !mynmi)
return NMI_DONE;
hpwdt_stop();
--
1.8.3.1
Bump version number to reflect recent bug fixes.
Signed-off-by: Jerry Hoemann <[email protected]>
---
drivers/watchdog/hpwdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index eb947bc..7af358b 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -26,7 +26,7 @@
#include <linux/watchdog.h>
#include <asm/nmi.h>
-#define HPWDT_VERSION "2.0.0"
+#define HPWDT_VERSION "2.0.1"
#define SECS_TO_TICKS(secs) ((secs) * 1000 / 128)
#define TICKS_TO_SECS(ticks) ((ticks) * 128 / 1000)
#define HPWDT_MAX_TIMER TICKS_TO_SECS(65535)
--
1.8.3.1
On 08/08/2018 12:13 PM, Jerry Hoemann wrote:
> The hwpdt driver is overloaded for handling both the iLO
> watchdog and the explicit "Generate NMI to System" virutal
> button. Hence NMI handler needs to claim NMI resulting
> from the virutal button.
>
> Claim if iLO generated accommodating firmware that might
> set wrong bit.
>
> Signed-off-by: Jerry Hoemann <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/hpwdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
> index fae9364..bb41714 100644
> --- a/drivers/watchdog/hpwdt.c
> +++ b/drivers/watchdog/hpwdt.c
> @@ -162,7 +162,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
> if (ilo5 && ulReason == NMI_UNKNOWN && !mynmi)
> return NMI_DONE;
>
> - if (ilo5 && !pretimeout)
> + if (ilo5 && !pretimeout && !mynmi)
> return NMI_DONE;
>
> hpwdt_stop();
>
On 08/08/2018 12:13 PM, Jerry Hoemann wrote:
> Print module parameters when the driver is loaded.
>
> Signed-off-by: Jerry Hoemann <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/hpwdt.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
> index bb41714..69a88b1 100644
> --- a/drivers/watchdog/hpwdt.c
> +++ b/drivers/watchdog/hpwdt.c
> @@ -320,9 +320,12 @@ static int hpwdt_init_one(struct pci_dev *dev,
> goto error_wd_register;
> }
>
> - dev_info(&dev->dev, "HPE Watchdog Timer Driver: %s"
> - ", timer margin: %d seconds (nowayout=%d).\n",
> - HPWDT_VERSION, hpwdt_dev.timeout, nowayout);
> + dev_info(&dev->dev, "HPE Watchdog Timer Driver: Version: %s\n",
> + HPWDT_VERSION);
> + dev_info(&dev->dev, "timeout: %d seconds (nowayout=%d)\n",
> + hpwdt_dev.timeout, nowayout);
> + dev_info(&dev->dev, "pretimeout: %s.\n",
> + pretimeout ? "on" : "off");
>
> if (dev->subsystem_vendor == PCI_VENDOR_ID_HP_3PAR)
> ilo5 = true;
>
On 08/08/2018 12:13 PM, Jerry Hoemann wrote:
> Add module parameter "timeout" as an alias to "soft_margin."
> This aligns hpwdt usage more closely with other WDT while
> retaining backwards compatibility.
>
> Signed-off-by: Jerry Hoemann <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/hpwdt.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
> index 69a88b1..eb947bc 100644
> --- a/drivers/watchdog/hpwdt.c
> +++ b/drivers/watchdog/hpwdt.c
> @@ -367,6 +367,9 @@ static void hpwdt_exit(struct pci_dev *dev)
> module_param(soft_margin, int, 0);
> MODULE_PARM_DESC(soft_margin, "Watchdog timeout in seconds");
>
> +module_param_named(timeout, soft_margin, int, 0);
> +MODULE_PARM_DESC(timeout, "Alias of soft_margin");
> +
> module_param(nowayout, bool, 0);
> MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>
On 08/08/2018 12:13 PM, Jerry Hoemann wrote:
> Bump version number to reflect recent bug fixes.
>
> Signed-off-by: Jerry Hoemann <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/hpwdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
> index eb947bc..7af358b 100644
> --- a/drivers/watchdog/hpwdt.c
> +++ b/drivers/watchdog/hpwdt.c
> @@ -26,7 +26,7 @@
> #include <linux/watchdog.h>
> #include <asm/nmi.h>
>
> -#define HPWDT_VERSION "2.0.0"
> +#define HPWDT_VERSION "2.0.1"
> #define SECS_TO_TICKS(secs) ((secs) * 1000 / 128)
> #define TICKS_TO_SECS(ticks) ((ticks) * 128 / 1000)
> #define HPWDT_MAX_TIMER TICKS_TO_SECS(65535)
>
On 08/08/2018 12:13 PM, Jerry Hoemann wrote:
> When the pretimeout is specified as a module parameter, the
> value should be reflected in hpwdt_dev.pretimeout. The default
> (on) case is correct. But, when disabling pretimeout, the value
> should be set to zero in hpwdt_dev.
>
> When compiling w/o CONFIG_HPWDT_NMI_DECODING defined, the pretimeout
> module parameter is ignored and the value internally will be 0.
>
> Signed-off-by: Jerry Hoemann <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/hpwdt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
> index 9dc62a4..fae9364 100644
> --- a/drivers/watchdog/hpwdt.c
> +++ b/drivers/watchdog/hpwdt.c
> @@ -205,9 +205,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
> .min_timeout = 1,
> .max_timeout = HPWDT_MAX_TIMER,
> .timeout = DEFAULT_MARGIN,
> -#ifdef CONFIG_HPWDT_NMI_DECODING
> .pretimeout = PRETIMEOUT_SEC,
> -#endif
> };
>
>
> @@ -313,6 +311,8 @@ static int hpwdt_init_one(struct pci_dev *dev,
> if (watchdog_init_timeout(&hpwdt_dev, soft_margin, NULL))
> dev_warn(&dev->dev, "Invalid soft_margin: %d.\n", soft_margin);
>
> + hpwdt_dev.pretimeout = pretimeout ? PRETIMEOUT_SEC : 0;
> +
> hpwdt_dev.parent = &dev->dev;
> retval = watchdog_register_device(&hpwdt_dev);
> if (retval < 0) {
>
On Wed, Aug 08, 2018 at 01:13:22PM -0600, Jerry Hoemann wrote:
> Changes for v2
>
> 1) Patch 0001: Simplify initialization of pretimeout removing #ifdef.
> 2) Patch 0002: Loosen check on mynmi to accommodate potential FW issue.
> 3) Patch 0003: Split dev_info into mulitple calls as output was getting long.
> 4) Patch 0004: New: Add alias to module parameter soft_margin.
>
Wim,
I would like to get our Distro partners to back port these changes,
but they must first be upstream. :)
Do you know when you will include these in a pull request so that I can
pass on to them when to schedule the work?
Thanks
The patch e-mails are at:
https://lkml.org/lkml/2018/8/8/769
Jerry
>
> -----------------------------
>
> Two defect fixes and one enhancement.
>
> 0001: Initialize pretimeout from module parameter.
>
> Bug Fix.
>
> Pretimeout was getting programmed correctly in the hardware,
> but this issue caused it to not be displayed correctly
> in sysfs.
>
>
>
> 0002: Claim NMI from iLO
>
> Bug Fix.
>
> Default configuration worked, but if user were to disable the
> pretimeout for the watchdog, then an explicit request to NMI
> the system from the iLO virutal button would fail.
>
>
> 0003: Display module parameters
>
> Enhancement.
>
> Display all the module parameters when loading the driver.
> Makes it easier to diagnose problems.
>
>
> Jerry
>
>
> Jerry Hoemann (5):
> watchdog: hpwdt: Initialize pretimeout from module parameter.
> watchdog: hpwdt: Claim NMI from iLO
> watchdog: hpwdt: Display module parameters.
> watchdog: hpwdt: Module paramerter alias.
> watchdog: hpwdt: Update version number.
>
> drivers/watchdog/hpwdt.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> --
> 1.8.3.1
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------