diff mbox series

[RFC] gpio: define gpio-init nodes to initialize pins similar to hogs

Message ID 20190909105919.30418-1-u.kleine-koenig@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [RFC] gpio: define gpio-init nodes to initialize pins similar to hogs | expand

Commit Message

Uwe Kleine-König Sept. 9, 2019, 10:59 a.m. UTC 
Sometimes it is handy to be able to easily define a "safe" state for a
GPIO. This might for example be used to ensure that an ethernet phy is
properly reset during startup or just that all pins have a defined state
to minimize leakage current. As such a pin must be requestable (and
changable) by a device driver, a gpio-hog cannot be used.

So define a GPIO initializer with a syntax identical to a GPIO hog just
using "gpio-init" as identifier instead of "gpio-hog".

The usage I have in mind (and also implemented in a custom patch stack
on top of barebox already) is targeting the bootloader and not
necessarily Linux as such an boot-up initialisation should be done as
early as possible.

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

maybe it also makes sense to use "gpio-safe"? Maybe it (then) makes
sense to reset the gpio in the indicated state after it is released?

Also it might be beneficial to make the wording more explicit in the
description and for example tell that only one of gpio-hog and gpio-init
must be provided. 

Best regards
Uwe

 Documentation/devicetree/bindings/gpio/gpio.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Linus Walleij Sept. 12, 2019, 9:05 a.m. UTC | #1 
On Mon, Sep 9, 2019 at 11:59 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> Sometimes it is handy to be able to easily define a "safe" state for a
> GPIO. This might for example be used to ensure that an ethernet phy is
> properly reset during startup or just that all pins have a defined state
> to minimize leakage current. As such a pin must be requestable (and
> changable) by a device driver, a gpio-hog cannot be used.
>
> So define a GPIO initializer with a syntax identical to a GPIO hog just
> using "gpio-init" as identifier instead of "gpio-hog".
>
> The usage I have in mind (and also implemented in a custom patch stack
> on top of barebox already) is targeting the bootloader and not
> necessarily Linux as such an boot-up initialisation should be done as
> early as possible.
>
> Not-yet-signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> maybe it also makes sense to use "gpio-safe"? Maybe it (then) makes
> sense to reset the gpio in the indicated state after it is released?
>
> Also it might be beneficial to make the wording more explicit in the
> description and for example tell that only one of gpio-hog and gpio-init
> must be provided.

It's no secret that I am in favor of this approach, as I like consistency
with the hogs.

The DT people have been against, as they prefer something like an
initial array of values akin to gpio-names IIRC. But this is a good
time for them to speak up.

Yours,
Linus Walleij
Uwe Kleine-König Sept. 12, 2019, 10:06 a.m. UTC | #2 
On Thu, Sep 12, 2019 at 10:05:23AM +0100, Linus Walleij wrote:
> On Mon, Sep 9, 2019 at 11:59 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > Sometimes it is handy to be able to easily define a "safe" state for a
> > GPIO. This might for example be used to ensure that an ethernet phy is
> > properly reset during startup or just that all pins have a defined state
> > to minimize leakage current. As such a pin must be requestable (and
> > changable) by a device driver, a gpio-hog cannot be used.
> >
> > So define a GPIO initializer with a syntax identical to a GPIO hog just
> > using "gpio-init" as identifier instead of "gpio-hog".
> >
> > The usage I have in mind (and also implemented in a custom patch stack
> > on top of barebox already) is targeting the bootloader and not
> > necessarily Linux as such an boot-up initialisation should be done as
> > early as possible.
> >
> > Not-yet-signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> >
> > maybe it also makes sense to use "gpio-safe"? Maybe it (then) makes
> > sense to reset the gpio in the indicated state after it is released?
> >
> > Also it might be beneficial to make the wording more explicit in the
> > description and for example tell that only one of gpio-hog and gpio-init
> > must be provided.
> 
> It's no secret that I am in favor of this approach, as I like consistency
> with the hogs.
> 
> The DT people have been against, as they prefer something like an
> initial array of values akin to gpio-names IIRC. But this is a good
> time for them to speak up.

To be fair, I added them to Cc:. For the new readers: The diff I
suggested looks as follows (probably whitespace broken as I cut-n-pasted):

> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> index a8895d339bfe..5b7883f5520f 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -182,13 +182,16 @@ gpio-controller@00000000 {
>                 "poweroff", "reset";
>  }
> 
> -The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
> -providing automatic GPIO request and configuration as part of the
> -gpio-controller's driver probe function.
> +The GPIO chip may contain GPIO hog and init definitions. GPIO hogging is a
> +mechanism providing automatic GPIO request and configuration as part of the
> +gpio-controller's driver probe function. An GPIO initializer is similar but
> +doesn't prevent later requesting and reconfiguration.
> 
>  Each GPIO hog definition is represented as a child node of the GPIO controller.
>  Required properties:
>  - gpio-hog:   A property specifying that this child node represents a GPIO hog.
> +- gpio-init:  A property specifying that this child node represents a GPIO
> +              initializer.
>  - 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).

How would this alternate approach look like? Something like:

	gpio-controler@123450 {
		compatible = "..";
		gpio-controller;
		#gpio-cells = <2>;

		init = "", "output-high", "", "input", "", "", "output-low";
	};

? Compared to the solution I suggested (and hogs) this differs as you cannot
pass flags like GPIO_ACTIVE_LOW.

(Sidenode: As

	mygpio {
		gpio-hog;
		gpios = <5 GPIO_ACTIVE_LOW>;
		output-low;
	};

makes AFAIK the output high it would be less surprising if the binding
supported "output-active" and "output-inactive".)

Best regards
Uwe
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index a8895d339bfe..5b7883f5520f 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -182,13 +182,16 @@  gpio-controller@00000000 {
 		"poweroff", "reset";
 }
 
-The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
-providing automatic GPIO request and configuration as part of the
-gpio-controller's driver probe function.
+The GPIO chip may contain GPIO hog and init definitions. GPIO hogging is a
+mechanism providing automatic GPIO request and configuration as part of the
+gpio-controller's driver probe function. An GPIO initializer is similar but
+doesn't prevent later requesting and reconfiguration.
 
 Each GPIO hog definition is represented as a child node of the GPIO controller.
 Required properties:
 - gpio-hog:   A property specifying that this child node represents a GPIO hog.
+- gpio-init:  A property specifying that this child node represents a GPIO
+	      initializer.
 - 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).