All these drivers is using watchdog_init_timeout() to set timeout.
If the timeout-parameter is set to an valid value, it will allways pick
that and not even consider if timeout-secs is set in the devicetree.
Most of the patches will just remove the initial value for
timeout-parameter.
Some of the drivers allready has documented device-tree-bindings for
timeout-secs (but will not work), add property for those which not.
I wrote a similiar (tested) patch for imx2 and simply did the same to these drivers.
These patches is *NOT* tested, so please review extra carefully.
Taken from Documentation/watchdog/watchdog-kernel-api.txt:
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.
Best regards
Marcus Folkesson
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson <[email protected]>
---
Documentation/devicetree/bindings/watchdog/meson-wdt.txt | 4 ++++
drivers/watchdog/meson_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
index 8a6d84cb36c9..7588cc3971bf 100644
--- a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
@@ -9,9 +9,13 @@ Required properties:
"amlogic,meson8m2-wdt" and "amlogic,meson8b-wdt" on Meson8m2 SoCs
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
Example:
wdt: watchdog@c1109900 {
compatible = "amlogic,meson6-wdt";
reg = <0xc1109900 0x8>;
+ timeout-sec = <10>;
};
diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
index 304274c67735..cd0275a6cdac 100644
--- a/drivers/watchdog/meson_wdt.c
+++ b/drivers/watchdog/meson_wdt.c
@@ -36,7 +36,7 @@
#define MESON_SEC_TO_TC(s, c) ((s) * (c))
static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int timeout = MESON_WDT_TIMEOUT;
+static unsigned int timeout;
struct meson_wdt_data {
unsigned int enable;
--
2.15.1
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
Following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt to make use of
the parameter logic.
Signed-off-by: Marcus Folkesson <[email protected]>
---
drivers/watchdog/coh901327_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/coh901327_wdt.c b/drivers/watchdog/coh901327_wdt.c
index 4410337f4f7f..437f865e5c6b 100644
--- a/drivers/watchdog/coh901327_wdt.c
+++ b/drivers/watchdog/coh901327_wdt.c
@@ -67,7 +67,7 @@
#define U300_WDOG_IFR_WILL_BARK_IRQ_FORCE_ENABLE 0x0001U
/* Default timeout in seconds = 1 minute */
-static unsigned int margin = 60;
+static unsigned int margin;
static int irq;
static void __iomem *virtbase;
static struct device *parent;
--
2.15.1
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
Following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt to make use of
the parameter logic.
Signed-off-by: Marcus Folkesson <[email protected]>
---
drivers/watchdog/pnx4008_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
index 0529aed158a4..8e261799c84e 100644
--- a/drivers/watchdog/pnx4008_wdt.c
+++ b/drivers/watchdog/pnx4008_wdt.c
@@ -78,7 +78,7 @@
#define WDOG_COUNTER_RATE 13000000 /*the counter clock is 13 MHz fixed */
static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int heartbeat = DEFAULT_HEARTBEAT;
+static unsigned int heartbeat;
static DEFINE_SPINLOCK(io_lock);
static void __iomem *wdt_base;
--
2.15.1
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson <[email protected]>
---
Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt | 4 ++++
drivers/watchdog/sunxi_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
index 62dd5baad70e..49900e72f6b1 100644
--- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
@@ -6,9 +6,13 @@ Required properties:
"allwinner,sun6i-a31-wdt"
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+- timeout-sec : Contains the watchdog timeout in seconds
+
Example:
wdt: watchdog@1c20c90 {
compatible = "allwinner,sun4i-a10-wdt";
reg = <0x01c20c90 0x10>;
+ timeout-sec = <10>;
};
diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
index 9728fa32c357..55f166bec0ca 100644
--- a/drivers/watchdog/sunxi_wdt.c
+++ b/drivers/watchdog/sunxi_wdt.c
@@ -39,7 +39,7 @@
#define DRV_VERSION "1.0"
static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int timeout = WDT_MAX_TIMEOUT;
+static unsigned int timeout;
/*
* This structure stores the register offsets for different variants
--
2.15.1
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson <[email protected]>
---
Documentation/devicetree/bindings/watchdog/mtk-wdt.txt | 4 ++++
drivers/watchdog/mtk_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
index 5b38a30e608c..859dee167b91 100644
--- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
@@ -11,9 +11,13 @@ Required properties:
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
Example:
wdt: watchdog@10000000 {
compatible = "mediatek,mt6589-wdt";
reg = <0x10000000 0x18>;
+ timeout-sec = <10>;
};
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index 7ed417a765c7..fcdc10ec28a3 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -57,7 +57,7 @@
#define DRV_VERSION "1.0"
static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int timeout = WDT_MAX_TIMEOUT;
+static unsigned int timeout;
struct mtk_wdt_dev {
struct watchdog_device wdt_dev;
--
2.15.1
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson <[email protected]>
---
Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt | 4 ++++
drivers/watchdog/sirfsoc_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt b/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
index 9cbc76c89b2b..0dce5e3100b4 100644
--- a/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
@@ -5,10 +5,14 @@ Required properties:
- reg: Address range of tick timer/WDT register set
- interrupts: interrupt number to the cpu
+Optional properties:
+- timeout-sec : Contains the watchdog timeout in seconds
+
Example:
timer@b0020000 {
compatible = "sirf,prima2-tick";
reg = <0xb0020000 0x1000>;
interrupts = <0>;
+ timeout-sec = <30>;
};
diff --git a/drivers/watchdog/sirfsoc_wdt.c b/drivers/watchdog/sirfsoc_wdt.c
index 4eea351e09b0..ac0c9d2c4aee 100644
--- a/drivers/watchdog/sirfsoc_wdt.c
+++ b/drivers/watchdog/sirfsoc_wdt.c
@@ -29,7 +29,7 @@
#define SIRFSOC_WDT_MAX_TIMEOUT (10 * 60) /* 10 mins */
#define SIRFSOC_WDT_DEFAULT_TIMEOUT 30 /* 30 secs */
-static unsigned int timeout = SIRFSOC_WDT_DEFAULT_TIMEOUT;
+static unsigned int timeout;
static bool nowayout = WATCHDOG_NOWAYOUT;
module_param(timeout, uint, 0);
--
2.15.1
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
Following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt to make use of
the parameter logic.
Signed-off-by: Marcus Folkesson <[email protected]>
---
drivers/watchdog/sama5d4_wdt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 0ae947c3d7bc..e6c679383734 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -33,7 +33,7 @@ struct sama5d4_wdt {
unsigned long last_ping;
};
-static int wdt_timeout = WDT_DEFAULT_TIMEOUT;
+static int wdt_timeout;
static bool nowayout = WATCHDOG_NOWAYOUT;
module_param(wdt_timeout, int, 0);
@@ -212,7 +212,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
return -ENOMEM;
wdd = &wdt->wdd;
- wdd->timeout = wdt_timeout;
+ wdd->timeout = WDT_DEFAULT_TIMEOUT;
wdd->info = &sama5d4_wdt_info;
wdd->ops = &sama5d4_wdt_ops;
wdd->min_timeout = MIN_WDT_TIMEOUT;
--
2.15.1
The summary email did not make it for some reason.
However.
All these drivers is using watchdog_init_timeout() to set timeout.
If the timeout-parameter is set to an valid value, it will allways pick
that and not even consider if timeout-secs is set in devicetree.
Most of the patches will just remove the initial value for
timeout-parameter.
Some of the drivers allready has documented device-tree-bindings for
timeout-secs (but will not work), add property for those which not.
I wrote a similiar (tested) patch for imx2 and simply did the same to these drivers.
These patches is *NOT* tested, so please review extra carefully.
Taken from Documentation/watchdog/watchdog-kernel-api.txt:
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.
Best regards
Marcus Folkesson
On Fri, Feb 09, 2018 at 08:27:18PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> Following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt to make use of
> the parameter logic.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/sama5d4_wdt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index 0ae947c3d7bc..e6c679383734 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c
> @@ -33,7 +33,7 @@ struct sama5d4_wdt {
> unsigned long last_ping;
> };
>
> -static int wdt_timeout = WDT_DEFAULT_TIMEOUT;
> +static int wdt_timeout;
> static bool nowayout = WATCHDOG_NOWAYOUT;
>
> module_param(wdt_timeout, int, 0);
> @@ -212,7 +212,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> wdd = &wdt->wdd;
> - wdd->timeout = wdt_timeout;
> + wdd->timeout = WDT_DEFAULT_TIMEOUT;
> wdd->info = &sama5d4_wdt_info;
> wdd->ops = &sama5d4_wdt_ops;
> wdd->min_timeout = MIN_WDT_TIMEOUT;
> --
> 2.15.1
>
On Fri, Feb 09, 2018 at 08:27:19PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> By following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt, it also
> let us to set timout-sec property in devicetree.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt | 4 ++++
> drivers/watchdog/sunxi_wdt.c | 2 +-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> index 62dd5baad70e..49900e72f6b1 100644
> --- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
> @@ -6,9 +6,13 @@ Required properties:
> "allwinner,sun6i-a31-wdt"
> - reg : Specifies base physical address and size of the registers.
>
> +Optional properties:
> +- timeout-sec : Contains the watchdog timeout in seconds
> +
> Example:
>
> wdt: watchdog@1c20c90 {
> compatible = "allwinner,sun4i-a10-wdt";
> reg = <0x01c20c90 0x10>;
> + timeout-sec = <10>;
> };
> diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
> index 9728fa32c357..55f166bec0ca 100644
> --- a/drivers/watchdog/sunxi_wdt.c
> +++ b/drivers/watchdog/sunxi_wdt.c
> @@ -39,7 +39,7 @@
> #define DRV_VERSION "1.0"
>
> static bool nowayout = WATCHDOG_NOWAYOUT;
> -static unsigned int timeout = WDT_MAX_TIMEOUT;
> +static unsigned int timeout;
>
> /*
> * This structure stores the register offsets for different variants
> --
> 2.15.1
>
On Fri, Feb 09, 2018 at 08:27:20PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> By following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt, it also
> let us to set timout-sec property in devicetree.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt | 4 ++++
> drivers/watchdog/sirfsoc_wdt.c | 2 +-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt b/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
> index 9cbc76c89b2b..0dce5e3100b4 100644
> --- a/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
> @@ -5,10 +5,14 @@ Required properties:
> - reg: Address range of tick timer/WDT register set
> - interrupts: interrupt number to the cpu
>
> +Optional properties:
> +- timeout-sec : Contains the watchdog timeout in seconds
> +
> Example:
>
> timer@b0020000 {
> compatible = "sirf,prima2-tick";
> reg = <0xb0020000 0x1000>;
> interrupts = <0>;
> + timeout-sec = <30>;
> };
> diff --git a/drivers/watchdog/sirfsoc_wdt.c b/drivers/watchdog/sirfsoc_wdt.c
> index 4eea351e09b0..ac0c9d2c4aee 100644
> --- a/drivers/watchdog/sirfsoc_wdt.c
> +++ b/drivers/watchdog/sirfsoc_wdt.c
> @@ -29,7 +29,7 @@
> #define SIRFSOC_WDT_MAX_TIMEOUT (10 * 60) /* 10 mins */
> #define SIRFSOC_WDT_DEFAULT_TIMEOUT 30 /* 30 secs */
>
> -static unsigned int timeout = SIRFSOC_WDT_DEFAULT_TIMEOUT;
> +static unsigned int timeout;
> static bool nowayout = WATCHDOG_NOWAYOUT;
>
> module_param(timeout, uint, 0);
> --
> 2.15.1
>
On Fri, Feb 09, 2018 at 08:27:21PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> Following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt to make use of
> the parameter logic.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/pnx4008_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
> index 0529aed158a4..8e261799c84e 100644
> --- a/drivers/watchdog/pnx4008_wdt.c
> +++ b/drivers/watchdog/pnx4008_wdt.c
> @@ -78,7 +78,7 @@
> #define WDOG_COUNTER_RATE 13000000 /*the counter clock is 13 MHz fixed */
>
> static bool nowayout = WATCHDOG_NOWAYOUT;
> -static unsigned int heartbeat = DEFAULT_HEARTBEAT;
> +static unsigned int heartbeat;
>
> static DEFINE_SPINLOCK(io_lock);
> static void __iomem *wdt_base;
> --
> 2.15.1
>
On Fri, Feb 09, 2018 at 08:27:21PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> Following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt to make use of
> the parameter logic.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/pnx4008_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
> index 0529aed158a4..8e261799c84e 100644
> --- a/drivers/watchdog/pnx4008_wdt.c
> +++ b/drivers/watchdog/pnx4008_wdt.c
> @@ -78,7 +78,7 @@
> #define WDOG_COUNTER_RATE 13000000 /*the counter clock is 13 MHz fixed */
>
> static bool nowayout = WATCHDOG_NOWAYOUT;
> -static unsigned int heartbeat = DEFAULT_HEARTBEAT;
> +static unsigned int heartbeat;
>
> static DEFINE_SPINLOCK(io_lock);
> static void __iomem *wdt_base;
> --
> 2.15.1
>
On Fri, Feb 09, 2018 at 08:27:22PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> By following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt, it also
> let us to set timout-sec property in devicetree.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> Documentation/devicetree/bindings/watchdog/mtk-wdt.txt | 4 ++++
> drivers/watchdog/mtk_wdt.c | 2 +-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> index 5b38a30e608c..859dee167b91 100644
> --- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
> @@ -11,9 +11,13 @@ Required properties:
>
> - reg : Specifies base physical address and size of the registers.
>
> +Optional properties:
> +- timeout-sec: contains the watchdog timeout in seconds.
> +
> Example:
>
> wdt: watchdog@10000000 {
> compatible = "mediatek,mt6589-wdt";
> reg = <0x10000000 0x18>;
> + timeout-sec = <10>;
> };
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 7ed417a765c7..fcdc10ec28a3 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -57,7 +57,7 @@
> #define DRV_VERSION "1.0"
>
> static bool nowayout = WATCHDOG_NOWAYOUT;
> -static unsigned int timeout = WDT_MAX_TIMEOUT;
> +static unsigned int timeout;
>
> struct mtk_wdt_dev {
> struct watchdog_device wdt_dev;
> --
> 2.15.1
>
On Fri, Feb 09, 2018 at 08:27:23PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> By following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt, it also
> let us to set timout-sec property in devicetree.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
Reviewed-by: Guenetr Roeck <[email protected]>
> ---
> Documentation/devicetree/bindings/watchdog/meson-wdt.txt | 4 ++++
> drivers/watchdog/meson_wdt.c | 2 +-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
> index 8a6d84cb36c9..7588cc3971bf 100644
> --- a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
> @@ -9,9 +9,13 @@ Required properties:
> "amlogic,meson8m2-wdt" and "amlogic,meson8b-wdt" on Meson8m2 SoCs
> - reg : Specifies base physical address and size of the registers.
>
> +Optional properties:
> +- timeout-sec: contains the watchdog timeout in seconds.
> +
> Example:
>
> wdt: watchdog@c1109900 {
> compatible = "amlogic,meson6-wdt";
> reg = <0xc1109900 0x8>;
> + timeout-sec = <10>;
> };
> diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
> index 304274c67735..cd0275a6cdac 100644
> --- a/drivers/watchdog/meson_wdt.c
> +++ b/drivers/watchdog/meson_wdt.c
> @@ -36,7 +36,7 @@
> #define MESON_SEC_TO_TC(s, c) ((s) * (c))
>
> static bool nowayout = WATCHDOG_NOWAYOUT;
> -static unsigned int timeout = MESON_WDT_TIMEOUT;
> +static unsigned int timeout;
>
> struct meson_wdt_data {
> unsigned int enable;
> --
> 2.15.1
>
On Fri, Feb 09, 2018 at 08:27:24PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> Following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt to make use of
> the parameter logic.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
> ---
> drivers/watchdog/coh901327_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/coh901327_wdt.c b/drivers/watchdog/coh901327_wdt.c
> index 4410337f4f7f..437f865e5c6b 100644
> --- a/drivers/watchdog/coh901327_wdt.c
> +++ b/drivers/watchdog/coh901327_wdt.c
> @@ -67,7 +67,7 @@
> #define U300_WDOG_IFR_WILL_BARK_IRQ_FORCE_ENABLE 0x0001U
>
> /* Default timeout in seconds = 1 minute */
> -static unsigned int margin = 60;
> +static unsigned int margin;
> static int irq;
> static void __iomem *virtbase;
> static struct device *parent;
I would suggest to initialize .timeout in 'static struct watchdog_device
coh901327_wdt', and drop the error check when calling watchdog_init_timeout().
Guenter
Hi Guenter,
On 09/02/2018 at 14:48:49 -0800, Guenter Roeck wrote:
> On Fri, Feb 09, 2018 at 08:27:23PM +0100, Marcus Folkesson wrote:
> > watchdog_init_timeout() will allways pick timeout_param since it
> > defaults to a valid timeout.
> >
> > By following best practice described in
> > Documentation/watchdog/watchdog-kernel-api.txt, it also
> > let us to set timout-sec property in devicetree.
> >
> > Signed-off-by: Marcus Folkesson <[email protected]>
>
> Reviewed-by: Guenetr Roeck <[email protected]>
^
A small typo in your name
> > ---
> > Documentation/devicetree/bindings/watchdog/meson-wdt.txt | 4 ++++
> > drivers/watchdog/meson_wdt.c | 2 +-
> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
> > index 8a6d84cb36c9..7588cc3971bf 100644
> > --- a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
> > +++ b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
> > @@ -9,9 +9,13 @@ Required properties:
> > "amlogic,meson8m2-wdt" and "amlogic,meson8b-wdt" on Meson8m2 SoCs
> > - reg : Specifies base physical address and size of the registers.
> >
> > +Optional properties:
> > +- timeout-sec: contains the watchdog timeout in seconds.
> > +
> > Example:
> >
> > wdt: watchdog@c1109900 {
> > compatible = "amlogic,meson6-wdt";
> > reg = <0xc1109900 0x8>;
> > + timeout-sec = <10>;
> > };
> > diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
> > index 304274c67735..cd0275a6cdac 100644
> > --- a/drivers/watchdog/meson_wdt.c
> > +++ b/drivers/watchdog/meson_wdt.c
> > @@ -36,7 +36,7 @@
> > #define MESON_SEC_TO_TC(s, c) ((s) * (c))
> >
> > static bool nowayout = WATCHDOG_NOWAYOUT;
> > -static unsigned int timeout = MESON_WDT_TIMEOUT;
> > +static unsigned int timeout;
> >
> > struct meson_wdt_data {
> > unsigned int enable;
> > --
> > 2.15.1
> >
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
On Sat, Feb 10, 2018 at 12:19:10AM +0100, Alexandre Belloni wrote:
> Hi Guenter,
>
> On 09/02/2018 at 14:48:49 -0800, Guenter Roeck wrote:
> > On Fri, Feb 09, 2018 at 08:27:23PM +0100, Marcus Folkesson wrote:
> > > watchdog_init_timeout() will allways pick timeout_param since it
> > > defaults to a valid timeout.
> > >
> > > By following best practice described in
> > > Documentation/watchdog/watchdog-kernel-api.txt, it also
> > > let us to set timout-sec property in devicetree.
> > >
> > > Signed-off-by: Marcus Folkesson <[email protected]>
> >
> > Reviewed-by: Guenetr Roeck <[email protected]>
> ^
> A small typo in your name
>
Mr. Fat Fingers At Work :-)
Guenter
Hi,
On Fri, Feb 09, 2018 at 02:52:00PM -0800, Guenter Roeck wrote:
> On Fri, Feb 09, 2018 at 08:27:24PM +0100, Marcus Folkesson wrote:
> > watchdog_init_timeout() will allways pick timeout_param since it
> > defaults to a valid timeout.
> >
> > Following best practice described in
> > Documentation/watchdog/watchdog-kernel-api.txt to make use of
> > the parameter logic.
> >
> > Signed-off-by: Marcus Folkesson <[email protected]>
> > ---
> > drivers/watchdog/coh901327_wdt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/watchdog/coh901327_wdt.c b/drivers/watchdog/coh901327_wdt.c
> > index 4410337f4f7f..437f865e5c6b 100644
> > --- a/drivers/watchdog/coh901327_wdt.c
> > +++ b/drivers/watchdog/coh901327_wdt.c
> > @@ -67,7 +67,7 @@
> > #define U300_WDOG_IFR_WILL_BARK_IRQ_FORCE_ENABLE 0x0001U
> >
> > /* Default timeout in seconds = 1 minute */
> > -static unsigned int margin = 60;
> > +static unsigned int margin;
> > static int irq;
> > static void __iomem *virtbase;
> > static struct device *parent;
>
> I would suggest to initialize .timeout in 'static struct watchdog_device
> coh901327_wdt', and drop the error check when calling watchdog_init_timeout().
Will do, thank you.
>
> Guenter
Best regards
Marcus Folkesson
On Fri, Feb 09, 2018 at 08:27:18PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> Following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt to make use of
> the parameter logic.
>
> Signed-off-by: Marcus Folkesson <[email protected]>
> Reviewed-by: Guenter Roeck <[email protected]>
Unfortunately I'll have to withdraw the Reviewed-by:. wdt_timeout is used at
the end of the probe function to display the selected timeout. This will have
to be changed to display the actual timeout.
Guenter
> ---
> drivers/watchdog/sama5d4_wdt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index 0ae947c3d7bc..e6c679383734 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c
> @@ -33,7 +33,7 @@ struct sama5d4_wdt {
> unsigned long last_ping;
> };
>
> -static int wdt_timeout = WDT_DEFAULT_TIMEOUT;
> +static int wdt_timeout;
> static bool nowayout = WATCHDOG_NOWAYOUT;
>
> module_param(wdt_timeout, int, 0);
> @@ -212,7 +212,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> wdd = &wdt->wdd;
> - wdd->timeout = wdt_timeout;
> + wdd->timeout = WDT_DEFAULT_TIMEOUT;
> wdd->info = &sama5d4_wdt_info;
> wdd->ops = &sama5d4_wdt_ops;
> wdd->min_timeout = MIN_WDT_TIMEOUT;