Re: [PATCH v6 00/15] pinctrl: introduce the concept of a GPIO pin function category

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 28/08/2025 18:00, Bartosz Golaszewski wrote:
Problem: when pinctrl core binds pins to a consumer device and the
pinmux ops of the underlying driver are marked as strict, the pin in
question can no longer be requested as a GPIO using the GPIO descriptor
API. It will result in the following error:

[    5.095688] sc8280xp-tlmm f100000.pinctrl: pin GPIO_25 already requested by regulator-edp-3p3; cannot claim for f100000.pinctrl:570
[    5.107822] sc8280xp-tlmm f100000.pinctrl: error -EINVAL: pin-25 (f100000.pinctrl:570)

This typically makes sense except when the pins are muxed to a function
that actually says "GPIO". Of course, the function name is just a string
so it has no meaning to the pinctrl subsystem.

We have many Qualcomm SoCs (and I can imagine it's a common pattern in
other platforms as well) where we mux a pin to "gpio" function using the
`pinctrl-X` property in order to configure bias or drive-strength and
then access it using the gpiod API. This makes it impossible to mark the
pin controller module as "strict".

This series proposes to introduce a concept of a sub-category of
pinfunctions: GPIO functions where the above is not true and the pin
muxed as a GPIO can still be accessed via the GPIO consumer API even for
strict pinmuxers.

To that end: we first clean up the drivers that use struct function_desc
and make them use the smaller struct pinfunction instead - which is the
correct structure for drivers to describe their pin functions with. We
also rework pinmux core to not duplicate memory used to store the
pinfunctions unless they're allocated dynamically.

First: provide the kmemdup_const() helper which only duplicates memory
if it's not in the .rodata section. Then rework all pinctrl drivers that
instantiate objects of type struct function_desc as they should only be
created by pinmux core. Next constify the return value of the accessor
used to expose these structures to users and finally convert the
pinfunction object within struct function_desc to a pointer and use
kmemdup_const() to assign it. With this done proceed to add
infrastructure for the GPIO pin function category and use it in Qualcomm
drivers. At the very end: make the Qualcomm pinmuxer strict.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
---
Changes in v6:
- Select GENERIC_PINMUX_FUNCTIONS when using generic pinmux helpers in
   qcom pinctrl drivers to fix build on ARM 32-bit platforms
- Assume that a pin can be requested in pin_request() if it has no
   mux_setting assigned
- Also check if a function is a GPIO for pins within GPIO ranges
- Fix an issue with the imx pinctrl driver where the conversion patch
   confused the function and pin group radix trees
- Add a FIXME to the imx driver mentioning the need to switch to the
   provided helpers for accessing the group radix tree
- Link to v5: https://lore.kernel.org/r/20250815-pinctrl-gpio-pinfuncs-v5-0-955de9fd91db@xxxxxxxxxx

Changes in v5:
- Fix a potential NULL-pointer dereference in
   pinmux_can_be_used_for_gpio()
- Use PINCTRL_PINFUNCTION() in pinctrl-airoha
- Link to v4: https://lore.kernel.org/r/20250812-pinctrl-gpio-pinfuncs-v4-0-bb3906c55e64@xxxxxxxxxx

Changes in v4:
- Update the GPIO pin function definitions to include the new qcom
   driver (milos)
- Provide devm_kmemdup_const() instead of a non-managed kmemdup_const()
   as a way to avoid casting out the 'const' modifier when passing the
   const pointer to devm_add_action_or_reset()
- Use devm_krealloc_array() where applicable instead of devm_krealloc()
- Fix typos
- Fix kerneldocs
- Improve commit messages
- Small tweaks as pointed out by Andy
- Rebased on top of v6.17-rc1
- Link to v3: https://lore.kernel.org/r/20250724-pinctrl-gpio-pinfuncs-v3-0-af4db9302de4@xxxxxxxxxx

Changes in v3:
- Add more patches in front: convert pinctrl drivers to stop defining
   their own struct function_desc objects and make pinmux core not
   duplicate .rodata memory in which struct pinfunction objects are
   stored.
- Add a patch constifying pinmux_generic_get_function().
- Drop patches that were applied upstream.
- Link to v2: https://lore.kernel.org/r/20250709-pinctrl-gpio-pinfuncs-v2-0-b6135149c0d9@xxxxxxxxxx

Changes in v2:
- Extend the series with providing pinmux_generic_add_pinfunction(),
   using it in several drivers and converting pinctrl-msm to using
   generic pinmux helpers
- Add a generic function_is_gpio() callback for pinmux_ops
- Convert all qualcomm drivers to using the new GPIO pin category so
   that we can actually enable the strict flag
- Link to v1: https://lore.kernel.org/r/20250702-pinctrl-gpio-pinfuncs-v1-0-ed2bd0f9468d@xxxxxxxxxx

---
Bartosz Golaszewski (15):
       devres: provide devm_kmemdup_const()
       pinctrl: ingenic: use struct pinfunction instead of struct function_desc
       pinctrl: airoha: replace struct function_desc with struct pinfunction
       pinctrl: mediatek: mt7988: use PINCTRL_PIN_FUNCTION()
       pinctrl: mediatek: moore: replace struct function_desc with struct pinfunction
       pinctrl: imx: don't access the pin function radix tree directly
       pinctrl: keembay: release allocated memory in detach path
       pinctrl: keembay: use a dedicated structure for the pinfunction description
       pinctrl: constify pinmux_generic_get_function()
       pinctrl: make struct pinfunction a pointer in struct function_desc
       pinctrl: qcom: use generic pin function helpers
       pinctrl: allow to mark pin functions as requestable GPIOs
       pinctrl: qcom: add infrastructure for marking pin functions as GPIOs
       pinctrl: qcom: mark the `gpio` and `egpio` pins function as non-strict functions
       pinctrl: qcom: make the pinmuxing strict

  drivers/base/devres.c                            | 21 ++++++++
  drivers/pinctrl/freescale/pinctrl-imx.c          | 43 +++++++--------
  drivers/pinctrl/mediatek/pinctrl-airoha.c        | 19 +++----
  drivers/pinctrl/mediatek/pinctrl-moore.c         | 10 ++--
  drivers/pinctrl/mediatek/pinctrl-moore.h         |  7 +--
  drivers/pinctrl/mediatek/pinctrl-mt7622.c        |  2 +-
  drivers/pinctrl/mediatek/pinctrl-mt7623.c        |  2 +-
  drivers/pinctrl/mediatek/pinctrl-mt7629.c        |  2 +-
  drivers/pinctrl/mediatek/pinctrl-mt7981.c        |  2 +-
  drivers/pinctrl/mediatek/pinctrl-mt7986.c        |  2 +-
  drivers/pinctrl/mediatek/pinctrl-mt7988.c        | 44 ++++++---------
  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h |  2 +-
  drivers/pinctrl/pinctrl-equilibrium.c            |  2 +-
  drivers/pinctrl/pinctrl-ingenic.c                | 49 ++++++++---------
  drivers/pinctrl/pinctrl-keembay.c                | 26 +++++----
  drivers/pinctrl/pinctrl-single.c                 |  4 +-
  drivers/pinctrl/pinmux.c                         | 68 ++++++++++++++++++++----
  drivers/pinctrl/pinmux.h                         |  9 ++--
  drivers/pinctrl/qcom/Kconfig                     |  1 +
  drivers/pinctrl/qcom/pinctrl-ipq5018.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-ipq5332.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-ipq5424.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-ipq6018.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-ipq8074.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-ipq9574.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-mdm9607.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-mdm9615.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-milos.c             |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm.c               | 45 ++++++----------
  drivers/pinctrl/qcom/pinctrl-msm.h               |  5 ++
  drivers/pinctrl/qcom/pinctrl-msm8226.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8660.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8909.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8916.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8917.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8953.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8960.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8976.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8994.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8996.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8998.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-msm8x74.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-qcm2290.c           |  4 +-
  drivers/pinctrl/qcom/pinctrl-qcs404.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-qcs615.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-qcs8300.c           |  4 +-
  drivers/pinctrl/qcom/pinctrl-qdu1000.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-sa8775p.c           |  4 +-
  drivers/pinctrl/qcom/pinctrl-sar2130p.c          |  2 +-
  drivers/pinctrl/qcom/pinctrl-sc7180.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sc7280.c            |  4 +-
  drivers/pinctrl/qcom/pinctrl-sc8180x.c           |  2 +-
  drivers/pinctrl/qcom/pinctrl-sc8280xp.c          |  4 +-
  drivers/pinctrl/qcom/pinctrl-sdm660.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sdm670.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sdm845.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sdx55.c             |  2 +-
  drivers/pinctrl/qcom/pinctrl-sdx65.c             |  2 +-
  drivers/pinctrl/qcom/pinctrl-sdx75.c             |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm4450.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm6115.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm6125.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm6350.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm6375.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm7150.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm8150.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm8250.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm8350.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm8450.c            |  4 +-
  drivers/pinctrl/qcom/pinctrl-sm8550.c            |  2 +-
  drivers/pinctrl/qcom/pinctrl-sm8650.c            |  4 +-
  drivers/pinctrl/qcom/pinctrl-sm8750.c            |  4 +-
  drivers/pinctrl/qcom/pinctrl-x1e80100.c          |  2 +-
  drivers/pinctrl/renesas/pinctrl-rza1.c           |  2 +-
  drivers/pinctrl/renesas/pinctrl-rza2.c           |  2 +-
  drivers/pinctrl/renesas/pinctrl-rzg2l.c          |  2 +-
  drivers/pinctrl/renesas/pinctrl-rzv2m.c          |  2 +-
  include/linux/device/devres.h                    |  2 +
  include/linux/pinctrl/pinctrl.h                  | 14 +++++
  include/linux/pinctrl/pinmux.h                   |  2 +
  80 files changed, 287 insertions(+), 224 deletions(-)
---
base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
change-id: 20250701-pinctrl-gpio-pinfuncs-de82bd9aac43

Best regards,

Tested on a bunch on qcom boards, no issues observed, regression of v5 no longers appears.

Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>

Probably only applies on patches 1, 9, 10, 11, 12, 13, 14, & 15

Neil




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux