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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux