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 v3] irqchip/exiu: Fix acknowledgment of edge triggered interrupts Date: Tue, 3 May 2022 14:45:41 +0100 [thread overview] Message-ID: <20220503134541.2566457-1-daniel.thompson@linaro.org> (raw) 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
WARNING: multiple messages have this Message-ID (diff)
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 v3] irqchip/exiu: Fix acknowledgment of edge triggered interrupts Date: Tue, 3 May 2022 14:45:41 +0100 [thread overview] Message-ID: <20220503134541.2566457-1-daniel.thompson@linaro.org> (raw) 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2022-05-03 13:45 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-03 13:45 Daniel Thompson [this message] 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-03 14:28 ` Ard Biesheuvel 2022-05-04 16:15 ` [irqchip: irq/irqchip-next] " irqchip-bot for Daniel Thompson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220503134541.2566457-1-daniel.thompson@linaro.org \ --to=daniel.thompson@linaro.org \ --cc=ardb@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maz@kernel.org \ --cc=tglx@linutronix.de \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.