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
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(-)
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
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(-)
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
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
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
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 <[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
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 <[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.
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 <[email protected]>
> ---
> .../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
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 <[email protected]>
> > ---
> > .../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