[U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms

[Cyril Chemparathy <cyril@ti.com>]

Hi Tom,

[...]
>> +#ifndef CONFIG_SKIP_LOWLEVEL_INIT
> 
> CONFIG_SKIP_LOWLEVEL_INIT is not used in the other patches.
> Why is this needed ?
> board/samsung/samsung/smdk6400 has a lowlevel_init.o function.
> It is confusing why this function is being if-def and not the real
> lowlevel_init..

Will remove.

>> +#ifdef CONFIG_ENABLE_MMU
> 
> This logic is may not be quite correct
>  From include/configs/smdk6400.h
> #if !defined(CONFIG_NAND_SPL) && (TEXT_BASE >= 0xc0000000)
> #define CONFIG_ENABLE_MMU
> #endif
> Please check

Thanks.  This logic was broken.

I have reworked this logic as follows, and removed the ifdef:

-       adr     r1, mmu_disable_phys
-       /* We presume we're within the first 1024 bytes */
-       and     r1, r1, #0x3fc
-       ldr     r2, _TEXT_PHY_BASE
-       ldr     r3, =0xfff00000
-       and     r2, r2, r3
-       orr     r2, r2, r1
+       adr     r2, mmu_disable_phys
+       sub     r2, r2, #(CONFIG_SYS_PHY_UBOOT_BASE - TEXT_BASE)

The earlier implementation was masking out too many bits in trying to
convert the jump address from virtual to physical.  Any comments on this
change?

[...]
>>  	/* Prepare to disable the MMU */
>>  	adr	r1, mmu_disable_phys
>>  	/* We presume we're within the first 1024 bytes */
>> @@ -187,20 +189,60 @@ mmu_disable:
>>  	nop
>>  	nop
>>  	mov	pc, r2
>> +mmu_disable_phys:
>> +#else
>> +	mcr	p15, 0, r0, c1, c0, 0
> 
> Are the noop's above needed here?

I think the original author's intent was to entirely occupy one cache
line.  I don't know enough about the s3c64xx architecture to comment.

[...]
>>  	mcr	p15,0,r0,c15,c2,4       @ 256M (0x70000000 - 0x7fffffff)
>
> This comment '@ 256 .. ' is no longer valid.

Thanks. Will fix.

[...]
>> -	/* move to reserved a couple spots for abort stack */
>> +	/* reserved a couple spots for abort stack */
> 
> The old comment makes more sense.
> Revert this.

Thanks. Will fix.

[...]
>> +#define CONFIG_SKIP_RELOCATE_UBOOT
> 
> There is logic later in this file to undef this value.
> This is likely an error.
> If it isn't, add a comment.

I have removed the subsequent undef and verified that the generated
disassembly of start.S remains the same (with the exception of the
address computation change above) for both usb and non-usb smdk6400 builds.


Thanks
Cyril.