* [PATCH] watchdog: meson_gxbb_wdt: improve
@ 2021-06-22 9:56 Artem Lapkin
2021-06-22 11:08 ` Neil Armstrong
0 siblings, 1 reply; 5+ messages in thread
From: Artem Lapkin @ 2021-06-22 9:56 UTC (permalink / raw)
To: narmstrong
Cc: wim, linux, khilman, jbrunet, christianshewitt,
martin.blumenstingl, linux-watchdog, linux-arm-kernel,
linux-amlogic, linux-kernel, art, nick, gouwa
Improve meson_gxbb_wdt watchdog driver
1) added module param timeout and nowayout same as other modules
2) print watchdog driver start status
3) add watchdog_stop_on_unregister
4) remove watchdog_stop_on_reboot ( still can be activated by
watchdog.stop_on_reboot=1 ) i think this driver configuration more useful
becouse we can get reboot waranty for abnormal situations on shutdown stage
Signed-off-by: Artem Lapkin <art@khadas.com>
---
drivers/watchdog/meson_gxbb_wdt.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
index 5a9ca10fbcfa..15c889932c13 100644
--- a/drivers/watchdog/meson_gxbb_wdt.c
+++ b/drivers/watchdog/meson_gxbb_wdt.c
@@ -35,6 +35,17 @@ struct meson_gxbb_wdt {
struct clk *clk;
};
+static bool nowayout = WATCHDOG_NOWAYOUT;
+static unsigned int timeout = DEFAULT_TIMEOUT;
+
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
+ __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds="
+ __MODULE_STRING(DEFAULT_TIMEOUT) ")");
+
static int meson_gxbb_wdt_start(struct watchdog_device *wdt_dev)
{
struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
@@ -174,7 +185,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
data->wdt_dev.ops = &meson_gxbb_wdt_ops;
data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
data->wdt_dev.min_timeout = 1;
- data->wdt_dev.timeout = DEFAULT_TIMEOUT;
+ data->wdt_dev.timeout = timeout;
watchdog_set_drvdata(&data->wdt_dev, data);
/* Setup with 1ms timebase */
@@ -186,7 +197,12 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout);
- watchdog_stop_on_reboot(&data->wdt_dev);
+ watchdog_set_nowayout(&data->wdt_dev, nowayout);
+ watchdog_stop_on_unregister(&data->wdt_dev);
+
+ dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
+ data->wdt_dev.timeout, nowayout);
+
return devm_watchdog_register_device(dev, &data->wdt_dev);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] watchdog: meson_gxbb_wdt: improve
2021-06-22 9:56 [PATCH] watchdog: meson_gxbb_wdt: improve Artem Lapkin
@ 2021-06-22 11:08 ` Neil Armstrong
2021-06-22 11:53 ` Art Nikpal
0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2021-06-22 11:08 UTC (permalink / raw)
To: Artem Lapkin
Cc: wim, linux, khilman, jbrunet, christianshewitt,
martin.blumenstingl, linux-watchdog, linux-arm-kernel,
linux-amlogic, linux-kernel, art, nick, gouwa
Hi Art,
On 22/06/2021 11:56, Artem Lapkin wrote:
> Improve meson_gxbb_wdt watchdog driver
> 1) added module param timeout and nowayout same as other modules
> 2) print watchdog driver start status
> 3) add watchdog_stop_on_unregister
> 4) remove watchdog_stop_on_reboot ( still can be activated by
> watchdog.stop_on_reboot=1 ) i think this driver configuration more useful
> becouse we can get reboot waranty for abnormal situations on shutdown stage
Can you split the patch in 4 distinct changes ?
Neil
>
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---
> drivers/watchdog/meson_gxbb_wdt.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> index 5a9ca10fbcfa..15c889932c13 100644
> --- a/drivers/watchdog/meson_gxbb_wdt.c
> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> @@ -35,6 +35,17 @@ struct meson_gxbb_wdt {
> struct clk *clk;
> };
>
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +static unsigned int timeout = DEFAULT_TIMEOUT;
> +
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +module_param(timeout, uint, 0);
> +MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds="
> + __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> +
> static int meson_gxbb_wdt_start(struct watchdog_device *wdt_dev)
> {
> struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
> @@ -174,7 +185,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
> data->wdt_dev.ops = &meson_gxbb_wdt_ops;
> data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
> data->wdt_dev.min_timeout = 1;
> - data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> + data->wdt_dev.timeout = timeout;
> watchdog_set_drvdata(&data->wdt_dev, data);
>
> /* Setup with 1ms timebase */
> @@ -186,7 +197,12 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
>
> meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout);
>
> - watchdog_stop_on_reboot(&data->wdt_dev);
> + watchdog_set_nowayout(&data->wdt_dev, nowayout);
> + watchdog_stop_on_unregister(&data->wdt_dev);
> +
> + dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
> + data->wdt_dev.timeout, nowayout);
> +
> return devm_watchdog_register_device(dev, &data->wdt_dev);
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] watchdog: meson_gxbb_wdt: improve
2021-06-22 11:08 ` Neil Armstrong
@ 2021-06-22 11:53 ` Art Nikpal
2021-06-22 12:43 ` Neil Armstrong
0 siblings, 1 reply; 5+ messages in thread
From: Art Nikpal @ 2021-06-22 11:53 UTC (permalink / raw)
To: Neil Armstrong
Cc: wim, linux, Kevin Hilman, Jerome Brunet, Christian Hewitt,
Martin Blumenstingl, linux-watchdog, linux-arm-kernel,
open list:ARM/Amlogic Meson...,
linux-kernel, Artem Lapkin, Nick Xie, Gouwa Wang
> Neil
> Can you split the patch in 4 distinct changes ?
yes no problem i can try to do it tomorrow !
maybe somebody have other ideas, suggestion, comments ...
On Tue, Jun 22, 2021 at 7:08 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi Art,
>
> On 22/06/2021 11:56, Artem Lapkin wrote:
> > Improve meson_gxbb_wdt watchdog driver
> > 1) added module param timeout and nowayout same as other modules
> > 2) print watchdog driver start status
> > 3) add watchdog_stop_on_unregister
> > 4) remove watchdog_stop_on_reboot ( still can be activated by
> > watchdog.stop_on_reboot=1 ) i think this driver configuration more useful
> > becouse we can get reboot waranty for abnormal situations on shutdown stage
>
> Can you split the patch in 4 distinct changes ?
>
> Neil
>
> >
> > Signed-off-by: Artem Lapkin <art@khadas.com>
> > ---
> > drivers/watchdog/meson_gxbb_wdt.c | 20 ++++++++++++++++++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> > index 5a9ca10fbcfa..15c889932c13 100644
> > --- a/drivers/watchdog/meson_gxbb_wdt.c
> > +++ b/drivers/watchdog/meson_gxbb_wdt.c
> > @@ -35,6 +35,17 @@ struct meson_gxbb_wdt {
> > struct clk *clk;
> > };
> >
> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> > +static unsigned int timeout = DEFAULT_TIMEOUT;
> > +
> > +module_param(nowayout, bool, 0);
> > +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
> > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +module_param(timeout, uint, 0);
> > +MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds="
> > + __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> > +
> > static int meson_gxbb_wdt_start(struct watchdog_device *wdt_dev)
> > {
> > struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
> > @@ -174,7 +185,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
> > data->wdt_dev.ops = &meson_gxbb_wdt_ops;
> > data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
> > data->wdt_dev.min_timeout = 1;
> > - data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> > + data->wdt_dev.timeout = timeout;
> > watchdog_set_drvdata(&data->wdt_dev, data);
> >
> > /* Setup with 1ms timebase */
> > @@ -186,7 +197,12 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
> >
> > meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout);
> >
> > - watchdog_stop_on_reboot(&data->wdt_dev);
> > + watchdog_set_nowayout(&data->wdt_dev, nowayout);
> > + watchdog_stop_on_unregister(&data->wdt_dev);
> > +
> > + dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
> > + data->wdt_dev.timeout, nowayout);
> > +
> > return devm_watchdog_register_device(dev, &data->wdt_dev);
> > }
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] watchdog: meson_gxbb_wdt: improve
2021-06-22 11:53 ` Art Nikpal
@ 2021-06-22 12:43 ` Neil Armstrong
2021-06-22 17:00 ` Guenter Roeck
0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2021-06-22 12:43 UTC (permalink / raw)
To: Art Nikpal
Cc: wim, linux, Kevin Hilman, Jerome Brunet, Christian Hewitt,
Martin Blumenstingl, linux-watchdog, linux-arm-kernel,
open list:ARM/Amlogic Meson...,
linux-kernel, Artem Lapkin, Nick Xie, Gouwa Wang
Hi,
On 22/06/2021 13:53, Art Nikpal wrote:
>> Neil
>> Can you split the patch in 4 distinct changes ?
>
> yes no problem i can try to do it tomorrow !
> maybe somebody have other ideas, suggestion, comments ...
The changeset is clean, and overall I'm ok with the changes, but I'm pretty sure the wdt maintainers
will prefer separate changes in order to comment of each.
Neil
>
>
> On Tue, Jun 22, 2021 at 7:08 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> Hi Art,
>>
>> On 22/06/2021 11:56, Artem Lapkin wrote:
>>> Improve meson_gxbb_wdt watchdog driver
>>> 1) added module param timeout and nowayout same as other modules
>>> 2) print watchdog driver start status
>>> 3) add watchdog_stop_on_unregister
>>> 4) remove watchdog_stop_on_reboot ( still can be activated by
>>> watchdog.stop_on_reboot=1 ) i think this driver configuration more useful
>>> becouse we can get reboot waranty for abnormal situations on shutdown stage
>>
>> Can you split the patch in 4 distinct changes ?
>>
>> Neil
>>
>>>
>>> Signed-off-by: Artem Lapkin <art@khadas.com>
>>> ---
>>> drivers/watchdog/meson_gxbb_wdt.c | 20 ++++++++++++++++++--
>>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
>>> index 5a9ca10fbcfa..15c889932c13 100644
>>> --- a/drivers/watchdog/meson_gxbb_wdt.c
>>> +++ b/drivers/watchdog/meson_gxbb_wdt.c
>>> @@ -35,6 +35,17 @@ struct meson_gxbb_wdt {
>>> struct clk *clk;
>>> };
>>>
>>> +static bool nowayout = WATCHDOG_NOWAYOUT;
>>> +static unsigned int timeout = DEFAULT_TIMEOUT;
>>> +
>>> +module_param(nowayout, bool, 0);
>>> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
>>> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>>> +
>>> +module_param(timeout, uint, 0);
>>> +MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds="
>>> + __MODULE_STRING(DEFAULT_TIMEOUT) ")");
>>> +
>>> static int meson_gxbb_wdt_start(struct watchdog_device *wdt_dev)
>>> {
>>> struct meson_gxbb_wdt *data = watchdog_get_drvdata(wdt_dev);
>>> @@ -174,7 +185,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
>>> data->wdt_dev.ops = &meson_gxbb_wdt_ops;
>>> data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
>>> data->wdt_dev.min_timeout = 1;
>>> - data->wdt_dev.timeout = DEFAULT_TIMEOUT;
>>> + data->wdt_dev.timeout = timeout;
>>> watchdog_set_drvdata(&data->wdt_dev, data);
>>>
>>> /* Setup with 1ms timebase */
>>> @@ -186,7 +197,12 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
>>>
>>> meson_gxbb_wdt_set_timeout(&data->wdt_dev, data->wdt_dev.timeout);
>>>
>>> - watchdog_stop_on_reboot(&data->wdt_dev);
>>> + watchdog_set_nowayout(&data->wdt_dev, nowayout);
>>> + watchdog_stop_on_unregister(&data->wdt_dev);
>>> +
>>> + dev_info(dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
>>> + data->wdt_dev.timeout, nowayout);
>>> +
>>> return devm_watchdog_register_device(dev, &data->wdt_dev);
>>> }
>>>
>>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] watchdog: meson_gxbb_wdt: improve
2021-06-22 12:43 ` Neil Armstrong
@ 2021-06-22 17:00 ` Guenter Roeck
0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2021-06-22 17:00 UTC (permalink / raw)
To: Neil Armstrong
Cc: Art Nikpal, wim, Kevin Hilman, Jerome Brunet, Christian Hewitt,
Martin Blumenstingl, linux-watchdog, linux-arm-kernel,
open list:ARM/Amlogic Meson...,
linux-kernel, Artem Lapkin, Nick Xie, Gouwa Wang
On Tue, Jun 22, 2021 at 02:43:45PM +0200, Neil Armstrong wrote:
> Hi,
>
> On 22/06/2021 13:53, Art Nikpal wrote:
> >> Neil
> >> Can you split the patch in 4 distinct changes ?
> >
> > yes no problem i can try to do it tomorrow !
> > maybe somebody have other ideas, suggestion, comments ...
>
> The changeset is clean, and overall I'm ok with the changes, but I'm pretty sure the wdt maintainers
> will prefer separate changes in order to comment of each.
Correct. As per guidelines, "one logical change per patch".
Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-22 17:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22 9:56 [PATCH] watchdog: meson_gxbb_wdt: improve Artem Lapkin
2021-06-22 11:08 ` Neil Armstrong
2021-06-22 11:53 ` Art Nikpal
2021-06-22 12:43 ` Neil Armstrong
2021-06-22 17:00 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).