mbox series

[v3,0/3] pinctrl: msm interrupt and muxing fixes

Message ID 20180816200648.90458-1-swboyd@chromium.org
Headers show
Series pinctrl: msm interrupt and muxing fixes | expand

Message

Stephen Boyd Aug. 16, 2018, 8:06 p.m. UTC
Here's a collection of pinctrl fixes for the qcom driver that
make things a little smoother for DT writers while also fixing
a problem seen with level triggered interrupts.

The first patch fixes an issue where we always see one extra level
triggered interrupt when the interrupt triggers. The second and third
patches make things nice for DT writers so they don't have to explicitly
mux out pins as 'GPIO' function and as 'input' instead of output so that
interrupts work properly and also makes sure that a gpio is muxed out
properly to 'GPIO' function when a gpio is requested by gpiod_request()
and friends.

The discussion never really completed on the previous thread so I'm just
resending these patches to restart the conversation. In the cases
where a driver needs to do both pinctrl muxing for some non-gpio
function and also GPIO control they'll need to explicitly mux the pins
at the right time. If we force them to mux the pins into the function
mode after requesting the GPIO at boot then we'll be better off because
it will force the code to mux out the function or GPIO explicitly all
the time.

We will have a case in the near future where the UART driver will want
to mux the RX pin into GPIO mode so it can get a wakeup interrupt during
suspend path and then swizzle the pin back into QUP/UART mode when the
wakeup interrupt isn't necessary anymore. In this case, I imagine the
driver will request the pin as an interrupt during probe, that will
convert the GPIO into an irq and mux it out as a GPIO function input
pin, disable that IRQ because it's only needed at suspend time, and then
need to explicitly mux the device into "UART" mode before finishing
driver probe. Then when it goes into suspend, it will need to remux the
pin as a GPIO function input pin, enable the irq, and wait for wakeup.
On resume, it will disable the irq and remux the pin as "UART" mode.

Changes from v2:
 * Better comment in patch#1 to describe this stuff
 * Squashed the raw status bit part into the same write in mask path
   based on suggestion from Doug Andersson

Changes from v1:
 * Squashed the raw status bit part into the same write in unmask path
   based on suggestion from Doug Andersson

Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Doug Anderson <dianders@chromium.org>

Stephen Boyd (3):
  pinctrl: msm: Really mask level interrupts to prevent latching
  pinctrl: msm: Mux out gpio function with gpio_request()
  pinctrl: msm: Configure interrupts as input and gpio mode

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

Comments

Niklas Cassel Sept. 12, 2018, 10:34 a.m. UTC | #1
Hello Stephen,

I'm getting this warning on dragonboard 820c (msm8996) when booting linux-next:

[    3.211575] WARNING: CPU: 1 PID: 1 at drivers/pinctrl/qcom/pinctrl-msm.c:164 msm_pinmux_set_mux+0xc8/0x150
[    3.212127] l28: ramp_delay not set
[    3.215146] Modules linked in:
[    3.215168] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G        W         4.19.0-rc2-next-2018904-00004-gd8c6804d9e9b #54
[    3.215175] Hardware name: Qualcomm Technologies, Inc. DB820c (DT)
[    3.215184] pstate: 60400005 (nZCv daif +PAN -UAO)
[    3.215197] pc : msm_pinmux_set_mux+0xc8/0x150
[    3.215206] lr : msm_pinmux_set_mux+0x30/0x150
[    3.215213] sp : ffff00000803b880
[    3.224815] l28: 925 mV
[    3.228149] x29: ffff00000803b880 x28: ffff00000978a22c
[    3.228172] x27: ffff000009f22000 x26: ffff000009640764
[    3.228193] x25: 0000000000000000
[    3.241956] x24: ffff8000d93f5f18
[    3.241971] x23: ffff8000d93fec18 x22: 0000000000000000
[    3.241992] x21: ffff000009100d10 x20: 000000000000003c
[    3.252891] x19: 000000000000000a x18: ffffffffffffffff
[    3.252913] x17: 00000000000017c7 x16: ffff000009fdaac0
[    3.252935] x15: ffff000009cbe1c8
[    3.257467] l29: supplied by regulator-dummy
[    3.261743] x14: ffff8000d85fea14
[    3.261757] x13: ffff8000d85fea13 x12: ffff8000d9f488b0
[    3.261778] x11: 0000000005f5e0ff x10: 0000000000000040
[    3.267734] x9 : ffff8000d91bae18 x8 : ffff8000d981efe8
[    3.267755] x7 : ffff8000d981f128 x6 : 000000000000000a
[    3.267776] x5 : 0000000000000040
[    3.273347] l29: Bringing 0uV into 2800000-2800000uV
[    3.278323] x4 : ffffffffffffffff
[    3.278338] x3 : 000000000000000a x2 : 00000000ffffffc6
[    3.278359] x1 : ffff000009d3eec8 x0 : 00000000000000f3
[    3.282026] l29: ramp_delay not set
[    3.292566] Call trace:
[    3.292581]  msm_pinmux_set_mux+0xc8/0x150
[    3.292590]  msm_pinmux_request_gpio+0x5c/0x68
[    3.292599]  pin_request+0x154/0x2c0
[    3.292608]  pinmux_request_gpio+0x60/0xa0
[    3.296080] l29: 2800 mV
[    3.301503]  pinctrl_gpio_request+0x148/0x1d8
[    3.301514]  gpiochip_generic_request+0x2c/0x38
[    3.301523]  gpiod_request_commit+0xc4/0x1a0
[    3.301532]  gpiod_request+0x6c/0x110
[    3.301544]  gpiod_get_index+0x134/0x360
[    3.322166]  devm_gpiod_get_index+0x64/0xc0
[    3.322174]  devm_gpiod_get_optional+0x38/0x58
[    3.322185]  qcom_pcie_probe+0xa4/0x230
[    3.322197]  platform_drv_probe+0x58/0xa8
[    3.322207]  really_probe+0x280/0x3d8
[    3.322215]  driver_probe_device+0x60/0x148
[    3.322224]  __driver_attach+0x144/0x148
[    3.322233]  bus_for_each_dev+0x84/0xd8
[    3.322241]  driver_attach+0x30/0x40
[    3.322250]  bus_add_driver+0x234/0x2a8
[    3.322258]  driver_register+0x64/0x110
[    3.322267]  __platform_driver_register+0x54/0x60
[    3.322279]  qcom_pcie_driver_init+0x20/0x28
[    3.322290]  do_one_initcall+0x94/0x3f8
[    3.322301]  kernel_init_freeable+0x47c/0x528
[    3.322315]  kernel_init+0x18/0x110
[    3.322323]  ret_from_fork+0x10/0x1c


If I revert patch 2/3 and 3/3 in this series, the warning goes away.


Kind regards,
Niklas
Stephen Boyd Oct. 1, 2018, 6:46 p.m. UTC | #2
Quoting Niklas Cassel (2018-09-12 03:34:31)
> Hello Stephen,
> 
> I'm getting this warning on dragonboard 820c (msm8996) when booting linux-next:
> 
> [    3.211575] WARNING: CPU: 1 PID: 1 at drivers/pinctrl/qcom/pinctrl-msm.c:164 msm_pinmux_set_mux+0xc8/0x150
> [    3.212127] l28: ramp_delay not set

Thanks for testing. I think the problem is I messed up the function 0
and enum 0. Can you try this patch?

--8<---
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 1684b2da09d5..b925b8feac95 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -188,7 +188,7 @@ static int msm_pinmux_request_gpio(struct pinctrl_dev *pctldev,
 		return 0;
 
 	/* For now assume function 0 is GPIO because it always is */
-	return msm_pinmux_set_mux(pctldev, 0, offset);
+	return msm_pinmux_set_mux(pctldev, g->funcs[0], offset);
 }
 
 static const struct pinmux_ops msm_pinmux_ops = {
Niklas Cassel Oct. 1, 2018, 7:34 p.m. UTC | #3
On Mon, Oct 01, 2018 at 11:46:57AM -0700, Stephen Boyd wrote:
> Quoting Niklas Cassel (2018-09-12 03:34:31)
> > Hello Stephen,
> > 
> > I'm getting this warning on dragonboard 820c (msm8996) when booting linux-next:
> > 
> > [    3.211575] WARNING: CPU: 1 PID: 1 at drivers/pinctrl/qcom/pinctrl-msm.c:164 msm_pinmux_set_mux+0xc8/0x150
> > [    3.212127] l28: ramp_delay not set
> 
> Thanks for testing. I think the problem is I messed up the function 0
> and enum 0. Can you try this patch?

Hello Stephen,

This patch does indeed remove the warning.
Thanks!

Kind regards,
Niklas

> 
> --8<---
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 1684b2da09d5..b925b8feac95 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -188,7 +188,7 @@ static int msm_pinmux_request_gpio(struct pinctrl_dev *pctldev,
>  		return 0;
>  
>  	/* For now assume function 0 is GPIO because it always is */
> -	return msm_pinmux_set_mux(pctldev, 0, offset);
> +	return msm_pinmux_set_mux(pctldev, g->funcs[0], offset);
>  }
>  
>  static const struct pinmux_ops msm_pinmux_ops = {