| Message ID | 20211105130338.241100-3-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/3] gpiolib: remove irq_to_gpio() definition | expand |
Hi Arnd, On Fri, Nov 5, 2021 at 2:05 PM Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Now that coldfire is the only user of a custom asm/gpio.h, it seems > better to remove this as well, and have the same interface everywhere. > > For the gpio_get_value()/gpio_set_value()/gpio_to_irq(), gpio_cansleep() > functions, the custom version is only a micro-optimization to inline the > function for constant GPIO numbers. However, in the coldfire defconfigs, > I was unable to find a single instance where this micro-optimization > was even used, so to my best knowledge removing this has no downsides. The only user seems to be QSPI chip select handling (not bit-banged data transfer) in arch/m68k/coldfire/device.c, but that indeed depends on CONFIG_SPI_COLDFIRE_QSPI, which is not set in any of the defconfigs. That doesn't mean there were/are no real users, though ;-) > The custom gpio_request_one() function is even less useful, as it is > guarded by an #ifdef that is never true. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi arnd, Geert, On 8/11/21 6:24 pm, Geert Uytterhoeven wrote: > Hi Arnd, > > On Fri, Nov 5, 2021 at 2:05 PM Arnd Bergmann <arnd@kernel.org> wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> Now that coldfire is the only user of a custom asm/gpio.h, it seems >> better to remove this as well, and have the same interface everywhere. >> >> For the gpio_get_value()/gpio_set_value()/gpio_to_irq(), gpio_cansleep() >> functions, the custom version is only a micro-optimization to inline the >> function for constant GPIO numbers. However, in the coldfire defconfigs, >> I was unable to find a single instance where this micro-optimization >> was even used, so to my best knowledge removing this has no downsides. > > The only user seems to be QSPI chip select handling (not bit-banged > data transfer) in arch/m68k/coldfire/device.c, but that indeed depends > on CONFIG_SPI_COLDFIRE_QSPI, which is not set in any of the defconfigs. > That doesn't mean there were/are no real users, though ;-) That is definitely used by some. But the generalization and removal of the special casing seems like a win to me. >> The custom gpio_request_one() function is even less useful, as it is >> guarded by an #ifdef that is never true. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Reviewed-by: Greg Ungerer <gerg@linux-m68k.org> Regards Greg > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds >
On Fri, Nov 5, 2021 at 2:05 PM Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Now that coldfire is the only user of a custom asm/gpio.h, it seems > better to remove this as well, and have the same interface everywhere. > > For the gpio_get_value()/gpio_set_value()/gpio_to_irq(), gpio_cansleep() > functions, the custom version is only a micro-optimization to inline the > function for constant GPIO numbers. However, in the coldfire defconfigs, > I was unable to find a single instance where this micro-optimization > was even used, so to my best knowledge removing this has no downsides. > > The custom gpio_request_one() function is even less useful, as it is > guarded by an #ifdef that is never true. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
diff --git a/arch/m68k/Kconfig.cpu b/arch/m68k/Kconfig.cpu index 0d00ef5117dc..8256ff6b5b87 100644 --- a/arch/m68k/Kconfig.cpu +++ b/arch/m68k/Kconfig.cpu @@ -24,7 +24,6 @@ config M68KCLASSIC config COLDFIRE bool "Coldfire CPU family support" - select ARCH_HAVE_CUSTOM_GPIO_H select CPU_HAS_NO_BITFIELDS select CPU_HAS_NO_CAS select CPU_HAS_NO_MULDIV64 diff --git a/arch/m68k/include/asm/gpio.h b/arch/m68k/include/asm/gpio.h deleted file mode 100644 index 5cfc0996ba94..000000000000 --- a/arch/m68k/include/asm/gpio.h +++ /dev/null @@ -1,95 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Coldfire generic GPIO support - * - * (C) Copyright 2009, Steven King <sfking@fdwdc.com> -*/ - -#ifndef coldfire_gpio_h -#define coldfire_gpio_h - -#include <linux/io.h> -#include <asm/coldfire.h> -#include <asm/mcfsim.h> -#include <asm/mcfgpio.h> -/* - * The Generic GPIO functions - * - * If the gpio is a compile time constant and is one of the Coldfire gpios, - * use the inline version, otherwise dispatch thru gpiolib. - */ - -static inline int gpio_get_value(unsigned gpio) -{ - if (__builtin_constant_p(gpio) && gpio < MCFGPIO_PIN_MAX) - return mcfgpio_read(__mcfgpio_ppdr(gpio)) & mcfgpio_bit(gpio); - else - return __gpio_get_value(gpio); -} - -static inline void gpio_set_value(unsigned gpio, int value) -{ - if (__builtin_constant_p(gpio) && gpio < MCFGPIO_PIN_MAX) { - if (gpio < MCFGPIO_SCR_START) { - unsigned long flags; - MCFGPIO_PORTTYPE data; - - local_irq_save(flags); - data = mcfgpio_read(__mcfgpio_podr(gpio)); - if (value) - data |= mcfgpio_bit(gpio); - else - data &= ~mcfgpio_bit(gpio); - mcfgpio_write(data, __mcfgpio_podr(gpio)); - local_irq_restore(flags); - } else { - if (value) - mcfgpio_write(mcfgpio_bit(gpio), - MCFGPIO_SETR_PORT(gpio)); - else - mcfgpio_write(~mcfgpio_bit(gpio), - MCFGPIO_CLRR_PORT(gpio)); - } - } else - __gpio_set_value(gpio, value); -} - -static inline int gpio_to_irq(unsigned gpio) -{ -#if defined(MCFGPIO_IRQ_MIN) - if ((gpio >= MCFGPIO_IRQ_MIN) && (gpio < MCFGPIO_IRQ_MAX)) -#else - if (gpio < MCFGPIO_IRQ_MAX) -#endif - return gpio + MCFGPIO_IRQ_VECBASE; - else - return __gpio_to_irq(gpio); -} - -static inline int gpio_cansleep(unsigned gpio) -{ - return gpio < MCFGPIO_PIN_MAX ? 0 : __gpio_cansleep(gpio); -} - -#ifndef CONFIG_GPIOLIB -static inline int gpio_request_one(unsigned gpio, unsigned long flags, const char *label) -{ - int err; - - err = gpio_request(gpio, label); - if (err) - return err; - - if (flags & GPIOF_DIR_IN) - err = gpio_direction_input(gpio); - else - err = gpio_direction_output(gpio, - (flags & GPIOF_INIT_HIGH) ? 1 : 0); - - if (err) - gpio_free(gpio); - - return err; -} -#endif /* !CONFIG_GPIOLIB */ -#endif diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 9ef5d3fdd405..5b233e69a98f 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -3,14 +3,6 @@ # GPIO infrastructure and drivers # -config ARCH_HAVE_CUSTOM_GPIO_H - bool - help - Selecting this config option from the architecture Kconfig allows - the architecture to provide a custom asm/gpio.h implementation - overriding the default implementations. New uses of this are - strongly discouraged. - menuconfig GPIOLIB bool "GPIO Support" help diff --git a/include/linux/gpio.h b/include/linux/gpio.h index d8d7daa7eb94..7e6b1b8277ca 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -54,11 +54,6 @@ struct gpio { }; #ifdef CONFIG_GPIOLIB - -#ifdef CONFIG_ARCH_HAVE_CUSTOM_GPIO_H -#include <asm/gpio.h> -#else - #include <asm-generic/gpio.h> static inline int gpio_get_value(unsigned int gpio) @@ -81,8 +76,6 @@ static inline int gpio_to_irq(unsigned int gpio) return __gpio_to_irq(gpio); } -#endif /* ! CONFIG_ARCH_HAVE_CUSTOM_GPIO_H */ - /* CONFIG_GPIOLIB: bindings for managed devices that want to request gpios */ struct device;