[4/6] Watchdog: introdouce "pretimeout" into framework

Message ID 1431689090-3125-1-git-send-email-fu.wei@linaro.org
State New
Headers show

Commit Message

Fu Wei May 15, 2015, 11:24 a.m.
From: Fu Wei <fu.wei@linaro.org>

Reasons:
(1)kernel already has two watchdog drivers are using "pretimeout":
	drivers/char/ipmi/ipmi_watchdog.c
	drivers/watchdog/kempld_wdt.c(but the definition is different)
(2)some other dirvers are going to use this: ARM SBSA Generic Watchdog

Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 drivers/watchdog/watchdog_core.c | 66 ++++++++++++++++++++++++++++++++++++++++
 drivers/watchdog/watchdog_dev.c  | 48 +++++++++++++++++++++++++++++
 include/linux/watchdog.h         | 19 ++++++++++++
 3 files changed, 133 insertions(+)

Comments

Fu Wei May 15, 2015, 1:49 p.m. | #1
Hi Guenter,

Great thanks for your review,
feedback inline below :-)

On 15 May 2015 at 21:33, Guenter Roeck <linux@roeck-us.net> wrote:
> On 05/15/2015 04:24 AM, fu.wei@linaro.org wrote:
>>
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> Reasons:
>> (1)kernel already has two watchdog drivers are using "pretimeout":
>>         drivers/char/ipmi/ipmi_watchdog.c
>>         drivers/watchdog/kempld_wdt.c(but the definition is different)
>> (2)some other dirvers are going to use this: ARM SBSA Generic Watchdog
>>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> ---
>>   drivers/watchdog/watchdog_core.c | 66
>> ++++++++++++++++++++++++++++++++++++++++
>>   drivers/watchdog/watchdog_dev.c  | 48 +++++++++++++++++++++++++++++
>>   include/linux/watchdog.h         | 19 ++++++++++++
>>   3 files changed, 133 insertions(+)
>>
>> diff --git a/drivers/watchdog/watchdog_core.c
>> b/drivers/watchdog/watchdog_core.c
>> index cec9b55..6ca9578 100644
>> --- a/drivers/watchdog/watchdog_core.c
>> +++ b/drivers/watchdog/watchdog_core.c
>> @@ -54,6 +54,14 @@ static void watchdog_check_min_max_timeout(struct
>> watchdog_device *wdd)
>>                 wdd->min_timeout = 0;
>>                 wdd->max_timeout = 0;
>>         }
>> +       if (wdd->min_pretimeout && wdd->min_timeout < wdd->min_pretimeout)
>> {
>> +               pr_info("Invalid min timeout, resetting to min
>> pretimeout!\n");
>> +               wdd->min_timeout = wdd->min_pretimeout;
>> +       }
>> +       if (wdd->max_pretimeout && wdd->max_timeout < wdd->max_pretimeout)
>> {
>> +               pr_info("Invalid max timeout, resetting to max
>> pretimeout!\n");
>> +               wdd->max_timeout = wdd->max_pretimeout;
>> +       }
>
>
> I am a bit concerned about the context dependency introduced here. If
> someone calls
> _init_pretimeout after calling init_timeout, this may result in still
> invalid timeout
> values.

yes, that logic is not very clean, so my thought is :
maybe we can integrate watchdog_init_timeout and watchdog_init_pretimeout,
if maintainer agree to add pretimeout into framework.

>
>
>>   }
>>
>>   /**
>> @@ -98,6 +106,63 @@ int watchdog_init_timeout(struct watchdog_device *wdd,
>>   }
>>   EXPORT_SYMBOL_GPL(watchdog_init_timeout);
>>
>> +static void watchdog_check_min_max_pretimeout(struct watchdog_device
>> *wdd)
>> +{
>> +       /*
>> +        * Check that we have valid min and max pretimeout values, if
>> +        * not reset them both to 0 (=not used or unknown)
>> +        */
>> +       if (wdd->min_pretimeout > wdd->max_pretimeout) {
>> +               pr_info("Invalid min and max pretimeout, resetting to
>> 0!\n");
>> +               wdd->min_pretimeout = 0;
>> +               wdd->max_pretimeout = 0;
>> +       }
>> +}
>> +
>> +/**
>> + * watchdog_init_pretimeout() - initialize the pretimeout field
>> + * @pretimeout_parm: pretimeout module parameter
>> + * @dev: Device that stores the timeout-sec property
>> + *
>> + * Initialize the pretimeout field of the watchdog_device struct with
>> either
>> + * the pretimeout module parameter (if it is valid value) or the
>> timeout-sec
>> + * property (only if it is a valid value and the timeout_parm is out of
>> bounds).
>> + * If none of them are valid then we keep the old value (which should
>> normally
>> + * be the default pretimeout value.
>> + *
>> + * A zero is returned on success and -EINVAL for failure.
>> + */
>
>
> The new API function also needs to be documented in
> Documentation/watchdog/watchdog-kernel-api.txt.

good point, thanks will do

>
>> +int watchdog_init_pretimeout(struct watchdog_device *wdd,
>> +                            unsigned int pretimeout_parm, struct device
>> *dev)
>> +{
>> +       int ret = 0;
>> +       u32 timeouts[2];
>> +
>> +       watchdog_check_min_max_pretimeout(wdd);
>> +
>> +       /* try to get the timeout module parameter first */
>> +       if (!watchdog_pretimeout_invalid(wdd, pretimeout_parm) &&
>> +           pretimeout_parm) {
>> +               wdd->pretimeout = pretimeout_parm;
>> +               return ret;
>> +       }
>> +       if (pretimeout_parm)
>> +               ret = -EINVAL;
>> +
>> +       /* try to get the timeout_sec property */
>> +       if (!dev || !dev->of_node)
>> +               return ret;
>> +       ret = of_property_read_u32_array(dev->of_node,
>> +                                        "timeout-sec", timeouts, 2);
>> +       if (!watchdog_pretimeout_invalid(wdd, timeouts[1]) && timeouts[1])
>
>
> Guess we are still waiting for feedback from the devicetree maintainers on
> this.

yes!

>
> Both the above synchronization concern and this makes me wonder if we should
> introduce
> watchdog_init_timeouts() instead, which would take pretimeout as additional
> parameter.
> What do you think about that ?

yes, that is what I am thinking about

>
>
>> +               wdd->pretimeout = timeouts[1];
>> +       else
>> +               ret = -EINVAL;
>> +
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(watchdog_init_pretimeout);
>> +
>>   /**
>>    * watchdog_register_device() - register a watchdog device
>>    * @wdd: watchdog device
>> @@ -119,6 +184,7 @@ int watchdog_register_device(struct watchdog_device
>> *wdd)
>>         if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
>>                 return -EINVAL;
>>
>> +       watchdog_check_min_max_pretimeout(wdd);
>>         watchdog_check_min_max_timeout(wdd);
>>
>>         /*
>> diff --git a/drivers/watchdog/watchdog_dev.c
>> b/drivers/watchdog/watchdog_dev.c
>> index 6aaefba..b519257 100644
>> --- a/drivers/watchdog/watchdog_dev.c
>> +++ b/drivers/watchdog/watchdog_dev.c
>> @@ -218,6 +218,38 @@ out_timeout:
>>   }
>>
>>   /*
>> + *     watchdog_set_pretimeout: set the watchdog timer pretimeout
>> + *     @wddev: the watchdog device to set the timeout for
>> + *     @pretimeout: pretimeout to set in seconds
>> + */
>> +
>> +static int watchdog_set_pretimeout(struct watchdog_device *wddev,
>> +                                  unsigned int pretimeout)
>> +{
>> +       int err;
>> +
>> +       if (!wddev->ops->set_pretimeout ||
>> +           !(wddev->info->options & WDIOF_PRETIMEOUT))
>> +               return -EOPNOTSUPP;
>> +
>> +       if (watchdog_pretimeout_invalid(wddev, pretimeout))
>> +               return -EINVAL;
>> +
>> +       mutex_lock(&wddev->lock);
>> +
>> +       if (test_bit(WDOG_UNREGISTERED, &wddev->status)) {
>> +               err = -ENODEV;
>> +               goto out_pretimeout;
>> +       }
>> +
>> +       err = wddev->ops->set_pretimeout(wddev, pretimeout);
>> +
>> +out_pretimeout:
>> +       mutex_unlock(&wddev->lock);
>> +       return err;
>> +}
>> +
>> +/*
>>    *    watchdog_get_timeleft: wrapper to get the time left before a
>> reboot
>>    *    @wddev: the watchdog device to get the remaining time from
>>    *    @timeleft: the time that's left
>> @@ -388,6 +420,22 @@ static long watchdog_ioctl(struct file *file,
>> unsigned int cmd,
>>                 if (wdd->timeout == 0)
>>                         return -EOPNOTSUPP;
>>                 return put_user(wdd->timeout, p);
>> +       case WDIOC_SETPRETIMEOUT:
>> +               if (get_user(val, p))
>> +                       return -EFAULT;
>> +               err = watchdog_set_pretimeout(wdd, val);
>> +               if (err < 0)
>> +                       return err;
>> +               /* If the watchdog is active then we send a keepalive ping
>> +                * to make sure that the watchdog keep's running (and if
>
>
> s/keep's/keeps/

Great thanks, typo

>
>> +                * possible that it takes the new timeout) */
>
>
> Please use the common multi-line comment style (that it isn't used above is
> not
> an argument).

yes, my bad, will fixed .

>
>
>> +               watchdog_ping(wdd);
>> +               /* Fall */
>> +       case WDIOC_GETPRETIMEOUT:
>> +               /* timeout == 0 means that we don't use the pretimeout */
>> +               if (wdd->pretimeout == 0)
>> +                       return -EOPNOTSUPP;
>> +               return put_user(wdd->pretimeout, p);
>>         case WDIOC_GETTIMELEFT:
>>                 err = watchdog_get_timeleft(wdd, &val);
>>                 if (err)
>> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
>> index a746bf5..f0a3c5b 100644
>> --- a/include/linux/watchdog.h
>> +++ b/include/linux/watchdog.h
>> @@ -25,6 +25,7 @@ struct watchdog_device;
>>    * @ping:     The routine that sends a keepalive ping to the watchdog
>> device.
>>    * @status:   The routine that shows the status of the watchdog device.
>>    * @set_timeout:The routine for setting the watchdog devices timeout
>> value.
>> + * @set_pretimeout:The routine for setting the watchdog devices
>> pretimeout value
>>    * @get_timeleft:The routine that get's the time that's left before a
>> reset.
>>    * @ref:      The ref operation for dyn. allocated watchdog_device
>> structs
>>    * @unref:    The unref operation for dyn. allocated watchdog_device
>> structs
>> @@ -44,6 +45,7 @@ struct watchdog_ops {
>>         int (*ping)(struct watchdog_device *);
>>         unsigned int (*status)(struct watchdog_device *);
>>         int (*set_timeout)(struct watchdog_device *, unsigned int);
>> +       int (*set_pretimeout)(struct watchdog_device *, unsigned int);
>>         unsigned int (*get_timeleft)(struct watchdog_device *);
>>         void (*ref)(struct watchdog_device *);
>>         void (*unref)(struct watchdog_device *);
>> @@ -62,6 +64,9 @@ struct watchdog_ops {
>>    * @timeout:  The watchdog devices timeout value.
>>    * @min_timeout:The watchdog devices minimum timeout value.
>>    * @max_timeout:The watchdog devices maximum timeout value.
>> + * @pretimeout:        The watchdog devices pretimeout value.
>> + * @min_pretimeout:The watchdog devices minimum pretimeout value.
>> + * @max_pretimeout:The watchdog devices maximum pretimeout value.
>>    * @driver-data:Pointer to the drivers private data.
>>    * @lock:     Lock for watchdog core internal use only.
>>    * @status:   Field that contains the devices internal status bits.
>> @@ -86,6 +91,9 @@ struct watchdog_device {
>>         unsigned int timeout;
>>         unsigned int min_timeout;
>>         unsigned int max_timeout;
>> +       unsigned int pretimeout;
>> +       unsigned int min_pretimeout;
>> +       unsigned int max_pretimeout;
>>         void *driver_data;
>>         struct mutex lock;
>>         unsigned long status;
>> @@ -120,6 +128,14 @@ static inline bool watchdog_timeout_invalid(struct
>> watchdog_device *wdd, unsigne
>>                 (t < wdd->min_timeout || t > wdd->max_timeout));
>>   }
>>
>> +/* Use the following function to check if a pretimeout value is invalid
>> */
>> +static inline bool watchdog_pretimeout_invalid(struct watchdog_device
>> *wdd,
>> +                                              unsigned int t)
>> +{
>> +       return ((wdd->max_pretimeout != 0) &&
>> +               (t < wdd->min_pretimeout || t > wdd->max_pretimeout));
>> +}
>> +
>>   /* Use the following functions to manipulate watchdog driver specific
>> data */
>>   static inline void watchdog_set_drvdata(struct watchdog_device *wdd,
>> void *data)
>>   {
>> @@ -134,6 +150,9 @@ static inline void *watchdog_get_drvdata(struct
>> watchdog_device *wdd)
>>   /* drivers/watchdog/watchdog_core.c */
>>   extern int watchdog_init_timeout(struct watchdog_device *wdd,
>>                                   unsigned int timeout_parm, struct device
>> *dev);
>> +extern int watchdog_init_pretimeout(struct watchdog_device *wdd,
>> +                                   unsigned int pretimeout_parm,
>> +                                   struct device *dev);
>
>
> Please drop the 'extern'. Guess it may be time to clean up the watchdog core
> code ;-).

yes, you are right, but in different patchset ??

>
> Thanks,
> Guenter
>
Fu Wei May 18, 2015, 5:19 p.m. | #2
Hi Arnd,

Great thanks for your suggestion :-)

feedback inline below

On 15 May 2015 at 22:04, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 15 May 2015 19:24:48 fu.wei@linaro.org wrote:
>> +static void watchdog_check_min_max_pretimeout(struct watchdog_device *wdd)
>> +{
>> +       /*
>> +        * Check that we have valid min and max pretimeout values, if
>> +        * not reset them both to 0 (=not used or unknown)
>> +        */
>> +       if (wdd->min_pretimeout > wdd->max_pretimeout) {
>> +               pr_info("Invalid min and max pretimeout, resetting to 0!\n");
>> +               wdd->min_pretimeout = 0;
>> +               wdd->max_pretimeout = 0;
>> +       }
>> +}
>
> I would probably just fold this function into the existing
> watchdog_check_min_max_timeout() and check both normal and pre-timeout
> there.

yes, I can do that , and that is good idea

>
>> +/**
>> + * watchdog_init_pretimeout() - initialize the pretimeout field
>> + * @pretimeout_parm: pretimeout module parameter
>> + * @dev: Device that stores the timeout-sec property
>> + *
>> + * Initialize the pretimeout field of the watchdog_device struct with either
>> + * the pretimeout module parameter (if it is valid value) or the timeout-sec
>> + * property (only if it is a valid value and the timeout_parm is out of bounds).
>> + * If none of them are valid then we keep the old value (which should normally
>> + * be the default pretimeout value.
>> + *
>> + * A zero is returned on success and -EINVAL for failure.
>> + */
>> +int watchdog_init_pretimeout(struct watchdog_device *wdd,
>> +                            unsigned int pretimeout_parm, struct device *dev)
>> +{
>> +       int ret = 0;
>> +       u32 timeouts[2];
>> +
>> +       watchdog_check_min_max_pretimeout(wdd);
>> +
>> +       /* try to get the timeout module parameter first */
>> +       if (!watchdog_pretimeout_invalid(wdd, pretimeout_parm) &&
>> +           pretimeout_parm) {
>> +               wdd->pretimeout = pretimeout_parm;
>> +               return ret;
>> +       }
>> +       if (pretimeout_parm)
>> +               ret = -EINVAL;
>> +
>> +       /* try to get the timeout_sec property */
>> +       if (!dev || !dev->of_node)
>> +               return ret;
>> +       ret = of_property_read_u32_array(dev->of_node,
>> +                                        "timeout-sec", timeouts, 2);
>> +       if (!watchdog_pretimeout_invalid(wdd, timeouts[1]) && timeouts[1])
>> +               wdd->pretimeout = timeouts[1];
>> +       else
>> +               ret = -EINVAL;
>> +
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(watchdog_init_pretimeout);
>
> Same here: the function is very similar to the watchdog_init_timeout
> function, and it reads the same property, so just do both here.
>
> The easiest way for that is probably to use of_find_property()
> and of_prop_next_u32() to read the two numbers.

integrate watchdog_init_pretimeout and watchdog_init_timeout will be a
little hard,
we may need to change this API to :

watchdog_init_timeouts(struct watchdog_device *wdd, unsigned int timeout_parm,
                             unsigned int pretimeout_parm, struct device *dev)

then we need to update all the watchdog drivers which use this API,
maybe we can do this in a individual patchset, after this pretimeout
patch is merged.

Is that OK  ? :-)  any thought?


>
>         Arnd
>
Fu Wei May 18, 2015, 5:22 p.m. | #3
Hi Guenter,

yes, I think it is OK for me,

Once this patchset is merged, I will try to make a new patchset just
for this integration.

On 16 May 2015 at 02:01, Guenter Roeck <linux@roeck-us.net> wrote:
> On Fri, May 15, 2015 at 09:49:07PM +0800, Fu Wei wrote:
>> Hi Guenter,
>>
>> Great thanks for your review,
>> feedback inline below :-)
>>
>> On 15 May 2015 at 21:33, Guenter Roeck <linux@roeck-us.net> wrote:
>
> [ ... ]
>
>> >> +       if (wdd->max_pretimeout && wdd->max_timeout < wdd->max_pretimeout)
>> >> {
>> >> +               pr_info("Invalid max timeout, resetting to max
>> >> pretimeout!\n");
>> >> +               wdd->max_timeout = wdd->max_pretimeout;
>> >> +       }
>> >
>> >
>> > I am a bit concerned about the context dependency introduced here. If
>> > someone calls
>> > _init_pretimeout after calling init_timeout, this may result in still
>> > invalid timeout
>> > values.
>>
>> yes, that logic is not very clean, so my thought is :
>> maybe we can integrate watchdog_init_timeout and watchdog_init_pretimeout,
>> if maintainer agree to add pretimeout into framework.
>>
> I think we should just assume that Wim will accept it, and try to find
> the best possible solution (or at least a good one).
>
> Guenter
Fu Wei May 18, 2015, 5:39 p.m. | #4
Hi Guenter,

np, will do so  :-)

On 19 May 2015 at 01:23, Guenter Roeck <linux@roeck-us.net> wrote:
> On Tue, May 19, 2015 at 01:19:22AM +0800, Fu Wei wrote:
>> Hi Arnd,
>>
>> Great thanks for your suggestion :-)
>>
>> feedback inline below
>>
>> On 15 May 2015 at 22:04, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Friday 15 May 2015 19:24:48 fu.wei@linaro.org wrote:
>> >> +static void watchdog_check_min_max_pretimeout(struct watchdog_device *wdd)
>> >> +{
>> >> +       /*
>> >> +        * Check that we have valid min and max pretimeout values, if
>> >> +        * not reset them both to 0 (=not used or unknown)
>> >> +        */
>> >> +       if (wdd->min_pretimeout > wdd->max_pretimeout) {
>> >> +               pr_info("Invalid min and max pretimeout, resetting to 0!\n");
>> >> +               wdd->min_pretimeout = 0;
>> >> +               wdd->max_pretimeout = 0;
>> >> +       }
>> >> +}
>> >
>> > I would probably just fold this function into the existing
>> > watchdog_check_min_max_timeout() and check both normal and pre-timeout
>> > there.
>>
>> yes, I can do that , and that is good idea
>>
>> >
>> >> +/**
>> >> + * watchdog_init_pretimeout() - initialize the pretimeout field
>> >> + * @pretimeout_parm: pretimeout module parameter
>> >> + * @dev: Device that stores the timeout-sec property
>> >> + *
>> >> + * Initialize the pretimeout field of the watchdog_device struct with either
>> >> + * the pretimeout module parameter (if it is valid value) or the timeout-sec
>> >> + * property (only if it is a valid value and the timeout_parm is out of bounds).
>> >> + * If none of them are valid then we keep the old value (which should normally
>> >> + * be the default pretimeout value.
>> >> + *
>> >> + * A zero is returned on success and -EINVAL for failure.
>> >> + */
>> >> +int watchdog_init_pretimeout(struct watchdog_device *wdd,
>> >> +                            unsigned int pretimeout_parm, struct device *dev)
>> >> +{
>> >> +       int ret = 0;
>> >> +       u32 timeouts[2];
>> >> +
>> >> +       watchdog_check_min_max_pretimeout(wdd);
>> >> +
>> >> +       /* try to get the timeout module parameter first */
>> >> +       if (!watchdog_pretimeout_invalid(wdd, pretimeout_parm) &&
>> >> +           pretimeout_parm) {
>> >> +               wdd->pretimeout = pretimeout_parm;
>> >> +               return ret;
>> >> +       }
>> >> +       if (pretimeout_parm)
>> >> +               ret = -EINVAL;
>> >> +
>> >> +       /* try to get the timeout_sec property */
>> >> +       if (!dev || !dev->of_node)
>> >> +               return ret;
>> >> +       ret = of_property_read_u32_array(dev->of_node,
>> >> +                                        "timeout-sec", timeouts, 2);
>> >> +       if (!watchdog_pretimeout_invalid(wdd, timeouts[1]) && timeouts[1])
>> >> +               wdd->pretimeout = timeouts[1];
>> >> +       else
>> >> +               ret = -EINVAL;
>> >> +
>> >> +       return ret;
>> >> +}
>> >> +EXPORT_SYMBOL_GPL(watchdog_init_pretimeout);
>> >
>> > Same here: the function is very similar to the watchdog_init_timeout
>> > function, and it reads the same property, so just do both here.
>> >
>> > The easiest way for that is probably to use of_find_property()
>> > and of_prop_next_u32() to read the two numbers.
>>
>> integrate watchdog_init_pretimeout and watchdog_init_timeout will be a
>> little hard,
>> we may need to change this API to :
>>
>> watchdog_init_timeouts(struct watchdog_device *wdd, unsigned int timeout_parm,
>>                              unsigned int pretimeout_parm, struct device *dev)
>>
>> then we need to update all the watchdog drivers which use this API,
>> maybe we can do this in a individual patchset, after this pretimeout
>> patch is merged.
>>
>> Is that OK  ? :-)  any thought?
>>
> That is what I would recommend.
>
> Guenter
Fu Wei May 19, 2015, 1:12 a.m. | #5
Hi Arnd, Guenter,

yes, that is brilliant idea!!
I will try to do so , that solve the compatibility problem , so I
guess we can try this time :-)

On 19 May 2015 at 04:14, Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, May 18, 2015 at 10:03:52PM +0200, Arnd Bergmann wrote:
>> On Monday 18 May 2015 10:23:30 Guenter Roeck wrote:
>> > >
>> > > integrate watchdog_init_pretimeout and watchdog_init_timeout will be a
>> > > little hard,
>> > > we may need to change this API to :
>> > >
>> > > watchdog_init_timeouts(struct watchdog_device *wdd, unsigned int timeout_parm,
>> > >                              unsigned int pretimeout_parm, struct device *dev)
>> > >
>> > > then we need to update all the watchdog drivers which use this API,
>> > > maybe we can do this in a individual patchset, after this pretimeout
>> > > patch is merged.
>> > >
>> > > Is that OK  ?   any thought?
>> > >
>> > That is what I would recommend.
>> >
>>
>> The API change is fine, but I don't think you need to change all drivers.
>>
>> Just add a small wrapper function in the header file doing the conversion:
>>
>> static inline int watchdog_init_timeout(struct watchdog_device *wdd,
>>                                 unsigned int timeout_parm, struct device *dev)
>> {
>>       return watchdog_init_timeouts(wdd, timeout_parm, ~0ul, dev);
>> }
>>
>> Then you can update the drivers that actually use the pretimeout to
>> use the new function at some point, and leave all other drivers calling
>> the wrapper function.
>>
> Excellent idea.
>
> Guenter

Patch hide | download patch | download mbox

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index cec9b55..6ca9578 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -54,6 +54,14 @@  static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
 		wdd->min_timeout = 0;
 		wdd->max_timeout = 0;
 	}
+	if (wdd->min_pretimeout && wdd->min_timeout < wdd->min_pretimeout) {
+		pr_info("Invalid min timeout, resetting to min pretimeout!\n");
+		wdd->min_timeout = wdd->min_pretimeout;
+	}
+	if (wdd->max_pretimeout && wdd->max_timeout < wdd->max_pretimeout) {
+		pr_info("Invalid max timeout, resetting to max pretimeout!\n");
+		wdd->max_timeout = wdd->max_pretimeout;
+	}
 }
 
 /**
@@ -98,6 +106,63 @@  int watchdog_init_timeout(struct watchdog_device *wdd,
 }
 EXPORT_SYMBOL_GPL(watchdog_init_timeout);
 
+static void watchdog_check_min_max_pretimeout(struct watchdog_device *wdd)
+{
+	/*
+	 * Check that we have valid min and max pretimeout values, if
+	 * not reset them both to 0 (=not used or unknown)
+	 */
+	if (wdd->min_pretimeout > wdd->max_pretimeout) {
+		pr_info("Invalid min and max pretimeout, resetting to 0!\n");
+		wdd->min_pretimeout = 0;
+		wdd->max_pretimeout = 0;
+	}
+}
+
+/**
+ * watchdog_init_pretimeout() - initialize the pretimeout field
+ * @pretimeout_parm: pretimeout module parameter
+ * @dev: Device that stores the timeout-sec property
+ *
+ * Initialize the pretimeout field of the watchdog_device struct with either
+ * the pretimeout module parameter (if it is valid value) or the timeout-sec
+ * property (only if it is a valid value and the timeout_parm is out of bounds).
+ * If none of them are valid then we keep the old value (which should normally
+ * be the default pretimeout value.
+ *
+ * A zero is returned on success and -EINVAL for failure.
+ */
+int watchdog_init_pretimeout(struct watchdog_device *wdd,
+			     unsigned int pretimeout_parm, struct device *dev)
+{
+	int ret = 0;
+	u32 timeouts[2];
+
+	watchdog_check_min_max_pretimeout(wdd);
+
+	/* try to get the timeout module parameter first */
+	if (!watchdog_pretimeout_invalid(wdd, pretimeout_parm) &&
+	    pretimeout_parm) {
+		wdd->pretimeout = pretimeout_parm;
+		return ret;
+	}
+	if (pretimeout_parm)
+		ret = -EINVAL;
+
+	/* try to get the timeout_sec property */
+	if (!dev || !dev->of_node)
+		return ret;
+	ret = of_property_read_u32_array(dev->of_node,
+					 "timeout-sec", timeouts, 2);
+	if (!watchdog_pretimeout_invalid(wdd, timeouts[1]) && timeouts[1])
+		wdd->pretimeout = timeouts[1];
+	else
+		ret = -EINVAL;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(watchdog_init_pretimeout);
+
 /**
  * watchdog_register_device() - register a watchdog device
  * @wdd: watchdog device
@@ -119,6 +184,7 @@  int watchdog_register_device(struct watchdog_device *wdd)
 	if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
 		return -EINVAL;
 
+	watchdog_check_min_max_pretimeout(wdd);
 	watchdog_check_min_max_timeout(wdd);
 
 	/*
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 6aaefba..b519257 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -218,6 +218,38 @@  out_timeout:
 }
 
 /*
+ *	watchdog_set_pretimeout: set the watchdog timer pretimeout
+ *	@wddev: the watchdog device to set the timeout for
+ *	@pretimeout: pretimeout to set in seconds
+ */
+
+static int watchdog_set_pretimeout(struct watchdog_device *wddev,
+				   unsigned int pretimeout)
+{
+	int err;
+
+	if (!wddev->ops->set_pretimeout ||
+	    !(wddev->info->options & WDIOF_PRETIMEOUT))
+		return -EOPNOTSUPP;
+
+	if (watchdog_pretimeout_invalid(wddev, pretimeout))
+		return -EINVAL;
+
+	mutex_lock(&wddev->lock);
+
+	if (test_bit(WDOG_UNREGISTERED, &wddev->status)) {
+		err = -ENODEV;
+		goto out_pretimeout;
+	}
+
+	err = wddev->ops->set_pretimeout(wddev, pretimeout);
+
+out_pretimeout:
+	mutex_unlock(&wddev->lock);
+	return err;
+}
+
+/*
  *	watchdog_get_timeleft: wrapper to get the time left before a reboot
  *	@wddev: the watchdog device to get the remaining time from
  *	@timeleft: the time that's left
@@ -388,6 +420,22 @@  static long watchdog_ioctl(struct file *file, unsigned int cmd,
 		if (wdd->timeout == 0)
 			return -EOPNOTSUPP;
 		return put_user(wdd->timeout, p);
+	case WDIOC_SETPRETIMEOUT:
+		if (get_user(val, p))
+			return -EFAULT;
+		err = watchdog_set_pretimeout(wdd, val);
+		if (err < 0)
+			return err;
+		/* If the watchdog is active then we send a keepalive ping
+		 * to make sure that the watchdog keep's running (and if
+		 * possible that it takes the new timeout) */
+		watchdog_ping(wdd);
+		/* Fall */
+	case WDIOC_GETPRETIMEOUT:
+		/* timeout == 0 means that we don't use the pretimeout */
+		if (wdd->pretimeout == 0)
+			return -EOPNOTSUPP;
+		return put_user(wdd->pretimeout, p);
 	case WDIOC_GETTIMELEFT:
 		err = watchdog_get_timeleft(wdd, &val);
 		if (err)
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index a746bf5..f0a3c5b 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -25,6 +25,7 @@  struct watchdog_device;
  * @ping:	The routine that sends a keepalive ping to the watchdog device.
  * @status:	The routine that shows the status of the watchdog device.
  * @set_timeout:The routine for setting the watchdog devices timeout value.
+ * @set_pretimeout:The routine for setting the watchdog devices pretimeout value
  * @get_timeleft:The routine that get's the time that's left before a reset.
  * @ref:	The ref operation for dyn. allocated watchdog_device structs
  * @unref:	The unref operation for dyn. allocated watchdog_device structs
@@ -44,6 +45,7 @@  struct watchdog_ops {
 	int (*ping)(struct watchdog_device *);
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
+	int (*set_pretimeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
 	void (*ref)(struct watchdog_device *);
 	void (*unref)(struct watchdog_device *);
@@ -62,6 +64,9 @@  struct watchdog_ops {
  * @timeout:	The watchdog devices timeout value.
  * @min_timeout:The watchdog devices minimum timeout value.
  * @max_timeout:The watchdog devices maximum timeout value.
+ * @pretimeout:	The watchdog devices pretimeout value.
+ * @min_pretimeout:The watchdog devices minimum pretimeout value.
+ * @max_pretimeout:The watchdog devices maximum pretimeout value.
  * @driver-data:Pointer to the drivers private data.
  * @lock:	Lock for watchdog core internal use only.
  * @status:	Field that contains the devices internal status bits.
@@ -86,6 +91,9 @@  struct watchdog_device {
 	unsigned int timeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
+	unsigned int pretimeout;
+	unsigned int min_pretimeout;
+	unsigned int max_pretimeout;
 	void *driver_data;
 	struct mutex lock;
 	unsigned long status;
@@ -120,6 +128,14 @@  static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigne
 		(t < wdd->min_timeout || t > wdd->max_timeout));
 }
 
+/* Use the following function to check if a pretimeout value is invalid */
+static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd,
+					       unsigned int t)
+{
+	return ((wdd->max_pretimeout != 0) &&
+		(t < wdd->min_pretimeout || t > wdd->max_pretimeout));
+}
+
 /* Use the following functions to manipulate watchdog driver specific data */
 static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
 {
@@ -134,6 +150,9 @@  static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 /* drivers/watchdog/watchdog_core.c */
 extern int watchdog_init_timeout(struct watchdog_device *wdd,
 				  unsigned int timeout_parm, struct device *dev);
+extern int watchdog_init_pretimeout(struct watchdog_device *wdd,
+				    unsigned int pretimeout_parm,
+				    struct device *dev);
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);