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

[Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > ---
> > 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

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