Messages in this thread | | | Date | Thu, 17 Feb 2011 12:00:43 +0100 | From | Wolfram Sang <> | Subject | Re: [PATCH v2 2/4] gpiolib: Add ability to get GPIO pin direction |
| |
On Wed, Feb 16, 2011 at 06:56:54PM -0600, Peter Tyser wrote: > Add a new get_direction() function to the gpio_chip structure. This is > useful so that the direction of a pin can be determined when its > initially exported. Previously, the direction defaulted to "unknown" > regardless of the actual configuration of the GPIO pin. > > If a GPIO driver implements get_direction(), it is called in > gpio_request() to set the initial direction of the pin accurately. > > Cc: Alek Du <alek.du@intel.com> > Cc: Samuel Ortiz <sameo@linux.intel.com> > Cc: David Brownell <dbrownell@users.sourceforge.net> > Cc: Eric Miao <eric.y.miao@gmail.com> > Cc: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com> > Cc: Joe Perches <joe@perches.com> > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > Cc: Grant Likely <grant.likely@secretlab.ca> > Signed-off-by: Peter Tyser <ptyser@xes-inc.com> > --- > Changes since v1: > - Add support for "unknown" direction > > drivers/gpio/gpiolib.c | 20 ++++++++++++++++++++ > include/asm-generic/gpio.h | 4 ++++ > 2 files changed, 24 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 0113c10..7723beb 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1214,6 +1214,26 @@ int gpio_request(unsigned gpio, const char *label) > } > } > > + if (chip->get_direction) { > + /* chip->get_direction may sleep */ > + spin_unlock_irqrestore(&gpio_lock, flags); > + switch (chip->get_direction(chip, gpio - chip->base)) { > + case -1:
Use some #defines instead of hardcoded values?
> + clear_bit(FLAG_DIR_OUT, &desc->flags); > + clear_bit(FLAG_DIR_IN, &desc->flags); > + break; > + case 0: > + set_bit(FLAG_DIR_IN, &desc->flags); > + clear_bit(FLAG_DIR_OUT, &desc->flags); > + break; > + case 1: > + set_bit(FLAG_DIR_OUT, &desc->flags); > + clear_bit(FLAG_DIR_IN, &desc->flags); > + break; > + } > + spin_lock_irqsave(&gpio_lock, flags);
You are changing the flags without the lock. I think it would be good to copy the locking-pattern of chip->request for now.
Regards,
Wolfram
-- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
| |