Messages in this thread | | | Date | Fri, 27 Jan 2012 10:18:02 +0200 | From | Peter De Schrijver <> | Subject | Re: [PATCH v1 3/8] ARM: tegra: rework Tegra secondary CPU core bringup |
| |
On Thu, Jan 26, 2012 at 09:25:53PM +0100, Stephen Warren wrote: > Peter De Schrijver wrote at Thursday, January 26, 2012 10:07 AM: > > Prepare the Tegra secondary CPU core bringup code for other Tegra variants. > > The reset handler is also generalized to allow for future introduction of > > powersaving modes which turn off the CPU cores. > > > diff --git a/arch/arm/mach-tegra/headsmp.S b/arch/arm/mach-tegra/headsmp.S > > > ENTRY(tegra_secondary_startup) > ... > > + enable_coresight r0 > > > +ENTRY(__tegra_cpu_reset_handler) > > + > > +#if DEBUG_CPU_RESET_HANDLER > > + enable_coresight r0 > > + b . > > +#endif > > I'm not sure why the macro call enable_coresight is ifdef'd in one place > but not the other... Should just the instruction "b ." be inside the > ifdef? >
This code path will also be used by LP2 and LP1 resume in the future, I'm not sure we should unconditionally enable Coresight in that case.
> > diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c > > > +static void tegra_cpu_reset_handler_enable(void) > > > + /* > > + * Prevent further modifications to the physical reset vector. > > + * NOTE: Has no effect on chips prior to Tegra30. > > + */ > > + reg = readl(sb_ctrl); > > + reg |= 2; > > + writel(reg, sb_ctrl); > > + wmb(); > > Should we skip that on Tegra20 then? >
Might make sense. OTOH the bit is just unused on Tegra20 so the write doesn't cause any harm.
> > +void __init tegra_cpu_reset_handler_init(void) > > +{ > > + unsigned long *cpu_reset_data_start, *cpu_reset_data_end; > > Those variables are unused.
Indeed... Leftover from the past...
Cheers,
Peter.
| |