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

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 1/29/26 06:23, Andy Shevchenko wrote:
> On Tue, Jan 27, 2026 at 11:47 PM Florian Fainelli
> <florian.fainelli@broadcom.com> 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