arm/tegra: convert tegra20 to GIC devicetree binding

PATCH Superseded Stephen Warren 4 participants 11 messages Nov 29, 2011

Subsystems (4)

Device tree bindings Arm (32-bit) Arm sub-architectures Tegra architecture support

lore

Share & tools

lore

Stephen Warren

author

2011-11-29 00:54 UTC #0 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

From: [email protected] <[email protected]>

Convert tegra20 IRQ intialization to the GIC devicetree binding. Modify the interrupt definitions in the dts files according to Documentation/devicetree/bindings/arm/gic.txt

v2 (swarren):
* Removed some stale GIC init code from board-dt.c
* Undid some accidental 0x -> 0x0 search/replace.

Signed-off-by Peter De Schrijver <[email protected]>

Acked-by Rob Herring <[email protected]>

Signed-off-by Stephen Warren <[email protected]>

---
 arch/arm/boot/dts/tegra-paz00.dts |    2 +-
 arch/arm/boot/dts/tegra20.dtsi    |   48 ++++++++++++++++++++----------------
 arch/arm/mach-tegra/board-dt.c    |   11 --------
 arch/arm/mach-tegra/irq.c         |   18 ++++++++++++-
 4 files changed, 44 insertions(+), 35 deletions(-)

arch/arm/boot/dts/tegra-paz00.dts +1 -1

index 4f6a8aa..1a1d702 100644
@@ -27,7 +27,7 @@
 		#size-cells = <0>;
 		compatible = "nvidia,nvec";
 		reg = <0x7000C500 0x100>;
-		interrupts = <124>;
+		interrupts = <0 92 0x04>;
 		clock-frequency = <80000>;
 		request-gpios = <&gpio 170 0>;
 		slave-addr = <138>;

arch/arm/boot/dts/tegra20.dtsi +27 ~~-21~~

164 lines not on this page · raw message

arch/arm/mach-tegra/board-dt.c +0 ~~-11~~

index 6e35417..bcbb677 100644
@@ -87,11 +87,6 @@ static struct of_device_id tegra_dt_match_table[] __initdata = {
 	{}
 };
 
-static struct of_device_id tegra_dt_gic_match[] __initdata = {
-	{ .compatible = "nvidia,tegra20-gic", },
-	{}
-};
-
 static struct {
 	char *machine;
 	void (*init)(void);

@@ -105,14 +100,8 @@ static struct {
 
 static void __init tegra_dt_init(void)
 {
-	struct device_node *node;
 	int i;
 
-	node = of_find_matching_node_by_address(NULL, tegra_dt_gic_match,
-						TEGRA_ARM_INT_DIST_BASE);
-	if (node)
-		irq_domain_add_simple(node, INT_GIC_BASE);
-
 	tegra_clk_init_from_table(tegra_dt_clk_init_table);
 
 	/*

arch/arm/mach-tegra/irq.c +16 -2

index 8ad82af..c6d02a4 100644
@@ -21,6 +21,7 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/io.h>
+#include <linux/of_irq.h>
 
 #include <asm/hardware/gic.h>

@@ -109,6 +110,11 @@ static int tegra_retrigger(struct irq_data *d)
 	return 1;
 }
 
+static const struct of_device_id tegra_irq_match[] __initconst = {
+	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init },
+	{ /* sentinel */ }
+};
+
 void __init tegra_init_irq(void)
 {
 	int i;

@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
 	gic_arch_extn.irq_unmask = tegra_unmask;
 	gic_arch_extn.irq_retrigger = tegra_retrigger;
 
-	gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-		 IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+	/* Check if there is a devicetree present as of_irq_init doesn't
+	 * indicate if an interrupt controller node was found.
+	 */
+	if (of_find_node_by_path("/"))
+		of_irq_init(tegra_irq_match);
+	else
+#endif
+		gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
+			IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
 }

Signature · 1 line

1.7.0.4

Cousson, Benoit

2011-11-29 13:00 UTC #1 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Hi Stephen & Peter,

Quoted 1 line · Stephen Warren

From: pdeschrijver at nvidia.com<[email protected]>

[...]

Quoted 4 lines

arch/arm/mach-tegra/irq.c +0 -0

@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
  	gic_arch_extn.irq_unmask = tegra_unmask;
  	gic_arch_extn.irq_retrigger = tegra_retrigger;

arch/arm/mach-tegra/irq.c void __init tegra_init_irq(void) · line 141

Quoted 10 lines

arch/arm/mach-tegra/irq.c +8 -2

-	gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-		 IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+	/* Check if there is a devicetree present as of_irq_init doesn't
+	 * indicate if an interrupt controller node was found.
+	 */
+	if (of_find_node_by_path("/"))
+		of_irq_init(tegra_irq_match);
+	else
+#endif

For the same kind of need, I found the following API:

of_have_populated_dt()

Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.

Regards,
Benoit

Stephen Warren

author

2011-11-29 17:13 UTC #2 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:

Quoted 24 lines

Hi Stephen & Peter,
Quoted 1 line · Stephen Warren
From: pdeschrijver at nvidia.com<[email protected]>
[...]
Quoted 4 lines
arch/arm/mach-tegra/irq.c +0 -0
@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
  	gic_arch_extn.irq_unmask = tegra_unmask;
  	gic_arch_extn.irq_retrigger = tegra_retrigger;
 
arch/arm/mach-tegra/irq.c void __init tegra_init_irq(void) · line 141
Quoted 10 lines
arch/arm/mach-tegra/irq.c +8 -2
-	gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-		 IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+	/* Check if there is a devicetree present as of_irq_init doesn't
+	 * indicate if an interrupt controller node was found.
+	 */
+	if (of_find_node_by_path("/"))
+		of_irq_init(tegra_irq_match);
+	else
+#endif
For the same kind of need, I found the following API:

Quoted 3 lines

of_have_populated_dt()
Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.

That sounds like a great idea. Unfortunately, of_irq_init() is a DT-only function. Exynos has the following in mainline to solve this:

        if (!of_have_populated_dt())
                gic_init_bases(...);
#ifdef CONFIG_OF
        else
                of_irq_init(exynos4_dt_irq_match);

#endif

Perhaps we should add a dummy of_irq_init() too, so that we can remove this ifdef.

Signature · 1 line

nvpublic

Nicolas Ferre

2011-11-29 17:17 UTC #3 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

On 11/29/2011 06:13 PM, Stephen Warren :

Quoted 37 lines

Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:
Quoted 24 lines
Hi Stephen& Peter,
Quoted 1 line · Stephen Warren
From: pdeschrijver at nvidia.com<[email protected]>
[...]
Quoted 4 lines
arch/arm/mach-tegra/irq.c +0 -0
@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
   	gic_arch_extn.irq_unmask = tegra_unmask;
   	gic_arch_extn.irq_retrigger = tegra_retrigger;
 
arch/arm/mach-tegra/irq.c void __init tegra_init_irq(void) · line 141
Quoted 10 lines
arch/arm/mach-tegra/irq.c +8 -2
-	gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-		 IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+	/* Check if there is a devicetree present as of_irq_init doesn't
+	 * indicate if an interrupt controller node was found.
+	 */
+	if (of_find_node_by_path("/"))
+		of_irq_init(tegra_irq_match);
+	else
+#endif
For the same kind of need, I found the following API:
Quoted 3 lines
of_have_populated_dt()
Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.
That sounds like a great idea. Unfortunately, of_irq_init() is a DT-only function. Exynos has the following in mainline to solve this:
         if (!of_have_populated_dt())
                 gic_init_bases(...);
#ifdef CONFIG_OF
         else
                 of_irq_init(exynos4_dt_irq_match);

Quoted 4 lines

#endif
Perhaps we should add a dummy of_irq_init() too, so that we can remove this ifdef.

+1 for this idea: I have the same issue with at91.
I think that a macro is preferred instead a function: it will also remove the need for a protection for the matching table.

Best regards,

Signature · 1 line

Nicolas Ferre

Cousson, Benoit

2011-11-29 17:47 UTC #4 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

In reply to Stephen Warren

Quoted 37 lines · Stephen Warren

Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:
Quoted 24 lines
Hi Stephen& Peter,
Quoted 1 line · Stephen Warren
From: pdeschrijver at nvidia.com<[email protected]>
[...]
Quoted 4 lines
arch/arm/mach-tegra/irq.c +0 -0
@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
   	gic_arch_extn.irq_unmask = tegra_unmask;
   	gic_arch_extn.irq_retrigger = tegra_retrigger;
 
arch/arm/mach-tegra/irq.c void __init tegra_init_irq(void) · line 141
Quoted 10 lines
arch/arm/mach-tegra/irq.c +8 -2
-	gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-		 IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+	/* Check if there is a devicetree present as of_irq_init doesn't
+	 * indicate if an interrupt controller node was found.
+	 */
+	if (of_find_node_by_path("/"))
+		of_irq_init(tegra_irq_match);
+	else
+#endif
For the same kind of need, I found the following API:
Quoted 3 lines
of_have_populated_dt()
Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.
That sounds like a great idea. Unfortunately, of_irq_init() is a DT-only function. Exynos has the following in mainline to solve this:
         if (!of_have_populated_dt())
                 gic_init_bases(...);
#ifdef CONFIG_OF
         else
                 of_irq_init(exynos4_dt_irq_match);

Quoted 4 lines

#endif
Perhaps we should add a dummy of_irq_init() too, so that we can remove this ifdef.

Yes, indeed, I think that's much better. Most of_XXX APIs do have a dummy version, but for some reason some of them do not. That one clearly deserve a dummy version.

Regards,
Benoit

Rob Herring

2011-11-29 19:52 UTC #5 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Quoted 40 lines · Benoit

Quoted 39 lines · Stephen Warren
Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:
Quoted 24 lines
Hi Stephen& Peter,
Quoted 1 line · Stephen Warren
From: pdeschrijver at nvidia.com<[email protected]>
[...]
Quoted 4 lines
arch/arm/mach-tegra/irq.c +0 -0
@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
       gic_arch_extn.irq_unmask = tegra_unmask;
       gic_arch_extn.irq_retrigger = tegra_retrigger;
 
arch/arm/mach-tegra/irq.c void __init tegra_init_irq(void) · line 141
Quoted 10 lines
arch/arm/mach-tegra/irq.c +8 -2
-    gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-         IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+    /* Check if there is a devicetree present as of_irq_init doesn't
+     * indicate if an interrupt controller node was found.
+     */
+    if (of_find_node_by_path("/"))
+        of_irq_init(tegra_irq_match);
+    else
+#endif
For the same kind of need, I found the following API:
Quoted 3 lines
of_have_populated_dt()
Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.
That sounds like a great idea. Unfortunately, of_irq_init() is a DT-only function. Exynos has the following in mainline to solve this:
         if (!of_have_populated_dt())
                 gic_init_bases(...);
#ifdef CONFIG_OF
         else
                 of_irq_init(exynos4_dt_irq_match);
#endif

Quoted 6 lines

Quoted 2 lines
Perhaps we should add a dummy of_irq_init() too, so that we can remove this ifdef.
Yes, indeed, I think that's much better. Most of_XXX APIs do have a dummy version, but for some reason some of them do not. That one clearly deserve a dummy version.

I had an empty version originally and removed it based on Grant's review...

The original intent was this call should be in a DT board file and therefore always enabled. Perhaps you can split tegra_init_irq into 2 functions with one having all but gic_init and then call that function from board-dt.c along with of_irq_init.

Rob

Stephen Warren

author

2011-11-29 21:08 UTC #6 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Rob Herring wrote at Tuesday, November 29, 2011 12:52 PM:

Quoted 48 lines · Benoit

Quoted 40 lines · Stephen Warren
Quoted 39 lines
Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:
Quoted 24 lines
Hi Stephen& Peter,
Quoted 1 line · Stephen Warren
From: pdeschrijver at nvidia.com<[email protected]>
[...]
Quoted 4 lines
arch/arm/mach-tegra/irq.c +0 -0
@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
       gic_arch_extn.irq_unmask = tegra_unmask;
       gic_arch_extn.irq_retrigger = tegra_retrigger;
 
arch/arm/mach-tegra/irq.c void __init tegra_init_irq(void) · line 141
Quoted 10 lines
arch/arm/mach-tegra/irq.c +8 -2
-    gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-         IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+    /* Check if there is a devicetree present as of_irq_init doesn't
+     * indicate if an interrupt controller node was found.
+     */
+    if (of_find_node_by_path("/"))
+        of_irq_init(tegra_irq_match);
+    else
+#endif
For the same kind of need, I found the following API:
Quoted 3 lines
of_have_populated_dt()
Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.
That sounds like a great idea. Unfortunately, of_irq_init() is a DT-only function. Exynos has the following in mainline to solve this:
         if (!of_have_populated_dt())
                 gic_init_bases(...);
#ifdef CONFIG_OF
         else
                 of_irq_init(exynos4_dt_irq_match);
#endif
Quoted 6 lines
Quoted 2 lines
Perhaps we should add a dummy of_irq_init() too, so that we can remove this ifdef.
Yes, indeed, I think that's much better. Most of_XXX APIs do have a dummy version, but for some reason some of them do not. That one clearly deserve a dummy version.

Quoted 6 lines

I had an empty version originally and removed it based on Grant's review...
The original intent was this call should be in a DT board file and therefore always enabled. Perhaps you can split tegra_init_irq into 2 functions with one having all but gic_init and then call that function from board-dt.c along with of_irq_init.

Well, that's certainly possible, but it seems a little inconsistent to initialize the GIC from tegra_init_irq for non-DT, yet do it somewhere else in the DT case. It seems like putting all the GIC stuff into tegra_irq_init is the right thing to do. I note that Exynos works this way already.

I couldn't find Grant's email to see his rationale for not wanting an empty or_irq_init(); do you have a link?

Thanks.

Signature · 1 line

nvpublic

Rob Herring

2011-11-29 22:24 UTC #7 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Quoted 56 lines · Stephen Warren

Rob Herring wrote at Tuesday, November 29, 2011 12:52 PM:
Quoted 48 lines · Benoit
Quoted 40 lines · Stephen Warren
Quoted 39 lines
Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:
Quoted 24 lines
Hi Stephen& Peter,
Quoted 1 line · Stephen Warren
From: pdeschrijver at nvidia.com<[email protected]>
[...]
Quoted 4 lines
arch/arm/mach-tegra/irq.c +0 -0
@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
       gic_arch_extn.irq_unmask = tegra_unmask;
       gic_arch_extn.irq_retrigger = tegra_retrigger;
 
arch/arm/mach-tegra/irq.c void __init tegra_init_irq(void) · line 141
Quoted 10 lines
arch/arm/mach-tegra/irq.c +8 -2
-    gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-         IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+    /* Check if there is a devicetree present as of_irq_init doesn't
+     * indicate if an interrupt controller node was found.
+     */
+    if (of_find_node_by_path("/"))
+        of_irq_init(tegra_irq_match);
+    else
+#endif
For the same kind of need, I found the following API:
Quoted 3 lines
of_have_populated_dt()
Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.
That sounds like a great idea. Unfortunately, of_irq_init() is a DT-only function. Exynos has the following in mainline to solve this:
         if (!of_have_populated_dt())
                 gic_init_bases(...);
#ifdef CONFIG_OF
         else
                 of_irq_init(exynos4_dt_irq_match);
#endif
Quoted 6 lines
Quoted 2 lines
Perhaps we should add a dummy of_irq_init() too, so that we can remove this ifdef.
Yes, indeed, I think that's much better. Most of_XXX APIs do have a dummy version, but for some reason some of them do not. That one clearly deserve a dummy version.
Quoted 6 lines
I had an empty version originally and removed it based on Grant's review...
The original intent was this call should be in a DT board file and therefore always enabled. Perhaps you can split tegra_init_irq into 2 functions with one having all but gic_init and then call that function from board-dt.c along with of_irq_init.

Quoted 9 lines

Well, that's certainly possible, but it seems a little inconsistent to initialize the GIC from tegra_init_irq for non-DT, yet do it somewhere else in the DT case. It seems like putting all the GIC stuff into tegra_irq_init is the right thing to do. I note that Exynos works this way already.
I couldn't find Grant's email to see his rationale for not wanting an empty or_irq_init(); do you have a link?

My brain is rotting... I think it was gic_of_init that I was thinking of but still can't find the email.

Will your code even link currently with !CONFIG_OF. I would think it cannot resolve gic_of_init in that case.

I have a lot of concerns that supporting both CONFIG_OF and !CONFIG_OF is going to be a pain as it's yet another variable to compile against. In order to actually start reducing reducing the size of the ARM platform code (that is the goal, right?), platforms need to be always OF enabled and start removing the !CONFIG_OF code. So why not always turn on CONFIG_OF for Tegra and not worry about this? Eventually, CONFIG_OF will always be enabled anyway.

Rob

Stephen Warren

author

2011-11-29 22:58 UTC #8 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Rob Herring wrote at Tuesday, November 29, 2011 3:25 PM:

Quoted 73 lines · Stephen Warren

Quoted 56 lines
Rob Herring wrote at Tuesday, November 29, 2011 12:52 PM:
Quoted 48 lines · Benoit
Quoted 40 lines · Stephen Warren
Quoted 39 lines
Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:
Quoted 24 lines
Hi Stephen& Peter,
Quoted 1 line · Stephen Warren
From: pdeschrijver at nvidia.com<[email protected]>
[...]
Quoted 4 lines
(quoted diff) +0 -0
@@ -125,6 +131,14 @@ void __init tegra_init_irq(void)
       gic_arch_extn.irq_unmask = tegra_unmask;
       gic_arch_extn.irq_retrigger = tegra_retrigger;
 
Review void __init tegra_init_irq(void) · line 141
Quoted 10 lines
(quoted diff) +8 -2
-    gic_init(0, 29, IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE),
-         IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
+#ifdef CONFIG_OF
+    /* Check if there is a devicetree present as of_irq_init doesn't
+     * indicate if an interrupt controller node was found.
+     */
+    if (of_find_node_by_path("/"))
+        of_irq_init(tegra_irq_match);
+    else
+#endif
For the same kind of need, I found the following API:
Quoted 3 lines
of_have_populated_dt()
Moreover, it returns false if !CONFIG_OF, so it will avoid the #ifdef.
That sounds like a great idea. Unfortunately, of_irq_init() is a DT-only function. Exynos has the following in mainline to solve this:
         if (!of_have_populated_dt())
                 gic_init_bases(...);
#ifdef CONFIG_OF
         else
                 of_irq_init(exynos4_dt_irq_match);
#endif
Quoted 6 lines
Quoted 2 lines
Perhaps we should add a dummy of_irq_init() too, so that we can remove this ifdef.
Yes, indeed, I think that's much better. Most of_XXX APIs do have a dummy version, but for some reason some of them do not. That one clearly deserve a dummy version.
Quoted 6 lines
I had an empty version originally and removed it based on Grant's review...
The original intent was this call should be in a DT board file and therefore always enabled. Perhaps you can split tegra_init_irq into 2 functions with one having all but gic_init and then call that function from board-dt.c along with of_irq_init.
Quoted 9 lines
Well, that's certainly possible, but it seems a little inconsistent to initialize the GIC from tegra_init_irq for non-DT, yet do it somewhere else in the DT case. It seems like putting all the GIC stuff into tegra_irq_init is the right thing to do. I note that Exynos works this way already.
I couldn't find Grant's email to see his rationale for not wanting an empty or_irq_init(); do you have a link?
My brain is rotting... I think it was gic_of_init that I was thinking of but still can't find the email.
Will your code even link currently with !CONFIG_OF. I would think it cannot resolve gic_of_init in that case.

Quoted 7 lines

I have a lot of concerns that supporting both CONFIG_OF and !CONFIG_OF is going to be a pain as it's yet another variable to compile against. In order to actually start reducing reducing the size of the ARM platform code (that is the goal, right?), platforms need to be always OF enabled and start removing the !CONFIG_OF code. So why not always turn on CONFIG_OF for Tegra and not worry about this? Eventually, CONFIG_OF will always be enabled anyway.

I've been looking at hooking up the Tegra GPIO controller's IRQ support through device tree. It looks like I need to call irq_domain_add_simple() for the GPIO node to make this work, and I think this needs to be triggered by of_irq_init()'s match table, and hence I need a single of_irq_init() call in Tegra's board-dt.c, which includes both the GIC and GPIO controller entries.

Is my understanding here all correct? If so, I will just follow your initial suggestion and have tegra_init_irq() do just:

if (!of_have_populated_dt())

        gic_init_bases(...);

... and move the of_irq_init call into board-dt.c.

Thanks.

Signature · 1 line

nvpublic

Rob Herring

2011-11-29 23:16 UTC #9 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Quoted 2 lines · Stephen Warren

Rob Herring wrote at Tuesday, November 29, 2011 3:25 PM:
Quoted 1 line · Stephen Warren

snip

Quoted 21 lines

Quoted 6 lines
My brain is rotting... I think it was gic_of_init that I was thinking of but still can't find the email.
Will your code even link currently with !CONFIG_OF. I would think it cannot resolve gic_of_init in that case.
Quoted 7 lines
I have a lot of concerns that supporting both CONFIG_OF and !CONFIG_OF is going to be a pain as it's yet another variable to compile against. In order to actually start reducing reducing the size of the ARM platform code (that is the goal, right?), platforms need to be always OF enabled and start removing the !CONFIG_OF code. So why not always turn on CONFIG_OF for Tegra and not worry about this? Eventually, CONFIG_OF will always be enabled anyway.
I've been looking at hooking up the Tegra GPIO controller's IRQ support through device tree. It looks like I need to call irq_domain_add_simple() for the GPIO node to make this work, and I think this needs to be triggered by of_irq_init()'s match table, and hence I need a single of_irq_init() call in Tegra's board-dt.c, which includes both the GIC and GPIO controller entries.

Quoted 2 lines

Is my understanding here all correct? If so, I will just follow your initial suggestion and have tegra_init_irq() do just:

Yes, you should have 1 of_irq_init call. Really the irqdomain code should go into the gpio code. Do you use generic irqchip for gpio? If so, I'm working on a patch adding irqdomain to generic irqchip. I still need to debug a problem with it and have been too busy to get back to it.

Rob

Quoted 7 lines

if (!of_have_populated_dt())
        gic_init_bases(...);
... and move the of_irq_init call into board-dt.c.
Thanks.

Stephen Warren

author

2011-11-29 23:22 UTC #10 lore raw

PATCH v2 1/1 arm/tegra: convert tegra20 to GIC devicetree binding

Rob Herring wrote at Tuesday, November 29, 2011 4:17 PM:

Quoted 1 line · Stephen Warren

...

Quoted 7 lines

Quoted 7 lines
I've been looking at hooking up the Tegra GPIO controller's IRQ support through device tree. It looks like I need to call irq_domain_add_simple() for the GPIO node to make this work, and I think this needs to be triggered by of_irq_init()'s match table, and hence I need a single of_irq_init() call in Tegra's board-dt.c, which includes both the GIC and GPIO controller entries.

Quoted 5 lines

Quoted 2 lines
Is my understanding here all correct? If so, I will just follow your initial suggestion and have tegra_init_irq() do just:
Yes, you should have 1 of_irq_init call. Really the irqdomain code should go into the gpio code.

It sounds like you're saying:

* The of_irq_init() call doesn't have to include all the IRQ controllers in the system.
* So, it's fine for the Tegra GPIO controller's probe() function to simply call irq_domain_add_simple() for itself.

Is that correct? If so, I'll gladly add the call to the GPIO controller and leave just the GIC to be handled via of_irq_init().

The only issue here is the usual device ordering issue; if the Tegra GPIO controller doesn't probe before devices that use it, then the IRQ domain won't exist. But, this is true for the interrupts themselves, so nothing new there.

Quoted 1 line

Do you use generic irqchip for gpio?

No, it's custom code. Some of the code could probably use the generic code, but there is some additional functionality beyond that.

Quoted 3 lines

If
so, I'm working on a patch adding irqdomain to generic irqchip. I still need to debug a problem with it and have been too busy to get back to it.

Signature · 1 line

nvpublic