Messages in this thread | | | From | "Billie Alsup (balsup)" <> | Subject | Re: [PATCH] Allow configuration of ARCH_NR_GPIO | Date | Sat, 30 Jul 2022 22:33:22 +0000 |
| |
>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 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.
| |