Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   Daniel Thompson <daniel.thompson@linaro.org>
To:     Marc Zyngier <maz@kernel.org>, Thomas Gleixner <tglx@linutronix.de>
Cc:     Daniel Thompson <daniel.thompson@linaro.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] irqchip/exiu: Fix acknowledgment of edge triggered interrupts
Date:   Fri, 29 Apr 2022 17:53:14 +0100
Message-Id: <20220429165314.2343705-1-daniel.thompson@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Currently the EXIU uses the fasteoi interrupt flow that is configured by
it's parent (irq-gic-v3.c). With this flow the only chance to clear the
interrupt request happens during .irq_eoi() and (obviously) this happens
after the interrupt handler has run. EXIU requires edge triggered
interrupts to be acked prior to interrupt handling. Without this we
risk incorrect interrupt dismissal when a new interrupt is delivered
after the handler reads and acknowledges the peripheral but before the
irq_eoi() takes place.

Fix this by clearing the interrupt request from .irq_ack() if we are
configured for edge triggered interrupts. This requires adopting the
fasteoi-ack flow instead of the fasteoi to ensure the ack gets called.

These changes have been tested using the power button on a
Developerbox/SC2A11 combined with some hackery in gpio-keys so I can
play with the different trigger mode (and an mdelay(500) so I can
can check what happens on a double click in both modes.

Fixes: 706cffc1b912 ("irqchip/exiu: Add support for Socionext Synquacer EXIU controller")
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---

Notes:
    Changes in v2:
    
     * Switch to dynamic selection of handle_fasteoi_irq and
       handle_fasteoi_ack_irq and reintroduce exiu_irq_eoi() since we need
       that for level triggered interrupts (Ard B).
     * Above changes mean we are no longer using sun6i NMI code as a
       template to tidy up the description accordingly.

 arch/arm64/Kconfig.platforms   |  1 +
 drivers/irqchip/irq-sni-exiu.c | 33 +++++++++++++++++++++++++++++----
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 30b123cde02c..aaeaf57c8222 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -253,6 +253,7 @@ config ARCH_INTEL_SOCFPGA

 config ARCH_SYNQUACER
 	bool "Socionext SynQuacer SoC Family"
+	select IRQ_FASTEOI_HIERARCHY_HANDLERS

 config ARCH_TEGRA
 	bool "NVIDIA Tegra SoC Family"
diff --git a/drivers/irqchip/irq-sni-exiu.c b/drivers/irqchip/irq-sni-exiu.c
index abd011fcecf4..651a82dead01 100644
--- a/drivers/irqchip/irq-sni-exiu.c
+++ b/drivers/irqchip/irq-sni-exiu.c
@@ -37,11 +37,20 @@ struct exiu_irq_data {
 	u32		spi_base;
 };

-static void exiu_irq_eoi(struct irq_data *d)
+static void exiu_irq_ack(struct irq_data *d)
 {
 	struct exiu_irq_data *data = irq_data_get_irq_chip_data(d);

 	writel(BIT(d->hwirq), data->base + EIREQCLR);
+}
+
+static void exiu_irq_eoi(struct irq_data *d)
+{
+	struct exiu_irq_data *data = irq_data_get_irq_chip_data(d);
+	u32 edge_triggered = readl_relaxed(data->base + EIEDG);
+
+	if (!(edge_triggered & BIT(d->hwirq)))
+		writel(BIT(d->hwirq), data->base + EIREQCLR);
 	irq_chip_eoi_parent(d);
 }

@@ -91,10 +100,13 @@ static int exiu_irq_set_type(struct irq_data *d, unsigned int type)
 	writel_relaxed(val, data->base + EILVL);

 	val = readl_relaxed(data->base + EIEDG);
-	if (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_LEVEL_HIGH)
+	if (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_LEVEL_HIGH) {
 		val &= ~BIT(d->hwirq);
-	else
+		irq_set_handler_locked(d, handle_fasteoi_irq);
+	} else {
 		val |= BIT(d->hwirq);
+		irq_set_handler_locked(d, handle_fasteoi_ack_irq);
+	}
 	writel_relaxed(val, data->base + EIEDG);

 	writel_relaxed(BIT(d->hwirq), data->base + EIREQCLR);
@@ -104,6 +116,7 @@ static int exiu_irq_set_type(struct irq_data *d, unsigned int type)

 static struct irq_chip exiu_irq_chip = {
 	.name			= "EXIU",
+	.irq_ack		= exiu_irq_ack,
 	.irq_eoi		= exiu_irq_eoi,
 	.irq_enable		= exiu_irq_enable,
 	.irq_mask		= exiu_irq_mask,
@@ -148,6 +161,8 @@ static int exiu_domain_alloc(struct irq_domain *dom, unsigned int virq,
 	struct irq_fwspec parent_fwspec;
 	struct exiu_irq_data *info = dom->host_data;
 	irq_hw_number_t hwirq;
+	int i, ret;
+	u32 edge_triggered;

 	parent_fwspec = *fwspec;
 	if (is_of_node(dom->parent->fwnode)) {
@@ -165,7 +180,17 @@ static int exiu_domain_alloc(struct irq_domain *dom, unsigned int virq,
 	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &exiu_irq_chip, info);

 	parent_fwspec.fwnode = dom->parent->fwnode;
-	return irq_domain_alloc_irqs_parent(dom, virq, nr_irqs, &parent_fwspec);
+	ret = irq_domain_alloc_irqs_parent(dom, virq, nr_irqs, &parent_fwspec);
+	if (ret)
+		return ret;
+
+	edge_triggered = readl_relaxed(info->base + EIEDG);
+	for (i = 0; i < nr_irqs; i++)
+		irq_set_handler(virq + i, edge_triggered & BIT(i) ?
+						  handle_fasteoi_ack_irq :
+							handle_fasteoi_irq);
+
+	return 0;
 }

 static const struct irq_domain_ops exiu_domain_ops = {

base-commit: b2d229d4ddb17db541098b83524d901257e93845
--
2.35.1