Messages in this thread | | | Subject | Re: [RFC patch 2/3] genirq: Add buslock support for irq chips on slow busses | From | Peter Zijlstra <> | Date | Fri, 14 Aug 2009 11:29:35 +0200 |
| |
On Fri, 2009-08-14 at 11:18 +0200, Ingo Molnar wrote: > * Thomas Gleixner <tglx@linutronix.de> wrote: > > > On Fri, 14 Aug 2009, Peter Zijlstra wrote: > > > On Thu, 2009-08-13 at 19:40 +0000, Thomas Gleixner wrote: > > > > > > > > +/** > > > > + * disable_slowbus_irq - disable an slowbus irq and wait for completion > > > > + * @irq: Interrupt to disable > > > > + * > > > > + * Disable the selected interrupt line. Enables and Disables are > > > > + * nested. > > > > + * This function waits for any pending IRQ handlers for this interrupt > > > > + * to complete before returning. If you use this function while > > > > + * holding a resource the IRQ handler may need you will deadlock. > > > > + * > > > > + * This function must not be called from IRQ context. > > > > + */ > > > > +void disable_slowbus_irq(unsigned int irq) > > > > +{ > > > > + struct irq_desc *desc = irq_to_desc(irq); > > > > + > > > > + if (!desc || !desc->chip || !desc->chip->bus_lock) > > > > + return; > > > > + > > > > + desc->chip->bus_lock(irq); > > > > + disable_irq_nosync(irq); > > > > + if (desc->action) > > > > + synchronize_irq(irq); > > > > + desc->chip->bus_sync_unlock(irq); > > > > +} > > > > +EXPORT_SYMBOL(disable_slowbus_irq); > > > > > > Should we also not check that desc->chip->bus_lock is not set for the > > > regular function disable_irq()? > > > > > > It seems to me mixing disable_irq() and disable_slowbus_irq() > > > is a recipe for disaster. > > > > > > Same for the other slowbus functions of course. > > > > Yeah, that's what I wanted to avoid with the first version, which > > did the conditional locking and did not require a separate API, > > but Ingo frowned upon the conditional lock. > > Mind posting that version too? > > Conditional locking is really nasty but if the only other option is > nastier there's not much we can do, is there?
Ah what I meant was that we can make the posted version better by adding
BUG_ON(desc && desc->chip && desc->chip->bus_lock);
to disable_irq/enable_irq/free_irq.
| |