Re: [PATCH v3 1/2] ARM: hip04: set ARCH_NR_GPIO to 128

Arnd Bergmann
2014-11-28T21:16:38Z

On Friday 28 November 2014 16:54:44 Linus Walleij wrote:
> On Fri, Nov 28, 2014 at 10:33 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > On Friday 28 November 2014 14:29:47 Zhou Wang wrote:
>
> >> Set ARCH_NR_GPIO for Hisilicon Soc Hip04, which has 4 GPIO
> >> controllers with 32 GPIOs each.
> >>
> >> Signed-off-by: Zhou Wang <wangzhou.bry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >> arch/arm/Kconfig | 1 +
> >> arch/arm/configs/hisi_defconfig | 3 +++
> >> 2 files changed, 4 insertions(+)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 89c4b5c..26aae1e 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -1509,6 +1509,7 @@ config ARCH_NR_GPIO
> >> default 352 if ARCH_VT8500
> >> default 288 if ARCH_ROCKCHIP
> >> default 264 if MACH_H4700
> >> + default 128 if ARCH_HIP04
> >> default 0
> >> help
> >> Maximum number of GPIOs in the system.
> >>
> >
> > If I remember correctly, you don't actually need to set this if all gpio
> > clients are using the new gpio descriptor interfaces instead of gpio
> > numbers.
>
> Unfortunately you still have to. We are working on removing the
> dependency on ARCH_NR_GPIO, old habits die hard.

Ok, I see.

> But I just
> merged this patch:
> http://marc.info/?l=linux-gpio&m=141638350328535&w=2
>
> Which makes the situation better.

Ah, cool!

> There is however some other use of this define, so there is
> some work required still to get rid of it.

These are the only ones I could find:

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 9f0682534e2f..fe05d8b375ca 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -224,9 +224,6 @@ static int davinci_gpio_probe(struct platform_device *pdev)
return -EINVAL;
}

- if (WARN_ON(ARCH_NR_GPIOS < ngpio))
- ngpio = ARCH_NR_GPIOS;
-
chips = devm_kzalloc(dev,
ngpio * sizeof(struct davinci_gpio_controller),
GFP_KERNEL);
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 746db6acf648..4f70024c2f9a 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -281,12 +281,12 @@ struct nmk_pinctrl {
void __iomem *prcm_base;
};

-static struct nmk_gpio_chip *
-nmk_gpio_chips[DIV_ROUND_UP(ARCH_NR_GPIOS, NMK_GPIO_PER_CHIP)];
+#define NUM_BANKS 9 /* increase if necessary */
+
+static struct nmk_gpio_chip *nmk_gpio_chips[NUM_BANKS];

static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);

-#define NUM_BANKS ARRAY_SIZE(nmk_gpio_chips)

static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
unsigned offset, int gpio_mode)
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 383ade1a211b..b53bcf5c58e7 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -13,23 +13,6 @@
#include <linux/gpio/driver.h>
#include <linux/gpio/consumer.h>

-/* Platforms may implement their GPIO interface with library code,
- * at a small performance cost for non-inlined operations and some
- * extra memory (for code and for per-GPIO table entries).
- *
- * While the GPIO programming interface defines valid GPIO numbers
- * to be in the range 0..MAX_INT, this library restricts them to the
- * smaller range 0..ARCH_NR_GPIOS-1.
- *
- * ARCH_NR_GPIOS is somewhat arbitrary; it usually reflects the sum of
- * builtin/SoC GPIOs plus a number of GPIOs on expanders; the latter is
- * actually an estimate of a board-specific value.
- */
-
-#ifndef ARCH_NR_GPIOS
-#define ARCH_NR_GPIOS 512
-#endif
-
/*
* "valid" GPIO numbers are nonnegative and may be passed to
* setup routines like gpio_request(). only some valid numbers
@@ -41,7 +24,11 @@

static inline bool gpio_is_valid(int number)
{
- return number >= 0 && number < ARCH_NR_GPIOS;
+#ifdef ARCH_NR_GPIOS
+ if (number > ARCH_NR_GPIOS)
+ return 0;
+#endif
+ return number >= 0;
}

struct device;

> > However if one builds a kernel that just enables OMAP4 and HIP04, I suspect
> > it can't work on OMAP4 for any gpio line above 128, which seems to be
> > a fundamental multiplatform problem.
>
> Yes I guess you are right :(
>
> It's probably just so that so many platforms converge on 512.
>
> > Do we neet to increase the default to 512 for all ARCH_MULTIPLATFORM
> > configurations and just leave ARCH_SHMOBILE, ARCH_TEGRA and MACH_H4700
> > here as special cases?
>
> That'd be good while we are working to kill off
> ARCH_NR_GPIO for good.
>
> I guess there could be arch-specific problems with trying
> to get rid of ARCH_NR_GPIO for good, do you have some
> input on this? (Arch maintainer hat on...)

I think killing off the hardcoded number as soon as possible is
best, but it doesn't seem appropriate for stable backports or 3.18,
so let's do the 512 default on arm32-multiplatform with stable
backports.

Arnd