| Message ID | 20190410153921.66546-2-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/7] gpiolib: Indent entry values of enum gpio_lookup_flags | expand |
On Wed, Apr 10, 2019 at 06:39:16PM +0300, Andy Shevchenko wrote: > The library uses enum gpio_lookup_flags to define the possible > characteristics of GPIO pin. Since enumerator listed only individual > bits the common use of it is in a form of a bitmask of > gpio_lookup_flags GPIO_* values. The more correct type for this is > unsigned long. > > Due to above convert all users to use unsigned long instead of > enum gpio_lookup_flags except enumerator definition. > > While here, make field and parameter descriptions consistent as well. > > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> One small comment: > --- > drivers/gpio/gpiolib-acpi.c | 14 +++++++++----- > drivers/gpio/gpiolib-of.c | 11 +++++------ > drivers/gpio/gpiolib.c | 13 ++++++------- > drivers/gpio/gpiolib.h | 9 ++++----- > include/linux/gpio/machine.h | 8 ++++---- > 5 files changed, 28 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c > index 30d0baf7ddae..ba9cafa13ca2 100644 > --- a/drivers/gpio/gpiolib-acpi.c > +++ b/drivers/gpio/gpiolib-acpi.c > @@ -696,7 +696,7 @@ struct gpio_desc *acpi_find_gpio(struct device *dev, > const char *con_id, > unsigned int idx, > enum gpiod_flags *dflags, > - enum gpio_lookup_flags *lookupflags) > + unsigned long *lookupflags) > { > struct acpi_device *adev = ACPI_COMPANION(dev); > struct acpi_gpio_info info; > @@ -992,9 +992,12 @@ static void acpi_gpiochip_free_regions(struct acpi_gpio_chip *achip) > } > } > > -static struct gpio_desc *acpi_gpiochip_parse_own_gpio( > - struct acpi_gpio_chip *achip, struct fwnode_handle *fwnode, > - const char **name, unsigned int *lflags, unsigned int *dflags) > +static struct gpio_desc * > +acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip, > + struct fwnode_handle *fwnode, > + const char **name, > + unsigned long *lflags, > + unsigned int *dflags) Why not "unsigned int"? Now it looks inconsistent.
On Thu, Apr 11, 2019 at 11:57:13AM +0300, Mika Westerberg wrote: > On Wed, Apr 10, 2019 at 06:39:16PM +0300, Andy Shevchenko wrote: > > The library uses enum gpio_lookup_flags to define the possible > > characteristics of GPIO pin. Since enumerator listed only individual > > bits the common use of it is in a form of a bitmask of > > gpio_lookup_flags GPIO_* values. The more correct type for this is > > unsigned long. > > > > Due to above convert all users to use unsigned long instead of > > enum gpio_lookup_flags except enumerator definition. > > > > While here, make field and parameter descriptions consistent as well. > > > > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > One small comment: > > > --- > > drivers/gpio/gpiolib-acpi.c | 14 +++++++++----- > > drivers/gpio/gpiolib-of.c | 11 +++++------ > > drivers/gpio/gpiolib.c | 13 ++++++------- > > drivers/gpio/gpiolib.h | 9 ++++----- > > include/linux/gpio/machine.h | 8 ++++---- > > 5 files changed, 28 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c > > index 30d0baf7ddae..ba9cafa13ca2 100644 > > --- a/drivers/gpio/gpiolib-acpi.c > > +++ b/drivers/gpio/gpiolib-acpi.c > > @@ -696,7 +696,7 @@ struct gpio_desc *acpi_find_gpio(struct device *dev, > > const char *con_id, > > unsigned int idx, > > enum gpiod_flags *dflags, > > - enum gpio_lookup_flags *lookupflags) > > + unsigned long *lookupflags) > > { > > struct acpi_device *adev = ACPI_COMPANION(dev); > > struct acpi_gpio_info info; > > @@ -992,9 +992,12 @@ static void acpi_gpiochip_free_regions(struct acpi_gpio_chip *achip) > > } > > } > > > > -static struct gpio_desc *acpi_gpiochip_parse_own_gpio( > > - struct acpi_gpio_chip *achip, struct fwnode_handle *fwnode, > > - const char **name, unsigned int *lflags, unsigned int *dflags) > > +static struct gpio_desc * > > +acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip, > > + struct fwnode_handle *fwnode, > > + const char **name, > > + unsigned long *lflags, > > + unsigned int *dflags) > > Why not "unsigned int"? Now it looks inconsistent. inconsistent with what part?
On Thu, Apr 11, 2019 at 12:58:48PM +0300, Andy Shevchenko wrote: > > > +acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip, > > > + struct fwnode_handle *fwnode, > > > + const char **name, > > > + unsigned long *lflags, > > > + unsigned int *dflags) > > > > Why not "unsigned int"? Now it looks inconsistent. > > inconsistent with what part? Now you have: unsigned long *lflags unsigned int *dflags why not unsigned int *lflags unsigned int *dflags ?
On Thu, Apr 11, 2019 at 01:37:55PM +0300, Mika Westerberg wrote: > On Thu, Apr 11, 2019 at 12:58:48PM +0300, Andy Shevchenko wrote: > > > > +acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip, > > > > + struct fwnode_handle *fwnode, > > > > + const char **name, > > > > + unsigned long *lflags, > > > > + unsigned int *dflags) > > > > > > Why not "unsigned int"? Now it looks inconsistent. > > > > inconsistent with what part? > > Now you have: > > unsigned long *lflags > unsigned int *dflags > > why not > > unsigned int *lflags > unsigned int *dflags > > ? Have you looked at the patch 4 in the series? It changes type of dflags to be enum gpiod_flags.
On Thu, Apr 11, 2019 at 03:00:23PM +0300, Andy Shevchenko wrote: > On Thu, Apr 11, 2019 at 01:37:55PM +0300, Mika Westerberg wrote: > > On Thu, Apr 11, 2019 at 12:58:48PM +0300, Andy Shevchenko wrote: > > > > > +acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip, > > > > > + struct fwnode_handle *fwnode, > > > > > + const char **name, > > > > > + unsigned long *lflags, > > > > > + unsigned int *dflags) > > > > > > > > Why not "unsigned int"? Now it looks inconsistent. > > > > > > inconsistent with what part? > > > > Now you have: > > > > unsigned long *lflags > > unsigned int *dflags > > > > why not > > > > unsigned int *lflags > > unsigned int *dflags > > > > ? > > Have you looked at the patch 4 in the series? Actually I did not. > It changes type of dflags to be enum gpiod_flags. Indeed it does, so please ignore my comment then :-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 30d0baf7ddae..ba9cafa13ca2 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -696,7 +696,7 @@ struct gpio_desc *acpi_find_gpio(struct device *dev, const char *con_id, unsigned int idx, enum gpiod_flags *dflags, - enum gpio_lookup_flags *lookupflags) + unsigned long *lookupflags) { struct acpi_device *adev = ACPI_COMPANION(dev); struct acpi_gpio_info info; @@ -992,9 +992,12 @@ static void acpi_gpiochip_free_regions(struct acpi_gpio_chip *achip) } } -static struct gpio_desc *acpi_gpiochip_parse_own_gpio( - struct acpi_gpio_chip *achip, struct fwnode_handle *fwnode, - const char **name, unsigned int *lflags, unsigned int *dflags) +static struct gpio_desc * +acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip, + struct fwnode_handle *fwnode, + const char **name, + unsigned long *lflags, + unsigned int *dflags) { struct gpio_chip *chip = achip->chip; struct gpio_desc *desc; @@ -1037,7 +1040,8 @@ static void acpi_gpiochip_scan_gpios(struct acpi_gpio_chip *achip) struct fwnode_handle *fwnode; device_for_each_child_node(chip->parent, fwnode) { - unsigned int lflags, dflags; + unsigned long lflags; + unsigned int dflags; struct gpio_desc *desc; const char *name; int ret; diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 6a3ec575a404..da69a37bc9a7 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -288,8 +288,7 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev, const char * } struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, - unsigned int idx, - enum gpio_lookup_flags *flags) + unsigned int idx, unsigned long *flags) { char prop_name[32]; /* 32 is max size of property name */ enum of_gpio_flags of_flags; @@ -362,8 +361,8 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, * @chip: GPIO chip whose hog is parsed * @idx: Index of the GPIO to parse * @name: GPIO line name - * @lflags: gpio_lookup_flags - returned from of_find_gpio() or - * of_parse_own_gpio() + * @lflags: bitmask of gpio_lookup_flags GPIO_* values - returned from + * of_find_gpio() or of_parse_own_gpio() * @dflags: gpiod_flags - optional GPIO initialization flags * * Returns GPIO descriptor to use with Linux GPIO API, or one of the errno @@ -372,7 +371,7 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, static struct gpio_desc *of_parse_own_gpio(struct device_node *np, struct gpio_chip *chip, unsigned int idx, const char **name, - enum gpio_lookup_flags *lflags, + unsigned long *lflags, enum gpiod_flags *dflags) { struct device_node *chip_np; @@ -445,7 +444,7 @@ static int of_gpiochip_scan_gpios(struct gpio_chip *chip) struct gpio_desc *desc = NULL; struct device_node *np; const char *name; - enum gpio_lookup_flags lflags; + unsigned long lflags; enum gpiod_flags dflags; unsigned int i; int ret; diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 11dbe32ea9d5..9accbeb8a89a 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3911,8 +3911,7 @@ static struct gpiod_lookup_table *gpiod_find_lookup_table(struct device *dev) } static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, - unsigned int idx, - enum gpio_lookup_flags *flags) + unsigned int idx, unsigned long *flags) { struct gpio_desc *desc = ERR_PTR(-ENOENT); struct gpiod_lookup_table *table; @@ -4068,8 +4067,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_optional); * gpiod_configure_flags - helper function to configure a given GPIO * @desc: gpio whose value will be assigned * @con_id: function within the GPIO consumer - * @lflags: gpio_lookup_flags - returned from of_find_gpio() or - * of_get_gpio_hog() + * @lflags: bitmask of gpio_lookup_flags GPIO_* values - returned from + * of_find_gpio() or of_get_gpio_hog() * @dflags: gpiod_flags - optional GPIO initialization flags * * Return 0 on success, -ENOENT if no GPIO has been assigned to the @@ -4151,9 +4150,9 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev, unsigned int idx, enum gpiod_flags flags) { + unsigned long lookupflags = 0; struct gpio_desc *desc = NULL; int status; - enum gpio_lookup_flags lookupflags = 0; /* Maybe we have a device name, maybe not */ const char *devname = dev ? dev_name(dev) : "?"; @@ -4391,8 +4390,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_index_optional); * gpiod_hog - Hog the specified GPIO desc given the provided flags * @desc: gpio whose value will be assigned * @name: gpio line name - * @lflags: gpio_lookup_flags - returned from of_find_gpio() or - * of_get_gpio_hog() + * @lflags: bitmask of gpio_lookup_flags GPIO_* values - returned from + * of_find_gpio() or of_get_gpio_hog() * @dflags: gpiod_flags - optional GPIO initialization flags */ int gpiod_hog(struct gpio_desc *desc, const char *name, diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 078ab17b96bf..5dbfce616ae1 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -17,7 +17,6 @@ #include <linux/cdev.h> enum of_gpio_flags; -enum gpio_lookup_flags; struct acpi_device; /** @@ -95,7 +94,7 @@ static __maybe_unused const char * const gpio_suffixes[] = { "gpios", "gpio" }; struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, unsigned int idx, - enum gpio_lookup_flags *flags); + unsigned long *lookupflags); struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np, const char *list_name, int index, enum of_gpio_flags *flags); int of_gpiochip_add(struct gpio_chip *gc); @@ -104,7 +103,7 @@ void of_gpiochip_remove(struct gpio_chip *gc); static inline struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, unsigned int idx, - enum gpio_lookup_flags *flags) + unsigned long *lookupflags) { return ERR_PTR(-ENOENT); } @@ -131,7 +130,7 @@ struct gpio_desc *acpi_find_gpio(struct device *dev, const char *con_id, unsigned int idx, enum gpiod_flags *dflags, - enum gpio_lookup_flags *lookupflags); + unsigned long *lookupflags); struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode, const char *propname, int index, struct acpi_gpio_info *info); @@ -158,7 +157,7 @@ acpi_gpio_update_gpiod_flags(enum gpiod_flags *flags, struct acpi_gpio_info *inf static inline struct gpio_desc * acpi_find_gpio(struct device *dev, const char *con_id, unsigned int idx, enum gpiod_flags *dflags, - enum gpio_lookup_flags *lookupflags) + unsigned long *lookupflags) { return ERR_PTR(-ENOENT); } diff --git a/include/linux/gpio/machine.h b/include/linux/gpio/machine.h index a0a981676490..dad392158550 100644 --- a/include/linux/gpio/machine.h +++ b/include/linux/gpio/machine.h @@ -22,7 +22,7 @@ enum gpio_lookup_flags { * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO * @con_id: name of the GPIO from the device's point of view * @idx: index of the GPIO in case several GPIOs share the same name - * @flags: mask of GPIO_* values + * @flags: bitmask of gpio_lookup_flags GPIO_* values * * gpiod_lookup is a lookup table for associating GPIOs to specific devices and * functions using platform data. @@ -32,7 +32,7 @@ struct gpiod_lookup { u16 chip_hwnum; const char *con_id; unsigned int idx; - enum gpio_lookup_flags flags; + unsigned long flags; }; struct gpiod_lookup_table { @@ -46,7 +46,7 @@ struct gpiod_lookup_table { * @chip_label: name of the chip the GPIO belongs to * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO * @line_name: consumer name for the hogged line - * @lflags: mask of GPIO lookup flags + * @lflags: bitmask of gpio_lookup_flags GPIO_* values * @dflags: GPIO flags used to specify the direction and value */ struct gpiod_hog { @@ -54,7 +54,7 @@ struct gpiod_hog { const char *chip_label; u16 chip_hwnum; const char *line_name; - enum gpio_lookup_flags lflags; + unsigned long lflags; int dflags; };
The library uses enum gpio_lookup_flags to define the possible characteristics of GPIO pin. Since enumerator listed only individual bits the common use of it is in a form of a bitmask of gpio_lookup_flags GPIO_* values. The more correct type for this is unsigned long. Due to above convert all users to use unsigned long instead of enum gpio_lookup_flags except enumerator definition. While here, make field and parameter descriptions consistent as well. Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpiolib-acpi.c | 14 +++++++++----- drivers/gpio/gpiolib-of.c | 11 +++++------ drivers/gpio/gpiolib.c | 13 ++++++------- drivers/gpio/gpiolib.h | 9 ++++----- include/linux/gpio/machine.h | 8 ++++---- 5 files changed, 28 insertions(+), 27 deletions(-)