Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
* Florian Fainelli | 2012-10-19 15:40:29 [+0200]: >On Friday 19 October 2012 11:36:25 Fainelli wrote: >> Sebastian Andrzej Siewior linutronix.de> writes: >> > >> > No. You do have a compatible entry. It first appeared on the ce4100 >> > CPU. If it happens to also work on the n450 then it seems to be >> > compatible with that one. "This" is documented somewhere??? >> > Usually you add 'compatible = "your cpu", "generic binding"' in case >> > you need a fixup / errata whatever for "your cpu". Even if you compare >> > all hpets from Intel there is the one or other difference / errata. >> >> Can we make sure that his hits the future 3.6 stable releases? We had to >> merge >> this back to your 3.6 kernel tree in order to have a functionnal CE4100 >> system. >> >> Thank you! > >Adding Adding Thomas, Ingo and Sebastian in CC. If someone needs it, yes. My understanding was that Thierry said we need a few OF specific patches and this alone won't help. Care to backport and test & post it? >-- >Florian Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
On Friday 19 October 2012 11:36:25 Fainelli wrote: > Sebastian Andrzej Siewior linutronix.de> writes: > > > > No. You do have a compatible entry. It first appeared on the ce4100 > > CPU. If it happens to also work on the n450 then it seems to be > > compatible with that one. "This" is documented somewhere… > > Usually you add 'compatible = "your cpu", "generic binding"' in case > > you need a fixup / errata whatever for "your cpu". Even if you compare > > all hpets from Intel there is the one or other difference / errata. > > Can we make sure that his hits the future 3.6 stable releases? We had to merge > this back to your 3.6 kernel tree in order to have a functionnal CE4100 > system. > > Thank you! Adding Adding Thomas, Ingo and Sebastian in CC. -- Florian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
Sebastian Andrzej Siewior linutronix.de> writes: > > No. You do have a compatible entry. It first appeared on the ce4100 > CPU. If it happens to also work on the n450 then it seems to be > compatible with that one. "This" is documented somewhere… > Usually you add 'compatible = "your cpu", "generic binding"' in case > you need a fixup / errata whatever for "your cpu". Even if you compare > all hpets from Intel there is the one or other difference / errata. Can we make sure that his hits the future 3.6 stable releases? We had to merge this back to your 3.6 kernel tree in order to have a functionnal CE4100 system. Thank you! -- Florian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
On Sat, Aug 11, 2012 at 07:26:38PM +0200, Sebastian Andrzej Siewior wrote: > * Thierry Reding | 2012-08-08 14:07:37 [+0200]: > > >With that in place, the driver code can match on "intel,hpet" to catch > >all implementations and use the more specific entries if quirks are > >required for the specific hardware. > > from http://lkml.org/lkml/2011/2/16/350: > > |"intel,ioapic" is probably too generic and can be dropped. Newer > |devices can claim compatibility with "intel,ioapic-ce4100" if they are > |indeed compatible so that device drivers don't need to be modified. > |It is better to anchor compatible values to real implementations that > |try to come up with 'generic' or wildcard strings. Ditto through the > |rest of the file. Oh well. I've seen just the opposite used on ARM, where you start from a generic implementation and compatible value and use more specific compatible values for device-specific quirks. But okay, the hardware that I use seems to work fine anyway, so I'll just leave it as-is. Thierry pgp8DXorgPHdB.pgp Description: PGP signature
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
* Thierry Reding | 2012-08-08 14:07:37 [+0200]: >With that in place, the driver code can match on "intel,hpet" to catch >all implementations and use the more specific entries if quirks are >required for the specific hardware. from http://lkml.org/lkml/2011/2/16/350: |"intel,ioapic" is probably too generic and can be dropped. Newer |devices can claim compatibility with "intel,ioapic-ce4100" if they are |indeed compatible so that device drivers don't need to be modified. |It is better to anchor compatible values to real implementations that |try to come up with 'generic' or wildcard strings. Ditto through the |rest of the file. >Thierry Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
On Wed, Aug 08, 2012 at 01:51:36PM +0200, Sebastian Andrzej Siewior wrote: > On 08/08/2012 12:46 PM, Thierry Reding wrote: > >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. > > No. You do have a compatible entry. It first appeared on the ce4100 > CPU. If it happens to also work on the n450 then it seems to be > compatible with that one. "This" is documented somewhere… > Usually you add 'compatible = "your cpu", "generic binding"' in case > you need a fixup / errata whatever for "your cpu". Even if you compare > all hpets from Intel there is the one or other difference / errata. Exactly, but "ce4100-hpet" isn't very generic. What I'm saying is that the last entry in the compatible list should be something generic, like "intel,hpet", which can be overridden by putting a more specific entry in front. I'd expect the ce4100 HPET to use something like this: compatible = "intel,ce4100-hpet", "intel,hpet"; On N450 this could for instance be: compatible = "intel,n450-hpet", "intel,hpet"; With that in place, the driver code can match on "intel,hpet" to catch all implementations and use the more specific entries if quirks are required for the specific hardware. Thierry pgpEXli2Pgtnp.pgp Description: PGP signature
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
On 08/08/2012 12:46 PM, Thierry Reding wrote:
+ id = irq_domain_add_linear(np, num,
+ &ioapic_irq_domain_ops,
+ (void *)ioapic_num);
This fits on two lines instead of three.
k
+ 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.
I see.
Besides the above nitpicks:
Reviewed-by: Thierry Reding
Tested-by: Thierry Reding
Thanks for testing.
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.
No. You do have a compatible entry. It first appeared on the ce4100
CPU. If it happens to also work on the n450 then it seems to be
compatible with that one. "This" is documented somewhere…
Usually you add 'compatible = "your cpu", "generic binding"' in case
you need a fixup / errata whatever for "your cpu". Even if you compare
all hpets from Intel there is the one or other difference / errata.
Thierry
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
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
> ---
> 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
Tested-by: Thierry Reding
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
Re: [PATCH] x86/dt: use linear irq domain for ioapic(s).
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 From a quick glance this looks much better than my patch. This depends on a couple of patches in linux-next it seems, so I'll have to do some rebasing before I can test. Still I think I should be able to get back to you until the end of the week. Thierry pgpC4Ek14Paqs.pgp Description: PGP signature
