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

From: Sean Anderson
Date: Thu Oct 28 2021 - 11:19:43 EST


Hi Rob,

On 10/26/21 10:27 PM, Rob Herring wrote:
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.

That would be nice, but it seems like someone would have to go through
every driver with a reset-gpios property and convert them. Since the
reset GPIOs are


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+

GPL-2.0+ is a strict superset. And bindings are required to be BSD
anyway. I don't see the issue.

+%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.

Yes [1]. This device has a "reset done" signal, but no reset timings
specified in the datasheet. I don't know if this is truly needed,
because we can read the ID register, but it is nice when bringing up the
device. I left it in because I was using it.

[1] https://lore.kernel.org/netdev/20211004191527.1610759-16-sean.anderson@xxxxxxxx/

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

Well, there are already things like reset-assert-us and
reset-deassert-us in [2, 3, 4] (with different names(!)). Part of what I
want to address with this device is that there are several existing
properties which specify various aspects of the above timings. I think
it would be good to standardize on these. Maybe this should be a
property which applies to the reset consumer? Analogously, we also
have assigned-clocks so that not every driver has to know what the
correct frequency/parent is (especially when they can vary among
different hardware variations).

--Sean

[2] Documentation/devicetree/bindings/net/ethernet-phy.yaml
[3] Documentation/devicetree/bindings/net/mdio.yaml
[4] Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml

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