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

[Eric Miao]

On Wed, Jan 13, 2010 at 7:09 AM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 8 Jan 2010 13:28:57 +0800
> Eric Miao <eric.y.miao@xxxxxxxxx> wrote:
>
>> Typo in GPIOF_* definitions, updated patch follows:
>>
>> commit 29cd35f57699fd93a12132186d52109a55ed57e7
>> Author: Eric Miao <eric.y.miao@xxxxxxxxx>
>> 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@xxxxxxxxxxxxxxxxxxxxx>
>> Â Â Signed-off-by: Eric Miao <eric.y.miao@xxxxxxxxx>
>>
>> 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.
>

OK, will do.

>> Â/**
>> Â * 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.
>

These three fields should be sufficient to support future requirements
of GPIO by using additional flags (e.g. open-drain capable can be
actually made a flag quite easy here), so in my opinion - yes, this
should be a proper name.

>> +extern int gpio_request_one(unsigned gpio, unsigned long flags, const
>> char *label);
>
> Your email client is wordwrapping the patches.
>

Sorry.
--
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/