gpio: mvebu: convert to noirq suspend/resume to prevent interrupt storm on resume
PATCH v2 1/1 gpio: mvebu: convert to noirq suspend/resume to prevent interrupt storm on resume
The driver uses the legacy .suspend/.resume callbacks, but sets IRQCHIP_MASK_ON_SUSPEND on the irq_chip. During resume, the PM core runs dpm_resume_noirq() first, which calls irq_pm_resume() to unmask interrupts, and only then runs dpm_resume() which invokes the driver's .resume callback to restore GPIO registers (GPIO_IN_POL, GPIO_IO_CONF, mask registers).
This ordering means interrupts are unmasked while the hardware is still in its reset state, potentially with incorrect polarities, causing spurious level-triggered interrupts before local IRQs are re-enabled.
Convert the driver from legacy .suspend/.resume callbacks to noirq callbacks via dev_pm_ops. The noirq phase runs before resume_device_irqs() on resume and after suspend_device_irqs() on suspend, ensuring GPIO registers are restored before interrupts are unmasked.
---
v2: add __maybe_unused to fix compilation
drivers/gpio/gpio-mvebu.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
drivers/gpio/gpio-mvebu.c +9 -6
@@ -979,9 +979,9 @@ static const struct of_device_id mvebu_gpio_of_match[] = { }, }; -static int mvebu_gpio_suspend (struct platform_device *pdev,pm_message_t state) +static __maybe_unused int mvebu_gpio_suspend (struct device *dev) { -struct mvebu_gpio_chip *mvchip =platform_get_drvdata (pdev); +struct mvebu_gpio_chip *mvchip =dev_get_drvdata (dev);int i; regmap_read(mvchip->regs, GPIO_OUT_OFF + mvchip->offset,
@@ -1031,9 +1031,9 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)return 0 ; } -static int mvebu_gpio_resume (struct platform_device *pdev) +static __maybe_unused int mvebu_gpio_resume (struct device *dev) { -struct mvebu_gpio_chip *mvchip =platform_get_drvdata (pdev); +struct mvebu_gpio_chip *mvchip =dev_get_drvdata (dev);int i; regmap_write(mvchip->regs, GPIO_OUT_OFF + mvchip->offset,
@@ -1339,13 +1339,16 @@ static int mvebu_gpio_probe(struct platform_device *pdev)return 0 ; } +static const struct dev_pm_ops mvebu_gpio_pm_ops = { +SET_NOIRQ_SYSTEM_SLEEP_PM_OPS (mvebu_gpio_suspend, mvebu_gpio_resume) +}; +static struct platform_driver mvebu_gpio_driver = { .driver = { .name ="mvebu-gpio" , .of_match_table = mvebu_gpio_of_match, + .pm =pm_sleep_ptr (&mvebu_gpio_pm_ops), }, .probe = mvebu_gpio_probe, - .suspend = mvebu_gpio_suspend, - .resume = mvebu_gpio_resume, };builtin_platform_driver (mvebu_gpio_driver);
Signature · 1 line
2.55.0
Quoted 17 lines · Rosen Penev
The driver uses the legacy .suspend/.resume callbacks, but sets
IRQCHIP_MASK_ON_SUSPENDon theirq_chip. During resume, the PM core runsdpm_resume_noirq()first, which callsirq_pm_resume()to unmask interrupts, and only then runsdpm_resume()which invokes the driver's .resume callback to restore GPIO registers (GPIO_IN_POL,GPIO_IO_CONF, mask registers).This ordering means interrupts are unmasked while the hardware is still in its reset state, potentially with incorrect polarities, causing spurious level-triggered interrupts before local IRQs are re-enabled.
Convert the driver from legacy .suspend/.resume callbacks to noirq callbacks via
dev_pm_ops. The noirq phase runs beforeresume_device_irqs()on resume and aftersuspend_device_irqs()on suspend, ensuring GPIO registers are restored before interrupts are unmasked.
Quoted 4 lines
Assisted-by opencode:big-pickleSigned-off-by Rosen Penev <[email protected]>---
v2: add__maybe_unusedto fix compilation
Should I resend?
Quoted 53 lines
drivers/gpio/gpio-mvebu.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)drivers/gpio/gpio-mvebu.c +9
-7@@ -979,9 +979,9 @@ static const struct of_device_id mvebu_gpio_of_match[] = { }, }; -static int mvebu_gpio_suspend (struct platform_device *pdev,pm_message_t state) +static __maybe_unused int mvebu_gpio_suspend (struct device *dev) { -struct mvebu_gpio_chip *mvchip =platform_get_drvdata (pdev); +struct mvebu_gpio_chip *mvchip =dev_get_drvdata (dev);int i; regmap_read(mvchip->regs, GPIO_OUT_OFF + mvchip->offset,@@ -1031,9 +1031,9 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)return 0 ; } -static int mvebu_gpio_resume (struct platform_device *pdev) +static __maybe_unused int mvebu_gpio_resume (struct device *dev) { -struct mvebu_gpio_chip *mvchip =platform_get_drvdata (pdev); +struct mvebu_gpio_chip *mvchip =dev_get_drvdata (dev);int i; regmap_write(mvchip->regs, GPIO_OUT_OFF + mvchip->offset,@@ -1339,13 +1339,16 @@ static int mvebu_gpio_probe(struct platform_device *pdev)return 0 ; } +static const struct dev_pm_ops mvebu_gpio_pm_ops = { +SET_NOIRQ_SYSTEM_SLEEP_PM_OPS (mvebu_gpio_suspend, mvebu_gpio_resume) +}; +static struct platform_driver mvebu_gpio_driver = { .driver = { .name ="mvebu-gpio" , .of_match_table = mvebu_gpio_of_match, + .pm =pm_sleep_ptr (&mvebu_gpio_pm_ops), }, .probe = mvebu_gpio_probe, - .suspend = mvebu_gpio_suspend, - .resume = mvebu_gpio_resume, };builtin_platform_driver (mvebu_gpio_driver); --2.55.0
Quoted 11 lines · Rosen Penev
The driver uses the legacy .suspend/.resume callbacks, but sets
IRQCHIP_MASK_ON_SUSPENDon theirq_chip. During resume, the PM core runsdpm_resume_noirq()first, which callsirq_pm_resume()to unmask interrupts, and only then runsdpm_resume()which invokes the driver's .resume callback to restore GPIO registers (GPIO_IN_POL,GPIO_IO_CONF, mask registers).This ordering means interrupts are unmasked while the hardware is still in its reset state, potentially with incorrect polarities, causing spurious level-triggered interrupts before local IRQs are re-enabled.
Quoted 6 lines
Convert the driver from legacy .suspend/.resume callbacks to noirq callbacks via
dev_pm_ops. The noirq phase runs beforeresume_device_irqs()on resume and aftersuspend_device_irqs()on suspend, ensuring GPIO registers are restored before interrupts are unmasked.Assisted-by opencode:big-pickle
Should be
these days (Bartosz can probs fix when applying)
Quoted 1 line
Signed-off-by Rosen Penev <[email protected]>
Yours,
Linus Walleij
Quoted 19 lines · Linus Walleij
Quoted 16 lines · Rosen Penev
The driver uses the legacy .suspend/.resume callbacks, but sets
IRQCHIP_MASK_ON_SUSPENDon theirq_chip. During resume, the PM core runsdpm_resume_noirq()first, which callsirq_pm_resume()to unmask interrupts, and only then runsdpm_resume()which invokes the driver's .resume callback to restore GPIO registers (GPIO_IN_POL,GPIO_IO_CONF, mask registers).This ordering means interrupts are unmasked while the hardware is still in its reset state, potentially with incorrect polarities, causing spurious level-triggered interrupts before local IRQs are re-enabled.
Convert the driver from legacy .suspend/.resume callbacks to noirq callbacks via
dev_pm_ops. The noirq phase runs beforeresume_device_irqs()on resume and aftersuspend_device_irqs()on suspend, ensuring GPIO registers are restored before interrupts are unmasked.
Quoted 5 lines
Quoted 1 line
Assisted-by opencode:big-pickleShould be
Assisted-by LLMthese days (Bartosz can probs fix when applying)
I'll post a v3. There's a better way than __maybe_unused.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Rosen Penev <[email protected]>Reviewed-by Linus Walleij <[email protected]>Yours,
Linus Walleij