Re: Question: pinctrl-backed GPIO set_config and gpio_chip::can_sleep

[Linus Walleij]

Hi Runyu,

thanks for your analysis!

On Thu, Jun 18, 2026 at 7:42 AM Runyu Xiao <runyu.xiao@xxxxxxxxxx> wrote:

> The path we are concerned about is:
>
>   gpiod_set_config()
>     -> gpio_do_set_config()
>        -> gpiochip_generic_config()
>        -> pinctrl_gpio_set_config()
>        -> pinctrl_get_device_gpio_range()
>        -> mutex_lock(&pctldev->mutex)

That would be mutex_lock(&pinctrldev_list_mutex); would it not?

> If gpiod_cansleep() returns false, a GPIO forwarder or another consumer
> can choose an atomic carrier and call gpiod_set_config() while holding a
> spinlock.

I see the problem.

> The local draft I am considering marks only these controllers as
> sleeping:
>
>   pinctrl: at91-pio4: mark the GPIO controller as sleeping
>   pinctrl: stm32: mark the GPIO controller as sleeping
>   pinctrl: sunxi: mark the GPIO controller as sleeping
>
> The reason is that all three expose gpiochip_generic_config() and can
> therefore reach the pinctrl mutex from the GPIO set_config path, while
> their current gpio_chip registration does not set can_sleep.

But that's not the right solution is it? These controllers can probably
just write a register and be done with it, they all look like they are
memory-mapped? That means we introduce sleep where not needed.

Can we simply replace pinctrldev_list_mutex with a spinlock?
The list isn't gonna be huge and all in-memory anyway.
If it takes too much time we need to think about putting the
ranges in a better data structure such as the maple tree.

mutex_lock(&pinctrldev_list_mutex); could then be turned
into spinlock_irqsave() or even better
guard(spinlock_irqsave)(&pinctrldev_list_lock) in
pinctrl_get_device_gpio_range().

This would mean we just take two different spinlocks in seqence
and save state in each so it should work just fine.

Yours,
Linus Walleij