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

[Runyu Xiao]

Hi,

While auditing pinctrl-backed GPIO chips, our static analysis tool
flagged several controllers that expose gpiochip_generic_config() while
still advertising a non-sleeping gpio_chip.  We then manually reviewed
the findings against the current tree.

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)

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.  A minimal Lockdep reproducer preserving this carrier reports:

  BUG: sleeping function called from invalid context
  #0: ... (&global_shared_desc.spinlock) ...
  pinctrl_get_device_gpio_range
  gpiochip_generic_config
  [ BUG: Invalid wait context ]

This looks similar to the Meson fix that marked the GPIO controller as
sleeping when GPIO configuration is routed through the pinctrl-backed
generic config path.

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.

Two other candidates from the same audit, bcm2835 and keembay, are not
part of this draft.  They use gpio_irq_chip::parent_handler, and gpiolib
rejects chained interrupts on a GPIO chip that may sleep.  Setting
can_sleep there would therefore break registration instead of fixing the
contract; those cases need a separate irqchip design review.

Does setting gpio_chip::can_sleep for the at91-pio4, stm32 and sunxi
pinctrl-backed GPIO chips sound like the right fix for this set_config
contract mismatch?  Or would you prefer this to be handled elsewhere in
gpiolib/pinctrl so that set_config sleepability can be represented more
narrowly than the whole gpio_chip?

Thanks,
Runyu