Messages in this thread | | | Date | Sat, 28 Oct 2006 19:05:16 +0400 | From | Sergei Shtylyov <> | Subject | Re: [PATCH -rt] powerpc update |
| |
Hello.
Ingo Molnar wrote: > * Daniel Walker <dwalker@mvista.com> wrote:
>>Pay close attention to the fasteoi interrupt threading. I added usage >>of mask/unmask instead of using level handling, which worked well on >>PPC.
> this is wrong - it should be doing mask+ack.
It's what it was doing effectively. And what was wrong was calling ack() which OpenPIC driver didn't (and was not obliged to) support.
> also note that you changed:
>>- goto out_unlock;
> to:
>>+ goto out;
> and you even tried to hide your tracks: > > >> out: >> desc->chip->eoi(irq); >>-out_unlock: >> spin_unlock(&desc->lock);
> :-)
> really, the ->eoi() op should only be called for true fasteoi cases.
Why is that? eoi() is effectively the same as ack() in this case. I must note that what's the "standard" 8259 driver is doing in mask_ack() seems misleading since it actually masks IRQ and sends EOI there.
> What we want here is to turn the fasteoi handler into a handler that > does mask+ack and then unmask. Not 'mask+eoi ... unmask' as your patch > does.
That's effectively the same for OpenPIC. Maybe that implemetation just didn't look graceful but it was *correct*. And the current one is at least incomplete.
I can see 3 ways to get out of this situation now:
1. Revert this change and use mask() + eoi() approach suggested by Daniel.
2. Add the ack() handler to OpenPIC driver -- and point it to mpic_eoi().
3. Do the same as x86 APIC driver does and use level/egde flows instead of fasteoi for the case when IRQs are threaded -- that ensues doing (2) as well.
Note that all three aproaches lead to the effectively the same behavior WRT OpenPIC (except for the edge-triggered IRQs in 3rd case). Opinions?
> Ingo
WBR, Sergei - 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/
| |