* [PATCH 01/15] hpwdt_pretimeout reorganization
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 02/15] remove unnecessary includes dann frazier
` (15 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
Reorganize this function to remove excess indentation and highlight
the single return code.
No functional change.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 36 +++++++++++++++++++-----------------
1 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index dbe650e..64ea6fc 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -476,24 +476,26 @@ static int hpwdt_pretimeout(struct notifier_block *nb, unsigned long ulReason,
static int die_nmi_called;
if (ulReason != DIE_NMI && ulReason != DIE_NMI_IPI)
- return NOTIFY_OK;
-
- if (hpwdt_nmi_sourcing) {
- spin_lock_irqsave(&rom_lock, rom_pl);
- if (!die_nmi_called)
- asminline_call(&cmn_regs, cru_rom_addr);
- die_nmi_called = 1;
- spin_unlock_irqrestore(&rom_lock, rom_pl);
- if (cmn_regs.u1.ral == 0) {
- printk(KERN_WARNING "hpwdt: An NMI occurred, "
- "but unable to determine source.\n");
- } else {
- if (allow_kdump)
- hpwdt_stop();
- panic("An NMI occurred, please see the Integrated "
- "Management Log for details.\n");
- }
+ goto out;
+
+ if (!hpwdt_nmi_sourcing)
+ goto out;
+
+ spin_lock_irqsave(&rom_lock, rom_pl);
+ if (!die_nmi_called)
+ asminline_call(&cmn_regs, cru_rom_addr);
+ die_nmi_called = 1;
+ spin_unlock_irqrestore(&rom_lock, rom_pl);
+ if (cmn_regs.u1.ral == 0) {
+ printk(KERN_WARNING "hpwdt: An NMI occurred, "
+ "but unable to determine source.\n");
+ } else {
+ if (allow_kdump)
+ hpwdt_stop();
+ panic("An NMI occurred, please see the Integrated "
+ "Management Log for details.\n");
}
+out:
return NOTIFY_OK;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 02/15] remove unnecessary includes
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
2010-07-27 23:50 ` [PATCH 01/15] hpwdt_pretimeout reorganization dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 03/15] include spinlock.h dann frazier
` (14 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 10 ----------
1 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 64ea6fc..fc81122 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -16,30 +16,20 @@
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/init.h>
-#include <linux/interrupt.h>
#include <linux/io.h>
-#include <linux/irq.h>
#include <linux/nmi.h>
#include <linux/kernel.h>
#include <linux/miscdevice.h>
-#include <linux/mm.h>
#include <linux/module.h>
#include <linux/kdebug.h>
#include <linux/moduleparam.h>
#include <linux/notifier.h>
#include <linux/pci.h>
#include <linux/pci_ids.h>
-#include <linux/reboot.h>
-#include <linux/sched.h>
-#include <linux/timer.h>
#include <linux/types.h>
#include <linux/uaccess.h>
#include <linux/watchdog.h>
#include <linux/dmi.h>
-#include <linux/efi.h>
-#include <linux/string.h>
-#include <linux/bootmem.h>
-#include <asm/desc.h>
#include <asm/cacheflush.h>
#define PCI_BIOS32_SD_VALUE 0x5F32335F /* "_32_" */
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 03/15] include spinlock.h
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
2010-07-27 23:50 ` [PATCH 01/15] hpwdt_pretimeout reorganization dann frazier
2010-07-27 23:50 ` [PATCH 02/15] remove unnecessary includes dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 04/15] add include for linux/bitops.h dann frazier
` (13 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
We use a spinlock, but lacked the include
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index fc81122..48bb536 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -30,6 +30,7 @@
#include <linux/uaccess.h>
#include <linux/watchdog.h>
#include <linux/dmi.h>
+#include <linux/spinlock.h>
#include <asm/cacheflush.h>
#define PCI_BIOS32_SD_VALUE 0x5F32335F /* "_32_" */
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 04/15] add include for linux/bitops.h
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (2 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 03/15] include spinlock.h dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 05/15] Group together includes specific to NMI sourcing dann frazier
` (12 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
Needed for test_and_set_bit/clear_bit
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 48bb536..391bb43 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -18,6 +18,7 @@
#include <linux/init.h>
#include <linux/io.h>
#include <linux/nmi.h>
+#include <linux/bitops.h>
#include <linux/kernel.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 05/15] Group together includes specific to NMI sourcing
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (3 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 04/15] add include for linux/bitops.h dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 06/15] Group options that affect watchdog behavior together dann frazier
` (11 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
No functional change.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 391bb43..b8b791f 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -17,14 +17,11 @@
#include <linux/fs.h>
#include <linux/init.h>
#include <linux/io.h>
-#include <linux/nmi.h>
#include <linux/bitops.h>
#include <linux/kernel.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
-#include <linux/kdebug.h>
#include <linux/moduleparam.h>
-#include <linux/notifier.h>
#include <linux/pci.h>
#include <linux/pci_ids.h>
#include <linux/types.h>
@@ -32,6 +29,9 @@
#include <linux/watchdog.h>
#include <linux/dmi.h>
#include <linux/spinlock.h>
+#include <linux/nmi.h>
+#include <linux/kdebug.h>
+#include <linux/notifier.h>
#include <asm/cacheflush.h>
#define PCI_BIOS32_SD_VALUE 0x5F32335F /* "_32_" */
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 06/15] Group options that affect watchdog behavior together
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (4 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 05/15] Group together includes specific to NMI sourcing dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 07/15] Group defines only used by NMI sourcing together dann frazier
` (10 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
Reorganization only.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index b8b791f..796fcb8 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -808,13 +808,13 @@ MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
module_param(soft_margin, int, 0);
MODULE_PARM_DESC(soft_margin, "Watchdog timeout in seconds");
-module_param(allow_kdump, int, 0);
-MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
-
module_param(nowayout, int, 0);
MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+module_param(allow_kdump, int, 0);
+MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
+
module_param(priority, int, 0);
MODULE_PARM_DESC(priority, "The hpwdt driver handles NMIs first or last"
" (default = 0/Last)\n");
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 07/15] Group defines only used by NMI sourcing together
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (5 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 06/15] Group options that affect watchdog behavior together dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 08/15] Group declarations specific to " dann frazier
` (9 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
No functional change.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 796fcb8..f337c22 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -34,16 +34,16 @@
#include <linux/notifier.h>
#include <asm/cacheflush.h>
+#define SECS_TO_TICKS(secs) ((secs) * 1000 / 128)
+#define TICKS_TO_SECS(ticks) ((ticks) * 128 / 1000)
+#define HPWDT_MAX_TIMER TICKS_TO_SECS(65535)
+#define HPWDT_VERSION "1.1.1"
+
#define PCI_BIOS32_SD_VALUE 0x5F32335F /* "_32_" */
#define CRU_BIOS_SIGNATURE_VALUE 0x55524324
#define PCI_BIOS32_PARAGRAPH_LEN 16
#define PCI_ROM_BASE1 0x000F0000
#define ROM_SIZE 0x10000
-#define HPWDT_VERSION "1.1.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)
struct bios32_service_dir {
u32 signature;
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 08/15] Group declarations specific to NMI sourcing together
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (6 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 07/15] Group defines only used by NMI sourcing together dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 09/15] Despecificate driver from iLO2 dann frazier
` (8 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
No functional change.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index f337c22..cedcc61 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -115,19 +115,11 @@ static int nowayout = WATCHDOG_NOWAYOUT;
static char expect_release;
static unsigned long hpwdt_is_open;
static unsigned int allow_kdump;
-static unsigned int hpwdt_nmi_sourcing;
-static unsigned int priority; /* hpwdt at end of die_notify list */
static void __iomem *pci_mem_addr; /* the PCI-memory address */
static unsigned long __iomem *hpwdt_timer_reg;
static unsigned long __iomem *hpwdt_timer_con;
-static DEFINE_SPINLOCK(rom_lock);
-
-static void *cru_rom_addr;
-
-static struct cmn_registers cmn_regs;
-
static struct pci_device_id hpwdt_devices[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_COMPAQ, 0xB203) },
{ PCI_DEVICE(PCI_VENDOR_ID_HP, 0x3306) },
@@ -135,6 +127,12 @@ static struct pci_device_id hpwdt_devices[] = {
};
MODULE_DEVICE_TABLE(pci, hpwdt_devices);
+static unsigned int hpwdt_nmi_sourcing;
+static unsigned int priority; /* hpwdt at end of die_notify list */
+static DEFINE_SPINLOCK(rom_lock);
+static void *cru_rom_addr;
+static struct cmn_registers cmn_regs;
+
extern asmlinkage void asminline_call(struct cmn_registers *pi86Regs,
unsigned long *pRomEntry);
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 09/15] Despecificate driver from iLO2
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (7 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 08/15] Group declarations specific to " dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-08-04 20:13 ` Wim Van Sebroeck
2010-07-27 23:50 ` [PATCH 10/15] Fix a typo dann frazier
` (7 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
This driver supports both iLO2 and iLO3, but our user-visible strings
currently only reference iLO2. Let's just call it "iLO" to avoid having
to update strings for each iLO generation. This driver doesn't support
iLO ASICs prior to iLO2, but that is sufficiently explained in Kconfig.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/Kconfig | 2 +-
drivers/watchdog/hpwdt.c | 14 +++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index afcfacc..cde31f9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -556,7 +556,7 @@ config IT87_WDT
be called it87_wdt.
config HP_WATCHDOG
- tristate "HP Proliant iLO 2 Hardware Watchdog Timer"
+ tristate "HP Proliant iLO Hardware Watchdog Timer"
depends on X86
help
A software monitoring watchdog and NMI sourcing driver. This driver
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index cedcc61..a6b37fe 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -121,8 +121,8 @@ static unsigned long __iomem *hpwdt_timer_reg;
static unsigned long __iomem *hpwdt_timer_con;
static struct pci_device_id hpwdt_devices[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_COMPAQ, 0xB203) },
- { PCI_DEVICE(PCI_VENDOR_ID_HP, 0x3306) },
+ { PCI_DEVICE(PCI_VENDOR_ID_COMPAQ, 0xB203) }, /* iLO2 */
+ { PCI_DEVICE(PCI_VENDOR_ID_HP, 0x3306) }, /* iLO3 */
{0}, /* terminate list */
};
MODULE_DEVICE_TABLE(pci, hpwdt_devices);
@@ -557,7 +557,7 @@ static const struct watchdog_info ident = {
.options = WDIOF_SETTIMEOUT |
WDIOF_KEEPALIVEPING |
WDIOF_MAGICCLOSE,
- .identity = "HP iLO2 HW Watchdog Timer",
+ .identity = "HP iLO HW Watchdog Timer",
};
static long hpwdt_ioctl(struct file *file, unsigned int cmd,
@@ -667,13 +667,13 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
hpwdt_check_nmi_sourcing(dev);
/*
- * First let's find out if we are on an iLO2 server. We will
+ * First let's find out if we are on an iLO2+ server. We will
* not run on a legacy ASM box.
* So we only support the G5 ProLiant servers and higher.
*/
if (dev->subsystem_vendor != PCI_VENDOR_ID_HP) {
- dev_warn(&dev->dev,
- "This server does not have an iLO2 ASIC.\n");
+ dev_warn(&dev->dev, "This server does not have an iLO ASIC"
+ " version 2 or greater.\n");
return -ENODEV;
}
@@ -687,7 +687,7 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
pci_mem_addr = pci_iomap(dev, 1, 0x80);
if (!pci_mem_addr) {
dev_warn(&dev->dev,
- "Unable to detect the iLO2 server memory.\n");
+ "Unable to detect the iLO server memory.\n");
retval = -ENOMEM;
goto error_pci_iomap;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 09/15] Despecificate driver from iLO2
2010-07-27 23:50 ` [PATCH 09/15] Despecificate driver from iLO2 dann frazier
@ 2010-08-04 20:13 ` Wim Van Sebroeck
2010-08-04 20:18 ` Mingarelli, Thomas
2010-08-05 5:10 ` dann frazier
0 siblings, 2 replies; 26+ messages in thread
From: Wim Van Sebroeck @ 2010-08-04 20:13 UTC (permalink / raw)
To: dann frazier; +Cc: linux-kernel, Thomas Mingarelli
Hi Dan,
> This driver supports both iLO2 and iLO3, but our user-visible strings
> currently only reference iLO2. Let's just call it "iLO" to avoid having
> to update strings for each iLO generation. This driver doesn't support
> iLO ASICs prior to iLO2, but that is sufficiently explained in Kconfig.
Just a thought: Wouldn't it be more consistent if it's called iLO2+ everywhere?
> - tristate "HP Proliant iLO 2 Hardware Watchdog Timer"
> + tristate "HP Proliant iLO Hardware Watchdog Timer"
would be: tristate "HP Proliant iLO2+ Hardware Watchdog Timer"
> - .identity = "HP iLO2 HW Watchdog Timer",
> + .identity = "HP iLO HW Watchdog Timer",
and: .identity = "HP iLO2+ HW Watchdog Timer",
> - * First let's find out if we are on an iLO2 server. We will
> + * First let's find out if we are on an iLO2+ server. We will
> - dev_warn(&dev->dev,
> - "This server does not have an iLO2 ASIC.\n");
> + dev_warn(&dev->dev, "This server does not have an iLO ASIC"
> + " version 2 or greater.\n");
dev_warn(&dev->dev,
"This server does not have an iLO2+ ASIC.\n");
> - "Unable to detect the iLO2 server memory.\n");
> + "Unable to detect the iLO server memory.\n");
"Unable to detect the iLO2+ server memory.\n");
For the rest I'm still reviewing the code.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 09/15] Despecificate driver from iLO2
2010-08-04 20:13 ` Wim Van Sebroeck
@ 2010-08-04 20:18 ` Mingarelli, Thomas
2010-08-05 5:10 ` dann frazier
1 sibling, 0 replies; 26+ messages in thread
From: Mingarelli, Thomas @ 2010-08-04 20:18 UTC (permalink / raw)
To: Wim Van Sebroeck, Frazier, Daniel Kent (ProLiant Linux)
Cc: linux-kernel@vger.kernel.org
I like that suggestion Wim. Sure would make future iLO releases easier to include.
Tom
> -----Original Message-----
> From: Wim Van Sebroeck [mailto:wim@iguana.be]
> Sent: Wednesday, August 04, 2010 3:14 PM
> To: Frazier, Daniel Kent (ProLiant Linux)
> Cc: linux-kernel@vger.kernel.org; Mingarelli, Thomas
> Subject: Re: [PATCH 09/15] Despecificate driver from iLO2
>
> Hi Dan,
>
> > This driver supports both iLO2 and iLO3, but our user-visible strings
> > currently only reference iLO2. Let's just call it "iLO" to avoid having
> > to update strings for each iLO generation. This driver doesn't support
> > iLO ASICs prior to iLO2, but that is sufficiently explained in Kconfig.
>
> Just a thought: Wouldn't it be more consistent if it's called iLO2+
> everywhere?
>
> > - tristate "HP Proliant iLO 2 Hardware Watchdog Timer"
> > + tristate "HP Proliant iLO Hardware Watchdog Timer"
>
> would be: tristate "HP Proliant iLO2+ Hardware Watchdog Timer"
>
> > - .identity = "HP iLO2 HW Watchdog Timer",
> > + .identity = "HP iLO HW Watchdog Timer",
>
> and: .identity = "HP iLO2+ HW Watchdog Timer",
>
> > - * First let's find out if we are on an iLO2 server. We will
> > + * First let's find out if we are on an iLO2+ server. We will
>
> > - dev_warn(&dev->dev,
> > - "This server does not have an iLO2 ASIC.\n");
> > + dev_warn(&dev->dev, "This server does not have an iLO ASIC"
> > + " version 2 or greater.\n");
>
> dev_warn(&dev->dev,
> "This server does not have an iLO2+ ASIC.\n");
>
> > - "Unable to detect the iLO2 server memory.\n");
> > + "Unable to detect the iLO server memory.\n");
>
> "Unable to detect the iLO2+ server memory.\n");
>
> For the rest I'm still reviewing the code.
>
> Kind regards,
> Wim.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/15] Despecificate driver from iLO2
2010-08-04 20:13 ` Wim Van Sebroeck
2010-08-04 20:18 ` Mingarelli, Thomas
@ 2010-08-05 5:10 ` dann frazier
1 sibling, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-08-05 5:10 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli
On Wed, Aug 04, 2010 at 10:13:41PM +0200, Wim Van Sebroeck wrote:
> Hi Dan,
>
> > This driver supports both iLO2 and iLO3, but our user-visible strings
> > currently only reference iLO2. Let's just call it "iLO" to avoid having
> > to update strings for each iLO generation. This driver doesn't support
> > iLO ASICs prior to iLO2, but that is sufficiently explained in Kconfig.
>
> Just a thought: Wouldn't it be more consistent if it's called iLO2+
> everywhere?
works for me :)
>
> > - tristate "HP Proliant iLO 2 Hardware Watchdog Timer"
> > + tristate "HP Proliant iLO Hardware Watchdog Timer"
>
> would be: tristate "HP Proliant iLO2+ Hardware Watchdog Timer"
>
> > - .identity = "HP iLO2 HW Watchdog Timer",
> > + .identity = "HP iLO HW Watchdog Timer",
>
> and: .identity = "HP iLO2+ HW Watchdog Timer",
>
> > - * First let's find out if we are on an iLO2 server. We will
> > + * First let's find out if we are on an iLO2+ server. We will
>
> > - dev_warn(&dev->dev,
> > - "This server does not have an iLO2 ASIC.\n");
> > + dev_warn(&dev->dev, "This server does not have an iLO ASIC"
> > + " version 2 or greater.\n");
>
> dev_warn(&dev->dev,
> "This server does not have an iLO2+ ASIC.\n");
>
> > - "Unable to detect the iLO2 server memory.\n");
> > + "Unable to detect the iLO server memory.\n");
>
> "Unable to detect the iLO2+ server memory.\n");
>
> For the rest I'm still reviewing the code.
Thanks - I'll make this change in my local git tree for now and
resubmit after you've completed the review.
--
dann frazier | ProLiant Linux
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 10/15] Fix a typo
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (8 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 09/15] Despecificate driver from iLO2 dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:50 ` [PATCH 11/15] Make x86 assembly ifdef guard more strict dann frazier
` (6 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index cde31f9..fc3d6b5 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -560,7 +560,7 @@ config HP_WATCHDOG
depends on X86
help
A software monitoring watchdog and NMI sourcing driver. This driver
- will detect lockups and provide stack trace. Also, when an NMI
+ will detect lockups and provide a stack trace. Also, when an NMI
occurs this driver will make the necessary BIOS calls to log
the cause of the NMI. This is a driver that will only load on a
HP ProLiant system with a minimum of iLO2 support.
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 11/15] Make x86 assembly ifdef guard more strict
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (9 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 10/15] Fix a typo dann frazier
@ 2010-07-27 23:50 ` dann frazier
2010-07-27 23:51 ` [PATCH 12/15] Construct status message w/ kasprintf and emit it with dev_info dann frazier
` (5 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:50 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
The 32-bit assembly is guarded by an #ifndef CONFIG_X86_64. Kconfig prevents
us from building this driver on !X86, so that happens to suffice - but we
should really lock it down to #ifdef CONFIG_X86_32.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index a6b37fe..f46e4f2 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -136,7 +136,7 @@ static struct cmn_registers cmn_regs;
extern asmlinkage void asminline_call(struct cmn_registers *pi86Regs,
unsigned long *pRomEntry);
-#ifndef CONFIG_X86_64
+#ifdef CONFIG_X86_32
/* --32 Bit Bios------------------------------------------------------------ */
#define HPWDT_ARCH 32
@@ -325,8 +325,9 @@ static int __devinit detect_cru_service(void)
iounmap(p);
return rc;
}
-
-#else
+/* ------------------------------------------------------------------------- */
+#endif /* CONFIG_X86_32 */
+#ifdef CONFIG_X86_64
/* --64 Bit Bios------------------------------------------------------------ */
#define HPWDT_ARCH 64
@@ -404,10 +405,7 @@ static int __devinit detect_cru_service(void)
/* if cru_rom_addr has been set then we found a CRU service */
return ((cru_rom_addr != NULL) ? 0 : -ENODEV);
}
-
-/* ------------------------------------------------------------------------- */
-
-#endif
+#endif /* CONFIG_X86_64 */
/*
* Watchdog operations
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 12/15] Construct status message w/ kasprintf and emit it with dev_info
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (10 preceding siblings ...)
2010-07-27 23:50 ` [PATCH 11/15] Make x86 assembly ifdef guard more strict dann frazier
@ 2010-07-27 23:51 ` dann frazier
2010-07-28 18:38 ` dann frazier
2010-07-27 23:51 ` [PATCH 13/15] Use "decoding" instead of "sourcing" dann frazier
` (4 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:51 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
Constructing the string dynamically w/ kasprintf will make it easier to
ifdef-out parts of it. Also, this is for a device, so let's use dev_info.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index f46e4f2..482785e 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -658,6 +658,7 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
const struct pci_device_id *ent)
{
int retval;
+ char *buf;
/*
* Check if we can do NMI sourcing or not
@@ -741,14 +742,16 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
goto error_misc_register;
}
- printk(KERN_INFO
- "hp Watchdog Timer Driver: %s"
- ", timer margin: %d seconds (nowayout=%d)"
- ", allow kernel dump: %s (default = 0/OFF)"
- ", priority: %s (default = 0/LAST).\n",
- HPWDT_VERSION, soft_margin, nowayout,
- (allow_kdump == 0) ? "OFF" : "ON",
- (priority == 0) ? "LAST" : "FIRST");
+ buf = kasprintf(GFP_KERNEL, "hp Watchdog Timer Driver: %s"
+ ", timer margin: %d seconds (nowayout=%d)"
+ ", allow kernel dump: %s (default = 0/OFF)"
+ ", priority: %s (default = 0/LAST)",
+ HPWDT_VERSION, soft_margin, nowayout,
+ (allow_kdump == 0) ? "OFF" : "ON",
+ (priority == 0) ? "LAST" : "FIRST");
+ if (!buf)
+ goto error_nomem;
+ dev_info(&dev->dev, "%s.\n", buf);
return 0;
@@ -761,6 +764,7 @@ error_get_cru:
pci_iounmap(dev, pci_mem_addr);
error_pci_iomap:
pci_disable_device(dev);
+error_nomem:
return retval;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 12/15] Construct status message w/ kasprintf and emit it with dev_info
2010-07-27 23:51 ` [PATCH 12/15] Construct status message w/ kasprintf and emit it with dev_info dann frazier
@ 2010-07-28 18:38 ` dann frazier
0 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-28 18:38 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli
On Tue, Jul 27, 2010 at 05:51:00PM -0600, dann frazier wrote:
> Constructing the string dynamically w/ kasprintf will make it easier to
> ifdef-out parts of it. Also, this is for a device, so let's use
> dev_info.
A coworker caught a silly mistake in this one - I missed
freeing the buf allocated by kasprintf.
Here's a fixed patch.
Construct status message w/ kasprintf and emit it with dev_info
Constructing the string dynamically w/ kasprintf will make it easier to
ifdef-out parts of it. Also, this is for a device, so let's use dev_info.
Signed-off-by: dann frazier <dannf@hp.com>
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index f46e4f2..f03078d 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -658,6 +658,7 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
const struct pci_device_id *ent)
{
int retval;
+ char *buf;
/*
* Check if we can do NMI sourcing or not
@@ -741,14 +742,17 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
goto error_misc_register;
}
- printk(KERN_INFO
- "hp Watchdog Timer Driver: %s"
- ", timer margin: %d seconds (nowayout=%d)"
- ", allow kernel dump: %s (default = 0/OFF)"
- ", priority: %s (default = 0/LAST).\n",
- HPWDT_VERSION, soft_margin, nowayout,
- (allow_kdump == 0) ? "OFF" : "ON",
- (priority == 0) ? "LAST" : "FIRST");
+ buf = kasprintf(GFP_KERNEL, "hp Watchdog Timer Driver: %s"
+ ", timer margin: %d seconds (nowayout=%d)"
+ ", allow kernel dump: %s (default = 0/OFF)"
+ ", priority: %s (default = 0/LAST)",
+ HPWDT_VERSION, soft_margin, nowayout,
+ (allow_kdump == 0) ? "OFF" : "ON",
+ (priority == 0) ? "LAST" : "FIRST");
+ if (!buf)
+ goto error_nomem;
+ dev_info(&dev->dev, "%s.\n", buf);
+ kfree(buf);
return 0;
@@ -761,6 +765,7 @@ error_get_cru:
pci_iounmap(dev, pci_mem_addr);
error_pci_iomap:
pci_disable_device(dev);
+error_nomem:
return retval;
}
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 13/15] Use "decoding" instead of "sourcing"
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (11 preceding siblings ...)
2010-07-27 23:51 ` [PATCH 12/15] Construct status message w/ kasprintf and emit it with dev_info dann frazier
@ 2010-07-27 23:51 ` dann frazier
2010-07-27 23:51 ` [PATCH 14/15] Make NMI decoding a compile-time option dann frazier
` (3 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:51 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
The term "decoding" more clearly explains what hpwdt is doing. It isn't
just finding the source of the interrupt, but rather aids in decoding what
the interrupt means.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 482785e..00aea69 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -127,7 +127,7 @@ static struct pci_device_id hpwdt_devices[] = {
};
MODULE_DEVICE_TABLE(pci, hpwdt_devices);
-static unsigned int hpwdt_nmi_sourcing;
+static unsigned int hpwdt_nmi_decoding;
static unsigned int priority; /* hpwdt at end of die_notify list */
static DEFINE_SPINLOCK(rom_lock);
static void *cru_rom_addr;
@@ -466,7 +466,7 @@ static int hpwdt_pretimeout(struct notifier_block *nb, unsigned long ulReason,
if (ulReason != DIE_NMI && ulReason != DIE_NMI_IPI)
goto out;
- if (!hpwdt_nmi_sourcing)
+ if (!hpwdt_nmi_decoding)
goto out;
spin_lock_irqsave(&rom_lock, rom_pl);
@@ -633,23 +633,23 @@ static struct notifier_block die_notifier = {
*/
#ifdef ARCH_HAS_NMI_WATCHDOG
-static void __devinit hpwdt_check_nmi_sourcing(struct pci_dev *dev)
+static void __devinit hpwdt_check_nmi_decoding(struct pci_dev *dev)
{
/*
* If nmi_watchdog is turned off then we can turn on
- * our nmi sourcing capability.
+ * our nmi decoding capability.
*/
if (!nmi_watchdog_active())
- hpwdt_nmi_sourcing = 1;
+ hpwdt_nmi_decoding = 1;
else
- dev_warn(&dev->dev, "NMI sourcing is disabled. To enable this "
+ dev_warn(&dev->dev, "NMI decoding is disabled. To enable this "
"functionality you must reboot with nmi_watchdog=0 "
"and load the hpwdt driver with priority=1.\n");
}
#else
-static void __devinit hpwdt_check_nmi_sourcing(struct pci_dev *dev)
+static void __devinit hpwdt_check_nmi_decoding(struct pci_dev *dev)
{
- dev_warn(&dev->dev, "NMI sourcing is disabled. "
+ dev_warn(&dev->dev, "NMI decoding is disabled. "
"Your kernel does not support a NMI Watchdog.\n");
}
#endif
@@ -661,9 +661,9 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
char *buf;
/*
- * Check if we can do NMI sourcing or not
+ * Check if we can do NMI decoding or not
*/
- hpwdt_check_nmi_sourcing(dev);
+ hpwdt_check_nmi_decoding(dev);
/*
* First let's find out if we are on an iLO2+ server. We will
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 14/15] Make NMI decoding a compile-time option
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (12 preceding siblings ...)
2010-07-27 23:51 ` [PATCH 13/15] Use "decoding" instead of "sourcing" dann frazier
@ 2010-07-27 23:51 ` dann frazier
2010-07-27 23:51 ` [PATCH 15/15] Bump version to 1.2.0 dann frazier
` (2 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:51 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
hpwdt is quite functional without the NMI decoding feature.
This change lets users disable the NMI portion at compile-time
via the new HPWDT_NMI_DECODING config option.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/Kconfig | 20 ++++++++++++++------
drivers/watchdog/hpwdt.c | 43 ++++++++++++++++++++++++++++++++++++++-----
2 files changed, 52 insertions(+), 11 deletions(-)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index fc3d6b5..3d77697 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -560,12 +560,20 @@ config HP_WATCHDOG
depends on X86
help
A software monitoring watchdog and NMI sourcing driver. This driver
- will detect lockups and provide a stack trace. Also, when an NMI
- occurs this driver will make the necessary BIOS calls to log
- the cause of the NMI. This is a driver that will only load on a
- HP ProLiant system with a minimum of iLO2 support.
- To compile this driver as a module, choose M here: the
- module will be called hpwdt.
+ will detect lockups and provide a stack trace. This is a driver that
+ will only load on a HP ProLiant system with a minimum of iLO2 support.
+ To compile this driver as a module, choose M here: the module will be
+ called hpwdt.
+
+if HP_WATCHDOG
+
+config HPWDT_NMI_DECODING
+ bool "NMI decoding support for the HP ProLiant iLO Hardware Watchdog Timer"
+ help
+ When an NMI occurs this feature will make the necessary BIOS calls to
+ log the cause of the NMI.
+
+endif
config SC1200_WDT
tristate "National Semiconductor PC87307/PC97307 (ala SC1200) Watchdog"
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 00aea69..68f5e37 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -27,18 +27,21 @@
#include <linux/types.h>
#include <linux/uaccess.h>
#include <linux/watchdog.h>
+#ifdef CONFIG_HPWDT_NMI_DECODING
#include <linux/dmi.h>
#include <linux/spinlock.h>
#include <linux/nmi.h>
#include <linux/kdebug.h>
#include <linux/notifier.h>
#include <asm/cacheflush.h>
+#endif /* CONFIG_HPWDT_NMI_DECODING */
#define SECS_TO_TICKS(secs) ((secs) * 1000 / 128)
#define TICKS_TO_SECS(ticks) ((ticks) * 128 / 1000)
#define HPWDT_MAX_TIMER TICKS_TO_SECS(65535)
#define HPWDT_VERSION "1.1.1"
+#ifdef CONFIG_HPWDT_NMI_DECODING
#define PCI_BIOS32_SD_VALUE 0x5F32335F /* "_32_" */
#define CRU_BIOS_SIGNATURE_VALUE 0x55524324
#define PCI_BIOS32_PARAGRAPH_LEN 16
@@ -107,6 +110,7 @@ struct cmn_registers {
u16 res;
u32 reflags;
} __attribute__((packed));
+#endif /* CONFIG_HPWDT_NMI_DECODING */
#define DEFAULT_MARGIN 30
static unsigned int soft_margin = DEFAULT_MARGIN; /* in seconds */
@@ -127,6 +131,7 @@ static struct pci_device_id hpwdt_devices[] = {
};
MODULE_DEVICE_TABLE(pci, hpwdt_devices);
+#ifdef CONFIG_HPWDT_NMI_DECODING
static unsigned int hpwdt_nmi_decoding;
static unsigned int priority; /* hpwdt at end of die_notify list */
static DEFINE_SPINLOCK(rom_lock);
@@ -406,6 +411,7 @@ static int __devinit detect_cru_service(void)
return ((cru_rom_addr != NULL) ? 0 : -ENODEV);
}
#endif /* CONFIG_X86_64 */
+#endif /* CONFIG_HPWDT_NMI_DECODING */
/*
* Watchdog operations
@@ -454,6 +460,7 @@ static int hpwdt_change_timer(int new_margin)
return 0;
}
+#ifdef CONFIG_HPWDT_NMI_DECODING
/*
* NMI Handler
*/
@@ -486,6 +493,7 @@ static int hpwdt_pretimeout(struct notifier_block *nb, unsigned long ulReason,
out:
return NOTIFY_OK;
}
+#endif /* CONFIG_HPWDT_NMI_DECODING */
/*
* /dev/watchdog handling
@@ -623,15 +631,18 @@ static struct miscdevice hpwdt_miscdev = {
.fops = &hpwdt_fops,
};
+#ifdef CONFIG_HPWDT_NMI_DECODING
static struct notifier_block die_notifier = {
.notifier_call = hpwdt_pretimeout,
.priority = 0,
};
+#endif /* CONFIG_HPWDT_NMI_DECODING */
/*
* Init & Exit
*/
+#ifdef CONFIG_HPWDT_NMI_DECODING
#ifdef ARCH_HAS_NMI_WATCHDOG
static void __devinit hpwdt_check_nmi_decoding(struct pci_dev *dev)
{
@@ -652,13 +663,21 @@ static void __devinit hpwdt_check_nmi_decoding(struct pci_dev *dev)
dev_warn(&dev->dev, "NMI decoding is disabled. "
"Your kernel does not support a NMI Watchdog.\n");
}
-#endif
+#endif /* ARCH_HAS_NMI_WATCHDOG */
+#else /* !CONFIG_HPWDT_NMI_DECODING */
+static void __devinit hpwdt_check_nmi_decoding(struct pci_dev *dev)
+{
+}
+#endif /* CONFIG_HPWDT_NMI_DECODING */
static int __devinit hpwdt_init_one(struct pci_dev *dev,
const struct pci_device_id *ent)
{
int retval;
char *buf;
+#ifdef CONFIG_HPWDT_NMI_DECODING
+ char *tmp;
+#endif
/*
* Check if we can do NMI decoding or not
@@ -697,6 +716,7 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
if (hpwdt_change_timer(soft_margin))
hpwdt_change_timer(DEFAULT_MARGIN);
+#ifdef CONFIG_HPWDT_NMI_DECODING
/*
* We need to map the ROM to get the CRU service.
* For 32 bit Operating Systems we need to go through the 32 Bit
@@ -733,6 +753,7 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
retval);
goto error_die_notifier;
}
+#endif
retval = misc_register(&hpwdt_miscdev);
if (retval < 0) {
@@ -744,23 +765,31 @@ static int __devinit hpwdt_init_one(struct pci_dev *dev,
buf = kasprintf(GFP_KERNEL, "hp Watchdog Timer Driver: %s"
", timer margin: %d seconds (nowayout=%d)"
- ", allow kernel dump: %s (default = 0/OFF)"
- ", priority: %s (default = 0/LAST)",
+ ", allow kernel dump: %s (default = 0/OFF)",
HPWDT_VERSION, soft_margin, nowayout,
- (allow_kdump == 0) ? "OFF" : "ON",
- (priority == 0) ? "LAST" : "FIRST");
+ (allow_kdump == 0) ? "OFF" : "ON");
if (!buf)
goto error_nomem;
+#ifdef CONFIG_HPWDT_NMI_DECODING
+ tmp = buf;
+ buf = kasprintf(GFP_KERNEL, "%s, priority: %s (default = 0/LAST)",
+ buf, (priority == 0) ? "LAST" : "FIRST");
+ kfree(tmp);
+ if (!buf)
+ goto error_nomem;
+#endif
dev_info(&dev->dev, "%s.\n", buf);
return 0;
error_misc_register:
+#ifdef CONFIG_HPWDT_NMI_DECODING
unregister_die_notifier(&die_notifier);
error_die_notifier:
if (cru_rom_addr)
iounmap(cru_rom_addr);
error_get_cru:
+#endif
pci_iounmap(dev, pci_mem_addr);
error_pci_iomap:
pci_disable_device(dev);
@@ -774,10 +803,12 @@ static void __devexit hpwdt_exit(struct pci_dev *dev)
hpwdt_stop();
misc_deregister(&hpwdt_miscdev);
+#ifdef CONFIG_HPWDT_NMI_DECODING
unregister_die_notifier(&die_notifier);
if (cru_rom_addr)
iounmap(cru_rom_addr);
+#endif /* CONFIG_HPWDT_NMI_DECODING */
pci_iounmap(dev, pci_mem_addr);
pci_disable_device(dev);
}
@@ -815,9 +846,11 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
module_param(allow_kdump, int, 0);
MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
+#ifdef CONFIG_HPWDT_NMI_DECODING
module_param(priority, int, 0);
MODULE_PARM_DESC(priority, "The hpwdt driver handles NMIs first or last"
" (default = 0/Last)\n");
+#endif
module_init(hpwdt_init);
module_exit(hpwdt_cleanup);
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 15/15] Bump version to 1.2.0
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (13 preceding siblings ...)
2010-07-27 23:51 ` [PATCH 14/15] Make NMI decoding a compile-time option dann frazier
@ 2010-07-27 23:51 ` dann frazier
2010-07-28 13:10 ` [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) Mingarelli, Thomas
2010-08-05 21:41 ` Wim Van Sebroeck
16 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-07-27 23:51 UTC (permalink / raw)
To: Wim Van Sebroeck; +Cc: linux-kernel, Thomas Mingarelli, dann frazier
Making NMI sourcing a compile-time option is probably a significant-
enough change to warrant a minor version bump.
Signed-off-by: dann frazier <dannf@hp.com>
---
drivers/watchdog/hpwdt.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 68f5e37..e008788 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -39,7 +39,7 @@
#define SECS_TO_TICKS(secs) ((secs) * 1000 / 128)
#define TICKS_TO_SECS(ticks) ((ticks) * 128 / 1000)
#define HPWDT_MAX_TIMER TICKS_TO_SECS(65535)
-#define HPWDT_VERSION "1.1.1"
+#define HPWDT_VERSION "1.2.0"
#ifdef CONFIG_HPWDT_NMI_DECODING
#define PCI_BIOS32_SD_VALUE 0x5F32335F /* "_32_" */
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* RE: [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup)
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (14 preceding siblings ...)
2010-07-27 23:51 ` [PATCH 15/15] Bump version to 1.2.0 dann frazier
@ 2010-07-28 13:10 ` Mingarelli, Thomas
2010-08-05 21:41 ` Wim Van Sebroeck
16 siblings, 0 replies; 26+ messages in thread
From: Mingarelli, Thomas @ 2010-07-28 13:10 UTC (permalink / raw)
To: Frazier, Daniel Kent (ProLiant Linux), Wim Van Sebroeck
Cc: linux-kernel@vger.kernel.org
Acked-By: Tom Mingarelli <Thomas.mingarelli@hp.com>
> -----Original Message-----
> From: Frazier, Daniel Kent (ProLiant Linux)
> Sent: Tuesday, July 27, 2010 6:51 PM
> To: Wim Van Sebroeck
> Cc: linux-kernel@vger.kernel.org; Mingarelli, Thomas
> Subject: [PATCH 00/15] hpwdt: make NMI code a config option (+ other
> cleanup)
>
> hwpdt includes an NMI-decoding feature, but the watchdog interface is
> quite
> usable without it. This patchset makes it possible to build hpwdt without
> the
> NMI functionality, as well as some general cleanup.
>
> Kconfig | 22 +++++--
> hpwdt.c | 181 ++++++++++++++++++++++++++++++++++++----------------------
> ------
> 2 files changed, 119 insertions(+), 84 deletions(-)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup)
2010-07-27 23:50 [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) dann frazier
` (15 preceding siblings ...)
2010-07-28 13:10 ` [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup) Mingarelli, Thomas
@ 2010-08-05 21:41 ` Wim Van Sebroeck
2010-08-06 21:26 ` Wim Van Sebroeck
16 siblings, 1 reply; 26+ messages in thread
From: Wim Van Sebroeck @ 2010-08-05 21:41 UTC (permalink / raw)
To: dann frazier; +Cc: linux-kernel, Thomas Mingarelli
Hi Dan,
> hwpdt includes an NMI-decoding feature, but the watchdog interface is quite
> usable without it. This patchset makes it possible to build hpwdt without the
> NMI functionality, as well as some general cleanup.
Looking at the overall overview we have:
watchdog: hpwdt (1/3): Introduce SECS_TO_TICKS() macro
watchdog: hpwdt (2/3): allow full range of timer values supported by hardware
watchdog: hpwdt (3/3): implement WDIOC_GETTIMELEFT
[PATCH 01/15] hpwdt_pretimeout reorganization
[PATCH 02/15] remove unnecessary includes
[PATCH 03/15] include spinlock.h
[PATCH 04/15] add include for linux/bitops.h
[PATCH 05/15] Group together includes specific to NMI sourcing
[PATCH 06/15] Group options that affect watchdog behavior together
[PATCH 07/15] Group defines only used by NMI sourcing together
[PATCH 08/15] Group declarations specific to NMI sourcing together
[PATCH 09/15] Despecificate driver from iLO2
[PATCH 10/15] Fix a typo
[PATCH 11/15] Make x86 assembly ifdef guard more strict
[PATCH 12/15] Construct status message w/ kasprintf and emit it with dev_info
[PATCH 13/15] Use "decoding" instead of "sourcing"
[PATCH 14/15] Make NMI decoding a compile-time option
[PATCH 15/15] Bump version to 1.2.0
When I look closer I see that:
* Some of the clean-up stuff changes things that were introduced in the first 3 (timer) patches.
Since these patches are not in mainline yet, it would be better to directly do it there.
(Which means that you don't need patch 7 anymore).
* The clean-up of includes (2, 3, 4 and maybe 5) can be a single patch.
* Patch 10 is correct but since you change the Kconfig help text in patch 14 anyway,
I would incorporate that in patch 14. Same for patch 15 -> this should be combined with patch 14 also.
* Personally I prefer to do the general clean-up first and then do the different changes for the NMI part.
* I don't like the ifdef's in the init and exit procedures. A procedure/function that is defined for both cases is the way that we should go here. (see patch 14).
So overall: patches look good, but let's re-order the patches a bit (so that we first clean-up the driver and then add the NMI related changes (If we ever need to revert some things then we at least have a clean-driver before we start bisecting the NMI changes)).
And secondly: let's try to have the ifdef's out of the init and exit procedures.
If you can look at how we can get rid of the ifdef's in init and exit, then I will reorder the patches and change the 3 timer patches and put that allready in a git tree.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup)
2010-08-05 21:41 ` Wim Van Sebroeck
@ 2010-08-06 21:26 ` Wim Van Sebroeck
2010-08-06 21:29 ` dann frazier
0 siblings, 1 reply; 26+ messages in thread
From: Wim Van Sebroeck @ 2010-08-06 21:26 UTC (permalink / raw)
To: dann frazier; +Cc: linux-kernel, Linux Watchdog Mailing List, Thomas Mingarelli
Hi Dan,
> So overall: patches look good, but let's re-order the patches a bit (so that we first clean-up the driver and then add the NMI related changes (If we ever need to revert some things then we at least have a clean-driver before we start bisecting the NMI changes)).
> And secondly: let's try to have the ifdef's out of the init and exit procedures.
>
> If you can look at how we can get rid of the ifdef's in init and exit, then I will reorder the patches and change the 3 timer patches and put that allready in a git tree.
I reorganised the sequence of patches. I'll sent them to you for verification (and will put a copy in linux-watchdog mailing list).
the patches now look like:
watchdog: hpwdt (1/12): clean-up include-files.
watchdog: hpwdt (2/12): Group options that affect watchdog behavior together
watchdog: hpwdt (3/12): Group NMI sourcing specific items together
watchdog: hpwdt (4/12): Despecificate driver from iLO2
watchdog: hpwdt (5/12): Make x86 assembly ifdef guard more strict
watchdog: hpwdt (6/12): Introduce SECS_TO_TICKS() macro
watchdog: hpwdt (7/12): allow full range of timer values supported by hardware
watchdog: hpwdt (8/12): implement WDIOC_GETTIMELEFT
watchdog: hpwdt (9/12): hpwdt_pretimeout reorganization
watchdog: hpwdt (10/12): Construct status message w/ kasprintf and emit it with dev_info
watchdog: hpwdt (11/12): Use "decoding" instead of "sourcing"
watchdog: hpwdt (12/12): Make NMI decoding a compile-time option
(patch 12 can still be improved).
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup)
2010-08-06 21:26 ` Wim Van Sebroeck
@ 2010-08-06 21:29 ` dann frazier
2010-08-11 12:49 ` Wim Van Sebroeck
0 siblings, 1 reply; 26+ messages in thread
From: dann frazier @ 2010-08-06 21:29 UTC (permalink / raw)
To: Wim Van Sebroeck
Cc: linux-kernel, Linux Watchdog Mailing List, Thomas Mingarelli
On Fri, Aug 06, 2010 at 11:26:48PM +0200, Wim Van Sebroeck wrote:
> Hi Dan,
>
> > So overall: patches look good, but let's re-order the patches a bit (so that we first clean-up the driver and then add the NMI related changes (If we ever need to revert some things then we at least have a clean-driver before we start bisecting the NMI changes)).
> > And secondly: let's try to have the ifdef's out of the init and exit procedures.
> >
> > If you can look at how we can get rid of the ifdef's in init and exit, then I will reorder the patches and change the 3 timer patches and put that allready in a git tree.
>
> I reorganised the sequence of patches. I'll sent them to you for verification (and will put a copy in linux-watchdog mailing list).
>
> the patches now look like:
> watchdog: hpwdt (1/12): clean-up include-files.
> watchdog: hpwdt (2/12): Group options that affect watchdog behavior together
> watchdog: hpwdt (3/12): Group NMI sourcing specific items together
> watchdog: hpwdt (4/12): Despecificate driver from iLO2
> watchdog: hpwdt (5/12): Make x86 assembly ifdef guard more strict
> watchdog: hpwdt (6/12): Introduce SECS_TO_TICKS() macro
> watchdog: hpwdt (7/12): allow full range of timer values supported by hardware
> watchdog: hpwdt (8/12): implement WDIOC_GETTIMELEFT
> watchdog: hpwdt (9/12): hpwdt_pretimeout reorganization
> watchdog: hpwdt (10/12): Construct status message w/ kasprintf and emit it with dev_info
> watchdog: hpwdt (11/12): Use "decoding" instead of "sourcing"
> watchdog: hpwdt (12/12): Make NMI decoding a compile-time option
>
> (patch 12 can still be improved).
Thanks! I'll work on the #ifdef'ing & get you a new #12
--
dann frazier | ProLiant Linux
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup)
2010-08-06 21:29 ` dann frazier
@ 2010-08-11 12:49 ` Wim Van Sebroeck
2010-08-13 7:29 ` dann frazier
0 siblings, 1 reply; 26+ messages in thread
From: Wim Van Sebroeck @ 2010-08-11 12:49 UTC (permalink / raw)
To: dann frazier; +Cc: linux-kernel, Linux Watchdog Mailing List, Thomas Mingarelli
Hi Dan,
> > watchdog: hpwdt (10/12): Construct status message w/ kasprintf and emit it with dev_info
> > watchdog: hpwdt (11/12): Use "decoding" instead of "sourcing"
> > watchdog: hpwdt (12/12): Make NMI decoding a compile-time option
> >
> > (patch 12 can still be improved).
>
> Thanks! I'll work on the #ifdef'ing & get you a new #12
I did some changes in the last 3 patches:
I removed patch 10, made patch 11 patch 10 and added and extra patch that moves the nmi decoding init and exit functions in seperate modules.
I'll sent you the 3 new patches. Just let me know if they are OK.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/15] hpwdt: make NMI code a config option (+ other cleanup)
2010-08-11 12:49 ` Wim Van Sebroeck
@ 2010-08-13 7:29 ` dann frazier
0 siblings, 0 replies; 26+ messages in thread
From: dann frazier @ 2010-08-13 7:29 UTC (permalink / raw)
To: Wim Van Sebroeck
Cc: linux-kernel, Linux Watchdog Mailing List, Thomas Mingarelli
On Wed, Aug 11, 2010 at 02:49:11PM +0200, Wim Van Sebroeck wrote:
> Hi Dan,
>
> > > watchdog: hpwdt (10/12): Construct status message w/ kasprintf and emit it with dev_info
> > > watchdog: hpwdt (11/12): Use "decoding" instead of "sourcing"
> > > watchdog: hpwdt (12/12): Make NMI decoding a compile-time option
> > >
> > > (patch 12 can still be improved).
> >
> > Thanks! I'll work on the #ifdef'ing & get you a new #12
>
> I did some changes in the last 3 patches:
> I removed patch 10, made patch 11 patch 10 and added and extra patch that moves the nmi decoding init and exit functions in seperate modules.
> I'll sent you the 3 new patches. Just let me know if they are OK.
Yes, they look very much like the changes I was preparing - but I got
sidetracked by vacation before I could test :) I pulled your tree &
ran a few sanity tests - looks good. Thanks for doing this!
--
dann frazier | ProLiant Linux
^ permalink raw reply [flat|nested] 26+ messages in thread