[PATCH 0/2] gpio: TODO: add item about GPIO drivers reading struct gpio_chip::base

[PATCH 0/2] gpio: TODO: add item about GPIO drivers reading struct gpio_chip::base

[Ahmad Fatoum]

Bartosz requested that I add this to the TODO, so here goes.
While at it, I also added a FIXME into the driver.

I'll be away most of the month, so feel free to squash changes as
appropriate.

---
Ahmad Fatoum (2):
      gpio: TODO: add item about GPIO drivers reading struct gpio_chip::base
      pinctrl: at91: add FIXME about read back of struct gpio_chip::base

 drivers/gpio/TODO              | 7 +++++++
 drivers/pinctrl/pinctrl-at91.c | 5 +++++
 2 files changed, 12 insertions(+)
---
base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb
change-id: 20250507-gpio-chip-base-readback-c7d8eb8b3cd9

Best regards,
-- 
Ahmad Fatoum <a.fatoum@pengutronix.de>

[PATCH 1/2] gpio: TODO: add item about GPIO drivers reading struct gpio_chip::base

[Ahmad Fatoum]

drivers/pinctrl/pinctrl-at91.c uses struct gpio_chip::base to find out
which bit to set in a register:

  dev_dbg(npct->dev, "enable pin %u as GPIO\n", offset);

  mask = 1 << (offset - chip->base);

This adds a non-obvious dependency on the global numberspace from the
GPIO driver itself, even if all consumers use the descriptor API.

More such instances may exist and will need to be fixed in the quest for
removal of the global numberspace, so add a reminder about it to the
TODO list.

Link: https://lore.kernel.org/all/1d00c056-3d61-4c22-bedd-3bae0bf1ddc4@pengutronix.de/
Suggested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/TODO | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpio/TODO b/drivers/gpio/TODO
index 4b70cbaa1caacdcc04044039782e86a7a44135c1..4a8b349f2483a91883c74b07a43efb1462dbd377 100644
--- a/drivers/gpio/TODO
+++ b/drivers/gpio/TODO
@@ -44,6 +44,13 @@ Work items:
   to a machine description such as device tree, ACPI or fwnode that
   implicitly does not use global GPIO numbers.
 
+- Fix drivers to not read back struct gpio_chip::base. Some drivers do
+  that and would be broken by attempts to poison it or make it dynamic.
+  Example in AT91 pinctrl driver:
+  https://lore.kernel.org/all/1d00c056-3d61-4c22-bedd-3bae0bf1ddc4@pengutronix.de/
+  This particular driver is also DT-only, so with the above fixed, the
+  base can be made dynamic (set to -1) if CONFIG_GPIO_SYSFS is disabled.
+
 - When this work is complete (will require some of the items in the
   following ongoing work as well) we can delete the old global
   numberspace accessors from <linux/gpio.h> and eventually delete

-- 
2.39.5

[PATCH 2/2] pinctrl: at91: add FIXME about read back of struct gpio_chip::base

[Ahmad Fatoum]

drivers/gpio/gpiolib.c has this:

  /*
   * TODO: it should not be necessary to reflect the
   * assigned base outside of the GPIO subsystem. Go over
   * drivers and see if anyone makes use of this, else
   * drop this and assign a poison instead.
   */
  gc->base = base;

This would break the driver, because it assumes gpio_chip::base to be
the same static base that was written into it.

Point that out in a comment, until it's fixed.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pinctrl/pinctrl-at91.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 93ab277d9943cff3a0b771f3e997594cb144826c..b0e00f3793312de2288aa1b900793d9e2b6125d6 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -949,6 +949,11 @@ static int at91_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 	dev_dbg(npct->dev, "enable pin %u as GPIO\n", offset);
 
+	/*
+	 * The GPIO chip base is an internal detail that will
+	 * eventually go away alongside sysfs and the global numberspace.
+	 * FIXME: stop reading it back here
+	 */
 	mask = 1 << (offset - chip->base);
 
 	dev_dbg(npct->dev, "enable pin %u as PIO%c%d 0x%x\n",

-- 
2.39.5

Re: (subset) [PATCH 0/2] gpio: TODO: add item about GPIO drivers reading struct gpio_chip::base

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Wed, 07 May 2025 19:28:00 +0200, Ahmad Fatoum wrote:
> Bartosz requested that I add this to the TODO, so here goes.
> While at it, I also added a FIXME into the driver.
> 
> I'll be away most of the month, so feel free to squash changes as
> appropriate.
> 

Applied, thanks!

[1/2] gpio: TODO: add item about GPIO drivers reading struct gpio_chip::base
      https://git.kernel.org/brgl/linux/c/833c086f22ecebe576af42051733796d1690dd71

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>