Re: [PATCH] I/O APIC: Timer through 8259A revamp

[Ingo Molnar <mingo@elte.hu>]

* Maciej W. Rozycki <macro@linux-mips.org> wrote:

> There is some seemingly unnecessary cruft accumulated in the 8254 
> timer setup path for the I/O APIC.  Some attempts have been made to 
> cater for broken BIOSes reporting the ExtINTA I/O APIC pin cascaded 
> from the master 8259A as the native timer interrupt input, which 
> interfere with the setup sequence in a non-obvious way and require a 
> pair of additional kernel command-line parameters.
> 
> I think this is unnecessary.  All the bits are already in place.  With 
> the removal of AEOI bits for non-i82489DX systems the whole setup can 
> be simplified.  Here is a set of changes that reuse the existing code 
> for potentially broken systems.  The observation is broken systems do 
> not have a notion of ExtINTA pins, so only one pin/apic pair will be 
> determined as useful for the timer interrupt.  Therefore it should be 
> safe to select this single pin/apic to test both directly and through 
> the 8259A.
> 
> This change implements this approach by copying timer pin information 
> found to the other pin if one has been found only.  This way both 
> tests are always performed.  The timer input of the 8259A is carefully 
> unmasked if needed only and masked again if found unneeded.  This is 
> because some systems seem sensitive for the master 8259A interrupt 
> being active even if all the APIC inputs this line is wired to are 
> masked.
> 
> The "disable_8254_timer" and "enable_8254_timer" kernel parameters are 
> removed as no longer needed.

applied to latest -tip, thanks.

> Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
> ---
>  It works for my system, but it definitely requires much, much more 
> testing.  The patch depends on patch-2.6.26-rc1-20080505-timer_ack-1 
> sent earlier.

agreed, scary patch - but nice cleanups!

> Of course these "if (pin? != -1)" statements are useless (though 
> harmless) now, but I think that change deserves a separate patch not 
> to obfuscate changes to code which is obscure enough already.

please send another patch for that. If you can think of a good way of 
splitting up this patch into smaller units feel free to do that too ... 
Dangerous changes are much better if they happen in small incremental 
steps. (even if the sum of the changes is not any less dangerous - it 
just makes any trouble easier to bisect and fix)

	Ingo