Re: [PATCH v1 3/7] gpiolib: Change type of lflags in gpiod_hog() and gpiod_configure_flags()

[Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>]

On Thu, Apr 04, 2019 at 11:16:19PM +0700, Linus Walleij wrote:
> The enum gpio_lookup_flags is essentially something I came up with
> for <linux/gpio/machine.h> to use used by struct gpiod_lookup.
> 
> It looks like it does (not just 0, 1, 2, 3... ) because it mirrors the defines
> for e.g. <linux/gpio.h> so if we can get rid of that legacy file we can
> change the values for enum gpio_lookup_flags.
> 
> Maybe it was stupid of me. We could change the enum gpio_lookup_flags
> to just
> #define GPIO_ACTIVE_HIGH 0
> #define GPIO_ACTIVE_LOW BIT(0)
> etc.
> 
> And then just have some unsigned long in struct gpiod_lookup.
> 
> What do you folks think makes most sense?

I think it is fine to have enum gpio_lookup_flags declaring the
constants:

	enum gpio_lookup_flags {
		GPIO_ACTIVE_HIGH = (0 << 0),
		GPIO_ACTIVE_LOW = (1 << 0),
		...

But when we pass combination (bitmask) of those constants to a function,
say gpiod_hog(), IMHO the right type is unsigned int/long as it is
already using:

	int gpiod_hog(struct gpio_desc *desc, const char *name,
		      unsigned long lflags, enum gpiod_flags dflags)

If it takes only a single enumerated value then using enum
gpio_lookup_flags would be proper option.

Same goes of course with variables and members of a struct.