Rob Herring wrote at Tuesday, November 29, 2011 3:25 PM:
> On 11/29/2011 03:08 PM, Stephen Warren wrote:
> > 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?
> >
>
> 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.
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.
--
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
