All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
	Andrej Picej <andrej.picej@norik.com>
Cc: Support Opensource <Support.Opensource@diasemi.com>,
	"wim@linux-watchdog.org" <wim@linux-watchdog.org>,
	"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"linux-imx@nxp.com" <linux-imx@nxp.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 3/4] dt-bindings: watchdog: da9062: add watchdog timeout mode
Date: Tue, 30 Nov 2021 08:40:17 -0800	[thread overview]
Message-ID: <dcd75a82-5837-8d78-0a9f-6e5b7eafff28@roeck-us.net> (raw)
In-Reply-To: <DB9PR10MB4652C8A69A6A3F38B93ED18880679@DB9PR10MB4652.EURPRD10.PROD.OUTLOOK.COM>

On 11/30/21 8:11 AM, Adam Thomson wrote:
> On Guenter Roeck wrote:
> 
>>> Document the watchdog timeout mode property. If this property is used
>>> the user can select what happens on watchdog timeout. Set this property
>>> to 1 to enable SHUTDOWN (the device resets), set it to 0 and the device
>>> will go to POWERDOWN on watchdog timeout.
>>>
>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>> ---
>>>    Documentation/devicetree/bindings/watchdog/da9062-wdt.txt | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>> b/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>>> index 950e4fba8dbc..e3e6e56cee21 100644
>>> --- a/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>>> +++ b/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>>> @@ -10,6 +10,9 @@ Optional 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).
>>> +- dlg,wdt-sd: Set what happens on watchdog timeout. If this bit is set the
>>> +	watchdog timeout triggers SHUTDOWN, if cleared the watchdog triggers
>>> +	POWERDOWN. Can be 0 or 1.
>>>
>>
>> Why does it need a value ? Why not just bool ?
> 
> One argument might be that if the property isn't provided then the OTP
> configured value can persist without needing a FW change around this DT binding.
> 
> My belief though is that the majority of users would have this property set to 0
> by default in OTP, so a boolean would be OK I think here to enable watchdog
> shutdown.
> 

Sorry, you lost me.
	dlg,wdt-sd = <0>;
is the current situation, and identical to not having the property in
the first place.
	dlg,wdt-sd = <1>;
is new. I don't see the difference to
	dlg,wdt-sd;
vs. not having the property at all (which is, again, the current situation).
Since it has to be backward compatible,
	dlg,wdt-sd = <0>;
will always be identical to not having the property at all.
I can not find a situation where an integer would have any benefits over a boolean.

Guenter

WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net>
To: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
	Andrej Picej <andrej.picej@norik.com>
Cc: Support Opensource <Support.Opensource@diasemi.com>,
	"wim@linux-watchdog.org" <wim@linux-watchdog.org>,
	"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"linux-imx@nxp.com" <linux-imx@nxp.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 3/4] dt-bindings: watchdog: da9062: add watchdog timeout mode
Date: Tue, 30 Nov 2021 08:40:17 -0800	[thread overview]
Message-ID: <dcd75a82-5837-8d78-0a9f-6e5b7eafff28@roeck-us.net> (raw)
In-Reply-To: <DB9PR10MB4652C8A69A6A3F38B93ED18880679@DB9PR10MB4652.EURPRD10.PROD.OUTLOOK.COM>

On 11/30/21 8:11 AM, Adam Thomson wrote:
> On Guenter Roeck wrote:
> 
>>> Document the watchdog timeout mode property. If this property is used
>>> the user can select what happens on watchdog timeout. Set this property
>>> to 1 to enable SHUTDOWN (the device resets), set it to 0 and the device
>>> will go to POWERDOWN on watchdog timeout.
>>>
>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>> ---
>>>    Documentation/devicetree/bindings/watchdog/da9062-wdt.txt | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>> b/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>>> index 950e4fba8dbc..e3e6e56cee21 100644
>>> --- a/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>>> +++ b/Documentation/devicetree/bindings/watchdog/da9062-wdt.txt
>>> @@ -10,6 +10,9 @@ Optional 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).
>>> +- dlg,wdt-sd: Set what happens on watchdog timeout. If this bit is set the
>>> +	watchdog timeout triggers SHUTDOWN, if cleared the watchdog triggers
>>> +	POWERDOWN. Can be 0 or 1.
>>>
>>
>> Why does it need a value ? Why not just bool ?
> 
> One argument might be that if the property isn't provided then the OTP
> configured value can persist without needing a FW change around this DT binding.
> 
> My belief though is that the majority of users would have this property set to 0
> by default in OTP, so a boolean would be OK I think here to enable watchdog
> shutdown.
> 

Sorry, you lost me.
	dlg,wdt-sd = <0>;
is the current situation, and identical to not having the property in
the first place.
	dlg,wdt-sd = <1>;
is new. I don't see the difference to
	dlg,wdt-sd;
vs. not having the property at all (which is, again, the current situation).
Since it has to be backward compatible,
	dlg,wdt-sd = <0>;
will always be identical to not having the property at all.
I can not find a situation where an integer would have any benefits over a boolean.

Guenter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-11-30 16:40 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-30 13:42 [PATCH v2 1/4] mfd: da9062: make register CONFIG_I writable Andrej Picej
2021-11-30 13:42 ` Andrej Picej
2021-11-30 13:42 ` [PATCH v2 2/4] watchdog: da9062: reset board on watchdog timeout Andrej Picej
2021-11-30 13:42   ` Andrej Picej
2021-11-30 13:54   ` Andrej Picej
2021-11-30 13:54     ` Andrej Picej
2021-11-30 13:42 ` [PATCH v2 3/4] dt-bindings: watchdog: da9062: add watchdog timeout mode Andrej Picej
2021-11-30 13:42   ` Andrej Picej
2021-11-30 13:54   ` Andrej Picej
2021-11-30 13:54     ` Andrej Picej
2021-11-30 14:32   ` Guenter Roeck
2021-11-30 14:32     ` Guenter Roeck
2021-11-30 16:11     ` Adam Thomson
2021-11-30 16:11       ` Adam Thomson
2021-11-30 16:40       ` Guenter Roeck [this message]
2021-11-30 16:40         ` Guenter Roeck
2021-11-30 17:46         ` Adam Thomson
2021-11-30 17:46           ` Adam Thomson
2021-12-01  6:42           ` Andrej Picej
2021-12-01  6:42             ` Andrej Picej
2021-12-01  7:01             ` Guenter Roeck
2021-12-01  7:01               ` Guenter Roeck
2021-12-01  7:05               ` Andrej Picej
2021-12-01  7:05                 ` Andrej Picej
2021-11-30 13:42 ` [PATCH v2 4/4] ARM: dts: imx6: phycore-som: set watchdog timeout mode to shutdown Andrej Picej
2021-11-30 13:42   ` Andrej Picej
2021-11-30 13:56   ` Andrej Picej
2021-11-30 13:56     ` Andrej Picej
2021-11-30 14:31   ` Guenter Roeck
2021-11-30 14:31     ` Guenter Roeck
2021-11-30 13:49 ` [PATCH v2 1/4] mfd: da9062: make register CONFIG_I writable Andrej Picej
2021-11-30 13:49   ` Andrej Picej

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dcd75a82-5837-8d78-0a9f-6e5b7eafff28@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=Adam.Thomson.Opensource@diasemi.com \
    --cc=Support.Opensource@diasemi.com \
    --cc=andrej.picej@norik.com \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.