Re: [PATCH] dt-bindings: power: reset: gpio-poweroff: Convert txt bindings to yaml

[Sebastian Reichel]

Hi,

On Sat, Oct 09, 2021 at 06:32:26PM +0200, David Heidelberg wrote:
> Convert power-off action connected to the GPIO documentation to the YAML syntax.
> 
> Signed-off-by: David Heidelberg <david@xxxxxxx>
> ---
>  .../bindings/power/reset/gpio-poweroff.txt    | 41 ------------
>  .../bindings/power/reset/gpio-poweroff.yaml   | 64 +++++++++++++++++++
>  2 files changed, 64 insertions(+), 41 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt
>  create mode 100644 Documentation/devicetree/bindings/power/reset/gpio-poweroff.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt b/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt
> deleted file mode 100644
> index 3e56c1b34a4c..000000000000
> --- a/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -Driver a GPIO line that can be used to turn the power off.
> -
> -The driver supports both level triggered and edge triggered power off.
> -At driver load time, the driver will request the given gpio line and
> -install a handler to power off the system. If the optional properties
> -'input' is not found, the GPIO line will be driven in the inactive
> -state. Otherwise its configured as an input.
> -
> -When the power-off handler is called, the gpio is configured as an
> -output, and drive active, so triggering a level triggered power off
> -condition. This will also cause an inactive->active edge condition, so
> -triggering positive edge triggered power off. After a delay of 100ms,
> -the GPIO is set to inactive, thus causing an active->inactive edge,
> -triggering negative edge triggered power off. After another 100ms
> -delay the GPIO is driver active again. If the power is still on and
> -the CPU still running after a 3000ms delay, a WARN_ON(1) is emitted.
> -
> -Required properties:
> -- compatible : should be "gpio-poweroff".
> -- gpios : The GPIO to set high/low, see "gpios property" in
> -  Documentation/devicetree/bindings/gpio/gpio.txt. If the pin should be
> -  low to power down the board set it to "Active Low", otherwise set
> -  gpio to "Active High".
> -
> -Optional properties:
> -- input : Initially configure the GPIO line as an input. Only reconfigure
> -  it to an output when the power-off handler is called. If this optional
> -  property is not specified, the GPIO is initialized as an output in its
> -  inactive state.
> -- active-delay-ms: Delay (default 100) to wait after driving gpio active
> -- inactive-delay-ms: Delay (default 100) to wait after driving gpio inactive
> -- timeout-ms: Time to wait before asserting a WARN_ON(1). If nothing is
> -              specified, 3000 ms is used.
> -
> -Examples:
> -
> -gpio-poweroff {
> -	compatible = "gpio-poweroff";
> -	gpios = <&gpio 4 0>;
> -	timeout-ms = <3000>;
> -};
> diff --git a/Documentation/devicetree/bindings/power/reset/gpio-poweroff.yaml b/Documentation/devicetree/bindings/power/reset/gpio-poweroff.yaml
> new file mode 100644
> index 000000000000..50ae0cec6493
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/gpio-poweroff.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/gpio-poweroff.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Driver a GPIO line that can be used to turn the power off

This is a DT binding, not kernel driver documentation. Title should
be something like

title: GPIO controlled power off

> +
> +maintainers:
> +  - Sebastian Reichel <sre@xxxxxxxxxx>
> +
> +description:
> +  The driver supports both level triggered and edge triggered power off.
> +  At driver load time, the driver will request the given gpio line and
> +  install a handler to power off the system. If the optional properties
> +  'input' is not found, the GPIO line will be driven in the inactive
> +  state. Otherwise its configured as an input.
> +
> +  When the power-off handler is called, the gpio is configured as an
> +  output, and drive active, so triggering a level triggered power off
> +  condition. This will also cause an inactive->active edge condition, so
> +  triggering positive edge triggered power off. After a delay of 100ms,
> +  the GPIO is set to inactive, thus causing an active->inactive edge,
> +  triggering negative edge triggered power off. After another 100ms
> +  delay the GPIO is driver active again. If the power is still on and
> +  the CPU still running after a 3000ms delay, a WARN_ON(1) is emitted.

This description is quite Linux specific. I think it should be
easily possible to fix that:

System power off support via a GPIO line. When a shutdown is
executed the operating system is expected to switch the GPIO
from inactive to active. After a delay (active-delay-ms) it
is expected to be switched back to inactive. After another
delay (inactive-delay-ms) it is configured as active again.
Finally the operating system assumes the power off failed if
the system is still running after waiting some time (timeout-ms).

> +properties:
> +  compatible:
> +    const: gpio-poweroff
> +
> +  gpios: true

maxItems: 1

> +
> +  input:
> +    description: |
> +      Initially configure the GPIO line as an input. Only reconfigure
> +      it to an output when the power-off handler is called. If this optional

type: boolean

s/handler is called/power off sequence is initiated/

> +      property is not specified, the GPIO is initialized as an output in its inactive state.
> +
> +  active-delay-ms:
> +    default: 100
> +    description: Delay to wait after driving gpio active
> +
> +  inactive-delay-ms:
> +    default: 100
> +    description: Delay to wait after driving gpio inactive
> +
> +  timeout-ms:
> +    default: 3000
> +    description: Time to wait before asserting a WARN_ON(1).

Time to wait before assuming the power off sequence failed.

> +required:
> +  - compatible
> +  - gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    gpio-poweroff {
> +        compatible = "gpio-poweroff";
> +        gpios = <&gpio 4 0>;
> +        timeout-ms = <3000>;
> +    };

Thanks,

-- Sebastian

Attachment: signature.asc
Description: PGP signature