[U-Boot] [PATCH v1 8/8] mpc85xx: introduce the kmp204x reference design support

[York Sun]

On 08/20/2013 11:21 AM, Scott Wood wrote:
> On Tue, 2013-08-20 at 13:20 -0500, Scott Wood wrote:
>> On Mon, 2013-08-19 at 18:02 -0700, York Sun wrote:
>>> On 08/19/2013 05:48 PM, Scott Wood wrote:
>>>> On Mon, 2013-08-19 at 17:50 +0200, Valentin Longchamp wrote:
>>>>> On 08/13/2013 11:38 PM, Scott Wood wrote:
>>>>>> On Fri, 2013-07-26 at 12:02 +0200, Valentin Longchamp wrote:
>>>
>>> <snip>
>>>
>>>>>>> +	/* TLB 1 */
>>>>>>> +	/* *I*** - Covers boot page */
>>>>>>> +	/* *I*G - L3SRAM. When L3 is used as 1M SRAM, the address of the
>>>>>>> +	 * SRAM is at 0xfff00000, it covered the 0xfffff000.
>>>>>>> +	 */
>>>>>>> +	SET_TLB_ENTRY(1, CONFIG_SYS_INIT_L3_ADDR, CONFIG_SYS_INIT_L3_ADDR,
>>>>>>> +		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
>>>>>>> +		      0, 0, BOOKE_PAGESZ_1M, 1),
>>>>>>
>>>>>> What does that "covers boot page" comment refer to?
>>>>>>
>>>>>> Why is L3SRAM I+G?
>>>>>>
>>>>>
>>>>> I have taken this from the corenet SYS_RAMBOOT boot scenario since it's also the
>>>>> way our board boots.
>>>>
>>>> York, can you answer this?
>>>>
>>>> I suspect the "covers boot page" comment is left over from before the
>>>> recent spin table changes.
>>>
>>> Look at the context, this is used as SRAM with PBL boot method. Notice
>>> these macros in header file
>>
>> I'm not talking about the SRAM comment, but the "covers boot page"
>> comment before it.

I think this entry replaces the default TLB out of reset and it does
cover the boot page 0xfffff000~0xffffffff. It is not unique to this
platform. You can find many similar existing code.

>>
>> At the very least this mapping can't be *I*G and *I** at the same time.

I agree the G bit shouldn't be set here.


>>
>>> +#define CONFIG_SYS_RAMBOOT
>>> +#define CONFIG_RAMBOOT_PBL
>>> +#define CONFIG_RAMBOOT_TEXT_BASE	CONFIG_SYS_TEXT_BASE
>>>
>>> and
>>>
>>> +/*
>>> + *  Config the L3 Cache as L3 SRAM
>>> + */
>>> +#define CONFIG_SYS_INIT_L3_ADDR		CONFIG_RAMBOOT_TEXT_BASE
>>> +#define CONFIG_SYS_INIT_L3_ADDR_PHYS	(0xf00000000ull | \
>>> +		CONFIG_RAMBOOT_TEXT_BASE)
>>> +#define CONFIG_SYS_L3_SIZE		(1024 << 10)
>>> +#define CONFIG_SYS_INIT_L3_END (CONFIG_SYS_INIT_L3_ADDR +
>>> CONFIG_SYS_L3_SIZE)
>>
>> ...and this doesn't cover the boot page.
> 
> Also, can you answer the question about why the L3 SRAM mapping is
> cache-inhibited?

I suspect this is the idea carried from early NAND boot implementation.
You are mostly familiar with NAND and SPL boot, can you examine if we
can turn on the cache for these cases?

York