public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] pinctrl: mediatek: common-v1: Directly modify registers to set GPIO direction
@ 2026-04-27  6:17 Chen-Yu Tsai
  2026-04-28 10:48 ` Linus Walleij
  0 siblings, 1 reply; 2+ messages in thread
From: Chen-Yu Tsai @ 2026-04-27  6:17 UTC (permalink / raw)
  To: Sean Wang, Matthias Brugger, AngeloGioacchino Del Regno,
	Linus Walleij
  Cc: Chen-Yu Tsai, linux-mediatek, linux-gpio, linux-arm-kernel,
	linux-kernel

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.

Since the Mediatek hardware has separate clear/set registers, there is
no risk of clobbering other bits like with a read-modify-write pattern.
Switch to directly setting the GPIO direction register bits to avoid
the mutex.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Only compile tested. Accidentally fixed the wrong file when my target
actually used pinctrl-paris.c
---
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
index 3f518dce6d23..9c258e205e39 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
@@ -806,16 +806,24 @@ static const struct pinmux_ops mtk_pmx_ops = {
 	.gpio_request_enable	= mtk_pmx_gpio_request_enable,
 };
 
+static int mtk_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	struct mtk_pinctrl *pctl = gpiochip_get_data(chip);
+
+	return mtk_pmx_gpio_set_direction(pctl->pctl_dev, NULL, offset, true);
+}
+
 static int mtk_gpio_direction_output(struct gpio_chip *chip,
 					unsigned offset, int value)
 {
+	struct mtk_pinctrl *pctl = gpiochip_get_data(chip);
 	int ret;
 
 	ret = mtk_gpio_set(chip, offset, value);
 	if (ret)
 		return ret;
 
-	return pinctrl_gpio_direction_output(chip, offset);
+	return mtk_pmx_gpio_set_direction(pctl->pctl_dev, NULL, offset, true);
 }
 
 static int mtk_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
@@ -895,7 +903,7 @@ static const struct gpio_chip mtk_gpio_chip = {
 	.request		= gpiochip_generic_request,
 	.free			= gpiochip_generic_free,
 	.get_direction		= mtk_gpio_get_direction,
-	.direction_input	= pinctrl_gpio_direction_input,
+	.direction_input	= mtk_gpio_direction_input,
 	.direction_output	= mtk_gpio_direction_output,
 	.get			= mtk_gpio_get,
 	.set			= mtk_gpio_set,
-- 
2.54.0.rc2.544.gc7ae2d5bb8-goog



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] pinctrl: mediatek: common-v1: Directly modify registers to set GPIO direction
  2026-04-27  6:17 [PATCH] pinctrl: mediatek: common-v1: Directly modify registers to set GPIO direction Chen-Yu Tsai
@ 2026-04-28 10:48 ` Linus Walleij
  0 siblings, 0 replies; 2+ messages in thread
From: Linus Walleij @ 2026-04-28 10:48 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Sean Wang, Matthias Brugger, AngeloGioacchino Del Regno,
	linux-mediatek, linux-gpio, linux-arm-kernel, linux-kernel

On Mon, Apr 27, 2026 at 8:17 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.
>
> Since the Mediatek hardware has separate clear/set registers, there is
> no risk of clobbering other bits like with a read-modify-write pattern.
> Switch to directly setting the GPIO direction register bits to avoid
> the mutex.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Here you need to at least delete the assignment of mtk_pmx_gpio_set_direction()
from:

static const struct pinmux_ops mtk_pmx_ops = {
        .get_functions_count    = mtk_pmx_get_funcs_cnt,
        .get_function_name      = mtk_pmx_get_func_name,
        .get_function_groups    = mtk_pmx_get_func_groups,
        .set_mux                = mtk_pmx_set_mux,
        .gpio_set_direction     = mtk_pmx_gpio_set_direction,

^ Here, it should no longer be used, right?

Yours,
Linus Walleij


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-28 10:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27  6:17 [PATCH] pinctrl: mediatek: common-v1: Directly modify registers to set GPIO direction Chen-Yu Tsai
2026-04-28 10:48 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox