On 12/22/2014 08:07 AM, Muller, Francois-nicolas wrote:
Subject: [PATCH] TCO watchdog warning interrupt handler
Adding TCO watchdog warning interrupt handling.
This feature is useful to root cause watchdog expiration.
It is activated by boot parameter 'warn_irq' (disabled by default).
Upon first expiration of the TCO watchdog, a warning interrupt is
fired and the driver calls panic() to dump debug information (registers and
call stacks).
At the same time, the TCO watchdog reloads with 2.4 seconds timeout
value and runs until the second expiration.
Signed-off-by: Francois-Nicolas Muller <francois-nicolas.muller@xxxxxxxxx>
---
drivers/watchdog/iTCO_wdt.c | 66 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c index e802a54..1ce9ad8 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -49,6 +49,8 @@
/* Module and version information */
#define DRV_NAME "iTCO_wdt"
#define DRV_VERSION "1.11"
+#define DRV_NAME_ACPI "iTCO_wdt_wirq"
+#define TCO_CLASS DRV_NAME
/* Includes */
#include <linux/module.h> /* For module specific items */
@@ -68,6 +70,9 @@
#include <linux/pm.h> /* For suspend/resume */
#include <linux/mfd/core.h>
#include <linux/mfd/lpc_ich.h>
+#include <linux/nmi.h>
+#include <linux/acpi.h>
+#include <acpi/actypes.h>
#include "iTCO_vendor.h"
@@ -107,6 +112,12 @@ static struct { /* this is private data for the iTCO_wdt device */
bool started;
} iTCO_wdt_private;
+static const struct acpi_device_id iTCO_wdt_ids[] = {
+ {"8086229C", 0},
+ {"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, iTCO_wdt_ids);
+
Doesn't that result in auto-loading the driver ?
If so, if that is really desirable, it should be a separate patch.
/* module parameters */
#define WATCHDOG_TIMEOUT 30 /* 30 sec default heartbeat */
static int heartbeat = WATCHDOG_TIMEOUT; /* in seconds */ @@ -126,6 +137,11 @@ module_param(turn_SMI_watchdog_clear_off, int, 0); MODULE_PARM_DESC(turn_SMI_watchdog_clear_off,
"Turn off SMI clearing watchdog (depends on TCO-version)(default=1)");
Really looks like the patch is corrupted, with lines merged.
+static bool warn_irq;
+module_param(warn_irq, bool, 0);
+MODULE_PARM_DESC(warn_irq,
+ "Watchdog trigs a panic at first expiration (default=0)");
+
/*
* Some TCO specific functions
*/
@@ -200,6 +216,37 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void)
return ret; /* returns: 0 = OK, -EIO = Error */ }
+static u32 iTCO_wdt_wirq(acpi_handle gpe_device, u32 gpe, void
+*context) {
Either the patch is corrupted, or you need to look into kernel
coding style rules.
+ pr_warn("[SHTDWN] %s, WATCHDOG TIMEOUT HANDLER!\n", __func__);
+
That is kind of redundant with the following panic.
+ trigger_all_cpu_backtrace();
+ panic("Kernel Watchdog");
+
Quite frankly, I don't see the point of this patch. Sure, this will
dump the stack. Ok, but so what ? What value would it have for the
user to see the interrupt call stack that is seen if the watchdog
times out ?
Guess I must be missing something.
+ /* This code should not be reached */
+ return IRQ_HANDLED;
+}
+
+static int iTCO_wdt_acpi_add(struct acpi_device *device) {
+ unsigned long long gpe;
+ acpi_status status;
+
+ status = acpi_evaluate_integer(device->handle, "_GPE", NULL, &gpe);
+ if (ACPI_FAILURE(status))
+ return -EINVAL;
+
+ status = acpi_install_gpe_handler(NULL, gpe, ACPI_GPE_EDGE_TRIGGERED,
+ iTCO_wdt_wirq, NULL);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ acpi_enable_gpe(NULL, gpe);
+
+ pr_info("interrupt=SCI GPE=0x%02llx", gpe);
I fail to see the value of this log message.
+ return 0;
+}
+
static int iTCO_wdt_start(struct watchdog_device *wd_dev) {
unsigned int val;
@@ -628,6 +675,15 @@ static struct platform_driver iTCO_wdt_driver = {
},
};
+static struct acpi_driver iTCO_wdt_acpi_driver = {
+ .name = DRV_NAME_ACPI,
+ .class = TCO_CLASS,
+ .ids = iTCO_wdt_ids,
+ .ops = {
+ .add = iTCO_wdt_acpi_add,
+ },
+};
+
static int __init iTCO_wdt_init_module(void) {
int err;
@@ -638,12 +694,22 @@ static int __init iTCO_wdt_init_module(void)
if (err)
return err;
+ if (warn_irq) {
+ err = acpi_bus_register_driver(&iTCO_wdt_acpi_driver);
+ if (err) {
+ platform_driver_unregister(&iTCO_wdt_driver);
+ return err;
+ }
+ }
+
return 0;
}
static void __exit iTCO_wdt_cleanup_module(void) {
The original code is not formatted like this.
platform_driver_unregister(&iTCO_wdt_driver);
+ if (warn_irq)
+ acpi_bus_unregister_driver(&iTCO_wdt_acpi_driver);
pr_info("Watchdog Module Unloaded\n"); }
--
1.7.9.5
-----Original Message-----
From: Darren Hart [mailto:dvhart@xxxxxxxxxxxxx]
Sent: Monday, December 22, 2014 4:41 PM
To: Muller, Francois-nicolas; Guenter Roeck
Cc: 'platform-driver-x86@xxxxxxxxxxxxxxx'; Rafael Wysocki; Linux ACPI Mailing List; linux-watchdog@xxxxxxxxxxxxxxx; Wim Van Sebroeck
Subject: RE: [PATCH] TCO Watchdog warning interrupt driver creation
On December 22, 2014 5:18:33 AM PST, "Muller, Francois-nicolas" <francois-nicolas.muller@xxxxxxxxx> wrote:
Thanks Darren and Guenter for your review.
Here is a new patchset, including the feature in the watchdog driver
and using a boot parameter to be enabled.
François-Nicolas
Subject: [PATCH] TCO watchdog warning interrupt handler
Please provide a complete commit message.
Signed-off-by: Francois-Nicolas Muller
<francois-nicolas.muller@xxxxxxxxx>
--
Darren Hart
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
