| Message ID | 20230124013138.358595-1-pierluigi.p@variscite.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] gpiolib: fix linker errors when GPIOLIB is disabled | expand |
On Tue, Jan 24, 2023 at 2:31 AM Pierluigi Passaro <pierluigi.p@variscite.com> wrote: > > Both the functions gpiochip_request_own_desc and > gpiochip_free_own_desc are exported from > drivers/gpio/gpiolib.c > but this file is compiled only when CONFIG_GPIOLIB is enabled. > Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide > reasonable definitions and includes in the "#else" branch. > > Fixes: 9091373ab7ea ("gpio: remove less important #ifdef around declarations") > Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com> > Reported-by: kernel test robot <lkp@intel.com> > --- > Changes in v2: > - add Fixes tag > Changes in v3: > - add includes to fix builds against x86_64-defconfig > Hey Pierluigi! Thanks for the quick fix. When this happens - a bug report after a patch was applied - please generally submit a fix based on what's already in next, not another version. This time, I'll back it out of next because I have some comments on this, so please do send a v4. > include/linux/gpio/driver.h | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h > index 44783fc16125..e00eaba724dc 100644 > --- a/include/linux/gpio/driver.h > +++ b/include/linux/gpio/driver.h > @@ -758,6 +758,8 @@ gpiochip_remove_pin_ranges(struct gpio_chip *gc) > > #endif /* CONFIG_PINCTRL */ > > +#ifdef CONFIG_GPIOLIB > + > struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > unsigned int hwnum, > const char *label, > @@ -765,8 +767,6 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > enum gpiod_flags dflags); > void gpiochip_free_own_desc(struct gpio_desc *desc); > > -#ifdef CONFIG_GPIOLIB > - > /* lock/unlock as IRQ */ > int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset); > void gpiochip_unlock_as_irq(struct gpio_chip *gc, unsigned int offset); > @@ -776,6 +776,25 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc); > > #else /* CONFIG_GPIOLIB */ > > +#include <linux/gpio/machine.h> > +#include <linux/gpio/consumer.h> Please move those headers to the top and arrange them alphabetically with the rest of the <linux/ headers. Since you're now including those, remove any forward declarations of the types in question. Bart > + > +static inline struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > + unsigned int hwnum, > + const char *label, > + enum gpio_lookup_flags lflags, > + enum gpiod_flags dflags) > +{ > + /* GPIO can never have been requested */ > + WARN_ON(1); > + return ERR_PTR(-ENODEV); > +} > + > +static inline void gpiochip_free_own_desc(struct gpio_desc *desc) > +{ > + WARN_ON(1); > +} > + > static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc) > { > /* GPIO can never have been requested */ > -- > 2.34.1 >
On Tue, Jan 24, 2023 at 09:54:54AM +0100, Bartosz Golaszewski wrote: > On Tue, Jan 24, 2023 at 2:31 AM Pierluigi Passaro > <pierluigi.p@variscite.com> wrote: ... > > #else /* CONFIG_GPIOLIB */ > > > > +#include <linux/gpio/machine.h> > > +#include <linux/gpio/consumer.h> > > Please move those headers to the top and arrange them alphabetically > with the rest of the <linux/ headers. Since you're now including > those, remove any forward declarations of the types in question. That's not correct way. The headers will make the whole purpose of splitting (between driver, machine, and consumer) useless. Lemme look at it, but I believe the solution is not so simple. We need to clean up header inclusions as we did for pin control.
On Tue, Jan 24, 2023 at 02:31:38AM +0100, Pierluigi Passaro wrote: > Both the functions gpiochip_request_own_desc and > gpiochip_free_own_desc are exported from > drivers/gpio/gpiolib.c > but this file is compiled only when CONFIG_GPIOLIB is enabled. > Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide > reasonable definitions and includes in the "#else" branch. So far as a quick fix I can agree on the change with a caveat that inclusion hell should be untangled in the future. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> P.S. The root cause of this problem is that some drivers without being GPIO chips abuse this API. Better fix can be to go through each of such drivers and add ugly ifdeffery with a FIXME comment that this shouldn't be actually used to begin with. > Fixes: 9091373ab7ea ("gpio: remove less important #ifdef around declarations") > Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com> > Reported-by: kernel test robot <lkp@intel.com> > --- > Changes in v2: > - add Fixes tag > Changes in v3: > - add includes to fix builds against x86_64-defconfig > > include/linux/gpio/driver.h | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h > index 44783fc16125..e00eaba724dc 100644 > --- a/include/linux/gpio/driver.h > +++ b/include/linux/gpio/driver.h > @@ -758,6 +758,8 @@ gpiochip_remove_pin_ranges(struct gpio_chip *gc) > > #endif /* CONFIG_PINCTRL */ > > +#ifdef CONFIG_GPIOLIB > + > struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > unsigned int hwnum, > const char *label, > @@ -765,8 +767,6 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > enum gpiod_flags dflags); > void gpiochip_free_own_desc(struct gpio_desc *desc); > > -#ifdef CONFIG_GPIOLIB > - > /* lock/unlock as IRQ */ > int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset); > void gpiochip_unlock_as_irq(struct gpio_chip *gc, unsigned int offset); > @@ -776,6 +776,25 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc); > > #else /* CONFIG_GPIOLIB */ > > +#include <linux/gpio/machine.h> > +#include <linux/gpio/consumer.h> > + > +static inline struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, > + unsigned int hwnum, > + const char *label, > + enum gpio_lookup_flags lflags, > + enum gpiod_flags dflags) > +{ > + /* GPIO can never have been requested */ > + WARN_ON(1); > + return ERR_PTR(-ENODEV); > +} > + > +static inline void gpiochip_free_own_desc(struct gpio_desc *desc) > +{ > + WARN_ON(1); > +} > + > static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc) > { > /* GPIO can never have been requested */ > -- > 2.34.1 > >
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 44783fc16125..e00eaba724dc 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -758,6 +758,8 @@ gpiochip_remove_pin_ranges(struct gpio_chip *gc) #endif /* CONFIG_PINCTRL */ +#ifdef CONFIG_GPIOLIB + struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, unsigned int hwnum, const char *label, @@ -765,8 +767,6 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, enum gpiod_flags dflags); void gpiochip_free_own_desc(struct gpio_desc *desc); -#ifdef CONFIG_GPIOLIB - /* lock/unlock as IRQ */ int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset); void gpiochip_unlock_as_irq(struct gpio_chip *gc, unsigned int offset); @@ -776,6 +776,25 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc); #else /* CONFIG_GPIOLIB */ +#include <linux/gpio/machine.h> +#include <linux/gpio/consumer.h> + +static inline struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc, + unsigned int hwnum, + const char *label, + enum gpio_lookup_flags lflags, + enum gpiod_flags dflags) +{ + /* GPIO can never have been requested */ + WARN_ON(1); + return ERR_PTR(-ENODEV); +} + +static inline void gpiochip_free_own_desc(struct gpio_desc *desc) +{ + WARN_ON(1); +} + static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc) { /* GPIO can never have been requested */
Both the functions gpiochip_request_own_desc and gpiochip_free_own_desc are exported from drivers/gpio/gpiolib.c but this file is compiled only when CONFIG_GPIOLIB is enabled. Move the prototypes under "#ifdef CONFIG_GPIOLIB" and provide reasonable definitions and includes in the "#else" branch. Fixes: 9091373ab7ea ("gpio: remove less important #ifdef around declarations") Signed-off-by: Pierluigi Passaro <pierluigi.p@variscite.com> Reported-by: kernel test robot <lkp@intel.com> --- Changes in v2: - add Fixes tag Changes in v3: - add includes to fix builds against x86_64-defconfig include/linux/gpio/driver.h | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)