[PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[Primoz Fiser]

Document the watchdog disable option which can be used if the hardware
automatic suspend option is broken.

Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
suspend disable option").

Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
---
 Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt b/Documentation/devicetree/bindings/mfd/da9063.txt
index 91b79a21d403..aa8b800cc4ad 100644
--- a/Documentation/devicetree/bindings/mfd/da9063.txt
+++ b/Documentation/devicetree/bindings/mfd/da9063.txt
@@ -64,10 +64,13 @@ Sub-nodes:
     and KEY_SLEEP.
 
 - watchdog : This node defines settings for the Watchdog timer associated
-  with the DA9063 and DA9063L. There are currently no entries in this
-  binding, however compatible = "dlg,da9063-watchdog" should be added
-  if a node is created.
+  with the DA9063 and DA9063L. The node should contain the compatible property
+  with the value "dlg,da9063-watchdog".
 
+  Optional watchdog properties:
+  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.
+  Only use this option if you can't use the watchdog automatic suspend
+  function during a suspend (see register CONTROL_B).
 
 Example:
 
-- 
2.25.1

[PATCH 2/3] watchdog: da9063: optionally disable watchdog during suspend

[Primoz Fiser]

Optionally disable watchdog during suspend (if enabled) and re-enable
it upon resume.
This enables boards to sleep without being interrupted by the watchdog.

This patch is based on commit f6c98b08381c ("watchdog: da9062: add
power management ops") and commit 8541673d2a5f ("watchdog: da9062: fix
power management ops") and brings the same functionality to DA9063.

Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
---
 drivers/watchdog/da9063_wdt.c | 36 +++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
index 9adad1862bbd..09a4af4c58fc 100644
--- a/drivers/watchdog/da9063_wdt.c
+++ b/drivers/watchdog/da9063_wdt.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 #include <linux/mfd/da9063/registers.h>
 #include <linux/mfd/da9063/core.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 
 /*
@@ -26,6 +27,8 @@
  *   others: timeout = 2048 ms * 2^(TWDSCALE-1).
  */
 static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 };
+static bool use_sw_pm;
+
 #define DA9063_TWDSCALE_DISABLE		0
 #define DA9063_TWDSCALE_MIN		1
 #define DA9063_TWDSCALE_MAX		(ARRAY_SIZE(wdt_timeout) - 1)
@@ -218,6 +221,8 @@ static int da9063_wdt_probe(struct platform_device *pdev)
 	if (!wdd)
 		return -ENOMEM;
 
+	use_sw_pm = device_property_present(dev, "dlg,use-sw-pm");
+
 	wdd->info = &da9063_watchdog_info;
 	wdd->ops = &da9063_watchdog_ops;
 	wdd->min_timeout = DA9063_WDT_MIN_TIMEOUT;
@@ -228,6 +233,7 @@ static int da9063_wdt_probe(struct platform_device *pdev)
 
 	watchdog_set_restart_priority(wdd, 128);
 	watchdog_set_drvdata(wdd, da9063);
+	dev_set_drvdata(dev, wdd);
 
 	wdd->timeout = DA9063_WDG_TIMEOUT;
 
@@ -249,10 +255,40 @@ static int da9063_wdt_probe(struct platform_device *pdev)
 	return devm_watchdog_register_device(dev, wdd);
 }
 
+static int __maybe_unused da9063_wdt_suspend(struct device *dev)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	if (!use_sw_pm)
+		return 0;
+
+	if (watchdog_active(wdd))
+		return da9063_wdt_stop(wdd);
+
+	return 0;
+}
+
+static int __maybe_unused da9063_wdt_resume(struct device *dev)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	if (!use_sw_pm)
+		return 0;
+
+	if (watchdog_active(wdd))
+		return da9063_wdt_start(wdd);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(da9063_wdt_pm_ops,
+			da9063_wdt_suspend, da9063_wdt_resume);
+
 static struct platform_driver da9063_wdt_driver = {
 	.probe = da9063_wdt_probe,
 	.driver = {
 		.name = DA9063_DRVNAME_WATCHDOG,
+		.pm = &da9063_wdt_pm_ops,
 	},
 };
 module_platform_driver(da9063_wdt_driver);
-- 
2.25.1

RE: [PATCH 2/3] watchdog: da9063: optionally disable watchdog during suspend

[DLG Adam Thomson]

On 22 April 2022 08:27, Primoz Fiser wrote:

> Optionally disable watchdog during suspend (if enabled) and re-enable
> it upon resume.
> This enables boards to sleep without being interrupted by the watchdog.
>
> This patch is based on commit f6c98b08381c ("watchdog: da9062: add
> power management ops") and commit 8541673d2a5f ("watchdog: da9062: fix
> power management ops") and brings the same functionality to DA9063.

There's a WATCHDOG_PD bit (set to 0) to achieve this I believe, and thus
removes the need for the suspend/resume PM functions. Is this something you've
tried? Also seems to be present for DA9061/2 as well so can't remember why that
wasn't used there.

Re: [PATCH 2/3] watchdog: da9063: optionally disable watchdog during suspend

[Primoz Fiser]

On 9. 05. 22 12:38, DLG Adam Thomson wrote:
> On 22 April 2022 08:27, Primoz Fiser wrote:
> 
>> Optionally disable watchdog during suspend (if enabled) and re-enable
>> it upon resume.
>> This enables boards to sleep without being interrupted by the watchdog.
>>
>> This patch is based on commit f6c98b08381c ("watchdog: da9062: add
>> power management ops") and commit 8541673d2a5f ("watchdog: da9062: fix
>> power management ops") and brings the same functionality to DA9063.
> 
> There's a WATCHDOG_PD bit (set to 0) to achieve this I believe, and thus
> removes the need for the suspend/resume PM functions. Is this something you've
> tried? Also seems to be present for DA9061/2 as well so can't remember why that
> wasn't used there.

Ideally one should be able to use WATCHDOG_PD bit indeed.

However there are boards out there which don't have the ability to use 
the PMIC's powerdown/active mode due to PCB design and thus PMIC is left 
enabled during suspend i.e. cannot use the POWERDOWN mode.

Check mailing list correspondence [1] which already gave explanation why 
there was a need to implement such quirks for da9062 and the need to 
handle this in software instead of hardware.

Links:

[1] 
https://lore.kernel.org/all/20191128171931.22563-1-m.felsch@pengutronix.de/

RE: [PATCH 2/3] watchdog: da9063: optionally disable watchdog during suspend

[DLG Adam Thomson]

On 11 May 2022 09:48, Primoz Fiser wrote:

> >> Optionally disable watchdog during suspend (if enabled) and re-enable
> >> it upon resume.
> >> This enables boards to sleep without being interrupted by the watchdog.
> >>
> >> This patch is based on commit f6c98b08381c ("watchdog: da9062: add
> >> power management ops") and commit 8541673d2a5f ("watchdog: da9062: fix
> >> power management ops") and brings the same functionality to DA9063.
> >
> > There's a WATCHDOG_PD bit (set to 0) to achieve this I believe, and thus
> > removes the need for the suspend/resume PM functions. Is this something
> you've
> > tried? Also seems to be present for DA9061/2 as well so can't remember why
> that
> > wasn't used there.
>
> Ideally one should be able to use WATCHDOG_PD bit indeed.
>
> However there are boards out there which don't have the ability to use
> the PMIC's powerdown/active mode due to PCB design and thus PMIC is left
> enabled during suspend i.e. cannot use the POWERDOWN mode.
>
> Check mailing list correspondence [1] which already gave explanation why
> there was a need to implement such quirks for da9062 and the need to
> handle this in software instead of hardware.
>
> Links:
>
> [1]
> https://lore.kernel.org/all/20191128171931.22563-1-m.felsch@pengutronix.de/

At least I'm consistent in my first response :)

Yes I remember the discussion now. Thanks for the reminder. In which case:

Reviewed-by: Adam Thomson <DLG-Adam.Thomson.Opensource@dm.renesas.com>

Re: [PATCH 2/3] watchdog: da9063: optionally disable watchdog during suspend

[Guenter Roeck]

On Fri, Apr 22, 2022 at 09:27:12AM +0200, Primoz Fiser wrote:
> Optionally disable watchdog during suspend (if enabled) and re-enable
> it upon resume.
> This enables boards to sleep without being interrupted by the watchdog.
> 
> This patch is based on commit f6c98b08381c ("watchdog: da9062: add
> power management ops") and commit 8541673d2a5f ("watchdog: da9062: fix
> power management ops") and brings the same functionality to DA9063.
> 
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> Reviewed-by: Adam Thomson <DLG-Adam.Thomson.Opensource@dm.renesas.com>

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

> ---
>  drivers/watchdog/da9063_wdt.c | 36 +++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
> index 9adad1862bbd..09a4af4c58fc 100644
> --- a/drivers/watchdog/da9063_wdt.c
> +++ b/drivers/watchdog/da9063_wdt.c
> @@ -18,6 +18,7 @@
>  #include <linux/delay.h>
>  #include <linux/mfd/da9063/registers.h>
>  #include <linux/mfd/da9063/core.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  
>  /*
> @@ -26,6 +27,8 @@
>   *   others: timeout = 2048 ms * 2^(TWDSCALE-1).
>   */
>  static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 };
> +static bool use_sw_pm;
> +
>  #define DA9063_TWDSCALE_DISABLE		0
>  #define DA9063_TWDSCALE_MIN		1
>  #define DA9063_TWDSCALE_MAX		(ARRAY_SIZE(wdt_timeout) - 1)
> @@ -218,6 +221,8 @@ static int da9063_wdt_probe(struct platform_device *pdev)
>  	if (!wdd)
>  		return -ENOMEM;
>  
> +	use_sw_pm = device_property_present(dev, "dlg,use-sw-pm");
> +
>  	wdd->info = &da9063_watchdog_info;
>  	wdd->ops = &da9063_watchdog_ops;
>  	wdd->min_timeout = DA9063_WDT_MIN_TIMEOUT;
> @@ -228,6 +233,7 @@ static int da9063_wdt_probe(struct platform_device *pdev)
>  
>  	watchdog_set_restart_priority(wdd, 128);
>  	watchdog_set_drvdata(wdd, da9063);
> +	dev_set_drvdata(dev, wdd);
>  
>  	wdd->timeout = DA9063_WDG_TIMEOUT;
>  
> @@ -249,10 +255,40 @@ static int da9063_wdt_probe(struct platform_device *pdev)
>  	return devm_watchdog_register_device(dev, wdd);
>  }
>  
> +static int __maybe_unused da9063_wdt_suspend(struct device *dev)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +
> +	if (!use_sw_pm)
> +		return 0;
> +
> +	if (watchdog_active(wdd))
> +		return da9063_wdt_stop(wdd);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused da9063_wdt_resume(struct device *dev)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +
> +	if (!use_sw_pm)
> +		return 0;
> +
> +	if (watchdog_active(wdd))
> +		return da9063_wdt_start(wdd);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(da9063_wdt_pm_ops,
> +			da9063_wdt_suspend, da9063_wdt_resume);
> +
>  static struct platform_driver da9063_wdt_driver = {
>  	.probe = da9063_wdt_probe,
>  	.driver = {
>  		.name = DA9063_DRVNAME_WATCHDOG,
> +		.pm = &da9063_wdt_pm_ops,
>  	},
>  };
>  module_platform_driver(da9063_wdt_driver);

[PATCH 3/3] ARM: dts: imx6: phyFLEX: disable da9063 watchdog during suspend

[Primoz Fiser]

If DA9063 PMIC is left enabled during suspend, PMIC's watchdog needs to
be disabled before entering suspended state to allow board to sleep for
longer period than the watchdog timeout period. Otherwise board will be
reset by the watchdog. Thus disable watchdog on suspend and re-enable it
upon resume.

Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
---
 arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
index 1f2ba6f6254e..56b29c3294c6 100644
--- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
@@ -212,6 +212,7 @@ da9063_rtc: rtc {
 
 		da9063_wdog: watchdog {
 			compatible = "dlg,da9063-watchdog";
+			dlg,use-sw-pm;
 		};
 
 		onkey {
-- 
2.25.1

Re: [PATCH 3/3] ARM: dts: imx6: phyFLEX: disable da9063 watchdog during suspend

[Guenter Roeck]

On Fri, Apr 22, 2022 at 09:27:13AM +0200, Primoz Fiser wrote:
> If DA9063 PMIC is left enabled during suspend, PMIC's watchdog needs to
> be disabled before entering suspended state to allow board to sleep for
> longer period than the watchdog timeout period. Otherwise board will be
> reset by the watchdog. Thus disable watchdog on suspend and re-enable it
> upon resume.
> 
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>

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

> ---
>  arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
> index 1f2ba6f6254e..56b29c3294c6 100644
> --- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
> @@ -212,6 +212,7 @@ da9063_rtc: rtc {
>  
>  		da9063_wdog: watchdog {
>  			compatible = "dlg,da9063-watchdog";
> +			dlg,use-sw-pm;
>  		};
>  
>  		onkey {

Re: [PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[Rob Herring]

On Fri, Apr 22, 2022 at 09:27:11AM +0200, Primoz Fiser wrote:
> Document the watchdog disable option which can be used if the hardware
> automatic suspend option is broken.
> 
> Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
> suspend disable option").
> 
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> ---
>  Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt b/Documentation/devicetree/bindings/mfd/da9063.txt
> index 91b79a21d403..aa8b800cc4ad 100644
> --- a/Documentation/devicetree/bindings/mfd/da9063.txt
> +++ b/Documentation/devicetree/bindings/mfd/da9063.txt
> @@ -64,10 +64,13 @@ Sub-nodes:
>      and KEY_SLEEP.
>  
>  - watchdog : This node defines settings for the Watchdog timer associated
> -  with the DA9063 and DA9063L. There are currently no entries in this
> -  binding, however compatible = "dlg,da9063-watchdog" should be added
> -  if a node is created.
> +  with the DA9063 and DA9063L. The node should contain the compatible property
> +  with the value "dlg,da9063-watchdog".
>  
> +  Optional watchdog properties:
> +  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.

Name the property based on the h/w quirk rather than what to do in 
response. Something like 'dlg,hw-suspend-broken'

> +  Only use this option if you can't use the watchdog automatic suspend
> +  function during a suspend (see register CONTROL_B).
>  
>  Example:
>  
> -- 
> 2.25.1
> 
> 

Re: [PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[Primoz Fiser]

Hi Rob,

>> Name the property based on the h/w quirk rather than what to do in
>> response. Something like 'dlg,hw-suspend-broken'

Shouldn't we match da9062's property?

As this commit is based on c514430c51ee8 ("dt-bindings: watchdog: 
da9062: add suspend disable option") which uses "dlg,use-sw-pm" as 
property to implement the same functionality.

Sure I can spin up v2 with your proposal but I think that would create 
unnecessary ambiguity and confusion?

For example, phyCORE board uses da9062 PMIC while phyFLEX uses da9063 as 
PMIC. Boards are from the same SoM vendor. So one board would have to 
use "dlg,use-sw-pm" and the other one "dlg,hw-suspend-broken" property 
to achieve the same thing?


On 2. 05. 22 22:56, Rob Herring wrote:
> On Fri, Apr 22, 2022 at 09:27:11AM +0200, Primoz Fiser wrote:
>> Document the watchdog disable option which can be used if the hardware
>> automatic suspend option is broken.
>>
>> Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
>> suspend disable option").
>>
>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>> ---
>>   Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt b/Documentation/devicetree/bindings/mfd/da9063.txt
>> index 91b79a21d403..aa8b800cc4ad 100644
>> --- a/Documentation/devicetree/bindings/mfd/da9063.txt
>> +++ b/Documentation/devicetree/bindings/mfd/da9063.txt
>> @@ -64,10 +64,13 @@ Sub-nodes:
>>       and KEY_SLEEP.
>>   
>>   - watchdog : This node defines settings for the Watchdog timer associated
>> -  with the DA9063 and DA9063L. There are currently no entries in this
>> -  binding, however compatible = "dlg,da9063-watchdog" should be added
>> -  if a node is created.
>> +  with the DA9063 and DA9063L. The node should contain the compatible property
>> +  with the value "dlg,da9063-watchdog".
>>   
>> +  Optional watchdog properties:
>> +  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.
> 
> Name the property based on the h/w quirk rather than what to do in
> response. Something like 'dlg,hw-suspend-broken'
> 
>> +  Only use this option if you can't use the watchdog automatic suspend
>> +  function during a suspend (see register CONTROL_B).
>>   
>>   Example:
>>   
>> -- 
>> 2.25.1
>>
>>

Re: [PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[Rob Herring]

On Tue, May 03, 2022 at 08:53:42AM +0200, Primoz Fiser wrote:
> Hi Rob,
> 

Please don't top post. Trim any irrelevant parts and reply below the 
original quoted text.

> > > Name the property based on the h/w quirk rather than what to do in
> > > response. Something like 'dlg,hw-suspend-broken'
> 
> Shouldn't we match da9062's property?

Ah, yes. I failed to grasp that from the 'based on commit...' in the 
commit msg.


> 
> As this commit is based on c514430c51ee8 ("dt-bindings: watchdog: da9062:
> add suspend disable option") which uses "dlg,use-sw-pm" as property to
> implement the same functionality.

The 'which uses "dlg,use-sw-pm" as property to implement the same 
functionality' part would be useful in the commit msg. With that,

Reviewed-by: Rob Herring <robh@kernel.org>

> 
> Sure I can spin up v2 with your proposal but I think that would create
> unnecessary ambiguity and confusion?
> 
> For example, phyCORE board uses da9062 PMIC while phyFLEX uses da9063 as
> PMIC. Boards are from the same SoM vendor. So one board would have to use
> "dlg,use-sw-pm" and the other one "dlg,hw-suspend-broken" property to
> achieve the same thing?
> 
> 
> On 2. 05. 22 22:56, Rob Herring wrote:
> > On Fri, Apr 22, 2022 at 09:27:11AM +0200, Primoz Fiser wrote:
> > > Document the watchdog disable option which can be used if the hardware
> > > automatic suspend option is broken.
> > > 
> > > Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
> > > suspend disable option").
> > > 
> > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> > > ---
> > >   Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
> > >   1 file changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt b/Documentation/devicetree/bindings/mfd/da9063.txt
> > > index 91b79a21d403..aa8b800cc4ad 100644
> > > --- a/Documentation/devicetree/bindings/mfd/da9063.txt
> > > +++ b/Documentation/devicetree/bindings/mfd/da9063.txt
> > > @@ -64,10 +64,13 @@ Sub-nodes:
> > >       and KEY_SLEEP.
> > >   - watchdog : This node defines settings for the Watchdog timer associated
> > > -  with the DA9063 and DA9063L. There are currently no entries in this
> > > -  binding, however compatible = "dlg,da9063-watchdog" should be added
> > > -  if a node is created.
> > > +  with the DA9063 and DA9063L. The node should contain the compatible property
> > > +  with the value "dlg,da9063-watchdog".
> > > +  Optional watchdog properties:
> > > +  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.
> > 
> > Name the property based on the h/w quirk rather than what to do in
> > response. Something like 'dlg,hw-suspend-broken'
> > 
> > > +  Only use this option if you can't use the watchdog automatic suspend
> > > +  function during a suspend (see register CONTROL_B).
> > >   Example:
> > > -- 
> > > 2.25.1
> > > 
> > > 

RE: [PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[DLG Adam Thomson]

On 22 April 2022 08:27, Primoz Fiser wrote:

> Document the watchdog disable option which can be used if the hardware
> automatic suspend option is broken.
> 
> Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
> suspend disable option").
> 
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> ---
>  Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt
> b/Documentation/devicetree/bindings/mfd/da9063.txt
> index 91b79a21d403..aa8b800cc4ad 100644
> --- a/Documentation/devicetree/bindings/mfd/da9063.txt
> +++ b/Documentation/devicetree/bindings/mfd/da9063.txt
> @@ -64,10 +64,13 @@ Sub-nodes:
>      and KEY_SLEEP.
> 
>  - watchdog : This node defines settings for the Watchdog timer associated

I don't know if this is just me, but it looks like you're deleting this line
above, but not replacing it.....

> -  with the DA9063 and DA9063L. There are currently no entries in this
> -  binding, however compatible = "dlg,da9063-watchdog" should be added
> -  if a node is created.

....here, if I'm reading this patch correctly. This means we're losing that
property description, and starting a text block with the below text.

> +  with the DA9063 and DA9063L. The node should contain the compatible
> property
> +  with the value "dlg,da9063-watchdog".
> 
> +  Optional watchdog properties:
> +  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.
> +  Only use this option if you can't use the watchdog automatic suspend
> +  function during a suspend (see register CONTROL_B).
> 
>  Example:
> 
> --
> 2.25.1

Re: [PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[Primoz Fiser]

On 9. 05. 22 10:50, DLG Adam Thomson wrote:
> On 22 April 2022 08:27, Primoz Fiser wrote:
> 
>> Document the watchdog disable option which can be used if the hardware
>> automatic suspend option is broken.
>>
>> Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
>> suspend disable option").
>>
>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>> ---
>>   Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt
>> b/Documentation/devicetree/bindings/mfd/da9063.txt
>> index 91b79a21d403..aa8b800cc4ad 100644
>> --- a/Documentation/devicetree/bindings/mfd/da9063.txt
>> +++ b/Documentation/devicetree/bindings/mfd/da9063.txt
>> @@ -64,10 +64,13 @@ Sub-nodes:
>>       and KEY_SLEEP.
>>
>>   - watchdog : This node defines settings for the Watchdog timer associated
> 
> I don't know if this is just me, but it looks like you're deleting this line
> above, but not replacing it.....

I am not deleting this line, please note the leading white-space.

But yeah, if you don't pay close attention it looks a bit confusing 
indeed :)

> 
>> -  with the DA9063 and DA9063L. There are currently no entries in this
>> -  binding, however compatible = "dlg,da9063-watchdog" should be added
>> -  if a node is created.
> 
> ....here, if I'm reading this patch correctly. This means we're losing that
> property description, and starting a text block with the below text.
> 
>> +  with the DA9063 and DA9063L. The node should contain the compatible
>> property
>> +  with the value "dlg,da9063-watchdog".
>>
>> +  Optional watchdog properties:
>> +  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.
>> +  Only use this option if you can't use the watchdog automatic suspend
>> +  function during a suspend (see register CONTROL_B).
>>
>>   Example:
>>
>> --
>> 2.25.1
> 

Would you like:

The node should contain the compatible property with the value 
compatible = "dlg,da9063-watchdog".

i.e. explicitly adding "compatible =" in front, for v2?

RE: [PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[DLG Adam Thomson]

On 11 May 2022 09:10, Primoz Fiser wrote:

> >> Document the watchdog disable option which can be used if the hardware
> >> automatic suspend option is broken.
> >>
> >> Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
> >> suspend disable option").
> >>
> >> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> >> ---
> >>   Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
> >>   1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt
> >> b/Documentation/devicetree/bindings/mfd/da9063.txt
> >> index 91b79a21d403..aa8b800cc4ad 100644
> >> --- a/Documentation/devicetree/bindings/mfd/da9063.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/da9063.txt
> >> @@ -64,10 +64,13 @@ Sub-nodes:
> >>       and KEY_SLEEP.
> >>
> >>   - watchdog : This node defines settings for the Watchdog timer associated
> >
> > I don't know if this is just me, but it looks like you're deleting this line
> > above, but not replacing it.....
> 
> I am not deleting this line, please note the leading white-space.
> 
> But yeah, if you don't pay close attention it looks a bit confusing
> indeed :)

There you go. Thought it would be a strange deletion. Ignore me :)

> 
> >
> >> -  with the DA9063 and DA9063L. There are currently no entries in this
> >> -  binding, however compatible = "dlg,da9063-watchdog" should be added
> >> -  if a node is created.
> >
> > ....here, if I'm reading this patch correctly. This means we're losing that
> > property description, and starting a text block with the below text.
> >
> >> +  with the DA9063 and DA9063L. The node should contain the compatible
> >> property
> >> +  with the value "dlg,da9063-watchdog".
> >>
> >> +  Optional watchdog properties:
> >> +  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.
> >> +  Only use this option if you can't use the watchdog automatic suspend
> >> +  function during a suspend (see register CONTROL_B).
> >>
> >>   Example:
> >>
> >> --
> >> 2.25.1
> >
> 
> Would you like:
> 
> The node should contain the compatible property with the value
> compatible = "dlg,da9063-watchdog".
> 
> i.e. explicitly adding "compatible =" in front, for v2?

No change necessary. It just looked odd when I thought that first line was being
deleted. Given that's not the case:

Reviewed-by: Adam Thomson <DLG-Adam.Thomson.Opensource@dm.renesas.com>

Re: [PATCH 1/3] dt-bindings: mfd: da9063: watchdog: add suspend disable option

[Guenter Roeck]

On Fri, Apr 22, 2022 at 09:27:11AM +0200, Primoz Fiser wrote:
> Document the watchdog disable option which can be used if the hardware
> automatic suspend option is broken.
> 
> Based on commit c514430c51ee8 ("dt-bindings: watchdog: da9062: add
> suspend disable option").
> 
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Adam Thomson <DLG-Adam.Thomson.Opensource@dm.renesas.com>

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

> ---
>  Documentation/devicetree/bindings/mfd/da9063.txt | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/da9063.txt b/Documentation/devicetree/bindings/mfd/da9063.txt
> index 91b79a21d403..aa8b800cc4ad 100644
> --- a/Documentation/devicetree/bindings/mfd/da9063.txt
> +++ b/Documentation/devicetree/bindings/mfd/da9063.txt
> @@ -64,10 +64,13 @@ Sub-nodes:
>      and KEY_SLEEP.
>  
>  - watchdog : This node defines settings for the Watchdog timer associated
> -  with the DA9063 and DA9063L. There are currently no entries in this
> -  binding, however compatible = "dlg,da9063-watchdog" should be added
> -  if a node is created.
> +  with the DA9063 and DA9063L. The node should contain the compatible property
> +  with the value "dlg,da9063-watchdog".
>  
> +  Optional watchdog properties:
> +  - dlg,use-sw-pm: Add this property to disable the watchdog during suspend.
> +  Only use this option if you can't use the watchdog automatic suspend
> +  function during a suspend (see register CONTROL_B).
>  
>  Example:
>