[PATCH] gpio/rockchip: Stop calling pinctrl for set_direction

[PATCH] gpio/rockchip: Stop calling pinctrl for set_direction

[Robin Murphy]

Marking the whole controller as sleeping due to the pinctrl calls in the
.direction_{input,output} callbacks has the unfortunate side effect that
legitimate invocations of .get and .set, which cannot themselves sleep,
in atomic context now spew WARN()s from gpiolib.

However, as Heiko points out, the driver doing this is a bit silly to
begin with, as the pinctrl .gpio_set_direction hook doesn't even care
about the direction, the hook is only used to claim the mux. And sure
enough, the .gpio_request_enable hook exists to serve this very purpose,
so switch to that and remove the problematic business entirely.

Cc: stable@vger.kernel.org
Fixes: 20cf2aed89ac ("gpio: rockchip: mark the GPIO controller as sleeping")
Suggested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
If I don't have to redesign bits of the GPIO subsystem itself, this much
I can do :)

Tested on RK3399 NanoPC-T4 with GPIOs with and without explicit pinctrl,
and PROVE_LOCKING and DEBUG_ATOMIC_SLEEP stayed quiet (although I don't
think this board has any shared pins)
---
 drivers/gpio/gpio-rockchip.c       | 8 --------
 drivers/pinctrl/pinctrl-rockchip.c | 9 ++++-----
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index bae2061f15fc..0fff4a699f12 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -18,7 +18,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
-#include <linux/pinctrl/consumer.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -164,12 +163,6 @@ static int rockchip_gpio_set_direction(struct gpio_chip *chip,
 	unsigned long flags;
 	u32 data = input ? 0 : 1;
 
-
-	if (input)
-		pinctrl_gpio_direction_input(chip, offset);
-	else
-		pinctrl_gpio_direction_output(chip, offset);
-
 	raw_spin_lock_irqsave(&bank->slock, flags);
 	rockchip_gpio_writel_bit(bank, offset, data, bank->gpio_regs->port_ddr);
 	raw_spin_unlock_irqrestore(&bank->slock, flags);
@@ -593,7 +586,6 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
 	gc->ngpio = bank->nr_pins;
 	gc->label = bank->name;
 	gc->parent = bank->dev;
-	gc->can_sleep = true;
 
 	ret = gpiochip_add_data(gc, bank);
 	if (ret) {
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index e44ef262beec..2fc67aeafdb3 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3545,10 +3545,9 @@ static int rockchip_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 	return 0;
 }
 
-static int rockchip_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
-					   struct pinctrl_gpio_range *range,
-					   unsigned offset,
-					   bool input)
+static int rockchip_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
+					    struct pinctrl_gpio_range *range,
+					    unsigned int offset)
 {
 	struct rockchip_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
 	struct rockchip_pin_bank *bank;
@@ -3562,7 +3561,7 @@ static const struct pinmux_ops rockchip_pmx_ops = {
 	.get_function_name	= rockchip_pmx_get_func_name,
 	.get_function_groups	= rockchip_pmx_get_groups,
 	.set_mux		= rockchip_pmx_set,
-	.gpio_set_direction	= rockchip_pmx_gpio_set_direction,
+	.gpio_request_enable	= rockchip_pmx_gpio_request_enable,
 };
 
 /*
-- 
2.39.2.101.g768bb238c484.dirty

Re: [PATCH] gpio/rockchip: Stop calling pinctrl for set_direction

[Bartosz Golaszewski]

On Mon, Jan 26, 2026 at 1:12 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> Marking the whole controller as sleeping due to the pinctrl calls in the
> .direction_{input,output} callbacks has the unfortunate side effect that
> legitimate invocations of .get and .set, which cannot themselves sleep,
> in atomic context now spew WARN()s from gpiolib.
>
> However, as Heiko points out, the driver doing this is a bit silly to
> begin with, as the pinctrl .gpio_set_direction hook doesn't even care
> about the direction, the hook is only used to claim the mux. And sure
> enough, the .gpio_request_enable hook exists to serve this very purpose,
> so switch to that and remove the problematic business entirely.
>
> Cc: stable@vger.kernel.org
> Fixes: 20cf2aed89ac ("gpio: rockchip: mark the GPIO controller as sleeping")
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---

Linus,

With your Ack I can queue this for v6.19-rc8.

Bart

Re: [PATCH] gpio/rockchip: Stop calling pinctrl for set_direction

[Heiko Stübner]

Am Montag, 26. Januar 2026, 13:12:26 Mitteleuropäische Normalzeit schrieb Robin Murphy:
> Marking the whole controller as sleeping due to the pinctrl calls in the
> .direction_{input,output} callbacks has the unfortunate side effect that
> legitimate invocations of .get and .set, which cannot themselves sleep,
> in atomic context now spew WARN()s from gpiolib.
> 
> However, as Heiko points out, the driver doing this is a bit silly to
> begin with, as the pinctrl .gpio_set_direction hook doesn't even care
> about the direction, the hook is only used to claim the mux. And sure
> enough, the .gpio_request_enable hook exists to serve this very purpose,
> so switch to that and remove the problematic business entirely.
> 
> Cc: stable@vger.kernel.org
> Fixes: 20cf2aed89ac ("gpio: rockchip: mark the GPIO controller as sleeping")
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

This really looks great and solves the problem of the sleeping gpio-driver
in a nice way. Especially as we really only need the pinmux on request
at all.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
> If I don't have to redesign bits of the GPIO subsystem itself, this much
> I can do :)
> 
> Tested on RK3399 NanoPC-T4 with GPIOs with and without explicit pinctrl,
> and PROVE_LOCKING and DEBUG_ATOMIC_SLEEP stayed quiet (although I don't
> think this board has any shared pins)
> ---
>  drivers/gpio/gpio-rockchip.c       | 8 --------
>  drivers/pinctrl/pinctrl-rockchip.c | 9 ++++-----
>  2 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index bae2061f15fc..0fff4a699f12 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -18,7 +18,6 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> -#include <linux/pinctrl/consumer.h>
>  #include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> @@ -164,12 +163,6 @@ static int rockchip_gpio_set_direction(struct gpio_chip *chip,
>  	unsigned long flags;
>  	u32 data = input ? 0 : 1;
>  
> -
> -	if (input)
> -		pinctrl_gpio_direction_input(chip, offset);
> -	else
> -		pinctrl_gpio_direction_output(chip, offset);
> -
>  	raw_spin_lock_irqsave(&bank->slock, flags);
>  	rockchip_gpio_writel_bit(bank, offset, data, bank->gpio_regs->port_ddr);
>  	raw_spin_unlock_irqrestore(&bank->slock, flags);
> @@ -593,7 +586,6 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
>  	gc->ngpio = bank->nr_pins;
>  	gc->label = bank->name;
>  	gc->parent = bank->dev;
> -	gc->can_sleep = true;
>  
>  	ret = gpiochip_add_data(gc, bank);
>  	if (ret) {
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index e44ef262beec..2fc67aeafdb3 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -3545,10 +3545,9 @@ static int rockchip_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
>  	return 0;
>  }
>  
> -static int rockchip_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
> -					   struct pinctrl_gpio_range *range,
> -					   unsigned offset,
> -					   bool input)
> +static int rockchip_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
> +					    struct pinctrl_gpio_range *range,
> +					    unsigned int offset)
>  {
>  	struct rockchip_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
>  	struct rockchip_pin_bank *bank;
> @@ -3562,7 +3561,7 @@ static const struct pinmux_ops rockchip_pmx_ops = {
>  	.get_function_name	= rockchip_pmx_get_func_name,
>  	.get_function_groups	= rockchip_pmx_get_groups,
>  	.set_mux		= rockchip_pmx_set,
> -	.gpio_set_direction	= rockchip_pmx_gpio_set_direction,
> +	.gpio_request_enable	= rockchip_pmx_gpio_request_enable,
>  };
>  
>  /*
> 

Re: [PATCH] gpio/rockchip: Stop calling pinctrl for set_direction

[Bartosz Golaszewski]

On Mon, 26 Jan 2026 12:12:26 +0000, Robin Murphy wrote:
> Marking the whole controller as sleeping due to the pinctrl calls in the
> .direction_{input,output} callbacks has the unfortunate side effect that
> legitimate invocations of .get and .set, which cannot themselves sleep,
> in atomic context now spew WARN()s from gpiolib.
> 
> However, as Heiko points out, the driver doing this is a bit silly to
> begin with, as the pinctrl .gpio_set_direction hook doesn't even care
> about the direction, the hook is only used to claim the mux. And sure
> enough, the .gpio_request_enable hook exists to serve this very purpose,
> so switch to that and remove the problematic business entirely.
> 
> [...]

Applied, thanks!

[1/1] gpio/rockchip: Stop calling pinctrl for set_direction
      commit: 7ca497be00163610afb663867db24ac408752f13

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH] gpio/rockchip: Stop calling pinctrl for set_direction

[Linus Walleij]

On Mon, Jan 26, 2026 at 2:48 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
> On Mon, Jan 26, 2026 at 1:12 PM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > Marking the whole controller as sleeping due to the pinctrl calls in the
> > .direction_{input,output} callbacks has the unfortunate side effect that
> > legitimate invocations of .get and .set, which cannot themselves sleep,
> > in atomic context now spew WARN()s from gpiolib.
> >
> > However, as Heiko points out, the driver doing this is a bit silly to
> > begin with, as the pinctrl .gpio_set_direction hook doesn't even care
> > about the direction, the hook is only used to claim the mux. And sure
> > enough, the .gpio_request_enable hook exists to serve this very purpose,
> > so switch to that and remove the problematic business entirely.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 20cf2aed89ac ("gpio: rockchip: mark the GPIO controller as sleeping")
> > Suggested-by: Heiko Stuebner <heiko@sntech.de>
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > ---
>
> Linus,
>
> With your Ack I can queue this for v6.19-rc8.

Late to the show :)

Heiko's ACK is more than enough for this driver, he knows
Rockchip way better than me.

Yours,
Linus Walleij