Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220730003846.1730265-1-balsup@cisco.com> <CAK8P3a0Bn9xKRX9z369zVCy6CuguTvNbtXOOPbf_OfS697cDyQ@mail.gmail.com>
 <D7DCF238-0AE9-44A0-86B1-8C731FF8BB59@cisco.com>
In-Reply-To: <D7DCF238-0AE9-44A0-86B1-8C731FF8BB59@cisco.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Sun, 31 Jul 2022 13:37:57 +0200
Message-ID: <CAK8P3a1QOU+kSJo04zga8S-tHatJ3WNQBCR1Nxa10Ux1qNYb4A@mail.gmail.com>
Subject: Re: [PATCH] Allow configuration of ARCH_NR_GPIO
To:     "Billie Alsup (balsup)" <balsup@cisco.com>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        "linux@armlinux.org.uk" <linux@armlinux.org.uk>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>,
        "linus.walleij@linaro.org" <linus.walleij@linaro.org>,
        "ardb@kernel.org" <ardb@kernel.org>,
        "rmk+kernel@armlinux.org.uk" <rmk+kernel@armlinux.org.uk>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "nick.hawkins@hpe.com" <nick.hawkins@hpe.com>,
        "john@phrozen.org" <john@phrozen.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Sun, Jul 31, 2022 at 12:33 AM Billie Alsup (balsup) <balsup@cisco.com> wrote:
>   >On 7/30/22, 4:54 AM, "Arnd Bergmann" <arnd@arndb.de> wrote:
>
>   >What is the use case for manually setting this rather than deriving
>   >it from the selected platforms?
>
> A Cisco GPIO IP block supports 1022 pins.  A single FPGA can support
> multiple GPIO IP blocks (although typically 1 or 2).  A system may
> have two or three FPGAs directly accessible through the PCI bus, and
> an additional 8 to 36 FPGAS accessible through alternate means (e.g.,
> SLPC, i2c, or a proprietary point-to-point bus).  Although typically
> the full set of pins is not used, in total, it is very easy to exceed
> the limit.  Previous patches have simply bumped up the number, and I
> thought a more generic approach that would solve the use case for
> customer devices would be more appropriate.  I am also trying to keep
> the ARM and X86 implementations similar, as the devices are
> accessible from both an ARM-based BMC as well as an X86 type CPU.

It would be nice to keep it consistent between the major architectures,
at least arm64, powerpc, mips and riscv in addition to the two you already
have.

One way we could do this is to keep the existing
CONFIG_ARCH_NR_GPIO for on-chip GPIOs, and then add
a new Kconfig option for extra GPIOs on things like PCI cards or
programmable logic.

>  It is desirable to use the same kernel configuration for multiple
> products, even though the number of FPGAS, and therefore the number
> of supported GPIO pins can vary.  Such is also the case for Open NOS
> environments, such as SONiC and FBOSS, which attempt to use a single
> kernel configuration across multiple (multi-vendor) products. A
> platform specific option would not seem to work in such cases.
> Ideally, I would like to see this configuration knob go away
> completely, but that might be a more complicated and controversial
> change.  It is not used in very many places, but the problematic use
> is within the inline function gpio_is_valid.

I think the plan for making the option obsolete is to convert every user
of the legacy GPIO interface to use GPIO descriptors, which don't
have the limitation and don't need gpio_is_valid().

In your use case, can you identify a set of drivers that need access
to old-style GPIO numbers above the hardcoded on-chip numbers?
Maybe we can prioritize fixing those drivers.

>   >Have you tried to use both a platform-specific option for the minimum
>   >number of this setting, and then restictricting the
>   >CONFIG_ARCH_NR_GPIO
>   >setting with a 'range' statement?
>
> I have not tried a platform specific option, as we would need to
> choose the worst case platform for such a configuration (in order to
> share a kernel build across products), but this in turn may conflict
> with the chosen platform for some other “worst-case” setting of
> another variable.  It seems prudent to simply allow the kernel
> builder to choose the options they want.  This is just one example
> where the configuration is not directly available to the kernel
> builder, but must be set indirectly.  We have other configuration
> options that cannot be set directly, and instead require us to enable
> an unrelated (from a product standpoint) configuration item purely
> for the side-effect of enabling such an option.  CONFIG_ARCH_NR_GPIO
> is a similar issue, where you choose a platform to indirectly set the
> value, but that value really needs to be configurable by the builder.
> The last such patch for X86 simply bumped up the number.  I thought
> it better to simply allow configuration.  I have no issue with
> restricting the value to a minimum, if that is a worry.

I have no problem with allowing a kernel build to use larger values
than the default, I'm just worried that allowing smaller values than what
we know is needed for the existing platforms can break distro kernels
that have to work across many machines.

        Arnd