The interrupt controller shutdown path does not need to save the mask of
enabled interrupts because the next state the system is going to be in
is akin to a cold boot, or a kexec'd kernel. Saving the mask only makes
sense if the software state needs to preserve the hardware state across
a system suspend/resume cycle.
As an optimization, and given that there are systems with dozens of such
interrupt controller, we can save a "slow" memory mapped I/O read in the
shutdown path where no saving/restoring is required.
Reported-by: Tim Ross <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
Changes in v2:
- expand the commit message to explain this is an optimiation saving a
slow MMIO read
drivers/irqchip/irq-brcmstb-l2.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-brcmstb-l2.c b/drivers/irqchip/irq-brcmstb-l2.c
index 2b0b3175cea0..c988886917f7 100644
--- a/drivers/irqchip/irq-brcmstb-l2.c
+++ b/drivers/irqchip/irq-brcmstb-l2.c
@@ -118,7 +118,7 @@ static void brcmstb_l2_intc_irq_handle(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
-static void brcmstb_l2_intc_suspend(struct irq_data *d)
+static void __brcmstb_l2_intc_suspend(struct irq_data *d, bool save)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
@@ -127,7 +127,8 @@ static void brcmstb_l2_intc_suspend(struct irq_data *d)
irq_gc_lock_irqsave(gc, flags);
/* Save the current mask */
- b->saved_mask = irq_reg_readl(gc, ct->regs.mask);
+ if (save)
+ b->saved_mask = irq_reg_readl(gc, ct->regs.mask);
if (b->can_wake) {
/* Program the wakeup mask */
@@ -137,6 +138,16 @@ static void brcmstb_l2_intc_suspend(struct irq_data *d)
irq_gc_unlock_irqrestore(gc, flags);
}
+static void brcmstb_l2_intc_shutdown(struct irq_data *d)
+{
+ __brcmstb_l2_intc_suspend(d, false);
+}
+
+static void brcmstb_l2_intc_suspend(struct irq_data *d)
+{
+ __brcmstb_l2_intc_suspend(d, true);
+}
+
static void brcmstb_l2_intc_resume(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
@@ -252,7 +263,7 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
ct->chip.irq_suspend = brcmstb_l2_intc_suspend;
ct->chip.irq_resume = brcmstb_l2_intc_resume;
- ct->chip.irq_pm_shutdown = brcmstb_l2_intc_suspend;
+ ct->chip.irq_pm_shutdown = brcmstb_l2_intc_shutdown;
if (data->can_wake) {
/* This IRQ chip can wake the system, set all child interrupts
--
2.34.1
The following commit has been merged into the irq/core branch of tip:
Commit-ID: c7cad38d37486668a448215fc92bace9c8cf747a
Gitweb: https://git.kernel.org/tip/c7cad38d37486668a448215fc92bace9c8cf747a
Author: Florian Fainelli <[email protected]>
AuthorDate: Wed, 24 Apr 2024 10:57:32 -07:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 24 Apr 2024 21:11:48 +02:00
irqchip/irq-brcmstb-l2: Avoid saving mask on shutdown
The interrupt controller shutdown path does not need to save the mask of
enabled interrupts because the next state the system is going to be in is
akin to a cold boot, or a kexec'd kernel. Saving the mask only makes sense
if the software state needs to preserve the hardware state across a system
suspend/resume cycle.
As an optimization, and given that there are systems with dozens of such
interrupt controller, save a "slow" memory mapped I/O read in the shutdown
path where no saving/restoring is required.
Reported-by: Tim Ross <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-brcmstb-l2.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-brcmstb-l2.c b/drivers/irqchip/irq-brcmstb-l2.c
index 2b0b317..c988886 100644
--- a/drivers/irqchip/irq-brcmstb-l2.c
+++ b/drivers/irqchip/irq-brcmstb-l2.c
@@ -118,7 +118,7 @@ out:
chained_irq_exit(chip, desc);
}
-static void brcmstb_l2_intc_suspend(struct irq_data *d)
+static void __brcmstb_l2_intc_suspend(struct irq_data *d, bool save)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
@@ -127,7 +127,8 @@ static void brcmstb_l2_intc_suspend(struct irq_data *d)
irq_gc_lock_irqsave(gc, flags);
/* Save the current mask */
- b->saved_mask = irq_reg_readl(gc, ct->regs.mask);
+ if (save)
+ b->saved_mask = irq_reg_readl(gc, ct->regs.mask);
if (b->can_wake) {
/* Program the wakeup mask */
@@ -137,6 +138,16 @@ static void brcmstb_l2_intc_suspend(struct irq_data *d)
irq_gc_unlock_irqrestore(gc, flags);
}
+static void brcmstb_l2_intc_shutdown(struct irq_data *d)
+{
+ __brcmstb_l2_intc_suspend(d, false);
+}
+
+static void brcmstb_l2_intc_suspend(struct irq_data *d)
+{
+ __brcmstb_l2_intc_suspend(d, true);
+}
+
static void brcmstb_l2_intc_resume(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
@@ -252,7 +263,7 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
ct->chip.irq_suspend = brcmstb_l2_intc_suspend;
ct->chip.irq_resume = brcmstb_l2_intc_resume;
- ct->chip.irq_pm_shutdown = brcmstb_l2_intc_suspend;
+ ct->chip.irq_pm_shutdown = brcmstb_l2_intc_shutdown;
if (data->can_wake) {
/* This IRQ chip can wake the system, set all child interrupts