From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The strict flag in struct pinmux_ops disallows the usage of the same pin
as a GPIO and for another function. Without it, a rouge user-space
process with enough privileges (or even a buggy driver) can request a
used pin as GPIO and drive it, potentially confusing devices or even
crashing the system. Set it globally for all pinctrl-msm users.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Having discussed this with Bjorn and Konrad - it seems the consensus is
that it makes sense to set this flag in pinctrl-msm. I'm marking this
patch as an RFC/RFT as I'm a bit afraid existing use-cases could break
with it but I suggest we give it a shot and see what happens.

 drivers/pinctrl/qcom/pinctrl-msm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index f713c80d7f3ed..b78492dc05adc 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -293,6 +293,7 @@ static const struct pinmux_ops msm_pinmux_ops = {
 	.get_function_groups	= msm_get_function_groups,
 	.gpio_request_enable	= msm_pinmux_request_gpio,
 	.set_mux		= msm_pinmux_set_mux,
+	.strict			= true,
 };
 
 static int msm_config_reg(struct msm_pinctrl *pctrl,
-- 
2.48.1

On 6/25/25 5:37 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The strict flag in struct pinmux_ops disallows the usage of the same pin
> as a GPIO and for another function. Without it, a rouge user-space
> process with enough privileges (or even a buggy driver) can request a
> used pin as GPIO and drive it, potentially confusing devices or even
> crashing the system. Set it globally for all pinctrl-msm users.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---

SC8280XP CRD breaks with this.. looks like there's a conflict between
regulator-fixed accessing the pin with gpiod APIs and setting a pinmux:

[    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)


Konrad

On Thu, Jun 26, 2025 at 9:06 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 6/25/25 5:37 PM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > The strict flag in struct pinmux_ops disallows the usage of the same pin
> > as a GPIO and for another function. Without it, a rouge user-space
> > process with enough privileges (or even a buggy driver) can request a
> > used pin as GPIO and drive it, potentially confusing devices or even
> > crashing the system. Set it globally for all pinctrl-msm users.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
>
> SC8280XP CRD breaks with this.. looks like there's a conflict between
> regulator-fixed accessing the pin with gpiod APIs and setting a pinmux:
>
> [    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)
>
>
> Konrad

Yeah, I would be surprised if nothing broke.It's probably worth
looking into the implementation of the strict flag as it makes every
muxed pin unavailable as GPIO even if - like in this case - the
function *is* "gpio". Of course the "gpio" string has no real meaning
to the pinctrl core, it's just a name but it would be awesome if we
could say for a given function that this means GPIO and as such should
be available to the GPIOLIB API.

Bart

On Fri, Jun 27, 2025 at 10:26 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> Yeah, I would be surprised if nothing broke.It's probably worth
> looking into the implementation of the strict flag as it makes every
> muxed pin unavailable as GPIO even if - like in this case - the
> function *is* "gpio". Of course the "gpio" string has no real meaning
> to the pinctrl core, it's just a name but it would be awesome if we
> could say for a given function that this means GPIO and as such should
> be available to the GPIOLIB API.

Can't we just add a special callback to the pinmux_ops for that?
like

int (*is_gpio_mode) (struct pinctrl_dev *pctldev, unsigned int pin);

That the core code can call to ask the driver if a pin is in GPIO
mode already? A simple strcmp("gpio", ...) is one way for the
Qualcomm driver to implement that.

Yours,
Linus Walleij

On Fri, Jul 4, 2025 at 12:21 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Jun 27, 2025 at 10:26 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > Yeah, I would be surprised if nothing broke.It's probably worth
> > looking into the implementation of the strict flag as it makes every
> > muxed pin unavailable as GPIO even if - like in this case - the
> > function *is* "gpio". Of course the "gpio" string has no real meaning
> > to the pinctrl core, it's just a name but it would be awesome if we
> > could say for a given function that this means GPIO and as such should
> > be available to the GPIOLIB API.
>
> Can't we just add a special callback to the pinmux_ops for that?
> like
>
> int (*is_gpio_mode) (struct pinctrl_dev *pctldev, unsigned int pin);
>
> That the core code can call to ask the driver if a pin is in GPIO
> mode already? A simple strcmp("gpio", ...) is one way for the
> Qualcomm driver to implement that.
>

Yeah, this is similar to what I proposed in my RFC except that I went
with a flag in struct pinfunction. I think it's more future-proof than
string comparison, especially when we also have functions called
"egpio" which, while not used now, may also need this (possibly).

It's unfortunate that the struct pinfunction list for given controller
- while defined globally - does not seem to be stored anywhere in
pinctrl core data structures. That would allow the core to query these
things like function name and - in this case - whether it's a GPIO
without going through pinmux callbacks. Maybe this could be added to
struct pinctrl_desc?

Bart