[PATCH v4 0/3] watchdog: meson_gxbb_wdt: improve

[PATCH v4 0/3] watchdog: meson_gxbb_wdt: improve

[Artem Lapkin]

* Added nowayout module parameter
* Added timeout module parameter
* Removed watchdog_stop_on_reboot

https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t

Artem Lapkin (3):
  watchdog: meson_gxbb_wdt: add nowayout parameter
  watchdog: meson_gxbb_wdt: add timeout parameter
  watchdog: meson_gxbb_wdt: remove stop_on_reboot

 drivers/watchdog/meson_gxbb_wdt.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v4 1/3] watchdog: meson_gxbb_wdt: add nowayout parameter

[Artem Lapkin]

Add nowayout module parameter

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 drivers/watchdog/meson_gxbb_wdt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
index 5a9ca10fbcfa..5aebc3a09652 100644
--- a/drivers/watchdog/meson_gxbb_wdt.c
+++ b/drivers/watchdog/meson_gxbb_wdt.c
@@ -29,6 +29,11 @@
 #define GXBB_WDT_TCNT_SETUP_MASK		(BIT(16) - 1)
 #define GXBB_WDT_TCNT_CNT_SHIFT			16
 
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
+		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
 struct meson_gxbb_wdt {
 	void __iomem *reg_base;
 	struct watchdog_device wdt_dev;
@@ -175,6 +180,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
 	data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
 	data->wdt_dev.min_timeout = 1;
 	data->wdt_dev.timeout = DEFAULT_TIMEOUT;
+	watchdog_set_nowayout(&data->wdt_dev, nowayout);
 	watchdog_set_drvdata(&data->wdt_dev, data);
 
 	/* Setup with 1ms timebase */
-- 
2.25.1

[PATCH v4 2/3] watchdog: meson_gxbb_wdt: add timeout parameter

[Artem Lapkin]

Add timeout module parameter

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 drivers/watchdog/meson_gxbb_wdt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
index 5aebc3a09652..945f5e65db57 100644
--- a/drivers/watchdog/meson_gxbb_wdt.c
+++ b/drivers/watchdog/meson_gxbb_wdt.c
@@ -34,6 +34,11 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
 		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+static unsigned int timeout;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds="
+		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
+
 struct meson_gxbb_wdt {
 	void __iomem *reg_base;
 	struct watchdog_device wdt_dev;
@@ -180,6 +185,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
 	data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
 	data->wdt_dev.min_timeout = 1;
 	data->wdt_dev.timeout = DEFAULT_TIMEOUT;
+	watchdog_init_timeout(&data->wdt_dev, timeout, dev);
 	watchdog_set_nowayout(&data->wdt_dev, nowayout);
 	watchdog_set_drvdata(&data->wdt_dev, data);
 
-- 
2.25.1

[PATCH v4 3/3] watchdog: meson_gxbb_wdt: remove stop_on_reboot

[Artem Lapkin]

Remove watchdog_stop_on_reboot()

Meson platform still have some hardware drivers problems for some
configurations which can freeze device on shutdown/reboot stage and i
think better to have reboot warranty by default.

I feel that it is important to keep the watchdog running during the
reboot sequence, in the event that an abnormal driver freezes the reboot
process.

This is my personal opinion and I hope the driver authors will agree
with my proposal, or just ignore this commit if not.

https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 drivers/watchdog/meson_gxbb_wdt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
index 945f5e65db57..d3c9e2f6e63b 100644
--- a/drivers/watchdog/meson_gxbb_wdt.c
+++ b/drivers/watchdog/meson_gxbb_wdt.c
@@ -198,7 +198,6 @@ 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);
 	return devm_watchdog_register_device(dev, &data->wdt_dev);
 }
 
-- 
2.25.1

Re: [PATCH v4 1/3] watchdog: meson_gxbb_wdt: add nowayout parameter

[Guenter Roeck]

On Fri, Jul 30, 2021 at 12:13:53PM +0800, Artem Lapkin wrote:
> Add nowayout module parameter
> 
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---

<Formletter>  
Change log goes here. If it is missing, I won't know what changed.
That means I will have to dig out older patch versions to compare.
That costs time and would hold up both this patch as well as all other
patches which I still have to review.

For this reason, I will not review patches without change log.
</Formletter>

The change is small and recent enough that I remember, so

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

but please keep this in mind for future submissions.

Thanks,
Guenter

>  drivers/watchdog/meson_gxbb_wdt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> index 5a9ca10fbcfa..5aebc3a09652 100644
> --- a/drivers/watchdog/meson_gxbb_wdt.c
> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> @@ -29,6 +29,11 @@
>  #define GXBB_WDT_TCNT_SETUP_MASK		(BIT(16) - 1)
>  #define GXBB_WDT_TCNT_CNT_SHIFT			16
>  
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
>  struct meson_gxbb_wdt {
>  	void __iomem *reg_base;
>  	struct watchdog_device wdt_dev;
> @@ -175,6 +180,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
>  	data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
>  	data->wdt_dev.min_timeout = 1;
>  	data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> +	watchdog_set_nowayout(&data->wdt_dev, nowayout);
>  	watchdog_set_drvdata(&data->wdt_dev, data);
>  
>  	/* Setup with 1ms timebase */
> -- 
> 2.25.1
> 

Re: [PATCH v4 2/3] watchdog: meson_gxbb_wdt: add timeout parameter

[Guenter Roeck]

On Fri, Jul 30, 2021 at 12:13:54PM +0800, Artem Lapkin wrote:
> Add timeout module parameter
> 
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---

<Formletter>  
Change log goes here. If it is missing, I won't know what changed.
That means I will have to dig out older patch versions to compare.
That costs time and would hold up both this patch as well as all other
patches which I still have to review.

For this reason, I will not review patches without change log.
</Formletter>

As before, the change log is small and recent enough that I remember,
so you are lucky.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

>  drivers/watchdog/meson_gxbb_wdt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> index 5aebc3a09652..945f5e65db57 100644
> --- a/drivers/watchdog/meson_gxbb_wdt.c
> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> @@ -34,6 +34,11 @@ module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
>  		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> +static unsigned int timeout;
> +module_param(timeout, uint, 0);
> +MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds="
> +		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> +
>  struct meson_gxbb_wdt {
>  	void __iomem *reg_base;
>  	struct watchdog_device wdt_dev;
> @@ -180,6 +185,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
>  	data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
>  	data->wdt_dev.min_timeout = 1;
>  	data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> +	watchdog_init_timeout(&data->wdt_dev, timeout, dev);
>  	watchdog_set_nowayout(&data->wdt_dev, nowayout);
>  	watchdog_set_drvdata(&data->wdt_dev, data);
>  
> -- 
> 2.25.1
> 

Re: [PATCH v4 3/3] watchdog: meson_gxbb_wdt: remove stop_on_reboot

[Guenter Roeck]

On Fri, Jul 30, 2021 at 12:13:55PM +0800, Artem Lapkin wrote:
> Remove watchdog_stop_on_reboot()
> 
> Meson platform still have some hardware drivers problems for some
> configurations which can freeze device on shutdown/reboot stage and i
> think better to have reboot warranty by default.
> 
> I feel that it is important to keep the watchdog running during the
> reboot sequence, in the event that an abnormal driver freezes the reboot
> process.
> 
> This is my personal opinion and I hope the driver authors will agree
> with my proposal, or just ignore this commit if not.
> 
> https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t
> 

A much better description would be something like

"The Meson platform still has some hardware drivers problems for some
 configurations which can freeze devices on shutdown/reboot.
 Remove watchdog_stop_on_reboot() to catch this situation and ensure
 that the reboot happens anyway.
 Users who still want to stop the watchdog on reboot can still do so
 using the watchdog.stop_on_reboot=1 module parameter.
 "

That leaves the personal opinion out of the picture and provides both
a rationale for the change and an alternative for people who want
to stop the watchdog on reboot anyway.

> Signed-off-by: Artem Lapkin <art@khadas.com>

As mentioned, I'd still like to get an opinion from the driver
author and/or some other users of this platform. However, I'll
accept the patch with the above description change if I don't get
additional feedback.

Thanks,
Guenter

> ---
>  drivers/watchdog/meson_gxbb_wdt.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> index 945f5e65db57..d3c9e2f6e63b 100644
> --- a/drivers/watchdog/meson_gxbb_wdt.c
> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> @@ -198,7 +198,6 @@ 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);
>  	return devm_watchdog_register_device(dev, &data->wdt_dev);
>  }
>  
> -- 
> 2.25.1
> 

Re: [PATCH v4 3/3] watchdog: meson_gxbb_wdt: remove stop_on_reboot

[Art Nikpal]

Thank you very much for the helpful and detailed comments

Artem

On Fri, Jul 30, 2021 at 12:59 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Jul 30, 2021 at 12:13:55PM +0800, Artem Lapkin wrote:
> > Remove watchdog_stop_on_reboot()
> >
> > Meson platform still have some hardware drivers problems for some
> > configurations which can freeze device on shutdown/reboot stage and i
> > think better to have reboot warranty by default.
> >
> > I feel that it is important to keep the watchdog running during the
> > reboot sequence, in the event that an abnormal driver freezes the reboot
> > process.
> >
> > This is my personal opinion and I hope the driver authors will agree
> > with my proposal, or just ignore this commit if not.
> >
> > https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t
> >
>
> A much better description would be something like
>
> "The Meson platform still has some hardware drivers problems for some
>  configurations which can freeze devices on shutdown/reboot.
>  Remove watchdog_stop_on_reboot() to catch this situation and ensure
>  that the reboot happens anyway.
>  Users who still want to stop the watchdog on reboot can still do so
>  using the watchdog.stop_on_reboot=1 module parameter.
>  "
>
> That leaves the personal opinion out of the picture and provides both
> a rationale for the change and an alternative for people who want
> to stop the watchdog on reboot anyway.
>
> > Signed-off-by: Artem Lapkin <art@khadas.com>
>
> As mentioned, I'd still like to get an opinion from the driver
> author and/or some other users of this platform. However, I'll
> accept the patch with the above description change if I don't get
> additional feedback.
>
> Thanks,
> Guenter
>
> > ---
> >  drivers/watchdog/meson_gxbb_wdt.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> > index 945f5e65db57..d3c9e2f6e63b 100644
> > --- a/drivers/watchdog/meson_gxbb_wdt.c
> > +++ b/drivers/watchdog/meson_gxbb_wdt.c
> > @@ -198,7 +198,6 @@ 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);
> >       return devm_watchdog_register_device(dev, &data->wdt_dev);
> >  }
> >
> > --
> > 2.25.1
> >

Re: [PATCH v4 3/3] watchdog: meson_gxbb_wdt: remove stop_on_reboot

[Neil Armstrong]

Hi Guenter,

On 30/07/2021 06:58, Guenter Roeck wrote:
> On Fri, Jul 30, 2021 at 12:13:55PM +0800, Artem Lapkin wrote:
>> Remove watchdog_stop_on_reboot()
>>
>> Meson platform still have some hardware drivers problems for some
>> configurations which can freeze device on shutdown/reboot stage and i
>> think better to have reboot warranty by default.
>>
>> I feel that it is important to keep the watchdog running during the
>> reboot sequence, in the event that an abnormal driver freezes the reboot
>> process.
>>
>> This is my personal opinion and I hope the driver authors will agree
>> with my proposal, or just ignore this commit if not.
>>
>> https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t
>>
> 
> A much better description would be something like
> 
> "The Meson platform still has some hardware drivers problems for some
>  configurations which can freeze devices on shutdown/reboot.
>  Remove watchdog_stop_on_reboot() to catch this situation and ensure
>  that the reboot happens anyway.
>  Users who still want to stop the watchdog on reboot can still do so
>  using the watchdog.stop_on_reboot=1 module parameter.
>  "
> 
> That leaves the personal opinion out of the picture and provides both
> a rationale for the change and an alternative for people who want
> to stop the watchdog on reboot anyway.
> 
>> Signed-off-by: Artem Lapkin <art@khadas.com>
> 
> As mentioned, I'd still like to get an opinion from the driver
> author and/or some other users of this platform. However, I'll
> accept the patch with the above description change if I don't get
> additional feedback.

Sorry for the reply delay and thanks a lot for your review.

The rationale from Artem is OK for me.

Please add my Acked-by for the whole patchset.

Neil

> 
> Thanks,
> Guenter
> 
>> ---
>>  drivers/watchdog/meson_gxbb_wdt.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
>> index 945f5e65db57..d3c9e2f6e63b 100644
>> --- a/drivers/watchdog/meson_gxbb_wdt.c
>> +++ b/drivers/watchdog/meson_gxbb_wdt.c
>> @@ -198,7 +198,6 @@ 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);
>>  	return devm_watchdog_register_device(dev, &data->wdt_dev);
>>  }
>>  
>> -- 
>> 2.25.1
>>

Re: [PATCH v4 0/3] watchdog: meson_gxbb_wdt: improve

[Art Nikpal]

hi Guenter Roeck
why still not merged to upstream ?

On Fri, Jul 30, 2021 at 12:14 PM Artem Lapkin <email2tema@gmail.com> wrote:
>
> * Added nowayout module parameter
> * Added timeout module parameter
> * Removed watchdog_stop_on_reboot
>
> https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t
>
> Artem Lapkin (3):
>   watchdog: meson_gxbb_wdt: add nowayout parameter
>   watchdog: meson_gxbb_wdt: add timeout parameter
>   watchdog: meson_gxbb_wdt: remove stop_on_reboot
>
>  drivers/watchdog/meson_gxbb_wdt.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> --
> 2.25.1
>

Re: [PATCH v4 1/3] watchdog: meson_gxbb_wdt: add nowayout parameter

[Art Nikpal]

hi Guenter Roeck
why still not merged to upstream ?

On Fri, Jul 30, 2021 at 12:45 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Jul 30, 2021 at 12:13:53PM +0800, Artem Lapkin wrote:
> > Add nowayout module parameter
> >
> > Signed-off-by: Artem Lapkin <art@khadas.com>
> > ---
>
> <Formletter>
> Change log goes here. If it is missing, I won't know what changed.
> That means I will have to dig out older patch versions to compare.
> That costs time and would hold up both this patch as well as all other
> patches which I still have to review.
>
> For this reason, I will not review patches without change log.
> </Formletter>
>
> The change is small and recent enough that I remember, so
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> but please keep this in mind for future submissions.
>
> Thanks,
> Guenter
>
> >  drivers/watchdog/meson_gxbb_wdt.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> > index 5a9ca10fbcfa..5aebc3a09652 100644
> > --- a/drivers/watchdog/meson_gxbb_wdt.c
> > +++ b/drivers/watchdog/meson_gxbb_wdt.c
> > @@ -29,6 +29,11 @@
> >  #define GXBB_WDT_TCNT_SETUP_MASK             (BIT(16) - 1)
> >  #define GXBB_WDT_TCNT_CNT_SHIFT                      16
> >
> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> > +module_param(nowayout, bool, 0);
> > +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
> > +              __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> >  struct meson_gxbb_wdt {
> >       void __iomem *reg_base;
> >       struct watchdog_device wdt_dev;
> > @@ -175,6 +180,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
> >       data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
> >       data->wdt_dev.min_timeout = 1;
> >       data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> > +     watchdog_set_nowayout(&data->wdt_dev, nowayout);
> >       watchdog_set_drvdata(&data->wdt_dev, data);
> >
> >       /* Setup with 1ms timebase */
> > --
> > 2.25.1
> >

Re: [PATCH v4 2/3] watchdog: meson_gxbb_wdt: add timeout parameter

[Art Nikpal]

hi Guenter Roeck
why still not merged to upstream ?

On Fri, Jul 30, 2021 at 12:14 PM Artem Lapkin <email2tema@gmail.com> wrote:
>
> Add timeout module parameter
>
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---
>  drivers/watchdog/meson_gxbb_wdt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> index 5aebc3a09652..945f5e65db57 100644
> --- a/drivers/watchdog/meson_gxbb_wdt.c
> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> @@ -34,6 +34,11 @@ module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started default="
>                  __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>
> +static unsigned int timeout;
> +module_param(timeout, uint, 0);
> +MODULE_PARM_DESC(timeout, "Watchdog heartbeat in seconds="
> +                __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> +
>  struct meson_gxbb_wdt {
>         void __iomem *reg_base;
>         struct watchdog_device wdt_dev;
> @@ -180,6 +185,7 @@ static int meson_gxbb_wdt_probe(struct platform_device *pdev)
>         data->wdt_dev.max_hw_heartbeat_ms = GXBB_WDT_TCNT_SETUP_MASK;
>         data->wdt_dev.min_timeout = 1;
>         data->wdt_dev.timeout = DEFAULT_TIMEOUT;
> +       watchdog_init_timeout(&data->wdt_dev, timeout, dev);
>         watchdog_set_nowayout(&data->wdt_dev, nowayout);
>         watchdog_set_drvdata(&data->wdt_dev, data);
>
> --
> 2.25.1
>

Re: [PATCH v4 3/3] watchdog: meson_gxbb_wdt: remove stop_on_reboot

[Art Nikpal]

hi Guenter Roeck
why still not merged to upstream ?

On Fri, Jul 30, 2021 at 12:14 PM Artem Lapkin <email2tema@gmail.com> wrote:
>
> Remove watchdog_stop_on_reboot()
>
> Meson platform still have some hardware drivers problems for some
> configurations which can freeze device on shutdown/reboot stage and i
> think better to have reboot warranty by default.
>
> I feel that it is important to keep the watchdog running during the
> reboot sequence, in the event that an abnormal driver freezes the reboot
> process.
>
> This is my personal opinion and I hope the driver authors will agree
> with my proposal, or just ignore this commit if not.
>
> https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t
>
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---
>  drivers/watchdog/meson_gxbb_wdt.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> index 945f5e65db57..d3c9e2f6e63b 100644
> --- a/drivers/watchdog/meson_gxbb_wdt.c
> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> @@ -198,7 +198,6 @@ 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);
>         return devm_watchdog_register_device(dev, &data->wdt_dev);
>  }
>
> --
> 2.25.1
>

Re: [PATCH v4 3/3] watchdog: meson_gxbb_wdt: remove stop_on_reboot

[Guenter Roeck]

On 11/8/21 11:59 PM, Art Nikpal wrote:
> hi Guenter Roeck
> why still not merged to upstream ?
> 

I had asked you to provide an updated description, without the "personal
opinion" part which does not belong into a commit log. The other two
patches wait for Wim to send them upstream.

Guenter


> On Fri, Jul 30, 2021 at 12:14 PM Artem Lapkin <email2tema@gmail.com> wrote:
>>
>> Remove watchdog_stop_on_reboot()
>>
>> Meson platform still have some hardware drivers problems for some
>> configurations which can freeze device on shutdown/reboot stage and i
>> think better to have reboot warranty by default.
>>
>> I feel that it is important to keep the watchdog running during the
>> reboot sequence, in the event that an abnormal driver freezes the reboot
>> process.
>>
>> This is my personal opinion and I hope the driver authors will agree
>> with my proposal, or just ignore this commit if not.
>>
>> https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t
>>
>> Signed-off-by: Artem Lapkin <art@khadas.com>
>> ---
>>   drivers/watchdog/meson_gxbb_wdt.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
>> index 945f5e65db57..d3c9e2f6e63b 100644
>> --- a/drivers/watchdog/meson_gxbb_wdt.c
>> +++ b/drivers/watchdog/meson_gxbb_wdt.c
>> @@ -198,7 +198,6 @@ 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);
>>          return devm_watchdog_register_device(dev, &data->wdt_dev);
>>   }
>>
>> --
>> 2.25.1
>>

Re: [PATCH v4 3/3] watchdog: meson_gxbb_wdt: remove stop_on_reboot

[Art Nikpal]

On Tue, Nov 9, 2021 at 11:30 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 11/8/21 11:59 PM, Art Nikpal wrote:
> > hi Guenter Roeck
> > why still not merged to upstream ?
> >
>
> I had asked you to provide an updated description, without the "personal
> opinion" part which does not belong into a commit log. The other two
> patches wait for Wim to send them upstream.
>

ok i have send this patch with new description again

> Guenter
>
>
> > On Fri, Jul 30, 2021 at 12:14 PM Artem Lapkin <email2tema@gmail.com> wrote:
> >>
> >> Remove watchdog_stop_on_reboot()
> >>
> >> Meson platform still have some hardware drivers problems for some
> >> configurations which can freeze device on shutdown/reboot stage and i
> >> think better to have reboot warranty by default.
> >>
> >> I feel that it is important to keep the watchdog running during the
> >> reboot sequence, in the event that an abnormal driver freezes the reboot
> >> process.
> >>
> >> This is my personal opinion and I hope the driver authors will agree
> >> with my proposal, or just ignore this commit if not.
> >>
> >> https://lore.kernel.org/linux-watchdog/20210729072308.1908904-1-art@khadas.com/T/#t
> >>
> >> Signed-off-by: Artem Lapkin <art@khadas.com>
> >> ---
> >>   drivers/watchdog/meson_gxbb_wdt.c | 1 -
> >>   1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/watchdog/meson_gxbb_wdt.c b/drivers/watchdog/meson_gxbb_wdt.c
> >> index 945f5e65db57..d3c9e2f6e63b 100644
> >> --- a/drivers/watchdog/meson_gxbb_wdt.c
> >> +++ b/drivers/watchdog/meson_gxbb_wdt.c
> >> @@ -198,7 +198,6 @@ 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);
> >>          return devm_watchdog_register_device(dev, &data->wdt_dev);
> >>   }
> >>
> >> --
> >> 2.25.1
> >>
>