gpio: Guard devm_* functions behind CONFIG_GPIO_DEVRES not CONFIG_GPIOLIB

PATCH In review Keno Fischer 2 participants 2 messages Nov 10, 2016 – Nov 15, 2016

Subsystems (2)

GPIO User-mode Linux (UML)

lore

Share & tools

lore

Keno Fischer

author

2016-11-10 01:56 UTC #0 lore raw

PATCH 1/1 gpio: Guard devm_* functions behind CONFIG_GPIO_DEVRES not CONFIG_GPIOLIB

These functions are defined in devres.c, which only gets compiled with CONFIG_GPIO_DEVRES (in addition to CONFIG_GPIOLIB). However, in the header files, the difference between the declaration and the inline stub was only guarded by CONFIG_GPIOLIB, not CONFIG_GPIO_DEVRES, causing undefined symbol problems in modpost.

Signed-off-by Keno Fischer <[email protected]>

---

I encountered this while trying to build uml in an attempt to debug some kernel behavior I don't understand. To be as close to my actual kernel as possible, I used the same .config, which of course tried to build a bunch of drivers. Arguably I should just not build those, but this seems correct nonetheless and allows the build to go through.

 include/linux/gpio.h          |  28 +++---
 include/linux/gpio/consumer.h | 193 ++++++++++++++++++++++--------------------
 2 files changed, 117 insertions(+), 104 deletions(-)

include/linux/gpio.h +18 ~~-10~~

index d12b5d5..7a1b979 100644
@@ -77,15 +77,6 @@ static inline int irq_to_gpio(unsigned int irq)
 
 #endif /* ! CONFIG_ARCH_HAVE_CUSTOM_GPIO_H */
 
-/* CONFIG_GPIOLIB: bindings for managed devices that want to request gpios */
-
-struct device;
-
-int devm_gpio_request(struct device *dev, unsigned gpio, const char *label);
-int devm_gpio_request_one(struct device *dev, unsigned gpio,
-			  unsigned long flags, const char *label);
-void devm_gpio_free(struct device *dev, unsigned int gpio);
-
 #else /* ! CONFIG_GPIOLIB */
 
 #include <linux/kernel.h>

@@ -253,6 +244,23 @@ gpiochip_remove_pin_ranges(struct gpio_chip *chip)
 	WARN_ON(1);
 }
 
+#endif /* ! CONFIG_GPIOLIB */
+
+#ifdef CONFIG_GPIO_DEVRES
+
+/* CONFIG_GPIOLIB: bindings for managed devices that want to request gpios */
+
+struct device;
+
+int devm_gpio_request(struct device *dev, unsigned gpio, const char *label);
+int devm_gpio_request_one(struct device *dev, unsigned gpio,
+			  unsigned long flags, const char *label);
+void devm_gpio_free(struct device *dev, unsigned int gpio);
+
+#else
+
+struct device;
+
 static inline int devm_gpio_request(struct device *dev, unsigned gpio,
 				    const char *label)
 {

@@ -272,6 +280,6 @@ static inline void devm_gpio_free(struct device *dev, unsigned int gpio)
 	WARN_ON(1);
 }
 
-#endif /* ! CONFIG_GPIOLIB */
+#endif
 
 #endif /* __LINUX_GPIO_H */

include/linux/gpio/consumer.h +99 ~~-94~~

221 lines not on this page · raw message

Signature · 1 line

2.9.3

Linus Walleij

2016-11-15 09:10 UTC #1 lore raw

Quoted 6 lines · Keno Fischer

These functions are defined in devres.c, which only gets compiled with CONFIG_GPIO_DEVRES (in addition to CONFIG_GPIOLIB). However, in the header files, the difference between the declaration and the inline stub was only guarded by CONFIG_GPIOLIB, not CONFIG_GPIO_DEVRES, causing undefined symbol problems in modpost.

Quoted 8 lines

Signed-off-by Keno Fischer <[email protected]>
---
I encountered this while trying to build uml in an attempt to debug some kernel behavior I don't understand. To be as close to my actual kernel as possible, I used the same .config, which of course tried to build a bunch of drivers. Arguably I should just not build those, but this seems correct nonetheless and allows the build to go through.

Ha! Good point. But it's the wrong solution.

The culprit is this:

if GPIOLIB

config GPIO_DEVRES
def_bool y
depends on HAS_IOMEM

But devres has no dependency on HAS_IOMEM whatsoever.

And certainly UML should have access to the devres variants of the calls.

The proper patch is to remove the Kconfig option GPIO_DEVRES altogether, make it compulsory, and remove *all* ifdefs for CONFIG_GPIO_DEVRES and alter the Makefile so that it is compiled-in for CONFIG_GPIOLIB.

Do you wanna take a stab at patching this or should I do it?

Yours,
Linus Walleij