All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Diederik de Haas" <diederik@cknow-tech.com>
To: "Robin Murphy" <robin.murphy@arm.com>,
	"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 21:02:19 +0100	[thread overview]
Message-ID: <DFW8JGOI41QK.2AFPP8SSK8TH1@cknow-tech.com> (raw)
In-Reply-To: <447e8d5a-916b-4d58-b39c-3467c152379c@arm.com>

On Fri Jan 23, 2026 at 2:27 PM CET, Robin Murphy wrote:
> 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:
>>> 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 :/

Yeah, I'm getting it too on several of my boards, like on Rock64:
https://paste.sr.ht/~diederik/154c5023a3a50d77f1da2195e7bb9a96f6a88555
(that's just a fraction as dmesg ran out of its buffer ...)
Also mentioned here:
https://lore.kernel.org/all/DFOEGOTI1AQ9.175GP7V1VK1XU@cknow-tech.com/

Diederik

>
> 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
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip



WARNING: multiple messages have this Message-ID (diff)
From: "Diederik de Haas" <diederik@cknow-tech.com>
To: "Robin Murphy" <robin.murphy@arm.com>,
	"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 21:02:19 +0100	[thread overview]
Message-ID: <DFW8JGOI41QK.2AFPP8SSK8TH1@cknow-tech.com> (raw)
In-Reply-To: <447e8d5a-916b-4d58-b39c-3467c152379c@arm.com>

On Fri Jan 23, 2026 at 2:27 PM CET, Robin Murphy wrote:
> 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:
>>> 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 :/

Yeah, I'm getting it too on several of my boards, like on Rock64:
https://paste.sr.ht/~diederik/154c5023a3a50d77f1da2195e7bb9a96f6a88555
(that's just a fraction as dmesg ran out of its buffer ...)
Also mentioned here:
https://lore.kernel.org/all/DFOEGOTI1AQ9.175GP7V1VK1XU@cknow-tech.com/

Diederik

>
> 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
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  parent reply	other threads:[~2026-01-23 20:02 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-06  9:00 [PATCH] gpio: rockchip: mark the GPIO controller as sleeping Bartosz Golaszewski
2026-01-06  9:00 ` Bartosz Golaszewski
2026-01-06 17:10 ` Heiko Stuebner
2026-01-06 17:10   ` Heiko Stuebner
2026-01-07  8:31 ` Bartosz Golaszewski
2026-01-07  8:31   ` Bartosz Golaszewski
2026-01-09 23:55 ` Sebastian Reichel
2026-01-09 23:55   ` Sebastian Reichel
2026-01-12  9:08   ` Bartosz Golaszewski
2026-01-12  9:08     ` Bartosz Golaszewski
2026-01-23 13:27     ` Robin Murphy
2026-01-23 13:27       ` Robin Murphy
2026-01-23 19:27       ` Bartosz Golaszewski
2026-01-23 19:27         ` Bartosz Golaszewski
2026-01-23 20:57         ` Robin Murphy
2026-01-23 20:57           ` Robin Murphy
2026-01-23 21:52           ` Heiko Stübner
2026-01-23 21:52             ` Heiko Stübner
2026-01-23 23:45             ` Robin Murphy
2026-01-23 23:45               ` Robin Murphy
2026-01-24 21:07               ` Bartosz Golaszewski
2026-01-24 21:07                 ` Bartosz Golaszewski
2026-01-25 13:23                 ` Heiko Stübner
2026-01-25 13:23                   ` Heiko Stübner
2026-01-26  8:49                   ` Bartosz Golaszewski
2026-01-26  8:49                     ` Bartosz Golaszewski
2026-01-23 20:02       ` Diederik de Haas [this message]
2026-01-23 20:02         ` Diederik de Haas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DFW8JGOI41QK.2AFPP8SSK8TH1@cknow-tech.com \
    --to=diederik@cknow-tech.com \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=brgl@kernel.org \
    --cc=heiko@sntech.de \
    --cc=linusw@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=m.szyprowski@samsung.com \
    --cc=robin.murphy@arm.com \
    --cc=sebastian.reichel@collabora.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.