| Message ID | 20250113-b4-imx-gpio-base-warning-v1-1-0a28731a5cf6@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: mxc: silence warning about GPIO base being statically allocated | expand |
On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > Some drivers have had deterministic GPIO numbering for most of > their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a > ("gpio/mxc: specify gpio base for device tree probe"), more than > 12 years ago. > > Reverting this to dynamically numbered will break existing setups in > the worst manner possible: The build will succeed, the kernel will not > print warnings, but users will find their devices essentially toggling > GPIOs at random with the potential of permanent damage. > > As these concerns won't go away until the sysfs interface is removed, > let's add a new struct gpio_chip::legacy_static_base member that can be > used by existing drivers that have been grandfathered in to suppress > the warning currently being printed: > > gpio gpiochip0: Static allocation of GPIO base is deprecated, > use dynamic allocation. Warning is harmless and still a good reminder for the stuff that needs more love. NAK.
On 14.01.25 10:49, Andy Shevchenko wrote: > On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: >> >> Some drivers have had deterministic GPIO numbering for most of >> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a >> ("gpio/mxc: specify gpio base for device tree probe"), more than >> 12 years ago. >> >> Reverting this to dynamically numbered will break existing setups in >> the worst manner possible: The build will succeed, the kernel will not >> print warnings, but users will find their devices essentially toggling >> GPIOs at random with the potential of permanent damage. >> >> As these concerns won't go away until the sysfs interface is removed, >> let's add a new struct gpio_chip::legacy_static_base member that can be >> used by existing drivers that have been grandfathered in to suppress >> the warning currently being printed: >> >> gpio gpiochip0: Static allocation of GPIO base is deprecated, >> use dynamic allocation. > > Warning is harmless and still a good reminder for the stuff that needs > more love. > NAK. A warning is a call-to-action and it's counterproductive to keep tricking people into removing the static base and breaking other users' scripts. I don't understand what love you think this will spawn with regards to the i.MX GPIO driver. Can you explain? Cheers, Ahmad
On Tue, Jan 14, 2025 at 12:06 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > On 14.01.25 10:49, Andy Shevchenko wrote: > > On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > >> > >> Some drivers have had deterministic GPIO numbering for most of > >> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a > >> ("gpio/mxc: specify gpio base for device tree probe"), more than > >> 12 years ago. > >> > >> Reverting this to dynamically numbered will break existing setups in > >> the worst manner possible: The build will succeed, the kernel will not > >> print warnings, but users will find their devices essentially toggling > >> GPIOs at random with the potential of permanent damage. > >> > >> As these concerns won't go away until the sysfs interface is removed, > >> let's add a new struct gpio_chip::legacy_static_base member that can be > >> used by existing drivers that have been grandfathered in to suppress > >> the warning currently being printed: > >> > >> gpio gpiochip0: Static allocation of GPIO base is deprecated, > >> use dynamic allocation. > > > > Warning is harmless and still a good reminder for the stuff that needs > > more love. > > NAK. > > A warning is a call-to-action and it's counterproductive to keep tricking > people into removing the static base and breaking other users' scripts. Are you prepared to say the same when the entire GPIO SYSFS will be removed? Because that's exactly what I referred to in the reply to the cover letter as an impediment to move forward. > I don't understand what love you think this will spawn with regards > to the i.MX GPIO driver. Can you explain? To fix the bugs you found. If it's not the GPIO driver a culprit, we need to find the real one and fix that.
On 14.01.25 20:38, Andy Shevchenko wrote: > On Tue, Jan 14, 2025 at 12:06 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: >> On 14.01.25 10:49, Andy Shevchenko wrote: >>> On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: >>>> >>>> Some drivers have had deterministic GPIO numbering for most of >>>> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a >>>> ("gpio/mxc: specify gpio base for device tree probe"), more than >>>> 12 years ago. >>>> >>>> Reverting this to dynamically numbered will break existing setups in >>>> the worst manner possible: The build will succeed, the kernel will not >>>> print warnings, but users will find their devices essentially toggling >>>> GPIOs at random with the potential of permanent damage. >>>> >>>> As these concerns won't go away until the sysfs interface is removed, >>>> let's add a new struct gpio_chip::legacy_static_base member that can be >>>> used by existing drivers that have been grandfathered in to suppress >>>> the warning currently being printed: >>>> >>>> gpio gpiochip0: Static allocation of GPIO base is deprecated, >>>> use dynamic allocation. >>> >>> Warning is harmless and still a good reminder for the stuff that needs >>> more love. >>> NAK. >> >> A warning is a call-to-action and it's counterproductive to keep tricking >> people into removing the static base and breaking other users' scripts. > > Are you prepared to say the same when the entire GPIO SYSFS will be > removed? Because that's exactly what I referred to in the reply to the > cover letter as an impediment to move forward. No. But this gives me an idea: We could make the warning dependent on CONFIG_GPIO_SYSFS and add a comment to the i.MX code suggesting users do that instead. What do you think? >> I don't understand what love you think this will spawn with regards >> to the i.MX GPIO driver. Can you explain? > > To fix the bugs you found. If it's not the GPIO driver a culprit, we > need to find the real one and fix that. Again: jumbling GPIOs with potential hardware harm as a result is not a fix. Cheers, Ahmad
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 679ed764cb143c4b3357106de1570e8d38441372..bedeb8f28badfb7287c4929f9ad0825e050a79c9 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1011,7 +1011,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, * drop this and assign a poison instead. */ gc->base = base; - } else { + } else if (!gc->legacy_static_base) { dev_warn(&gdev->dev, "Static allocation of GPIO base is deprecated, use dynamic allocation.\n"); } diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 2dd7cb9cc270a68ddedbcdd5d44e0d0f88dfa785..6e820d79d03e61123f89aaf884d35d4a1a5918a7 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -382,6 +382,10 @@ struct gpio_irq_chip { * implies that if the chip supports IRQs, these IRQs need to be threaded * as the chip access may sleep when e.g. reading out the IRQ status * registers. + * @legacy_static_base: set for some existing drivers, where @base is non-negative + * and changing that would induce so much breakage that they were + * grandfathered in until GPIO sysfs support is removed altogether. + * Do not set this for any new drivers. * @read_reg: reader function for generic GPIO * @write_reg: writer function for generic GPIO * @be_bits: if the generic GPIO has big endian bit order (bit 31 is representing @@ -467,6 +471,7 @@ struct gpio_chip { u16 offset; const char *const *names; bool can_sleep; + bool legacy_static_base; #if IS_ENABLED(CONFIG_GPIO_GENERIC) unsigned long (*read_reg)(void __iomem *reg);
Some drivers have had deterministic GPIO numbering for most of their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a ("gpio/mxc: specify gpio base for device tree probe"), more than 12 years ago. Reverting this to dynamically numbered will break existing setups in the worst manner possible: The build will succeed, the kernel will not print warnings, but users will find their devices essentially toggling GPIOs at random with the potential of permanent damage. As these concerns won't go away until the sysfs interface is removed, let's add a new struct gpio_chip::legacy_static_base member that can be used by existing drivers that have been grandfathered in to suppress the warning currently being printed: gpio gpiochip0: Static allocation of GPIO base is deprecated, use dynamic allocation. Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- drivers/gpio/gpiolib.c | 2 +- include/linux/gpio/driver.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)