[PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Rene Herman]

[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

[ 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.

[-- Attachment #2: 0001-WATCHDOG-don-t-auto-grab-eurotechwdt.patch --]
[-- Type: text/plain, Size: 4180 bytes --]

>From b31c25c371dd33e6316c327f3f143a2e260d866b Mon Sep 17 00:00:00 2001
From: Rene Herman <rene.herman@gmail.com>
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(-)

diff --git a/drivers/watchdog/eurotechwdt.c b/drivers/watchdog/eurotechwdt.c
index b14e9d1..c4eb06c 100644
--- a/drivers/watchdog/eurotechwdt.c
+++ b/drivers/watchdog/eurotechwdt.c
@@ -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");
-- 
1.5.5

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Alan Cox]

On Sat, 09 Aug 2008 23:42:50 +0200
Rene Herman <rene.herman@keyaccess.nl> 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. 

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Rene Herman]

On 09-08-08 23:42, Rene Herman 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:
> 
> 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.

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

Rene.

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Wim Van Sebroeck]

Hi Rene,

> >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.

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Adrian Bunk]

On Sat, Aug 09, 2008 at 11:42:50PM +0200, Rene Herman 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:
>...

WTF?

You are expecting randconfig kernels to boot?

cu
Adrian

-- 

       "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

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Rene Herman]

On 12-08-08 14:58, Adrian Bunk wrote:

> On Sat, Aug 09, 2008 at 11:42:50PM +0200, Rene Herman wrote:

>> 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.

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Adrian Bunk]

On Tue, Aug 12, 2008 at 03:12:12PM +0200, Rene Herman wrote:
> On 12-08-08 14:58, Adrian Bunk wrote:
>
>> On Sat, Aug 09, 2008 at 11:42:50PM +0200, Rene Herman wrote:
>
>>> 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.

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.

...

> Rene.

cu
Adrian

-- 

       "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

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Rene Herman]

On 12-08-08 15:18, Adrian Bunk wrote:

>>> 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...

> 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.

Re: [PATCH] WATCHDOG: don't auto-grab eurotechwdt.

[Adrian Bunk]

On Tue, Aug 12, 2008 at 03:30:42PM +0200, Rene Herman wrote:
> On 12-08-08 15:18, Adrian Bunk wrote:
>
>>>> 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...
>
>> 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.

> Rene.

cu
Adrian

-- 

       "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