versatile: don't generate a duplicate IRQ domain

PATCH New Jamie Iles 3 participants 9 messages Jan 6, 2012 – Jan 12, 2012

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm/versatile express platform Arm integrator, versatile and realview support

lore

Share & tools

lore

Jamie Iles

author

2012-01-06 16:39 UTC #0 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Now that the VIC driver handles the irqdomain natively we don't need to generate one in the versatile core code. Longer term we should move the initialisation of both IRQ controllers to using of_irq_init() but that'll need a little more work.

Cc Russell King <[email protected]>

Cc Grant Likely <[email protected]>

Signed-off-by Jamie Iles <[email protected]>

---

I don't have a versatile platform so I can't run-test this, but I suspect that when booting with device tree without this patch it'll warn with "error: irq_desc already assigned to a domain".

I don't think this can be converted to of_irq_init() straight away without having all of the devices using IRQ's from DT and not relying on static IRQ numbers and without the hardware to test it I don't have enough confidence to do that.

 arch/arm/mach-versatile/core.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

arch/arm/mach-versatile/core.c +0 -7

index cbcda61..e716fa4 100644
@@ -86,12 +86,6 @@ static struct fpga_irq_data sic_irq = {
 #define PIC_MASK	0
 #endif
 
-/* Lookup table for finding a DT node that represents the vic instance */
-static const struct of_device_id vic_of_match[] __initconst = {
-	{ .compatible = "arm,versatile-vic", },
-	{}
-};
-
 static const struct of_device_id sic_of_match[] __initconst = {
 	{ .compatible = "arm,versatile-sic", },
 	{}

@@ -100,7 +94,6 @@ static const struct of_device_id sic_of_match[] __initconst = {
 void __init versatile_init_irq(void)
 {
 	vic_init(VA_VIC_BASE, IRQ_VIC_START, ~0, 0);
-	irq_domain_generate_simple(vic_of_match, VERSATILE_VIC_BASE, IRQ_VIC_START);
 
 	writel(~0, VA_SIC_BASE + SIC_IRQ_ENABLE_CLEAR);

Signature · 1 line

1.7.5.4

Russell King - ARM Linux

2012-01-12 20:37 UTC #1 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Quoted 8 lines · Jamie Iles

Now that the VIC driver handles the irqdomain natively we don't need to generate one in the versatile core code. Longer term we should move the initialisation of both IRQ controllers to using of_irq_init() but that'll need a little more work.
Cc Russell King <[email protected]>
Cc Grant Likely <[email protected]>
Signed-off-by Jamie Iles <[email protected]>

Please confirm that this patch can be applied ontop of commit 356b95424cfb456e14a59eaa579422ce014c424b
(Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k)

Quoted 8 lines

I don't have a versatile platform so I can't run-test this, but I suspect that when booting with device tree without this patch it'll warn with "error: irq_desc already assigned to a domain".
I don't think this can be converted to of_irq_init() straight away without having all of the devices using IRQ's from DT and not relying on static IRQ numbers and without the hardware to test it I don't have enough confidence to do that.

What's needed to properly test this? Note that I don't run these boards with DT.

Jamie Iles

author

2012-01-12 21:58 UTC #2 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Hi Russell,

Quoted 6 lines · Russell King - ARM Linux

Quoted 5 lines · Jamie Iles
Now that the VIC driver handles the irqdomain natively we don't need to generate one in the versatile core code. Longer term we should move the initialisation of both IRQ controllers to using of_irq_init() but that'll need a little more work.

Quoted 7 lines

Quoted 3 lines
Cc Russell King <[email protected]>
Cc Grant Likely <[email protected]>
Signed-off-by Jamie Iles <[email protected]>
Please confirm that this patch can be applied ontop of commit 356b95424cfb456e14a59eaa579422ce014c424b
(Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k)

OK, this does apply, but it doesn't fail in the way I had expected. Actually, it doesn't barf at all because the generated domain that my patch removes has 0 IRQ's so it never gets attached to the irq_desc's.

However, this patch is still valid as we end up with two domains associated with the VIC but we get lucky as the real domain is registered first. I can update the patch description to reflect this if you still want to apply it.

Incidentally, I've managed to get the versatile kernel booting under qemu (using an experimental build with DT support, up to trying to mount a rootfs), which seems to work nicely. I just need to find a suitable rootfs and that'll be a nice test system.

Jamie

Russell King - ARM Linux

2012-01-12 22:14 UTC #3 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Quoted 13 lines · Jamie Iles

Hi Russell,
Quoted 10 lines · Russell King - ARM Linux
Quoted 8 lines · Jamie Iles
Now that the VIC driver handles the irqdomain natively we don't need to generate one in the versatile core code. Longer term we should move the initialisation of both IRQ controllers to using of_irq_init() but that'll need a little more work.
Cc Russell King <[email protected]>
Cc Grant Likely <[email protected]>
Signed-off-by Jamie Iles <[email protected]>

Quoted 7 lines

Quoted 3 lines
Please confirm that this patch can be applied ontop of commit 356b95424cfb456e14a59eaa579422ce014c424b
(Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k)
OK, this does apply, but it doesn't fail in the way I had expected. Actually, it doesn't barf at all because the generated domain that my patch removes has 0 IRQ's so it never gets attached to the irq_desc's.

If you say...

Quoted 4 lines

However, this patch is still valid as we end up with two domains associated with the VIC but we get lucky as the real domain is registered first. I can update the patch description to reflect this if you still want to apply it.

If you think the patch comments don't reflect what the patch is doing then yes please.

I don't know how this irq domain stuff works so I really can't say anything much about this patch other than trust you that it's correct.

Grant Likely

2012-01-12 23:07 UTC #4 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Quoted 29 lines · Russell King - ARM Linux

Quoted 13 lines · Jamie Iles
Hi Russell,
Quoted 10 lines · Russell King - ARM Linux
Quoted 8 lines · Jamie Iles
Now that the VIC driver handles the irqdomain natively we don't need to generate one in the versatile core code. ?Longer term we should move the initialisation of both IRQ controllers to using of_irq_init() but that'll need a little more work.
Cc Russell King <[email protected]>
Cc Grant Likely <[email protected]>
Signed-off-by Jamie Iles <[email protected]>
Quoted 7 lines
Quoted 3 lines
Please confirm that this patch can be applied ontop of commit 356b95424cfb456e14a59eaa579422ce014c424b
(Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k)
OK, this does apply, but it doesn't fail in the way I had expected. Actually, it doesn't barf at all because the generated domain that my patch removes has 0 IRQ's so it never gets attached to the irq_desc's.
If you say...
Quoted 4 lines
However, this patch is still valid as we end up with two domains associated with the VIC but we get lucky as the real domain is registered first. ?I can update the patch description to reflect this if you still want to apply it.

Quoted 5 lines

If you think the patch comments don't reflect what the patch is doing then yes please.
I don't know how this irq domain stuff works so I really can't say anything much about this patch other than trust you that it's correct.

This patch looks incomplete to me. With it applied I don't think the vic's dt node won't get associated with the irq_domain, which will make dt irq mapping fail to work. The node pointer needs to get passed into the vic initialization routine if the irq_domain_generate_simple() call is to be removed.

Signature · 2 lines

Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Russell King - ARM Linux

2012-01-12 23:18 UTC #5 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Quoted 34 lines · Grant Likely

Quoted 32 lines · Russell King - ARM Linux
Quoted 13 lines · Jamie Iles
Hi Russell,
Quoted 10 lines · Russell King - ARM Linux
Quoted 8 lines · Jamie Iles
Now that the VIC driver handles the irqdomain natively we don't need to generate one in the versatile core code. ?Longer term we should move the initialisation of both IRQ controllers to using of_irq_init() but that'll need a little more work.
Cc Russell King <[email protected]>
Cc Grant Likely <[email protected]>
Signed-off-by Jamie Iles <[email protected]>
Quoted 7 lines
Quoted 3 lines
Please confirm that this patch can be applied ontop of commit 356b95424cfb456e14a59eaa579422ce014c424b
(Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k)
OK, this does apply, but it doesn't fail in the way I had expected. Actually, it doesn't barf at all because the generated domain that my patch removes has 0 IRQ's so it never gets attached to the irq_desc's.
If you say...
Quoted 4 lines
However, this patch is still valid as we end up with two domains associated with the VIC but we get lucky as the real domain is registered first. ?I can update the patch description to reflect this if you still want to apply it.
If you think the patch comments don't reflect what the patch is doing then yes please.

Quoted 8 lines

Quoted 2 lines
I don't know how this irq domain stuff works so I really can't say anything much about this patch other than trust you that it's correct.
This patch looks incomplete to me. With it applied I don't think the vic's dt node won't get associated with the irq_domain, which will make dt irq mapping fail to work. The node pointer needs to get passed into the vic initialization routine if the irq_domain_generate_simple() call is to be removed.

Even with this:

        v->domain.irq_base = irq;
        v->domain.nr_irq = 32;
#ifdef CONFIG_OF_IRQ
        v->domain.of_node = of_node_get(node);
#endif /* CONFIG_OF */
        v->domain.ops = &irq_domain_simple_ops;
        irq_domain_add(&v->domain);

which can be found in vic_register() ?

Jamie Iles

author

2012-01-12 23:26 UTC #6 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Quoted 54 lines · Russell King - ARM Linux

Quoted 34 lines · Grant Likely
Quoted 32 lines · Russell King - ARM Linux
Quoted 13 lines · Jamie Iles
Hi Russell,
Quoted 10 lines · Russell King - ARM Linux
Quoted 8 lines · Jamie Iles
Now that the VIC driver handles the irqdomain natively we don't need to generate one in the versatile core code. ?Longer term we should move the initialisation of both IRQ controllers to using of_irq_init() but that'll need a little more work.
Cc Russell King <[email protected]>
Cc Grant Likely <[email protected]>
Signed-off-by Jamie Iles <[email protected]>
Quoted 7 lines
Quoted 3 lines
Please confirm that this patch can be applied ontop of commit 356b95424cfb456e14a59eaa579422ce014c424b
(Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k)
OK, this does apply, but it doesn't fail in the way I had expected. Actually, it doesn't barf at all because the generated domain that my patch removes has 0 IRQ's so it never gets attached to the irq_desc's.
If you say...
Quoted 4 lines
However, this patch is still valid as we end up with two domains associated with the VIC but we get lucky as the real domain is registered first. ?I can update the patch description to reflect this if you still want to apply it.
If you think the patch comments don't reflect what the patch is doing then yes please.
Quoted 8 lines
Quoted 2 lines
I don't know how this irq domain stuff works so I really can't say anything much about this patch other than trust you that it's correct.
This patch looks incomplete to me. With it applied I don't think the vic's dt node won't get associated with the irq_domain, which will make dt irq mapping fail to work. The node pointer needs to get passed into the vic initialization routine if the irq_domain_generate_simple() call is to be removed.
Even with this:
        v->domain.irq_base = irq;
        v->domain.nr_irq = 32;
#ifdef CONFIG_OF_IRQ
        v->domain.of_node = of_node_get(node);
#endif /* CONFIG_OF */
        v->domain.ops = &irq_domain_simple_ops;
        irq_domain_add(&v->domain);

Quoted 1 line

which can be found in vic_register() ?

No, because:

	vic_init()
	  __vic_init() -> with NULL np pointer
	    vic_register()

So we won't have the device_node for the VIC. I think I really need to convert the versatile stuff over to the full of_irq_init() method of probing, which isn't as bad as I thought as everything is populated from the DT there but it looked like it could be registered statically too from versatile_init(). I'll try and get everything going from the DT now.

Jamie

Russell King - ARM Linux

2012-01-12 23:28 UTC #7 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Quoted 6 lines · Jamie Iles

No, because:

	vic_init()
	  __vic_init() -> with NULL np pointer
	    vic_register()

Quoted 6 lines

So we won't have the device_node for the VIC. I think I really need to convert the versatile stuff over to the full of_irq_init() method of probing, which isn't as bad as I thought as everything is populated from the DT there but it looked like it could be registered statically too from versatile_init(). I'll try and get everything going from the DT now.

Won't this make Versatile a DT-only platform?

Jamie Iles

author

2012-01-12 23:30 UTC #8 lore raw

PATCH 1/1 versatile: don't generate a duplicate IRQ domain

Quoted 7 lines · Russell King - ARM Linux

Quoted 6 lines · Jamie Iles
No, because:
	vic_init()
	  __vic_init() -> with NULL np pointer
	    vic_register()

Quoted 8 lines

Quoted 6 lines
So we won't have the device_node for the VIC. I think I really need to convert the versatile stuff over to the full of_irq_init() method of probing, which isn't as bad as I thought as everything is populated from the DT there but it looked like it could be registered statically too from versatile_init(). I'll try and get everything going from the DT now.
Won't this make Versatile a DT-only platform?

No, crap explanation by me there. I'll add a versatile_dt_irq_init() that creates the controllers from the DT, for the non DT case everything will be as before using the static assignments and static irq_descs. I'll make sure to test using both booting methods.

Jamie