Skip to contentSkip to conversation

WATCHDOG: don't auto-grab eurotechwdt.

PATCH In review Rene Herman 4 participants 9 messages Aug 9, 2008 – Aug 12, 2008 Watchdog device drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 WATCHDOG: don't auto-grab eurotechwdt.

[ Andrew: not submitted ]

Hi Wim.

I'm going over a list of drivers that break the boot of randconfig kernels by keeping resources busy:

http://lkml.org/lkml/2008/8/1/96

and eurotechwdt and plain wdt are in that list. Below is an eurotechwdt that makes passing in io and irq values mandatory as to keep that from happening.

I saw pcwd was already isafied and followed that -- due to the comment in eurwdt_release() though, I'm not sure about a .shutdown() method.

Do you want this in the first place? The randconfig testing does find actual bugs so it is useful.

I'll do wdt and the other applicable ones in there as well if yes.

Rene.

Quoted 1 line

From b31c25c371dd33e6316c327f3f143a2e260d866b Mon Sep 17 00:00:00 2001

From: Rene Herman <[email protected]>
Date: Sat, 9 Aug 2008 23:24:34 +0200
Subject: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

---
 drivers/watchdog/eurotechwdt.c |   82 +++++++++++++++++++++++++++-------------
 1 files changed, 55 insertions(+), 27 deletions(-)
drivers/watchdog/eurotechwdt.c +55 -27
index b14e9d1..c4eb06c 100644
@@ -56,6 +56,7 @@
 #include <linux/notifier.h>
 #include <linux/reboot.h>
 #include <linux/init.h>
+#include <linux/isa.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>
@@ -70,8 +71,8 @@ static char eur_expect_close;
  * You can use eurwdt=x,y to set these now.
  */
 
-static int io = 0x3f0;
-static int irq = 10;
+static int io;			/* 0x3f0 */
+static int irq = -1;		/* 10    */
 static char *ev = "int";
 
 #define WDT_TIMEOUT		60                /* 1 minute */
@@ -95,9 +96,9 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" _
 
 
 module_param(io, int, 0);
-MODULE_PARM_DESC(io, "Eurotech WDT io port (default=0x3f0)");
+MODULE_PARM_DESC(io, "Eurotech WDT io port (mandatory)");
 module_param(irq, int, 0);
-MODULE_PARM_DESC(irq, "Eurotech WDT irq (default=10)");
+MODULE_PARM_DESC(irq, "Eurotech WDT irq (mandatory)");
 module_param(ev, charp, 0);
 MODULE_PARM_DESC(ev, "Eurotech WDT event type (default is `int')");
 
@@ -385,41 +386,28 @@ static struct notifier_block eurwdt_notifier = {
 	.notifier_call = eurwdt_notify_sys,
 };
 
-/**
- * cleanup_module:
- *
- * Unload the watchdog. You cannot do this with any file handles open.
- * If your watchdog is set to continue ticking on close and you unload
- * it, well it keeps ticking. We won't get the interrupt but the board
- * will not touch PC memory so all is fine. You just have to load a new
- * module in 60 seconds or reboot.
- */
-
-static void __exit eurwdt_exit(void)
+static int __devinit eurwdt_isa_match(struct device *dev, unsigned int id)
 {
-	eurwdt_lock_chip();
+	int match = io != 0 && irq != -1;
 
-	misc_deregister(&eurwdt_miscdev);
+	if (!match)
+		dev_err(dev, "please specify io and irq\n");
 
-	unregister_reboot_notifier(&eurwdt_notifier);
-	release_region(io, 2);
-	free_irq(irq, NULL);
+	return match;
 }
 
-/**
- * eurwdt_init:
- *
+/*
  * Set up the WDT watchdog board. After grabbing the resources
  * we require we need also to unlock the device.
  * The open() function will actually kick the board off.
  */
 
-static int __init eurwdt_init(void)
+static int __devinit eurwdt_isa_probe(struct device *dev, unsigned int id)
 {
 	int ret;
 
 	ret = request_irq(irq, eurwdt_interrupt, IRQF_DISABLED, "eurwdt", NULL);
-	if(ret) {
+	if (ret) {
 		printk(KERN_ERR "eurwdt: IRQ %d is not free.\n", irq);
 		goto out;
 	}
@@ -464,8 +452,48 @@ outirq:
 	goto out;
 }
 
-module_init(eurwdt_init);
-module_exit(eurwdt_exit);
+/*
+ * Unload the watchdog. You cannot do this with any file handles open.
+ * If your watchdog is set to continue ticking on close and you unload
+ * it, well it keeps ticking. We won't get the interrupt but the board
+ * will not touch PC memory so all is fine. You just have to load a new
+ * module in 60 seconds or reboot.
+ */
+
+static int __devexit eurwdt_isa_remove(struct device *dev, unsigned int id)
+{
+	eurwdt_lock_chip();
+
+	misc_deregister(&eurwdt_miscdev);
+
+	unregister_reboot_notifier(&eurwdt_notifier);
+	release_region(io, 2);
+	free_irq(irq, NULL);
+	return 0;
+}
+
+static struct isa_driver eurwdt_isa_driver = {
+	.match		= eurwdt_isa_match,
+	.probe		= eurwdt_isa_probe,
+	.remove		= __devexit_p(eurwdt_isa_remove),
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "eurwdt",
+	},
+};
+
+static int __init eurwdt_init_module(void)
+{
+	return isa_register_driver(&eurwdt_isa_driver, 1);
+}
+
+static void __exit eurwdt_exit_module(void)
+{
+	isa_unregister_driver(&eurwdt_isa_driver);
+}
+
+module_init(eurwdt_init_module);
+module_exit(eurwdt_exit_module);
 
 MODULE_AUTHOR("Rodolfo Giometti");
 MODULE_DESCRIPTION("Driver for Eurotech CPU-1220/1410 on board watchdog");
Signature · 1 line
1.5.5
#1 lore raw
Quoted 6 lines · On Sat, 09 Aug 2008 23:42:50 +0200 Rene Herman <[email protected]> wrote:

[ Andrew: not submitted ]

Hi Wim.

I'm going over a list of drivers that break the boot of randconfig kernels by keeping resources busy:

NAK this one. Same reason - if its a module the auto-grab should be enabled.

#2 lore raw
In reply to Rene Herman
Quoted 8 lines · Rene Herman

[ Andrew: not submitted ]

Hi Wim.

I'm going over a list of drivers that break the boot of randconfig kernels by keeping resources busy:

http://lkml.org/lkml/2008/8/1/96

Quoted 7 lines

and eurotechwdt and plain wdt are in that list. Below is an eurotechwdt that makes passing in io and irq values mandatory as to keep that from happening.

I saw pcwd was already isafied and followed that -- due to the comment in eurwdt_release() though, I'm not sure about a .shutdown() method.

Do you want this in the first place? The randconfig testing does find actual bugs so it is useful.

I'll do wdt and the other applicable ones in there as well if yes.

Wim, do you want them with the grab when modular, not grab when builtin distinction?

Rene.

#3 lore raw

Hi Rene,

Quoted 11 lines
Quoted 11 lines

I'm going over a list of drivers that break the boot of randconfig kernels by keeping resources busy:

http://lkml.org/lkml/2008/8/1/96

and eurotechwdt and plain wdt are in that list. Below is an eurotechwdt that makes passing in io and irq values mandatory as to keep that from happening.

I saw pcwd was already isafied and followed that -- due to the comment in eurwdt_release() though, I'm not sure about a .shutdown() method.

The Berkshire PC-Watchdog card is indeed allready isafied. This is (just like the mixcom and the ICS WDT50x watchdog cards) an ISA card that can be inserted into an ISA slot. (The other two will be isafied also). The shutdown notifier does the same thing then the reboot_notifier.

Most of the other drivers (except the 2 PCI cards and 1 USB card) are however directly attached to the motherboard of the system. These are thus in my opinion more platform devices then isa device drivers.

The problem with a lott of these devices (however) is that they can not be probed. You either have the hardware or you don't. Some can't even be stopped once started, some are allready active on boot.
My feeling: If you have the hardware, you will compile and load the driver of these "platform" watchdog devices, if not you won't.

What we do best, however, to avoid problems at boot with random kernels looks to be another issue to me.

Kind regards,
Wim.

#4 lore raw
In reply to Rene Herman
Quoted 7 lines · Rene Herman

[ Andrew: not submitted ]

Hi Wim.

I'm going over a list of drivers that break the boot of randconfig kernels by keeping resources busy:
...

WTF?

You are expecting randconfig kernels to boot?

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#5 lore raw
Quoted 8 lines · Adrian Bunk
Quoted 2 lines · Rene Herman

I'm going over a list of drivers that break the boot of randconfig kernels by keeping resources busy:
...

WTF?

You are expecting randconfig kernels to boot?

90% does it seems. Ingo has been using it as part of automated testing for some time now.

Rene.

#6 lore raw
Quoted 8 lines · Rene Herman
Quoted 6 lines · Adrian Bunk
Quoted 3 lines · Rene Herman

I'm going over a list of drivers that break the boot of randconfig kernels by keeping resources busy:
...

Quoted 6 lines
Quoted 3 lines

WTF?

You are expecting randconfig kernels to boot?

90% does it seems. Ingo has been using it as part of automated testing for some time now.

Please define "boot".

E.g. for your harddisk driver alone the probability of not having it included in a randconfig kernel is definitely > 10%.

Same goes for the network driver.

...

Quoted 1 line

Rene.

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#7 lore raw
Quoted 4 lines · Adrian Bunk
Quoted 2 lines
Quoted 1 line

You are expecting randconfig kernels to boot?

90% does it seems. Ingo has been using it as part of automated testing for some time now.

Please define "boot".

I can't since I'm not doing the testing, Ingo is. When you ask him, do take into account the inherent goodness of things that catch bugs...

Quoted 3 lines

E.g. for your harddisk driver alone the probability of not having it included in a randconfig kernel is definitely > 10%.

Same goes for the network driver.

I suppose he has local allrandom.config files.

Rene.

#8 lore raw
Quoted 11 lines · Rene Herman
Quoted 5 lines · Adrian Bunk
Quoted 3 lines
Quoted 1 line

You are expecting randconfig kernels to boot?

90% does it seems. Ingo has been using it as part of automated testing for some time now.

Please define "boot".

I can't since I'm not doing the testing, Ingo is. When you ask him, do take into account the inherent goodness of things that catch bugs...

Quoted 6 lines
Quoted 4 lines

E.g. for your harddisk driver alone the probability of not having it included in a randconfig kernel is definitely > 10%.

Same goes for the network driver.

I suppose he has local allrandom.config files.

Then he could add a few additional options there if required.

Quoted 1 line

Rene.

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed