Re: [PATCH 1/2] dt-bindings: reset: Add generic GPIO reset binding

[Rob Herring]

On Mon, Oct 18, 2021 at 07:49:21PM -0400, Sean Anderson wrote:
> This adds a binding for a generic GPIO reset driver. This driver is
> designed to easily add a GPIO-based reset to a driver which expected a
> reset controller. It offers greater flexibility than a reset-gpios
> property, and allows for one code path to be shared for GPIO resets and
> MMIO-based resets.

I would like to do this last part, but not requiring a binding change. 
IOW, be able to register any 'reset-gpios' property as a reset provider 
directly without this added level of indirection.

> 
> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx>
> ---
> 
>  .../devicetree/bindings/reset/gpio-reset.yaml | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/gpio-reset.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/gpio-reset.yaml b/Documentation/devicetree/bindings/reset/gpio-reset.yaml
> new file mode 100644
> index 000000000000..de2ab074cea3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/gpio-reset.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)

GPL-2.0-only not GPL-2.0+

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/gpio-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic GPIO reset driver
> +
> +maintainers:
> +  - Sean Anderson <seanga2@xxxxxxxxx>
> +
> +description: |
> +  This is a generic GPIO reset driver which can provide a reset-controller
> +  interface for GPIO-based reset lines. This driver always operates with
> +  logical GPIO values; to invert the polarity, specify GPIO_ACTIVE_LOW in the
> +  GPIO's flags.
> +
> +properties:
> +  compatible:
> +    const: gpio-reset
> +
> +  '#reset-cells':
> +    const: 1
> +
> +  reset-gpios:
> +    description: |
> +      GPIOs to assert when asserting a reset. There is a one-to-one mapping
> +      between the reset specifier and the index of the GPIO in this list to
> +      assert.
> +
> +  done-gpios:
> +    description: |
> +      GPIOs which indicate that the device controlled by the GPIO has exited
> +      reset. There must be one done GPIO for each reset GPIO, or no done GPIOs
> +      at all. The driver will wait for up to done-timeout-us for the
> +      corresponding done GPIO to assert before returning.

This is odd. Do you have some examples of h/w needing this done signal? 
It certainly doesn't seem like something we have a generic need for.

> +
> +  pre-assert-us:
> +    default: 0
> +    description: |
> +      Microseconds to delay between when the reset was requested to be
> +      asserted, and asserting the reset GPIO
> +
> +  post-assert-us:
> +    default: 0
> +    description: |
> +      Microseconds to delay after asserting the reset GPIO and before returning
> +      to the caller.
> +
> +  pre-deassert-us:
> +    default: 0
> +    description: |
> +      Microseconds to delay between when the reset was requested to be
> +      deasserted, and asserting the reset GPIO
> +
> +  post-deassert-us:
> +    default: 0
> +    description: |
> +      Microseconds to delay after deasserting the reset GPIO and before
> +      returning to the caller. This delay is always present, even if the done
> +      GPIO goes high earlier.
> +
> +  done-timeout-us:
> +    default: 1000
> +    description:
> +      Microseconds to wait for the done GPIO to assert after deasserting the
> +      reset GPIO. If post-deassert-us is present, this property defaults to 10
> +      times that delay. The timeout starts after waiting for the post deassert
> +      delay.

There's a reason we don't have all these timing values in DT. The timing 
requirements are defined by each device (being reset) and implied by 
their compatible strings. If we wanted a macro language for power 
sequence timings of regulators, clocks, resets, enables, etc., then we 
would have designed such a thing already.

> +
> +required:
> +  - '#reset-cells'
> +  - compatible
> +  - reset-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    pcs_reset: reset-pcs {
> +        #reset-cells = <1>;
> +        compatible = "gpio-reset";
> +        reset-gpios = <&gpio 0 GPIO_ACTIVE_LOW>,
> +                      <&gpio 1 GPIO_ACTIVE_LOW>,
> +                      <&gpio 2 GPIO_ACTIVE_LOW>,
> +                      <&gpio 3 GPIO_ACTIVE_LOW>;
> +        done-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>,
> +                     <&gpio 5 GPIO_ACTIVE_HIGH>,
> +                     <&gpio 6 GPIO_ACTIVE_HIGH>,
> +                     <&gpio 7 GPIO_ACTIVE_HIGH>;
> +        post-deassert-us = <100>;
> +    };
> -- 
> 2.25.1
> 
>