| Message ID | 1360562743-21689-1-git-send-email-hdoyu@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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.
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.
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?
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
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.
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
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)
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
diff --git a/arch/arm/mach-tegra/board-dt-tegra114.c b/arch/arm/mach-tegra/board-dt-tegra114.c index 085d636..08e8294 100644 --- a/arch/arm/mach-tegra/board-dt-tegra114.c +++ b/arch/arm/mach-tegra/board-dt-tegra114.c @@ -36,7 +36,7 @@ static const char * const tegra114_dt_board_compat[] = { DT_MACHINE_START(TEGRA114_DT, "NVIDIA Tegra114 (Flattened Device Tree)") .smp = smp_ops(tegra_smp_ops), .map_io = tegra_map_common_io, - .init_early = tegra114_init_early, + .init_early = tegra_init_early, .init_irq = tegra_dt_init_irq, .init_time = clocksource_of_init, .init_machine = tegra114_dt_init, diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c index a0edf25..fca18e9 100644 --- a/arch/arm/mach-tegra/board-dt-tegra20.c +++ b/arch/arm/mach-tegra/board-dt-tegra20.c @@ -145,7 +145,7 @@ static const char *tegra20_dt_board_compat[] = { DT_MACHINE_START(TEGRA_DT, "nVidia Tegra20 (Flattened Device Tree)") .map_io = tegra_map_common_io, .smp = smp_ops(tegra_smp_ops), - .init_early = tegra20_init_early, + .init_early = tegra_init_early, .init_irq = tegra_dt_init_irq, .init_time = clocksource_of_init, .init_machine = tegra_dt_init, diff --git a/arch/arm/mach-tegra/board-dt-tegra30.c b/arch/arm/mach-tegra/board-dt-tegra30.c index bf68567..63f8139 100644 --- a/arch/arm/mach-tegra/board-dt-tegra30.c +++ b/arch/arm/mach-tegra/board-dt-tegra30.c @@ -3,7 +3,7 @@ * * NVIDIA Tegra30 device tree board support * - * Copyright (C) 2011 NVIDIA Corporation + * Copyright (C) 2011, 2013, NVIDIA Corporation * * Derived from: * @@ -50,7 +50,7 @@ static const char *tegra30_dt_board_compat[] = { DT_MACHINE_START(TEGRA30_DT, "NVIDIA Tegra30 (Flattened Device Tree)") .smp = smp_ops(tegra_smp_ops), .map_io = tegra_map_common_io, - .init_early = tegra30_init_early, + .init_early = tegra_init_early, .init_irq = tegra_dt_init_irq, .init_time = clocksource_of_init, .init_machine = tegra30_dt_init, diff --git a/arch/arm/mach-tegra/board.h b/arch/arm/mach-tegra/board.h index 86851c8..60431de 100644 --- a/arch/arm/mach-tegra/board.h +++ b/arch/arm/mach-tegra/board.h @@ -26,9 +26,7 @@ void tegra_assert_system_reset(char mode, const char *cmd); -void __init tegra20_init_early(void); -void __init tegra30_init_early(void); -void __init tegra114_init_early(void); +void __init tegra_init_early(void); void __init tegra_map_common_io(void); void __init tegra_init_irq(void); void __init tegra_dt_init_irq(void); diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c index 5449a3f..f0315c9 100644 --- a/arch/arm/mach-tegra/common.c +++ b/arch/arm/mach-tegra/common.c @@ -94,7 +94,7 @@ static void __init tegra_init_cache(void) } -static void __init tegra_init_early(void) +void __init tegra_init_early(void) { tegra_cpu_reset_handler_init(); tegra_apb_io_init(); @@ -102,31 +102,9 @@ static void __init tegra_init_early(void) tegra_init_cache(); tegra_pmc_init(); tegra_powergate_init(); + tegra_hotplug_init(); } -#ifdef CONFIG_ARCH_TEGRA_2x_SOC -void __init tegra20_init_early(void) -{ - tegra_init_early(); - tegra20_hotplug_init(); -} -#endif - -#ifdef CONFIG_ARCH_TEGRA_3x_SOC -void __init tegra30_init_early(void) -{ - tegra_init_early(); - tegra30_hotplug_init(); -} -#endif - -#ifdef CONFIG_ARCH_TEGRA_114_SOC -void __init tegra114_init_early(void) -{ - tegra_init_early(); -} -#endif - void __init tegra_init_late(void) { tegra_powergate_debugfs_init(); diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c index a599f6e..8da9f78 100644 --- a/arch/arm/mach-tegra/hotplug.c +++ b/arch/arm/mach-tegra/hotplug.c @@ -1,8 +1,7 @@ /* - * * Copyright (C) 2002 ARM Ltd. * All Rights Reserved - * Copyright (c) 2010, 2012 NVIDIA Corporation. All rights reserved. + * Copyright (c) 2010, 2012-2013, NVIDIA Corporation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -15,6 +14,7 @@ #include <asm/cacheflush.h> #include <asm/smp_plat.h> +#include "fuse.h" #include "sleep.h" static void (*tegra_hotplug_shutdown)(void); @@ -56,18 +56,13 @@ int tegra_cpu_disable(unsigned int cpu) return cpu == 0 ? -EPERM : 0; } -#ifdef CONFIG_ARCH_TEGRA_2x_SOC -extern void tegra20_hotplug_shutdown(void); -void __init tegra20_hotplug_init(void) +void __init tegra_hotplug_init(void) { - tegra_hotplug_shutdown = tegra20_hotplug_shutdown; -} -#endif + if (!IS_ENABLED(CONFIG_HOTPLUG_CPU)) + return; -#ifdef CONFIG_ARCH_TEGRA_3x_SOC -extern void tegra30_hotplug_shutdown(void); -void __init tegra30_hotplug_init(void) -{ - tegra_hotplug_shutdown = tegra30_hotplug_shutdown; + if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && tegra_chip_id == TEGRA20) + tegra_hotplug_shutdown = tegra20_hotplug_shutdown; + if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) && tegra_chip_id == TEGRA30) + tegra_hotplug_shutdown = tegra30_hotplug_shutdown; } -#endif diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h index 4ffae54..970ebd5 100644 --- a/arch/arm/mach-tegra/sleep.h +++ b/arch/arm/mach-tegra/sleep.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2012, NVIDIA Corporation. All rights reserved. + * Copyright (c) 2010-2013, NVIDIA Corporation. All rights reserved. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -124,11 +124,11 @@ int tegra_sleep_cpu_finish(unsigned long); void tegra_disable_clean_inv_dcache(void); #ifdef CONFIG_HOTPLUG_CPU -void tegra20_hotplug_init(void); -void tegra30_hotplug_init(void); +void tegra20_hotplug_shutdown(void); +void tegra30_hotplug_shutdown(void); +void tegra_hotplug_init(void); #else -static inline void tegra20_hotplug_init(void) {} -static inline void tegra30_hotplug_init(void) {} +static inline void tegra_hotplug_init(void) {} #endif void tegra20_cpu_shutdown(int cpu);
Refactored tegra{20,30,114}_init_early() so that we have the unified tegra_init_early(). Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> --- Update: Used IS_ENABLED() instead of ifdefs as Arnd/Felipe suggested. --- arch/arm/mach-tegra/board-dt-tegra114.c | 2 +- arch/arm/mach-tegra/board-dt-tegra20.c | 2 +- arch/arm/mach-tegra/board-dt-tegra30.c | 4 ++-- arch/arm/mach-tegra/board.h | 4 +--- arch/arm/mach-tegra/common.c | 26 ++------------------------ arch/arm/mach-tegra/hotplug.c | 23 +++++++++-------------- arch/arm/mach-tegra/sleep.h | 10 +++++----- 7 files changed, 21 insertions(+), 50 deletions(-)