[PATCH] pinctrl: mediatek: paris: Directly modify registers to set GPIO direction

[PATCH] pinctrl: mediatek: paris: Directly modify registers to set GPIO direction

[Chen-Yu Tsai]

pinctrl_gpio_direction_input() / pinctrl_gpio_direction_output() take
the pinctrl mutex. This causes a gpiochip operations to need to sleep.
Worse yet, the .can_sleep field in the gpiochip is not set. This causes
the shared GPIO proxy to trip over, as it uses gpiod_cansleep() to check
whether it can use a spinlock or needs a mutex. In this case, it ends
up taking a spinlock, then calls pinctrl_gpio_direction_output(), which
takes a mutex. This causes a huge warning.

While this class of Mediatek hardware does not have separate clear/set
registers, the pinctrl context has a spinlock that is taken whenever
a register read-modify-write is done.

Switch to directly setting the GPIO direction register bits to avoid
the mutex.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/pinctrl/mediatek/pinctrl-paris.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
index 6bf37d8085fa..e4c0bc27d984 100644
--- a/drivers/pinctrl/mediatek/pinctrl-paris.c
+++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
@@ -886,19 +886,24 @@ static int mtk_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value)
 
 static int mtk_gpio_direction_input(struct gpio_chip *chip, unsigned int gpio)
 {
-	return pinctrl_gpio_direction_input(chip, gpio);
+	struct mtk_pinctrl *hw = gpiochip_get_data(chip);
+	const struct mtk_pin_desc *desc = &hw->soc->pins[gpio];
+
+	return mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_DIR, 0);
 }
 
 static int mtk_gpio_direction_output(struct gpio_chip *chip, unsigned int gpio,
 				     int value)
 {
+	struct mtk_pinctrl *hw = gpiochip_get_data(chip);
+	const struct mtk_pin_desc *desc = &hw->soc->pins[gpio];
 	int ret;
 
 	ret = mtk_gpio_set(chip, gpio, value);
 	if (ret)
 		return ret;
 
-	return pinctrl_gpio_direction_output(chip, gpio);
+	return mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_DIR, 1);
 }
 
 static int mtk_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
-- 
2.54.0.rc2.544.gc7ae2d5bb8-goog

Re: [PATCH] pinctrl: mediatek: paris: Directly modify registers to set GPIO direction

[Linus Walleij]

On Mon, Apr 27, 2026 at 4:10 AM Chen-Yu Tsai <wenst@chromium.org> wrote:

> pinctrl_gpio_direction_input() / pinctrl_gpio_direction_output() take
> the pinctrl mutex. This causes a gpiochip operations to need to sleep.
> Worse yet, the .can_sleep field in the gpiochip is not set. This causes
> the shared GPIO proxy to trip over, as it uses gpiod_cansleep() to check
> whether it can use a spinlock or needs a mutex. In this case, it ends
> up taking a spinlock, then calls pinctrl_gpio_direction_output(), which
> takes a mutex. This causes a huge warning.
>
> While this class of Mediatek hardware does not have separate clear/set
> registers, the pinctrl context has a spinlock that is taken whenever
> a register read-modify-write is done.
>
> Switch to directly setting the GPIO direction register bits to avoid
> the mutex.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

You are essentially decoupling the pin control back-end from
the GPIO front-end, can you try to do this a more friendly way
that doesn't wrangle registers out of the pin controller like this?

If you insist on doing this, you also need to DELETE the pin
control back-end function
mtk_pinmux_gpio_set_direction(), which is what gets called.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: mediatek: paris: Directly modify registers to set GPIO direction

[Chen-Yu Tsai]

On Tue, Apr 28, 2026 at 7:44 PM Linus Walleij <linusw@kernel.org> wrote:
>
> On Mon, Apr 27, 2026 at 4:10 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> > pinctrl_gpio_direction_input() / pinctrl_gpio_direction_output() take
> > the pinctrl mutex. This causes a gpiochip operations to need to sleep.
> > Worse yet, the .can_sleep field in the gpiochip is not set. This causes
> > the shared GPIO proxy to trip over, as it uses gpiod_cansleep() to check
> > whether it can use a spinlock or needs a mutex. In this case, it ends
> > up taking a spinlock, then calls pinctrl_gpio_direction_output(), which
> > takes a mutex. This causes a huge warning.
> >
> > While this class of Mediatek hardware does not have separate clear/set
> > registers, the pinctrl context has a spinlock that is taken whenever
> > a register read-modify-write is done.
> >
> > Switch to directly setting the GPIO direction register bits to avoid
> > the mutex.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
> You are essentially decoupling the pin control back-end from
> the GPIO front-end, can you try to do this a more friendly way
> that doesn't wrangle registers out of the pin controller like this?

This is essentially following

    commit 8df89a7cbc63 ("pinctrl-sunxi: don't call pinctrl_gpio_direction()")

And in MediaTek's hardware, the GPIO direction is even further removed
from pinctrl. GPIO is a function that gets muxed, but after that the
direction and data bits are in separate registers that are grouped
separately from the pinmux registers. The pin controller doesn't have
to be involved.

> If you insist on doing this, you also need to DELETE the pin
> control back-end function
> mtk_pinmux_gpio_set_direction(), which is what gets called.

You mean delete the .gpio_set_direction field from pinmux_ops?
Sure I can do that.


Thanks
ChenYu