[v5,4/8] Watchdog: introdouce "pretimeout" into framework

Message ID 1433943713-32466-5-git-send-email-fu.wei@linaro.org
State New
Headers show

Commit Message

Fu Wei June 10, 2015, 1:41 p.m.
From: Fu Wei <fu.wei@linaro.org>

Also update Documentation/watchdog/watchdog-kernel-api.txt to
introduce:
(1)the new elements in the watchdog_device and watchdog_ops struct;
(2)the new API "watchdog_init_timeouts"

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 drivers are going to use this: ARM SBSA Generic Watchdog

Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 Documentation/watchdog/watchdog-kernel-api.txt |  47 ++++++++++--
 drivers/watchdog/watchdog_core.c               | 100 +++++++++++++++++--------
 drivers/watchdog/watchdog_dev.c                |  53 +++++++++++++
 include/linux/watchdog.h                       |  36 ++++++++-
 4 files changed, 197 insertions(+), 39 deletions(-)

Comments

Fu Wei June 11, 2015, 11:22 a.m. | #1
Hi Guenter,

Great thanks for your time.

On 11 June 2015 at 00:21, Guenter Roeck <linux@roeck-us.net> wrote:
> On 06/10/2015 06:41 AM, fu.wei@linaro.org wrote:
>>
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> Also update Documentation/watchdog/watchdog-kernel-api.txt to
>> introduce:
>> (1)the new elements in the watchdog_device and watchdog_ops struct;
>> (2)the new API "watchdog_init_timeouts"
>>
>> 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 drivers are going to use this: ARM SBSA Generic Watchdog
>>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> ---
>>   Documentation/watchdog/watchdog-kernel-api.txt |  47 ++++++++++--
>>   drivers/watchdog/watchdog_core.c               | 100
>> +++++++++++++++++--------
>>   drivers/watchdog/watchdog_dev.c                |  53 +++++++++++++
>>   include/linux/watchdog.h                       |  36 ++++++++-
>>   4 files changed, 197 insertions(+), 39 deletions(-)
>>
>> diff --git a/Documentation/watchdog/watchdog-kernel-api.txt
>> b/Documentation/watchdog/watchdog-kernel-api.txt
>> index a0438f3..95b355d 100644
>> --- a/Documentation/watchdog/watchdog-kernel-api.txt
>> +++ b/Documentation/watchdog/watchdog-kernel-api.txt
>> @@ -49,6 +49,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;
>> @@ -70,6 +73,9 @@ It contains following fields:
>>   * timeout: the watchdog timer's timeout value (in seconds).
>>   * min_timeout: the watchdog timer's minimum timeout value (in seconds).
>>   * max_timeout: the watchdog timer's maximum timeout value (in seconds).
>> +* pretimeout: the watchdog timer's pretimeout value (in seconds).
>> +* min_pretimeout: the watchdog timer's minimum pretimeout value (in
>> seconds).
>> +* max_pretimeout: the watchdog timer's maximum pretimeout value (in
>> seconds).
>>   * bootstatus: status of the device after booting (reported with watchdog
>>     WDIOF_* status bits).
>>   * driver_data: a pointer to the drivers private data of a watchdog
>> device.
>> @@ -92,6 +98,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 *);
>> @@ -153,9 +160,19 @@ they are supported. These optional
>> routines/operations are:
>>     and -EIO for "could not write value to the watchdog". On success this
>>     routine should set the timeout value of the watchdog_device to the
>>     achieved timeout value (which may be different from the requested one
>> -  because the watchdog does not necessarily has a 1 second resolution).
>> +  because the watchdog does not necessarily has a 1 second resolution;
>> +  If the driver supports pretimeout, then the timeout value must be
>> greater
>> +  than that).
>>     (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of
>> the
>>     watchdog's info structure).
>> +* set_pretimeout: this routine checks and changes the pretimeout of the
>> +  watchdog timer device. It returns 0 on success, -EINVAL for "parameter
>> out of
>> +  range" and -EIO for "could not write value to the watchdog". On success
>> this
>> +  routine should set the pretimeout value of the watchdog_device to the
>> +  achieved pretimeout value (which may be different from the requested
>> one
>> +  because the watchdog does not necessarily has a 1 second resolution).
>> +  (Note: the WDIOF_PRETIMEOUT needs to be set in the options field of the
>> +  watchdog's info structure).
>>   * get_timeleft: this routines returns the time that's left before a
>> reset.
>>   * ref: the operation that calls kref_get on the kref of a dynamically
>>     allocated watchdog_device struct.
>> @@ -219,8 +236,28 @@ extern int watchdog_init_timeout(struct
>> watchdog_device *wdd,
>>                                     unsigned int timeout_parm, struct
>> device *dev);
>>
>>   The watchdog_init_timeout function allows you to initialize the timeout
>> field
>> -using the module timeout parameter or by retrieving the timeout-sec
>> property from
>> -the device tree (if the module timeout parameter is invalid). Best
>> practice is
>> -to set the default timeout value as timeout value in the watchdog_device
>> and
>> -then use this function to set the user "preferred" timeout value.
>> +using the module timeout parameter or by retrieving the first element of
>> +the timeout-sec property from the device tree (if the module timeout
>> parameter
>
>
> Missing space before (.
>
>> +is invalid). Best practice is to set the default timeout value as timeout
>> value
>> +in the watchdog_device and then use this function to set the user
>> "preferred"
>> +timeout value.
>> +This routine returns zero on success and a negative errno code for
>> failure.
>> +
>> +Some watchdog timers have two stage of timeouts(timeout and pretimeout),
>> +to initialize the timeout and pretimeout fields at the same time, the
>> following
>> +function can be used:
>> +
>> +extern int watchdog_init_timeouts(struct watchdog_device *wdd,
>> +                                  unsigned int pretimeout_parm,
>> +                                  unsigned int timeout_parm,
>> +                                  struct device *dev);
>> +
>> +The watchdog_init_timeouts function allows you to initialize the
>> pretimeout and
>> +timeout fields using the module pretimeout and timeout parameter or by
>> +retrieving the elements in the timeout-sec property(the first element is
>> for
>> +timeout, the second one is for pretimeout) from the device tree(if the
>> module
>> +pretimeout and timeout parameter are invalid).
>
>
> Missing spaces before (.

Fixed these problem above , thanks

>
>> +Best practice is to set the default pretimeout and timeout value as
>> pretimeout
>> +and timeout value in the watchdog_device and then use this function to
>> set the
>> +user "preferred" pretimeout value.
>>   This routine returns zero on success and a negative errno code for
>> failure.
>> diff --git a/drivers/watchdog/watchdog_core.c
>> b/drivers/watchdog/watchdog_core.c
>> index cec9b55..bdd4e43 100644
>> --- a/drivers/watchdog/watchdog_core.c
>> +++ b/drivers/watchdog/watchdog_core.c
>> @@ -43,60 +43,98 @@
>>   static DEFINE_IDA(watchdog_ida);
>>   static struct class *watchdog_class;
>>
>> -static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
>> +static void watchdog_check_min_max_timeouts(struct watchdog_device *wdd)
>
>
> Please don't rename this function. This avoids conflicts with other pending
> patches.

OK, changed it back.  Fixed.

>
>>   {
>>         /*
>> -        * Check that we have valid min and max timeout values, if
>> -        * not reset them both to 0 (=not used or unknown)
>> +        * Check that we have valid min and max pretimeout and timeout
>> values,
>> +        * if not, reset them all to 0 (=not used or unknown)
>>          */
>> -       if (wdd->min_timeout > wdd->max_timeout) {
>> -               pr_info("Invalid min and max timeout values, resetting to
>> 0!\n");
>> +       if (wdd->min_pretimeout > wdd->max_pretimeout ||
>> +           wdd->min_timeout > wdd->max_timeout ||
>> +           wdd->min_timeout < wdd->min_pretimeout ||
>> +           wdd->max_timeout < wdd->max_pretimeout) {
>> +               pr_info("Invalid min and max timeouts, resetting to 0\n");
>
>
> timeouts or pretimeouts.

OK , thanks

>
>
>> +               wdd->min_pretimeout = 0;
>> +               wdd->max_pretimeout = 0;
>>                 wdd->min_timeout = 0;
>>                 wdd->max_timeout = 0;
>>         }
>>   }
>>
>>   /**
>> - * watchdog_init_timeout() - initialize the timeout field
>> + * watchdog_init_timeouts() - initialize the pretimeout and timeout field
>> + * @pretimeout_parm: pretimeout module parameter
>>    * @timeout_parm: timeout module parameter
>>    * @dev: Device that stores the timeout-sec property
>>    *
>> - * Initialize the timeout field of the watchdog_device struct with either
>> the
>> - * timeout 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 timeout value.
>> + * Initialize the pretimeout and timeout field of the watchdog_device
>> struct
>> + * with either the pretimeout and timeout module parameter (if it is
>> valid) or
>> + * the timeout-sec property (only if it is valid and the pretimeout_parm
>> or
>> + * timeout_parm is out of bounds). If none of them is valid, then we keep
>> + * the old value (which should normally be the default timeout value).
>>    *
>>    * A zero is returned on success and -EINVAL for failure.
>>    */
>> -int watchdog_init_timeout(struct watchdog_device *wdd,
>> -                               unsigned int timeout_parm, struct device
>> *dev)
>> +int watchdog_init_timeouts(struct watchdog_device *wdd,
>> +                          unsigned int pretimeout_parm,
>> +                          unsigned int timeout_parm,
>> +                          struct device *dev)
>>   {
>> -       unsigned int t = 0;
>> -       int ret = 0;
>> +       int ret = 0, length = 0;
>> +       u32 timeouts[2] = {0};
>> +       struct property *prop;
>>
>> -       watchdog_check_min_max_timeout(wdd);
>> +       watchdog_check_min_max_timeouts(wdd);
>>
>> -       /* try to get the timeout module parameter first */
>> -       if (!watchdog_timeout_invalid(wdd, timeout_parm) && timeout_parm)
>> {
>> -               wdd->timeout = timeout_parm;
>> -               return ret;
>> -       }
>> -       if (timeout_parm)
>> +       /*
>> +        * Try to get the pretimeout module parameter first
>> +        */
>> +       if (!watchdog_pretimeout_invalid(wdd, pretimeout_parm))
>> +               timeouts[1] = pretimeout_parm;
>> +       else
>> +               ret = -EINVAL; /* pretimeout_parm is invalid */
>> +
>
> If I understand the code correctly, this permits for pretimeout being
> specified
> as module parameter and timeout as devicetree property. I don't think this
> is a good idea. Use either one or the other, but not both. So this can be
> simplified to
>
>         if (watchdog_pretimeout_invalid(wdd, pretimeout_parm)
>                 ret = _EINVAL;
>
> if you use pretimeout directly below.
>
>> +       /*
>> +        * Try to get the timeout module parameter,
>> +        * if it's valid and pretimeout is valid(ret == 0),
>> +        * assignment and return zero. Otherwise, try dtb.
>> +        */
>> +       if (timeout_parm) {
>
>
> You can use
>         if (timeout_parm && !ret) {
>
>> +               if (!watchdog_timeout_invalid(wdd, timeout_parm) && !ret)
>> {
>
>
> instead of checking ret here.
>
>> +                       wdd->timeout = timeout_parm;
>> +                       wdd->pretimeout = timeouts[1];
>
>
> This is really pretimeout_parm here, which should be used.
> Then you don't need timeouts[1].

yes, you are right on this,  I have improved it  :-)

>
>> +                       return 0;
>> +               }
>>                 ret = -EINVAL;
>> +       }
>>
>
> Unfortunately this does not set wdd->pretimeout if pretimeout_parm
> is set but timeout_parm isn't. So I think you need
>
>         } else if (pretimeout_parm && !ret) {
>                 wdd->pretimeout = pretimeout;
>                 return 0;
>         }
>
> However, that actually won't work as we may try to set both timeout
> and pretimeout, but the current checks validate against the previous
> values. I'll need to think about that.
>
> Also, this is getting a bit complicated. Wonder if the code can be
> simplified.
> Something else to think about.

I think we don't need this, because only if both parameters are valid,
we can update the  wdd->pretimeout and wdd->timeout.
otherwise we ret = -EINVAL; then try devicetree.

>
>
>> -       /* try to get the timeout_sec property */
>> +       /*
>> +        * Either at least one of the module parameters is invalid,
>> +        * or timeout_parm is 0. Try to get the timeout_sec property.
>> +        */
>>         if (dev == NULL || dev->of_node == NULL)
>>                 return ret;
>> -       of_property_read_u32(dev->of_node, "timeout-sec", &t);
>> -       if (!watchdog_timeout_invalid(wdd, t) && t)
>> -               wdd->timeout = t;
>> -       else
>> -               ret = -EINVAL;
>>
>> -       return ret;
>> +       prop = of_find_property(dev->of_node, "timeout-sec", &length);
>> +       if (prop && length > 0 && length <= sizeof(u32) * 2) {
>> +               of_property_read_u32_array(dev->of_node,
>> +                                          "timeout-sec", timeouts,
>> +                                          length / sizeof(u32));
>> +               if (length == sizeof(u32) * 2 &&
>> +                   watchdog_pretimeout_invalid(wdd, timeouts[1]))
>> +                               return -EINVAL;
>> +
>> +               if (!watchdog_timeout_invalid(wdd, timeouts[0]) &&
>> +                   timeouts[0]) {
>> +                       wdd->timeout = timeouts[0];
>> +                       wdd->pretimeout = timeouts[1];
>
>
> Please only set pretimeout here if specified in devicetree.

yes , fixed this problem

>
>
>> +                       return 0;
>> +               }
>> +       }
>> +
>> +       return -EINVAL;
>>   }
>> -EXPORT_SYMBOL_GPL(watchdog_init_timeout);
>> +EXPORT_SYMBOL_GPL(watchdog_init_timeouts);
>>
>>   /**
>>    * watchdog_register_device() - register a watchdog device
>> @@ -119,7 +157,7 @@ int watchdog_register_device(struct watchdog_device
>> *wdd)
>>         if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
>>                 return -EINVAL;
>>
>> -       watchdog_check_min_max_timeout(wdd);
>> +       watchdog_check_min_max_timeouts(wdd);
>>
>>         /*
>>          * Note: now that all watchdog_device data has been verified, we
>> diff --git a/drivers/watchdog/watchdog_dev.c
>> b/drivers/watchdog/watchdog_dev.c
>> index 6aaefba..af0777e 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,27 @@ 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:
>> +               /* check if we support the pretimeout */
>> +               if (!(wdd->info->options & WDIOF_PRETIMEOUT))
>> +                       return -EOPNOTSUPP;
>> +               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 keeps running (and if
>> +                * possible that it takes the new pretimeout)
>> +                */
>> +               watchdog_ping(wdd);
>> +               /* Fall */
>> +       case WDIOC_GETPRETIMEOUT:
>> +               /* check if we support the pretimeout */
>> +               if (wdd->info->options & WDIOF_PRETIMEOUT)
>> +                       return put_user(wdd->pretimeout, p);
>> +               return -EOPNOTSUPP;
>>         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..0a7acf0 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;
>> @@ -117,7 +125,20 @@ static inline void watchdog_set_nowayout(struct
>> watchdog_device *wdd, bool noway
>>   static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd,
>> unsigned int t)
>>   {
>>         return ((wdd->max_timeout != 0) &&
>> -               (t < wdd->min_timeout || t > wdd->max_timeout));
>> +               (t < wdd->min_timeout || t > wdd->max_timeout ||
>> +                       t <= wdd->pretimeout));
>
>
> pretimeout can be 0, and t needs to be evaluated against pretimeout even if
> max_timeout is 0.
>
> Try
>         return (wdd->max_timeout &&
>                 (t < wdd->min_timeout || t > wdd->max_timeout)) ||
>                (wdd->pretimeout && t <= wdd->pretimeout);

yes, you are right . fixed.

>
>> +}
>> +
>> +/*
>> + * Use the following function to check if a pretimeout value is invalid.
>> + * It can be "0", that means we don't use pretimeout.
>> + */
>> +static inline bool watchdog_pretimeout_invalid(struct watchdog_device
>> *wdd,
>> +                                              unsigned int t)
>> +{
>> +       return (wdd->pretimeout != 0 && wdd->max_pretimeout != 0 &&
>> +               (t < wdd->min_pretimeout || t > wdd->max_pretimeout ||
>> +                       (wdd->timeout != 0 && t >= wdd->timeout)));
>
>
> The check for wdd->pretimeout doesn't make much sense here, as this is the

sorry , that is typo, I mean "t" here.

if t == 0 , return 0. because 0 is a valid value of pretimeout.

> value we are trying to set. Effectively the above accepts every pretimeout
> if wdd->pretimeout is 0. It also accepts every pretimeout if
> max_pretimeout == 0, even if wdd->timeout is set and t >= wdd->timeout.
>
> Try
>
>         return (wdd->max_pretimeout && (t < wdd->min_pretimeout ||
>                                         t > wdd->max_pretimeout)) ||
>                 (wdd->timeout && t >= wdd->timeout);
>
>>   }

/*
 * Use the following function to check if a pretimeout value is invalid.
 * It can be "0", that means we don't use pretimeout.
 * This function returns 0, when pretimeout is 0.
 */
static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd,
                           unsigned int t)
{
    return t && (wdd->max_pretimeout &&
        (t < wdd->min_pretimeout || t > wdd->max_pretimeout)) ||
           (wdd->timeout && t >= wdd->timeout);
}


>>
>>   /* Use the following functions to manipulate watchdog driver specific
>> data */
>> @@ -132,11 +153,20 @@ 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);
>> +int watchdog_init_timeouts(struct watchdog_device *wdd,
>> +                                 unsigned int pretimeout_parm,
>> +                                 unsigned int timeout_parm,
>> +                                 struct device *dev);
>
>
> Please align continuation lines with '('.

Fixed , thanks

>
>
>>   extern int watchdog_register_device(struct watchdog_device *);
>>   extern void watchdog_unregister_device(struct watchdog_device *);
>>
>> +static inline int watchdog_init_timeout(struct watchdog_device *wdd,
>> +                                       unsigned int timeout_parm,
>> +                                       struct device *dev)
>> +{
>> +       return watchdog_init_timeouts(wdd, 0, timeout_parm, dev);
>> +}
>> +
>>   #ifdef CONFIG_HARDLOCKUP_DETECTOR
>>   void watchdog_nmi_disable_all(void);
>>   void watchdog_nmi_enable_all(void);
>>
>

Patch hide | download patch | download mbox

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index a0438f3..95b355d 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -49,6 +49,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;
@@ -70,6 +73,9 @@  It contains following fields:
 * timeout: the watchdog timer's timeout value (in seconds).
 * min_timeout: the watchdog timer's minimum timeout value (in seconds).
 * max_timeout: the watchdog timer's maximum timeout value (in seconds).
+* pretimeout: the watchdog timer's pretimeout value (in seconds).
+* min_pretimeout: the watchdog timer's minimum pretimeout value (in seconds).
+* max_pretimeout: the watchdog timer's maximum pretimeout value (in seconds).
 * bootstatus: status of the device after booting (reported with watchdog
   WDIOF_* status bits).
 * driver_data: a pointer to the drivers private data of a watchdog device.
@@ -92,6 +98,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 *);
@@ -153,9 +160,19 @@  they are supported. These optional routines/operations are:
   and -EIO for "could not write value to the watchdog". On success this
   routine should set the timeout value of the watchdog_device to the
   achieved timeout value (which may be different from the requested one
-  because the watchdog does not necessarily has a 1 second resolution).
+  because the watchdog does not necessarily has a 1 second resolution;
+  If the driver supports pretimeout, then the timeout value must be greater
+  than that).
   (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the
   watchdog's info structure).
+* set_pretimeout: this routine checks and changes the pretimeout of the
+  watchdog timer device. It returns 0 on success, -EINVAL for "parameter out of
+  range" and -EIO for "could not write value to the watchdog". On success this
+  routine should set the pretimeout value of the watchdog_device to the
+  achieved pretimeout value (which may be different from the requested one
+  because the watchdog does not necessarily has a 1 second resolution).
+  (Note: the WDIOF_PRETIMEOUT needs to be set in the options field of the
+  watchdog's info structure).
 * get_timeleft: this routines returns the time that's left before a reset.
 * ref: the operation that calls kref_get on the kref of a dynamically
   allocated watchdog_device struct.
@@ -219,8 +236,28 @@  extern int watchdog_init_timeout(struct watchdog_device *wdd,
                                   unsigned int timeout_parm, struct device *dev);
 
 The watchdog_init_timeout function allows you to initialize the timeout field
-using the module timeout parameter or by retrieving the timeout-sec property from
-the device tree (if the module timeout parameter is invalid). Best practice is
-to set the default timeout value as timeout value in the watchdog_device and
-then use this function to set the user "preferred" timeout value.
+using the module timeout parameter or by retrieving the first element of
+the timeout-sec property from the device tree (if the module timeout parameter
+is invalid). Best practice is to set the default timeout value as timeout value
+in the watchdog_device and then use this function to set the user "preferred"
+timeout value.
+This routine returns zero on success and a negative errno code for failure.
+
+Some watchdog timers have two stage of timeouts(timeout and pretimeout),
+to initialize the timeout and pretimeout fields at the same time, the following
+function can be used:
+
+extern int watchdog_init_timeouts(struct watchdog_device *wdd,
+                                  unsigned int pretimeout_parm,
+                                  unsigned int timeout_parm,
+                                  struct device *dev);
+
+The watchdog_init_timeouts function allows you to initialize the pretimeout and
+timeout fields using the module pretimeout and timeout parameter or by
+retrieving the elements in the timeout-sec property(the first element is for
+timeout, the second one is for pretimeout) from the device tree(if the module
+pretimeout and timeout parameter are invalid).
+Best practice is to set the default pretimeout and timeout value as pretimeout
+and timeout value in the watchdog_device and then use this function to set the
+user "preferred" pretimeout value.
 This routine returns zero on success and a negative errno code for failure.
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index cec9b55..bdd4e43 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -43,60 +43,98 @@ 
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
-static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
+static void watchdog_check_min_max_timeouts(struct watchdog_device *wdd)
 {
 	/*
-	 * Check that we have valid min and max timeout values, if
-	 * not reset them both to 0 (=not used or unknown)
+	 * Check that we have valid min and max pretimeout and timeout values,
+	 * if not, reset them all to 0 (=not used or unknown)
 	 */
-	if (wdd->min_timeout > wdd->max_timeout) {
-		pr_info("Invalid min and max timeout values, resetting to 0!\n");
+	if (wdd->min_pretimeout > wdd->max_pretimeout ||
+	    wdd->min_timeout > wdd->max_timeout ||
+	    wdd->min_timeout < wdd->min_pretimeout ||
+	    wdd->max_timeout < wdd->max_pretimeout) {
+		pr_info("Invalid min and max timeouts, resetting to 0\n");
+		wdd->min_pretimeout = 0;
+		wdd->max_pretimeout = 0;
 		wdd->min_timeout = 0;
 		wdd->max_timeout = 0;
 	}
 }
 
 /**
- * watchdog_init_timeout() - initialize the timeout field
+ * watchdog_init_timeouts() - initialize the pretimeout and timeout field
+ * @pretimeout_parm: pretimeout module parameter
  * @timeout_parm: timeout module parameter
  * @dev: Device that stores the timeout-sec property
  *
- * Initialize the timeout field of the watchdog_device struct with either the
- * timeout 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 timeout value.
+ * Initialize the pretimeout and timeout field of the watchdog_device struct
+ * with either the pretimeout and timeout module parameter (if it is valid) or
+ * the timeout-sec property (only if it is valid and the pretimeout_parm or
+ * timeout_parm is out of bounds). If none of them is valid, then we keep
+ * the old value (which should normally be the default timeout value).
  *
  * A zero is returned on success and -EINVAL for failure.
  */
-int watchdog_init_timeout(struct watchdog_device *wdd,
-				unsigned int timeout_parm, struct device *dev)
+int watchdog_init_timeouts(struct watchdog_device *wdd,
+			   unsigned int pretimeout_parm,
+			   unsigned int timeout_parm,
+			   struct device *dev)
 {
-	unsigned int t = 0;
-	int ret = 0;
+	int ret = 0, length = 0;
+	u32 timeouts[2] = {0};
+	struct property *prop;
 
-	watchdog_check_min_max_timeout(wdd);
+	watchdog_check_min_max_timeouts(wdd);
 
-	/* try to get the timeout module parameter first */
-	if (!watchdog_timeout_invalid(wdd, timeout_parm) && timeout_parm) {
-		wdd->timeout = timeout_parm;
-		return ret;
-	}
-	if (timeout_parm)
+	/*
+	 * Try to get the pretimeout module parameter first
+	 */
+	if (!watchdog_pretimeout_invalid(wdd, pretimeout_parm))
+		timeouts[1] = pretimeout_parm;
+	else
+		ret = -EINVAL; /* pretimeout_parm is invalid */
+
+	/*
+	 * Try to get the timeout module parameter,
+	 * if it's valid and pretimeout is valid(ret == 0),
+	 * assignment and return zero. Otherwise, try dtb.
+	 */
+	if (timeout_parm) {
+		if (!watchdog_timeout_invalid(wdd, timeout_parm) && !ret) {
+			wdd->timeout = timeout_parm;
+			wdd->pretimeout = timeouts[1];
+			return 0;
+		}
 		ret = -EINVAL;
+	}
 
-	/* try to get the timeout_sec property */
+	/*
+	 * Either at least one of the module parameters is invalid,
+	 * or timeout_parm is 0. Try to get the timeout_sec property.
+	 */
 	if (dev == NULL || dev->of_node == NULL)
 		return ret;
-	of_property_read_u32(dev->of_node, "timeout-sec", &t);
-	if (!watchdog_timeout_invalid(wdd, t) && t)
-		wdd->timeout = t;
-	else
-		ret = -EINVAL;
 
-	return ret;
+	prop = of_find_property(dev->of_node, "timeout-sec", &length);
+	if (prop && length > 0 && length <= sizeof(u32) * 2) {
+		of_property_read_u32_array(dev->of_node,
+					   "timeout-sec", timeouts,
+					   length / sizeof(u32));
+		if (length == sizeof(u32) * 2 &&
+		    watchdog_pretimeout_invalid(wdd, timeouts[1]))
+				return -EINVAL;
+
+		if (!watchdog_timeout_invalid(wdd, timeouts[0]) &&
+		    timeouts[0]) {
+			wdd->timeout = timeouts[0];
+			wdd->pretimeout = timeouts[1];
+			return 0;
+		}
+	}
+
+	return -EINVAL;
 }
-EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+EXPORT_SYMBOL_GPL(watchdog_init_timeouts);
 
 /**
  * watchdog_register_device() - register a watchdog device
@@ -119,7 +157,7 @@  int watchdog_register_device(struct watchdog_device *wdd)
 	if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
 		return -EINVAL;
 
-	watchdog_check_min_max_timeout(wdd);
+	watchdog_check_min_max_timeouts(wdd);
 
 	/*
 	 * Note: now that all watchdog_device data has been verified, we
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 6aaefba..af0777e 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,27 @@  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:
+		/* check if we support the pretimeout */
+		if (!(wdd->info->options & WDIOF_PRETIMEOUT))
+			return -EOPNOTSUPP;
+		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 keeps running (and if
+		 * possible that it takes the new pretimeout)
+		 */
+		watchdog_ping(wdd);
+		/* Fall */
+	case WDIOC_GETPRETIMEOUT:
+		/* check if we support the pretimeout */
+		if (wdd->info->options & WDIOF_PRETIMEOUT)
+			return put_user(wdd->pretimeout, p);
+		return -EOPNOTSUPP;
 	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..0a7acf0 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;
@@ -117,7 +125,20 @@  static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway
 static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t)
 {
 	return ((wdd->max_timeout != 0) &&
-		(t < wdd->min_timeout || t > wdd->max_timeout));
+		(t < wdd->min_timeout || t > wdd->max_timeout ||
+			t <= wdd->pretimeout));
+}
+
+/*
+ * Use the following function to check if a pretimeout value is invalid.
+ * It can be "0", that means we don't use pretimeout.
+ */
+static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd,
+					       unsigned int t)
+{
+	return (wdd->pretimeout != 0 && wdd->max_pretimeout != 0 &&
+		(t < wdd->min_pretimeout || t > wdd->max_pretimeout ||
+			(wdd->timeout != 0 && t >= wdd->timeout)));
 }
 
 /* Use the following functions to manipulate watchdog driver specific data */
@@ -132,11 +153,20 @@  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);
+int watchdog_init_timeouts(struct watchdog_device *wdd,
+				  unsigned int pretimeout_parm,
+				  unsigned int timeout_parm,
+				  struct device *dev);
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+static inline int watchdog_init_timeout(struct watchdog_device *wdd,
+					unsigned int timeout_parm,
+					struct device *dev)
+{
+	return watchdog_init_timeouts(wdd, 0, timeout_parm, dev);
+}
+
 #ifdef CONFIG_HARDLOCKUP_DETECTOR
 void watchdog_nmi_disable_all(void);
 void watchdog_nmi_enable_all(void);