Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values

[Wim Van Sebroeck]

Hi Javier,

> The watchdog maximum timeout value is determined by the number of bits
> for the interval timer counter, its source clock frequency, the number
> of bits of the prescaler and maximum divider value.
> 
> This can be calculated with the following equation:
> 
> max_timeout = counter / (freq / (max_prescale + 1) / max_divider)
> 
> Setting a maximum timeout value will allow the watchdog core to refuse
> user-space calls to the WDIOC_SETTIMEOUT ioctl that sets not supported
> timeout values.
> 
> For example, systemd tries to set a timeout of 10 minutes on reboot to
> ensure that the machine will be rebooted even if a reboot failed. This
> leads to the following error message on an Exynos5422 Odroid XU4 board:
> 
> [  147.986045] s3c2410-wdt 101d0000.watchdog: timeout 600 too big
> 
> Reported-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
> Signed-off-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> 
> ---
> Hello,
> 
> I'm sending this as an RFC because I don't have access to user manuals
> for all the Exynos SoCs that this driver supports. But at least in all
> the ones I have here, the maximum prescaler and divider values are the
> same and also all have a 16-bit interval timer counter.
> 
> So this patch assumes that all IP blocks supported by this driver are
> the same on that regard, if that's not the case then we can move these
> values on a per IP block basis using struct of_device_id .data field.
> 
> Best regards,
> Javier
> 
>  drivers/watchdog/s3c2410_wdt.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 0093450441fe..f289c9fc353a 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -47,6 +47,8 @@
>  #define S3C2410_WTDAT		0x04
>  #define S3C2410_WTCNT		0x08
>  
> +#define S3C2410_WTCNT_MAXCNT	0xffff
> +
>  #define S3C2410_WTCON_RSTEN	(1 << 0)
>  #define S3C2410_WTCON_INTEN	(1 << 2)
>  #define S3C2410_WTCON_ENABLE	(1 << 5)
> @@ -56,8 +58,11 @@
>  #define S3C2410_WTCON_DIV64	(2 << 3)
>  #define S3C2410_WTCON_DIV128	(3 << 3)
>  
> +#define S3C2410_WTCON_MAXDIV	0x80
> +
>  #define S3C2410_WTCON_PRESCALE(x)	((x) << 8)
>  #define S3C2410_WTCON_PRESCALE_MASK	(0xff << 8)
> +#define S3C2410_WTCON_PRESCALE_MAX	0xff
>  
>  #define CONFIG_S3C2410_WATCHDOG_ATBOOT		(0)
>  #define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME	(15)
> @@ -198,6 +203,14 @@ do {							\
>  
>  /* functions */
>  
> +static inline unsigned int s3c2410wdt_max_timeout(struct clk *clock)
> +{
> +	unsigned long freq = clk_get_rate(clock);
> +
> +	return S3C2410_WTCNT_MAXCNT / (freq / (S3C2410_WTCON_PRESCALE_MAX + 1)
> +				       / S3C2410_WTCON_MAXDIV);
> +}
> +
>  static inline struct s3c2410_wdt *freq_to_wdt(struct notifier_block *nb)
>  {
>  	return container_of(nb, struct s3c2410_wdt, freq_transition);
> @@ -567,6 +580,9 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	wdt->wdt_device.min_timeout = 1;
> +	wdt->wdt_device.max_timeout = s3c2410wdt_max_timeout(wdt->clock);
> +
>  	ret = s3c2410wdt_cpufreq_register(wdt);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to register cpufreq\n");
> -- 
> 2.5.0
> 

This patch has been added to linux-watchdog-next.

Kind regards,
Wim.