[PATCH 0/1] stm32_iwdg: set WDOG_HW_RUNNING at probe

[PATCH 0/1] stm32_iwdg: set WDOG_HW_RUNNING at probe

[Christophe Roullier]

If the watchdog hardware is already enabled during the boot process,
when the Linux watchdog driver loads, it should reset the watchdog and
tell the watchdog framework. As a result, ping can be generated from
the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
until the userspace watchdog daemon takes over control

Verified :
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE is not set + daemon watchdog in userland ON ==> No reset IWDG2
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE is not set ==> Reset IWDG2
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE=y ==> No reset IWDG2
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE=y + daemon watchdog in userland ON puis OFF ==> Reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE is not set ==> No reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE=y ==> No reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE=y + daemon watchdog in userland ON ==> No reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE=y + daemon watchdog in userland ON puis OFF ==> Reset IWDG2

Christophe Roullier (1):
  drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

 drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 17 deletions(-)

-- 
2.17.1

[PATCH 0/1] stm32_iwdg: set WDOG_HW_RUNNING at probe

[Christophe Roullier]

If the watchdog hardware is already enabled during the boot process,
when the Linux watchdog driver loads, it should reset the watchdog and
tell the watchdog framework. As a result, ping can be generated from
the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
until the userspace watchdog daemon takes over control

Verified :
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE is not set + daemon watchdog in userland ON ==> No reset IWDG2
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE is not set ==> Reset IWDG2
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE=y ==> No reset IWDG2
Watchdog enable in Uboot + HANDLE_BOOT_ENABLE=y + daemon watchdog in userland ON puis OFF ==> Reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE is not set ==> No reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE=y ==> No reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE=y + daemon watchdog in userland ON ==> No reset IWDG2
Watchdog disable in Uboot + HANDLE_BOOT_ENABLE=y + daemon watchdog in userland ON puis OFF ==> Reset IWDG2

Christophe Roullier (1):
  drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

 drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 17 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Christophe Roullier]

If the watchdog hardware is already enabled during the boot process,
when the Linux watchdog driver loads, it should reset the watchdog and
tell the watchdog framework. As a result, ping can be generated from
the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
until the userspace watchdog daemon takes over control

Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index a3a329011a06..2b3be3b1c15b 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, u32 reg, u32 val)
 static int stm32_iwdg_start(struct watchdog_device *wdd)
 {
 	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
-	u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
-	int ret;
+
+	dev_dbg(wdd->parent, "%s\n", __func__);
+
+	/*  Start the watchdog */
+	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
+
+	/* reload watchdog */
+	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
+
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+	return 0;
+}
+
+static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
+{
+	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
+	u32 tout, presc, iwdg_rlr, iwdg_pr;
 
 	dev_dbg(wdd->parent, "%s\n", __func__);
 
@@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct watchdog_device *wdd)
 	/* set prescaler & reload registers */
 	reg_write(wdt->regs, IWDG_PR, iwdg_pr);
 	reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
-	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
-
-	/* wait for the registers to be updated (max 100ms) */
-	ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
-					 !(iwdg_sr & (SR_PVU | SR_RVU)),
-					 SLEEP_US, TIMEOUT_US);
-	if (ret) {
-		dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
-		return ret;
-	}
-
-	/* reload watchdog */
-	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
 
 	return 0;
 }
@@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
 
 	dev_dbg(wdd->parent, "%s\n", __func__);
 
+	/*  Start the watchdog */
+	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
+
 	/* reload watchdog */
 	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
 
@@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
 static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
 				  unsigned int timeout)
 {
+	int ret;
+
 	dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
 
 	wdd->timeout = timeout;
 
-	if (watchdog_active(wdd))
-		return stm32_iwdg_start(wdd);
+	if (watchdog_active(wdd)) {
+		ret = stm32_iwdg_setprescaler(wdd);
+		if (ret) {
+			dev_err(wdd->parent, "failed to set prescaler\n");
+			return ret;
+		} else {
+			return stm32_iwdg_start(wdd);
+		}
+	}
 
 	return 0;
 }
@@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct platform_device *pdev)
 	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
 	watchdog_init_timeout(wdd, 0, dev);
 
+	/* Make sure the watchdog is serviced */
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+
 	ret = devm_watchdog_register_device(dev, wdd);
 	if (ret)
 		return ret;
 
 	platform_set_drvdata(pdev, wdt);
 
+	ret = stm32_iwdg_setprescaler(wdd);
+	if (ret) {
+		dev_err(dev, "failed to set prescaler\n");
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Christophe Roullier]

If the watchdog hardware is already enabled during the boot process,
when the Linux watchdog driver loads, it should reset the watchdog and
tell the watchdog framework. As a result, ping can be generated from
the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
until the userspace watchdog daemon takes over control

Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index a3a329011a06..2b3be3b1c15b 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, u32 reg, u32 val)
 static int stm32_iwdg_start(struct watchdog_device *wdd)
 {
 	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
-	u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
-	int ret;
+
+	dev_dbg(wdd->parent, "%s\n", __func__);
+
+	/*  Start the watchdog */
+	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
+
+	/* reload watchdog */
+	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
+
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+	return 0;
+}
+
+static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
+{
+	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
+	u32 tout, presc, iwdg_rlr, iwdg_pr;
 
 	dev_dbg(wdd->parent, "%s\n", __func__);
 
@@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct watchdog_device *wdd)
 	/* set prescaler & reload registers */
 	reg_write(wdt->regs, IWDG_PR, iwdg_pr);
 	reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
-	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
-
-	/* wait for the registers to be updated (max 100ms) */
-	ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
-					 !(iwdg_sr & (SR_PVU | SR_RVU)),
-					 SLEEP_US, TIMEOUT_US);
-	if (ret) {
-		dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
-		return ret;
-	}
-
-	/* reload watchdog */
-	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
 
 	return 0;
 }
@@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
 
 	dev_dbg(wdd->parent, "%s\n", __func__);
 
+	/*  Start the watchdog */
+	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
+
 	/* reload watchdog */
 	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
 
@@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
 static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
 				  unsigned int timeout)
 {
+	int ret;
+
 	dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
 
 	wdd->timeout = timeout;
 
-	if (watchdog_active(wdd))
-		return stm32_iwdg_start(wdd);
+	if (watchdog_active(wdd)) {
+		ret = stm32_iwdg_setprescaler(wdd);
+		if (ret) {
+			dev_err(wdd->parent, "failed to set prescaler\n");
+			return ret;
+		} else {
+			return stm32_iwdg_start(wdd);
+		}
+	}
 
 	return 0;
 }
@@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct platform_device *pdev)
 	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
 	watchdog_init_timeout(wdd, 0, dev);
 
+	/* Make sure the watchdog is serviced */
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+
 	ret = devm_watchdog_register_device(dev, wdd);
 	if (ret)
 		return ret;
 
 	platform_set_drvdata(pdev, wdt);
 
+	ret = stm32_iwdg_setprescaler(wdd);
+	if (ret) {
+		dev_err(dev, "failed to set prescaler\n");
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Guenter Roeck]

On 11/21/19 12:28 AM, Christophe Roullier wrote:
> If the watchdog hardware is already enabled during the boot process,
> when the Linux watchdog driver loads, it should reset the watchdog and
> tell the watchdog framework. As a result, ping can be generated from
> the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
> until the userspace watchdog daemon takes over control
> 

This is not what the code is doing. It sets the WDOG_HW_RUNNING flag
unconditionally, no matter if the watchdog is already running or not.
It also changes the semantic of the rest of the code, as well as
functionality. The code in start_timeout no longer waits, and the ping
code explicitly (re-)enables the watchdog.

If you want an option to start the watchdog at probe time unconditionally,
please add a module parameter to do it. Otherwise you'll need to check if
it is indeed enabled before setting WDOG_HW_RUNNING, and in that case it
should not be necessary to re-enable it. It should also not be necessary
to split the start function.

Thanks,
Guenter

> Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> ---
>   drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
>   1 file changed, 40 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> index a3a329011a06..2b3be3b1c15b 100644
> --- a/drivers/watchdog/stm32_iwdg.c
> +++ b/drivers/watchdog/stm32_iwdg.c
> @@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, u32 reg, u32 val)
>   static int stm32_iwdg_start(struct watchdog_device *wdd)
>   {
>   	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> -	u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
> -	int ret;
> +
> +	dev_dbg(wdd->parent, "%s\n", __func__);
> +
> +	/*  Start the watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> +
> +	/* reload watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> +
> +	set_bit(WDOG_HW_RUNNING, &wdd->status);
> +	return 0;
> +}
> +
> +static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
> +{
> +	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> +	u32 tout, presc, iwdg_rlr, iwdg_pr;
>   
>   	dev_dbg(wdd->parent, "%s\n", __func__);
>   
> @@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct watchdog_device *wdd)
>   	/* set prescaler & reload registers */
>   	reg_write(wdt->regs, IWDG_PR, iwdg_pr);
>   	reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
> -	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> -
> -	/* wait for the registers to be updated (max 100ms) */
> -	ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
> -					 !(iwdg_sr & (SR_PVU | SR_RVU)),
> -					 SLEEP_US, TIMEOUT_US);
> -	if (ret) {
> -		dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
> -		return ret;
> -	}
> -
> -	/* reload watchdog */
> -	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>   
>   	return 0;
>   }
> @@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
>   
>   	dev_dbg(wdd->parent, "%s\n", __func__);
>   
> +	/*  Start the watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> +
>   	/* reload watchdog */
>   	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>   
> @@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
>   static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
>   				  unsigned int timeout)
>   {
> +	int ret;
> +
>   	dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
>   
>   	wdd->timeout = timeout;
>   
> -	if (watchdog_active(wdd))
> -		return stm32_iwdg_start(wdd);
> +	if (watchdog_active(wdd)) {
> +		ret = stm32_iwdg_setprescaler(wdd);
> +		if (ret) {
> +			dev_err(wdd->parent, "failed to set prescaler\n");
> +			return ret;
> +		} else {
> +			return stm32_iwdg_start(wdd);
> +		}
> +	}
>   
>   	return 0;
>   }
> @@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct platform_device *pdev)
>   	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>   	watchdog_init_timeout(wdd, 0, dev);
>   
> +	/* Make sure the watchdog is serviced */
> +	set_bit(WDOG_HW_RUNNING, &wdd->status);
> +
>   	ret = devm_watchdog_register_device(dev, wdd);
>   	if (ret)
>   		return ret;
>   
>   	platform_set_drvdata(pdev, wdt);
>   
> +	ret = stm32_iwdg_setprescaler(wdd);
> +	if (ret) {
> +		dev_err(dev, "failed to set prescaler\n");
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>   
> 

Re: [PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Guenter Roeck]

On 11/21/19 12:28 AM, Christophe Roullier wrote:
> If the watchdog hardware is already enabled during the boot process,
> when the Linux watchdog driver loads, it should reset the watchdog and
> tell the watchdog framework. As a result, ping can be generated from
> the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
> until the userspace watchdog daemon takes over control
> 

This is not what the code is doing. It sets the WDOG_HW_RUNNING flag
unconditionally, no matter if the watchdog is already running or not.
It also changes the semantic of the rest of the code, as well as
functionality. The code in start_timeout no longer waits, and the ping
code explicitly (re-)enables the watchdog.

If you want an option to start the watchdog at probe time unconditionally,
please add a module parameter to do it. Otherwise you'll need to check if
it is indeed enabled before setting WDOG_HW_RUNNING, and in that case it
should not be necessary to re-enable it. It should also not be necessary
to split the start function.

Thanks,
Guenter

> Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> ---
>   drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
>   1 file changed, 40 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> index a3a329011a06..2b3be3b1c15b 100644
> --- a/drivers/watchdog/stm32_iwdg.c
> +++ b/drivers/watchdog/stm32_iwdg.c
> @@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, u32 reg, u32 val)
>   static int stm32_iwdg_start(struct watchdog_device *wdd)
>   {
>   	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> -	u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
> -	int ret;
> +
> +	dev_dbg(wdd->parent, "%s\n", __func__);
> +
> +	/*  Start the watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> +
> +	/* reload watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> +
> +	set_bit(WDOG_HW_RUNNING, &wdd->status);
> +	return 0;
> +}
> +
> +static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
> +{
> +	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> +	u32 tout, presc, iwdg_rlr, iwdg_pr;
>   
>   	dev_dbg(wdd->parent, "%s\n", __func__);
>   
> @@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct watchdog_device *wdd)
>   	/* set prescaler & reload registers */
>   	reg_write(wdt->regs, IWDG_PR, iwdg_pr);
>   	reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
> -	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> -
> -	/* wait for the registers to be updated (max 100ms) */
> -	ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
> -					 !(iwdg_sr & (SR_PVU | SR_RVU)),
> -					 SLEEP_US, TIMEOUT_US);
> -	if (ret) {
> -		dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
> -		return ret;
> -	}
> -
> -	/* reload watchdog */
> -	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>   
>   	return 0;
>   }
> @@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
>   
>   	dev_dbg(wdd->parent, "%s\n", __func__);
>   
> +	/*  Start the watchdog */
> +	reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> +
>   	/* reload watchdog */
>   	reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>   
> @@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd)
>   static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
>   				  unsigned int timeout)
>   {
> +	int ret;
> +
>   	dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
>   
>   	wdd->timeout = timeout;
>   
> -	if (watchdog_active(wdd))
> -		return stm32_iwdg_start(wdd);
> +	if (watchdog_active(wdd)) {
> +		ret = stm32_iwdg_setprescaler(wdd);
> +		if (ret) {
> +			dev_err(wdd->parent, "failed to set prescaler\n");
> +			return ret;
> +		} else {
> +			return stm32_iwdg_start(wdd);
> +		}
> +	}
>   
>   	return 0;
>   }
> @@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct platform_device *pdev)
>   	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>   	watchdog_init_timeout(wdd, 0, dev);
>   
> +	/* Make sure the watchdog is serviced */
> +	set_bit(WDOG_HW_RUNNING, &wdd->status);
> +
>   	ret = devm_watchdog_register_device(dev, wdd);
>   	if (ret)
>   		return ret;
>   
>   	platform_set_drvdata(pdev, wdt);
>   
> +	ret = stm32_iwdg_setprescaler(wdd);
> +	if (ret) {
> +		dev_err(dev, "failed to set prescaler\n");
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>   
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Christophe ROULLIER]

Hi Guenter,

On 11/21/19 10:53 AM, Guenter Roeck wrote:
> On 11/21/19 12:28 AM, Christophe Roullier wrote:
>> If the watchdog hardware is already enabled during the boot process,
>> when the Linux watchdog driver loads, it should reset the watchdog and
>> tell the watchdog framework. As a result, ping can be generated from
>> the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
>> until the userspace watchdog daemon takes over control
>>
>
> This is not what the code is doing. It sets the WDOG_HW_RUNNING flag
> unconditionally, no matter if the watchdog is already running or not.
> It also changes the semantic of the rest of the code, as well as
> functionality. The code in start_timeout no longer waits, and the ping
> code explicitly (re-)enables the watchdog.
>
> If you want an option to start the watchdog at probe time 
> unconditionally,
> please add a module parameter to do it. Otherwise you'll need to check if
> it is indeed enabled before setting WDOG_HW_RUNNING, and in that case it
> should not be necessary to re-enable it. It should also not be necessary
> to split the start function.

With our IWDG IP, there is no way to read values from hardware (if it is 
running or not)

We are in same case of intel-mid_wdt.c driver, we have Uboot which can

leaves the watchdog running. So we need to force the kicking of our 
watchdog.

Thanks

Christophe

>
> Thanks,
> Guenter
>
>> Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>>   drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
>>   1 file changed, 40 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/watchdog/stm32_iwdg.c 
>> b/drivers/watchdog/stm32_iwdg.c
>> index a3a329011a06..2b3be3b1c15b 100644
>> --- a/drivers/watchdog/stm32_iwdg.c
>> +++ b/drivers/watchdog/stm32_iwdg.c
>> @@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, 
>> u32 reg, u32 val)
>>   static int stm32_iwdg_start(struct watchdog_device *wdd)
>>   {
>>       struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
>> -    u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
>> -    int ret;
>> +
>> +    dev_dbg(wdd->parent, "%s\n", __func__);
>> +
>> +    /*  Start the watchdog */
>> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
>> +
>> +    /* reload watchdog */
>> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>> +
>> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
>> +    return 0;
>> +}
>> +
>> +static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
>> +{
>> +    struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
>> +    u32 tout, presc, iwdg_rlr, iwdg_pr;
>>         dev_dbg(wdd->parent, "%s\n", __func__);
>>   @@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct 
>> watchdog_device *wdd)
>>       /* set prescaler & reload registers */
>>       reg_write(wdt->regs, IWDG_PR, iwdg_pr);
>>       reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
>> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
>> -
>> -    /* wait for the registers to be updated (max 100ms) */
>> -    ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
>> -                     !(iwdg_sr & (SR_PVU | SR_RVU)),
>> -                     SLEEP_US, TIMEOUT_US);
>> -    if (ret) {
>> -        dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
>> -        return ret;
>> -    }
>> -
>> -    /* reload watchdog */
>> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>>         return 0;
>>   }
>> @@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device 
>> *wdd)
>>         dev_dbg(wdd->parent, "%s\n", __func__);
>>   +    /*  Start the watchdog */
>> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
>> +
>>       /* reload watchdog */
>>       reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>>   @@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct 
>> watchdog_device *wdd)
>>   static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
>>                     unsigned int timeout)
>>   {
>> +    int ret;
>> +
>>       dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
>>         wdd->timeout = timeout;
>>   -    if (watchdog_active(wdd))
>> -        return stm32_iwdg_start(wdd);
>> +    if (watchdog_active(wdd)) {
>> +        ret = stm32_iwdg_setprescaler(wdd);
>> +        if (ret) {
>> +            dev_err(wdd->parent, "failed to set prescaler\n");
>> +            return ret;
>> +        } else {
>> +            return stm32_iwdg_start(wdd);
>> +        }
>> +    }
>>         return 0;
>>   }
>> @@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct 
>> platform_device *pdev)
>>       watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>>       watchdog_init_timeout(wdd, 0, dev);
>>   +    /* Make sure the watchdog is serviced */
>> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
>> +
>>       ret = devm_watchdog_register_device(dev, wdd);
>>       if (ret)
>>           return ret;
>>         platform_set_drvdata(pdev, wdt);
>>   +    ret = stm32_iwdg_setprescaler(wdd);
>> +    if (ret) {
>> +        dev_err(dev, "failed to set prescaler\n");
>> +        return ret;
>> +    }
>> +
>>       return 0;
>>   }
>>
>

Re: [PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Christophe ROULLIER]

Hi Guenter,

On 11/21/19 10:53 AM, Guenter Roeck wrote:
> On 11/21/19 12:28 AM, Christophe Roullier wrote:
>> If the watchdog hardware is already enabled during the boot process,
>> when the Linux watchdog driver loads, it should reset the watchdog and
>> tell the watchdog framework. As a result, ping can be generated from
>> the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
>> until the userspace watchdog daemon takes over control
>>
>
> This is not what the code is doing. It sets the WDOG_HW_RUNNING flag
> unconditionally, no matter if the watchdog is already running or not.
> It also changes the semantic of the rest of the code, as well as
> functionality. The code in start_timeout no longer waits, and the ping
> code explicitly (re-)enables the watchdog.
>
> If you want an option to start the watchdog at probe time 
> unconditionally,
> please add a module parameter to do it. Otherwise you'll need to check if
> it is indeed enabled before setting WDOG_HW_RUNNING, and in that case it
> should not be necessary to re-enable it. It should also not be necessary
> to split the start function.

With our IWDG IP, there is no way to read values from hardware (if it is 
running or not)

We are in same case of intel-mid_wdt.c driver, we have Uboot which can

leaves the watchdog running. So we need to force the kicking of our 
watchdog.

Thanks

Christophe

>
> Thanks,
> Guenter
>
>> Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>>   drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
>>   1 file changed, 40 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/watchdog/stm32_iwdg.c 
>> b/drivers/watchdog/stm32_iwdg.c
>> index a3a329011a06..2b3be3b1c15b 100644
>> --- a/drivers/watchdog/stm32_iwdg.c
>> +++ b/drivers/watchdog/stm32_iwdg.c
>> @@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, 
>> u32 reg, u32 val)
>>   static int stm32_iwdg_start(struct watchdog_device *wdd)
>>   {
>>       struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
>> -    u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
>> -    int ret;
>> +
>> +    dev_dbg(wdd->parent, "%s\n", __func__);
>> +
>> +    /*  Start the watchdog */
>> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
>> +
>> +    /* reload watchdog */
>> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>> +
>> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
>> +    return 0;
>> +}
>> +
>> +static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
>> +{
>> +    struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
>> +    u32 tout, presc, iwdg_rlr, iwdg_pr;
>>         dev_dbg(wdd->parent, "%s\n", __func__);
>>   @@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct 
>> watchdog_device *wdd)
>>       /* set prescaler & reload registers */
>>       reg_write(wdt->regs, IWDG_PR, iwdg_pr);
>>       reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
>> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
>> -
>> -    /* wait for the registers to be updated (max 100ms) */
>> -    ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
>> -                     !(iwdg_sr & (SR_PVU | SR_RVU)),
>> -                     SLEEP_US, TIMEOUT_US);
>> -    if (ret) {
>> -        dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
>> -        return ret;
>> -    }
>> -
>> -    /* reload watchdog */
>> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>>         return 0;
>>   }
>> @@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device 
>> *wdd)
>>         dev_dbg(wdd->parent, "%s\n", __func__);
>>   +    /*  Start the watchdog */
>> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
>> +
>>       /* reload watchdog */
>>       reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
>>   @@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct 
>> watchdog_device *wdd)
>>   static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
>>                     unsigned int timeout)
>>   {
>> +    int ret;
>> +
>>       dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
>>         wdd->timeout = timeout;
>>   -    if (watchdog_active(wdd))
>> -        return stm32_iwdg_start(wdd);
>> +    if (watchdog_active(wdd)) {
>> +        ret = stm32_iwdg_setprescaler(wdd);
>> +        if (ret) {
>> +            dev_err(wdd->parent, "failed to set prescaler\n");
>> +            return ret;
>> +        } else {
>> +            return stm32_iwdg_start(wdd);
>> +        }
>> +    }
>>         return 0;
>>   }
>> @@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct 
>> platform_device *pdev)
>>       watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>>       watchdog_init_timeout(wdd, 0, dev);
>>   +    /* Make sure the watchdog is serviced */
>> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
>> +
>>       ret = devm_watchdog_register_device(dev, wdd);
>>       if (ret)
>>           return ret;
>>         platform_set_drvdata(pdev, wdt);
>>   +    ret = stm32_iwdg_setprescaler(wdd);
>> +    if (ret) {
>> +        dev_err(dev, "failed to set prescaler\n");
>> +        return ret;
>> +    }
>> +
>>       return 0;
>>   }
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Guenter Roeck]

On Thu, Nov 21, 2019 at 01:45:21PM +0000, Christophe ROULLIER wrote:
> Hi Guenter,
> 
> On 11/21/19 10:53 AM, Guenter Roeck wrote:
> > On 11/21/19 12:28 AM, Christophe Roullier wrote:
> >> If the watchdog hardware is already enabled during the boot process,
> >> when the Linux watchdog driver loads, it should reset the watchdog and
> >> tell the watchdog framework. As a result, ping can be generated from
> >> the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
> >> until the userspace watchdog daemon takes over control
> >>
> >
> > This is not what the code is doing. It sets the WDOG_HW_RUNNING flag
> > unconditionally, no matter if the watchdog is already running or not.
> > It also changes the semantic of the rest of the code, as well as
> > functionality. The code in start_timeout no longer waits, and the ping
> > code explicitly (re-)enables the watchdog.
> >
> > If you want an option to start the watchdog at probe time 
> > unconditionally,
> > please add a module parameter to do it. Otherwise you'll need to check if
> > it is indeed enabled before setting WDOG_HW_RUNNING, and in that case it
> > should not be necessary to re-enable it. It should also not be necessary
> > to split the start function.
> 
> With our IWDG IP, there is no way to read values from hardware (if it is 
> running or not)
> 
> We are in same case of intel-mid_wdt.c driver, we have Uboot which can
> 
> leaves the watchdog running. So we need to force the kicking of our 
> watchdog.
> 

"can" or "leaves" ? 

Anyway, if that is the case, please follow the guidance from intel-mid_wdt.c,
and explain it accordingly. Alternatively, please explain why something like

	ret = wdt_start(wdt_dev);
        if (ret)
                return ret;

	/* Make sure the watchdog is serviced */
        set_bit(WDOG_HW_RUNNING, &wdt_dev->status);

would not work in your case, and why all that added complexity
is needed.

Thanks,
Guenter

> Thanks
> 
> Christophe
> 
> >
> > Thanks,
> > Guenter
> >
> >> Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
> >> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> >> ---
> >>   drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
> >>   1 file changed, 40 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/watchdog/stm32_iwdg.c 
> >> b/drivers/watchdog/stm32_iwdg.c
> >> index a3a329011a06..2b3be3b1c15b 100644
> >> --- a/drivers/watchdog/stm32_iwdg.c
> >> +++ b/drivers/watchdog/stm32_iwdg.c
> >> @@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, 
> >> u32 reg, u32 val)
> >>   static int stm32_iwdg_start(struct watchdog_device *wdd)
> >>   {
> >>       struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> >> -    u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
> >> -    int ret;
> >> +
> >> +    dev_dbg(wdd->parent, "%s\n", __func__);
> >> +
> >> +    /*  Start the watchdog */
> >> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> >> +
> >> +    /* reload watchdog */
> >> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> >> +
> >> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
> >> +    return 0;
> >> +}
> >> +
> >> +static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
> >> +{
> >> +    struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> >> +    u32 tout, presc, iwdg_rlr, iwdg_pr;
> >>         dev_dbg(wdd->parent, "%s\n", __func__);
> >>   @@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct 
> >> watchdog_device *wdd)
> >>       /* set prescaler & reload registers */
> >>       reg_write(wdt->regs, IWDG_PR, iwdg_pr);
> >>       reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
> >> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> >> -
> >> -    /* wait for the registers to be updated (max 100ms) */
> >> -    ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
> >> -                     !(iwdg_sr & (SR_PVU | SR_RVU)),
> >> -                     SLEEP_US, TIMEOUT_US);
> >> -    if (ret) {
> >> -        dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
> >> -        return ret;
> >> -    }
> >> -
> >> -    /* reload watchdog */
> >> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> >>         return 0;
> >>   }
> >> @@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device 
> >> *wdd)
> >>         dev_dbg(wdd->parent, "%s\n", __func__);
> >>   +    /*  Start the watchdog */
> >> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> >> +
> >>       /* reload watchdog */
> >>       reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> >>   @@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct 
> >> watchdog_device *wdd)
> >>   static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
> >>                     unsigned int timeout)
> >>   {
> >> +    int ret;
> >> +
> >>       dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
> >>         wdd->timeout = timeout;
> >>   -    if (watchdog_active(wdd))
> >> -        return stm32_iwdg_start(wdd);
> >> +    if (watchdog_active(wdd)) {
> >> +        ret = stm32_iwdg_setprescaler(wdd);
> >> +        if (ret) {
> >> +            dev_err(wdd->parent, "failed to set prescaler\n");
> >> +            return ret;
> >> +        } else {
> >> +            return stm32_iwdg_start(wdd);
> >> +        }
> >> +    }
> >>         return 0;
> >>   }
> >> @@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct 
> >> platform_device *pdev)
> >>       watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
> >>       watchdog_init_timeout(wdd, 0, dev);
> >>   +    /* Make sure the watchdog is serviced */
> >> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
> >> +
> >>       ret = devm_watchdog_register_device(dev, wdd);
> >>       if (ret)
> >>           return ret;
> >>         platform_set_drvdata(pdev, wdt);
> >>   +    ret = stm32_iwdg_setprescaler(wdd);
> >> +    if (ret) {
> >> +        dev_err(dev, "failed to set prescaler\n");
> >> +        return ret;
> >> +    }
> >> +
> >>       return 0;
> >>   }
> >>
> >

Re: [PATCH 1/1] drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe

[Guenter Roeck]

On Thu, Nov 21, 2019 at 01:45:21PM +0000, Christophe ROULLIER wrote:
> Hi Guenter,
> 
> On 11/21/19 10:53 AM, Guenter Roeck wrote:
> > On 11/21/19 12:28 AM, Christophe Roullier wrote:
> >> If the watchdog hardware is already enabled during the boot process,
> >> when the Linux watchdog driver loads, it should reset the watchdog and
> >> tell the watchdog framework. As a result, ping can be generated from
> >> the watchdog framework (if CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is set),
> >> until the userspace watchdog daemon takes over control
> >>
> >
> > This is not what the code is doing. It sets the WDOG_HW_RUNNING flag
> > unconditionally, no matter if the watchdog is already running or not.
> > It also changes the semantic of the rest of the code, as well as
> > functionality. The code in start_timeout no longer waits, and the ping
> > code explicitly (re-)enables the watchdog.
> >
> > If you want an option to start the watchdog at probe time 
> > unconditionally,
> > please add a module parameter to do it. Otherwise you'll need to check if
> > it is indeed enabled before setting WDOG_HW_RUNNING, and in that case it
> > should not be necessary to re-enable it. It should also not be necessary
> > to split the start function.
> 
> With our IWDG IP, there is no way to read values from hardware (if it is 
> running or not)
> 
> We are in same case of intel-mid_wdt.c driver, we have Uboot which can
> 
> leaves the watchdog running. So we need to force the kicking of our 
> watchdog.
> 

"can" or "leaves" ? 

Anyway, if that is the case, please follow the guidance from intel-mid_wdt.c,
and explain it accordingly. Alternatively, please explain why something like

	ret = wdt_start(wdt_dev);
        if (ret)
                return ret;

	/* Make sure the watchdog is serviced */
        set_bit(WDOG_HW_RUNNING, &wdt_dev->status);

would not work in your case, and why all that added complexity
is needed.

Thanks,
Guenter

> Thanks
> 
> Christophe
> 
> >
> > Thanks,
> > Guenter
> >
> >> Fixes:4332d113c66a ("watchdog: Add STM32 IWDG driver")
> >> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> >> ---
> >>   drivers/watchdog/stm32_iwdg.c | 57 ++++++++++++++++++++++++-----------
> >>   1 file changed, 40 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/watchdog/stm32_iwdg.c 
> >> b/drivers/watchdog/stm32_iwdg.c
> >> index a3a329011a06..2b3be3b1c15b 100644
> >> --- a/drivers/watchdog/stm32_iwdg.c
> >> +++ b/drivers/watchdog/stm32_iwdg.c
> >> @@ -87,8 +87,23 @@ static inline void reg_write(void __iomem *base, 
> >> u32 reg, u32 val)
> >>   static int stm32_iwdg_start(struct watchdog_device *wdd)
> >>   {
> >>       struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> >> -    u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr;
> >> -    int ret;
> >> +
> >> +    dev_dbg(wdd->parent, "%s\n", __func__);
> >> +
> >> +    /*  Start the watchdog */
> >> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> >> +
> >> +    /* reload watchdog */
> >> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> >> +
> >> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
> >> +    return 0;
> >> +}
> >> +
> >> +static int stm32_iwdg_setprescaler(struct watchdog_device *wdd)
> >> +{
> >> +    struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> >> +    u32 tout, presc, iwdg_rlr, iwdg_pr;
> >>         dev_dbg(wdd->parent, "%s\n", __func__);
> >>   @@ -108,19 +123,6 @@ static int stm32_iwdg_start(struct 
> >> watchdog_device *wdd)
> >>       /* set prescaler & reload registers */
> >>       reg_write(wdt->regs, IWDG_PR, iwdg_pr);
> >>       reg_write(wdt->regs, IWDG_RLR, iwdg_rlr);
> >> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> >> -
> >> -    /* wait for the registers to be updated (max 100ms) */
> >> -    ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr,
> >> -                     !(iwdg_sr & (SR_PVU | SR_RVU)),
> >> -                     SLEEP_US, TIMEOUT_US);
> >> -    if (ret) {
> >> -        dev_err(wdd->parent, "Fail to set prescaler, reload regs\n");
> >> -        return ret;
> >> -    }
> >> -
> >> -    /* reload watchdog */
> >> -    reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> >>         return 0;
> >>   }
> >> @@ -131,6 +133,9 @@ static int stm32_iwdg_ping(struct watchdog_device 
> >> *wdd)
> >>         dev_dbg(wdd->parent, "%s\n", __func__);
> >>   +    /*  Start the watchdog */
> >> +    reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> >> +
> >>       /* reload watchdog */
> >>       reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> >>   @@ -140,12 +145,21 @@ static int stm32_iwdg_ping(struct 
> >> watchdog_device *wdd)
> >>   static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
> >>                     unsigned int timeout)
> >>   {
> >> +    int ret;
> >> +
> >>       dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
> >>         wdd->timeout = timeout;
> >>   -    if (watchdog_active(wdd))
> >> -        return stm32_iwdg_start(wdd);
> >> +    if (watchdog_active(wdd)) {
> >> +        ret = stm32_iwdg_setprescaler(wdd);
> >> +        if (ret) {
> >> +            dev_err(wdd->parent, "failed to set prescaler\n");
> >> +            return ret;
> >> +        } else {
> >> +            return stm32_iwdg_start(wdd);
> >> +        }
> >> +    }
> >>         return 0;
> >>   }
> >> @@ -262,12 +276,21 @@ static int stm32_iwdg_probe(struct 
> >> platform_device *pdev)
> >>       watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
> >>       watchdog_init_timeout(wdd, 0, dev);
> >>   +    /* Make sure the watchdog is serviced */
> >> +    set_bit(WDOG_HW_RUNNING, &wdd->status);
> >> +
> >>       ret = devm_watchdog_register_device(dev, wdd);
> >>       if (ret)
> >>           return ret;
> >>         platform_set_drvdata(pdev, wdt);
> >>   +    ret = stm32_iwdg_setprescaler(wdd);
> >> +    if (ret) {
> >> +        dev_err(dev, "failed to set prescaler\n");
> >> +        return ret;
> >> +    }
> >> +
> >>       return 0;
> >>   }
> >>
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel