[PATCH] irqchip/sifive-plic: Avoid clearing the per-hart enable bits

From: Samuel Holland
Date: Mon Jul 17 2023 - 14:59:39 EST


Writes to the PLIC completion register are ignored if the enable bit for
that (interrupt, hart) combination is cleared. This leaves the interrupt
in a claimed state, preventing it from being triggered again.

Originally, the enable bit was cleared in the .irq_mask operation, and
commit 69ea463021be ("irqchip/sifive-plic: Fixup EOI failed when masked")
added a workaround for this issue. Later, commit a1706a1c5062
("irqchip/sifive-plic: Separate the enable and mask operations") moved
toggling the enable bit to the .irq_enable/.irq_disable operations and
removed the workaround.

However, there are still places where .irq_disable can be called from
inside the hard IRQ handler, for example in irq_pm_check_wakeup(). As a
result, this issue causes an interrupt to get stuck in a claimed state
after being used to wake the system from s2idle.

There is no real benefit to implementing the .irq_enable/.irq_disable
operations using the enable bits. In fact, the existing mask/unmask
implementation using the threshold register is already more efficient,
as it requires no read/modify/write cycles. So let's leave the enable
bits set for the lifetime of the IRQ, using them only to control its
affinity.

Fixes: a1706a1c5062 ("irqchip/sifive-plic: Separate the enable and mask operations")
Signed-off-by: Samuel Holland <samuel.holland@xxxxxxxxxx>
---

drivers/irqchip/irq-sifive-plic.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index e1484905b7bd..c2673fdad8e5 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -120,12 +120,14 @@ static inline void plic_irq_toggle(const struct cpumask *mask,
}
}

-static void plic_irq_enable(struct irq_data *d)
+static unsigned int plic_irq_startup(struct irq_data *d)
{
plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 1);
+
+ return 0;
}

-static void plic_irq_disable(struct irq_data *d)
+static void plic_irq_shutdown(struct irq_data *d)
{
plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 0);
}
@@ -169,12 +171,12 @@ static int plic_set_affinity(struct irq_data *d,
if (cpu >= nr_cpu_ids)
return -EINVAL;

- plic_irq_disable(d);
+ plic_irq_shutdown(d);

irq_data_update_effective_affinity(d, cpumask_of(cpu));

- if (!irqd_irq_disabled(d))
- plic_irq_enable(d);
+ if (irqd_is_started(d))
+ plic_irq_startup(d);

return IRQ_SET_MASK_OK_DONE;
}
@@ -182,8 +184,8 @@ static int plic_set_affinity(struct irq_data *d,

static struct irq_chip plic_edge_chip = {
.name = "SiFive PLIC",
- .irq_enable = plic_irq_enable,
- .irq_disable = plic_irq_disable,
+ .irq_startup = plic_irq_startup,
+ .irq_shutdown = plic_irq_shutdown,
.irq_ack = plic_irq_eoi,
.irq_mask = plic_irq_mask,
.irq_unmask = plic_irq_unmask,
@@ -197,8 +199,8 @@ static struct irq_chip plic_edge_chip = {

static struct irq_chip plic_chip = {
.name = "SiFive PLIC",
- .irq_enable = plic_irq_enable,
- .irq_disable = plic_irq_disable,
+ .irq_startup = plic_irq_startup,
+ .irq_shutdown = plic_irq_shutdown,
.irq_mask = plic_irq_mask,
.irq_unmask = plic_irq_unmask,
.irq_eoi = plic_irq_eoi,
--
2.40.1