Messages in this thread | | | From | "Billie Alsup (balsup)" <> | Subject | Re: [PATCH] Allow configuration of ARCH_NR_GPIO | Date | Mon, 1 Aug 2022 15:36:57 +0000 |
| |
On 7/31/22, 4:44 AM, "Arnd Bergmann" <arnd@arndb.de> wrote:
>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.
Thank you for your feedback. This makes sense to me, but your next statement has me rethinking the problem.
> 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.
For FBOSS, libgpio is used, so controller specific offsets apply. The only issue is that our gpio controller allows the kernel to set the base pin number, and that will fail with a small ARCH_NR_GPIOS. We can workaround this issue by explicitly assigning a base pin number (as a property read from device-tree or ACPI). For SONiC however, GPIO_SYSFS is still used. From code inspection, the gpio_is_valid call would only apply if a pin number were used that does not yet exist. Such would be the case where the pin numbers are predefined, but perhaps the controller has not yet been hot-plugged. In such cases a pr_warn is emitted from gpio_to_desc. However, I think this should be ok (as it would be only an occasional race condition when the application still thought a device was plugged in and tried to access the device, while a user was in the process of removing said device).
Let me run some experiments with our controllers passing in an explicit base pin number instead of letting the kernel auto-assign it. This will of course be outside of the ARCH_NR_GPIO setting. If this is successful, then no Kconfig changes are necessary. If there are still issues, then I will follow up with a V2 that sets an EXTRA_NR_GPIO or similar in drivers/gpio/Kconfig as you have suggested. This can default to 0. This ensures that the platform specified CONFIG_ARCH_NR_GPIO is not set too low (inconsistent with the platform requirements), and gives us the flexibility to support additional controllers. This might be necessary if we hit the ARCH_NR_GPIO limit with generic i2c gpio expanders (although even in that case, reading the base pin from device-tree or ACPI seems like a better solution). gpio-pca953x does allow setting the base today from platform data, but we prefer device-tree and ACPI over the platform data technique.
Thank again for your feedback!
| |