* [PATCH 1/1] watchdog: da9063: Fix setting/changing timeout
@ 2018-05-09 10:31 Marco Felsch
2018-05-09 12:06 ` Guenter Roeck
0 siblings, 1 reply; 3+ messages in thread
From: Marco Felsch @ 2018-05-09 10:31 UTC (permalink / raw)
To: wim, linux
Cc: krystian.garbaciak, support.opensource, linux-watchdog, fzuuzf, kernel
The DA9063 watchdog always resets the system when it changes the timeout
value after the bootloader (e.g. Barebox) has it already set.
To update the timeout value we have to disable the watchdog, clear the
watchdog counter value and write the new timeout value to the watchdog.
Clearing the counter value is a feature to be on the safe side, because the
data sheet doesn't describe the behaviour of the watchdog counter value
after a watchdog disabling-enable-sequence.
The patch is based on Philipp Zabel's <p.zabel@pengutronix.de> previous
patch but doesn't wait 150us because the DA9063 doesn't need this delay.
Fixes: 5e9c16e37608 ("watchdog: Add DA9063 PMIC watchdog driver.")
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
drivers/watchdog/da9063_wdt.c | 42 +++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
index b17ac1bb1f28..c82541c39d35 100644
--- a/drivers/watchdog/da9063_wdt.c
+++ b/drivers/watchdog/da9063_wdt.c
@@ -45,10 +45,45 @@ static unsigned int da9063_wdt_timeout_to_sel(unsigned int secs)
return DA9063_TWDSCALE_MAX;
}
+/*
+ * Writing a '1' to the self-clearing WATCHDOG bit resets the watchdog counter
+ * value.
+ */
+static int _da9063_wdt_reset_timer(struct da9063 *da9063)
+{
+ return regmap_write(da9063->regmap,
+ DA9063_REG_CONTROL_F,
+ DA9063_WATCHDOG);
+}
+
static int _da9063_wdt_set_timeout(struct da9063 *da9063, unsigned int regval)
{
- return regmap_update_bits(da9063->regmap, DA9063_REG_CONTROL_D,
- DA9063_TWDSCALE_MASK, regval);
+ int ret;
+
+ /*
+ * The watchdog trigger a reboot if a timeout value is already
+ * programmed. Because the timeout value combines two functions in
+ * one: indicating the counter limit and starting the watchdog. To be
+ * able to set the watchdog a second time (first time was done by the
+ * bootloader) disable the watchdog clear the counter value manually and
+ * set the new timeout value.
+ */
+ ret = regmap_update_bits(da9063->regmap,
+ DA9063_REG_CONTROL_D,
+ DA9063_TWDSCALE_MASK,
+ DA9063_TWDSCALE_DISABLE);
+ if (ret)
+ dev_warn(da9063->dev,
+ "Failed to disable watchdog before setting new timeout\n");
+
+ ret = _da9063_wdt_reset_timer(da9063);
+ if (ret)
+ dev_warn(da9063->dev, "Failed to reset watchdog counter\n");
+
+ return regmap_update_bits(da9063->regmap,
+ DA9063_REG_CONTROL_D,
+ DA9063_TWDSCALE_MASK,
+ regval);
}
static int da9063_wdt_start(struct watchdog_device *wdd)
@@ -85,8 +120,7 @@ static int da9063_wdt_ping(struct watchdog_device *wdd)
struct da9063 *da9063 = watchdog_get_drvdata(wdd);
int ret;
- ret = regmap_write(da9063->regmap, DA9063_REG_CONTROL_F,
- DA9063_WATCHDOG);
+ ret = _da9063_wdt_reset_timer(da9063);
if (ret)
dev_alert(da9063->dev, "Failed to ping the watchdog (err = %d)\n",
ret);
--
2.17.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] watchdog: da9063: Fix setting/changing timeout
2018-05-09 10:31 [PATCH 1/1] watchdog: da9063: Fix setting/changing timeout Marco Felsch
@ 2018-05-09 12:06 ` Guenter Roeck
2018-05-09 12:11 ` Marco Felsch
0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2018-05-09 12:06 UTC (permalink / raw)
To: Marco Felsch, wim
Cc: krystian.garbaciak, support.opensource, linux-watchdog, fzuuzf, kernel
On 05/09/2018 03:31 AM, Marco Felsch wrote:
> The DA9063 watchdog always resets the system when it changes the timeout
> value after the bootloader (e.g. Barebox) has it already set.
>
> To update the timeout value we have to disable the watchdog, clear the
> watchdog counter value and write the new timeout value to the watchdog.
> Clearing the counter value is a feature to be on the safe side, because the
> data sheet doesn't describe the behaviour of the watchdog counter value
> after a watchdog disabling-enable-sequence.
>
> The patch is based on Philipp Zabel's <p.zabel@pengutronix.de> previous
> patch but doesn't wait 150us because the DA9063 doesn't need this delay.
>
> Fixes: 5e9c16e37608 ("watchdog: Add DA9063 PMIC watchdog driver.")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> drivers/watchdog/da9063_wdt.c | 42 +++++++++++++++++++++++++++++++----
> 1 file changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
> index b17ac1bb1f28..c82541c39d35 100644
> --- a/drivers/watchdog/da9063_wdt.c
> +++ b/drivers/watchdog/da9063_wdt.c
> @@ -45,10 +45,45 @@ static unsigned int da9063_wdt_timeout_to_sel(unsigned int secs)
> return DA9063_TWDSCALE_MAX;
> }
>
> +/*
> + * Writing a '1' to the self-clearing WATCHDOG bit resets the watchdog counter
> + * value.
> + */
> +static int _da9063_wdt_reset_timer(struct da9063 *da9063)
> +{
> + return regmap_write(da9063->regmap,
> + DA9063_REG_CONTROL_F,
> + DA9063_WATCHDOG);
> +}
> +
> static int _da9063_wdt_set_timeout(struct da9063 *da9063, unsigned int regval)
> {
> - return regmap_update_bits(da9063->regmap, DA9063_REG_CONTROL_D,
> - DA9063_TWDSCALE_MASK, regval);
> + int ret;
> +
> + /*
> + * The watchdog trigger a reboot if a timeout value is already
> + * programmed. Because the timeout value combines two functions in
> + * one: indicating the counter limit and starting the watchdog. To be
> + * able to set the watchdog a second time (first time was done by the
> + * bootloader) disable the watchdog clear the counter value manually and
> + * set the new timeout value.
> + */
> + ret = regmap_update_bits(da9063->regmap,
> + DA9063_REG_CONTROL_D,
> + DA9063_TWDSCALE_MASK,
> + DA9063_TWDSCALE_DISABLE);
> + if (ret)
> + dev_warn(da9063->dev,
> + "Failed to disable watchdog before setting new timeout\n");
> +
> + ret = _da9063_wdt_reset_timer(da9063);
> + if (ret)
> + dev_warn(da9063->dev, "Failed to reset watchdog counter\n");
> +
> + return regmap_update_bits(da9063->regmap,
> + DA9063_REG_CONTROL_D,
> + DA9063_TWDSCALE_MASK,
> + regval);
I am not a friend of unnecessary line breaks.
Guenter
> }
>
> static int da9063_wdt_start(struct watchdog_device *wdd)
> @@ -85,8 +120,7 @@ static int da9063_wdt_ping(struct watchdog_device *wdd)
> struct da9063 *da9063 = watchdog_get_drvdata(wdd);
> int ret;
>
> - ret = regmap_write(da9063->regmap, DA9063_REG_CONTROL_F,
> - DA9063_WATCHDOG);
> + ret = _da9063_wdt_reset_timer(da9063);
> if (ret)
> dev_alert(da9063->dev, "Failed to ping the watchdog (err = %d)\n",
> ret);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] watchdog: da9063: Fix setting/changing timeout
2018-05-09 12:06 ` Guenter Roeck
@ 2018-05-09 12:11 ` Marco Felsch
0 siblings, 0 replies; 3+ messages in thread
From: Marco Felsch @ 2018-05-09 12:11 UTC (permalink / raw)
To: Guenter Roeck, wim; +Cc: support.opensource, linux-watchdog, fzuuzf, kernel
On 05/09/2018 02:06 PM, Guenter Roeck wrote:
> On 05/09/2018 03:31 AM, Marco Felsch wrote:
>> The DA9063 watchdog always resets the system when it changes the timeout
>> value after the bootloader (e.g. Barebox) has it already set.
>>
>> To update the timeout value we have to disable the watchdog, clear the
>> watchdog counter value and write the new timeout value to the watchdog.
>> Clearing the counter value is a feature to be on the safe side,
>> because the
>> data sheet doesn't describe the behaviour of the watchdog counter value
>> after a watchdog disabling-enable-sequence.
>>
>> The patch is based on Philipp Zabel's <p.zabel@pengutronix.de> previous
>> patch but doesn't wait 150us because the DA9063 doesn't need this delay.
>>
>> Fixes: 5e9c16e37608 ("watchdog: Add DA9063 PMIC watchdog driver.")
>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>> ---
>> drivers/watchdog/da9063_wdt.c | 42 +++++++++++++++++++++++++++++++----
>> 1 file changed, 38 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/watchdog/da9063_wdt.c
>> b/drivers/watchdog/da9063_wdt.c
>> index b17ac1bb1f28..c82541c39d35 100644
>> --- a/drivers/watchdog/da9063_wdt.c
>> +++ b/drivers/watchdog/da9063_wdt.c
>> @@ -45,10 +45,45 @@ static unsigned int
>> da9063_wdt_timeout_to_sel(unsigned int secs)
>> return DA9063_TWDSCALE_MAX;
>> }
>> +/*
>> + * Writing a '1' to the self-clearing WATCHDOG bit resets the
>> watchdog counter
>> + * value.
>> + */
>> +static int _da9063_wdt_reset_timer(struct da9063 *da9063)
>> +{
>> + return regmap_write(da9063->regmap,
>> + DA9063_REG_CONTROL_F,
>> + DA9063_WATCHDOG);
>> +}
>> +
>> static int _da9063_wdt_set_timeout(struct da9063 *da9063, unsigned
>> int regval)
>> {
>> - return regmap_update_bits(da9063->regmap, DA9063_REG_CONTROL_D,
>> - DA9063_TWDSCALE_MASK, regval);
>> + int ret;
>> +
>> + /*
>> + * The watchdog trigger a reboot if a timeout value is already
>> + * programmed. Because the timeout value combines two functions in
>> + * one: indicating the counter limit and starting the watchdog.
>> To be
>> + * able to set the watchdog a second time (first time was done
>> by the
>> + * bootloader) disable the watchdog clear the counter value
>> manually and
>> + * set the new timeout value.
>> + */
>> + ret = regmap_update_bits(da9063->regmap,
>> + DA9063_REG_CONTROL_D,
>> + DA9063_TWDSCALE_MASK,
>> + DA9063_TWDSCALE_DISABLE);
>> + if (ret)
>> + dev_warn(da9063->dev,
>> + "Failed to disable watchdog before setting new
>> timeout\n");
>> +
>> + ret = _da9063_wdt_reset_timer(da9063);
>> + if (ret)
>> + dev_warn(da9063->dev, "Failed to reset watchdog counter\n");
>> +
>> + return regmap_update_bits(da9063->regmap,
>> + DA9063_REG_CONTROL_D,
>> + DA9063_TWDSCALE_MASK,
>> + regval);
>
> I am not a friend of unnecessary line breaks.
>
> Guenter
>
Okay, I will fix this in v2.
Marco
>> }
>> static int da9063_wdt_start(struct watchdog_device *wdd)
>> @@ -85,8 +120,7 @@ static int da9063_wdt_ping(struct watchdog_device
>> *wdd)
>> struct da9063 *da9063 = watchdog_get_drvdata(wdd);
>> int ret;
>> - ret = regmap_write(da9063->regmap, DA9063_REG_CONTROL_F,
>> - DA9063_WATCHDOG);
>> + ret = _da9063_wdt_reset_timer(da9063);
>> if (ret)
>> dev_alert(da9063->dev, "Failed to ping the watchdog (err =
>> %d)\n",
>> ret);
>>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-09 12:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-09 10:31 [PATCH 1/1] watchdog: da9063: Fix setting/changing timeout Marco Felsch
2018-05-09 12:06 ` Guenter Roeck
2018-05-09 12:11 ` Marco Felsch
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.