[RFC PATCH v1 0/3] convert gpio-restart bindings to yaml

[RFC PATCH v1 0/3] convert gpio-restart bindings to yaml

[Sander Vanheule]

This patch series first converts the gpio-restart bindings to the schema
format, so device descriptions can be verified against the binding.  Two
smaller patches then follow: one to fix the documentation, and a one to add
suffixes to values with implied units.

Open questions:
- Who should I add as maintainer for the binding, if not myself (patch 1)
- Should properties with names that don't match the guidelines be updated, or
  can the original names be kept? (patch 2, 3)
- Since the "priority" property is a Linux kernel specific thing, should it
  just deprecated entirely? (patch 3)

Sander Vanheule (3):
  dt-bindings: power: reset: Convert gpio-restart binding to schema
  dt-bindings: power: reset: gpio-restart: Add -ms suffix to delays
  dt-bindings: power: reset: gpio-restart: Correct default priority

 .../bindings/power/reset/gpio-restart.txt     |  54 ----------
 .../bindings/power/reset/gpio-restart.yaml    | 101 ++++++++++++++++++
 2 files changed, 101 insertions(+), 54 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/power/reset/gpio-restart.txt
 create mode 100644 Documentation/devicetree/bindings/power/reset/gpio-restart.yaml

-- 
2.33.1

[RFC PATCH v1 1/3] dt-bindings: power: reset: Convert gpio-restart binding to schema

[Sander Vanheule]

Convert the gpio-restart binding from plain text format to a schema
binding, maintaining the original content and updating formatting where
required.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 .../bindings/power/reset/gpio-restart.txt     | 54 -----------
 .../bindings/power/reset/gpio-restart.yaml    | 92 +++++++++++++++++++
 2 files changed, 92 insertions(+), 54 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/power/reset/gpio-restart.txt
 create mode 100644 Documentation/devicetree/bindings/power/reset/gpio-restart.yaml

diff --git a/Documentation/devicetree/bindings/power/reset/gpio-restart.txt b/Documentation/devicetree/bindings/power/reset/gpio-restart.txt
deleted file mode 100644
index af3701bc15c4..000000000000
--- a/Documentation/devicetree/bindings/power/reset/gpio-restart.txt
+++ /dev/null
@@ -1,54 +0,0 @@
-Drive a GPIO line that can be used to restart the system from a restart
-handler.
-
-This binding supports level and edge triggered reset.  At driver load
-time, the driver will request the given gpio line and install a restart
-handler. If the optional properties 'open-source' is not found, the GPIO line
-will be driven in the inactive state.  Otherwise its not driven until
-the restart is initiated.
-
-When the system is restarted, the restart handler will be invoked in
-priority order.  The gpio is configured as an output, and driven active,
-triggering a level triggered reset condition. This will also cause an
-inactive->active edge condition, triggering positive edge triggered
-reset. After a delay specified by active-delay, the GPIO is set to
-inactive, thus causing an active->inactive edge, triggering negative edge
-triggered reset. After a delay specified by inactive-delay, the GPIO
-is driven active again.  After a delay specified by wait-delay, the
-restart handler completes allowing other restart handlers to be attempted.
-
-Required properties:
-- compatible : should be "gpio-restart".
-- gpios : The GPIO to set high/low, see "gpios property" in
-  Documentation/devicetree/bindings/gpio/gpio.txt. If the pin should be
-  low to reset the board set it to "Active Low", otherwise set
-  gpio to "Active High".
-
-Optional properties:
-- open-source : Treat the GPIO as being open source and defer driving
-  it to when the restart is initiated.  If this optional property is not
-  specified, the GPIO is initialized as an output in its inactive state.
-- priority : A priority ranging from 0 to 255 (default 128) according to
-  the following guidelines:
-	0:	Restart handler of last resort, with limited restart
-		capabilities
-	128:	Default restart handler; use if no other restart handler is
-		expected to be available, and/or if restart functionality is
-		sufficient to restart the entire system
-	255:	Highest priority restart handler, will preempt all other
-		restart handlers
-- active-delay: Delay (default 100) to wait after driving gpio active [ms]
-- inactive-delay: Delay (default 100) to wait after driving gpio inactive [ms]
-- wait-delay: Delay (default 3000) to wait after completing restart
-  sequence [ms]
-
-Examples:
-
-gpio-restart {
-	compatible = "gpio-restart";
-	gpios = <&gpio 4 0>;
-	priority = <128>;
-	active-delay = <100>;
-	inactive-delay = <100>;
-	wait-delay = <3000>;
-};
diff --git a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
new file mode 100644
index 000000000000..6a1f4aeebf49
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/gpio-restart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO-driven system restart
+
+maintainers:
+  - TODO
+
+description:
+  Drive a GPIO line that can be used to restart the system from a restart
+  handler.
+
+  This binding supports level and edge triggered reset. At driver load time,
+  the driver will request the given gpio line and install a restart handler. If
+  the optional properties 'open-source' is not found, the GPIO line will be
+  driven in the inactive state. Otherwise its not driven until the restart is
+  initiated.
+
+  When the system is restarted, the restart handler will be invoked in priority
+  order. The gpio is configured as an output, and driven active, triggering a
+  level triggered reset condition. This will also cause an inactive->active
+  edge condition, triggering positive edge triggered reset. After a delay
+  specified by active-delay, the GPIO is set to inactive, thus causing an
+  active->inactive edge, triggering negative edge triggered reset. After a
+  delay specified by inactive-delay, the GPIO is driven active again. After a
+  delay specified by wait-delay, the restart handler completes allowing other
+  restart handlers to be attempted.
+
+properties:
+  compatible:
+    const: gpio-restart
+
+  gpios:
+    maxItems: 1
+    description:
+      The GPIO to set high/low, see "gpios property" in
+      Documentation/devicetree/bindings/gpio/gpio.txt. If the pin should be low
+      to reset the board set it to "Active Low", otherwise set gpio to "Active
+      High".
+
+  open-source:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Treat the GPIO as being open source and defer driving it to when the
+      restart is initiated. If this optional property is not specified, the
+      GPIO is initialized as an output in its inactive state.
+
+  priority:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      A priority ranging from 0 to 255 (default 128) according to the following
+      guidelines:
+      0:    Restart handler of last resort, with limited restart
+            capabilities
+      128:  Default restart handler; use if no other restart handler is
+            expected to be available, and/or if restart functionality is
+            sufficient to restart the entire system
+      255:  Highest priority restart handler, will preempt all other
+            restart handlers
+
+  active-delay:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Delay (default 100) to wait after driving gpio active [ms]
+
+  inactive-delay:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Delay (default 100) to wait after driving gpio inactive [ms]
+
+  wait-delay:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Delay (default 3000) to wait after completing restart sequence [ms]
+
+required:
+  - compatible
+  - gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    gpio-restart {
+      compatible = "gpio-restart";
+      gpios = <&gpio 4 0>;
+      priority = <128>;
+      active-delay = <100>;
+      inactive-delay = <100>;
+      wait-delay = <3000>;
+    };
-- 
2.33.1

[RFC PATCH v1 2/3] dt-bindings: power: reset: gpio-restart: Add -ms suffix to delays

[Sander Vanheule]

The delay properties are expressed in milliseconds, so the property
names should have a -ms suffix. Add the suffix, and deprecate the
original properties.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 .../bindings/power/reset/gpio-restart.yaml    | 27 ++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
index 6a1f4aeebf49..13827fe7b395 100644
--- a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
+++ b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
@@ -62,17 +62,26 @@ properties:
             restart handlers
 
   active-delay:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: Delay (default 100) to wait after driving gpio active [ms]
+    $ref: '#/properties/active-delay-ms'
+    deprecated: true
 
   inactive-delay:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: Delay (default 100) to wait after driving gpio inactive [ms]
+    $ref: '#/properties/inactive-delay-ms'
+    deprecated: true
 
   wait-delay:
-    $ref: /schemas/types.yaml#/definitions/uint32
+    $ref: '#/properties/wait-delay-ms'
+    deprecated: true
+
+  active-delay-ms:
+    description: Delay (default 100 ms) to wait after driving gpio active
+
+  inactive-delay-ms:
+    description: Delay (default 100 ms) to wait after driving gpio inactive
+
+  wait-delay-ms:
     description:
-      Delay (default 3000) to wait after completing restart sequence [ms]
+      Delay (default 3000 ms) to wait after completing restart sequence
 
 required:
   - compatible
@@ -86,7 +95,7 @@ examples:
       compatible = "gpio-restart";
       gpios = <&gpio 4 0>;
       priority = <128>;
-      active-delay = <100>;
-      inactive-delay = <100>;
-      wait-delay = <3000>;
+      active-delay-ms = <100>;
+      inactive-delay-ms = <100>;
+      wait-delay-ms = <3000>;
     };
-- 
2.33.1

[RFC PATCH v1 3/3] dt-bindings: power: reset: gpio-restart: Correct default priority

[Sander Vanheule]

Commit bcd56fe1aa97 ("power: reset: gpio-restart: increase priority
slightly") changed the default restart priority 129, but did not update
the documentation. Correct this, so the driver and documentation have
the same default value.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 Documentation/devicetree/bindings/power/reset/gpio-restart.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
index 13827fe7b395..ab26af93cb39 100644
--- a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
+++ b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
@@ -51,7 +51,7 @@ properties:
   priority:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: |
-      A priority ranging from 0 to 255 (default 128) according to the following
+      A priority ranging from 0 to 255 (default 129) according to the following
       guidelines:
       0:    Restart handler of last resort, with limited restart
             capabilities
-- 
2.33.1

Re: [RFC PATCH v1 1/3] dt-bindings: power: reset: Convert gpio-restart binding to schema

[Sander Vanheule]

On Thu, 2021-12-30 at 19:06 +0100, Sander Vanheule wrote:
> Convert the gpio-restart binding from plain text format to a schema
> binding, maintaining the original content and updating formatting where
> required.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>

I had these patches set aside since a few weeks already, but apparently this conversion
was merged last week. This patch can be disregarded, sorry for the noise.

Best,
Sander

Re: [RFC PATCH v1 1/3] dt-bindings: power: reset: Convert gpio-restart binding to schema

[Rob Herring]

On Thu, 30 Dec 2021 19:06:01 +0100, Sander Vanheule wrote:
> Convert the gpio-restart binding from plain text format to a schema
> binding, maintaining the original content and updating formatting where
> required.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---
>  .../bindings/power/reset/gpio-restart.txt     | 54 -----------
>  .../bindings/power/reset/gpio-restart.yaml    | 92 +++++++++++++++++++
>  2 files changed, 92 insertions(+), 54 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/power/reset/gpio-restart.txt
>  create mode 100644 Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml: maintainers:0: 'TODO' is not a 'email'
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml: ignoring, error in schema: maintainers: 0
Documentation/devicetree/bindings/power/reset/gpio-restart.example.dt.yaml:0:0: /example-0/gpio-restart: failed to match any schema with compatible: ['gpio-restart']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1574223

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [RFC PATCH v1 2/3] dt-bindings: power: reset: gpio-restart: Add -ms suffix to delays

[Rob Herring]

On Thu, Dec 30, 2021 at 07:06:02PM +0100, Sander Vanheule wrote:
> The delay properties are expressed in milliseconds, so the property
> names should have a -ms suffix. Add the suffix, and deprecate the
> original properties.
> 
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---
>  .../bindings/power/reset/gpio-restart.yaml    | 27 ++++++++++++-------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> index 6a1f4aeebf49..13827fe7b395 100644
> --- a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> @@ -62,17 +62,26 @@ properties:
>              restart handlers
>  
>    active-delay:
> -    $ref: /schemas/types.yaml#/definitions/uint32
> -    description: Delay (default 100) to wait after driving gpio active [ms]
> +    $ref: '#/properties/active-delay-ms'

While 'active-delay-ms' has a type because '.*-ms' has a type, you just 
removed the type for this property. Now 'active-delay = "foo"' is valid.

While it would be nice to change this, we're pretty much stuck with it 
forever. I don't think supporting both versions in the kernel is worth 
it.

Rob

Re: [RFC PATCH v1 2/3] dt-bindings: power: reset: gpio-restart: Add -ms suffix to delays

[Sander Vanheule]

Hi Rob,

On Mon, 2022-01-10 at 14:38 -0600, Rob Herring wrote:
> On Thu, Dec 30, 2021 at 07:06:02PM +0100, Sander Vanheule wrote:
> > The delay properties are expressed in milliseconds, so the property
> > names should have a -ms suffix. Add the suffix, and deprecate the
> > original properties.
> > 
> > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> > ---
> >  .../bindings/power/reset/gpio-restart.yaml    | 27 ++++++++++++-------
> >  1 file changed, 18 insertions(+), 9 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> > b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> > index 6a1f4aeebf49..13827fe7b395 100644
> > --- a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> > +++ b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml
> > @@ -62,17 +62,26 @@ properties:
> >              restart handlers
> >  
> >    active-delay:
> > -    $ref: /schemas/types.yaml#/definitions/uint32
> > -    description: Delay (default 100) to wait after driving gpio active [ms]
> > +    $ref: '#/properties/active-delay-ms'
> 
> While 'active-delay-ms' has a type because '.*-ms' has a type, you just 
> removed the type for this property. Now 'active-delay = "foo"' is valid.

Good to know, I would've expected the type to be inherited.

> While it would be nice to change this, we're pretty much stuck with it 
> forever. I don't think supporting both versions in the kernel is worth 
> it.

Figured as much. I'll just keep patch 3/3 then.

Thanks for the feedback!

Best,
Sander