linux-kernel - Re: [PATCH v2 3/3] gpio: brcmstb: allow parent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75Vc1om1XDT0os+md1D-mwYxrVZJGfbpepkdPJrHOmFPBvA@mail.gmail.com>
Date: Thu, 29 Jan 2026 16:23:41 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Florian Fainelli <florian.fainelli@...adcom.com>
Cc: linux-kernel@...r.kernel.org, Doug Berger <opendmb@...il.com>, 
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, Linus Walleij <linusw@...nel.org>, 
	Bartosz Golaszewski <brgl@...nel.org>, Christophe Leroy <chleroy@...nel.org>, 
	"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>, 
	"moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 3/3] gpio: brcmstb: allow parent_irq to wake

On Tue, Jan 27, 2026 at 11:47 PM Florian Fainelli
<florian.fainelli@...adcom.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.

--
With Best Regards,
Andy Shevchenko