Messages in this thread | | | From | Stephen Warren <> | Date | Tue, 15 Nov 2011 11:04:07 -0800 | Subject | RE: [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
| |