2022-08-29 16:21:04

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS

Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array")
there is no limitation on the number of GPIOs that can be allocated
in the system since the allocation is fully dynamic.

ARCH_NR_GPIOS is today only used in order to provide downwards
gpiobase allocation from that value, while static allocation is
performed upwards from 0. However that has the disadvantage of
limiting the number of GPIOs that can be registered in the system.

To overcome this limitation without requiring each and every
platform to provide its 'best-guess' maximum number, rework the
allocation to allocate upwards, allowing approx 2 millions of
GPIOs.

In order to still allow static allocation for legacy drivers, define
GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic
allocation.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/arm/include/asm/gpio.h | 1 -
drivers/gpio/gpio-sta2x11.c | 5 ++--
drivers/gpio/gpiolib.c | 10 +++----
include/asm-generic/gpio.h | 55 ++++++++++++++-----------------------
4 files changed, 28 insertions(+), 43 deletions(-)

diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
index f3bb8a2bf788..4ebbb58f06ea 100644
--- a/arch/arm/include/asm/gpio.h
+++ b/arch/arm/include/asm/gpio.h
@@ -2,7 +2,6 @@
#ifndef _ARCH_ARM_GPIO_H
#define _ARCH_ARM_GPIO_H

-/* 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 */
diff --git a/drivers/gpio/gpio-sta2x11.c b/drivers/gpio/gpio-sta2x11.c
index e07cca0f8d35..1206b2877caa 100644
--- a/drivers/gpio/gpio-sta2x11.c
+++ b/drivers/gpio/gpio-sta2x11.c
@@ -108,8 +108,8 @@ static void gsta_gpio_setup(struct gsta_gpio *chip) /* called from probe */
struct gpio_chip *gpio = &chip->gpio;

/*
- * ARCH_NR_GPIOS is currently 256 and dynamic allocation starts
- * from the end. However, for compatibility, we need the first
+ * Dynamic allocation starts at GPIO_DYNAMIC_BASE.
+ * However, for compatibility, we need the first
* ConneXt device to start from gpio 0: it's the main chipset
* on most boards so documents and drivers assume gpio0..gpio127
*/
@@ -129,7 +129,6 @@ static void gsta_gpio_setup(struct gsta_gpio *chip) /* called from probe */

/*
* After the first device, turn to dynamic gpio numbers.
- * For example, with ARCH_NR_GPIOS = 256 we can fit two cards
*/
if (!gpio_base)
gpio_base = -1;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3a6f29eeb72d..40ebc3d42a78 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -183,14 +183,14 @@ EXPORT_SYMBOL_GPL(gpiod_to_chip);
static int gpiochip_find_base(int ngpio)
{
struct gpio_device *gdev;
- int base = ARCH_NR_GPIOS - ngpio;
+ int base = GPIO_DYNAMIC_BASE;

- list_for_each_entry_reverse(gdev, &gpio_devices, list) {
+ list_for_each_entry(gdev, &gpio_devices, list) {
/* found a free space? */
- if (gdev->base + gdev->ngpio <= base)
+ if (gdev->base >= base + ngpio)
break;
- /* nope, check the space right before the chip */
- base = gdev->base - ngpio;
+ /* nope, check the space right after the chip */
+ base = gdev->base + gdev->ngpio;
}

if (gpio_is_valid(base)) {
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index aea9aee1f3e9..06a817d9de79 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -11,40 +11,18 @@
#include <linux/gpio/driver.h>
#include <linux/gpio/consumer.h>

-/* Platforms may implement their GPIO interface with library code,
+/*
+ * 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
-#if defined(CONFIG_ARCH_NR_GPIO) && CONFIG_ARCH_NR_GPIO > 0
-#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
- * setup routines like gpio_request(). only some valid numbers
- * can successfully be requested and used.
- *
- * Invalid GPIO numbers are useful for indicating no-such-GPIO in
- * platform data and other tables.
+ * At the end we want all GPIOs to be dynamically allocated from 0.
+ * However, some legacy drivers still perform fixed allocation.
+ * Until they are all fixed, leave 0-256 space for them.
*/
-
-static inline bool gpio_is_valid(int number)
-{
- return number >= 0 && number < ARCH_NR_GPIOS;
-}
+#define GPIO_DYNAMIC_BASE 256

struct device;
struct gpio;
@@ -140,12 +118,6 @@ static inline void gpio_unexport(unsigned gpio)

#include <linux/kernel.h>

-static inline bool gpio_is_valid(int number)
-{
- /* only non-negative numbers are valid */
- return number >= 0;
-}
-
/* platforms that don't directly support access to GPIOs through I2C, SPI,
* or other blocking infrastructure can use these wrappers.
*/
@@ -169,4 +141,19 @@ static inline void gpio_set_value_cansleep(unsigned gpio, int value)

#endif /* !CONFIG_GPIOLIB */

+/*
+ * "valid" GPIO numbers are nonnegative and may be passed to
+ * setup routines like gpio_request(). only some valid numbers
+ * can successfully be requested and used.
+ *
+ * Invalid GPIO numbers are useful for indicating no-such-GPIO in
+ * platform data and other tables.
+ */
+
+static inline bool gpio_is_valid(int number)
+{
+ /* only non-negative numbers are valid */
+ return number >= 0;
+}
+
#endif /* _ASM_GENERIC_GPIO_H */
--
2.37.1


2022-08-30 20:31:57

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS

On Mon, Aug 29, 2022 at 7:19 PM Christophe Leroy
<[email protected]> wrote:
>
> Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array")
> there is no limitation on the number of GPIOs that can be allocated
> in the system since the allocation is fully dynamic.
>
> ARCH_NR_GPIOS is today only used in order to provide downwards
> gpiobase allocation from that value, while static allocation is
> performed upwards from 0. However that has the disadvantage of
> limiting the number of GPIOs that can be registered in the system.
>
> To overcome this limitation without requiring each and every
> platform to provide its 'best-guess' maximum number, rework the
> allocation to allocate upwards, allowing approx 2 millions of
> GPIOs.
>
> In order to still allow static allocation for legacy drivers, define
> GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic
> allocation.

Not sure about 256, but I understand that this can only be the best guess.

--
With Best Regards,
Andy Shevchenko

2022-08-31 05:52:10

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS



Le 30/08/2022 à 22:18, Andy Shevchenko a écrit :
> On Mon, Aug 29, 2022 at 7:19 PM Christophe Leroy
> <[email protected]> wrote:
>>
>> Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array")
>> there is no limitation on the number of GPIOs that can be allocated
>> in the system since the allocation is fully dynamic.
>>
>> ARCH_NR_GPIOS is today only used in order to provide downwards
>> gpiobase allocation from that value, while static allocation is
>> performed upwards from 0. However that has the disadvantage of
>> limiting the number of GPIOs that can be registered in the system.
>>
>> To overcome this limitation without requiring each and every
>> platform to provide its 'best-guess' maximum number, rework the
>> allocation to allocate upwards, allowing approx 2 millions of
>> GPIOs.
>>
>> In order to still allow static allocation for legacy drivers, define
>> GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic
>> allocation.
>
> Not sure about 256, but I understand that this can only be the best guess.
>

Well, it's already just a precaution. Linus W's expectation is that
static ones are allocated at first, they should already be allocated
when we start doing dynamic allocations so he was even thinking that we
could have started at 0 already.

But I can start higher if you think it is safer, maybe at 512 which is
the default ARCH_NR_GPIOS today.

2022-08-31 21:49:00

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS

On Wed, Aug 31, 2022, at 7:49 AM, Christophe Leroy wrote:
> Le 30/08/2022 à 22:18, Andy Shevchenko a écrit :
>> On Mon, Aug 29, 2022 at 7:19 PM Christophe Leroy
>> <[email protected]> wrote:
>>>
>>> Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array")
>>> there is no limitation on the number of GPIOs that can be allocated
>>> in the system since the allocation is fully dynamic.
>>>
>>> ARCH_NR_GPIOS is today only used in order to provide downwards
>>> gpiobase allocation from that value, while static allocation is
>>> performed upwards from 0. However that has the disadvantage of
>>> limiting the number of GPIOs that can be registered in the system.
>>>
>>> To overcome this limitation without requiring each and every
>>> platform to provide its 'best-guess' maximum number, rework the
>>> allocation to allocate upwards, allowing approx 2 millions of
>>> GPIOs.
>>>
>>> In order to still allow static allocation for legacy drivers, define
>>> GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic
>>> allocation.
>>
>> Not sure about 256, but I understand that this can only be the best guess.
>>
>
> Well, it's already just a precaution. Linus W's expectation is that
> static ones are allocated at first, they should already be allocated
> when we start doing dynamic allocations so he was even thinking that we
> could have started at 0 already.
>
> But I can start higher if you think it is safer, maybe at 512 which is
> the default ARCH_NR_GPIOS today.

FWIW, I went through the drivers that set the base to a value other than
zero or -1, to see what they use:

arch/arm/common/scoop.c: devptr->gpio.base = inf->gpio_base; // 204
arch/arm/mach-s3c/gpio-samsung.c: .base = S3C2410_GPM(0), // 384
arch/arm/mach-s3c/gpio-samsung.c: .base = S3C64XX_GPQ(0), // 197
arch/arm/mach-s3c/mach-h1940.c: .base = H1940_LATCH_GPIO(0), // 384 + 22
arch/arm/mach-sa1100/simpad.c: cs3_gpio.base = SIMPAD_CS3_GPIO_BASE; // 27 + 11
arch/arm/plat-orion/gpio.c: ochip->chip.base = gpio_base; // 64 + 32
arch/mips/alchemy/common/gpiolib.c: .base = ALCHEMY_GPIO2_BASE, // 32 + 16
arch/mips/alchemy/common/gpiolib.c: .base = AU1300_GPIO_BASE, // 0 + 75
arch/mips/kernel/gpio_txx9.c: txx9_gpio_chip.base = base; // 0 + 16
arch/mips/txx9/generic/setup.c: iocled->chip.base = basenum; -1
drivers/bcma/driver_gpio.c: chip->base = bus->num * BCMA_GPIO_MAX_PINS; // probably 0
drivers/gpio/gpio-adp5520.c: gc->base = pdata->gpio_start; // unused
drivers/gpio/gpio-adp5588.c: gc->base = pdata->gpio_start; // unused
drivers/gpio/gpio-arizona.c: arizona_gpio->gpio_chip.base = pdata->gpio_base; // 197
drivers/gpio/gpio-brcmstb.c: gc->base = gpio_base; // 2 * 32
drivers/gpio/gpio-bt8xx.c: c->base = modparam_gpiobase; // from modprobe
drivers/gpio/gpio-da9052.c: gpio->gp.base = pdata->gpio_base; // unused
drivers/gpio/gpio-da9055.c: gpio->gp.base = pdata->gpio_base; // unused
drivers/gpio/gpio-davinci.c: chips->chip.base = pdata->no_auto_base ? pdata->base : -1; // 0 + 144
drivers/gpio/gpio-dwapb.c: port->gc.base = pp->gpio_base; // from DT, deprecated
drivers/gpio/gpio-f7188x.c: .base = _base, \ // 10*10, unused
drivers/gpio/gpio-htc-egpio.c: chip->base = pdata->chip[i].gpio_base; // 192 + 5 * 8
drivers/gpio/gpio-ich.c: chip->base = modparam_gpiobase; // from modprobe
drivers/gpio/gpio-kempld.c: chip->base = pdata->gpio_base; // 0
drivers/gpio/gpio-lpc32xx.c: .base = LPC32XX_GPO_P3_GRP, // 104
drivers/gpio/gpio-madera.c: madera_gpio->gpio_chip.base = pdata->gpio_base; // -1
drivers/gpio/gpio-max730x.c: ts->chip.base = pdata->base; // 200 + 28 (timberdale)
drivers/gpio/gpio-max732x.c: gc->base = gpio_start; // 192
drivers/gpio/gpio-mc33880.c: mc->chip.base = pdata->base; // 100 (timberdale)
drivers/gpio/gpio-merrifield.c: priv->chip.base = gpio_base; // 0 + 213
drivers/gpio/gpio-mmio.c: gc->base = pdata->base; // 197 + 32
drivers/gpio/gpio-mockup.c: gc->base = base; // module parama
drivers/gpio/gpio-mvebu.c: mvchip->chip.base = id * MVEBU_MAX_GPIO_PER_BANK; // 4 * 32, from DT
drivers/gpio/gpio-omap.c: bank->chip.base = OMAP_MPUIO(0); // 192
drivers/gpio/gpio-omap.c: bank->chip.base = gpio; // 7 * 32
drivers/gpio/gpio-palmas.c: palmas_gpio->gpio_chip.base = palmas_pdata->gpio_base; // unused
drivers/gpio/gpio-pca953x.c: gc->base = chip->gpio_start; // ???? used a lot
drivers/gpio/gpio-pcf857x.c: gpio->chip.base = pdata ? pdata->gpio_base : -1; // 160
drivers/gpio/gpio-rc5t583.c: rc5t583_gpio->gpio_chip.base = pdata->gpio_base; // unused
drivers/gpio/gpio-rockchip.c: gc->base = bank->pin_base; // 8 * 32
drivers/gpio/gpio-sch311x.c: block->chip.base = sch311x_gpio_blocks[i].base; // 6 * 10
drivers/gpio/gpio-sta2x11.c: gpio->base = gpio_base; // unused
drivers/gpio/gpio-timberdale.c: gc->base = pdata->gpio_base; // 0 + 100
drivers/gpio/gpio-tps6586x.c: tps6586x_gpio->gpio_chip.base = pdata->gpio_base; // -1
drivers/gpio/gpio-tps65910.c: tps65910_gpio->gpio_chip.base = pdata->gpio_base; // -1
drivers/gpio/gpio-ucb1400.c: ucb->gc.base = ucb->gpio_offset; // 0
drivers/gpio/gpio-wm831x.c: wm831x_gpio->gpio_chip.base = pdata->gpio_base; // 197 + 64
drivers/gpio/gpio-wm8350.c: wm8350_gpio->gpio_chip.base = pdata->gpio_base; // 0
drivers/gpio/gpio-wm8994.c: wm8994_gpio->gpio_chip.base = pdata->gpio_base; 197 + 8
drivers/input/keyboard/adp5588-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused
drivers/input/keyboard/adp5589-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused
drivers/leds/leds-pca9532.c: data->gpio.base = pdata->gpio_base; // unused
drivers/leds/leds-tca6507.c: tca->gpio.base = pdata->gpio_base; // unused
drivers/mfd/asic3.c: asic->gpio.base = pdata->gpio_base; // 300 + 100
drivers/mfd/htc-i2cpld.c: gpio_chip->base = plat_chip_data->gpio_in_base; // 192 + 16 + 8*8
drivers/mfd/sm501.c: gchip->base = base; // 0
drivers/mfd/tps65010.c: tps->chip.base = board->base; // 204
drivers/mfd/ucb1x00-core.c: ucb->gpio.base = pdata->gpio_base; // 27 + 13
drivers/pinctrl/nomadik/pinctrl-nomadik.c: chip->base = id * NMK_GPIO_PER_CHIP; // 9 * 32
drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c: pctrl->gpio_bank[id].gc.base = args.args[1]; // 8*32, from DT
drivers/pinctrl/pinctrl-at91.c: chip->base = alias_idx * MAX_NB_GPIO_PER_BANK; // 5*32
drivers/pinctrl/pinctrl-ingenic.c: jzgc->gc.base = bank * 32; // 6 * 32, from DT
drivers/pinctrl/pinctrl-mcp23s08.c: mcp->chip.base = base; // -1
drivers/pinctrl/pinctrl-oxnas.c: .base = GPIO_BANK_START(_bank), // 2*32
drivers/pinctrl/pinctrl-pic32.c: .base = GPIO_BANK_START(_bank), // 10 * 16
drivers/pinctrl/pinctrl-pistachio.c: .base = _pin_base, // 9 * 16
drivers/pinctrl/pinctrl-st.c: bank->gpio_chip.base = bank_num * ST_GPIO_PINS_PER_BANK; // 26 * 8
drivers/pinctrl/renesas/gpio.c: gc->base = pfc->nr_gpio_pins; // ??? don't understand
drivers/pinctrl/samsung/pinctrl-samsung.c: gc->base = bank->grange.base;
drivers/pinctrl/stm32/pinctrl-stm32.c: bank->gpio_chip.base = args.args[1];
drivers/pinctrl/stm32/pinctrl-stm32.c: bank->gpio_chip.base = bank_nr * STM32_GPIO_PINS_PER_BANK;
drivers/pinctrl/stm32/pinctrl-stm32.c: bank->gpio_chip.base = bank_nr * STM32_GPIO_PINS_PER_BANK;
drivers/pinctrl/sunxi/pinctrl-sunxi.c: pctl->chip->base = pctl->desc->pin_base;
sound/soc/codecs/wm5100.c: wm5100->gpio_chip.base = wm5100->pdata.gpio_base; // 197 + 8 + 6
sound/soc/codecs/wm8903.c: wm8903->gpio_chip.base = pdata->gpio_base; // 197 + 8
sound/soc/codecs/wm8962.c: wm8962->gpio_chip.base = pdata->gpio_base; // 197 + 8
sound/soc/codecs/wm8996.c: wm8996->gpio_chip.base = wm8996->pdata.gpio_base; // 197 + 8

Arnd

2022-09-02 10:34:06

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS



Le 31/08/2022 à 22:55, Arnd Bergmann a écrit :
> On Wed, Aug 31, 2022, at 7:49 AM, Christophe Leroy wrote:
>> Le 30/08/2022 à 22:18, Andy Shevchenko a écrit :
>>> On Mon, Aug 29, 2022 at 7:19 PM Christophe Leroy
>>> <[email protected]> wrote:
>>>>
>>>> Since commit 14e85c0e69d5 ("gpio: remove gpio_descs global array")
>>>> there is no limitation on the number of GPIOs that can be allocated
>>>> in the system since the allocation is fully dynamic.
>>>>
>>>> ARCH_NR_GPIOS is today only used in order to provide downwards
>>>> gpiobase allocation from that value, while static allocation is
>>>> performed upwards from 0. However that has the disadvantage of
>>>> limiting the number of GPIOs that can be registered in the system.
>>>>
>>>> To overcome this limitation without requiring each and every
>>>> platform to provide its 'best-guess' maximum number, rework the
>>>> allocation to allocate upwards, allowing approx 2 millions of
>>>> GPIOs.
>>>>
>>>> In order to still allow static allocation for legacy drivers, define
>>>> GPIO_DYNAMIC_BASE with the value 256 as the start for dynamic
>>>> allocation.
>>>
>>> Not sure about 256, but I understand that this can only be the best guess.
>>>
>>
>> Well, it's already just a precaution. Linus W's expectation is that
>> static ones are allocated at first, they should already be allocated
>> when we start doing dynamic allocations so he was even thinking that we
>> could have started at 0 already.
>>
>> But I can start higher if you think it is safer, maybe at 512 which is
>> the default ARCH_NR_GPIOS today.
>
> FWIW, I went through the drivers that set the base to a value other than
> zero or -1, to see what they use:
>

> arch/arm/mach-s3c/gpio-samsung.c: .base = S3C2410_GPM(0), // 384

> arch/arm/mach-s3c/mach-h1940.c: .base = H1940_LATCH_GPIO(0), // 384 + 22

> drivers/mfd/asic3.c: asic->gpio.base = pdata->gpio_base; // 300 + 100
> drivers/mfd/htc-i2cpld.c: gpio_chip->base = plat_chip_data->gpio_in_base; // 192 + 16 + 8*8


Ok, so 256 is not enough, have to go to 512.

Christophe

2022-09-02 11:13:00

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS

Hi Andy,

CC linux-sh

On Fri, Sep 2, 2022 at 12:53 PM Andy Shevchenko
<[email protected]> wrote:
> On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <[email protected]> wrote:
> > drivers/pinctrl/renesas/gpio.c: gc->base = pfc->nr_gpio_pins; // ??? don't understand
>
> I think, w/o looking into the code, that this just guarantees the
> continuous numbering for all banks (chips) on the platform.

This part of the code depends on CONFIG_PINCTRL_SH_FUNC_GPIO,
which is used only on SH.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-09-02 11:14:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS

(Nuno, one point below for you)

On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <[email protected]> wrote:

...

> drivers/gpio/gpio-adp5520.c: gc->base = pdata->gpio_start; // unused
> drivers/gpio/gpio-adp5588.c: gc->base = pdata->gpio_start; // unused
> drivers/input/keyboard/adp5588-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused
> drivers/input/keyboard/adp5589-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused

I believe we should convert them to -1.

> drivers/gpio/gpio-bt8xx.c: c->base = modparam_gpiobase; // from modprobe
> drivers/gpio/gpio-ich.c: chip->base = modparam_gpiobase; // from modprobe

I believe it was designed for Intel hardware and so it can't be higher
than 512 - ngpios, where the latter one is small enough (dozen or a
couple of dozens of pins).

> drivers/gpio/gpio-dwapb.c: port->gc.base = pp->gpio_base; // from DT, deprecated

From board files, since some platforms expect a fixed number for it.

> drivers/gpio/gpio-mockup.c: gc->base = base; // module parama

This is for testing, so the test cases should be amended accordingly.
But I think the module itself is deprecated, and gpio-sim seems not
using it as a modprobe parameter, but rather as configfs.

> drivers/gpio/gpio-pca953x.c: gc->base = chip->gpio_start; // ???? used a lot

To answer this one needs to go via all board files (most of them ARM
32-bit based) and look, but it means almost the same case as per Intel
above: 512-ngpios.

> drivers/pinctrl/renesas/gpio.c: gc->base = pfc->nr_gpio_pins; // ??? don't understand

I think, w/o looking into the code, that this just guarantees the
continuous numbering for all banks (chips) on the platform.

> drivers/pinctrl/stm32/pinctrl-stm32.c: bank->gpio_chip.base = args.args[1];

Device Tree?!

--
With Best Regards,
Andy Shevchenko

2022-09-02 11:49:59

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS

On Fri, 2022-09-02 at 13:52 +0300, Andy Shevchenko wrote:
> (Nuno, one point below for you)
>
> On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <[email protected]> wrote:
>
> ...
>
> > drivers/gpio/gpio-adp5520.c:    gc->base = pdata->gpio_start; //
> > unused
> > drivers/gpio/gpio-adp5588.c:            gc->base = pdata-
> > >gpio_start; // unused
> > drivers/input/keyboard/adp5588-keys.c:  kpad->gc.base = gpio_data-
> > >gpio_start; // unused
> > drivers/input/keyboard/adp5589-keys.c:  kpad->gc.base = gpio_data-
> > >gpio_start; // unused
>
> I believe we should convert them to -1.
>

Well, the adp5588-keys.c was already refactored [1] to use FW
properties so that -1 will be used. In the process, gpio-adp5588.c
was dropped.

For the adp5589-keys.c driver, we might also need to do a similar
work as I suspect there's no platform making use of pdata. Hence,
yes, I believe -1 is the way to go.

Ditto for gpio-adp5520.c...

[1]:
https://lore.kernel.org/linux-input/[email protected]/T/#m382bec5c587241010d453ce1000bea2d34b86380

- Nuno Sá
>

2022-09-02 12:21:56

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v1 4/8] gpiolib: Get rid of ARCH_NR_GPIOS

On Fri, Sep 2, 2022, at 12:52 PM, Andy Shevchenko wrote:
> On Wed, Aug 31, 2022 at 11:55 PM Arnd Bergmann <[email protected]> wrote:
>
> ...
>
>> drivers/gpio/gpio-adp5520.c: gc->base = pdata->gpio_start; // unused
>> drivers/gpio/gpio-adp5588.c: gc->base = pdata->gpio_start; // unused
>> drivers/input/keyboard/adp5588-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused
>> drivers/input/keyboard/adp5589-keys.c: kpad->gc.base = gpio_data->gpio_start; // unused
>
> I believe we should convert them to -1.

This is probably something we should do separately, but a lot of the
drivers currently don't have support for probing from DT or any other
firmware interface but rely on platform_data definitions from a
board file that was never part of the upstream kernel.

We are going to remove a lot more board files early next year,
and I was hoping to follow up with a treewide cleanup of such
drivers and remove a lot of them entirely.

>> drivers/gpio/gpio-dwapb.c: port->gc.base = pp->gpio_base; // from DT, deprecated
>
> From board files, since some platforms expect a fixed number for it.

>> drivers/gpio/gpio-pca953x.c: gc->base = chip->gpio_start; // ???? used a lot
>
> To answer this one needs to go via all board files (most of them ARM
> 32-bit based) and look, but it means almost the same case as per Intel
> above: 512-ngpios.

Right, I went through all the board files the other drivers,
this one just happens to be used more than the others:

arch/arm/mach-davinci/board-da850-evm.c:#include <linux/platform_data/pca953x.h>
arch/arm/mach-ep93xx/vision_ep9307.c:#include <linux/platform_data/pca953x.h>
arch/arm/mach-mmp/ttc_dkb.c:#include <linux/platform_data/pca953x.h>
arch/arm/mach-pxa/cm-x300.c:#include <linux/platform_data/pca953x.h>
arch/arm/mach-pxa/spitz.c:#include <linux/platform_data/pca953x.h>
arch/arm/mach-pxa/zeus.c:#include <linux/platform_data/pca953x.h>
arch/arm/mach-pxa/zylonite_pxa300.c:#include <linux/platform_data/pca953x.h>
arch/arm/mach-s3c/mach-crag6410.c:#include <linux/platform_data/pca953x.h>

The only ones that have known users though are crag6410
and vision_ep9307, the other ones will be removed.

Vision-ep9307 has 128 GPIOs total, crag6410 is complicated because it
many different GPIO controllers in various combinations.

>> drivers/pinctrl/renesas/gpio.c: gc->base = pfc->nr_gpio_pins; // ??? don't understand
>
> I think, w/o looking into the code, that this just guarantees the
> continuous numbering for all banks (chips) on the platform.

Yes, that seems to be the idea most of the pinctrl drivers.

Arnd