pinctrl: qcom: make the pinmuxing strict

From: Bartosz Golaszewski <[email protected]>

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.

---
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(+)

index f713c80d7f3ed..b78492dc05adc 100644
@@ -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

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

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

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