Re: [PATCH 11/25] i386 irq: Dynamic irq support

[Eric W. Biederman]

Rajesh Shah <rajesh.shah@xxxxxxxxx> writes:

> On Tue, Jun 20, 2006 at 08:21:00PM -0600, Eric W. Biederman wrote:
>> Rajesh Shah <rajesh.shah@xxxxxxxxx> writes:
>> 
>> > It would be really good to decouple MSI implementation from IO
>> > APICs, since there's really no real hardware dependence here.
>> > This code can actually go to arch/xxx/pci/msi-apic.c
>> 
>> I agree in theory.  In practice however msi interrupts look like io_apics.
>> with a different register set and the use all of the same support facilities.
>> So until that part of the architecture is refactored it doesn't make much
>> sense.  There is a slightly better case for moving the code into a separate
>> file.  Namely I think I know of a second common implementation for x86_64.
>> At which point the files will probably be named msi-intel.c and msi-amd.c
>> Or something like that.  
>> 
> Actually, I meant just the vector tracking code could be in a
> separate file and the ioapic and msi code could both assign
> vectors from a common routine. I had the patch below in my
> patchkit, plus another patch for x86_64 to do the same thing
> in io_apic.c and share the same intrvec.c file between the 
> two archs. Once you have this, the MSI callbacks in arch
> code can be moved out of io_apic.c

Well irq.c is probably the obvious place to put it.

But that goes way beyond small obviously correct steps.
So there is no way I'm going to include a change like
that in the middle of my patchset because it is unnecessary.

Doing this kind of thing later is certainly sane.

I guess this is a difference in focus.  You have been focused
on code cleanup.  I have been focused on breaking the unnatural tying
between parts of the code.

As for this specific patch it makes no sense to only move half
of assign_irq_vector to a different file.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/