Message ID | 1502319099-2782-3-git-send-email-david.daney@cavium.com |
---|---|
State | New |
Headers | show |
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() ? > /** > + * 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(). Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 16 Aug 2017, David Daney wrote: > 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? The latter. > > 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. Ok, so we might rename that thing to something else than edge() as all edge handlers have this processing loop which is required not to lose edges. Thanks tglx -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/irq.h b/include/linux/irq.h index d2d54379..86a2273 100644 --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -568,6 +568,8 @@ static inline int irq_set_parent(int irq, int parent_irq) extern int irq_chip_pm_get(struct irq_data *data); extern int irq_chip_pm_put(struct irq_data *data); #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY +extern void handle_fasteoi_edge_irq(struct irq_desc *desc); +extern void handle_fasteoi_level_irq(struct irq_desc *desc); extern void irq_chip_enable_parent(struct irq_data *data); extern void irq_chip_disable_parent(struct irq_data *data); extern void irq_chip_ack_parent(struct irq_data *data); diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c index 6514f07..7511591 100644 --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -1093,6 +1093,108 @@ void irq_cpu_offline(void) #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY /** + * 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); + + cond_unmask_eoi_irq(desc, chip); + + raw_spin_unlock(&desc->lock); + return; +out: + if (!(chip->flags & IRQCHIP_EOI_IF_HANDLED)) + chip->irq_eoi(&desc->irq_data); + raw_spin_unlock(&desc->lock); +} +EXPORT_SYMBOL_GPL(handle_fasteoi_edge_irq); + +/** + * handle_fasteoi_level_irq - irq handler for level 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_mask_ack() function + * called. + */ +void handle_fasteoi_level_irq(struct irq_desc *desc) +{ + struct irq_chip *chip = desc->irq_data.chip; + + raw_spin_lock(&desc->lock); + mask_ack_irq(desc); + + 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); + + preflow_handler(desc); + handle_irq_event(desc); + + cond_unmask_eoi_irq(desc, chip); + + raw_spin_unlock(&desc->lock); + return; +out: + if (!(chip->flags & IRQCHIP_EOI_IF_HANDLED)) + chip->irq_eoi(&desc->irq_data); + raw_spin_unlock(&desc->lock); +} +EXPORT_SYMBOL_GPL(handle_fasteoi_level_irq); + +/** * irq_chip_enable_parent - Enable the parent interrupt (defaults to unmask if * NULL) * @data: Pointer to interrupt specific data
Follow-on patch for gpio-thunderx uses a irqdomain hierarchy which requires slightly different flow handlers, add them to chip.c which contains most of the other flow handlers. Signed-off-by: David Daney <david.daney@cavium.com> --- include/linux/irq.h | 2 ++ kernel/irq/chip.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+)