Re: [PATCH 1/3] genirq: Add support for priority-drop/deactivate interrupt controllers

[Thomas Gleixner]

On Wed, 29 Oct 2014, Marc Zyngier wrote:
> On 28/10/14 20:14, Thomas Gleixner wrote:
> > irq_enable() calls chip->irq_unmask(), i.e. DIR. So that clears the
> > ACTIVE bit and then the IRQ either gets resent by hardware (in case of
> > level as the device interrupt is still active) or retriggered by the
> > irq_retrigger() callback.
> 
> The problem I see here is for an interrupt that has been flagged as
> disabled with irq_disabled(), but that hasn't fired. We'd end up doing a
> DIR on something that hasn't had an EOI first. I think that's the only
> wrinkle in this scheme.

Right. So the untested patch below should do the trick and prevent
irq_enable() to invoke irq_unmask() if the interrupt is not flagged
masked. And it only can be flagged masked if it was masked in the
handler. The startup callback will make sure that irq_enable() is not
invoked at startup time.

Thanks,

	tglx

------------------

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index e5202f00cabc..9c0f73e1994a 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -182,7 +182,7 @@ int irq_startup(struct irq_desc *desc, bool resend)
 		ret = desc->irq_data.chip->irq_startup(&desc->irq_data);
 		irq_state_clr_masked(desc);
 	} else {
-		irq_enable(desc);
+		irq_enable(desc, false);
 	}
 	if (resend)
 		check_irq_resend(desc, desc->irq_data.irq);
@@ -202,13 +202,14 @@ void irq_shutdown(struct irq_desc *desc)
 	irq_state_set_masked(desc);
 }
 
-void irq_enable(struct irq_desc *desc)
+void irq_enable(struct irq_desc *desc, bool ifmasked)
 {
 	irq_state_clr_disabled(desc);
-	if (desc->irq_data.chip->irq_enable)
+	if (desc->irq_data.chip->irq_enable) {
 		desc->irq_data.chip->irq_enable(&desc->irq_data);
-	else
+	} else if (!ifmasked || irqd_irq_masked(&desc->irq_data)) {
 		desc->irq_data.chip->irq_unmask(&desc->irq_data);
+	}
 	irq_state_clr_masked(desc);
 }
 
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 4332d766619d..6eff2678cf6d 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -68,7 +68,7 @@ extern void __enable_irq(struct irq_desc *desc, unsigned int irq);
 
 extern int irq_startup(struct irq_desc *desc, bool resend);
 extern void irq_shutdown(struct irq_desc *desc);
-extern void irq_enable(struct irq_desc *desc);
+extern void irq_enable(struct irq_desc *desc, bool ifmasked);
 extern void irq_disable(struct irq_desc *desc);
 extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
 extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 0a9104b4608b..d8c474608c2d 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -448,7 +448,7 @@ void __enable_irq(struct irq_desc *desc, unsigned int irq)
 			goto err_out;
 		/* Prevent probing on this irq: */
 		irq_settings_set_noprobe(desc);
-		irq_enable(desc);
+		irq_enable(desc, true);
 		check_irq_resend(desc, irq);
 		/* fall-through */
 	}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/