From: Doug Berger <opendmb@gmail.com>
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.
Signed-off-by: Doug Berger <opendmb@gmail.com>
[florian: port changes after generic gpio chip conversion]
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
drivers/gpio/gpio-brcmstb.c | 90 +++++++++++++++++++++++++------------
1 file changed, 62 insertions(+), 28 deletions(-)
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index bf0192b82276..5489c3090aa1 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -54,6 +54,7 @@ struct brcmstb_gpio_priv {
int parent_irq;
int num_gpios;
int parent_wake_irq;
+ bool suspended;
};
#define MAX_GPIO_PER_BANK 32
@@ -239,6 +240,9 @@ static int brcmstb_gpio_priv_set_wake(struct brcmstb_gpio_priv *priv,
{
int ret = 0;
+ if (priv->parent_wake_irq == priv->parent_irq)
+ return ret;
+
if (enable)
ret = enable_irq_wake(priv->parent_wake_irq);
else
@@ -289,6 +293,11 @@ static void brcmstb_gpio_irq_bank_handler(struct brcmstb_gpio_bank *bank)
while ((status = brcmstb_gpio_get_active_irqs(bank))) {
unsigned int offset;
+ if (priv->suspended && bank->wake_active & status) {
+ priv->suspended = false;
+ pm_wakeup_event(&priv->pdev->dev, 0);
+ }
+
for_each_set_bit(offset, &status, 32) {
if (offset >= bank->width)
dev_warn(&priv->pdev->dev,
@@ -462,18 +471,18 @@ static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
}
if (of_property_read_bool(np, "wakeup-source")) {
+ /*
+ * Set wakeup capability so we can process boot-time
+ * "wakeups" (e.g., from S5 cold boot)
+ */
+ device_set_wakeup_capable(dev, true);
+ device_wakeup_enable(dev);
priv->parent_wake_irq = platform_get_irq(pdev, 1);
if (priv->parent_wake_irq < 0) {
- priv->parent_wake_irq = 0;
+ priv->parent_wake_irq = priv->parent_irq;
dev_warn(dev,
"Couldn't get wake IRQ - GPIOs will not be able to wake from sleep");
} else {
- /*
- * Set wakeup capability so we can process boot-time
- * "wakeups" (e.g., from S5 cold boot)
- */
- device_set_wakeup_capable(dev, true);
- device_wakeup_enable(dev);
err = devm_request_irq(dev, priv->parent_wake_irq,
brcmstb_gpio_wake_irq_handler,
IRQF_SHARED,
@@ -484,6 +493,7 @@ static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
goto out_free_domain;
}
}
+ priv->irq_chip.irq_set_wake = brcmstb_gpio_irq_set_wake;
}
priv->irq_chip.name = dev_name(dev);
@@ -494,9 +504,6 @@ static int brcmstb_gpio_irq_setup(struct platform_device *pdev,
priv->irq_chip.irq_ack = brcmstb_gpio_irq_ack;
priv->irq_chip.irq_set_type = brcmstb_gpio_irq_set_type;
- if (priv->parent_wake_irq)
- priv->irq_chip.irq_set_wake = brcmstb_gpio_irq_set_wake;
-
irq_set_chained_handler_and_data(priv->parent_irq,
brcmstb_gpio_irq_handler, priv);
irq_set_status_flags(priv->parent_irq, IRQ_DISABLE_UNLAZY);
@@ -519,16 +526,11 @@ static void brcmstb_gpio_bank_save(struct brcmstb_gpio_priv *priv,
priv->reg_base + GIO_BANK_OFF(bank->id, i));
}
-static void brcmstb_gpio_quiesce(struct device *dev, bool save)
+static void brcmstb_gpio_quiesce(struct brcmstb_gpio_priv *priv, bool save)
{
- struct brcmstb_gpio_priv *priv = dev_get_drvdata(dev);
struct brcmstb_gpio_bank *bank;
u32 imask;
- /* disable non-wake interrupt */
- if (priv->parent_irq >= 0)
- disable_irq(priv->parent_irq);
-
list_for_each_entry(bank, &priv->bank_list, node) {
if (save)
brcmstb_gpio_bank_save(priv, bank);
@@ -546,8 +548,14 @@ static void brcmstb_gpio_quiesce(struct device *dev, bool save)
static void brcmstb_gpio_shutdown(struct platform_device *pdev)
{
+ struct brcmstb_gpio_priv *priv = dev_get_drvdata(&pdev->dev);
+
+ /* disable interrupts */
+ if (priv->parent_irq > 0)
+ disable_irq(priv->parent_irq);
+
/* Enable GPIO for S5 cold boot */
- brcmstb_gpio_quiesce(&pdev->dev, false);
+ brcmstb_gpio_quiesce(priv, false);
}
static void brcmstb_gpio_bank_restore(struct brcmstb_gpio_priv *priv,
@@ -563,7 +571,32 @@ static void brcmstb_gpio_bank_restore(struct brcmstb_gpio_priv *priv,
static int brcmstb_gpio_suspend(struct device *dev)
{
- brcmstb_gpio_quiesce(dev, true);
+ struct brcmstb_gpio_priv *priv = dev_get_drvdata(dev);
+
+ if (priv->parent_irq > 0)
+ priv->suspended = true;
+
+ return 0;
+}
+
+static int brcmstb_gpio_suspend_noirq(struct device *dev)
+{
+ struct brcmstb_gpio_priv *priv = dev_get_drvdata(dev);
+
+ /* Catch any wakeup sources occurring between suspend and noirq */
+ if (!priv->suspended)
+ return -EBUSY;
+
+ /* disable interrupts while we save the masks */
+ if (priv->parent_irq > 0)
+ disable_irq(priv->parent_irq);
+
+ brcmstb_gpio_quiesce(priv, true);
+
+ /* Now that the masks have been saved re-enable interrupts */
+ if (priv->parent_wake_irq)
+ enable_irq(priv->parent_irq);
+
return 0;
}
@@ -571,25 +604,26 @@ static int brcmstb_gpio_resume(struct device *dev)
{
struct brcmstb_gpio_priv *priv = dev_get_drvdata(dev);
struct brcmstb_gpio_bank *bank;
- bool need_wakeup_event = false;
- list_for_each_entry(bank, &priv->bank_list, node) {
- need_wakeup_event |= !!__brcmstb_gpio_get_active_irqs(bank);
- brcmstb_gpio_bank_restore(priv, bank);
- }
+ /* disable interrupts while we restore the masks */
+ if (priv->parent_wake_irq)
+ disable_irq(priv->parent_irq);
- if (priv->parent_wake_irq && need_wakeup_event)
- pm_wakeup_event(dev, 0);
+ priv->suspended = false;
+
+ list_for_each_entry(bank, &priv->bank_list, node)
+ brcmstb_gpio_bank_restore(priv, bank);
- /* enable non-wake interrupt */
- if (priv->parent_irq >= 0)
+ /* re-enable interrupts */
+ if (priv->parent_irq > 0)
enable_irq(priv->parent_irq);
return 0;
}
static const struct dev_pm_ops brcmstb_gpio_pm_ops = {
- .suspend_noirq = pm_sleep_ptr(brcmstb_gpio_suspend),
+ .suspend = pm_sleep_ptr(brcmstb_gpio_suspend),
+ .suspend_noirq = pm_sleep_ptr(brcmstb_gpio_suspend_noirq),
.resume_noirq = pm_sleep_ptr(brcmstb_gpio_resume),
};
--
2.43.0On 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.
--
With Best Regards,
Andy Shevchenko
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
© 2016 - 2026 Red Hat, Inc.