Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hi Marc,
Marc Dietrich wrote @ Sun, 10 Feb 2013 22:16:14 +0100:
> ah, ok - I just skipped the "also" in your sentence above. But still, the
> #ifdefs look strange to me and save only a few byte of code. Just me few
> cents.
What about the following as Arnd suggested[1]?
void __init tegra_hotplug_init(void)
{
if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
return;
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;
}
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/148632.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On Sunday 10 February 2013 13:20:49 Stephen Warren wrote:
> 1On 02/10/2013 10:28 AM, Marc Dietrich wrote:
> > On Friday 08 February 2013 10:09:10 Stephen Warren wrote:
> >> On 02/08/2013 05:29 AM, Marc Dietrich wrote:
> >>> Hiroshi,
> >>>
> >>> Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
> Refactored tegra{20,30,114}_init_early() so that we have the unified
> tegra_init_early().
>
> ...
>
> >>> are these ifdefs really needed? Multisoc kernels will enable them all
> >>> anyway and there is a case structure which protects the assignments.
> >>> Also
> >>> the hotplug functions are very tiny, so there shouldn't be a big loss.
> >>
> >> The files that contain/implement those functions are separate for each
> >> SoC and only included in the build when the individual SoCs are enabled.
> >>
> >> While multi-platform SoCs do make sense for distros, we also very
> >> specifically want to support the case where only Tegra, and only a
> >> single Tegra SoC, is enabled, hence this separation.
> >
> > Huh? so tegra_defconfig is not supported?
> >
> > grep "TEGRA_.*_SOC" tegra_defconfig:
> >
> > CONFIG_ARCH_TEGRA_2x_SOC=y
> > CONFIG_ARCH_TEGRA_3x_SOC=y
>
> I don't understand the question.
>
> But to be clear. There are now 3 variants of Tegra supported. (Tegra20,
> Tegra30, Tegra114). We want to be able to build a minimal-size kernel
> (e.g. for embedded applications) that supports just one, any combination
> of two, or all three Tegra variants.
ah, ok - I just skipped the "also" in your sentence above. But still, the
#ifdefs look strange to me and save only a few byte of code. Just me few
cents.
Marc
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
1On 02/10/2013 10:28 AM, Marc Dietrich wrote:
> On Friday 08 February 2013 10:09:10 Stephen Warren wrote:
>> On 02/08/2013 05:29 AM, Marc Dietrich wrote:
>>> Hiroshi,
>>>
>>> Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
Refactored tegra{20,30,114}_init_early() so that we have the unified
tegra_init_early().
...
>>> are these ifdefs really needed? Multisoc kernels will enable them all
>>> anyway and there is a case structure which protects the assignments. Also
>>> the hotplug functions are very tiny, so there shouldn't be a big loss.
>>
>> The files that contain/implement those functions are separate for each
>> SoC and only included in the build when the individual SoCs are enabled.
>>
>> While multi-platform SoCs do make sense for distros, we also very
>> specifically want to support the case where only Tegra, and only a
>> single Tegra SoC, is enabled, hence this separation.
>
> Huh? so tegra_defconfig is not supported?
>
> grep "TEGRA_.*_SOC" tegra_defconfig:
>
> CONFIG_ARCH_TEGRA_2x_SOC=y
> CONFIG_ARCH_TEGRA_3x_SOC=y
I don't understand the question.
But to be clear. There are now 3 variants of Tegra supported. (Tegra20,
Tegra30, Tegra114). We want to be able to build a minimal-size kernel
(e.g. for embedded applications) that supports just one, any combination
of two, or all three Tegra variants.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On Friday 08 February 2013 10:09:10 Stephen Warren wrote:
> On 02/08/2013 05:29 AM, Marc Dietrich wrote:
> > Hiroshi,
> >
> > Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
> >> Refactored tegra{20,30,114}_init_early() so that we have the unified
> >> tegra_init_early().
> >>
> >> diff --git a/arch/arm/mach-tegra/hotplug.c
> >> b/arch/arm/mach-tegra/hotplug.c
> >>
> >> +void __init tegra_hotplug_init(void)
> >>
> >> {
> >>
> >> + switch (tegra_chip_id) {
> >> +#ifdef CONFIG_ARCH_TEGRA_2x_SOC
> >> + case TEGRA20:
> >> + tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
> >> + break;
> >>
> >> #endif
> >>
> >> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
> >> + case TEGRA30:
> >> + tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
> >> + break;
> >>
> >> #endif
> >>
> >> + default:
> >> + BUG_ON(IS_ENABLED(CONFIG_HOTPLUG_CPU));
> >> + break;
> >> + }
> >> +}
> >
> > are these ifdefs really needed? Multisoc kernels will enable them all
> > anyway and there is a case structure which protects the assignments. Also
> > the hotplug functions are very tiny, so there shouldn't be a big loss.
>
> The files that contain/implement those functions are separate for each
> SoC and only included in the build when the individual SoCs are enabled.
>
> While multi-platform SoCs do make sense for distros, we also very
> specifically want to support the case where only Tegra, and only a
> single Tegra SoC, is enabled, hence this separation.
Huh? so tegra_defconfig is not supported?
grep "TEGRA_.*_SOC" tegra_defconfig:
CONFIG_ARCH_TEGRA_2x_SOC=y
CONFIG_ARCH_TEGRA_3x_SOC=y
Marc
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On Friday 08 February 2013 10:09:10 Stephen Warren wrote:
On 02/08/2013 05:29 AM, Marc Dietrich wrote:
Hiroshi,
Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
Refactored tegra{20,30,114}_init_early() so that we have the unified
tegra_init_early().
diff --git a/arch/arm/mach-tegra/hotplug.c
b/arch/arm/mach-tegra/hotplug.c
+void __init tegra_hotplug_init(void)
{
+ switch (tegra_chip_id) {
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+ case TEGRA20:
+ tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
+ break;
#endif
+#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
+ case TEGRA30:
+ tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
+ break;
#endif
+ default:
+ BUG_ON(IS_ENABLED(CONFIG_HOTPLUG_CPU));
+ break;
+ }
+}
are these ifdefs really needed? Multisoc kernels will enable them all
anyway and there is a case structure which protects the assignments. Also
the hotplug functions are very tiny, so there shouldn't be a big loss.
The files that contain/implement those functions are separate for each
SoC and only included in the build when the individual SoCs are enabled.
While multi-platform SoCs do make sense for distros, we also very
specifically want to support the case where only Tegra, and only a
single Tegra SoC, is enabled, hence this separation.
Huh? so tegra_defconfig is not supported?
grep TEGRA_.*_SOC tegra_defconfig:
CONFIG_ARCH_TEGRA_2x_SOC=y
CONFIG_ARCH_TEGRA_3x_SOC=y
Marc
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
1On 02/10/2013 10:28 AM, Marc Dietrich wrote:
On Friday 08 February 2013 10:09:10 Stephen Warren wrote:
On 02/08/2013 05:29 AM, Marc Dietrich wrote:
Hiroshi,
Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
Refactored tegra{20,30,114}_init_early() so that we have the unified
tegra_init_early().
...
are these ifdefs really needed? Multisoc kernels will enable them all
anyway and there is a case structure which protects the assignments. Also
the hotplug functions are very tiny, so there shouldn't be a big loss.
The files that contain/implement those functions are separate for each
SoC and only included in the build when the individual SoCs are enabled.
While multi-platform SoCs do make sense for distros, we also very
specifically want to support the case where only Tegra, and only a
single Tegra SoC, is enabled, hence this separation.
Huh? so tegra_defconfig is not supported?
grep TEGRA_.*_SOC tegra_defconfig:
CONFIG_ARCH_TEGRA_2x_SOC=y
CONFIG_ARCH_TEGRA_3x_SOC=y
I don't understand the question.
But to be clear. There are now 3 variants of Tegra supported. (Tegra20,
Tegra30, Tegra114). We want to be able to build a minimal-size kernel
(e.g. for embedded applications) that supports just one, any combination
of two, or all three Tegra variants.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On Sunday 10 February 2013 13:20:49 Stephen Warren wrote:
1On 02/10/2013 10:28 AM, Marc Dietrich wrote:
On Friday 08 February 2013 10:09:10 Stephen Warren wrote:
On 02/08/2013 05:29 AM, Marc Dietrich wrote:
Hiroshi,
Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
Refactored tegra{20,30,114}_init_early() so that we have the unified
tegra_init_early().
...
are these ifdefs really needed? Multisoc kernels will enable them all
anyway and there is a case structure which protects the assignments.
Also
the hotplug functions are very tiny, so there shouldn't be a big loss.
The files that contain/implement those functions are separate for each
SoC and only included in the build when the individual SoCs are enabled.
While multi-platform SoCs do make sense for distros, we also very
specifically want to support the case where only Tegra, and only a
single Tegra SoC, is enabled, hence this separation.
Huh? so tegra_defconfig is not supported?
grep TEGRA_.*_SOC tegra_defconfig:
CONFIG_ARCH_TEGRA_2x_SOC=y
CONFIG_ARCH_TEGRA_3x_SOC=y
I don't understand the question.
But to be clear. There are now 3 variants of Tegra supported. (Tegra20,
Tegra30, Tegra114). We want to be able to build a minimal-size kernel
(e.g. for embedded applications) that supports just one, any combination
of two, or all three Tegra variants.
ah, ok - I just skipped the also in your sentence above. But still, the
#ifdefs look strange to me and save only a few byte of code. Just me few
cents.
Marc
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hi Marc,
Marc Dietrich marvi...@gmx.de wrote @ Sun, 10 Feb 2013 22:16:14 +0100:
ah, ok - I just skipped the also in your sentence above. But still, the
#ifdefs look strange to me and save only a few byte of code. Just me few
cents.
What about the following as Arnd suggested[1]?
void __init tegra_hotplug_init(void)
{
if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
return;
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;
}
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-February/148632.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On Friday 08 February 2013, Hiroshi Doyu wrote:
> > > +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
> >
> > how about using:
> >
> > #if IS_BUILTIN(CONFIG_ARCH_TEGRA_3x_SOC)
> >
> > instead ?
>
> Why is IS_BUILTIN() prefered?
>
Inside of a function, if(IS_ENABLED(CONFIG_FOO)) or the respective IS_BUILTIN is
preferred over #ifdef because it provides better compile coverage and better
readability. Also, IS_ENABLED() is nice when you want to check if something
is builtin or module, without having to write a complex expression.
If you just replace the #ifdef with #if IS_BUILTIN as Felipe suggested, I see
no real benefit, but it would be nice to write this as
void __init tegra_hotplug_init(void)
{
if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
return;
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;
}
which completely avoids all preprocessor conditionals and replaces them
with compile-time choices based on constant expressions to eliminate the
code paths for disabled platforms.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On Friday 08 February 2013, Hiroshi Doyu wrote:
+#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
how about using:
#if IS_BUILTIN(CONFIG_ARCH_TEGRA_3x_SOC)
instead ?
Why is IS_BUILTIN() prefered?
Inside of a function, if(IS_ENABLED(CONFIG_FOO)) or the respective IS_BUILTIN is
preferred over #ifdef because it provides better compile coverage and better
readability. Also, IS_ENABLED() is nice when you want to check if something
is builtin or module, without having to write a complex expression.
If you just replace the #ifdef with #if IS_BUILTIN as Felipe suggested, I see
no real benefit, but it would be nice to write this as
void __init tegra_hotplug_init(void)
{
if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
return;
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;
}
which completely avoids all preprocessor conditionals and replaces them
with compile-time choices based on constant expressions to eliminate the
code paths for disabled platforms.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On 02/08/2013 05:29 AM, Marc Dietrich wrote:
> Hiroshi,
>
> Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
>> Refactored tegra{20,30,114}_init_early() so that we have the unified
>> tegra_init_early().
>> diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c
>> +void __init tegra_hotplug_init(void)
>> {
>> +switch (tegra_chip_id) {
>> +#ifdef CONFIG_ARCH_TEGRA_2x_SOC
>> +case TEGRA20:
>> +tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
>> +break;
>> #endif
>> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
>> +case TEGRA30:
>> +tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
>> +break;
>> #endif
>> +default:
>> +BUG_ON(IS_ENABLED(CONFIG_HOTPLUG_CPU));
>> +break;
>> +}
>> +}
>
> are these ifdefs really needed? Multisoc kernels will enable them all anyway
> and there is a case structure which protects the assignments. Also the
> hotplug
> functions are very tiny, so there shouldn't be a big loss.
The files that contain/implement those functions are separate for each
SoC and only included in the build when the individual SoCs are enabled.
While multi-platform SoCs do make sense for distros, we also very
specifically want to support the case where only Tegra, and only a
single Tegra SoC, is enabled, hence this separation.
(and Tegra doesn't support multi-platform yet; maybe in another kernel
release or two)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hiroshi,
Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
> Refactored tegra{20,30,114}_init_early() so that we have the unified
> tegra_init_early().
>
> Signed-off-by: Hiroshi Doyu
[...]
> diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c
> index a599f6e..9bcecb8 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
> #include
>
> +#include "fuse.h"
> #include "sleep.h"
>
> static void (*tegra_hotplug_shutdown)(void);
> @@ -56,18 +56,21 @@ 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;
> -}
> + switch (tegra_chip_id) {
> +#ifdef CONFIG_ARCH_TEGRA_2x_SOC
> + case TEGRA20:
> + tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
> + break;
> #endif
> -
> -#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 defined(CONFIG_ARCH_TEGRA_3x_SOC)
> + case TEGRA30:
> + tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
> + break;
> #endif
> + default:
> + BUG_ON(IS_ENABLED(CONFIG_HOTPLUG_CPU));
> + break;
> + }
> +}
are these ifdefs really needed? Multisoc kernels will enable them all anyway
and there is a case structure which protects the assignments. Also the hotplug
functions are very tiny, so there shouldn't be a big loss.
Marc
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hi Felipe,
Felipe Balbi wrote @ Fri, 8 Feb 2013 08:47:20 +0100:
> > +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
>
> how about using:
>
> #if IS_BUILTIN(CONFIG_ARCH_TEGRA_3x_SOC)
>
> instead ?
Why is IS_BUILTIN() prefered?
> > -void tegra20_hotplug_init(void);
> > -void tegra30_hotplug_init(void);
> > +void tegra20_hotplug_shutdown(void);
> > +void tegra30_hotplug_shutdown(void);
>
> aren't these two called only by tegra_hotplug_init() now ?
Yes
> That means they don't need to be exposed.
tegra{20,30}_hotplug_shutdown are defined in sleep-tegra{20,30}.S and
used in hotplug.c So I think that the above are necessary here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hi Felipe,
Felipe Balbi ba...@ti.com wrote @ Fri, 8 Feb 2013 08:47:20 +0100:
+#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
how about using:
#if IS_BUILTIN(CONFIG_ARCH_TEGRA_3x_SOC)
instead ?
Why is IS_BUILTIN() prefered?
-void tegra20_hotplug_init(void);
-void tegra30_hotplug_init(void);
+void tegra20_hotplug_shutdown(void);
+void tegra30_hotplug_shutdown(void);
aren't these two called only by tegra_hotplug_init() now ?
Yes
That means they don't need to be exposed.
tegra{20,30}_hotplug_shutdown are defined in sleep-tegra{20,30}.S and
used in hotplug.c So I think that the above are necessary here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hiroshi,
Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
Refactored tegra{20,30,114}_init_early() so that we have the unified
tegra_init_early().
Signed-off-by: Hiroshi Doyu hd...@nvidia.com
[...]
diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c
index a599f6e..9bcecb8 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,21 @@ 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;
-}
+ switch (tegra_chip_id) {
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+ case TEGRA20:
+ tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
+ break;
#endif
-
-#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 defined(CONFIG_ARCH_TEGRA_3x_SOC)
+ case TEGRA30:
+ tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
+ break;
#endif
+ default:
+ BUG_ON(IS_ENABLED(CONFIG_HOTPLUG_CPU));
+ break;
+ }
+}
are these ifdefs really needed? Multisoc kernels will enable them all anyway
and there is a case structure which protects the assignments. Also the hotplug
functions are very tiny, so there shouldn't be a big loss.
Marc
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
On 02/08/2013 05:29 AM, Marc Dietrich wrote:
Hiroshi,
Am Freitag, 8. Februar 2013, 09:29:31 schrieb Hiroshi Doyu:
Refactored tegra{20,30,114}_init_early() so that we have the unified
tegra_init_early().
diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c
+void __init tegra_hotplug_init(void)
{
+switch (tegra_chip_id) {
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+case TEGRA20:
+tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
+break;
#endif
+#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
+case TEGRA30:
+tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
+break;
#endif
+default:
+BUG_ON(IS_ENABLED(CONFIG_HOTPLUG_CPU));
+break;
+}
+}
are these ifdefs really needed? Multisoc kernels will enable them all anyway
and there is a case structure which protects the assignments. Also the
hotplug
functions are very tiny, so there shouldn't be a big loss.
The files that contain/implement those functions are separate for each
SoC and only included in the build when the individual SoCs are enabled.
While multi-platform SoCs do make sense for distros, we also very
specifically want to support the case where only Tegra, and only a
single Tegra SoC, is enabled, hence this separation.
(and Tegra doesn't support multi-platform yet; maybe in another kernel
release or two)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hi,
On Fri, Feb 08, 2013 at 09:29:31AM +0200, Hiroshi Doyu wrote:
> @@ -56,18 +56,21 @@ 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;
> -}
> + switch (tegra_chip_id) {
> +#ifdef CONFIG_ARCH_TEGRA_2x_SOC
> + case TEGRA20:
> + tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
> + break;
> #endif
> -
> -#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 defined(CONFIG_ARCH_TEGRA_3x_SOC)
how about using:
#if IS_BUILTIN(CONFIG_ARCH_TEGRA_3x_SOC)
instead ?
> 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);
aren't these two called only by tegra_hotplug_init() now ? That means
they don't need to be exposed.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hi,
On Fri, Feb 08, 2013 at 09:29:31AM +0200, Hiroshi Doyu wrote:
@@ -56,18 +56,21 @@ 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;
-}
+ switch (tegra_chip_id) {
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+ case TEGRA20:
+ tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
+ break;
#endif
-
-#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 defined(CONFIG_ARCH_TEGRA_3x_SOC)
how about using:
#if IS_BUILTIN(CONFIG_ARCH_TEGRA_3x_SOC)
instead ?
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);
aren't these two called only by tegra_hotplug_init() now ? That means
they don't need to be exposed.
--
balbi
signature.asc
Description: Digital signature
