* [PATCH v3] irqchip/exiu: Fix acknowledgment of edge triggered interrupts
@ 2022-05-03 13:45 Daniel Thompson
2022-05-03 14:28 ` Ard Biesheuvel
2022-05-04 16:15 ` [irqchip: irq/irqchip-next] " irqchip-bot for Daniel Thompson
0 siblings, 2 replies; 3+ messages in thread
From: Daniel Thompson @ 2022-05-03 13:45 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner
Cc: Daniel Thompson, Catalin Marinas, Will Deacon, Ard Biesheuvel,
linux-arm-kernel, linux-kernel
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 v3:
* Optimize exiu_irq_eoi() to avoid unnecessary register read (Marc Z)
* Add comment to exiu_irq_eoi() so future adventurers can better
understand how the EXIU on SC2A11 behaves (Marc Z)
* Remove redundant trigger mode management from exiu_domain_alloc()
(Marc Z)
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 | 25 ++++++++++++++++++++++---
2 files changed, 23 insertions(+), 3 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..c7db617e1a2f 100644
--- a/drivers/irqchip/irq-sni-exiu.c
+++ b/drivers/irqchip/irq-sni-exiu.c
@@ -37,11 +37,26 @@ 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);
+
+ /*
+ * Level triggered interrupts are latched and must be cleared during
+ * EOI or the interrupt will be jammed on. Of course if a level
+ * triggered interrupt is still asserted then the write will not clear
+ * the interrupt.
+ */
+ if (irqd_is_level_type(d))
+ writel(BIT(d->hwirq), data->base + EIREQCLR);
+
irq_chip_eoi_parent(d);
}
@@ -91,10 +106,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 +122,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,
base-commit: b2d229d4ddb17db541098b83524d901257e93845
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] irqchip/exiu: Fix acknowledgment of edge triggered interrupts
2022-05-03 13:45 [PATCH v3] irqchip/exiu: Fix acknowledgment of edge triggered interrupts Daniel Thompson
@ 2022-05-03 14:28 ` Ard Biesheuvel
2022-05-04 16:15 ` [irqchip: irq/irqchip-next] " irqchip-bot for Daniel Thompson
1 sibling, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2022-05-03 14:28 UTC (permalink / raw)
To: Daniel Thompson
Cc: Marc Zyngier, Thomas Gleixner, Catalin Marinas, Will Deacon,
Linux ARM, Linux Kernel Mailing List
On Tue, 3 May 2022 at 15:45, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>
> 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>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>
> Notes:
> Changes in v3:
> * Optimize exiu_irq_eoi() to avoid unnecessary register read (Marc Z)
> * Add comment to exiu_irq_eoi() so future adventurers can better
> understand how the EXIU on SC2A11 behaves (Marc Z)
> * Remove redundant trigger mode management from exiu_domain_alloc()
> (Marc Z)
>
> 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 | 25 ++++++++++++++++++++++---
> 2 files changed, 23 insertions(+), 3 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..c7db617e1a2f 100644
> --- a/drivers/irqchip/irq-sni-exiu.c
> +++ b/drivers/irqchip/irq-sni-exiu.c
> @@ -37,11 +37,26 @@ 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);
> +
> + /*
> + * Level triggered interrupts are latched and must be cleared during
> + * EOI or the interrupt will be jammed on. Of course if a level
> + * triggered interrupt is still asserted then the write will not clear
> + * the interrupt.
> + */
> + if (irqd_is_level_type(d))
> + writel(BIT(d->hwirq), data->base + EIREQCLR);
> +
> irq_chip_eoi_parent(d);
> }
>
> @@ -91,10 +106,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 +122,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,
>
> base-commit: b2d229d4ddb17db541098b83524d901257e93845
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [irqchip: irq/irqchip-next] irqchip/exiu: Fix acknowledgment of edge triggered interrupts
2022-05-03 13:45 [PATCH v3] irqchip/exiu: Fix acknowledgment of edge triggered interrupts Daniel Thompson
2022-05-03 14:28 ` Ard Biesheuvel
@ 2022-05-04 16:15 ` irqchip-bot for Daniel Thompson
1 sibling, 0 replies; 3+ messages in thread
From: irqchip-bot for Daniel Thompson @ 2022-05-04 16:15 UTC (permalink / raw)
To: linux-kernel; +Cc: Daniel Thompson, Ard Biesheuvel, Marc Zyngier, tglx
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: 4efc851c36e389f7ed432edac0149acc5f94b0c7
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/4efc851c36e389f7ed432edac0149acc5f94b0c7
Author: Daniel Thompson <daniel.thompson@linaro.org>
AuthorDate: Tue, 03 May 2022 14:45:41 +01:00
Committer: Marc Zyngier <maz@kernel.org>
CommitterDate: Wed, 04 May 2022 16:33:17 +01:00
irqchip/exiu: Fix acknowledgment of edge triggered interrupts
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>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20220503134541.2566457-1-daniel.thompson@linaro.org
---
arch/arm64/Kconfig.platforms | 1 +
drivers/irqchip/irq-sni-exiu.c | 25 ++++++++++++++++++++++---
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 30b123c..aaeaf57 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 abd011f..c7db617 100644
--- a/drivers/irqchip/irq-sni-exiu.c
+++ b/drivers/irqchip/irq-sni-exiu.c
@@ -37,11 +37,26 @@ 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);
+
+ /*
+ * Level triggered interrupts are latched and must be cleared during
+ * EOI or the interrupt will be jammed on. Of course if a level
+ * triggered interrupt is still asserted then the write will not clear
+ * the interrupt.
+ */
+ if (irqd_is_level_type(d))
+ writel(BIT(d->hwirq), data->base + EIREQCLR);
+
irq_chip_eoi_parent(d);
}
@@ -91,10 +106,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 +122,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,
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-04 16:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-03 13:45 [PATCH v3] irqchip/exiu: Fix acknowledgment of edge triggered interrupts Daniel Thompson
2022-05-03 14:28 ` Ard Biesheuvel
2022-05-04 16:15 ` [irqchip: irq/irqchip-next] " irqchip-bot for Daniel Thompson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).