* Steffen Persvold <[email protected]> wrote:
> Hi,
>
> We’re preparing our APIC driver (arch/x86/kernel/apic/apic_numachip.c) with
> next-gen hardware support and in that process I have a question on what the
> cleanest approach would be.
>
> Both current generation and next generation chips will share a lot of similar
> code, but some of the core functionality is slightly different (such as the
> address to which you communicate with the APIC ICR to send IPIs, how to
> derive
> APIC IDs etc.).
>
> The way I see it, we have few alternatives :
>
> 1) Create a new arc/x86/kernel/apic/apic_numachip2.c (and corresponding entry
> in
> the Makefile) which has a new “struct apic” with function pointers to the
> next-gen specific code. The new APIC driver would still only need
> CONFIG_X86_NUMACHIP to be compiled.
>
> 2) Modify the existing apic_numachip.c to recognise the different HW
> generations
> (trivial) and use function pointers to differentiate the IPI send calls
> (among
> other things), but use the *same* “struct apic” for both (the function
> pointers
> referenced in “struct apic” would need a new indirection level to
> differentiate
> between hardware revs).
>
> 3) Have two different “struct apic” entries in the existing apic_numachip.c
> source file, with separate oem_madt check functions etc. This would only be
> marginally different than 1) as far as implementation and code duplication
> goes,
> but it would be contained to one C source file and object file (silly
> question,
> maybe: would the apic_driver enumeration even work if it’s all in the same
> object file?)
So 1) is clearly suboptimal if the code is similar to a level of 90% (which
your
description suggests).
So I'd use a combination of 3) and 2): i.e. have two struct apic's, one of
which
gets picked during platform init, but try to share functions wherever possible.
If two different versions of functions are only trivially different, then
figure
out a way to share the code. If they are materially different (such as APIC ID
calculation would be I suspect), split it into two functions.
If needed you can also put runtime data into struct apic, so that the base
address
becomes configurable.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/