Re: [PATCH] gpio: extend gpiod_get*() with flags parameter

[Alexandre Courbot <gnurou@gmail.com>]

On Fri, Jul 25, 2014 at 1:23 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> diff --git a/Documentation/gpio/consumer.txt
>> b/Documentation/gpio/consumer.txt index 7ff30d2..a3fb1d7 100644
>> --- a/Documentation/gpio/consumer.txt
>> +++ b/Documentation/gpio/consumer.txt
>> @@ -29,13 +29,24 @@ gpiod_get() functions. Like many other kernel
>> subsystems, gpiod_get() takes the device that will use the GPIO and the
>> function the requested GPIO is supposed to fulfill:
>>
>> -     struct gpio_desc *gpiod_get(struct device *dev, const char *con_id)
>> +     struct gpio_desc *gpiod_get(struct device *dev, const char *con_id,
>> +                                 enum gpio_flags flags)
>
> I assume you mean enum gpiod_flags here and in the rest of the documentation.

Indeed, thanks for pointing it out.

>
>>  If a function is implemented by using several GPIOs together (e.g. a simple
>> LED device that displays digits), an additional index argument can be
>> specified:
>>
>>       struct gpio_desc *gpiod_get_index(struct device *dev,
>> -                                       const char *con_id, unsigned int idx)
>> +                                       const char *con_id, unsigned int idx,
>> +                                       enum gpio_flags flags)
>> +
>> +The flags parameter is used to optionally specify a direction and initial
>> value +for the GPIO. Values can be:
>> +
>> +* AS_IS or 0 to not initialize the GPIO at all. The direction must be set
>> later
>> +  with one of the dedicated functions.
>> +* INPUT to initialize the GPIO as input.
>> +* OUTPUT_LOW to initialize the GPIO as output with a value of 0.
>> +* OUTPUT_HIGH to initialize the GPIO as output with a value of 1.
>
> Pretty please, could you at least prefix that with GPIOD_ ? Those names are
> too generic.
>
> How about renaming them to GPIOD_AS_IS, GPIOD_IN, GPIOD_OUT_INIT_LOW and
> GPIOD_OUT_INIT_HIGH in order to match the GPIOF_ flags ?

Yeah, you're right, the possibility of name collision is quite high here.

Thanks,
Alex.