Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction

[Uwe Kleine-König]

On Tue, Feb 15, 2011 at 11:49:11AM -0600, Peter Tyser wrote:
> On Tue, 2011-02-15 at 17:19 +0000, Alan Cox wrote:
> > > +       if (chip->get_direction) {
> > > +               /* chip->get_direction may sleep */
> > > +               spin_unlock_irqrestore(&gpio_lock, flags);
> > > +               if (chip->get_direction(chip, gpio - chip->base) > 0)
> > > +                       set_bit(FLAG_IS_OUT, &desc->flags);
> > > +               spin_lock_irqsave(&gpio_lock, flags);
> > > +       } else {
> > > +               set_bit(FLAG_IS_UNKNOWN, &desc->flags);
> > > +       }
> > > 
> > > This would have the side effect of having nearly all GPIO drivers
> > > default to an "unknown" direction until they implement the new
> > > get_direction() function, which I think is an improvement over the
> > 
> > This doesn't solve anything. If the hardware supports alt_func state then
> > it now can't implement get_direction, so that's useless.
> 
> I don't follow.  If a pin is configured for some alternate function,
> then requesting it for GPIO should fail, thus it doesn't matter if it
> implements get_direction()?  Since we can't easily toggle back and forth
> between GPIO and alt_func, I'd think we shouldn't be able to request
> alt_func pins for GPIO - they should be off-limits to the GPIO subsystem
> altogether.
hmm, I'm not sure.  Letting gpio_request fail looks good from the POV of
an uninformed driver.  But for some platform code, it seems more natural
to do:

	gpio_request(mygpio);
	myplatform_iomux_setup(pad_for_altfunc);
	do_something_special();
	/*
	 * the controler is unable to reset some component, so use
	 * bitbanging for that
	 */
	myplatform_iomux_setup(pad_for_gpio);
	gpio_direction_output(mygpio, 0);
	usleep(100);
	myplatform_iomux_setup(pad_for_altfunc);
	...

instead of only being able to gpio_request after
myplatform_iomux_setup(pad_for_gpio). (And so in theory take that risk
that another process grabs the gpio between mux-for-gpio and
gpio_request.) So if you ask me, it's gpio_direction_{in,out}put that
should fail, not gpio_request.  But I'm not that sure about it to
already know now to keep this opinion until after this discussion is
over.

> My understanding is that currently if some platform wants to toggle pins
> back and forth between alt_func and GPIO, it needs to handle that logic
> itself.  If platform code is handling that toggling, I'd think the GPIO
> code should not touch pins configured as alt_func.  If the platform is
> no longer using them as alt_func, then it should poke the appropriate
> registers to make them not alt_func so that they can then be used by the
> GPIO subsystem.
.. or at least make the usage via the gpio subsystem fail using it.
OTOH on arm/plat-mxc (at least the newer chips) there is no easy mapping
between pads and gpios.  So currently we do:  gpio_request and
gpio_direction_{in,out}put yield 0, but it depends on the pin muxing if
the gpio is "visible" anywhere.  I don't like that much, but I agree
that it's not worth to setup a huge table to map gpios to pads and back
just to return -ESOMETHING in the gpio functions.
 
Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/