[PATCH RFC] gpio: of: document gpio-init nodes

[PATCH RFC] gpio: of: document gpio-init nodes

[Uwe Kleine-König]

Sometimes it is desirable to define a "safe" configuration for a GPIO in
the device tree but let the operating system later still make use of
this pin.

This might for example be useful to initially configure a debug pin that
is usually unconnected as output to prevent floating until it is used
later.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

this picks up a discussion that pops up now and then with our customers.

Last time I discussed this topic with Linus Walleij my suggestion was to
merge this usecase with gpio-hogs, but he wasn't happy with it because
hogging implies that the pin is not free for other usage and he
suggested to use "gpio-init" instead.

Maybe it's arguable if this "initial configuration" belongs into the
device tree, but IMHO defining a "safe configuration" should have a
place and the requirements are identical. This isn't implied by the name
however, but I don't have a better idea for a different name.

Thinking further (which was also discussed last time) it would also be
nice to restrict usage. For example that a given pin that has
"output-low" as its safe setting might be configured later als high
output but not as input. Maybe:

	companion-reset {
		gpio-somethingwithsafe;
		gpios = <12 0>;
		output-low;
		fixed-direction;
	};

(Conceptually we would have a hog then when also adding "fixed-value".)

I'm not sure the early configuration should be implemented in Linux. I'd
target the bootloader for that instead, still having the blessing of a
binding document would be great.

I look forward to your comments and ideas.

Best regards
Uwe

 Documentation/devicetree/bindings/gpio/gpio.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index 802402f6cc5d..849d620cee4d 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -207,6 +207,11 @@ configuration.
 Optional properties:
 - line-name:  The GPIO label name. If not present the node name is used.
 
+Similar to hogging above GPIOs can be initialized to a certain configuration
+only which compared to hogs doesn't prevent the operating system to change the
+pin later. The syntax is similar to hog definitons, the difference is only that
+the identifying property is "gpio-init" instead of "gpio-hog".
+
 Example of two SOC GPIO banks defined as gpio-controller nodes:
 
 	qe_pio_a: gpio-controller@1400 {
@@ -221,6 +226,12 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
 			output-low;
 			line-name = "foo-bar-gpio";
 		};
+
+		companion-reset {
+			gpio-init;
+			gpios = <12 0>;
+			output-low;
+		};
 	};
 
 	qe_pio_e: gpio-controller@1460 {
-- 
2.11.0

Re: [PATCH RFC] gpio: of: document gpio-init nodes

[Linus Walleij]

On Fri, Sep 22, 2017 at 10:41 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> Sometimes it is desirable to define a "safe" configuration for a GPIO in
> the device tree but let the operating system later still make use of
> this pin.
>
> This might for example be useful to initially configure a debug pin that
> is usually unconnected as output to prevent floating until it is used
> later.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This makes perfect sense to me, and makes things simple, consistent
and easy to understand for DTS authors.

In my opinion.

But I would like to see some opinion from a DT maintainer, we need to
have some rough consensus on this.

Yours,
Linus Walleij

Re: [PATCH RFC] gpio: of: document gpio-init nodes

[Rob Herring]

On Fri, Sep 22, 2017 at 10:41:38PM +0200, Uwe Kleine-König wrote:
> Sometimes it is desirable to define a "safe" configuration for a GPIO in
> the device tree but let the operating system later still make use of
> this pin.
> 
> This might for example be useful to initially configure a debug pin that
> is usually unconnected as output to prevent floating until it is used
> later.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> this picks up a discussion that pops up now and then with our customers.
> 
> Last time I discussed this topic with Linus Walleij my suggestion was to
> merge this usecase with gpio-hogs, but he wasn't happy with it because
> hogging implies that the pin is not free for other usage and he
> suggested to use "gpio-init" instead.
> 
> Maybe it's arguable if this "initial configuration" belongs into the
> device tree, but IMHO defining a "safe configuration" should have a
> place and the requirements are identical. This isn't implied by the name
> however, but I don't have a better idea for a different name.

It can be argued that by the time the kernel boots, it is way to late to 
configure pins to a safe state. Of course, even secure world reads the 
DT these days (or are at least talking about doing so). Still any s/w 
handling this could be too slow to get to a safe state.

Maybe "optimal default" state would be more accurate. 

> 
> Thinking further (which was also discussed last time) it would also be
> nice to restrict usage. For example that a given pin that has
> "output-low" as its safe setting might be configured later als high
> output but not as input. Maybe:

I can't imagine that an output can't be an input. Regardless, what 
you're describing is constraints and that seems like a whole other 
problem than default/initial state.

Plus, for constraints I'd think we want this done at the pin level, not 
GPIO. And we kind of already have that with pin states.

> 	companion-reset {
> 		gpio-somethingwithsafe;
> 		gpios = <12 0>;

"gpios" is already a defined property with a type (phandle + args). dtc 
checks for this now though gpio-hogs is already one exception, and I 
don't want to add another. Maybe it could be generalized to be allowed 
when the parent is a gpio-controller, but really I'd like to avoid this 
pattern from spreading.

> 		output-low;
> 		fixed-direction;
> 	};
> 
> (Conceptually we would have a hog then when also adding "fixed-value".)
> 
> I'm not sure the early configuration should be implemented in Linux. I'd
> target the bootloader for that instead, still having the blessing of a
> binding document would be great.
> 
> I look forward to your comments and ideas.
> 
> Best regards
> Uwe
> 
>  Documentation/devicetree/bindings/gpio/gpio.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> index 802402f6cc5d..849d620cee4d 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -207,6 +207,11 @@ configuration.
>  Optional properties:
>  - line-name:  The GPIO label name. If not present the node name is used.
>  
> +Similar to hogging above GPIOs can be initialized to a certain configuration
> +only which compared to hogs doesn't prevent the operating system to change the
> +pin later. The syntax is similar to hog definitons, the difference is only that
> +the identifying property is "gpio-init" instead of "gpio-hog".
> +
>  Example of two SOC GPIO banks defined as gpio-controller nodes:
>  
>  	qe_pio_a: gpio-controller@1400 {
> @@ -221,6 +226,12 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
>  			output-low;
>  			line-name = "foo-bar-gpio";
>  		};
> +
> +		companion-reset {
> +			gpio-init;
> +			gpios = <12 0>;
> +			output-low;
> +		};
>  	};
>  
>  	qe_pio_e: gpio-controller@1460 {
> -- 
> 2.11.0
> 

Re: [PATCH RFC] gpio: of: document gpio-init nodes

[Uwe Kleine-König]

On Wed, Oct 04, 2017 at 03:53:06PM -0500, Rob Herring wrote:
> On Fri, Sep 22, 2017 at 10:41:38PM +0200, Uwe Kleine-König wrote:
> > Sometimes it is desirable to define a "safe" configuration for a GPIO in
> > the device tree but let the operating system later still make use of
> > this pin.
> > 
> > This might for example be useful to initially configure a debug pin that
> > is usually unconnected as output to prevent floating until it is used
> > later.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > this picks up a discussion that pops up now and then with our customers.
> > 
> > Last time I discussed this topic with Linus Walleij my suggestion was to
> > merge this usecase with gpio-hogs, but he wasn't happy with it because
> > hogging implies that the pin is not free for other usage and he
> > suggested to use "gpio-init" instead.
> > 
> > Maybe it's arguable if this "initial configuration" belongs into the
> > device tree, but IMHO defining a "safe configuration" should have a
> > place and the requirements are identical. This isn't implied by the name
> > however, but I don't have a better idea for a different name.
> 
> It can be argued that by the time the kernel boots, it is way to late to 
> configure pins to a safe state. Of course, even secure world reads the 
> DT these days (or are at least talking about doing so). Still any s/w 
> handling this could be too slow to get to a safe state.

Note I didn't target the kernel to implement this. I already have
patches that implement this in barebox which is also using dt. (After
all dt is about hardware description and not about what Linux should do,
right? :-)

> Maybe "optimal default" state would be more accurate. 
> 
> > Thinking further (which was also discussed last time) it would also be
> > nice to restrict usage. For example that a given pin that has
> > "output-low" as its safe setting might be configured later als high
> > output but not as input. Maybe:
> 
> I can't imagine that an output can't be an input.

It might make that line float which I'd consider "unsafe" (or "not
optimal").

> Regardless, what you're describing is constraints and that seems like
> a whole other problem than default/initial state.
> 
> Plus, for constraints I'd think we want this done at the pin level, not 
> GPIO. And we kind of already have that with pin states.

Not 100% sure I'm up to date here, if you mean

	pinctrl-names = "default", "idle"
	pinctrl-0 = ... /* that's default */
	pinctrl-1 = ... /* that's idle */

this doesn't help completely. If the idle/save state means that the pin
should be configured as low-output, you cannot define that in general.
You can only configure the pin into its gpio function but not say it
should be an output driving the pin low.

> > 	companion-reset {
> > 		gpio-somethingwithsafe;
> > 		gpios = <12 0>;
> 
> "gpios" is already a defined property with a type (phandle + args). dtc 
> checks for this now though gpio-hogs is already one exception, and I 
> don't want to add another. Maybe it could be generalized to be allowed 
> when the parent is a gpio-controller, but really I'd like to avoid this 
> pattern from spreading.

I choosed the same way as gpio-hogs because IMHO they are quite similar.
Also if the propery is supposed to be located in a child node of a
gpio-controller, repeating &gpioX seems to be at least arguable.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH RFC] gpio: of: document gpio-init nodes

[Rob Herring]

On Wed, Oct 4, 2017 at 5:00 PM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> On Wed, Oct 04, 2017 at 03:53:06PM -0500, Rob Herring wrote:
>> On Fri, Sep 22, 2017 at 10:41:38PM +0200, Uwe Kleine-König wrote:
>> > Sometimes it is desirable to define a "safe" configuration for a GPIO in
>> > the device tree but let the operating system later still make use of
>> > this pin.
>> >
>> > This might for example be useful to initially configure a debug pin that
>> > is usually unconnected as output to prevent floating until it is used
>> > later.
>> >
>> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> > ---
>> > Hello,
>> >
>> > this picks up a discussion that pops up now and then with our customers.
>> >
>> > Last time I discussed this topic with Linus Walleij my suggestion was to
>> > merge this usecase with gpio-hogs, but he wasn't happy with it because
>> > hogging implies that the pin is not free for other usage and he
>> > suggested to use "gpio-init" instead.
>> >
>> > Maybe it's arguable if this "initial configuration" belongs into the
>> > device tree, but IMHO defining a "safe configuration" should have a
>> > place and the requirements are identical. This isn't implied by the name
>> > however, but I don't have a better idea for a different name.
>>
>> It can be argued that by the time the kernel boots, it is way to late to
>> configure pins to a safe state. Of course, even secure world reads the
>> DT these days (or are at least talking about doing so). Still any s/w
>> handling this could be too slow to get to a safe state.
>
> Note I didn't target the kernel to implement this. I already have
> patches that implement this in barebox which is also using dt. (After
> all dt is about hardware description and not about what Linux should do,
> right? :-)

Right. But how do you know barebox is early enough?

>> Maybe "optimal default" state would be more accurate.
>>
>> > Thinking further (which was also discussed last time) it would also be
>> > nice to restrict usage. For example that a given pin that has
>> > "output-low" as its safe setting might be configured later als high
>> > output but not as input. Maybe:
>>
>> I can't imagine that an output can't be an input.
>
> It might make that line float which I'd consider "unsafe" (or "not
> optimal").
>
>> Regardless, what you're describing is constraints and that seems like
>> a whole other problem than default/initial state.
>>
>> Plus, for constraints I'd think we want this done at the pin level, not
>> GPIO. And we kind of already have that with pin states.
>
> Not 100% sure I'm up to date here, if you mean
>
>         pinctrl-names = "default", "idle"
>         pinctrl-0 = ... /* that's default */
>         pinctrl-1 = ... /* that's idle */
>
> this doesn't help completely. If the idle/save state means that the pin
> should be configured as low-output, you cannot define that in general.
> You can only configure the pin into its gpio function but not say it
> should be an output driving the pin low.

>
>> >     companion-reset {
>> >             gpio-somethingwithsafe;
>> >             gpios = <12 0>;
>>
>> "gpios" is already a defined property with a type (phandle + args). dtc
>> checks for this now though gpio-hogs is already one exception, and I
>> don't want to add another. Maybe it could be generalized to be allowed
>> when the parent is a gpio-controller, but really I'd like to avoid this
>> pattern from spreading.
>
> I choosed the same way as gpio-hogs because IMHO they are quite similar.
> Also if the propery is supposed to be located in a child node of a
> gpio-controller, repeating &gpioX seems to be at least arguable.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |