On 08/14/2017 03:25 AM, Thomas Gleixner wrote:
On Wed, 9 Aug 2017, David Daney wrote:
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
Can we please make them conditional in order not to bloat all kernels with
it? Like we do for handle_edge_eoi_irq() ?
Yes. Because these are initially used by exactly one driver, I could
just make it conditional on that driver being enabled.
Alternately, I could invent a new Kconfig symbol to gate these and
select that when the driver is enabled.
Do you have a preference?
/**
+ * handle_fasteoi_edge_irq - irq handler for edge hierarchy
+ * stacked on transparent controllers
+ *
+ * @desc: the interrupt description structure for this irq
+ *
+ * Like handle_fasteoi_irq(), but for use with hierarchy where
+ * the irq_chip also needs to have its ->irq_ack() function
+ * called.
+ */
+void handle_fasteoi_edge_irq(struct irq_desc *desc)
+{
+ struct irq_chip *chip = desc->irq_data.chip;
+
+ raw_spin_lock(&desc->lock);
+
+ if (!irq_may_run(desc))
+ goto out;
+
+ desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+
+ /*
+ * If its disabled or no action available
+ * then mask it and get out of here:
+ */
+ if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
+ desc->istate |= IRQS_PENDING;
+ mask_irq(desc);
+ goto out;
+ }
+
+ kstat_incr_irqs_this_cpu(desc);
+ if (desc->istate & IRQS_ONESHOT)
+ mask_irq(desc);
+
+ /* Start handling the irq */
+ desc->irq_data.chip->irq_ack(&desc->irq_data);
+
+ preflow_handler(desc);
+ handle_irq_event(desc);
Hmm. That's quite different to the way we handle edge interrupts
normally. See handle_edge_irq() and handle_edge_eoi_irq().
Yes, these are not standard edge interrupts. If they were, I wouldn't
need new handlers.
For this particular irqdomain hierarchy, I need exactly
handle_fasteoi_irq() semantics with the addition of a call to the
chip->irq_ack() function *before* the handler is called. I chose to
"clone" and enhance handle_fasteoi_irq(), rather than adding hooks with
runtime checks to the existing handle_fasteoi_irq(). There is code
bloat this way, but a smaller risk of breaking other things.
Any additional "stuff" that is not needed to cover this use case would
just be adding dead code. In the future, if there is a need to enable
more users of these functions, I would not object to doing more.
Thanks,
David Daney
Thanks,
tglx
