Re: Question: GPIO direction callbacks calling pinctrl in atomic paths

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

On Thu, Jun 18, 2026 at 11:06:09AM +0800, Runyu Xiao wrote:
> Hi,
> 
> While auditing GPIO direction callbacks, our static analysis tool flagged
> several drivers whose direction_input/direction_output paths call into
> the pinctrl core even though the GPIO chip is registered as non-sleeping.
> We then manually reviewed the findings against the current tree.
> 
> The class of path we looked at is:
> 
>   gpiod_direction_output_raw_commit()
>     -> <driver>_gpio_direction_output()
>        -> pinctrl_gpio_direction_output()
>        -> pinctrl_get_device_gpio_range()
>        -> mutex_lock(&pctldev->mutex)
> 
> That can be reached from shared GPIO users while a per-line spinlock is
> still held.  A minimal Lockdep reproducer preserving this direction path
> reports:
> 
>   BUG: sleeping function called from invalid context
>   #0: ... (&global_shared_desc.spinlock) ...
>   pinctrl_get_device_gpio_range
>   <driver>_gpio_direction_output
>   [ BUG: Invalid wait context ]
> 
> My first draft for this class was to mark the affected gpio_chip as
> can_sleep, but that looks like the wrong contract.  gpio_chip::can_sleep
> describes whether get()/set() may sleep, while the problematic operation
> here is not MMIO value access but an extra pinctrl direction round-trip.
> Rockchip history seems to support that concern: after the controller was
> marked sleeping, a follow-up change stopped calling pinctrl for
> set_direction because whole-chip can_sleep caused atomic get/set
> warnings.
> 
> For PXA and Tegra, I am considering a small series that removes the
> pinctrl_gpio_direction_input/output() calls from the GPIO direction
> callbacks and leaves direction programming on the drivers' existing MMIO
> paths.
> 
> For PXA, the driver already updates GPDR directly in
> pxa_gpio_direction_input/output().  The proposed change would drop the
> additional pinctrl direction call on variants where pxa_gpio_has_pinctrl()
> currently returns true.
> 
> For Tegra, the GPIO driver already programs the GPIO controller direction
> registers directly.  The Tegra pinmux ops appear to provide GPIO
> request/free handling, but no gpio_set_direction hook, so the
> pinctrl_gpio_direction_input/output() call seems to enter the pinctrl core
> without adding a Tegra-specific direction operation.  The proposed change
> would keep pinctrl involvement in request/free but not in GPIO direction.

I looked into this, and yes, we don't provide gpio_set_direction
callbacks for the Tegra pinctrl driver, so what you're proposing looks
fine.

However, I'm on the fence about this because I think conceptually it is
correct to call into the pinctrl subsystem to set the direction. The
GPIO driver should be oblivious to the fact that it isn't strictly
necessary.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]