[U-Boot] [PATCH V2] arm: Tegra2: add support for A9 CPU init

[Peter Tyser <ptyser@xes-inc.com>]

Hi Tom,
Things look pretty good.  Minor comments/questions below.

<snip>

> +/*
> + * TBD: Move cold_boot() to assembly file.
> + * Values/offsets of the table vars make this difficult.
> + */
> +
> +void cold_boot(void)
> +{
> +	asm volatile(
> +		"msr	cpsr_c, #0xD3	\n"
> +		/*
> +		* Check current processor: CPU or AVP?
> +		* If CPU, go to CPU boot code, else continue on AVP path.
> +		*/
> +		"mov	r0, %0		\n"
> +		"ldrb	r2, [r0, %1]	\n"
> +		/* are we the CPU? */
> +		"cmp	r2, %2		\n"
> +		"mov	sp, %3		\n"
> +		/*  yep, we are the CPU */
> +		"bxeq	%4		\n"
> +
> +		/* AVP initialization follows this path */
> +		"mov	sp, %5		\n"
> +		/* Init and start CPU */
> +		"b	startup_cpu	\n"
> +		:
> +		: "i"(NV_PA_PG_UP_BASE),
> +		"i"(PG_UP_TAG_0),
> +		"r"(proc_tag),
> +		"r"(cpu_boot_stack),
> +		"r"(_armboot_start),
> +		"r"(avp_boot_stack)
> +		: "r0", "r2", "cc", "lr"
> +	);
> +}

What errors did you encounter when this was in the assembly file?  It'd
be nice to put it there now.  Likely it will never get fixed if it
doesn't implemented correctly off the bat.  If you post the errors
perhaps someone on the list can provide insight.

<snip>

> +.globl startup_cpu
> +startup_cpu:
> +	@ Initialize the AVP, clocks, and memory controller
> +	@ SDRAM is guaranteed to be on at this point
> +
> +	ldr     r0, =cold_boot			@ R0 = reset vector for CPU
> +	bl      start_cpu			@ start the CPU
> +
> +	@ Transfer control to the AVP code */
> +	bl      halt_avp
> +
> +	@ Should never get here
> +_loop_forever2:
> +	b	_loop_forever2
> +
> +.globl cache_configure
> +cache_configure:
> +	stmdb r13!,{r14}
> +@	/* invalidate instruction cache */

It looks like there's a combination of comment forms @, @ */, and @ /*
*/.  Is there a reason not to use the normal /* */ universally?

Best,
Peter