diff mbox series

[v2] irqchip/exiu: Fix acknowledgment of edge triggered interrupts

Message ID 20220429165314.2343705-1-daniel.thompson@linaro.org (mailing list archive)
State New, archived
Headers show
Series [v2] irqchip/exiu: Fix acknowledgment of edge triggered interrupts | expand

Commit Message

Daniel Thompson April 29, 2022, 4:53 p.m. UTC 
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(-)


base-commit: b2d229d4ddb17db541098b83524d901257e93845
--
2.35.1

Comments

Marc Zyngier April 29, 2022, 5:39 p.m. UTC | #1 
On Fri, 29 Apr 2022 17:53:14 +0100,
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>
> ---
> 
> 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);

I expect this to be pretty expensive. Why not directly check the state
flags with irqd_is_level_type()?

> +
> +	if (!(edge_triggered & BIT(d->hwirq)))
> +		writel(BIT(d->hwirq), data->base + EIREQCLR);

Is this write even needed for a level interrupt? Or does the register
always behave as a latch irrespective of the trigger?

>  	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;

Why do you need this at allocation time? I would have expected the
trigger configuration to be enough.

Thanks,

	M.
Daniel Thompson April 29, 2022, 6:36 p.m. UTC | #2 
On Fri, Apr 29, 2022 at 06:39:51PM +0100, Marc Zyngier wrote:
> On Fri, 29 Apr 2022 17:53:14 +0100,
> 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>
> > ---
> > 
> > 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);
> 
> I expect this to be pretty expensive. Why not directly check the state
> flags with irqd_is_level_type()?

Good point. Will change this.


> > +
> > +	if (!(edge_triggered & BIT(d->hwirq)))
> > +		writel(BIT(d->hwirq), data->base + EIREQCLR);
> 
> Is this write even needed for a level interrupt? Or does the register
> always behave as a latch irrespective of the trigger?

It is unconditionally latched; must be cleared or the interrupt will be
jammed on. Of course, for level interrupts that are still asserted then
the write will not clear the interrupt.


> >  	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;
> 
> Why do you need this at allocation time? I would have expected the
> trigger configuration to be enough.

I saw the following in the description of the interrupt trigger modes
: When requesting an interrupt without specifying a IRQF_TRIGGER, the
: setting should be assumed to be "as already configured", which may
: be as per machine or firmware initialisation.

From that I was concerned that the callback to set the trigger mode
would not be called in all cases. Thus when I saw that calling
__irq_set_trigger() was on a conditional code path in __setup_irq()
then I wrote the above logic.

I assume I overlooked something? Is a call to exiu_irq_set_type()
guaranteed to happen in all cases?


Daniel.
Marc Zyngier April 29, 2022, 9:46 p.m. UTC | #3 
On Fri, 29 Apr 2022 19:36:05 +0100,
Daniel Thompson <daniel.thompson@linaro.org> wrote:
> 
> On Fri, Apr 29, 2022 at 06:39:51PM +0100, Marc Zyngier wrote:
> > On Fri, 29 Apr 2022 17:53:14 +0100,
> > Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > > 
> > > +
> > > +	if (!(edge_triggered & BIT(d->hwirq)))
> > > +		writel(BIT(d->hwirq), data->base + EIREQCLR);
> > 
> > Is this write even needed for a level interrupt? Or does the register
> > always behave as a latch irrespective of the trigger?
> 
> It is unconditionally latched; must be cleared or the interrupt will be
> jammed on. Of course, for level interrupts that are still asserted then
> the write will not clear the interrupt.

OK. The HW folks missed a trick here, but hey.

> 
> 
> > >  	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;
> > 
> > Why do you need this at allocation time? I would have expected the
> > trigger configuration to be enough.
> 
> I saw the following in the description of the interrupt trigger modes
> : When requesting an interrupt without specifying a IRQF_TRIGGER, the
> : setting should be assumed to be "as already configured", which may
> : be as per machine or firmware initialisation.
> 
> From that I was concerned that the callback to set the trigger mode
> would not be called in all cases. Thus when I saw that calling
> __irq_set_trigger() was on a conditional code path in __setup_irq()
> then I wrote the above logic.
> 
> I assume I overlooked something? Is a call to exiu_irq_set_type()
> guaranteed to happen in all cases?

My expectations are that the interrupt is configured from
irq_create_fwspec_mapping(), which will set the trigger it obtained
from the firmware, long before the interrupt is setup.

The conditional code you saw in __setup_irq() is to handle the case
where request_irq() is passing a trigger configuration that isn't the
default one.

Either way, you should be able to safely remove this from the
allocation side.

	M.
diff mbox series

Patch

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 = {