[PATCH 0/4] clean up irq_to_gpio and asm/gpio.h

I noticed that arch/arm/include/asm/gpio.h can almost be removed,
after we have already removed the file for most other architectures
now.

When I removed it, I ran into problems with irq_to_gpio(), which
we had already killed off in ARM, but it survived (barely) in the
global headers. this kills it off some more as well.

I'd suggest merging all four patches through the GPIO tree.

	Arnd

gpiolib has removed the irq_to_gpio() API several years ago,
but the global header still provided a non-working stub.

Apparently one new user has shown up in arch/mips, so this patch
moves the broken definition to where it is used, ensuring that
we get new users but not changing the current behavior on jz4740.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/mips/jz4740/gpio.c |  7 +++++++
 include/linux/gpio.h    | 12 ------------
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/mips/jz4740/gpio.c b/arch/mips/jz4740/gpio.c
index 8c6d76c9b2d6..e9bb43714892 100644
--- a/arch/mips/jz4740/gpio.c
+++ b/arch/mips/jz4740/gpio.c

@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/compiler.h>
 
 #include <linux/io.h>
 #include <linux/gpio.h>

@@ -270,6 +271,12 @@ uint32_t jz_gpio_port_get_value(int port, uint32_t mask)
 }
 EXPORT_SYMBOL(jz_gpio_port_get_value);
 
+static inline __deprecated int irq_to_gpio(unsigned int irq)
+{
+	/* this has clearly not worked for a long time */
+	return -EINVAL;
+}
+
 #define IRQ_TO_BIT(irq) BIT(irq_to_gpio(irq) & 0x1f)
 
 static void jz_gpio_check_trigger_both(struct jz_gpio_chip *chip, unsigned int irq)

diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index d12b5d566e4b..6fc1c9e74854 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h

@@ -70,11 +70,6 @@ static inline int gpio_to_irq(unsigned int gpio)
 	return __gpio_to_irq(gpio);
 }
 
-static inline int irq_to_gpio(unsigned int irq)
-{
-	return -EINVAL;
-}
-
 #endif /* ! CONFIG_ARCH_HAVE_CUSTOM_GPIO_H */
 
 /* CONFIG_GPIOLIB: bindings for managed devices that want to request gpios */

@@ -222,13 +217,6 @@ static inline void gpiochip_unlock_as_irq(struct gpio_chip *chip,
 	WARN_ON(1);
 }
 
-static inline int irq_to_gpio(unsigned irq)
-{
-	/* irq can never have been returned from gpio_to_irq() */
-	WARN_ON(1);
-	return -EINVAL;
-}
-
 static inline int
 gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
 		       unsigned int gpio_offset, unsigned int pin_offset,

-- 
2.7.0

On 02/15/2016 04:46 PM, Arnd Bergmann wrote:

+static inline __deprecated int irq_to_gpio(unsigned int irq)
+{
+	/* this has clearly not worked for a long time */
+	return -EINVAL;
+}
+
 #define IRQ_TO_BIT(irq) BIT(irq_to_gpio(irq) & 0x1f)

The issue seems to be a fallout from commit 832f5dacfa0b ("MIPS: Remove all
the uses of custom gpio.h").

The irq_to_gpio() should be replaced with "(irq - JZ4740_IRQ_GPIO(0))".

- Lars

On Monday 15 February 2016 17:01:09 Lars-Peter Clausen wrote:

On 02/15/2016 04:46 PM, Arnd Bergmann wrote:

quoted

+static inline __deprecated int irq_to_gpio(unsigned int irq)
+{
+     /* this has clearly not worked for a long time */
+     return -EINVAL;
+}
+
 #define IRQ_TO_BIT(irq) BIT(irq_to_gpio(irq) & 0x1f)

The issue seems to be a fallout from commit 832f5dacfa0b ("MIPS: Remove all
the uses of custom gpio.h").

The irq_to_gpio() should be replaced with "(irq - JZ4740_IRQ_GPIO(0))".

Ah, that explain it, so it has not been broken for that long.

	Arnd

On Mon, Feb 15, 2016 at 4:46 PM, Arnd Bergmann [off-list ref] wrote:

gpiolib has removed the irq_to_gpio() API several years ago,
but the global header still provided a non-working stub.

Apparently one new user has shown up in arch/mips, so this patch
moves the broken definition to where it is used, ensuring that
we get new users but not changing the current behavior on jz4740.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.

I expect the driver maintainer to deal with the resulting
deprecation fallout.

Yours,
Linus Walleij

On Tuesday 16 February 2016 16:42:18 Linus Walleij wrote:

On Mon, Feb 15, 2016 at 4:46 PM, Arnd Bergmann [off-list ref] wrote:

quoted

gpiolib has removed the irq_to_gpio() API several years ago,
but the global header still provided a non-working stub.

Apparently one new user has shown up in arch/mips, so this patch
moves the broken definition to where it is used, ensuring that
we get new users but not changing the current behavior on jz4740.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.

I expect the driver maintainer to deal with the resulting
deprecation fallout.

I've just sent v2 of the series, with a separate patch for MIPS
that now conflicts with this one. Can you pick up the new patch
instead?

	Arnd

On Tue, Feb 16, 2016 at 4:45 PM, Arnd Bergmann [off-list ref] wrote:

I've just sent v2 of the series, with a separate patch for MIPS
that now conflicts with this one. Can you pick up the new patch
instead?

OK I'll back out and apply the v2 versions.

Yours,
Linus Walleij

The ks8695 gpio driver has its own copy of the irq_to_gpio()
function. This is completely unused in the mainline kernel
after we converted all remaining users several years ago,
so we can remove the definition as well.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpio/gpio-ks8695.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpio/gpio-ks8695.c b/drivers/gpio/gpio-ks8695.c
index 9f86ed9c753b..179723d02f55 100644
--- a/drivers/gpio/gpio-ks8695.c
+++ b/drivers/gpio/gpio-ks8695.c

@@ -205,18 +205,6 @@ static int ks8695_gpio_to_irq(struct gpio_chip *gc, unsigned int pin)
 	return gpio_irq[pin];
 }
 
-/*
- * Map IRQ number to GPIO line.
- */
-int irq_to_gpio(unsigned int irq)
-{
-	if ((irq < KS8695_IRQ_EXTERN0) || (irq > KS8695_IRQ_EXTERN3))
-		return -EINVAL;
-
-	return (irq - KS8695_IRQ_EXTERN0);
-}
-EXPORT_SYMBOL(irq_to_gpio);
-
 /* GPIOLIB interface */
 
 static struct gpio_chip ks8695_gpio_chip = {

-- 
2.7.0

On Mon, Feb 15, 2016 at 4:46 PM, Arnd Bergmann [off-list ref] wrote:

The ks8695 gpio driver has its own copy of the irq_to_gpio()
function. This is completely unused in the mainline kernel
after we converted all remaining users several years ago,
so we can remove the definition as well.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.

Yours,
Linus Walleij

The ep93xx goes through its own back-and-forth dance every time
it wants to know the gpio number for an irq line, when it really
just hardcodes a fixed offset in ep93xx_gpio_to_irq().

This removes the pointless macro and replaces the conversion inside
of the driver with simple add/subtract operations, using an
explicit macro.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpio/gpio-ep93xx.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index 20e5846bda28..cd83d30e8ff7 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c

@@ -18,12 +18,8 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/slab.h>
-#include <linux/gpio/driver.h>
-/* FIXME: this is here for gpio_to_irq() - get rid of this! */
 #include <linux/gpio.h>
 
-#define irq_to_gpio(irq)	((irq) - gpio_to_irq(0))
-
 void __iomem *ep93xx_gpio_base; /* FIXME: put this into irq_data */
 
 #define EP93XX_GPIO_REG(x)		(ep93xx_gpio_base + (x))

@@ -35,6 +31,7 @@ void __iomem *ep93xx_gpio_base; /* FIXME: put this into irq_data */
 #define EP93XX_GPIO_LINE_MAX		63
 
 /* maximum value for irq capable line identifiers */
+#define EP93XX_GPIO_IRQ_BASE		64
 #define EP93XX_GPIO_LINE_MAX_IRQ	23
 
 

@@ -77,7 +74,7 @@ static void ep93xx_gpio_update_int_params(unsigned port)
 
 static void ep93xx_gpio_int_debounce(unsigned int irq, bool enable)
 {
-	int line = irq_to_gpio(irq);
+	int line = irq - EP93XX_GPIO_IRQ_BASE;
 	int port = line >> 3;
 	int port_mask = 1 << (line & 7);
 

@@ -98,7 +95,7 @@ static void ep93xx_gpio_ab_irq_handler(struct irq_desc *desc)
 	status = readb(EP93XX_GPIO_A_INT_STATUS);
 	for (i = 0; i < 8; i++) {
 		if (status & (1 << i)) {
-			int gpio_irq = gpio_to_irq(0) + i;
+			int gpio_irq = EP93XX_GPIO_IRQ_BASE + i;
 			generic_handle_irq(gpio_irq);
 		}
 	}

@@ -106,7 +103,7 @@ static void ep93xx_gpio_ab_irq_handler(struct irq_desc *desc)
 	status = readb(EP93XX_GPIO_B_INT_STATUS);
 	for (i = 0; i < 8; i++) {
 		if (status & (1 << i)) {
-			int gpio_irq = gpio_to_irq(8) + i;
+			int gpio_irq = EP93XX_GPIO_IRQ_BASE + 8 + i;
 			generic_handle_irq(gpio_irq);
 		}
 	}

@@ -121,14 +118,14 @@ static void ep93xx_gpio_f_irq_handler(struct irq_desc *desc)
 	 */
 	unsigned int irq = irq_desc_get_irq(desc);
 	int port_f_idx = ((irq + 1) & 7) ^ 4; /* {19..22,47..50} -> {0..7} */
-	int gpio_irq = gpio_to_irq(16) + port_f_idx;
+	int gpio_irq = EP93XX_GPIO_IRQ_BASE + 16 + port_f_idx;
 
 	generic_handle_irq(gpio_irq);
 }
 
 static void ep93xx_gpio_irq_ack(struct irq_data *d)
 {
-	int line = irq_to_gpio(d->irq);
+	int line = d->irq - EP93XX_GPIO_IRQ_BASE;
 	int port = line >> 3;
 	int port_mask = 1 << (line & 7);
 

@@ -142,7 +139,7 @@ static void ep93xx_gpio_irq_ack(struct irq_data *d)
 
 static void ep93xx_gpio_irq_mask_ack(struct irq_data *d)
 {
-	int line = irq_to_gpio(d->irq);
+	int line = d->irq - EP93XX_GPIO_IRQ_BASE;
 	int port = line >> 3;
 	int port_mask = 1 << (line & 7);
 

@@ -157,7 +154,7 @@ static void ep93xx_gpio_irq_mask_ack(struct irq_data *d)
 
 static void ep93xx_gpio_irq_mask(struct irq_data *d)
 {
-	int line = irq_to_gpio(d->irq);
+	int line = d->irq - EP93XX_GPIO_IRQ_BASE;
 	int port = line >> 3;
 
 	gpio_int_unmasked[port] &= ~(1 << (line & 7));

@@ -166,7 +163,7 @@ static void ep93xx_gpio_irq_mask(struct irq_data *d)
 
 static void ep93xx_gpio_irq_unmask(struct irq_data *d)
 {
-	int line = irq_to_gpio(d->irq);
+	int line = d->irq - EP93XX_GPIO_IRQ_BASE;
 	int port = line >> 3;
 
 	gpio_int_unmasked[port] |= 1 << (line & 7);

@@ -180,7 +177,7 @@ static void ep93xx_gpio_irq_unmask(struct irq_data *d)
  */
 static int ep93xx_gpio_irq_type(struct irq_data *d, unsigned int type)
 {
-	const int gpio = irq_to_gpio(d->irq);
+	const int gpio = d->irq - EP93XX_GPIO_IRQ_BASE;
 	const int port = gpio >> 3;
 	const int port_mask = 1 << (gpio & 7);
 	irq_flow_handler_t handler;

@@ -241,14 +238,14 @@ static struct irq_chip ep93xx_gpio_irq_chip = {
 
 static void ep93xx_gpio_init_irq(struct platform_device *pdev)
 {
-	int gpio_irq;
+	int gpio;
 	int i;
 
-	for (gpio_irq = gpio_to_irq(0);
-	     gpio_irq <= gpio_to_irq(EP93XX_GPIO_LINE_MAX_IRQ); ++gpio_irq) {
-		irq_set_chip_and_handler(gpio_irq, &ep93xx_gpio_irq_chip,
+	for (gpio = 0; gpio <= EP93XX_GPIO_LINE_MAX_IRQ; ++gpio) {
+		irq_set_chip_and_handler(EP93XX_GPIO_IRQ_BASE + gpio,
+					 &ep93xx_gpio_irq_chip,
 					 handle_level_irq);
-		irq_clear_status_flags(gpio_irq, IRQ_NOREQUEST);
+		irq_clear_status_flags(EP93XX_GPIO_IRQ_BASE + gpio, IRQ_NOREQUEST);
 	}
 
 	irq_set_chained_handler(platform_get_irq(pdev, 0),

@@ -294,7 +291,7 @@ static int ep93xx_gpio_set_debounce(struct gpio_chip *chip,
 				    unsigned offset, unsigned debounce)
 {
 	int gpio = chip->base + offset;
-	int irq = gpio_to_irq(gpio);
+	int irq = EP93XX_GPIO_IRQ_BASE + gpio;
 
 	if (irq < 0)
 		return -EINVAL;

@@ -316,7 +313,7 @@ static int ep93xx_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 	if (gpio > EP93XX_GPIO_LINE_MAX_IRQ)
 		return -EINVAL;
 
-	return 64 + gpio;
+	return EP93XX_GPIO_IRQ_BASE + gpio;
 }
 
 static int ep93xx_gpio_add_bank(struct gpio_chip *gc, struct device *dev,

-- 
2.7.0

On Mon, Feb 15, 2016 at 4:46 PM, Arnd Bergmann [off-list ref] wrote:

The ep93xx goes through its own back-and-forth dance every time
it wants to know the gpio number for an irq line, when it really
just hardcodes a fixed offset in ep93xx_gpio_to_irq().

This removes the pointless macro and replaces the conversion inside
of the driver with simple add/subtract operations, using an
explicit macro.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

(...)

-#include <linux/gpio/driver.h>
-/* FIXME: this is here for gpio_to_irq() - get rid of this! */
 #include <linux/gpio.h>

No, what you want to get rid of is the #include <linux/gpio.h> not the
<linux/gpio/driver.h>, we want to be more specific. Sorry for putting the
comment wrong.

This doesn't apply to my tree :O

My EP93XX_GPIO_LINE_MAX_IRQ is in
arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h

So I think you must be having another patch somewhere in the ARM SoC
tree. (I can take it, just send it along with an update of this one.)

Yours,
Linus Walleij

On Tuesday 16 February 2016 16:51:57 Linus Walleij wrote:

On Mon, Feb 15, 2016 at 4:46 PM, Arnd Bergmann [off-list ref] wrote:

quoted

The ep93xx goes through its own back-and-forth dance every time
it wants to know the gpio number for an irq line, when it really
just hardcodes a fixed offset in ep93xx_gpio_to_irq().

This removes the pointless macro and replaces the conversion inside
of the driver with simple add/subtract operations, using an
explicit macro.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

(...)

quoted

-#include <linux/gpio/driver.h>
-/* FIXME: this is here for gpio_to_irq() - get rid of this! */
 #include <linux/gpio.h>

No, what you want to get rid of is the #include <linux/gpio.h> not the
<linux/gpio/driver.h>, we want to be more specific. Sorry for putting the
comment wrong.

I think I initially did that, but ran into another build error then.

quoted

@@ -35,6 +31,7 @@ void __iomem *ep93xx_gpio_base; /* FIXME: put this into irq_data */
 #define EP93XX_GPIO_LINE_MAX           63

 /* maximum value for irq capable line identifiers */
+#define EP93XX_GPIO_IRQ_BASE           64
 #define EP93XX_GPIO_LINE_MAX_IRQ       23

This doesn't apply to my tree :O

My EP93XX_GPIO_LINE_MAX_IRQ is in
arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h

So I think you must be having another patch somewhere in the ARM SoC
tree. (I can take it, just send it along with an update of this one.)

Ah, that is right. Let's just drop this patch for now,
it's not important.

I have an old patch below to make the driver independent of
the headers, but I have to look at that patch again before
submitting it. I'll sort this one next to it.

	Arnd

The ARM version of asm/gpio.h basically just contains the same definitions
as the gpiolib version, with the exception of ARCH_NR_GPIOS.

This adds the option for overriding the constant through Kconfig to
the architecture-independent header, and removes the ARM version at
the same time.

This patch depends on a series done by Bjorn Helgaas, and the patch
removing irq_to_gpio().

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/Kconfig            |  1 -
 arch/arm/include/asm/gpio.h | 25 -------------------------
 include/asm-generic/gpio.h  |  4 ++++
 3 files changed, 4 insertions(+), 26 deletions(-)
 delete mode 100644 arch/arm/include/asm/gpio.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0366ad63e2ed..27aefbe43eea 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig

@@ -5,7 +5,6 @@ config ARM
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
-	select ARCH_HAVE_CUSTOM_GPIO_H
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_SUPPORTS_ATOMIC_RMW

diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
deleted file mode 100644
index 504dcddebfcc..000000000000
--- a/arch/arm/include/asm/gpio.h
+++ /dev/null

@@ -1,25 +0,0 @@
-#ifndef _ARCH_ARM_GPIO_H
-#define _ARCH_ARM_GPIO_H
-
-#if CONFIG_ARCH_NR_GPIO > 0
-#define ARCH_NR_GPIOS CONFIG_ARCH_NR_GPIO
-#endif
-
-/* Note: this may rely upon the value of ARCH_NR_GPIOS set in mach/gpio.h */
-#include <asm-generic/gpio.h>
-
-/* The trivial gpiolib dispatchers */
-#define gpio_get_value  __gpio_get_value
-#define gpio_set_value  __gpio_set_value
-#define gpio_cansleep   __gpio_cansleep
-
-/*
- * Provide a default gpio_to_irq() which should satisfy every case.
- * However, some platforms want to do this differently, so allow them
- * to override it.
- */
-#ifndef gpio_to_irq
-#define gpio_to_irq	__gpio_to_irq
-#endif
-
-#endif /* _ARCH_ARM_GPIO_H */

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 40ec1433f05d..2d04b321db13 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h

@@ -26,8 +26,12 @@
  */
 
 #ifndef ARCH_NR_GPIOS
+#ifdef CONFIG_ARCH_NR_GPIO
+#define ARCH_NR_GPIOS CONFIG_ARCH_NR_GPIO
+#else
 #define ARCH_NR_GPIOS		512
 #endif
+#endif
 
 /*
  * "valid" GPIO numbers are nonnegative and may be passed to

-- 
2.7.0

On Mon, Feb 15, 2016 at 04:46:31PM +0100, Arnd Bergmann wrote:

-#if CONFIG_ARCH_NR_GPIO > 0
-#define ARCH_NR_GPIOS CONFIG_ARCH_NR_GPIO
-#endif

...

This really isn't the same thing, especially when you look at:

config ARCH_NR_GPIO
        int
        default 1024 if ARCH_BRCMSTB || ARCH_SHMOBILE || ARCH_TEGRA || \
                ARCH_ZYNQ
        default 512 if ARCH_EXYNOS || ARCH_KEYSTONE || SOC_OMAP5 || \
                SOC_DRA7XX || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210
        default 416 if ARCH_SUNXI
        default 392 if ARCH_U8500
        default 352 if ARCH_VT8500
        default 288 if ARCH_ROCKCHIP
        default 264 if MACH_H4700
        default 0

This has the effect of causing anything not mentioned above to end up
with ARCH_NR_GPIOS set to zero.

You really should _not_ be making these kinds of functional changes
when moving code around.  Separate the two changes: either first move
the code, and have a separate patch to make the functional changes, or
make the functional changes first, and then move the code around.

I'd have thought you would've known this by now...

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.