mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] watchdog: remove wrong uses of timeout_cur
@ 2019-10-24 15:24 Ahmad Fatoum
  2019-10-24 15:24 ` [PATCH 2/3] watchdog: rename timeout_curr to poller_timeout_curr internally Ahmad Fatoum
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2019-10-24 15:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The barebox watchdog poller uses the struct watchdog.timeout_cur as
the timeout value to configure the watchdog with.

There's no need for the device driver to set this. I didn't know that
when I wrote the drivers, but I do now, hence this commit.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/stm32_iwdg.c  | 1 -
 drivers/watchdog/stpmic1_wdt.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index 20536cb4ab2d..4d252e558c32 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -256,7 +256,6 @@ static int stm32_iwdg_probe(struct device_d *dev)
 	wdd->set_timeout = stm32_iwdg_set_timeout;
 	wdd->timeout_max = (RLR_MAX + 1) * data->max_prescaler * 1000;
 	wdd->timeout_max /= wd->rate * 1000;
-	wdd->timeout_cur = wdd->timeout_max;
 
 	ret = watchdog_register(wdd);
 	if (ret) {
diff --git a/drivers/watchdog/stpmic1_wdt.c b/drivers/watchdog/stpmic1_wdt.c
index eb8c43f716a8..f79b7e8c2768 100644
--- a/drivers/watchdog/stpmic1_wdt.c
+++ b/drivers/watchdog/stpmic1_wdt.c
@@ -175,7 +175,6 @@ static int stpmic1_wdt_probe(struct device_d *dev)
 	wdd->hwdev = dev;
 	wdd->set_timeout = stpmic1_wdt_set_timeout;
 	wdd->timeout_max = PMIC_WDT_MAX_TIMEOUT;
-	wdd->timeout_cur = PMIC_WDT_DEFAULT_TIMEOUT;
 
 	/* have the watchdog reset, not power-off the system */
 	regmap_write_bits(wdt->regmap, MAIN_CR, RREQ_EN, RREQ_EN);
-- 
2.23.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 2/3] watchdog: rename timeout_curr to poller_timeout_curr internally
  2019-10-24 15:24 [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Ahmad Fatoum
@ 2019-10-24 15:24 ` Ahmad Fatoum
  2019-10-24 15:24 ` [PATCH 3/3] watchdog: add timeout_cur parameter only when poller is enabled Ahmad Fatoum
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2019-10-24 15:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

timeout_curr is the timeout programmed into the watchdog hardware every
500 milliseconds. If watchdog poller support is disabled, it serves
no purpose, prefix it with poller_ to better communicate this fact.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/wd_core.c | 10 +++++-----
 include/watchdog.h         |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index e6e5ddecd2f8..5b984db8586a 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -45,7 +45,7 @@ static int watchdog_set_cur(struct param_d *param, void *priv)
 {
 	struct watchdog *wd = priv;
 
-	if (wd->timeout_cur > wd->timeout_max)
+	if (wd->poller_timeout_cur > wd->timeout_max)
 		return -EINVAL;
 
 	return 0;
@@ -55,7 +55,7 @@ static void watchdog_poller_cb(void *priv);
 
 static void watchdog_poller_start(struct watchdog *wd)
 {
-	_watchdog_set_timeout(wd, wd->timeout_cur);
+	_watchdog_set_timeout(wd, wd->poller_timeout_cur);
 	poller_call_async(&wd->poller, 500 * MSECOND,
 			watchdog_poller_cb, wd);
 
@@ -134,8 +134,8 @@ int watchdog_register(struct watchdog *wd)
 	if (!wd->timeout_max)
 		wd->timeout_max = 60 * 60 * 24;
 
-	if (!wd->timeout_cur || wd->timeout_cur > wd->timeout_max)
-		wd->timeout_cur = wd->timeout_max;
+	if (!wd->poller_timeout_cur || wd->poller_timeout_cur > wd->timeout_max)
+		wd->poller_timeout_cur = wd->timeout_max;
 
 	p = dev_add_param_uint32_ro(&wd->dev, "timeout_max",
 			&wd->timeout_max, "%u");
@@ -143,7 +143,7 @@ int watchdog_register(struct watchdog *wd)
 		return PTR_ERR(p);
 
 	p = dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur, NULL,
-			&wd->timeout_cur, "%u", wd);
+			&wd->poller_timeout_cur, "%u", wd);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
diff --git a/include/watchdog.h b/include/watchdog.h
index 0db4263a31ce..68c6b00233d9 100644
--- a/include/watchdog.h
+++ b/include/watchdog.h
@@ -22,7 +22,7 @@ struct watchdog {
 	struct device_d dev;
 	unsigned int priority;
 	unsigned int timeout_max;
-	unsigned int timeout_cur;
+	unsigned int poller_timeout_cur;
 	unsigned int poller_enable;
 	struct poller_async poller;
 	struct list_head list;
-- 
2.23.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 3/3] watchdog: add timeout_cur parameter only when poller is enabled
  2019-10-24 15:24 [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Ahmad Fatoum
  2019-10-24 15:24 ` [PATCH 2/3] watchdog: rename timeout_curr to poller_timeout_curr internally Ahmad Fatoum
@ 2019-10-24 15:24 ` Ahmad Fatoum
  2019-10-24 16:30 ` [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Oleksij Rempel
  2019-10-28 11:41 ` Sascha Hauer
  3 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2019-10-24 15:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

timeout_curr is the timeout programmed into the watchdog hardware every
500 milliseconds. If watchdog poller support is disabled, it still
shows up as a configurable device parameter, but has no effect.

Improve user experience by having it show up only if watchdog poller
support was compiled in. This is already the case for the autoping
parameter. The timeout_max parameter is a generic parameter and will
remain unchanged.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/wd_core.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index 5b984db8586a..1ce5a360eb61 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -134,20 +134,21 @@ int watchdog_register(struct watchdog *wd)
 	if (!wd->timeout_max)
 		wd->timeout_max = 60 * 60 * 24;
 
-	if (!wd->poller_timeout_cur || wd->poller_timeout_cur > wd->timeout_max)
-		wd->poller_timeout_cur = wd->timeout_max;
-
 	p = dev_add_param_uint32_ro(&wd->dev, "timeout_max",
 			&wd->timeout_max, "%u");
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
-	p = dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur, NULL,
-			&wd->poller_timeout_cur, "%u", wd);
-	if (IS_ERR(p))
-		return PTR_ERR(p);
-
 	if (IS_ENABLED(CONFIG_WATCHDOG_POLLER)) {
+		if (!wd->poller_timeout_cur ||
+		    wd->poller_timeout_cur > wd->timeout_max)
+			wd->poller_timeout_cur = wd->timeout_max;
+
+		p = dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur,
+				NULL, &wd->poller_timeout_cur, "%u", wd);
+		if (IS_ERR(p))
+			return PTR_ERR(p);
+
 		ret = watchdog_register_poller(wd);
 		if (ret)
 			return ret;
-- 
2.23.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] watchdog: remove wrong uses of timeout_cur
  2019-10-24 15:24 [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Ahmad Fatoum
  2019-10-24 15:24 ` [PATCH 2/3] watchdog: rename timeout_curr to poller_timeout_curr internally Ahmad Fatoum
  2019-10-24 15:24 ` [PATCH 3/3] watchdog: add timeout_cur parameter only when poller is enabled Ahmad Fatoum
@ 2019-10-24 16:30 ` Oleksij Rempel
  2019-10-28 17:29   ` Ahmad Fatoum
  2019-10-28 11:41 ` Sascha Hauer
  3 siblings, 1 reply; 6+ messages in thread
From: Oleksij Rempel @ 2019-10-24 16:30 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox

Hi,

How about using this variable to start watchdog(s) on kernel boot?


Am 24.10.19 um 17:24 schrieb Ahmad Fatoum:
> The barebox watchdog poller uses the struct watchdog.timeout_cur as
> the timeout value to configure the watchdog with.
>
> There's no need for the device driver to set this. I didn't know that
> when I wrote the drivers, but I do now, hence this commit.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/watchdog/stm32_iwdg.c  | 1 -
>  drivers/watchdog/stpmic1_wdt.c | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> index 20536cb4ab2d..4d252e558c32 100644
> --- a/drivers/watchdog/stm32_iwdg.c
> +++ b/drivers/watchdog/stm32_iwdg.c
> @@ -256,7 +256,6 @@ static int stm32_iwdg_probe(struct device_d *dev)
>  	wdd->set_timeout = stm32_iwdg_set_timeout;
>  	wdd->timeout_max = (RLR_MAX + 1) * data->max_prescaler * 1000;
>  	wdd->timeout_max /= wd->rate * 1000;
> -	wdd->timeout_cur = wdd->timeout_max;
>
>  	ret = watchdog_register(wdd);
>  	if (ret) {
> diff --git a/drivers/watchdog/stpmic1_wdt.c b/drivers/watchdog/stpmic1_wdt.c
> index eb8c43f716a8..f79b7e8c2768 100644
> --- a/drivers/watchdog/stpmic1_wdt.c
> +++ b/drivers/watchdog/stpmic1_wdt.c
> @@ -175,7 +175,6 @@ static int stpmic1_wdt_probe(struct device_d *dev)
>  	wdd->hwdev = dev;
>  	wdd->set_timeout = stpmic1_wdt_set_timeout;
>  	wdd->timeout_max = PMIC_WDT_MAX_TIMEOUT;
> -	wdd->timeout_cur = PMIC_WDT_DEFAULT_TIMEOUT;
>
>  	/* have the watchdog reset, not power-off the system */
>  	regmap_write_bits(wdt->regmap, MAIN_CR, RREQ_EN, RREQ_EN);
>


--
Regards,
Oleksij

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] watchdog: remove wrong uses of timeout_cur
  2019-10-24 15:24 [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2019-10-24 16:30 ` [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Oleksij Rempel
@ 2019-10-28 11:41 ` Sascha Hauer
  3 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2019-10-28 11:41 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Thu, Oct 24, 2019 at 05:24:26PM +0200, Ahmad Fatoum wrote:
> The barebox watchdog poller uses the struct watchdog.timeout_cur as
> the timeout value to configure the watchdog with.
> 
> There's no need for the device driver to set this. I didn't know that
> when I wrote the drivers, but I do now, hence this commit.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/watchdog/stm32_iwdg.c  | 1 -
>  drivers/watchdog/stpmic1_wdt.c | 1 -
>  2 files changed, 2 deletions(-)

Applied, thanks

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] watchdog: remove wrong uses of timeout_cur
  2019-10-24 16:30 ` [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Oleksij Rempel
@ 2019-10-28 17:29   ` Ahmad Fatoum
  0 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2019-10-28 17:29 UTC (permalink / raw)
  To: Oleksij Rempel, barebox

Hello Oleksij,

On 10/24/19 6:30 PM, Oleksij Rempel wrote:
> Hi,
> 
> How about using this variable to start watchdog(s) on kernel boot?

I originally thought that that is what the parameter is doing.
I think it'd be a good idea to add these semantic. Then it can
be made a generic parameter again.

Cheers

> 
> 
> Am 24.10.19 um 17:24 schrieb Ahmad Fatoum:
>> The barebox watchdog poller uses the struct watchdog.timeout_cur as
>> the timeout value to configure the watchdog with.
>>
>> There's no need for the device driver to set this. I didn't know that
>> when I wrote the drivers, but I do now, hence this commit.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/watchdog/stm32_iwdg.c  | 1 -
>>  drivers/watchdog/stpmic1_wdt.c | 1 -
>>  2 files changed, 2 deletions(-)
>>
>> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
>> index 20536cb4ab2d..4d252e558c32 100644
>> --- a/drivers/watchdog/stm32_iwdg.c
>> +++ b/drivers/watchdog/stm32_iwdg.c
>> @@ -256,7 +256,6 @@ static int stm32_iwdg_probe(struct device_d *dev)
>>  	wdd->set_timeout = stm32_iwdg_set_timeout;
>>  	wdd->timeout_max = (RLR_MAX + 1) * data->max_prescaler * 1000;
>>  	wdd->timeout_max /= wd->rate * 1000;
>> -	wdd->timeout_cur = wdd->timeout_max;
>>
>>  	ret = watchdog_register(wdd);
>>  	if (ret) {
>> diff --git a/drivers/watchdog/stpmic1_wdt.c b/drivers/watchdog/stpmic1_wdt.c
>> index eb8c43f716a8..f79b7e8c2768 100644
>> --- a/drivers/watchdog/stpmic1_wdt.c
>> +++ b/drivers/watchdog/stpmic1_wdt.c
>> @@ -175,7 +175,6 @@ static int stpmic1_wdt_probe(struct device_d *dev)
>>  	wdd->hwdev = dev;
>>  	wdd->set_timeout = stpmic1_wdt_set_timeout;
>>  	wdd->timeout_max = PMIC_WDT_MAX_TIMEOUT;
>> -	wdd->timeout_cur = PMIC_WDT_DEFAULT_TIMEOUT;
>>
>>  	/* have the watchdog reset, not power-off the system */
>>  	regmap_write_bits(wdt->regmap, MAIN_CR, RREQ_EN, RREQ_EN);
>>
> 
> 
> --
> Regards,
> Oleksij
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-28 17:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 15:24 [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Ahmad Fatoum
2019-10-24 15:24 ` [PATCH 2/3] watchdog: rename timeout_curr to poller_timeout_curr internally Ahmad Fatoum
2019-10-24 15:24 ` [PATCH 3/3] watchdog: add timeout_cur parameter only when poller is enabled Ahmad Fatoum
2019-10-24 16:30 ` [PATCH 1/3] watchdog: remove wrong uses of timeout_cur Oleksij Rempel
2019-10-28 17:29   ` Ahmad Fatoum
2019-10-28 11:41 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox