Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1569989 > unrolled thread

[PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core handle the heartbeat

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core handle the heartbeat H Hartley Sweeten <hsweeten@visionengravers.com> - 2017-01-30 19:30 +0100
    Re: [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core  handle the heartbeat Guenter Roeck <linux@roeck-us.net> - 2017-01-30 20:00 +0100
      RE: [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core handle  the heartbeat Hartley Sweeten <HartleyS@visionengravers.com> - 2017-01-30 20:20 +0100
        Re: [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core  handle the heartbeat Guenter Roeck <linux@roeck-us.net> - 2017-01-30 20:40 +0100

#1569989 — [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core handle the heartbeat

Cleanup this driver and remove the 200ms heartbeat timer. The core now
has the ability to handle the heartbeat.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/ep93xx_wdt.c | 115 +++++++++++++++++-------------------------
 1 file changed, 46 insertions(+), 69 deletions(-)

diff --git a/drivers/watchdog/ep93xx_wdt.c b/drivers/watchdog/ep93xx_wdt.c
index 0a4d7cc..756ca47 100644
--- a/drivers/watchdog/ep93xx_wdt.c
+++ b/drivers/watchdog/ep93xx_wdt.c
@@ -19,81 +19,55 @@
  * for us to rely on the user space daemon alone. So we ping the
  * wdt each ~200msec and eventually stop doing it if the user space
  * daemon dies.
- *
- * TODO:
- *
- *	- Test last reset from watchdog status
- *	- Add a few missing ioctls
  */
 
 #include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/watchdog.h>
-#include <linux/timer.h>
 #include <linux/io.h>
 
-#define WDT_VERSION	"0.4"
-
-/* default timeout (secs) */
-#define WDT_TIMEOUT 30
-
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started");
 
-static unsigned int timeout = WDT_TIMEOUT;
-module_param(timeout, uint, 0);
-MODULE_PARM_DESC(timeout,
-	"Watchdog timeout in seconds. (1<=timeout<=3600, default="
-				__MODULE_STRING(WDT_TIMEOUT) ")");
-
-static void __iomem *mmio_base;
-static struct timer_list timer;
-static unsigned long next_heartbeat;
-
 #define EP93XX_WATCHDOG		0x00
 #define EP93XX_WDSTATUS		0x04
 
-/* reset the wdt every ~200ms - the heartbeat of the device is 0.250 seconds*/
-#define WDT_INTERVAL (HZ/5)
-
-static void ep93xx_wdt_timer_ping(unsigned long data)
-{
-	if (time_before(jiffies, next_heartbeat))
-		writel(0x5555, mmio_base + EP93XX_WATCHDOG);
-
-	/* Re-set the timer interval */
-	mod_timer(&timer, jiffies + WDT_INTERVAL);
-}
+struct ep93xx_wdt_priv {
+	void __iomem *mmio;
+	struct watchdog_device wdd;
+};
 
 static int ep93xx_wdt_start(struct watchdog_device *wdd)
 {
-	next_heartbeat = jiffies + (timeout * HZ);
+	struct ep93xx_wdt_priv *priv = watchdog_get_drvdata(wdd);
 
-	writel(0xaaaa, mmio_base + EP93XX_WATCHDOG);
-	mod_timer(&timer, jiffies + WDT_INTERVAL);
+	writel(0xaaaa, priv->mmio + EP93XX_WATCHDOG);
 
 	return 0;
 }
 
 static int ep93xx_wdt_stop(struct watchdog_device *wdd)
 {
-	del_timer_sync(&timer);
-	writel(0xaa55, mmio_base + EP93XX_WATCHDOG);
+	struct ep93xx_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+	writel(0xaa55, priv->mmio + EP93XX_WATCHDOG);
 
 	return 0;
 }
 
-static int ep93xx_wdt_keepalive(struct watchdog_device *wdd)
+static int ep93xx_wdt_ping(struct watchdog_device *wdd)
 {
-	/* user land ping */
-	next_heartbeat = jiffies + (timeout * HZ);
+	struct ep93xx_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+	writel(0x5555, priv->mmio + EP93XX_WATCHDOG);
 
 	return 0;
 }
 
 static const struct watchdog_info ep93xx_wdt_ident = {
 	.options	= WDIOF_CARDRESET |
+			  WDIOF_SETTIMEOUT |
 			  WDIOF_MAGICCLOSE |
 			  WDIOF_KEEPALIVEPING,
 	.identity	= "EP93xx Watchdog",
@@ -103,47 +77,48 @@ static struct watchdog_ops ep93xx_wdt_ops = {
 	.owner		= THIS_MODULE,
 	.start		= ep93xx_wdt_start,
 	.stop		= ep93xx_wdt_stop,
-	.ping		= ep93xx_wdt_keepalive,
-};
-
-static struct watchdog_device ep93xx_wdt_wdd = {
-	.info		= &ep93xx_wdt_ident,
-	.ops		= &ep93xx_wdt_ops,
+	.ping		= ep93xx_wdt_ping,
 };
 
 static int ep93xx_wdt_probe(struct platform_device *pdev)
 {
+	struct ep93xx_wdt_priv *priv;
+	struct watchdog_device *wdd;
 	struct resource *res;
 	unsigned long val;
-	int err;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mmio_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(mmio_base))
-		return PTR_ERR(mmio_base);
+	priv->mmio = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->mmio))
+		return PTR_ERR(priv->mmio);
+
+	val = readl(priv->mmio + EP93XX_WATCHDOG);
 
-	if (timeout < 1 || timeout > 3600) {
-		timeout = WDT_TIMEOUT;
-		dev_warn(&pdev->dev,
-			"timeout value must be 1<=x<=3600, using %d\n",
-			timeout);
-	}
+	wdd = &priv->wdd;
+	wdd->bootstatus = (val & 0x01) ? WDIOF_CARDRESET : 0;
+	wdd->info = &ep93xx_wdt_ident;
+	wdd->ops = &ep93xx_wdt_ops;
+	wdd->timeout = 30;
+	wdd->min_timeout = 1;
+	wdd->max_hw_heartbeat_ms = 200;
+	wdd->parent = &pdev->dev;
 
-	val = readl(mmio_base + EP93XX_WATCHDOG);
-	ep93xx_wdt_wdd.bootstatus = (val & 0x01) ? WDIOF_CARDRESET : 0;
-	ep93xx_wdt_wdd.timeout = timeout;
-	ep93xx_wdt_wdd.parent = &pdev->dev;
+	watchdog_set_nowayout(wdd, nowayout);
 
-	watchdog_set_nowayout(&ep93xx_wdt_wdd, nowayout);
+	watchdog_set_drvdata(wdd, priv);
 
-	setup_timer(&timer, ep93xx_wdt_timer_ping, 1);
+	ret = watchdog_register_device(wdd);
+	if (ret)
+		return ret;
 
-	err = watchdog_register_device(&ep93xx_wdt_wdd);
-	if (err)
-		return err;
+	platform_set_drvdata(pdev, priv);
 
-	dev_info(&pdev->dev,
-		"EP93XX watchdog, driver version " WDT_VERSION "%s\n",
+	dev_info(&pdev->dev, "EP93XX watchdog driver %s\n",
 		(val & 0x08) ? " (nCS1 disable detected)" : "");
 
 	return 0;
@@ -151,7 +126,10 @@ static int ep93xx_wdt_probe(struct platform_device *pdev)
 
 static int ep93xx_wdt_remove(struct platform_device *pdev)
 {
-	watchdog_unregister_device(&ep93xx_wdt_wdd);
+	struct ep93xx_wdt_priv *priv = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&priv->wdd);
+
 	return 0;
 }
 
@@ -170,4 +148,3 @@ MODULE_AUTHOR("Alessandro Zummo <a.zummo@towertech.it>");
 MODULE_AUTHOR("H Hartley Sweeten <hsweeten@visionengravers.com>");
 MODULE_DESCRIPTION("EP93xx Watchdog");
 MODULE_LICENSE("GPL");
-MODULE_VERSION(WDT_VERSION);
-- 
2.10.0

[toc] | [next] | [standalone]

#1570023 — Re: [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core handle the heartbeat

On Mon, Jan 30, 2017 at 09:55:47AM -0700, H Hartley Sweeten wrote:
> Cleanup this driver and remove the 200ms heartbeat timer. The core now
> has the ability to handle the heartbeat.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/watchdog/ep93xx_wdt.c | 115 +++++++++++++++++-------------------------
>  1 file changed, 46 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/watchdog/ep93xx_wdt.c b/drivers/watchdog/ep93xx_wdt.c
> index 0a4d7cc..756ca47 100644
> --- a/drivers/watchdog/ep93xx_wdt.c
> +++ b/drivers/watchdog/ep93xx_wdt.c
> @@ -19,81 +19,55 @@
>   * for us to rely on the user space daemon alone. So we ping the
>   * wdt each ~200msec and eventually stop doing it if the user space
>   * daemon dies.
> - *
> - * TODO:
> - *
> - *	- Test last reset from watchdog status
> - *	- Add a few missing ioctls
>   */
>  
>  #include <linux/platform_device.h>
>  #include <linux/module.h>
>  #include <linux/watchdog.h>
> -#include <linux/timer.h>
>  #include <linux/io.h>
>  
> -#define WDT_VERSION	"0.4"
> -
> -/* default timeout (secs) */
> -#define WDT_TIMEOUT 30
> -

Personally I like those constants, even if used only once (I know, it is a good
candidate for bikeshedding). Two reasons: 1) It is already there, and 2) It
helps seeing the default without having to dig into the code.

>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started");
>  
> -static unsigned int timeout = WDT_TIMEOUT;
> -module_param(timeout, uint, 0);
> -MODULE_PARM_DESC(timeout,
> -	"Watchdog timeout in seconds. (1<=timeout<=3600, default="
> -				__MODULE_STRING(WDT_TIMEOUT) ")");
> -

Are you sure you want to take away the means to set the timeout with
a module parameter ? You could easily retain the module parameter
and call watchdog_init_timeout(wdd, timeout, dev). The parameter should
then be initialized with 0, though, to have the watchdog core take the
timeout from devicetree if provided.

> -static void __iomem *mmio_base;
> -static struct timer_list timer;
> -static unsigned long next_heartbeat;
> -
>  #define EP93XX_WATCHDOG		0x00
>  #define EP93XX_WDSTATUS		0x04
>  
> -/* reset the wdt every ~200ms - the heartbeat of the device is 0.250 seconds*/
> -#define WDT_INTERVAL (HZ/5)
> -
> -static void ep93xx_wdt_timer_ping(unsigned long data)
> -{
> -	if (time_before(jiffies, next_heartbeat))
> -		writel(0x5555, mmio_base + EP93XX_WATCHDOG);
> -
> -	/* Re-set the timer interval */
> -	mod_timer(&timer, jiffies + WDT_INTERVAL);
> -}
> +struct ep93xx_wdt_priv {
> +	void __iomem *mmio;
> +	struct watchdog_device wdd;
> +};
>  
>  static int ep93xx_wdt_start(struct watchdog_device *wdd)
>  {
> -	next_heartbeat = jiffies + (timeout * HZ);
> +	struct ep93xx_wdt_priv *priv = watchdog_get_drvdata(wdd);
>  
> -	writel(0xaaaa, mmio_base + EP93XX_WATCHDOG);
> -	mod_timer(&timer, jiffies + WDT_INTERVAL);
> +	writel(0xaaaa, priv->mmio + EP93XX_WATCHDOG);
>  
>  	return 0;
>  }
>  
>  static int ep93xx_wdt_stop(struct watchdog_device *wdd)
>  {
> -	del_timer_sync(&timer);
> -	writel(0xaa55, mmio_base + EP93XX_WATCHDOG);
> +	struct ep93xx_wdt_priv *priv = watchdog_get_drvdata(wdd);
> +
> +	writel(0xaa55, priv->mmio + EP93XX_WATCHDOG);
>  
>  	return 0;
>  }
>  
> -static int ep93xx_wdt_keepalive(struct watchdog_device *wdd)
> +static int ep93xx_wdt_ping(struct watchdog_device *wdd)
>  {
> -	/* user land ping */
> -	next_heartbeat = jiffies + (timeout * HZ);
> +	struct ep93xx_wdt_priv *priv = watchdog_get_drvdata(wdd);
> +
> +	writel(0x5555, priv->mmio + EP93XX_WATCHDOG);
>  
>  	return 0;
>  }
>  
>  static const struct watchdog_info ep93xx_wdt_ident = {
>  	.options	= WDIOF_CARDRESET |
> +			  WDIOF_SETTIMEOUT |
>  			  WDIOF_MAGICCLOSE |
>  			  WDIOF_KEEPALIVEPING,
>  	.identity	= "EP93xx Watchdog",
> @@ -103,47 +77,48 @@ static struct watchdog_ops ep93xx_wdt_ops = {
>  	.owner		= THIS_MODULE,
>  	.start		= ep93xx_wdt_start,
>  	.stop		= ep93xx_wdt_stop,
> -	.ping		= ep93xx_wdt_keepalive,
> -};
> -
> -static struct watchdog_device ep93xx_wdt_wdd = {
> -	.info		= &ep93xx_wdt_ident,
> -	.ops		= &ep93xx_wdt_ops,
> +	.ping		= ep93xx_wdt_ping,
>  };
>  
>  static int ep93xx_wdt_probe(struct platform_device *pdev)
>  {
> +	struct ep93xx_wdt_priv *priv;
> +	struct watchdog_device *wdd;
>  	struct resource *res;
>  	unsigned long val;
> -	int err;
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	mmio_base = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(mmio_base))
> -		return PTR_ERR(mmio_base);
> +	priv->mmio = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->mmio))
> +		return PTR_ERR(priv->mmio);
> +
> +	val = readl(priv->mmio + EP93XX_WATCHDOG);
>  
> -	if (timeout < 1 || timeout > 3600) {
> -		timeout = WDT_TIMEOUT;
> -		dev_warn(&pdev->dev,
> -			"timeout value must be 1<=x<=3600, using %d\n",
> -			timeout);
> -	}
> +	wdd = &priv->wdd;
> +	wdd->bootstatus = (val & 0x01) ? WDIOF_CARDRESET : 0;
> +	wdd->info = &ep93xx_wdt_ident;
> +	wdd->ops = &ep93xx_wdt_ops;
> +	wdd->timeout = 30;
> +	wdd->min_timeout = 1;
> +	wdd->max_hw_heartbeat_ms = 200;
> +	wdd->parent = &pdev->dev;
>  
> -	val = readl(mmio_base + EP93XX_WATCHDOG);
> -	ep93xx_wdt_wdd.bootstatus = (val & 0x01) ? WDIOF_CARDRESET : 0;
> -	ep93xx_wdt_wdd.timeout = timeout;
> -	ep93xx_wdt_wdd.parent = &pdev->dev;
> +	watchdog_set_nowayout(wdd, nowayout);
>  
> -	watchdog_set_nowayout(&ep93xx_wdt_wdd, nowayout);
> +	watchdog_set_drvdata(wdd, priv);
>  
> -	setup_timer(&timer, ep93xx_wdt_timer_ping, 1);
> +	ret = watchdog_register_device(wdd);

Looks like a perfect candidate for devm_watchdog_register_device().

> +	if (ret)
> +		return ret;
>  
> -	err = watchdog_register_device(&ep93xx_wdt_wdd);
> -	if (err)
> -		return err;
> +	platform_set_drvdata(pdev, priv);
>  
> -	dev_info(&pdev->dev,
> -		"EP93XX watchdog, driver version " WDT_VERSION "%s\n",
> +	dev_info(&pdev->dev, "EP93XX watchdog driver %s\n",
>  		(val & 0x08) ? " (nCS1 disable detected)" : "");
>  
>  	return 0;
> @@ -151,7 +126,10 @@ static int ep93xx_wdt_probe(struct platform_device *pdev)
>  
>  static int ep93xx_wdt_remove(struct platform_device *pdev)
>  {
> -	watchdog_unregister_device(&ep93xx_wdt_wdd);
> +	struct ep93xx_wdt_priv *priv = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(&priv->wdd);
> +
>  	return 0;
>  }
>  
> @@ -170,4 +148,3 @@ MODULE_AUTHOR("Alessandro Zummo <a.zummo@towertech.it>");
>  MODULE_AUTHOR("H Hartley Sweeten <hsweeten@visionengravers.com>");
>  MODULE_DESCRIPTION("EP93xx Watchdog");
>  MODULE_LICENSE("GPL");
> -MODULE_VERSION(WDT_VERSION);
> -- 
> 2.10.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1570042 — RE: [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core handle the heartbeat

On Monday, January 30, 2017 11:55 AM, Guenter Roeck wrote:
> On Mon, Jan 30, 2017 at 09:55:47AM -0700, H Hartley Sweeten wrote:
>> Cleanup this driver and remove the 200ms heartbeat timer. The core now
>> has the ability to handle the heartbeat.
>> 
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: Wim Van Sebroeck <wim@iguana.be>
>> Cc: Guenter Roeck <linux@roeck-us.net>

Hi Guenter,

I wasn't sure this patch was going to get delivered correctly. I got an "Undeliverable"
bounce due to possible spoofing. I am trying to figure out why right now.

Anyway...

<snip>

>> -#define WDT_VERSION	"0.4"
>> -
>> -/* default timeout (secs) */
>> -#define WDT_TIMEOUT 30
>> -
>
> Personally I like those constants, even if used only once (I know, it is a good
> candidate for bikeshedding). Two reasons: 1) It is already there, and 2) It
> helps seeing the default without having to dig into the code.

I assume the WDT_VERSION can go away...

As far as the WDT_TIMEOUT, I have no problem leaving it. I was just trying to remove
some cruft.

>>  static bool nowayout = WATCHDOG_NOWAYOUT;
>>  module_param(nowayout, bool, 0);
>>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started");
>>  
>> -static unsigned int timeout = WDT_TIMEOUT;
>> -module_param(timeout, uint, 0);
>> -MODULE_PARM_DESC(timeout,
>> -	"Watchdog timeout in seconds. (1<=timeout<=3600, default="
>> -				__MODULE_STRING(WDT_TIMEOUT) ")");
>> -
>
> Are you sure you want to take away the means to set the timeout with
> a module parameter ? You could easily retain the module parameter
> and call watchdog_init_timeout(wdd, timeout, dev). The parameter should
> then be initialized with 0, though, to have the watchdog core take the
> timeout from devicetree if provided.

Again, I have no problem leaving this. I personally don't use it but someone else
might. I'm not sure if the ep93xx will ever get converted to devicetree but I
might figure it one eventually.

<snip>

>> +	watchdog_set_drvdata(wdd, priv);
>>  
>> -	setup_timer(&timer, ep93xx_wdt_timer_ping, 1);
>> +	ret = watchdog_register_device(wdd);
>
> Looks like a perfect candidate for devm_watchdog_register_device().

I just saw your patches that do this. I will update this patch to use it.

Thanks,
Hartley

[toc] | [prev] | [next] | [standalone]

#1570063 — Re: [PATCH 1/2] watchdog: ep93xx_wdt: cleanup and let the core handle the heartbeat

On Mon, Jan 30, 2017 at 07:09:55PM +0000, Hartley Sweeten wrote:
> On Monday, January 30, 2017 11:55 AM, Guenter Roeck wrote:
> > On Mon, Jan 30, 2017 at 09:55:47AM -0700, H Hartley Sweeten wrote:
> >> Cleanup this driver and remove the 200ms heartbeat timer. The core now
> >> has the ability to handle the heartbeat.
> >> 
> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> >> Cc: Wim Van Sebroeck <wim@iguana.be>
> >> Cc: Guenter Roeck <linux@roeck-us.net>
> 
> Hi Guenter,
> 
> I wasn't sure this patch was going to get delivered correctly. I got an "Undeliverable"
> bounce due to possible spoofing. I am trying to figure out why right now.
> 
Interesting. If you find out, please let me know. Either case, I am subscribed
to the watchdog mailing list, so should get all e-mail sent to it. I also have
a patchwork instance running, so I see it there as well if the watchdog mailing
list is copied.

> Anyway...
> 
> <snip>
> 
> >> -#define WDT_VERSION	"0.4"
> >> -
> >> -/* default timeout (secs) */
> >> -#define WDT_TIMEOUT 30
> >> -
> >
> > Personally I like those constants, even if used only once (I know, it is a good
> > candidate for bikeshedding). Two reasons: 1) It is already there, and 2) It
> > helps seeing the default without having to dig into the code.
> 
> I assume the WDT_VERSION can go away...
> 
Yes, that is pretty much useless.

> As far as the WDT_TIMEOUT, I have no problem leaving it. I was just trying to remove
> some cruft.
> 
> >>  static bool nowayout = WATCHDOG_NOWAYOUT;
> >>  module_param(nowayout, bool, 0);
> >>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started");
> >>  
> >> -static unsigned int timeout = WDT_TIMEOUT;
> >> -module_param(timeout, uint, 0);
> >> -MODULE_PARM_DESC(timeout,
> >> -	"Watchdog timeout in seconds. (1<=timeout<=3600, default="
> >> -				__MODULE_STRING(WDT_TIMEOUT) ")");
> >> -
> >
> > Are you sure you want to take away the means to set the timeout with
> > a module parameter ? You could easily retain the module parameter
> > and call watchdog_init_timeout(wdd, timeout, dev). The parameter should
> > then be initialized with 0, though, to have the watchdog core take the
> > timeout from devicetree if provided.
> 
> Again, I have no problem leaving this. I personally don't use it but someone else
> might. I'm not sure if the ep93xx will ever get converted to devicetree but I
> might figure it one eventually.
> 

Thinking more about it, we should really leave the module parameter in.
As you say, someone else may be using it, and we should not change the
interface to user space. Using watchdog_init_timeout() is useful to check
the range (not that it really matters here); that it reads a value from
devicetree is an additional benefit.

Thanks,
Guenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web