Re: [PATCH] Allow configuration of ARCH_NR_GPIO

From: Billie Alsup (balsup)
Date: Sat Jul 30 2022 - 18:33:37 EST



>On 7/30/22, 4:54 AM, "Arnd Bergmann" <arnd@xxxxxxxx> 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
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.

>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 note that it doesn't seem that this setting has a negative impact on
resources within the system. Only one driver (gpio-aggregator) performs
memory allocation based on this value, and that is only for the duration
of a single function call to aggr_parse. Otherwise, there does not seem
to be any negative consequence to having a higher limit.