Re: [PATCH] gpio: introduce gpio_request_one() and friends

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 8 Jan 2010 13:28:57 +0800
Eric Miao <eric.y.miao@gmail.com> wrote:

> Typo in GPIOF_* definitions, updated patch follows:
> 
> commit 29cd35f57699fd93a12132186d52109a55ed57e7
> Author: Eric Miao <eric.y.miao@gmail.com>
> Date:   Fri Jan 8 12:16:28 2010 +0800
> 
>     gpio: introduce gpio_request_one() and friends
> 
>     gpio_request() without initial configuration of the GPIO is normally
>     useless, introduce gpio_request_one() together with GPIOF_ flags for
>     input/output direction and initial output level.
> 
>     gpio_{request,free}_array() for multiple GPIOs.
> 
>     Cc: David Brownell <dbrownell@users.sourceforge.net>
>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index a25ad28..e80a1f8 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1239,6 +1239,48 @@ void gpio_free(unsigned gpio)
>  }
>  EXPORT_SYMBOL_GPL(gpio_free);
> 
> +int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
> +{
> +	int err;
> +
> +	err = gpio_request(gpio, label);
> +	if (err)
> +		return err;
> +
> +	if (flags & GPIOF_DIR_IN)
> +		err = gpio_direction_input(gpio);
> +	else
> +		err = gpio_direction_output(gpio,
> +				(flags & GPIOF_INIT_HIGH) ? 1 : 0);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(gpio_request_one);
> +
> +int gpio_request_array(struct gpio *array, size_t num)
> +{
> +	int i, err;
> +
> +	for (i = 0; i < num; i++, array++) {
> +		err = gpio_request_one(array->gpio, array->flags, array->label);
> +		if (err)
> +			goto err_free;
> +	}
> +	return 0;
> +
> +err_free:
> +	while (i--)
> +		gpio_free((--array)->gpio);
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(gpio_request_array);
> +
> +void gpio_free_array(struct gpio *array, size_t num)
> +{
> +	while (num--)
> +		gpio_free((array++)->gpio);
> +}
> +EXPORT_SYMBOL_GPL(gpio_free_array);

Global, exported-to-modules interfaces should be documented, IMO.

>  /**
>   * gpiochip_is_requested - return string iff signal was requested
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 485eeb6..a9e0b94 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -136,6 +136,26 @@ extern int __gpio_cansleep(unsigned gpio);
> 
>  extern int __gpio_to_irq(unsigned gpio);
> 
> +#define GPIOF_DIR_OUT	(0 << 0)
> +#define GPIOF_DIR_IN	(1 << 0)
> +
> +#define GPIOF_INIT_LOW	(0 << 1)
> +#define GPIOF_INIT_HIGH	(1 << 1)
> +
> +#define GPIOF_IN		(GPIOF_DIR_IN)
> +#define GPIOF_OUT_INIT_LOW	(GPIOF_DIR_OUT | GPIOF_INIT_LOW)
> +#define GPIOF_OUT_INIT_HIGH	(GPIOF_DIR_OUT | GPIOF_INIT_HIGH)
> +
> +struct gpio {
> +	unsigned	gpio;
> +	unsigned long	flags;
> +	const char	*label;
> +};

hm.  Was "struct gpio" a well-chosen identifier?  The name implies that
this structure is the definitive, unified, generic kernel-wide
representation of a "gpio", whatever that is.

> +extern int gpio_request_one(unsigned gpio, unsigned long flags, const
> char *label);

Your email client is wordwrapping the patches.