RE: [PATCH V2] arm/tegra: convert tegra20 to GIC devicetree binding

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

On 11/29/2011 11:47 AM, Cousson, Benoit wrote:

quoted

On 11/29/2011 6:13 PM, Stephen Warren wrote:

quoted

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

quoted

Hi Stephen&  Peter,

On 11/29/2011 1:54 AM, Stephen Warren wrote:

quoted

From: pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org<redacted>

[...]

quoted

@@ -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

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.

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.

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.

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.

-- 
nvpublic