[v3,3/5] pinctrl: qcom: Use return value from irq_set_wake call

Message ID 1592818308-23001-4-git-send-email-mkshah@codeaurora.org
State New
Headers show
Series
  • [v3,1/5] pinctrl: qcom: Remove irq_disable callback from msmgpio irqchip
Related show

Commit Message

Maulik Shah June 22, 2020, 9:31 a.m.
msmgpio irqchip is not using return value of irq_set_wake call.
Start using it.

Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Doug Anderson July 13, 2020, 10:17 p.m. | #1
Hi,

On Mon, Jun 22, 2020 at 2:32 AM Maulik Shah <mkshah@codeaurora.org> wrote:
>

> msmgpio irqchip is not using return value of irq_set_wake call.

> Start using it.

>

> Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")

> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>

> ---

>  drivers/pinctrl/qcom/pinctrl-msm.c | 8 +++-----

>  1 file changed, 3 insertions(+), 5 deletions(-)


Seems right to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Linus Walleij July 16, 2020, 1:18 p.m. | #2
On Mon, Jun 22, 2020 at 11:32 AM Maulik Shah <mkshah@codeaurora.org> wrote:

> msmgpio irqchip is not using return value of irq_set_wake call.

> Start using it.

>

> Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")

> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>


Is this something that's causing regressions so I should apply it for
fixes, or is it fine to keep this with the rest of the series for v5.9?

Yours,
Linus Walleij
Doug Anderson July 16, 2020, 9:51 p.m. | #3
Hi,

On Thu, Jul 16, 2020 at 6:19 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>

> On Mon, Jun 22, 2020 at 11:32 AM Maulik Shah <mkshah@codeaurora.org> wrote:

>

> > msmgpio irqchip is not using return value of irq_set_wake call.

> > Start using it.

> >

> > Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")

> > Signed-off-by: Maulik Shah <mkshah@codeaurora.org>

>

> Is this something that's causing regressions so I should apply it for

> fixes, or is it fine to keep this with the rest of the series for v5.9?


I would let Maulik comment more, but as far as I can tell the function
has been ignoring the return value of irq_set_irq_wake() for much
longer.  Presumably one could logically say:

Fixes: 6aced33f4974 ("pinctrl: msm: drop wake_irqs bitmap")

...though when you get past the commit that Maulik tagged you need a
backport rather than a straight cherry-pick.

That would make me believe that there is no real hurry to land the fix here.


-Doug

Patch

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index b909ffe..92fe7d6 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -978,12 +978,10 @@  static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 	 * when TLMM is powered on. To allow that, enable the GPIO
 	 * summary line to be wakeup capable at GIC.
 	 */
-	if (d->parent_data)
-		irq_chip_set_wake_parent(d, on);
-
-	irq_set_irq_wake(pctrl->irq, on);
+	if (d->parent_data && test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return irq_chip_set_wake_parent(d, on);
 
-	return 0;
+	return irq_set_irq_wake(pctrl->irq, on);
 }
 
 static int msm_gpio_irq_reqres(struct irq_data *d)