Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: minyard@acm.org
Cc: Wim Van Sebroeck <wim@linux-watchdog.org>,
	linux-watchdog@vger.kernel.org,
	Corey Minyard <cminyard@mvista.com>
Subject: Re: [PATCH 04/10] watchdog: Add functions to set the timeout and pretimeout
Date: Tue, 30 Jun 2020 19:45:35 -0700
Message-ID: <20200701024535.GA84420@roeck-us.net> (raw)
In-Reply-To: <20200620174907.20229-5-minyard@acm.org>

On Sat, Jun 20, 2020 at 12:49:01PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> If the watchdog device wants to set it's pretimeout (say from module
> parameters), this lets them do that.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  drivers/watchdog/watchdog_dev.c | 54 +++++++++++++++++++++++++++++----
>  include/linux/watchdog.h        |  4 +++
>  2 files changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 45a0a4fe731d..6c423aed3f3c 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -364,14 +364,14 @@ static unsigned int watchdog_get_status(struct watchdog_device *wdd)
>  }
>  
>  /*
> - *	watchdog_set_timeout: set the watchdog timer timeout
> + *	_watchdog_set_timeout: set the watchdog timer timeout
>   *	@wdd: the watchdog device to set the timeout for
>   *	@timeout: timeout to set in seconds
>   *
>   *	The caller must hold wd_data->lock.
>   */
>  
> -static int watchdog_set_timeout(struct watchdog_device *wdd,
> +static int _watchdog_set_timeout(struct watchdog_device *wdd,
>  							unsigned int timeout)
>  {
>  	int err = 0;
> @@ -396,14 +396,35 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
>  	return err;
>  }
>  
> +/*
> + *	watchdog_set_timeout: set the watchdog timer timeout
> + *	@wdd: the watchdog device to set the timeout for
> + *	@timeout: timeout to set in seconds
> + */
> +
> +int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
> +{
> +	int err = 0;
> +
> +	if (!wdd->wd_data)
> +		return -ENODEV;
> +
> +	mutex_lock(&wdd->wd_data->lock);
> +	err = _watchdog_set_timeout(wdd, timeout);
> +	mutex_unlock(&wdd->wd_data->lock);

These functions are expected to be called prior to registration. wd_data isn't
set at that time, and even if it was using the mutex would be unnecessary
since there is nothing to synchronize against.

Besides being unnecessary, this change isn't mentioned in the patch description.

Guenter

> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_set_timeout);
> +
>  /*
>   *	watchdog_set_pretimeout: set the watchdog timer pretimeout
>   *	@wdd: the watchdog device to set the timeout for
>   *	@timeout: pretimeout to set in seconds
>   */
>  
> -static int watchdog_set_pretimeout(struct watchdog_device *wdd,
> -				   unsigned int timeout)
> +static int _watchdog_set_pretimeout(struct watchdog_device *wdd,
> +				    unsigned int timeout)
>  {
>  	int err = 0;
>  
> @@ -421,6 +442,27 @@ static int watchdog_set_pretimeout(struct watchdog_device *wdd,
>  	return err;
>  }
>  
> +/*
> + *	watchdog_set_pretimeout: set the watchdog timer pretimeout
> + *	@wdd: the watchdog device to set the timeout for
> + *	@timeout: pretimeout to set in seconds
> + */
> +
> +int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout)
> +{
> +	int err = 0;
> +
> +	if (!wdd->wd_data)
> +		return -ENODEV;
> +
> +	mutex_lock(&wdd->wd_data->lock);
> +	err = _watchdog_set_pretimeout(wdd, timeout);
> +	mutex_unlock(&wdd->wd_data->lock);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_set_pretimeout);
> +
>  /*
>   *	watchdog_get_timeleft: wrapper to get the time left before a reboot
>   *	@wdd: the watchdog device to get the remaining time from
> @@ -809,7 +851,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>  			err = -EFAULT;
>  			break;
>  		}
> -		err = watchdog_set_timeout(wdd, val);
> +		err = _watchdog_set_timeout(wdd, val);
>  		if (err < 0)
>  			break;
>  		/* If the watchdog is active then we send a keepalive ping
> @@ -838,7 +880,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>  			err = -EFAULT;
>  			break;
>  		}
> -		err = watchdog_set_pretimeout(wdd, val);
> +		err = _watchdog_set_pretimeout(wdd, val);
>  		break;
>  	case WDIOC_GETPRETIMEOUT:
>  		err = put_user(wdd->pretimeout, p);
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 36f99c8c973e..95396b644a9b 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -201,6 +201,10 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
>  	return wdd->driver_data;
>  }
>  
> +/* Allow the driver to set the timeout and pretimeout. */
> +int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout);
> +int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout);
> +
>  /* Use the following functions to report watchdog pretimeout event */
>  #if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
>  void watchdog_notify_pretimeout(struct watchdog_device *wdd);
> -- 
> 2.17.1
> 

  reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-20 17:48 [PATCH 00/10] Convert the IPMI watchdog to use the watchdog minyard
2020-06-20 17:48 ` [PATCH 01/10] watchdog: Ignore stop_on_reboot if no stop function minyard
2020-06-20 17:48 ` [PATCH 02/10] watchdog: Add read capability minyard
2020-06-20 17:49 ` [PATCH 03/10] watchdog: Add documentation for " minyard
2020-07-01  2:49   ` Guenter Roeck
2020-06-20 17:49 ` [PATCH 04/10] watchdog: Add functions to set the timeout and pretimeout minyard
2020-07-01  2:45   ` Guenter Roeck [this message]
2020-06-20 17:49 ` [PATCH 05/10] watchdog: Export an interface to start the watchdog minyard
2020-07-01  2:46   ` Guenter Roeck
2020-06-20 17:49 ` [PATCH 06/10] ipmi:watchdog: Convert over to the watchdog framework minyard
2020-06-20 17:49 ` [PATCH 07/10] ipmi:watchdog: Allow the reboot timeout to be specified minyard
2020-06-20 17:49 ` [PATCH 08/10] watchdog: Add a way to extend the timeout on a reboot minyard
2020-06-20 17:49 ` [PATCH 09/10] ipmi:watchdog: Convert over to watchdog framework reboot handling minyard
2020-06-20 17:49 ` [PATCH 10/10] ipmi:watchdog: Add the op to get the current timeout minyard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200701024535.GA84420@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=cminyard@mvista.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=minyard@acm.org \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git