Re: [PATCH] gpio: rockchip: mark the GPIO controller as sleeping

From: Robin Murphy <robin.murphy@arm.com>
To: Bartosz Golaszewski <brgl@kernel.org>,
	Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
	Linus Walleij <linusw@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH] gpio: rockchip: mark the GPIO controller as sleeping
Date: Fri, 23 Jan 2026 13:27:02 +0000	[thread overview]
Message-ID: <447e8d5a-916b-4d58-b39c-3467c152379c@arm.com> (raw)
In-Reply-To: <CAMRc=Mf0tRxRrh7tn5OaDn3a47N_qvUcjO=zqbTi-GhY-Y9hOg@mail.gmail.com>

On 2026-01-12 9:08 am, Bartosz Golaszewski wrote:
> On Sat, Jan 10, 2026 at 12:55 AM Sebastian Reichel
> <sebastian.reichel@collabora.com> wrote:
>>
>> Hi,
>>
>> On Tue, Jan 06, 2026 at 10:00:11AM +0100, Bartosz Golaszewski wrote:
>>> The GPIO controller is configured as non-sleeping but it uses generic
>>> pinctrl helpers which use a mutex for synchronization.
>>>
>>> This can cause the following lockdep splat with shared GPIOs enabled on
>>> boards which have multiple devices using the same GPIO:
>>>
> 
> [snip]
> 
>>>
>>> Fixes: 936ee2675eee ("gpio/rockchip: add driver for rockchip gpio")
>>> Cc: stable@vger.kernel.org
>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Closes: https://lore.kernel.org/all/d035fc29-3b03-4cd6-b8ec-001f93540bc6@samsung.com/
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>>> ---
>>>   drivers/gpio/gpio-rockchip.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
>>> index 47174eb3ba76..bae2061f15fc 100644
>>> --- a/drivers/gpio/gpio-rockchip.c
>>> +++ b/drivers/gpio/gpio-rockchip.c
>>> @@ -593,6 +593,7 @@ 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;
>>
>> This means all operations are marked as can_sleep, even though
>> pinctrl operations are only used for the direction setting.
>> I.e. the common get/set operations always worked in atomic mode,
>> but now complain. See for example:
>>
>> https://lore.kernel.org/all/20260108-media-synopsys-hdmirx-fix-gpio-cansleep-v1-1-3570518d8bab@kernel.org/
>>
>> It's not a big issue for the hdmirx driver specifically, but I wonder
>> how many more (less often tested) rockchip drivers use GPIOs from their
>> IRQ handler.

Yeah, seems this finally reached my distro kernel and now the kernel log 
on one of my boards is totally flooded from gpio_ir_recv_irq() 
(legitimately) calling gpio_get_value()... that's not really OK :/

Thanks,
Robin.

>> Considering setting or getting the GPIO from atomic context is much
>> more common than changing the direction - is there some way to
>> describe the sleep behavior in a more specific way in the GPIO
>> controller?
>>
> 
> No, there's no such switch at the moment. This is because there are
> paths that we can take, where we *do* end up setting direction from
> gpiod_set_value(). For instance:
> 
> gpiod_set_value()
>    gpiod_set_value_nocheck()
>      gpio_set_open_drain_value_commit()
>        gpiochip_direction_output()
> 
> I'm afraid, for correctness, it has to be either sleeping, or not. I
> would love - at some point - to make pinctrl mostly lockless with
> SRCU, like we did with GPIO. That would solve this issue correctly.
> But until then, I'm afraid we need to keep a chip-global switch for
> sleeping.
> 
> Bartosz
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip