Patchwork [v2,3/3] ARM: tegra: Unify Device tree board files

login
register
mail settings
Submitter Hiroshi Doyu
Date Feb. 11, 2013, 6:05 a.m.
Message ID <1360562743-21689-3-git-send-email-hdoyu@nvidia.com>
Download mbox | patch
Permalink /patch/219537/
State Changes Requested, archived
Headers show

Comments

Hiroshi Doyu - Feb. 11, 2013, 6:05 a.m.
Unify board-dt-tegra{30,114} to the Tegra20 DT board file, "tegra.c".

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/Makefile            |    5 ++-
 arch/arm/mach-tegra/board-dt-tegra114.c |   46 ------------------------
 arch/arm/mach-tegra/board-dt-tegra30.c  |   60 -------------------------------
 arch/arm/mach-tegra/tegra.c             |   24 +++++++------
 4 files changed, 16 insertions(+), 119 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/board-dt-tegra114.c
 delete mode 100644 arch/arm/mach-tegra/board-dt-tegra30.c
Stephen Warren - Feb. 11, 2013, 11:54 p.m.
On 02/10/2013 11:05 PM, Hiroshi Doyu wrote:
> Unify board-dt-tegra{30,114} to the Tegra20 DT board file, "tegra.c".

> diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile

>  obj-$(CONFIG_CPU_FREQ)                  += cpu-tegra.o
>  obj-$(CONFIG_TEGRA_PCI)			+= pcie.o
>  
> -obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra.o
> -obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
> -obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= board-dt-tegra114.o
> +obj-y					+= tegra.o
> +

I think I'd be tempted to move that line together with all the others
that don't depend on some config option.

> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c

> -static struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
> +static struct of_dev_auxdata tegra_auxdata_lookup[] __initdata = {
> +#ifdef CONFIG_ARCH_TEGRA_2x_SOC
>  	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5000000, "tegra-ehci.0",
>  		       &tegra_ehci1_pdata),
>  	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5004000, "tegra-ehci.1",
>  		       &tegra_ehci2_pdata),
>  	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5008000, "tegra-ehci.2",
>  		       &tegra_ehci3_pdata),
> +#endif
>  	{}
>  };

Why ifdef? This is certainly small enough that it shouldn't matter for
any Tegra30- or Tegra114-kernel, and it's hopefully going away very
soon, so I'd prefer to drop the addition of the ifdefs to avoid any
conflicts with any other changes to that table.

>  static void __init trimslice_init(void)
>  {
> -#ifdef CONFIG_TEGRA_PCI
>  	int ret;
>  
>  	ret = tegra_pcie_init(true, true);
>  	if (ret)
>  		pr_err("tegra_pci_init() failed: %d\n", ret);
> -#endif
>  }
>  
>  static void __init harmony_init(void)
>  {
> -#ifdef CONFIG_TEGRA_PCI
>  	int ret;
>  
>  	ret = harmony_pcie_init();
>  	if (ret)
>  		pr_err("harmony_pcie_init() failed: %d\n", ret);
> -#endif
>  }

Why drop those ifdefs? Does the code still compile (link) if built for
Tegra30-only or Tegra114-only, where the Tegra PCI driver won't be
enabled, and hence those functions don't exist?

>  static void __init paz00_init(void)
> @@ -129,6 +128,9 @@ static void __init tegra_dt_init_late(void)
>  
>  	tegra_init_late();
>  
> +	if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> +		return;

I don't think that's going to help any link issues, so I'd drop it and
keep this function simple. After all, what if someone wants to add some
non-PCI-related entry into board_init_funcs[]?

> -static const char *tegra20_dt_board_compat[] = {
> +static const char * const tegra_dt_board_compat[] = {
>  	"nvidia,tegra20",
> +	"nvidia,tegra30",
> +	"nvidia,tegra114",
>  	NULL
>  };

It'd be best to add the newer values first. It shouldn't matter, but
currently does at least for device compatible matching, so we may as
well be consistent here.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hiroshi Doyu - Feb. 12, 2013, 4:12 a.m.
Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 12 Feb 2013 00:54:03 +0100:

> > -obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra.o
> > -obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
> > -obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= board-dt-tegra114.o
> > +obj-y					+= tegra.o
> > +
> 
> I think I'd be tempted to move that line together with all the others
> that don't depend on some config option.

In arch/arm/mach-tegra/Makefile, we have:

obj-y                                   += common.o
obj-y                                   += io.o
obj-y                                   += irq.o
obj-y					+= fuse.o
obj-y					+= pmc.o
.....

Should we have a single entry for them as well?

obj-y:= common.o io.o irq.o fuse.o pmc.o flowctrl.o powergate.o apbio.o pm.o \
	reset.o reset-handler.o sleep.o tegra.o

I think that this moval could be done another patch.

> >  static void __init harmony_init(void)
> >  {
> > -#ifdef CONFIG_TEGRA_PCI
> >  	int ret;
> >  
> >  	ret = harmony_pcie_init();
> >  	if (ret)
> >  		pr_err("harmony_pcie_init() failed: %d\n", ret);
> > -#endif
> >  }
> 
> Why drop those ifdefs? Does the code still compile (link) if built for
> Tegra30-only or Tegra114-only, where the Tegra PCI driver won't be
> enabled, and hence those functions don't exist?

This function itself will be dropped by the following IS_ENABLED().

> >  static void __init paz00_init(void)
> > @@ -129,6 +128,9 @@ static void __init tegra_dt_init_late(void)
> >  
> >  	tegra_init_late();
> >  
> > +	if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> > +		return;
> 
> I don't think that's going to help any link issues, so I'd drop it and
> keep this function simple.

As explained in the above, a complier will drop unnecessary functions
automatically with this IS_ENABLED(), which could save many ifdefs.

> After all, what if someone wants to add some
> non-PCI-related entry into board_init_funcs[]?

I originally thought that one should try to solve those board specific
problems with DT basically and this tegra specific board_init_funcs[]
was left only for the legacy support during DT transition.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Feb. 12, 2013, 4:47 a.m.
On 02/11/2013 09:12 PM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 12 Feb 2013 00:54:03 +0100:
> 
>>> -obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra.o
>>> -obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
>>> -obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= board-dt-tegra114.o
>>> +obj-y					+= tegra.o
>>> +
>>
>> I think I'd be tempted to move that line together with all the others
>> that don't depend on some config option.
> 
> In arch/arm/mach-tegra/Makefile, we have:
> 
> obj-y                                   += common.o
> obj-y                                   += io.o
> obj-y                                   += irq.o
> obj-y					+= fuse.o
> obj-y					+= pmc.o
> .....
> 
> Should we have a single entry for them as well?
> 
> obj-y:= common.o io.o irq.o fuse.o pmc.o flowctrl.o powergate.o apbio.o pm.o \
> 	reset.o reset-handler.o sleep.o tegra.o
> 
> I think that this moval could be done another patch.

I just meant put the lines next to each-other. We definitely shouldn't
merge the lines together or it'll make it more painful to change the
list of files later.

>>>  static void __init harmony_init(void)
>>>  {
>>> -#ifdef CONFIG_TEGRA_PCI
>>>  	int ret;
>>>  
>>>  	ret = harmony_pcie_init();
>>>  	if (ret)
>>>  		pr_err("harmony_pcie_init() failed: %d\n", ret);
>>> -#endif
>>>  }
>>
>> Why drop those ifdefs? Does the code still compile (link) if built for
>> Tegra30-only or Tegra114-only, where the Tegra PCI driver won't be
>> enabled, and hence those functions don't exist?
> 
> This function itself will be dropped by the following IS_ENABLED().
> 
>>>  static void __init paz00_init(void)
>>> @@ -129,6 +128,9 @@ static void __init tegra_dt_init_late(void)
>>>  
>>>  	tegra_init_late();
>>>  
>>> +	if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
>>> +		return;
>>
>> I don't think that's going to help any link issues, so I'd drop it and
>> keep this function simple.
> 
> As explained in the above, a complier will drop unnecessary functions
> automatically with this IS_ENABLED(), which could save many ifdefs.

That sounds extremely brittle. Have you validated this on a wide variety
of compiler versions?

>> After all, what if someone wants to add some
>> non-PCI-related entry into board_init_funcs[]?
> 
> I originally thought that one should try to solve those board specific
> problems with DT basically and this tegra specific board_init_funcs[]
> was left only for the legacy support during DT transition.

That's the intent right now, but who knows what else might end up
getting added there. It seems simplest just to maintain the ifdefs since
they're already there.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hiroshi Doyu - Feb. 12, 2013, 5:04 a.m.
Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 12 Feb 2013 05:47:20 +0100:

> >>>  static void __init harmony_init(void)
> >>>  {
> >>> -#ifdef CONFIG_TEGRA_PCI
> >>>  	int ret;
> >>>  
> >>>  	ret = harmony_pcie_init();
> >>>  	if (ret)
> >>>  		pr_err("harmony_pcie_init() failed: %d\n", ret);
> >>> -#endif
> >>>  }
> >>
> >> Why drop those ifdefs? Does the code still compile (link) if built for
> >> Tegra30-only or Tegra114-only, where the Tegra PCI driver won't be
> >> enabled, and hence those functions don't exist?
> > 
> > This function itself will be dropped by the following IS_ENABLED().
> > 
> >>>  static void __init paz00_init(void)
> >>> @@ -129,6 +128,9 @@ static void __init tegra_dt_init_late(void)
> >>>  
> >>>  	tegra_init_late();
> >>>  
> >>> +	if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> >>> +		return;
> >>
> >> I don't think that's going to help any link issues, so I'd drop it and
> >> keep this function simple.
> > 
> > As explained in the above, a complier will drop unnecessary functions
> > automatically with this IS_ENABLED(), which could save many ifdefs.
> 
> That sounds extremely brittle. Have you validated this on a wide variety
> of compiler versions?

I verified with gcc-4.6.
IIRC, that's the point of IS_ENABLED() being introduced. Arnd?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann - Feb. 12, 2013, 1:50 p.m.
On Tuesday 12 February 2013, Hiroshi Doyu wrote:
> > >>>  static void __init paz00_init(void)
> > >>> @@ -129,6 +128,9 @@ static void __init tegra_dt_init_late(void)
> > >>>  
> > >>>   tegra_init_late();
> > >>>  
> > >>> + if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> > >>> +         return;
> > >>
> > >> I don't think that's going to help any link issues, so I'd drop it and
> > >> keep this function simple.
> > > 
> > > As explained in the above, a complier will drop unnecessary functions
> > > automatically with this IS_ENABLED(), which could save many ifdefs.
> > 
> > That sounds extremely brittle. Have you validated this on a wide variety
> > of compiler versions?
> 
> I verified with gcc-4.6.
> IIRC, that's the point of IS_ENABLED() being introduced. Arnd?

It's certainly expected to work with all compilers, yes. If a compiler
cannot remove conditional function calls that depend on a constant
expression, we have a lot of other problems alredy.

However, from what I see above, you have the logic reversed (it
should return if neither TEGRA_2x nor PCI are enabled, rather
than return if one of them is enabled). and it becomes a little
confusing to read.

If you want to get rid of the #ifdef here, I would suggest you put those
into the functions directly, like

static void __init harmony_init(void)
{
        int ret = 0;

        if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
		ret = harmony_pcie_init();
        if (ret)
                pr_err("harmony_pcie_init() failed: %d\n", ret);
}

which will turn into an empty function if one of the two is disabled.

Since we are not going to add any other board specfic init functions, you
can also unroll the loop and put everything into tegra_dt_init_late:

/* Board specific fixups, try not add any new ones here */
static void __init tegra_dt_init_late(void)
{
	tegra_powergate_debugfs_init();

	/* so far, these all apply only to tegra2x */
	if (!IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
		return;

	if (of_machine_is_compatible("compal,paz00"))
		tegra_paz00_wifikill_init();
	if (IS_ENABLED(CONFIG_PCI) && of_machine_is_compatible("nvidia,harmony")
		harmony_pcie_init();
	if (IS_ENABLED(CONFIG_PCI) && of_machine_is_compatible("compulab,trimslice")
		tegra_pcie_init(true, true);
}

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Feb. 12, 2013, 4:35 p.m.
On 02/12/2013 06:50 AM, Arnd Bergmann wrote:
> On Tuesday 12 February 2013, Hiroshi Doyu wrote:
>>>>>>  static void __init paz00_init(void)
>>>>>> @@ -129,6 +128,9 @@ static void __init tegra_dt_init_late(void)
>>>>>>  
>>>>>>   tegra_init_late();
>>>>>>  
>>>>>> + if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
>>>>>> +         return;
>>>>>
>>>>> I don't think that's going to help any link issues, so I'd drop it and
>>>>> keep this function simple.
>>>>
>>>> As explained in the above, a complier will drop unnecessary functions
>>>> automatically with this IS_ENABLED(), which could save many ifdefs.
>>>
>>> That sounds extremely brittle. Have you validated this on a wide variety
>>> of compiler versions?
>>
>> I verified with gcc-4.6.
>> IIRC, that's the point of IS_ENABLED() being introduced. Arnd?
> 
> It's certainly expected to work with all compilers, yes. If a compiler
> cannot remove conditional function calls that depend on a constant
> expression, we have a lot of other problems alredy.

Removing the function calls isn't guaranteed to remove the body of the
functions though. Those functions aren't implemented in some separate
file that's optionally included, but rather are implemented in the same
object file, now unconditionally, and they in turn reference (with no
IS_ENABLED logic) other functions that are implemented in conditionally
linked files.

> However, from what I see above, you have the logic reversed (it
> should return if neither TEGRA_2x nor PCI are enabled, rather
> than return if one of them is enabled). and it becomes a little
> confusing to read.
> 
> If you want to get rid of the #ifdef here, I would suggest you put those
> into the functions directly, like
> 
> static void __init harmony_init(void)
> {
>         int ret = 0;
> 
>         if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> 		ret = harmony_pcie_init();
>         if (ret)
>                 pr_err("harmony_pcie_init() failed: %d\n", ret);
> }
> 
> which will turn into an empty function if one of the two is disabled.

That would work.

However I'd like to avoid changing the body of those two functions at
all if possible, since I hope the PCIe driver rework will be merged in
3.10, and that will allow the Harmony and TrimSlice init functions to be
removed entirely. I'd rather not have conflicts with the removal patch.

> Since we are not going to add any other board specfic init functions, you
> can also unroll the loop and put everything into tegra_dt_init_late:

That's not necessarily true. While we certainly don't plan to, I don't
think we can rule it out; after all, we don't have rfkill bindings and
yet other boards will need them.

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann - Feb. 12, 2013, 5:32 p.m.
On Tuesday 12 February 2013, Stephen Warren wrote:
> >>>>> I don't think that's going to help any link issues, so I'd drop it and
> >>>>> keep this function simple.
> >>>>
> >>>> As explained in the above, a complier will drop unnecessary functions
> >>>> automatically with this IS_ENABLED(), which could save many ifdefs.
> >>>
> >>> That sounds extremely brittle. Have you validated this on a wide variety
> >>> of compiler versions?
> >>
> >> I verified with gcc-4.6.
> >> IIRC, that's the point of IS_ENABLED() being introduced. Arnd?
> > 
> > It's certainly expected to work with all compilers, yes. If a compiler
> > cannot remove conditional function calls that depend on a constant
> > expression, we have a lot of other problems alredy.
> 
> Removing the function calls isn't guaranteed to remove the body of the
> functions though. Those functions aren't implemented in some separate
> file that's optionally included, but rather are implemented in the same
> object file, now unconditionally, and they in turn reference (with no
> IS_ENABLED logic) other functions that are implemented in conditionally
> linked files.

I think gcc should remove all of that in this case, since board_init_funcs
becomes unused when the only code that references it cannot be reached,
and when that array is gone, the now unused functions would be removed
as well.

We have had bugs with prerelease gcc versions that did not handle cases
like this in the way we want it, but I think all releases get it right.
I don't think it's mandated anywhere in the C99 standard that this is
what happens, but we do rely on it anyway AFAIK.

> > static void __init harmony_init(void)
> > {
> >         int ret = 0;
> > 
> >         if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> >               ret = harmony_pcie_init();
> >         if (ret)
> >                 pr_err("harmony_pcie_init() failed: %d\n", ret);
> > }
> > 
> > which will turn into an empty function if one of the two is disabled.
> 
> That would work.
> 
> However I'd like to avoid changing the body of those two functions at
> all if possible, since I hope the PCIe driver rework will be merged in
> 3.10, and that will allow the Harmony and TrimSlice init functions to be
> removed entirely. I'd rather not have conflicts with the removal patch.

Yes, makes sense.

> > Since we are not going to add any other board specfic init functions, you
> > can also unroll the loop and put everything into tegra_dt_init_late:
> 
> That's not necessarily true. While we certainly don't plan to, I don't
> think we can rule it out; after all, we don't have rfkill bindings and
> yet other boards will need them.

Ok.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Feb. 12, 2013, 8:52 p.m.
On 02/12/2013 10:32 AM, Arnd Bergmann wrote:
> On Tuesday 12 February 2013, Stephen Warren wrote:
>>>>>>> I don't think that's going to help any link issues, so I'd drop it and
>>>>>>> keep this function simple.
>>>>>>
>>>>>> As explained in the above, a complier will drop unnecessary functions
>>>>>> automatically with this IS_ENABLED(), which could save many ifdefs.
>>>>>
>>>>> That sounds extremely brittle. Have you validated this on a wide variety
>>>>> of compiler versions?
>>>>
>>>> I verified with gcc-4.6.
>>>> IIRC, that's the point of IS_ENABLED() being introduced. Arnd?
>>>
>>> It's certainly expected to work with all compilers, yes. If a compiler
>>> cannot remove conditional function calls that depend on a constant
>>> expression, we have a lot of other problems alredy.
>>
>> Removing the function calls isn't guaranteed to remove the body of the
>> functions though. Those functions aren't implemented in some separate
>> file that's optionally included, but rather are implemented in the same
>> object file, now unconditionally, and they in turn reference (with no
>> IS_ENABLED logic) other functions that are implemented in conditionally
>> linked files.
> 
> I think gcc should remove all of that in this case, since board_init_funcs
> becomes unused when the only code that references it cannot be reached,
> and when that array is gone, the now unused functions would be removed
> as well.
> 
> We have had bugs with prerelease gcc versions that did not handle cases
> like this in the way we want it, but I think all releases get it right.
> I don't think it's mandated anywhere in the C99 standard that this is
> what happens, but we do rely on it anyway AFAIK.

Hmmm. Very surprisingly (to me), this does indeed work fine, even with
an older gcc-4.4.1 I had lying around (after fixing the test inversion
in tegra_dt_init_late).

I believe U-Boot enabled -ffunction-sections -fdata-sections or similar
(recently?) to get this kind of behaviour. I wonder why the kernel
didn't need that. Perhaps -O2 is more aggressive (within a file at
least) than I thought.

I did note a few warnings due to the ifdefs in tegra_auxdata_lookup[]:

arch/arm/mach-tegra/tegra.c:47: warning: 'tegra_ehci1_pdata' defined but
not used
arch/arm/mach-tegra/tegra.c:58: warning: 'tegra_ehci2_pdata' defined but
not used
arch/arm/mach-tegra/tegra.c:65: warning: 'tegra_ehci3_pdata' defined but
not used

(I built a tegra_defconfig kernel and modified it to enable
Tegra30+Tegra114, disable Tegra20, disable DRM)
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann - Feb. 12, 2013, 10:25 p.m.
On Tuesday 12 February 2013, Stephen Warren wrote:
> I believe U-Boot enabled -ffunction-sections -fdata-sections or similar
> (recently?) to get this kind of behaviour. I wonder why the kernel
> didn't need that. Perhaps -O2 is more aggressive (within a file at
> least) than I thought.

-ffunction-sections works across files, while the trick I described
only works on file static symbols. David Woodhouse at some point
had a working kernel build with -ffunction-sections but for some
reason that never got merged.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index c6d6be2..ee866337 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -27,9 +27,8 @@  obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
 obj-$(CONFIG_CPU_FREQ)                  += cpu-tegra.o
 obj-$(CONFIG_TEGRA_PCI)			+= pcie.o
 
-obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra.o
-obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
-obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= board-dt-tegra114.o
+obj-y					+= tegra.o
+
 ifeq ($(CONFIG_CPU_IDLE),y)
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= cpuidle-tegra114.o
 endif
diff --git a/arch/arm/mach-tegra/board-dt-tegra114.c b/arch/arm/mach-tegra/board-dt-tegra114.c
deleted file mode 100644
index 08e8294..0000000
--- a/arch/arm/mach-tegra/board-dt-tegra114.c
+++ /dev/null
@@ -1,46 +0,0 @@ 
-/*
- * NVIDIA Tegra114 device tree board support
- *
- * Copyright (C) 2013 NVIDIA Corporation
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/clocksource.h>
-
-#include <asm/mach/arch.h>
-
-#include "board.h"
-#include "common.h"
-
-static void __init tegra114_dt_init(void)
-{
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-}
-
-static const char * const tegra114_dt_board_compat[] = {
-	"nvidia,tegra114",
-	NULL,
-};
-
-DT_MACHINE_START(TEGRA114_DT, "NVIDIA Tegra114 (Flattened Device Tree)")
-	.smp		= smp_ops(tegra_smp_ops),
-	.map_io		= tegra_map_common_io,
-	.init_early	= tegra_init_early,
-	.init_irq	= tegra_dt_init_irq,
-	.init_time	= clocksource_of_init,
-	.init_machine	= tegra114_dt_init,
-	.init_late	= tegra_init_late,
-	.restart	= tegra_assert_system_reset,
-	.dt_compat	= tegra114_dt_board_compat,
-MACHINE_END
diff --git a/arch/arm/mach-tegra/board-dt-tegra30.c b/arch/arm/mach-tegra/board-dt-tegra30.c
deleted file mode 100644
index 63f8139..0000000
--- a/arch/arm/mach-tegra/board-dt-tegra30.c
+++ /dev/null
@@ -1,60 +0,0 @@ 
-/*
- * arch/arm/mach-tegra/board-dt-tegra30.c
- *
- * NVIDIA Tegra30 device tree board support
- *
- * Copyright (C) 2011, 2013, NVIDIA Corporation
- *
- * Derived from:
- *
- * arch/arm/mach-tegra/board-dt-tegra20.c
- *
- * Copyright (C) 2010 Secret Lab Technologies, Ltd.
- * Copyright (C) 2010 Google, Inc.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#include <linux/clocksource.h>
-#include <linux/kernel.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/of_fdt.h>
-#include <linux/of_irq.h>
-#include <linux/of_platform.h>
-
-#include <asm/mach/arch.h>
-
-#include "board.h"
-#include "common.h"
-#include "iomap.h"
-
-static void __init tegra30_dt_init(void)
-{
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-}
-
-static const char *tegra30_dt_board_compat[] = {
-	"nvidia,tegra30",
-	NULL
-};
-
-DT_MACHINE_START(TEGRA30_DT, "NVIDIA Tegra30 (Flattened Device Tree)")
-	.smp		= smp_ops(tegra_smp_ops),
-	.map_io		= tegra_map_common_io,
-	.init_early	= tegra_init_early,
-	.init_irq	= tegra_dt_init_irq,
-	.init_time	= clocksource_of_init,
-	.init_machine	= tegra30_dt_init,
-	.init_late	= tegra_init_late,
-	.restart	= tegra_assert_system_reset,
-	.dt_compat	= tegra30_dt_board_compat,
-MACHINE_END
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index fca18e9..00debef 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -1,6 +1,7 @@ 
 /*
- * nVidia Tegra device tree board support
+ * NVIDIA Tegra SoC device tree board support
  *
+ * Copyright (C) 2011, 2013, NVIDIA Corporation
  * Copyright (C) 2010 Secret Lab Technologies, Ltd.
  * Copyright (C) 2010 Google, Inc.
  *
@@ -67,13 +68,15 @@  static struct tegra_ehci_platform_data tegra_ehci3_pdata = {
 	.vbus_gpio = -1,
 };
 
-static struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
+static struct of_dev_auxdata tegra_auxdata_lookup[] __initdata = {
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
 	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5000000, "tegra-ehci.0",
 		       &tegra_ehci1_pdata),
 	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5004000, "tegra-ehci.1",
 		       &tegra_ehci2_pdata),
 	OF_DEV_AUXDATA("nvidia,tegra20-ehci", 0xC5008000, "tegra-ehci.2",
 		       &tegra_ehci3_pdata),
+#endif
 	{}
 };
 
@@ -84,29 +87,25 @@  static void __init tegra_dt_init(void)
 	 * devices
 	 */
 	of_platform_populate(NULL, of_default_bus_match_table,
-				tegra20_auxdata_lookup, NULL);
+				tegra_auxdata_lookup, NULL);
 }
 
 static void __init trimslice_init(void)
 {
-#ifdef CONFIG_TEGRA_PCI
 	int ret;
 
 	ret = tegra_pcie_init(true, true);
 	if (ret)
 		pr_err("tegra_pci_init() failed: %d\n", ret);
-#endif
 }
 
 static void __init harmony_init(void)
 {
-#ifdef CONFIG_TEGRA_PCI
 	int ret;
 
 	ret = harmony_pcie_init();
 	if (ret)
 		pr_err("harmony_pcie_init() failed: %d\n", ret);
-#endif
 }
 
 static void __init paz00_init(void)
@@ -129,6 +128,9 @@  static void __init tegra_dt_init_late(void)
 
 	tegra_init_late();
 
+	if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
+		return;
+
 	for (i = 0; i < ARRAY_SIZE(board_init_funcs); i++) {
 		if (of_machine_is_compatible(board_init_funcs[i].machine)) {
 			board_init_funcs[i].init();
@@ -137,12 +139,14 @@  static void __init tegra_dt_init_late(void)
 	}
 }
 
-static const char *tegra20_dt_board_compat[] = {
+static const char * const tegra_dt_board_compat[] = {
 	"nvidia,tegra20",
+	"nvidia,tegra30",
+	"nvidia,tegra114",
 	NULL
 };
 
-DT_MACHINE_START(TEGRA_DT, "nVidia Tegra20 (Flattened Device Tree)")
+DT_MACHINE_START(TEGRA_DT, "NVIDIA Tegra SoC (Flattened Device Tree)")
 	.map_io		= tegra_map_common_io,
 	.smp		= smp_ops(tegra_smp_ops),
 	.init_early	= tegra_init_early,
@@ -151,5 +155,5 @@  DT_MACHINE_START(TEGRA_DT, "nVidia Tegra20 (Flattened Device Tree)")
 	.init_machine	= tegra_dt_init,
 	.init_late	= tegra_dt_init_late,
 	.restart	= tegra_assert_system_reset,
-	.dt_compat	= tegra20_dt_board_compat,
+	.dt_compat	= tegra_dt_board_compat,
 MACHINE_END