Message-ID: <CAHp75Ve6zMC9s=TZT_pWoyxnKtXE0xipFCv_RDY4r4amnVbVxQ@mail.gmail.com>
Date:   Fri, 2 Sep 2022 17:58:01 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Christophe Leroy <christophe.leroy@...roup.eu>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Keerthy <j-keerthy@...com>, Russell King <linux@...linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>,
        Jonathan Corbet <corbet@....net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Davide Ciminaghi <ciminaghi@...dd.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Linux-Arch <linux-arch@...r.kernel.org>,
        Linux Documentation List <linux-doc@...r.kernel.org>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>
Subject: Re: [PATCH v2 5/9] gpiolib: Get rid of ARCH_NR_GPIOS

On Fri, Sep 2, 2022 at 4:57 PM Christophe Leroy
<christophe.leroy@...roup.eu> 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 512 as the start for dynamic
> allocation. The 512 value is chosen because it is the end of
> the current default range so all current static allocations are
> expected to be below that value. Of course that's just a rough
> estimate based on the default value, but assuming static
> allocations come first, even if there are more static allocations
> it should fit under the 512 value.
>
> In the future, it is expected that all static allocations go away
> and then dynamic allocation will be patched to start at 0.

Eventually we have to get rid of gpio_is_valid() completely...
But this is another story.
Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>

> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
> v2: Enhanced commit description and change from 256 to 512.
> ---
>  arch/arm/include/asm/gpio.h |  1 -
>  drivers/gpio/gpiolib.c      | 10 +++----
>  include/asm-generic/gpio.h  | 55 ++++++++++++++-----------------------
>  3 files changed, 26 insertions(+), 40 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/gpiolib.c b/drivers/gpio/gpiolib.c
> index 4e2fcb7b0a01..1846f24971e3 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..a7752cf152ce 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-512 space for them.
>   */
> -
> -static inline bool gpio_is_valid(int number)
> -{
> -       return number >= 0 && number < ARCH_NR_GPIOS;
> -}
> +#define GPIO_DYNAMIC_BASE      512
>
>  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
>


-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists