| Message ID | 20240830-x1e80100-bypass-pdc-v1-1-d4c00be0c3e3@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now | expand |
On Fri, Aug 30, 2024 at 11:09:07AM +0200, Stephan Gerhold wrote: > On X1E80100, GPIO interrupts for wakeup-capable pins have been broken since > the introduction of the pinctrl driver. This prevents keyboard and touchpad > from working on most of the X1E laptops. So far we have worked around this > by manually building a kernel with the "wakeup-parent" removed from the > pinctrl node in the device tree, but we cannot expect all users to do that. > > Implement a similar workaround in the driver by clearing the wakeirq_map > for X1E80100. This avoids using the PDC wakeup parent for all GPIOs > and handles the interrupts directly in the pinctrl driver instead. > > The PDC driver needs additional changes to support X1E80100 properly. > Adding a workaround separately first allows to land the necessary PDC > changes through the normal release cycle, while still solving the more > critical problem with keyboard and touchpad on the current stable kernel > versions. Bypassing the PDC is enough for now, because we have not yet > enabled the deep idle states where using the PDC becomes necessary. > > Cc: stable@vger.kernel.org > Fixes: 05e4941d97ef ("pinctrl: qcom: Add X1E80100 pinctrl driver") > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> I can confirm that this fixes the keyboard and touchpad interrupts on the x1e80100 CRD as expected: Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Tested-by: Johan Hovold <johan+linaro@kernel.org> Johan
On 30.08.2024 11:09 AM, Stephan Gerhold wrote: > On X1E80100, GPIO interrupts for wakeup-capable pins have been broken since > the introduction of the pinctrl driver. This prevents keyboard and touchpad > from working on most of the X1E laptops. So far we have worked around this > by manually building a kernel with the "wakeup-parent" removed from the > pinctrl node in the device tree, but we cannot expect all users to do that. > > Implement a similar workaround in the driver by clearing the wakeirq_map > for X1E80100. This avoids using the PDC wakeup parent for all GPIOs > and handles the interrupts directly in the pinctrl driver instead. > > The PDC driver needs additional changes to support X1E80100 properly. > Adding a workaround separately first allows to land the necessary PDC > changes through the normal release cycle, while still solving the more > critical problem with keyboard and touchpad on the current stable kernel > versions. Bypassing the PDC is enough for now, because we have not yet > enabled the deep idle states where using the PDC becomes necessary. > > Cc: stable@vger.kernel.org > Fixes: 05e4941d97ef ("pinctrl: qcom: Add X1E80100 pinctrl driver") > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> > --- Reviewed-by: Konrad Dybcio <konradybcio@kernel.org> Konrad
On 24-08-30 11:09:07, Stephan Gerhold wrote: > On X1E80100, GPIO interrupts for wakeup-capable pins have been broken since > the introduction of the pinctrl driver. This prevents keyboard and touchpad > from working on most of the X1E laptops. So far we have worked around this > by manually building a kernel with the "wakeup-parent" removed from the > pinctrl node in the device tree, but we cannot expect all users to do that. > > Implement a similar workaround in the driver by clearing the wakeirq_map > for X1E80100. This avoids using the PDC wakeup parent for all GPIOs > and handles the interrupts directly in the pinctrl driver instead. > > The PDC driver needs additional changes to support X1E80100 properly. > Adding a workaround separately first allows to land the necessary PDC > changes through the normal release cycle, while still solving the more > critical problem with keyboard and touchpad on the current stable kernel > versions. Bypassing the PDC is enough for now, because we have not yet > enabled the deep idle states where using the PDC becomes necessary. > > Cc: stable@vger.kernel.org > Fixes: 05e4941d97ef ("pinctrl: qcom: Add X1E80100 pinctrl driver") > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> Reviewed-by: Abel Vesa <abel.vesa@linaro.org> > --- > Commenting out .wakeirq_map as well would give an unused declaration > warning for x1e80100_pdc_map. The map itself is correct, so I just "clear" > it by setting .nwakeirq_map to 0 for now. It's just temporary - this patch > will be reverted after we add X1E80100 support to the PDC driver. > --- > drivers/pinctrl/qcom/pinctrl-x1e80100.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/qcom/pinctrl-x1e80100.c b/drivers/pinctrl/qcom/pinctrl-x1e80100.c > index 65ed933f05ce..abfcdd3da9e8 100644 > --- a/drivers/pinctrl/qcom/pinctrl-x1e80100.c > +++ b/drivers/pinctrl/qcom/pinctrl-x1e80100.c > @@ -1839,7 +1839,9 @@ static const struct msm_pinctrl_soc_data x1e80100_pinctrl = { > .ngroups = ARRAY_SIZE(x1e80100_groups), > .ngpios = 239, > .wakeirq_map = x1e80100_pdc_map, > - .nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map), > + /* TODO: Enabling PDC currently breaks GPIO interrupts */ > + .nwakeirq_map = 0, > + /* .nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map), */ > .egpio_func = 9, > }; > > > --- > base-commit: 128f71fe014fc91efa1407ce549f94a9a9f1072c > change-id: 20240830-x1e80100-bypass-pdc-39a8c1ae594f > > Best regards, > -- > Stephan Gerhold <stephan.gerhold@linaro.org> >
On Fri, Aug 30, 2024 at 11:09 AM Stephan Gerhold <stephan.gerhold@linaro.org> wrote: > On X1E80100, GPIO interrupts for wakeup-capable pins have been broken since > the introduction of the pinctrl driver. This prevents keyboard and touchpad > from working on most of the X1E laptops. So far we have worked around this > by manually building a kernel with the "wakeup-parent" removed from the > pinctrl node in the device tree, but we cannot expect all users to do that. > > Implement a similar workaround in the driver by clearing the wakeirq_map > for X1E80100. This avoids using the PDC wakeup parent for all GPIOs > and handles the interrupts directly in the pinctrl driver instead. > > The PDC driver needs additional changes to support X1E80100 properly. > Adding a workaround separately first allows to land the necessary PDC > changes through the normal release cycle, while still solving the more > critical problem with keyboard and touchpad on the current stable kernel > versions. Bypassing the PDC is enough for now, because we have not yet > enabled the deep idle states where using the PDC becomes necessary. > > Cc: stable@vger.kernel.org > Fixes: 05e4941d97ef ("pinctrl: qcom: Add X1E80100 pinctrl driver") > Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> Patch applied for fixes, excellent work as always. Thanks! Yours, Linus Walleij
diff --git a/drivers/pinctrl/qcom/pinctrl-x1e80100.c b/drivers/pinctrl/qcom/pinctrl-x1e80100.c index 65ed933f05ce..abfcdd3da9e8 100644 --- a/drivers/pinctrl/qcom/pinctrl-x1e80100.c +++ b/drivers/pinctrl/qcom/pinctrl-x1e80100.c @@ -1839,7 +1839,9 @@ static const struct msm_pinctrl_soc_data x1e80100_pinctrl = { .ngroups = ARRAY_SIZE(x1e80100_groups), .ngpios = 239, .wakeirq_map = x1e80100_pdc_map, - .nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map), + /* TODO: Enabling PDC currently breaks GPIO interrupts */ + .nwakeirq_map = 0, + /* .nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map), */ .egpio_func = 9, };
On X1E80100, GPIO interrupts for wakeup-capable pins have been broken since the introduction of the pinctrl driver. This prevents keyboard and touchpad from working on most of the X1E laptops. So far we have worked around this by manually building a kernel with the "wakeup-parent" removed from the pinctrl node in the device tree, but we cannot expect all users to do that. Implement a similar workaround in the driver by clearing the wakeirq_map for X1E80100. This avoids using the PDC wakeup parent for all GPIOs and handles the interrupts directly in the pinctrl driver instead. The PDC driver needs additional changes to support X1E80100 properly. Adding a workaround separately first allows to land the necessary PDC changes through the normal release cycle, while still solving the more critical problem with keyboard and touchpad on the current stable kernel versions. Bypassing the PDC is enough for now, because we have not yet enabled the deep idle states where using the PDC becomes necessary. Cc: stable@vger.kernel.org Fixes: 05e4941d97ef ("pinctrl: qcom: Add X1E80100 pinctrl driver") Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> --- Commenting out .wakeirq_map as well would give an unused declaration warning for x1e80100_pdc_map. The map itself is correct, so I just "clear" it by setting .nwakeirq_map to 0 for now. It's just temporary - this patch will be reverted after we add X1E80100 support to the PDC driver. --- drivers/pinctrl/qcom/pinctrl-x1e80100.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) --- base-commit: 128f71fe014fc91efa1407ce549f94a9a9f1072c change-id: 20240830-x1e80100-bypass-pdc-39a8c1ae594f Best regards,