Re: [PATCH v2 3/3] gpio: brcmstb: allow parent_irq to wake

[Florian Fainelli]

On 1/29/26 06:23, Andy Shevchenko wrote:
> On Tue, Jan 27, 2026 at 11:47 PM Florian Fainelli
> <florian.fainelli@xxxxxxxxxxxx> wrote:
>
> > The classic parent_wake_irq can only occur after the system has
> > been placed into a hardware managed power management state. This
> > prevents its use for waking from software managed suspend states
> > like s2idle.
> >
> > By allowing the parent_irq to be enabled for wake enabled GPIO
> > during suspend, these GPIO can now be used to wake from these
> > states. The 'suspended' boolean is introduced to support wake
> > event accounting.
>
> ...
>
> >          if (of_property_read_bool(np, "wakeup-source")) {
> > +               /*
> > +                * Set wakeup capability so we can process boot-time
> > +                * "wakeups" (e.g., from S5 cold boot)
>
> While at it, add a period at the end.
>
> > +                */
> > +               device_set_wakeup_capable(dev, true);
> > +               device_wakeup_enable(dev);
>
> >          }
>
> ...
>
> > +       /* disable interrupts */
>
> Still the comment is useless.
>
> > +       if (priv->parent_irq > 0)
> > +               disable_irq(priv->parent_irq);
>
> And looking more at this, I don't see why we even need the check. Does
> the code WARNs or so when there is no parent_irq available?
>
> *Yes, I saw this is the original code, perhaps can be addressed in a follow up.
>
> ...
>
> > +       /* disable interrupts while we save the masks */
>
> > +       if (priv->parent_irq > 0)
>
> Ditto.
>
> > +               disable_irq(priv->parent_irq);
>
> ...
>
> > +       /* disable interrupts while we restore the masks */
> > +       if (priv->parent_wake_irq)
>
> Ditto.
>
> > +               disable_irq(priv->parent_irq);
>
> ...
>
> > +       /* re-enable interrupts */
> > +       if (priv->parent_irq > 0)
>
> Same here.
>
> >                  enable_irq(priv->parent_irq);
>
> ...
>
> All we are diving into is the 2 questions:
> - is 0 on the particular platform an IRQ number and there is no sparse
> tree enabled?
> - is maple tree implementation clever enough to not crash (or have
> side effects) when we ask for a non-existing index?
>
> Anyway, this can be done later on.

OK, I will remove the superfluous comments, add punctuation where 
necessary and respin (removing patch #1 since it was applied already).

Thank you!
--
Florian