[PATCH v2 2/2] gpio: mmio: track the direction of chips without direction registers
flat view
COLD34d
From: Mehmet Fide <hidden>
Date: 2026-09-02 07:39:52
Also in:
imx, linux-gpio, lkml
Subsystem:
gpio subsystem, the rest · Maintainers:
Linus Walleij, Bartosz Golaszewski, Linus Torvalds
Revision v2 of 4 in this series.
Revisions (4)
- v2 current
- v4 [diff vs current]
- v5 [diff vs current]
- v6 [diff vs current]
From: Mehmet Fide <redacted> A generic chip with GPIO_GENERIC_PINCTRL_BACKEND and no direction registers sets the direction through pinctrl but has no get_direction callback, so every gpiod_get_direction() call trips the WARN in gpiolib and the direction gpiolib reports is whatever it assumed. On a Vybrid Colibri module that is 21 backtraces per boot. Keep the direction of such a chip in the existing shadow: the direction setters update sdir under the chip lock, and get_direction() is the shadow-reading path already used for unreadable direction registers. That keeps the callback usable in atomic context, which it has to be: gpiochip_lock_as_irq() calls it for !can_sleep chips from gpiochip_irq_domain_activate(), under the irq descriptor lock. The pad's actual state is read once, in process context, when a line is requested: gpiolib calls request() right before get_direction() for a new line, so the shadow is seeded from PIN_CONFIG_OUTPUT_ENABLE there and the line reports what the pin controller says. Lines pinctrl cannot answer for keep the input default, which is what gpiolib assumed before. Suggested-by: Bartosz Golaszewski <brgl@kernel.org> Signed-off-by: Mehmet Fide <redacted> --- v2: - keep the direction in the chip's shadow and reuse the shadow-reading get_direction(); seed the shadow from pinctrl in request(), because gpiochip_lock_as_irq() calls get_direction() under the irq descriptor lock for !can_sleep chips (Sashiko) - lines pinctrl cannot answer for keep the input default, no error returned to gpiochip_lock_as_irq() (Sashiko) - drop the claim about the initial direction scan, it runs before the pin ranges exist (Sashiko) drivers/gpio/gpio-mmio.c | 65 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) drivers/gpio/gpio-mmio.c | 63 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 7e4b3e8d609f..84bf8cc7a0c7 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c@@ -49,6 +49,7 @@ o ` ~~~~\___/~~~~ ` controller in FPGA is ,.` #include <linux/log2.h> #include <linux/module.h> #include <linux/pinctrl/consumer.h> +#include <linux/pinctrl/pinconf-generic.h> #include <linux/platform_device.h> #include <linux/property.h> #include <linux/spinlock.h>
@@ -372,7 +373,17 @@ static int gpio_mmio_dir_in_err(struct gpio_chip *gc, unsigned int gpio) static int gpio_mmio_simple_dir_in(struct gpio_chip *gc, unsigned int gpio) { - return gpio_mmio_dir_return(gc, gpio, false); + struct gpio_generic_chip *chip = to_gpio_generic_chip(gc); + int ret; + + ret = gpio_mmio_dir_return(gc, gpio, false); + if (ret) + return ret; + + guard(raw_spinlock_irqsave)(&chip->lock); + chip->sdir &= ~gpio_mmio_line2mask(gc, gpio); + + return 0; } static int gpio_mmio_dir_out_err(struct gpio_chip *gc, unsigned int gpio,
@@ -384,9 +395,19 @@ static int gpio_mmio_dir_out_err(struct gpio_chip *gc, unsigned int gpio, static int gpio_mmio_simple_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) { + struct gpio_generic_chip *chip = to_gpio_generic_chip(gc); + int ret; + gc->set(gc, gpio, val); - return gpio_mmio_dir_return(gc, gpio, true); + ret = gpio_mmio_dir_return(gc, gpio, true); + if (ret) + return ret; + + guard(raw_spinlock_irqsave)(&chip->lock); + chip->sdir |= gpio_mmio_line2mask(gc, gpio); + + return 0; } static int gpio_mmio_dir_in(struct gpio_chip *gc, unsigned int gpio)
@@ -601,20 +622,54 @@ static int gpio_mmio_setup_direction(struct gpio_generic_chip *chip, gc->direction_input = gpio_mmio_dir_in_err; else gc->direction_input = gpio_mmio_simple_dir_in; + + if (cfg->flags & GPIO_GENERIC_PINCTRL_BACKEND) { + chip->dir_unreadable = true; + gc->get_direction = gpio_mmio_get_dir; + } } return 0; } +/* + * Without direction registers the direction lives in the pin controller, + * so seed the shadow from there when a line is requested. + */ +static void gpio_mmio_seed_dir_from_pinctrl(struct gpio_chip *gc, + unsigned int gpio) +{ + struct gpio_generic_chip *chip = to_gpio_generic_chip(gc); + unsigned long config; + + if (!IS_ENABLED(CONFIG_PINCTRL) || chip->reg_dir_out || chip->reg_dir_in) + return; + + config = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 0); + if (pinctrl_gpio_get_config(gc, gpio, &config)) + return; + + guard(raw_spinlock_irqsave)(&chip->lock); + if (config) + chip->sdir |= gpio_mmio_line2mask(gc, gpio); + else + chip->sdir &= ~gpio_mmio_line2mask(gc, gpio); +} + static int gpio_mmio_request(struct gpio_chip *gc, unsigned int gpio_pin) { struct gpio_generic_chip *chip = to_gpio_generic_chip(gc); + int ret; if (gpio_pin >= gc->ngpio) return -EINVAL; - if (chip->pinctrl) - return gpiochip_generic_request(gc, gpio_pin); + if (chip->pinctrl) { + ret = gpiochip_generic_request(gc, gpio_pin); + if (ret) + return ret; + gpio_mmio_seed_dir_from_pinctrl(gc, gpio_pin); + } return 0; }
--
2.54.0