[PATCH v2 3/3] gpiolib: Add GPIO initialization

[Uwe Kleine-König]

Hello Linus,

On Thu, May 11, 2017 at 04:29:30PM +0200, Linus Walleij wrote:
> On Sun, May 7, 2017 at 11:45 AM, Uwe Kleine-König
> <u.kleine-koenig at pengutronix.de> wrote:
> > [Me]
> 
> >> >    IMHO it is sensible to allow requesting a gpio that is completely
> >> >    hogged and just prevent changing direction and (if it's an output)
> >> >    value.
> >>
> >> No it doesn't make sense at all to take something that is hogged
> >> because it is completely unintuitive to the very word "hog".
> >
> > What I want is that it is possible to restrict the usage of a GPIO more
> > fine-grained. For example only fix the direction to input but allow a
> > driver to still read out it's value. Or fix the direction to output but
> > allow changing the level.
> 
> So let me understand this right.
> 
> For something input-only, return -EINVAL from .direction_output()
> and vice versa mutatis mutandis for something output-only.

Not sure if it's -EINVAL, but some error for sure, yes.
 
> This semantic is set at runtime when using these functions.
> 
> Why can't you do this in the specific driver? Why does that have

It's not known to the driver. On an i.MX28 based machine a certain pin
of the SoC might be NC and so should be set as output-low. That's
specific to that machine while the i.MX28 (and so the driver) could well
operate that pin as input. It might also make sense to limit a line to
input only if it is connected to another device that drives the line.

> to be handled by the core? I mean of course we can add a
> flag to the gpio_desc if you think it's gonna be a common case,
> but then I want an indication that there is a bunch of drivers that
> need this done in the core.

So this is entirely independent of the driver in question.

> > So the semantic of a hogged gpio as
> > implemented today is a special case of my new use cases. So it seems
> > naturally to extend the already existing concept.
> 
> I disagree

I guess you disagree to the 2nd sentence only because the first is
obviously true.

>            and the DT maintainer Rob Herring already said he kind
> of dislikes the hogs already, so let's not expand their use beyond
> what is already being done, which is explicit hogging.
> 
> > And concerning the meaning of hogging: It still takes some freedom from
> > others, just not everything because you can still request the line. So
> > not being a native English speaker the word still fits for me.
> 
> IMO this breaks Rusty Russell's API Design Manifesto,
> levels 7 and 6:
> 
> 7. The obvious use is (probably) the correct one.
> 6. The name tells you how to use it.
> 
> In my case, just no. It is not obvuous and does not tell me how
> to use it.
> 
> Something like gpio-line-initial-directions, gpio-line-initial-values
> etc does.
> 
> > And technically it is sensible to implement the new use cases and
> > today's hogging together and so it is also sensible IMHO to use the same
> > mechanism in the device tree.
> 
> I disagree with that, sorry.

gpio-hogs are a special case of my suggestion. So even if the binding
looks different in the end, it doesn't make sense to have two
implementations for the same thing.

> > So to stick to your suggestion I would have in the end for two lines
> > io12 that must be 0 for ESD reasons and io13 that drives the RST-line of
> > a companion DSP that should be kept low until userspace is ready and
> > then allow being driven high via gpioctl:
> >
> >         io12 {
> >                 gpio-hog;
> >                 gpio = <4 0>;
> >                 output-low;
> >         };
> >
> >         io13 {
> >                 gpio-init;
> >                 gpio = <5 0>;
> >                 output-low;
> >                 fixed-direction;
> >         };
> >
> > For me it looks artificial to not use "gpio-hog" for the 2nd
> > specification but if that is the compromise that we can agree on, that's
> > ok for me.
> 
> This looks OK to me.

This is my favourite binding among the stuff discussed given you don't
want to "reuse" gpio-hog, but I'm open for better ones :-)

The following looks nice:

	io12 {
		gpio;
		gpio = <4 0>;
		init-output-low;
		fixed-direction;
		fixed-value;
	};

but won't work because there is "gpio" twice.

If you ask me, having

	io12 {
		gpio-init;
		gpio = <4 0>;
		output-low;
		fixed-direction;
		fixed-value;
	};

somehow makes it superfluous to be able to say

	io12 {
		gpio-hog;
		gpio = <4 0>;
		output-low;
	};

as the only difference would be that with the former the gpio could be
requested while this isn't possible with the latter. (Which is also a
difference that shouldn't be there in a hardware description.)

> I think you will have a problem with Rob on this though, he's not
> a big fan of these hogging nodes and I'm not sure he's gonna like
> the gpio-init nodes much.

Rob: IMHO it makes sense to describe limitations like "This pin should
not be driven (as there is another driver)" or "This is an output pin
with the safe initial level being low" (for a line that drives the reset
pin of a device) or maybe even "This pin might be used as input or
output, the safe value is input" (for a pin that is available on a
pinheader with a pullup, so the usage is unknown) in the device tree. I
don't really care about how they should be specified, so if you have a
different idea, I'm all ears.

Best regards
Uwe

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