Hi, On Tue, Aug 11, 2020 at 12:34 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > Quoting Maulik Shah (2020-08-10 04:20:55) > > msmgpio irqchip is not using return value of irq_set_wake call. > > Start using it. > > Does this work when the irq parent isn't setup in a hierarchy? I seem to > recall that this was written this way because sometimes > irq_set_irq_wake() would fail for the summary irq so it was a best > effort setting of wake on the summary line. > > > > > Fixes: e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy") > > Signed-off-by: Maulik Shah <mkshah@xxxxxxxxxxxxxx> > > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > > --- > > drivers/pinctrl/qcom/pinctrl-msm.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > > index 90edf61..c264561 100644 > > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > > @@ -1077,12 +1077,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); > > So this bit is probably fine. > > > > > - return 0; > > + return irq_set_irq_wake(pctrl->irq, on); > > But this one is probably not fine. Interesting. I wasn't aware of the history and thus assumed this was a bug. If Stephen is remembering correctly, please add a comment saying that we are purposely ignoring the return value in this case. -Doug