Rob Herring wrote at Tuesday, November 29, 2011 12:52 PM:
> On 11/29/2011 11:47 AM, Cousson, Benoit wrote:
> > On 11/29/2011 6:13 PM, Stephen Warren wrote:
> >> Cousson, Benoit wrote at Tuesday, November 29, 2011 6:01 AM:
> >>> Hi Stephen& Peter,
> >>>
> >>> On 11/29/2011 1:54 AM, Stephen Warren wrote:
> >>>> From: pdeschrijver@xxxxxxxxxx<pdeschrijver@xxxxxxxxxx>
> >>>
> >>> [...]
> >>>
> >>>> @@ -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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
