Re: [PATCH] watchdog: omap_wdt: start_enabled module parameter

2015-06-22 Thread Lars Poeschel
Am Freitag 19 Juni 2015, 18:44:52 schrieb Guenter Roeck:
> On 06/19/2015 06:18 AM, poesc...@lemonage.de wrote:
> > From: Lars Poeschel 
> > 
> > Add a start_enabled module parameter to the omap_wdt that starts the
> > watchdog on module insertion. The default value is 0 which does not
> > start the watchdog - which also does not change the behaviour if the
> 
> behavior

Thanks.

> > parameter is not given.
> > 
> > Signed-off-by: Lars Poeschel 
> > ---
> > 
> >   Documentation/watchdog/watchdog-parameters.txt |  1 +
> >   drivers/watchdog/omap_wdt.c| 10 ++
> >   2 files changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/watchdog/watchdog-parameters.txt
> > b/Documentation/watchdog/watchdog-parameters.txt index 692791c..fb023bb
> > 100644
> > --- a/Documentation/watchdog/watchdog-parameters.txt
> > +++ b/Documentation/watchdog/watchdog-parameters.txt
> > @@ -208,6 +208,7 @@ nowayout: Watchdog cannot be stopped once started
> > 
> >   -
> >   omap_wdt:
> >   timer_margin: initial watchdog timeout (in seconds)
> > 
> > +start_enabled: Watchdog is started on module insertion (default=0)
> 
> Please name the parameter early_enable. We have one driver with
> early_disable, so this makes it at least to some degree generic.

We also have one driver with start_enabled. pnx833x_wdt was my example. But I 
am not very attached to start_enabled. I can change this if you want.

> >   -
> >   orion_wdt:
> >   heartbeat: Initial watchdog heartbeat in seconds
> > 
> > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> > index a29a61d..7dc8de7 100644
> > --- a/drivers/watchdog/omap_wdt.c
> > +++ b/drivers/watchdog/omap_wdt.c
> > @@ -53,6 +53,13 @@ static unsigned timer_margin;
> > 
> >   module_param(timer_margin, uint, 0);
> >   MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");
> > 
> > +#define START_DEFAULT  0
> > +static bool start_enabled = START_DEFAULT;
> 
> Please drop the define. Static variables do not need to be initialized to 0.
> Cirumventing that with a define to defeat the resulting warning isn't
> really the idea.

I also got this inspiration from pnx833x_wdt. But there the default is to 
enable it by default, so the static variable is initialized to 1. The idea is 
not to prevent the warning but if the default behavior changes in the future, 
you only have to change it in one place.
But as above, I can change this. I will submit a v2.

> > +module_param(start_enabled, bool, 0);
> > +MODULE_PARM_DESC(start_enabled,
> > +   "Watchdog is started on module insertion (default="
> > +   __MODULE_STRING(START_DEFAULT) ")");
> 
> and just make this default=0.
> 
> Thanks,
> Guenter

Regards,
Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] watchdog: omap_wdt: start_enabled module parameter

2015-06-22 Thread Lars Poeschel
Am Freitag 19 Juni 2015, 18:44:52 schrieb Guenter Roeck:
 On 06/19/2015 06:18 AM, poesc...@lemonage.de wrote:
  From: Lars Poeschel poesc...@lemonage.de
  
  Add a start_enabled module parameter to the omap_wdt that starts the
  watchdog on module insertion. The default value is 0 which does not
  start the watchdog - which also does not change the behaviour if the
 
 behavior

Thanks.

  parameter is not given.
  
  Signed-off-by: Lars Poeschel poesc...@lemonage.de
  ---
  
Documentation/watchdog/watchdog-parameters.txt |  1 +
drivers/watchdog/omap_wdt.c| 10 ++
2 files changed, 11 insertions(+)
  
  diff --git a/Documentation/watchdog/watchdog-parameters.txt
  b/Documentation/watchdog/watchdog-parameters.txt index 692791c..fb023bb
  100644
  --- a/Documentation/watchdog/watchdog-parameters.txt
  +++ b/Documentation/watchdog/watchdog-parameters.txt
  @@ -208,6 +208,7 @@ nowayout: Watchdog cannot be stopped once started
  
-
omap_wdt:
timer_margin: initial watchdog timeout (in seconds)
  
  +start_enabled: Watchdog is started on module insertion (default=0)
 
 Please name the parameter early_enable. We have one driver with
 early_disable, so this makes it at least to some degree generic.

We also have one driver with start_enabled. pnx833x_wdt was my example. But I 
am not very attached to start_enabled. I can change this if you want.

-
orion_wdt:
heartbeat: Initial watchdog heartbeat in seconds
  
  diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
  index a29a61d..7dc8de7 100644
  --- a/drivers/watchdog/omap_wdt.c
  +++ b/drivers/watchdog/omap_wdt.c
  @@ -53,6 +53,13 @@ static unsigned timer_margin;
  
module_param(timer_margin, uint, 0);
MODULE_PARM_DESC(timer_margin, initial watchdog timeout (in seconds));
  
  +#define START_DEFAULT  0
  +static bool start_enabled = START_DEFAULT;
 
 Please drop the define. Static variables do not need to be initialized to 0.
 Cirumventing that with a define to defeat the resulting warning isn't
 really the idea.

I also got this inspiration from pnx833x_wdt. But there the default is to 
enable it by default, so the static variable is initialized to 1. The idea is 
not to prevent the warning but if the default behavior changes in the future, 
you only have to change it in one place.
But as above, I can change this. I will submit a v2.

  +module_param(start_enabled, bool, 0);
  +MODULE_PARM_DESC(start_enabled,
  +   Watchdog is started on module insertion (default=
  +   __MODULE_STRING(START_DEFAULT) ));
 
 and just make this default=0.
 
 Thanks,
 Guenter

Regards,
Lars
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] watchdog: omap_wdt: start_enabled module parameter

2015-06-19 Thread Guenter Roeck

On 06/19/2015 06:18 AM, poesc...@lemonage.de wrote:

From: Lars Poeschel 

Add a start_enabled module parameter to the omap_wdt that starts the
watchdog on module insertion. The default value is 0 which does not
start the watchdog - which also does not change the behaviour if the


behavior


parameter is not given.

Signed-off-by: Lars Poeschel 
---
  Documentation/watchdog/watchdog-parameters.txt |  1 +
  drivers/watchdog/omap_wdt.c| 10 ++
  2 files changed, 11 insertions(+)

diff --git a/Documentation/watchdog/watchdog-parameters.txt 
b/Documentation/watchdog/watchdog-parameters.txt
index 692791c..fb023bb 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -208,6 +208,7 @@ nowayout: Watchdog cannot be stopped once started
  -
  omap_wdt:
  timer_margin: initial watchdog timeout (in seconds)
+start_enabled: Watchdog is started on module insertion (default=0)


Please name the parameter early_enable. We have one driver with early_disable,
so this makes it at least to some degree generic.


  -
  orion_wdt:
  heartbeat: Initial watchdog heartbeat in seconds
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index a29a61d..7dc8de7 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -53,6 +53,13 @@ static unsigned timer_margin;
  module_param(timer_margin, uint, 0);
  MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");

+#define START_DEFAULT  0
+static bool start_enabled = START_DEFAULT;


Please drop the define. Static variables do not need to be initialized to 0.
Cirumventing that with a define to defeat the resulting warning isn't really
the idea.


+module_param(start_enabled, bool, 0);
+MODULE_PARM_DESC(start_enabled,
+   "Watchdog is started on module insertion (default="
+   __MODULE_STRING(START_DEFAULT) ")");


and just make this default=0.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] watchdog: omap_wdt: start_enabled module parameter

2015-06-19 Thread poeschel
From: Lars Poeschel 

Add a start_enabled module parameter to the omap_wdt that starts the
watchdog on module insertion. The default value is 0 which does not
start the watchdog - which also does not change the behaviour if the
parameter is not given.

Signed-off-by: Lars Poeschel 
---
 Documentation/watchdog/watchdog-parameters.txt |  1 +
 drivers/watchdog/omap_wdt.c| 10 ++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/watchdog/watchdog-parameters.txt 
b/Documentation/watchdog/watchdog-parameters.txt
index 692791c..fb023bb 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -208,6 +208,7 @@ nowayout: Watchdog cannot be stopped once started
 -
 omap_wdt:
 timer_margin: initial watchdog timeout (in seconds)
+start_enabled: Watchdog is started on module insertion (default=0)
 -
 orion_wdt:
 heartbeat: Initial watchdog heartbeat in seconds
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index a29a61d..7dc8de7 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -53,6 +53,13 @@ static unsigned timer_margin;
 module_param(timer_margin, uint, 0);
 MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)");
 
+#define START_DEFAULT  0
+static bool start_enabled = START_DEFAULT;
+module_param(start_enabled, bool, 0);
+MODULE_PARM_DESC(start_enabled,
+   "Watchdog is started on module insertion (default="
+   __MODULE_STRING(START_DEFAULT) ")");
+
 struct omap_wdt_dev {
void __iomem*base;  /* physical */
struct device   *dev;
@@ -280,6 +287,9 @@ static int omap_wdt_probe(struct platform_device *pdev)
 
pm_runtime_put_sync(wdev->dev);
 
+   if (start_enabled)
+   omap_wdt_start(omap_wdt);
+
return 0;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] watchdog: omap_wdt: start_enabled module parameter

2015-06-19 Thread Guenter Roeck

On 06/19/2015 06:18 AM, poesc...@lemonage.de wrote:

From: Lars Poeschel poesc...@lemonage.de

Add a start_enabled module parameter to the omap_wdt that starts the
watchdog on module insertion. The default value is 0 which does not
start the watchdog - which also does not change the behaviour if the


behavior


parameter is not given.

Signed-off-by: Lars Poeschel poesc...@lemonage.de
---
  Documentation/watchdog/watchdog-parameters.txt |  1 +
  drivers/watchdog/omap_wdt.c| 10 ++
  2 files changed, 11 insertions(+)

diff --git a/Documentation/watchdog/watchdog-parameters.txt 
b/Documentation/watchdog/watchdog-parameters.txt
index 692791c..fb023bb 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -208,6 +208,7 @@ nowayout: Watchdog cannot be stopped once started
  -
  omap_wdt:
  timer_margin: initial watchdog timeout (in seconds)
+start_enabled: Watchdog is started on module insertion (default=0)


Please name the parameter early_enable. We have one driver with early_disable,
so this makes it at least to some degree generic.


  -
  orion_wdt:
  heartbeat: Initial watchdog heartbeat in seconds
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index a29a61d..7dc8de7 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -53,6 +53,13 @@ static unsigned timer_margin;
  module_param(timer_margin, uint, 0);
  MODULE_PARM_DESC(timer_margin, initial watchdog timeout (in seconds));

+#define START_DEFAULT  0
+static bool start_enabled = START_DEFAULT;


Please drop the define. Static variables do not need to be initialized to 0.
Cirumventing that with a define to defeat the resulting warning isn't really
the idea.


+module_param(start_enabled, bool, 0);
+MODULE_PARM_DESC(start_enabled,
+   Watchdog is started on module insertion (default=
+   __MODULE_STRING(START_DEFAULT) ));


and just make this default=0.

Thanks,
Guenter

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] watchdog: omap_wdt: start_enabled module parameter

2015-06-19 Thread poeschel
From: Lars Poeschel poesc...@lemonage.de

Add a start_enabled module parameter to the omap_wdt that starts the
watchdog on module insertion. The default value is 0 which does not
start the watchdog - which also does not change the behaviour if the
parameter is not given.

Signed-off-by: Lars Poeschel poesc...@lemonage.de
---
 Documentation/watchdog/watchdog-parameters.txt |  1 +
 drivers/watchdog/omap_wdt.c| 10 ++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/watchdog/watchdog-parameters.txt 
b/Documentation/watchdog/watchdog-parameters.txt
index 692791c..fb023bb 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -208,6 +208,7 @@ nowayout: Watchdog cannot be stopped once started
 -
 omap_wdt:
 timer_margin: initial watchdog timeout (in seconds)
+start_enabled: Watchdog is started on module insertion (default=0)
 -
 orion_wdt:
 heartbeat: Initial watchdog heartbeat in seconds
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index a29a61d..7dc8de7 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -53,6 +53,13 @@ static unsigned timer_margin;
 module_param(timer_margin, uint, 0);
 MODULE_PARM_DESC(timer_margin, initial watchdog timeout (in seconds));
 
+#define START_DEFAULT  0
+static bool start_enabled = START_DEFAULT;
+module_param(start_enabled, bool, 0);
+MODULE_PARM_DESC(start_enabled,
+   Watchdog is started on module insertion (default=
+   __MODULE_STRING(START_DEFAULT) ));
+
 struct omap_wdt_dev {
void __iomem*base;  /* physical */
struct device   *dev;
@@ -280,6 +287,9 @@ static int omap_wdt_probe(struct platform_device *pdev)
 
pm_runtime_put_sync(wdev-dev);
 
+   if (start_enabled)
+   omap_wdt_start(omap_wdt);
+
return 0;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/