On Mon, Aug 06, 2012 at 09:38:11AM +0200, Sebastian Andrzej Siewior wrote:
> The former conversion to irq_domain_add_legacy() did not fully work
> since we miss the irq decs for NR_IRQS_LEGACY+.
> Ideally we could use irq_domain_add_simple() or the no-map variant (and
> program the virq <-> line mapping directly into ioapic) but this would
> require a different irq lookup in "do_IRQ()" and won't work with ACPI
> without changes. So this is probably easiest for everyone.
>
> Signed-off-by: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
> ---
> arch/x86/kernel/devicetree.c | 52
> ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
> index 3ae2ced..df225fc 100644
> --- a/arch/x86/kernel/devicetree.c
> +++ b/arch/x86/kernel/devicetree.c
> @@ -342,6 +342,48 @@ const struct irq_domain_ops ioapic_irq_domain_ops = {
> .xlate = ioapic_xlate,
> };
>
> +static void dt_add_ioapic_domain(unsigned int ioapic_num,
> + struct device_node *np)
> +{
> + struct irq_domain *id;
> + struct mp_ioapic_gsi *gsi_cfg;
> + int ret;
> + int num;
> +
> + gsi_cfg = mp_ioapic_gsi_routing(ioapic_num);
> + num = gsi_cfg->gsi_end - gsi_cfg->gsi_base + 1;
> +
> + id = irq_domain_add_linear(np, num,
> + &ioapic_irq_domain_ops,
> + (void *)ioapic_num);
This fits on two lines instead of three.
> + BUG_ON(!id);
> + if (gsi_cfg->gsi_base == 0) {
> + /*
> + * The first NR_IRQS_LEGACY irq descs are allocated in
> + * early_irq_init() and need just a mapping. The
> + * remaining irqs need both. All of them are preallocated
> + * and assigned so we can keep the 1:1 mapping which the ioapic
> + * is having.
> + */
> + ret = irq_domain_associate_many(id, 0, 0, NR_IRQS_LEGACY);
> + if (ret)
> + pr_err("Error mapping legacy irqs: %d\n", ret);
> +
> + if (num > NR_IRQS_LEGACY) {
> + ret = irq_create_strict_mappings(id, NR_IRQS_LEGACY,
> + NR_IRQS_LEGACY, num - NR_IRQS_LEGACY);
> + if (ret)
> + pr_err("Error creating mapping for the "
> + "remaining irqs: %d\n", ret);
There's an extra space between "remaining" and "irqs". Also other places
use the spelling IRQ and IRQs respectively in strings, so it may be nice
to stay consistent.
> + }
> + irq_set_default_host(id);
> + } else {
> + ret = irq_create_strict_mappings(id, gsi_cfg->gsi_base, 0, num);
> + if (ret)
> + pr_err("Error creating irq mapping: %d\n", ret);
> + }
> +}
> +
> static void __init ioapic_add_ofnode(struct device_node *np)
> {
> struct resource r;
> @@ -356,15 +398,7 @@ static void __init ioapic_add_ofnode(struct device_node
> *np)
>
> for (i = 0; i < nr_ioapics; i++) {
> if (r.start == mpc_ioapic_addr(i)) {
> - struct irq_domain *id;
> - struct mp_ioapic_gsi *gsi_cfg;
> -
> - gsi_cfg = mp_ioapic_gsi_routing(i);
> -
> - id = irq_domain_add_legacy(np, 32, gsi_cfg->gsi_base, 0,
> - &ioapic_irq_domain_ops,
> - (void*)i);
> - BUG_ON(!id);
> + dt_add_ioapic_domain(i, np);
> return;
> }
> }
Besides the above nitpicks:
Reviewed-by: Thierry Reding <thierry.red...@avionic-design.de>
Tested-by: Thierry Reding <thierry.red...@avionic-design.de>
On another note, I saw that you've used the "intel,ce4100" prefix in
various places and I wonder if it would be useful to replace them with
something more generic like "intel,hpet", "intel,lapic" and
"intel,ioapic" respectively. The hardware that I use is based on an Atom
N450 and works with the current code, so it really isn't ce4100-
specific.
Given that this is x86/devicetree only and fixes things that didn't work
before, can it go into 3.6? Backporting to stable is probably not worth
it because it depends on a number of other IRQ domain patches that are
only available in 3.6.
Thierry
pgpvs1llWiZx4.pgp
Description: PGP signature
