diff mbox series

[RFC,4/4] dt-bindings: gpio: Add bindings for SCMI pinctrl based gpio

Message ID 20231002021602.260100-5-takahiro.akashi@linaro.org
State New
Headers show
Series gpio: add SCMI pinctrl based driver | expand

Commit Message

AKASHI Takahiro Oct. 2, 2023, 2:16 a.m. UTC
A dt binding for SCMI pinctrl based gpio driver is defined in this
commit. It basically conforms to generic pinctrl-gpio mapping framework.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 .../bindings/gpio/arm,scmi-gpio.yaml          | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/arm,scmi-gpio.yaml

Comments

Cristian Marussi Oct. 2, 2023, 2:58 p.m. UTC | #1
On Mon, Oct 02, 2023 at 09:41:55AM -0500, Rob Herring wrote:
> On Mon, Oct 02, 2023 at 11:16:02AM +0900, AKASHI Takahiro wrote:
> > A dt binding for SCMI pinctrl based gpio driver is defined in this
> > commit. It basically conforms to generic pinctrl-gpio mapping framework.

[ snip]

> > +    additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - gpio-controller
> > +  - "#gpio-cells"
> > +  - gpio-ranges
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    scmi_gpio_0: scmi_gpio@0 {
> 
> gpio {
> 
> But doesn't SCMI have protocol numbers?
> 

My understanding is that this RFC GPIO driver from Akashi is built
completely on Pinctrl facilities (as he says in the cover), it is not
indeed a typical pure SCMI driver, it just happen to trigger the use
of SCMI if the underlying backend pinctrl driver is pinctrl-scmi;
but this driver does not really call directly into any SCMI API by
itself, i.e. it does not get and call any SCMI protocol ops.
(but it could indeed trigger the backend Pinctrl SCMI driver to issue
 such call on its behalf AFAIU...)

I wonder why it has even a dependency on PINCTRL_SCMI at this point;
is not that it could work (generically) even if the backend Pinctrl
driver is NOT SCMI ?
What makes it usable only against an SCMI Pinctrl backend ?
Cannot be a generic GPIO driver based on top of Pinctrl, no matter which
Pinctrl backend driver has been configured ?

...I maybe missing something here about Pinctrl AND GPIO frameworks :P

Thanks,
Cristian
AKASHI Takahiro Oct. 4, 2023, 7:08 a.m. UTC | #2
On Tue, Oct 03, 2023 at 03:16:49PM +0200, Linus Walleij wrote:
> Hi Takahiro,
> 
> first, thanks for working on this important and crucial driver!
> 
> I'll try to clarify and also explain something of what the others
> are saying (unless I misunderstand them...)

Ah, thank you.

> On Mon, Oct 2, 2023 at 4:17???AM AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> 
> > A dt binding for SCMI pinctrl based gpio driver is defined in this
> > commit. It basically conforms to generic pinctrl-gpio mapping framework.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> I think like Christian says that SCMI maybe has nothing to do
> with this binding? It is just one possible use case (though we don't know
> of any others.) The resource it is using is generic functionality that exist
> in any pin controller that provides ways to drive lines high and low
> etc.
> 
> Would it be named a generic pin control-based GPIO?

If you like :)
As I said, I was not confident that the driver be applicable
to other pinctrl-gpio cases.

> (...)
> > +++ b/Documentation/devicetree/bindings/gpio/arm,scmi-gpio.yaml
> (...)
> > +$id: http://devicetree.org/schemas/gpio/arm,scmi-gpio.yaml#
> 
> So no ARM, no scmi, just pin-control-gpio.yaml, be bold!

I'm not so ambitious.

> (I like this long unabbreviated name)
> 
> > +title: SCMI pinctrl based generic GPIO controller
> 
> Pin control-based generic GPIO controller
> 
> Add
> 
> description:
>   The pin control-based GPIO will facilitate a pin controllers ability
>   to drive electric lines high/low and other generic properties of a
>   pin controller to perform general-purpose one-bit binary I/O.
> 
> (At least I think this is the idea, I hope I understand correctly.)

Okay.

> > +properties:
> > +  $nodename:
> > +    pattern: "^scmi_gpio(@[0-9a-f]+)$"
> 
> These nodes are always just named gpio@...
> the resource marker is "this is a GPIO" that's all it means.

By following other gpio drivers' bindings, I will drop this rule.

> > +  compatible:
> > +    const: arm,scmi-gpio-generic
> 
> const: pin-control-gpio
> 
> Other than that I am aboard with the solution!

Hope that the driver works on real hardware :)

-Takahiro Akashi

> Yours,
> Linus Walleij
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpio/arm,scmi-gpio.yaml b/Documentation/devicetree/bindings/gpio/arm,scmi-gpio.yaml
new file mode 100644
index 000000000000..2601c5594567
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/arm,scmi-gpio.yaml
@@ -0,0 +1,71 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/arm,scmi-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SCMI pinctrl based generic GPIO controller
+
+maintainers:
+  - AKASHI Takahiro <akashi.takahiro@linaro.org>
+
+properties:
+  $nodename:
+    pattern: "^scmi_gpio(@[0-9a-f]+)$"
+
+  compatible:
+    const: arm,scmi-gpio-generic
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-ranges: true
+
+  gpio-ranges-group-names: true
+
+patternProperties:
+  "^.+-hog(-[0-9]+)?$":
+    type: object
+    properties:
+      gpio-hog: true
+      gpios: true
+      input: true
+      output-high: true
+      output-low: true
+      line-name: true
+
+    required:
+      - gpio-hog
+      - gpios
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - gpio-controller
+  - "#gpio-cells"
+  - gpio-ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    scmi_gpio_0: scmi_gpio@0 {
+        compatible = "arm,scmi-gpio";
+        gpio-controller;
+        #gpio-cells = <2>;
+        gpio-ranges = <&scmi_pinctrl 0 10 5>,
+                      <&scmi_pinctrl 5 0 0>;
+        gpio-ranges-group-names = "",
+                                  "pinmux_gpio";
+    };
+
+    // Consumer:
+    sdhci0_pwrseq {
+        compatible = "mmc-pwrseq-emmc";
+        reset-gpios = <&scmi_gpio_0 0 GPIO_ACTIVE_LOW>;
+    };