[PATCH] watchdog: core: module param to activate watchdog

[PATCH] watchdog: core: module param to activate watchdog

[Markus Pargmann]

Many watchdog driver reset the watchdog device on initialization. This
is a problem if the watchdog is activated by the bootloader and should
be active the whole time until the userspace can write to it.

This patch adds a module parameter (watchdog.activate_first) that
activates the first registered watchdog. Using this parameter it is
possible to have an active watchdog during the whole boot process.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/watchdog/watchdog_core.c |  4 ++++
 drivers/watchdog/watchdog_core.h |  4 ++++
 drivers/watchdog/watchdog_dev.c  | 21 ++++++++++++++++++++-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index cec9b55..6db16eb 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -43,6 +43,10 @@
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+unsigned int activate_first = 0;
+module_param(activate_first, uint, 0);
+MODULE_PARM_DESC(activate_first, "Activation timeout for first watchdog registered. 0 means no activation.");
+
 static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
 {
 	/*
diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
index 6c95141..4526170 100644
--- a/drivers/watchdog/watchdog_core.h
+++ b/drivers/watchdog/watchdog_core.h
@@ -28,6 +28,10 @@
 
 #define MAX_DOGS	32	/* Maximum number of watchdog devices */
 
+/* Activate first registered watchdog and set this as the timeout value
+ * (only != 0) */
+extern unsigned int activate_first;
+
 /*
  *	Functions/procedures to be called by the core
  */
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 6aaefba..fe60fdc 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -553,8 +553,27 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
 			misc_deregister(&watchdog_miscdev);
 			old_wdd = NULL;
 		}
+
+		return err;
 	}
-	return err;
+
+	if (activate_first && watchdog->id == 0) {
+		err = watchdog_set_timeout(watchdog, activate_first);
+		if (err && err != -EOPNOTSUPP) {
+			pr_err("watchdog%d failed to set timeout for first activation\n",
+					watchdog->id);
+			return err;
+		}
+
+		err = watchdog_start(watchdog);
+		if (err) {
+			pr_err("watchdog%d failed to start first watchdog\n",
+					watchdog->id);
+			return err;
+		}
+	}
+
+	return 0;
 }
 
 /*
-- 
1.8.5.3

Re: [PATCH] watchdog: core: module param to activate watchdog

[Guenter Roeck]

On 02/28/2014 08:16 AM, Markus Pargmann wrote:
> Many watchdog driver reset the watchdog device on initialization. This
> is a problem if the watchdog is activated by the bootloader and should
> be active the whole time until the userspace can write to it.
>

Shouldn't those watchdog drivers be fixed to keep the watchdog running
if it was already active ? Otherwise there is still the time between
driver initialization, which disables the watchdog, and it being re-enabled
by watchdog registration.

Also, does this affect or impact drivers which _do_ keep the watchdog
enabled ?

> This patch adds a module parameter (watchdog.activate_first) that
> activates the first registered watchdog. Using this parameter it is
> possible to have an active watchdog during the whole boot process.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>   drivers/watchdog/watchdog_core.c |  4 ++++
>   drivers/watchdog/watchdog_core.h |  4 ++++
>   drivers/watchdog/watchdog_dev.c  | 21 ++++++++++++++++++++-
>   3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index cec9b55..6db16eb 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -43,6 +43,10 @@
>   static DEFINE_IDA(watchdog_ida);
>   static struct class *watchdog_class;
>
> +unsigned int activate_first = 0;

Bad variable name for a global variable.

> +module_param(activate_first, uint, 0);
> +MODULE_PARM_DESC(activate_first, "Activation timeout for first watchdog registered. 0 means no activation.");
> +

Does that _have_ to be in this file ? Would be much easier
if it was in watchdog_dev.c, where it is used.

>   static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
>   {
>   	/*
> diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
> index 6c95141..4526170 100644
> --- a/drivers/watchdog/watchdog_core.h
> +++ b/drivers/watchdog/watchdog_core.h
> @@ -28,6 +28,10 @@
>
>   #define MAX_DOGS	32	/* Maximum number of watchdog devices */
>
> +/* Activate first registered watchdog and set this as the timeout value
> + * (only != 0) */

Coding style.

> +extern unsigned int activate_first;
> +
>   /*
>    *	Functions/procedures to be called by the core
>    */
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 6aaefba..fe60fdc 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -553,8 +553,27 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
>   			misc_deregister(&watchdog_miscdev);
>   			old_wdd = NULL;
>   		}
> +

Is that extra empty line really necessary ?

> +		return err;
>   	}
> -	return err;
> +
> +	if (activate_first && watchdog->id == 0) {
> +		err = watchdog_set_timeout(watchdog, activate_first);
> +		if (err && err != -EOPNOTSUPP) {
> +			pr_err("watchdog%d failed to set timeout for first activation\n",
> +					watchdog->id);
> +			return err;
> +		}
> +
> +		err = watchdog_start(watchdog);
> +		if (err) {
> +			pr_err("watchdog%d failed to start first watchdog\n",
> +					watchdog->id);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
>   }
>
>   /*
>

Re: [PATCH] watchdog: core: module param to activate watchdog

[Markus Pargmann]

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

Hi,

On Sat, Mar 01, 2014 at 09:11:18PM -0800, Guenter Roeck wrote:
> On 02/28/2014 08:16 AM, Markus Pargmann wrote:
> >Many watchdog driver reset the watchdog device on initialization. This
> >is a problem if the watchdog is activated by the bootloader and should
> >be active the whole time until the userspace can write to it.
> >
> 
> Shouldn't those watchdog drivers be fixed to keep the watchdog running
> if it was already active ? Otherwise there is still the time between
> driver initialization, which disables the watchdog, and it being re-enabled
> by watchdog registration.

Yes there is still a time between stopping watchdog and reenabling it.
But as mentioned in the other mail, I think this should be some generic
solution. Also the watchdog core would not know that the watchdog device
is active when the watchdog driver keeps it running.

> 
> Also, does this affect or impact drivers which _do_ keep the watchdog
> enabled ?

I don't know of such a driver. However those drivers would have to
expect that they are started at some point through the watchdog core. At
least when someone opens the watchdog device and pings it.

> 
> >This patch adds a module parameter (watchdog.activate_first) that
> >activates the first registered watchdog. Using this parameter it is
> >possible to have an active watchdog during the whole boot process.
> >
> >Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> >---
> >  drivers/watchdog/watchdog_core.c |  4 ++++
> >  drivers/watchdog/watchdog_core.h |  4 ++++
> >  drivers/watchdog/watchdog_dev.c  | 21 ++++++++++++++++++++-
> >  3 files changed, 28 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> >index cec9b55..6db16eb 100644
> >--- a/drivers/watchdog/watchdog_core.c
> >+++ b/drivers/watchdog/watchdog_core.c
> >@@ -43,6 +43,10 @@
> >  static DEFINE_IDA(watchdog_ida);
> >  static struct class *watchdog_class;
> >
> >+unsigned int activate_first = 0;
> 
> Bad variable name for a global variable.
> 
> >+module_param(activate_first, uint, 0);
> >+MODULE_PARM_DESC(activate_first, "Activation timeout for first watchdog registered. 0 means no activation.");
> >+
> 
> Does that _have_ to be in this file ? Would be much easier
> if it was in watchdog_dev.c, where it is used.

As far as I know, it has to be in this file, as this is the watchdog
module. Perhaps I can try to move it to watchdog_dev.c somehow.

I will fix the other comments if there is a second version of this
patch.

Thanks,

Markus


> 
> >  static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
> >  {
> >  	/*
> >diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
> >index 6c95141..4526170 100644
> >--- a/drivers/watchdog/watchdog_core.h
> >+++ b/drivers/watchdog/watchdog_core.h
> >@@ -28,6 +28,10 @@
> >
> >  #define MAX_DOGS	32	/* Maximum number of watchdog devices */
> >
> >+/* Activate first registered watchdog and set this as the timeout value
> >+ * (only != 0) */
> 
> Coding style.
> 
> >+extern unsigned int activate_first;
> >+
> >  /*
> >   *	Functions/procedures to be called by the core
> >   */
> >diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> >index 6aaefba..fe60fdc 100644
> >--- a/drivers/watchdog/watchdog_dev.c
> >+++ b/drivers/watchdog/watchdog_dev.c
> >@@ -553,8 +553,27 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
> >  			misc_deregister(&watchdog_miscdev);
> >  			old_wdd = NULL;
> >  		}
> >+
> 
> Is that extra empty line really necessary ?
> 
> >+		return err;
> >  	}
> >-	return err;
> >+
> >+	if (activate_first && watchdog->id == 0) {
> >+		err = watchdog_set_timeout(watchdog, activate_first);
> >+		if (err && err != -EOPNOTSUPP) {
> >+			pr_err("watchdog%d failed to set timeout for first activation\n",
> >+					watchdog->id);
> >+			return err;
> >+		}
> >+
> >+		err = watchdog_start(watchdog);
> >+		if (err) {
> >+			pr_err("watchdog%d failed to start first watchdog\n",
> >+					watchdog->id);
> >+			return err;
> >+		}
> >+	}
> >+
> >+	return 0;
> >  }
> >
> >  /*
> >
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] watchdog: core: module param to activate watchdog

[Wim Van Sebroeck]

Hi Markus,

> Many watchdog driver reset the watchdog device on initialization. This
> is a problem if the watchdog is activated by the bootloader and should
> be active the whole time until the userspace can write to it.
> 
> This patch adds a module parameter (watchdog.activate_first) that
> activates the first registered watchdog. Using this parameter it is
> possible to have an active watchdog during the whole boot process.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

NAK. It is the responsibility of the watchdog device driver to do this
and not that of the core. The core can't know which device(s) are 
running and can't be turned off. So you can have more then 1 device 
that needs to keep going...

The normal way do this is by using a timer. Take a look at 
at91sam9_wdt.c . We have a timer there that pings the watchdog if
1) the watchdog device is not active for userspace (so before opening
/dev/watchdog and after clossing /dev/watchdog correctly)
2) pings 1/2 or 1/4 of the watchdog heartbeat time as long as the
userspace ping keeps going. (Else it will expire and you trigger a 
reboot).
(Note: the heartbeat is the period after which a watchdog hardware
device will trigger a reboot, timeout is the period for userspace.
So for most devices heartbeat and timeout are the same, but for 
devices where a timer is used (we also use it if the heartbeat value
is too small (i.e. 1 second)) the values have a different meaning.)

So the logic for a watchdog device driver is:
at startup the watchdog device driver should make sure that a reboot
can only occur when /dev/watchdog is open. This means that if a
watchdog device is active at boot it should either be stopped (that's
why most drivers have a stop in their init/probe function) or have a 
timer that pings the watchdog as long as it is not active for 
userspace (like at91sam9_wdt.c, via_wdt.c, pika_wdt.c, ...)

Kind regards,
Wim.

Re: [PATCH] watchdog: core: module param to activate watchdog

[Markus Pargmann]

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

Hi Wim,

On Sun, Mar 02, 2014 at 10:43:23AM +0100, Wim Van Sebroeck wrote:
> Hi Markus,
> 
> > Many watchdog driver reset the watchdog device on initialization. This
> > is a problem if the watchdog is activated by the bootloader and should
> > be active the whole time until the userspace can write to it.
> > 
> > This patch adds a module parameter (watchdog.activate_first) that
> > activates the first registered watchdog. Using this parameter it is
> > possible to have an active watchdog during the whole boot process.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> 
> NAK. It is the responsibility of the watchdog device driver to do this
> and not that of the core. The core can't know which device(s) are 
> running and can't be turned off. So you can have more then 1 device 
> that needs to keep going...
> 
> The normal way do this is by using a timer. Take a look at 
> at91sam9_wdt.c . We have a timer there that pings the watchdog if
> 1) the watchdog device is not active for userspace (so before opening
> /dev/watchdog and after clossing /dev/watchdog correctly)
> 2) pings 1/2 or 1/4 of the watchdog heartbeat time as long as the
> userspace ping keeps going. (Else it will expire and you trigger a 
> reboot).
> (Note: the heartbeat is the period after which a watchdog hardware
> device will trigger a reboot, timeout is the period for userspace.
> So for most devices heartbeat and timeout are the same, but for 
> devices where a timer is used (we also use it if the heartbeat value
> is too small (i.e. 1 second)) the values have a different meaning.)
> 
> So the logic for a watchdog device driver is:
> at startup the watchdog device driver should make sure that a reboot
> can only occur when /dev/watchdog is open. This means that if a
> watchdog device is active at boot it should either be stopped (that's
> why most drivers have a stop in their init/probe function) or have a 
> timer that pings the watchdog as long as it is not active for 
> userspace (like at91sam9_wdt.c, via_wdt.c, pika_wdt.c, ...)

My goal is different to those watchdog drivers. I want to use the
watchdog device to ensure that kernel and userspace work properly. If
one of them fails I want to reset the system and boot into a fallback
system. In this case, it doesn't make sense to ping the watchdog
through the kernel itself. This may work when the kernel is guaranteed
to boot successfully. But the kernel may as well never reach the
userspace because of some bug somewhere.

Of course it is possible to add this behaviour to a single driver that
is used on this specific hardware. But I think the concept of an
active watchdog during the whole boot process is not so uncommon. For
example if you want to update embedded systems, you may want to have a
fallback kernel and rootfs.

Best regards,

Markus

> 
> Kind regards,
> Wim.
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] watchdog: core: module param to activate watchdog

[Wim Van Sebroeck]

Hi Markus,

> Hi Wim,
> 
> On Sun, Mar 02, 2014 at 10:43:23AM +0100, Wim Van Sebroeck wrote:
> > Hi Markus,
> > 
> > > Many watchdog driver reset the watchdog device on initialization. This
> > > is a problem if the watchdog is activated by the bootloader and should
> > > be active the whole time until the userspace can write to it.
> > > 
> > > This patch adds a module parameter (watchdog.activate_first) that
> > > activates the first registered watchdog. Using this parameter it is
> > > possible to have an active watchdog during the whole boot process.
> > > 
> > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > 
> > NAK. It is the responsibility of the watchdog device driver to do this
> > and not that of the core. The core can't know which device(s) are 
> > running and can't be turned off. So you can have more then 1 device 
> > that needs to keep going...
> > 
> > The normal way do this is by using a timer. Take a look at 
> > at91sam9_wdt.c . We have a timer there that pings the watchdog if
> > 1) the watchdog device is not active for userspace (so before opening
> > /dev/watchdog and after clossing /dev/watchdog correctly)
> > 2) pings 1/2 or 1/4 of the watchdog heartbeat time as long as the
> > userspace ping keeps going. (Else it will expire and you trigger a 
> > reboot).
> > (Note: the heartbeat is the period after which a watchdog hardware
> > device will trigger a reboot, timeout is the period for userspace.
> > So for most devices heartbeat and timeout are the same, but for 
> > devices where a timer is used (we also use it if the heartbeat value
> > is too small (i.e. 1 second)) the values have a different meaning.)
> > 
> > So the logic for a watchdog device driver is:
> > at startup the watchdog device driver should make sure that a reboot
> > can only occur when /dev/watchdog is open. This means that if a
> > watchdog device is active at boot it should either be stopped (that's
> > why most drivers have a stop in their init/probe function) or have a 
> > timer that pings the watchdog as long as it is not active for 
> > userspace (like at91sam9_wdt.c, via_wdt.c, pika_wdt.c, ...)
> 
> My goal is different to those watchdog drivers. I want to use the
> watchdog device to ensure that kernel and userspace work properly. If
> one of them fails I want to reset the system and boot into a fallback
> system. In this case, it doesn't make sense to ping the watchdog
> through the kernel itself. This may work when the kernel is guaranteed
> to boot successfully. But the kernel may as well never reach the
> userspace because of some bug somewhere.
>
> Of course it is possible to add this behaviour to a single driver that
> is used on this specific hardware. But I think the concept of an
> active watchdog during the whole boot process is not so uncommon. For
> example if you want to update embedded systems, you may want to have a
> fallback kernel and rootfs.

The goal of watchdog device drivers is to reboot when your system get's 
unstable. So when userspace is "up" and the system has booted correctly.
What you want is something totaly different and not wanted for most 
systems because you are going to wreck filesystems: think about system
booting, getting rebooted, fsck starts, system gets rebooted during fsck,
... 

To achieve what you want you should make sure that your "BIOS" starts
with the watchdog enabled and with a very long watchdog heartbeat (so
that it doesn't need to get pinged). Then the kernel can boot and 
userspace can start and then you can do the necessary with the
/dev/watchdog functionality. Only thing you need to take care of is 
that you don't stop the watchdog (via a module param or something 
similar) during the probe/init of the driver.
I believe we allready have a winbond driver doing that. Just checked:
it's w83697hf_wdt.c with the early_disable module parameter.

So, this is something that needs to be taken into account during
probe/init of the driver and for each watchdog that needs it.
And thus not for the first watchdog that get's registered.
Furthermore: not all watchdog devices have a heartbeat that is long 
enough to achieve this functionality.

Kind regards,
Wim.

Re: [PATCH] watchdog: core: module param to activate watchdog

[Markus Pargmann]

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

Hi Wim,

On Mon, Mar 03, 2014 at 11:19:40AM +0100, Wim Van Sebroeck wrote:
> Hi Markus,
> 
> > Hi Wim,
> > 
> > On Sun, Mar 02, 2014 at 10:43:23AM +0100, Wim Van Sebroeck wrote:
> > > Hi Markus,
> > > 
> > > > Many watchdog driver reset the watchdog device on initialization. This
> > > > is a problem if the watchdog is activated by the bootloader and should
> > > > be active the whole time until the userspace can write to it.
> > > > 
> > > > This patch adds a module parameter (watchdog.activate_first) that
> > > > activates the first registered watchdog. Using this parameter it is
> > > > possible to have an active watchdog during the whole boot process.
> > > > 
> > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > 
> > > NAK. It is the responsibility of the watchdog device driver to do this
> > > and not that of the core. The core can't know which device(s) are 
> > > running and can't be turned off. So you can have more then 1 device 
> > > that needs to keep going...
> > > 
> > > The normal way do this is by using a timer. Take a look at 
> > > at91sam9_wdt.c . We have a timer there that pings the watchdog if
> > > 1) the watchdog device is not active for userspace (so before opening
> > > /dev/watchdog and after clossing /dev/watchdog correctly)
> > > 2) pings 1/2 or 1/4 of the watchdog heartbeat time as long as the
> > > userspace ping keeps going. (Else it will expire and you trigger a 
> > > reboot).
> > > (Note: the heartbeat is the period after which a watchdog hardware
> > > device will trigger a reboot, timeout is the period for userspace.
> > > So for most devices heartbeat and timeout are the same, but for 
> > > devices where a timer is used (we also use it if the heartbeat value
> > > is too small (i.e. 1 second)) the values have a different meaning.)
> > > 
> > > So the logic for a watchdog device driver is:
> > > at startup the watchdog device driver should make sure that a reboot
> > > can only occur when /dev/watchdog is open. This means that if a
> > > watchdog device is active at boot it should either be stopped (that's
> > > why most drivers have a stop in their init/probe function) or have a 
> > > timer that pings the watchdog as long as it is not active for 
> > > userspace (like at91sam9_wdt.c, via_wdt.c, pika_wdt.c, ...)
> > 
> > My goal is different to those watchdog drivers. I want to use the
> > watchdog device to ensure that kernel and userspace work properly. If
> > one of them fails I want to reset the system and boot into a fallback
> > system. In this case, it doesn't make sense to ping the watchdog
> > through the kernel itself. This may work when the kernel is guaranteed
> > to boot successfully. But the kernel may as well never reach the
> > userspace because of some bug somewhere.
> >
> > Of course it is possible to add this behaviour to a single driver that
> > is used on this specific hardware. But I think the concept of an
> > active watchdog during the whole boot process is not so uncommon. For
> > example if you want to update embedded systems, you may want to have a
> > fallback kernel and rootfs.
> 
> The goal of watchdog device drivers is to reboot when your system get's 
> unstable. So when userspace is "up" and the system has booted correctly.
> What you want is something totaly different and not wanted for most 
> systems because you are going to wreck filesystems: think about system
> booting, getting rebooted, fsck starts, system gets rebooted during fsck,
> ... 
> 
> To achieve what you want you should make sure that your "BIOS" starts
> with the watchdog enabled and with a very long watchdog heartbeat (so
> that it doesn't need to get pinged). Then the kernel can boot and 
> userspace can start and then you can do the necessary with the
> /dev/watchdog functionality. Only thing you need to take care of is 
> that you don't stop the watchdog (via a module param or something 
> similar) during the probe/init of the driver.
> I believe we allready have a winbond driver doing that. Just checked:
> it's w83697hf_wdt.c with the early_disable module parameter.
> 
> So, this is something that needs to be taken into account during
> probe/init of the driver and for each watchdog that needs it.
> And thus not for the first watchdog that get's registered.
> Furthermore: not all watchdog devices have a heartbeat that is long 
> enough to achieve this functionality.

Okay, thanks for the clarification. So I will change the driver and add
an appropriate paramter to it instead as the winbond driver does.

Thanks,

Markus


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] watchdog: core: module param to activate watchdog

[Guenter Roeck]

>> To achieve what you want you should make sure that your "BIOS" starts
>> with the watchdog enabled and with a very long watchdog heartbeat (so
>> that it doesn't need to get pinged). Then the kernel can boot and
>> userspace can start and then you can do the necessary with the
>> /dev/watchdog functionality. Only thing you need to take care of is
>> that you don't stop the watchdog (via a module param or something
>> similar) during the probe/init of the driver.
>> I believe we allready have a winbond driver doing that. Just checked:
>> it's w83697hf_wdt.c with the early_disable module parameter.
>>
>> So, this is something that needs to be taken into account during
>> probe/init of the driver and for each watchdog that needs it.
>> And thus not for the first watchdog that get's registered.
>> Furthermore: not all watchdog devices have a heartbeat that is long
>> enough to achieve this functionality.
>
> Okay, thanks for the clarification. So I will change the driver and add
> an appropriate paramter to it instead as the winbond driver does.
>

Other drivers do the same, but don't even have a module parameter to
disable it if it is already running. They just ping the watchdog during
initialization to delay the timeout. w83627hf_wdt.c is an example.

Guenter