On Wed, Jun 06, 2012 at 09:39:57AM -0700, Stephen Warren wrote:
> On 06/05/2012 03:20 PM, Allen Martin wrote:
> > Add support for tegra20 arm7 boot processor. This processor is used
> > to power on the Cortex A9 and transfer control to it.
>
> > diff --git a/arch/arm/cpu/arm720t/cpu.c b/arch/arm/cpu/arm720t/cpu.c
>
> > #elif defined(CONFIG_INTEGRATOR) && defined(CONFIG_ARCH_INTEGRATOR)
> > /* No cleanup before linux for IntegratorAP/CM720T as yet */
> > +#elif defined (CONFIG_MACH_TEGRA_GENERIC)
> > + /* No cleanup before linux for tegra as yet */
> > #else
> > #error No cleanup_before_linux() defined for this CPU type
> > #endif
>
> Wouldn't CONFIG_TEGRA2 be a better define to use? Same comment throughout.
The SPL *should* work on tegra30 with only minor changes, so just
trying to look forward.
> > diff --git a/arch/arm/cpu/arm720t/tegra20/cpu.c
> > b/arch/arm/cpu/arm720t/tegra20/cpu.c
>
> > +/* Returns 1 if the current CPU executing is a Cortex-A9, else 0 */
> > +int ap20_cpu_is_cortexa9(void)
> > +{
> > + u32 id = readb(NV_PA_PG_UP_BASE + PG_UP_TAG_0);
> > + return id == (PG_UP_TAG_0_PID_CPU & 0xff);
> > +}
>
> Doesn't the SPL always run on the AVP, and the regular U-Boot build
> always run on the main CPUs - I'm not sure why this runtime API would be
> needed.
I think this is just a holdover from the old code, I'll look at
purging it.
> > +void init_pllx(void)
>
> I assume a lot of the code in this file is copied from the
> armv7/tegra20/*.c files. Shouldn't this patch remove the ARMv7 versions
> since presumably it's no longer needed - doesn't the non-SPL U-Boot rely
> on the SPL U-Boot always having run first in order to start the main
> U-Boot executing?
Yes, I remove some of it in "tegra20: Remove CPU init code from
tegra20 u-boot". I'll look at rolling that patch into this one, but I
originally made it two patches to try to preserve building and booting
at every patch.
>
> If not, then shouldn't this code be in tegra20-common so it can be shared?
>
> > diff --git a/arch/arm/cpu/arm720t/tegra20/cpu.h
> > b/arch/arm/cpu/arm720t/tegra20/cpu.h
>
> Similarly, this file seems like it'd be in tegra20-common/include or
> somewhere like that.
Those defines are only used for the Cortex A9 init, so it should only
be needed from the SPL.
-Allen
--
nvpublic
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
