[RFC,1/2] dt-bindings: gpio: Document shared GPIO line usage

Message ID 20191120133409.9217-2-peter.ujfalusi@ti.com
State New
Headers show
Series
  • [RFC,1/2] dt-bindings: gpio: Document shared GPIO line usage
Related show

Commit Message

Peter Ujfalusi Nov. 20, 2019, 1:34 p.m.
Boards might use the same GPIO line to control several external devices.
Add section to document on how a shared GPIO pin can be described.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

---
 .../devicetree/bindings/gpio/gpio.txt         | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)

-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Comments

Linus Walleij Nov. 28, 2019, 10:06 a.m. | #1
On Fri, Nov 22, 2019 at 2:36 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 22/11/2019 14.10, Linus Walleij wrote:

> > On Wed, Nov 20, 2019 at 2:34 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

> >

> >> Boards might use the same GPIO line to control several external devices.

> >> Add section to document on how a shared GPIO pin can be described.

> >>

> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

> >

> > As I've stated earlier I think this information is surplus.

> > If two devices have a phandle to the same GPIO line

> > then it is by definition shared.

>

> Well, phandle + line number to be precise.


This is what I mean when I say "phandle to the same GPIO line".
Like this:

foo-gpios = <&gpio0 5 GPIO_ACTIVE_LOW>;

If the phandle <&gpio0 5 *>; appear in some other
(non-disabled) node it has > 1 users.

> >> +               line_a {

> >> +                       gpio-shared;

> >

> > So this is unnecessary: if the same line is referenced

> > by phandle from two places it is shared, simple as that.

>

> phandle is pointing to the gpio controller, not to the line.


Cleared up above.

> >> +                       gpios = <5 0>;

> >> +                       output-low;

> >

> > This is overlapping with the use case to define initial

> > state values for GPIOs, something that has been

> > brought up repeatedly and I've collected links for

> > previous discussions several times.

>

> I don't mind this to go away and the first set would configure the level.

> Kept it here so I can reuse the gpio-hog code from gpiolib-of ;)


People have tried to reuse the hog code to set up
initial line levels as well, it failed because they could
not get the DT bindings through the door.

> > I guess if need be I have to look them up again.

> >

> > The DT maintainers don't like the hog syntax so

> > something else is desired for this.

>

> I see, so the gpio-hog might change?


They will not change since they are ABI, but their
use case will not be extended AFAICT.
Not my pick, I liked the hog syntax but we need
consensus.

> > (snip)

> >> +The shared GPIO line management strategy can be selected with either of the

> >> +following properties:

> >> +- refcounted-low: The line must be kept low as long as there is at least one

> >> +               request asking it to be low.

> >> +- refcounted-high: The line must be kept high as long as there is at least one

> >> +               request asking it to be high.

> >

> > Is this really needed? Isn't it more appropriate to just define the

> > semantics such that as soon as some consumer requests the line

> > high it will be refcounted high, and as soon as it is requested

> > low by any consumer it will be refcounted low.

>

> Well. How do we decide which level is the one that should be preserved?


First come first serve.

If there is any conflict amongst the consumers we are
screwed anyway so why try to establish where they should
agree if they don't agree?

> How would the core decide what to in a simplest case:

> two device, they are the same part.

> ENABLE pin which needs to be high to enable the device.

> When the driver probes it asks for initial deasserted GPIO as the device

> is not in active use.


This makes me think it should be a unique driver
with a unique compatible string, as it embodies
use cases.

It is too broad to just define
refcounted-high or refcounted-low, that is hiding the
real use case, so I would go for something like a
resource in the device tree that all other devices that
need it can take.

Like a reset controller, precisely:

reset: reset-controller {
    compatible = "reset-gpio";
    gpios = <&gpio0 5 GPIO_ACTIVE_LOW>;
    #reset-cells = <0>;
};

dev0 {
    resets = <&reset>;
};

dev1 {
    resets = <&reset>;
};

The ambition to use refcounted GPIOs to solve this
usecase is probably wrong, I would say try to go for a
GPIO-based reset controller instead.

The fact that some Linux drivers are already using explicit
GPIO's for their reset handling is maybe unfortunate,
they will simply have to grow code to deal with a reset
alternatively to GPIO, like first try to grab a reset
handle and if that doesn't fall back to use a GPIO.

I would say don't try to shoehorn this use case into the
gpio library but instead try to create a reset controller that
takes care of arbitrating the use of a single GPIO line.

Yours,
Linus Walleij
Linus Walleij Dec. 10, 2019, 11:54 p.m. | #2
On Wed, Dec 4, 2019 at 12:51 AM Rob Herring <robh@kernel.org> wrote:
> On Thu, Nov 28, 2019 at 11:06:35AM +0100, Linus Walleij wrote:


> > The ambition to use refcounted GPIOs to solve this

> > usecase is probably wrong, I would say try to go for a

> > GPIO-based reset controller instead.

>

> Yes, but I think we can have that AND use the existing binding.

>

> > The fact that some Linux drivers are already using explicit

> > GPIO's for their reset handling is maybe unfortunate,

> > they will simply have to grow code to deal with a reset

> > alternatively to GPIO, like first try to grab a reset

> > handle and if that doesn't fall back to use a GPIO.

>

> I think this could just be all handled within the reset subsystem given

> that we've been consistent in using 'reset-gpios' (GPIO regulators are

> similar, but we never had such consistency with GPIO names for

> regulators). We can implement a reset driver for the 'reset-gpios'

> property that deals with the sharing. Drivers to DT nodes doesn't have

> to be 1:1. It's convenient when they are, but that's encoding the OS's

> (current) driver structure into DT.


This seems like a good approach if it can be made to work.
reset-gpios should have the right polarity flags (else drivers
and/or device trees need to be fixed...) so the driver can simply
scan over them and try to build a consensus on how to assert
or deassert a shared reset-gpios line.

It is also a natural placeholder for the connection to device
core that will inevitably need to happen the day the device
hierarchy needs to be torn up/down for a reset on some
random device.

Peter, will you have a go at it?

Yours,
Linus Walleij

Patch

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index a8895d339bfe..644e6513607c 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -228,6 +228,72 @@  Example of two SOC GPIO banks defined as gpio-controller nodes:
 		#gpio-cells = <2>;
 	};
 
+On boards one GPIO line might be connected to multiple devices as reset, enable
+or other control pins. In order to make it safer for this usage of a GPIO line
+one can describe the shared GPIO pin.
+
+Each shared GPIO pin definition is represented as a child node of the GPIO
+controller.
+
+Required properties:
+- gpio-shared:	A property specifying that this child node represents a shared
+		GPIO pin.
+- gpios:	Store the GPIO information (id, flags, ...) for each GPIO to
+		affect. Shall contain an integer multiple of the number of cells
+		specified in its parent node (GPIO controller node).
+Only one of the following properties scanned in the order shown below.
+This means that when multiple properties are present they will be searched
+in the order presented below and the first match is taken as the intended
+configuration.
+- output-low:	A property specifying to set the GPIO direction as output with
+		the value low initially.
+- output-high:	A property specifying to set the GPIO direction as output with
+		the value high initially.
+The shared GPIO line management strategy can be selected with either of the
+following properties:
+- refcounted-low: The line must be kept low as long as there is at least one
+		request asking it to be low.
+- refcounted-high: The line must be kept high as long as there is at least one
+		request asking it to be high.
+If neither of the refcounting strategy was selected then the shared GPIO is
+handled as pass through. In this mode all user requests will be forwarded to the
+shared GPIO pin without refcounting.
+
+Optional properties:
+- line-name:  The GPIO label name. If not present the node name is used.
+
+Example of shared GPIO use:
+
+	qe_pio_a: gpio-controller@1400 {
+		compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank";
+		reg = <0x1400 0x18>;
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		line_a {
+			gpio-shared;
+			gpios = <5 0>;
+			output-low;
+			refcounted-high;
+			line-name = "enable-for-devices";
+		};
+	};
+
+	device@0 {
+		compatible = "some,device";
+		enable-gpios = <&qe_pio_a 5 0>;
+	};
+
+	device@1 {
+		compatible = "some,device";
+		enable-gpios = <&qe_pio_a 5 0>;
+	};
+
+	component@0 {
+		compatible = "some,component";
+		select-gpios = <&qe_pio_a 5 0>;
+	};
+
 2.1) gpio- and pin-controller interaction
 -----------------------------------------