Messages in this thread | | | Date | Thu, 9 Oct 2003 18:46:04 +0100 | From | viro@parcelfa ... | Subject | Re: [RFC] disable_irq()/enable_irq() semantics and ide-probe.c |
| |
On Thu, Oct 09, 2003 at 09:01:36AM -0700, Linus Torvalds wrote: > > On Thu, 9 Oct 2003 viro@parcelfarce.linux.theplanet.co.uk wrote: > > > > IOW, the question is: do we want enable_irq() to undo all effects of > > disable_irq()? Whether the current behaviour is intentional or not, > > it's worth documenting, IMO... > > I think yes, it would be clean to make enable_irq() clean up properly. > That includes clearing the IRQ_INPROGRESS bit along with the IRQ_DISABLED > but. It won't even make the codepath longer, it just changes a constant. > > And yes, I think we should just remove the clearing of depth in > setup_irq(), _and_ make the enabling of the irq be dependent on depth > being non-zero. > > In 2.6.x terms, we'd like to go towards something like the appended, but > this actually has a _different_ problem: we have separate "->startup()" vs > "->enable()" functions for the irq controller, and this means that if the > interrupt was disabled when the first irq handler was requested, > "->startup()" wouldn't be called at all. > > So my suggestion would be: > - do the IRQ_INPROGRESS clearing (safe, since disable_irq() will have > waited for it if it was valid) > - leave the depth reset as-is for now, and think about how we'd like to > solve it. > - make the synchronize_irq() conditional in irq_disable() (2.6.x now > already does that part, so it's not in the patch) > > Hmm?
I've looked through the uses of IRQ_DISABLED in 2.6. Fun things found:
a) on alpha: void i8259a_end_irq(unsigned int irq) { if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS))) i8259a_enable_irq(irq); } and similar in other ->end()
on x86: static void end_8259A_irq (unsigned int irq) { if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS)) && irq_desc[irq].action) enable_8259A_irq(irq); }
b) if we get *two* interrupts while irq is enabled and not registered, we'll be stuck with IRQ_PENDING in addition to IRQ_INPROGRESS. Which can, AFAICS, confuse other code.
c) mind-boggling amount of code duplication - are there any plans to take that stuff to kernel/*? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |