linux-kernel - Re: [PATCH] gpio/rockchip: Stop calling pinctrl for set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <10827351.CDJkKcVGEf@diego>
Date: Mon, 26 Jan 2026 17:38:25 +0100
From: Heiko Stübner <heiko@...ech.de>
To: brgl@...nel.org, linusw@...nel.org, Robin Murphy <robin.murphy@....com>
Cc: sebastian.reichel@...labora.com, m.szyprowski@...sung.com,
 linux-rockchip@...ts.infradead.org, linux-gpio@...r.kernel.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] gpio/rockchip: Stop calling pinctrl for set_direction

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@...r.kernel.org
> Fixes: 20cf2aed89ac ("gpio: rockchip: mark the GPIO controller as sleeping")
> Suggested-by: Heiko Stuebner <heiko@...ech.de>
> Signed-off-by: Robin Murphy <robin.murphy@....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@...ech.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,
>  };
>  
>  /*
>