convert gpio-restart bindings to yaml

RFC PATCH In review Sander Vanheule 2 participants 8 messages Dec 30, 2021 – Jan 10, 2022

Subsystems (2)

Device tree bindings System reset/shutdown drivers

lore

Share & tools

lore

Sander Vanheule

author

2021-12-30 18:06 UTC #0 lore raw

cover convert gpio-restart bindings to yaml

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

Signature · 1 line

2.33.1

Sander Vanheule

author

2021-12-30 18:06 UTC #1 lore raw

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

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 <[email protected]>

---
 .../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

Documentation/devicetree/bindings/power/reset/gpio-restart.txt deleted +0 ~~-54~~

deleted file mode 100644
index af3701bc15c4..000000000000
@@ -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>;
-};

Documentation/devicetree/bindings/power/reset/gpio-restart.yaml new file +92 -0

92 lines not on this page · raw message

Signature · 1 line

2.33.1

Sander Vanheule

author

2021-12-30 18:11 UTC #2 lore raw

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

Quoted 5 lines · 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 <[email protected]>

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

Rob Herring

maintainer

2022-01-01 22:01 UTC #3 lore raw

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

In reply to Sander Vanheule

Quoted 4 lines · Sander Vanheule

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

Quoted 8 lines

Signed-off-by Sander Vanheule <[email protected]>

---
 .../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.

Sander Vanheule

author

2021-12-30 18:06 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 .../bindings/power/reset/gpio-restart.yaml    | 27 ++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)

Documentation/devicetree/bindings/power/reset/gpio-restart.yaml +18 -9

index 6a1f4aeebf49..13827fe7b395 100644
@@ -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>;
     };

Signature · 1 line

2.33.1

Rob Herring

maintainer

2022-01-10 20:38 UTC #5 lore raw

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

Quoted 14 lines · 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 <[email protected]>
---
 .../bindings/power/reset/gpio-restart.yaml    | 27 ++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)
Documentation/devicetree/bindings/power/reset/gpio-restart.yaml +0 -0
index 6a1f4aeebf49..13827fe7b395 100644

Documentation/devicetree/bindings/power/reset/gpio-restart.yaml properties: · line 65

Quoted 6 lines

Documentation/devicetree/bindings/power/reset/gpio-restart.yaml +1 -2

@@ -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

Sander Vanheule

author

2022-01-10 20:55 UTC #6 lore raw

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

Hi Rob,

Quoted 10 lines · Rob Herring

Quoted 9 lines · 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 <[email protected]>
---
.../bindings/power/reset/gpio-restart.yaml | 27 ++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)

Quoted 15 lines

Quoted 6 lines
Documentation/devicetree/bindings/power/reset/gpio-restart.yaml +0 -0
index 6a1f4aeebf49..13827fe7b395 100644
@@ -62,17 +62,26 @@ properties:
Quoted 6 lines
             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.

Quoted 3 lines

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

Sander Vanheule

author

2021-12-30 18:06 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 Documentation/devicetree/bindings/power/reset/gpio-restart.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Documentation/devicetree/bindings/power/reset/gpio-restart.yaml +1 -1

index 13827fe7b395..ab26af93cb39 100644
@@ -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

Signature · 1 line

2.33.1