lkml.org 
[lkml]   [2011]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRE: [PATCH] pinctrl: indicate GPIO direction on single GPIO request
Thomas Abraham wrote at Monday, November 14, 2011 11:00 AM:
> On 14 November 2011 22:48, Stephen Warren <swarren@nvidia.com> wrote:
> > Linus Walleij wrote at Monday, November 14, 2011 2:11 AM:
> >> When requesting a single GPIO pin to be muxed in, some controllers
> >> will need to poke a different value into the control register
> >> depending on whether the pin will be used for GPIO output or GPIO
> >> input. So pass this info along for the gpio_request_enable()
> >> function, we assume this is not needed for the gpio_free_disable()
> >> function for the time being.
> >
> > I'm not sure this API change makes sense.
> >
> > Functions gpio_direction_{input,output} already exist to configure the
> > direction of a GPIO, and drivers should already be using them. These have
> > to work to allow drivers to toggle the direction dynamically. Requiring
> > them to additionally pass this same information to the pinmux driver when
> > setting up the pinmux seems like extra redundant work.
>
> Hardware that require pinmux controller also to be programmed for
> setting the gpio direction would require this. Yes, there does seem to
> be redundancy if the driver has to call both
> gpio_direction_{input,output} and pinmux_request_gpio (with the new
> direction parameter). Also, there seems to be a redundancy if the
> driver calls both gpio_request and pinmux_request_gpio.

Having all drivers call two APIs every time a GPIO needs to be configured
seems to be a Bad Thing. Perhaps we can get away with just gpio_request();
see below.

Igor Grinberg wrote at Tuesday, November 15, 2011 12:16 AM:
> On 11/14/11 19:18, Stephen Warren wrote:
...
> > Instead, shouldn't it work like this:
> >
> > * If the pinmux driver implementation behind pinmux_request_gpio() needs
> > to know the direction when configuring the HW, default to input for safety;
> > that will prevent the SoC driving a signal on a GPIO that's driven by some
> > other device.
>
> If the GPIO has been configured for output by boot loader
> and drives a value, and now you want Linux to take control over it,
> then configuring it for input will not be safe at all.
> I think this kind of flexibility is necessary (although it can be
> implemented in different ways).

That's true; it would be better to get to the final desired state in a
single explicit step.

So I think the solution for this is for gpio_request() to be redefined
to affect the pinmux where required.

Now I know that Documentation/gpio.txt explicitly says that gpio_request()
doesn't touch the pinmux... Does anyone know the history there; perhaps we
can revisit that.

In particular, perhaps on systems that need the pinmux programmed for
GPIO direction, pinmux_request_gpio() will just reserve the pin, and be
a no-op as far as HW is convered, whereas gpio_request_*() will call
into pinmux by a back-door to do the actual pinmux configuration?

Similarly on Tegra, to select a pin as a GPIO, you actually write a bit
to a GPIO register that overrides the pinmux function selection. Right
now, I have the pinmux driver calling into the GPIO driver to do this.
I could remove this backdoor call, and rely solely on gpio_request_*()
for this.

I suppose there's still a problem on HW that can mux multiple GPIOs onto
a single pin, /and/ needs to distinguish GPIO in vs. out while doing so.
I guess this could be handled by having pinmux_request_gpio do:

If pin is already a GPIO:
Leave it untouched
Else:
Configure pin pin to be a GPIO on desired controller,
picking in/out as appropriate for HW to avoid glitches

gpio_request_*() would still call into the pinmux driver by a back door
to select the final desired IO direction.

It'd be nice to define gpio_request() as always calling pinmux_gpio_request(),
but that won't work on systems where there isn't a 1:1 mapping between
pins and GPIOs: multiple controllers, or GPIOs can routed to different
pins.

--
nvpublic



\
 
 \ /
  Last update: 2011-11-15 20:07    [from the cache]
©2003-2011 Jasper Spaans