diff mbox

[v2,2/8] arm/tegra: prepare early init for multiple tegra variants

Message ID 1320242948-21061-3-git-send-email-pdeschrijver@nvidia.com
State Superseded, archived
Headers show

Commit Message

Peter De Schrijver Nov. 2, 2011, 2:09 p.m. UTC
This patch splits the early init code in a common and a tegra20 specific part.
L2 cache initialization is generalized and discovers the cache associativity
at runtime.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 arch/arm/mach-tegra/board-dt.c        |   19 +++++++++++++++++++
 arch/arm/mach-tegra/board-harmony.c   |    2 +-
 arch/arm/mach-tegra/board-paz00.c     |    2 +-
 arch/arm/mach-tegra/board-seaboard.c  |    6 +++---
 arch/arm/mach-tegra/board-trimslice.c |    2 +-
 arch/arm/mach-tegra/board.h           |    4 +---
 arch/arm/mach-tegra/common.c          |   33 ++++++++++++++++++++++-----------
 7 files changed, 48 insertions(+), 20 deletions(-)

Comments

Colin Cross Nov. 2, 2011, 6:30 p.m. UTC | #1
On Wed, Nov 2, 2011 at 7:09 AM, Peter De Schrijver
<pdeschrijver@nvidia.com> wrote:
> This patch splits the early init code in a common and a tegra20 specific part.
> L2 cache initialization is generalized and discovers the cache associativity
> at runtime.
>
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
<snip>

> -static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
> +void (*arch_reset)(char mode, const char *cmd) = tegra_assert_system_reset;
> +
This is unrelated to the patch description?

> +static __initdata struct tegra_clk_init_table tegra20_clk_init_table[] = {
>        /* name         parent          rate            enabled */
>        { "clk_m",      NULL,           0,              true },
>        { "pll_p",      "clk_m",        216000000,      true },
> @@ -59,25 +61,34 @@ static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
>        { NULL,         NULL,           0,              0},
>  };
>
> -static void __init tegra_init_cache(void)
> +static void __init tegra_init_cache(u32 tag_latency, u32 data_latency)
>  {
>  #ifdef CONFIG_CACHE_L2X0
>        void __iomem *p = IO_ADDRESS(TEGRA_ARM_PERIF_BASE) + 0x3000;
> +       u32 aux_ctrl;
> +
> +       writel_relaxed(tag_latency, p + L2X0_TAG_LATENCY_CTRL);
> +       writel_relaxed(data_latency, p + L2X0_DATA_LATENCY_CTRL);
>
> -       writel_relaxed(0x331, p + L2X0_TAG_LATENCY_CTRL);
> -       writel_relaxed(0x441, p + L2X0_DATA_LATENCY_CTRL);
> +       aux_ctrl = readl(p + L2X0_CACHE_TYPE);
> +       aux_ctrl = (aux_ctrl & 0x700) << (17-8);

If I'm interpreting the PL310 TRM correctly, the cache associativity
value in the L2X0_CACHE_TYPE register is read from the aux control
register, so this is just leaving the existing value in place.  If the
value is not being modified, just take it out of the aux_ctrl mask
passed to l2x0_init.

> +       aux_ctrl |= 0x7C000001;

This adds the BRESP bit to the aux control register.  Considering the
problems I've seen with random bits in the L2 cache causing all sorts
of instabilities, this should be split out into a separate change so
that it's obvious in a bisect.

>
> -       l2x0_init(p, 0x6C080001, 0x8200c3fe);
> +       l2x0_init(p, aux_ctrl, 0x8200c3fe);
>  #endif
>
>  }
>
> -void __init tegra_init_early(void)
> +static void __init tegra_setup_system_reset(void)
> +{
> +       arm_arch_reset = tegra_assert_system_reset;
> +}
> +
> +void __init tegra20_init_early(void)
>  {
>        tegra_init_fuse();
>        tegra_init_clock();
> -       tegra_clk_init_from_table(common_clk_init_table);
> -       tegra_init_cache();
> -
> -       arm_arch_reset = tegra_assert_system_reset;
> +       tegra_clk_init_from_table(tegra20_clk_init_table);
> +       tegra_init_cache(0x331, 0x441);
> +       tegra_setup_system_reset();

It seems likely that there will be multiple init functions that need
to be called from init_early on both tegra20 and tegra30.  Maybe add a
common function that only calls tegra_setup_system_reset() for now?

>  }
> --
> 1.7.7.rc0.72.g4b5ea.dirty
>
> --
> 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
>
--
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
Peter De Schrijver Nov. 3, 2011, 9:21 a.m. UTC | #2
On Wed, Nov 02, 2011 at 07:30:43PM +0100, Colin Cross wrote:
> On Wed, Nov 2, 2011 at 7:09 AM, Peter De Schrijver
> <pdeschrijver@nvidia.com> wrote:
> > This patch splits the early init code in a common and a tegra20 specific part.
> > L2 cache initialization is generalized and discovers the cache associativity
> > at runtime.
> >
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > ---
> <snip>
> 
> > -static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
> > +void (*arch_reset)(char mode, const char *cmd) = tegra_assert_system_reset;
> > +
> This is unrelated to the patch description?
> 

This is a left over from the past. Will be removed from the next version.

> > +static __initdata struct tegra_clk_init_table tegra20_clk_init_table[] = {
> >        /* name         parent          rate            enabled */
> >        { "clk_m",      NULL,           0,              true },
> >        { "pll_p",      "clk_m",        216000000,      true },
> > @@ -59,25 +61,34 @@ static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
> >        { NULL,         NULL,           0,              0},
> >  };
> >
> > -static void __init tegra_init_cache(void)
> > +static void __init tegra_init_cache(u32 tag_latency, u32 data_latency)
> >  {
> >  #ifdef CONFIG_CACHE_L2X0
> >        void __iomem *p = IO_ADDRESS(TEGRA_ARM_PERIF_BASE) + 0x3000;
> > +       u32 aux_ctrl;
> > +
> > +       writel_relaxed(tag_latency, p + L2X0_TAG_LATENCY_CTRL);
> > +       writel_relaxed(data_latency, p + L2X0_DATA_LATENCY_CTRL);
> >
> > -       writel_relaxed(0x331, p + L2X0_TAG_LATENCY_CTRL);
> > -       writel_relaxed(0x441, p + L2X0_DATA_LATENCY_CTRL);
> > +       aux_ctrl = readl(p + L2X0_CACHE_TYPE);
> > +       aux_ctrl = (aux_ctrl & 0x700) << (17-8);
> 
> If I'm interpreting the PL310 TRM correctly, the cache associativity
> value in the L2X0_CACHE_TYPE register is read from the aux control
> register, so this is just leaving the existing value in place.  If the
> value is not being modified, just take it out of the aux_ctrl mask
> passed to l2x0_init.
> 

This seems to work fine on our android kernels though. Need to dig deeper
what's going on here.

> > +       aux_ctrl |= 0x7C000001;
> 
> This adds the BRESP bit to the aux control register.  Considering the
> problems I've seen with random bits in the L2 cache causing all sorts
> of instabilities, this should be split out into a separate change so
> that it's obvious in a bisect.
> 

Ok. Will remove that for now.

> >
> > -       l2x0_init(p, 0x6C080001, 0x8200c3fe);
> > +       l2x0_init(p, aux_ctrl, 0x8200c3fe);
> >  #endif
> >
> >  }
> >
> > -void __init tegra_init_early(void)
> > +static void __init tegra_setup_system_reset(void)
> > +{
> > +       arm_arch_reset = tegra_assert_system_reset;
> > +}
> > +
> > +void __init tegra20_init_early(void)
> >  {
> >        tegra_init_fuse();
> >        tegra_init_clock();
> > -       tegra_clk_init_from_table(common_clk_init_table);
> > -       tegra_init_cache();
> > -
> > -       arm_arch_reset = tegra_assert_system_reset;
> > +       tegra_clk_init_from_table(tegra20_clk_init_table);
> > +       tegra_init_cache(0x331, 0x441);
> > +       tegra_setup_system_reset();
> 
> It seems likely that there will be multiple init functions that need
> to be called from init_early on both tegra20 and tegra30.  Maybe add a
> common function that only calls tegra_setup_system_reset() for now?
> 

Ok. Will do that.

Cheers,

Peter.
--
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
Russell King - ARM Linux Nov. 3, 2011, 10:02 a.m. UTC | #3
On Thu, Nov 03, 2011 at 11:21:16AM +0200, Peter De Schrijver wrote:
> On Wed, Nov 02, 2011 at 07:30:43PM +0100, Colin Cross wrote:
> > On Wed, Nov 2, 2011 at 7:09 AM, Peter De Schrijver
> > <pdeschrijver@nvidia.com> wrote:
> > > This patch splits the early init code in a common and a tegra20 specific part.
> > > L2 cache initialization is generalized and discovers the cache associativity
> > > at runtime.
> > >
> > > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > > ---
> > <snip>
> > 
> > > -static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
> > > +void (*arch_reset)(char mode, const char *cmd) = tegra_assert_system_reset;
> > > +
> > This is unrelated to the patch description?
> > 
> 
> This is a left over from the past. Will be removed from the next version.

Please can Tegra move over to hooking the restart via arm_pm_restart()
rather than via arch_reset()?  That would leave arch_reset() for
software-based reboot methods - which I suspect Tegra doesn't use.

That would allow Tegra to dovetail into the arch_reset cleanup work much
better than it would otherwise do.
--
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
Olof Johansson Nov. 4, 2011, 3:09 p.m. UTC | #4
On Thu, Nov 3, 2011 at 3:02 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Nov 03, 2011 at 11:21:16AM +0200, Peter De Schrijver wrote:
>> On Wed, Nov 02, 2011 at 07:30:43PM +0100, Colin Cross wrote:
>> > On Wed, Nov 2, 2011 at 7:09 AM, Peter De Schrijver
>> > <pdeschrijver@nvidia.com> wrote:
>> > > This patch splits the early init code in a common and a tegra20 specific part.
>> > > L2 cache initialization is generalized and discovers the cache associativity
>> > > at runtime.
>> > >
>> > > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>> > > ---
>> > <snip>
>> >
>> > > -static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
>> > > +void (*arch_reset)(char mode, const char *cmd) = tegra_assert_system_reset;
>> > > +
>> > This is unrelated to the patch description?
>> >
>>
>> This is a left over from the past. Will be removed from the next version.
>
> Please can Tegra move over to hooking the restart via arm_pm_restart()
> rather than via arch_reset()?  That would leave arch_reset() for
> software-based reboot methods - which I suspect Tegra doesn't use.
>
> That would allow Tegra to dovetail into the arch_reset cleanup work much
> better than it would otherwise do.

Sounds reasonable to me. Want a patch to put in your arch_reset
cleanup branch, I presume? I'll post something later today.


-Olof
--
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
Russell King - ARM Linux Nov. 4, 2011, 4:30 p.m. UTC | #5
On Fri, Nov 04, 2011 at 08:09:42AM -0700, Olof Johansson wrote:
> Sounds reasonable to me. Want a patch to put in your arch_reset
> cleanup branch, I presume? I'll post something later today.

I would, but I've just changed the method slightly.

It was becoming silly adding functions to every platform to initialize
arm_pm_restart() in some way (preferably as early as possible) so I've
scrapped that idea.

I've just replaced it with something a little better, which is to put
a pointer directly in the machine_desc stuff, and have generic code
setup the pointer if this is set.

I'll post some new patches sometime in the next few days.
--
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
Peter De Schrijver Nov. 7, 2011, 12:36 p.m. UTC | #6
On Thu, Nov 03, 2011 at 10:21:16AM +0100, Peter De Schrijver wrote:
> On Wed, Nov 02, 2011 at 07:30:43PM +0100, Colin Cross wrote:
> > On Wed, Nov 2, 2011 at 7:09 AM, Peter De Schrijver
> > <pdeschrijver@nvidia.com> wrote:
> > > This patch splits the early init code in a common and a tegra20 specific part.
> > > L2 cache initialization is generalized and discovers the cache associativity
> > > at runtime.
> > >
> > > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > > ---
> > <snip>
> > 
> > > -static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
> > > +void (*arch_reset)(char mode, const char *cmd) = tegra_assert_system_reset;
> > > +
> > This is unrelated to the patch description?
> > 
> 
> This is a left over from the past. Will be removed from the next version.
> 
> > > +static __initdata struct tegra_clk_init_table tegra20_clk_init_table[] = {
> > >        /* name         parent          rate            enabled */
> > >        { "clk_m",      NULL,           0,              true },
> > >        { "pll_p",      "clk_m",        216000000,      true },
> > > @@ -59,25 +61,34 @@ static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
> > >        { NULL,         NULL,           0,              0},
> > >  };
> > >
> > > -static void __init tegra_init_cache(void)
> > > +static void __init tegra_init_cache(u32 tag_latency, u32 data_latency)
> > >  {
> > >  #ifdef CONFIG_CACHE_L2X0
> > >        void __iomem *p = IO_ADDRESS(TEGRA_ARM_PERIF_BASE) + 0x3000;
> > > +       u32 aux_ctrl;
> > > +
> > > +       writel_relaxed(tag_latency, p + L2X0_TAG_LATENCY_CTRL);
> > > +       writel_relaxed(data_latency, p + L2X0_DATA_LATENCY_CTRL);
> > >
> > > -       writel_relaxed(0x331, p + L2X0_TAG_LATENCY_CTRL);
> > > -       writel_relaxed(0x441, p + L2X0_DATA_LATENCY_CTRL);
> > > +       aux_ctrl = readl(p + L2X0_CACHE_TYPE);
> > > +       aux_ctrl = (aux_ctrl & 0x700) << (17-8);
> > 
> > If I'm interpreting the PL310 TRM correctly, the cache associativity
> > value in the L2X0_CACHE_TYPE register is read from the aux control
> > register, so this is just leaving the existing value in place.  If the
> > value is not being modified, just take it out of the aux_ctrl mask
> > passed to l2x0_init.
> > 
> 
> This seems to work fine on our android kernels though. Need to dig deeper
> what's going on here.

Doesn't seem to be that simple on tegra. The L2X0_CACHE_TYPE can indicate a
different waysize depending on fuses.

Cheers,

Peter.

--
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
diff mbox

Patch

diff --git a/arch/arm/mach-tegra/board-dt.c b/arch/arm/mach-tegra/board-dt.c
index d368f8d..a7bae50 100644
--- a/arch/arm/mach-tegra/board-dt.c
+++ b/arch/arm/mach-tegra/board-dt.c
@@ -118,6 +118,25 @@  static void __init tegra_dt_init(void)
 	of_platform_populate(NULL, tegra_dt_match_table, tegra20_auxdata_lookup, NULL);
 }
 
+static struct {
+	const char *machine;
+	void (*init)(void);
+} early_init[] __initdata = {
+	{ "nvidia,tegra20", tegra20_init_early },
+};
+
+static void __init tegra_init_early(void)
+{
+
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(early_init); i++)
+		if (of_machine_is_compatible(early_init[i].machine))
+			return early_init[i].init();
+
+	pr_warn("Unknown platform detected\n");
+}
+
 static const char * tegra_dt_board_compat[] = {
 	"nvidia,harmony",
 	"nvidia,seaboard",
diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c
index f0bdc5e..c422aeb 100644
--- a/arch/arm/mach-tegra/board-harmony.c
+++ b/arch/arm/mach-tegra/board-harmony.c
@@ -185,7 +185,7 @@  MACHINE_START(HARMONY, "harmony")
 	.atag_offset	= 0x100,
 	.fixup		= tegra_harmony_fixup,
 	.map_io         = tegra_map_common_io,
-	.init_early	= tegra_init_early,
+	.init_early	= tegra20_init_early,
 	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
 	.init_machine   = tegra_harmony_init,
diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 602f8dd..6997a47 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -188,7 +188,7 @@  MACHINE_START(PAZ00, "Toshiba AC100 / Dynabook AZ")
 	.atag_offset	= 0x100,
 	.fixup		= tegra_paz00_fixup,
 	.map_io         = tegra_map_common_io,
-	.init_early	= tegra_init_early,
+	.init_early	= tegra20_init_early,
 	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
 	.init_machine   = tegra_paz00_init,
diff --git a/arch/arm/mach-tegra/board-seaboard.c b/arch/arm/mach-tegra/board-seaboard.c
index bf13ea3..5e42e08 100644
--- a/arch/arm/mach-tegra/board-seaboard.c
+++ b/arch/arm/mach-tegra/board-seaboard.c
@@ -282,7 +282,7 @@  static void __init tegra_wario_init(void)
 MACHINE_START(SEABOARD, "seaboard")
 	.atag_offset    = 0x100,
 	.map_io         = tegra_map_common_io,
-	.init_early     = tegra_init_early,
+	.init_early     = tegra20_init_early,
 	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
 	.init_machine   = tegra_seaboard_init,
@@ -291,7 +291,7 @@  MACHINE_END
 MACHINE_START(KAEN, "kaen")
 	.atag_offset    = 0x100,
 	.map_io         = tegra_map_common_io,
-	.init_early     = tegra_init_early,
+	.init_early     = tegra20_init_early,
 	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
 	.init_machine   = tegra_kaen_init,
@@ -300,7 +300,7 @@  MACHINE_END
 MACHINE_START(WARIO, "wario")
 	.atag_offset    = 0x100,
 	.map_io         = tegra_map_common_io,
-	.init_early     = tegra_init_early,
+	.init_early     = tegra20_init_early,
 	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
 	.init_machine   = tegra_wario_init,
diff --git a/arch/arm/mach-tegra/board-trimslice.c b/arch/arm/mach-tegra/board-trimslice.c
index e008c0e..7117e81 100644
--- a/arch/arm/mach-tegra/board-trimslice.c
+++ b/arch/arm/mach-tegra/board-trimslice.c
@@ -175,7 +175,7 @@  MACHINE_START(TRIMSLICE, "trimslice")
 	.atag_offset	= 0x100,
 	.fixup		= tegra_trimslice_fixup,
 	.map_io         = tegra_map_common_io,
-	.init_early	= tegra_init_early,
+	.init_early	= tegra20_init_early,
 	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
 	.init_machine   = tegra_trimslice_init,
diff --git a/arch/arm/mach-tegra/board.h b/arch/arm/mach-tegra/board.h
index 1d14df7..b86cdab 100644
--- a/arch/arm/mach-tegra/board.h
+++ b/arch/arm/mach-tegra/board.h
@@ -23,9 +23,7 @@ 
 
 #include <linux/types.h>
 
-void tegra_assert_system_reset(char mode, const char *cmd);
-
-void __init tegra_init_early(void);
+void __init tegra20_init_early(void);
 void __init tegra_map_common_io(void);
 void __init tegra_init_irq(void);
 void __init tegra_init_clock(void);
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 03c2bd4..6173fd8 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -1,5 +1,5 @@ 
 /*
- * arch/arm/mach-tegra/board-harmony.c
+ * arch/arm/mach-tegra/common.c
  *
  * Copyright (C) 2010 Google, Inc.
  *
@@ -42,7 +42,9 @@  void tegra_assert_system_reset(char mode, const char *cmd)
 	writel_relaxed(reg, reset);
 }
 
-static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
+void (*arch_reset)(char mode, const char *cmd) = tegra_assert_system_reset;
+
+static __initdata struct tegra_clk_init_table tegra20_clk_init_table[] = {
 	/* name		parent		rate		enabled */
 	{ "clk_m",	NULL,		0,		true },
 	{ "pll_p",	"clk_m",	216000000,	true },
@@ -59,25 +61,34 @@  static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
 	{ NULL,		NULL,		0,		0},
 };
 
-static void __init tegra_init_cache(void)
+static void __init tegra_init_cache(u32 tag_latency, u32 data_latency)
 {
 #ifdef CONFIG_CACHE_L2X0
 	void __iomem *p = IO_ADDRESS(TEGRA_ARM_PERIF_BASE) + 0x3000;
+	u32 aux_ctrl;
+
+	writel_relaxed(tag_latency, p + L2X0_TAG_LATENCY_CTRL);
+	writel_relaxed(data_latency, p + L2X0_DATA_LATENCY_CTRL);
 
-	writel_relaxed(0x331, p + L2X0_TAG_LATENCY_CTRL);
-	writel_relaxed(0x441, p + L2X0_DATA_LATENCY_CTRL);
+	aux_ctrl = readl(p + L2X0_CACHE_TYPE);
+	aux_ctrl = (aux_ctrl & 0x700) << (17-8);
+	aux_ctrl |= 0x7C000001;
 
-	l2x0_init(p, 0x6C080001, 0x8200c3fe);
+	l2x0_init(p, aux_ctrl, 0x8200c3fe);
 #endif
 
 }
 
-void __init tegra_init_early(void)
+static void __init tegra_setup_system_reset(void)
+{
+	arm_arch_reset = tegra_assert_system_reset;
+}
+
+void __init tegra20_init_early(void)
 {
 	tegra_init_fuse();
 	tegra_init_clock();
-	tegra_clk_init_from_table(common_clk_init_table);
-	tegra_init_cache();
-
-	arm_arch_reset = tegra_assert_system_reset;
+	tegra_clk_init_from_table(tegra20_clk_init_table);
+	tegra_init_cache(0x331, 0x441);
+	tegra_setup_system_reset();
 }