On 08/20/2013 12:47 PM, Scott Wood wrote:
On Tue, 2013-08-20 at 12:40 -0700, York Sun wrote:
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.
That's not what the comment appears to say (unless you read the word "cover" in a non-intuitive and ambiguous way). These comments generally talk about what the new TLB is, not what is being replaced.
It is not unique to this platform. You can find many similar existing code.
I know that. That's why I'm asking you to explain it rather than Valentin. :-)
We have many developers around the globe so people understand "cover" differently. I interpret the "cover" here as this TLB translates the address space which includes the boot page.
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.
Usually I and G go together...
The default TLB out of reset has I bit but not G bit. I have to admit that I don't remember when I used G bit intentionally.
+#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?
NAND SPL on some targets is so space constrained that adding a few instructions to turn cache on might go over the limit. :-)
Are you talking about mapping the NAND buffer that we boot directly out of, or the L2SRAM that we sometimes load the SPL payload into? If the former, that is I+G because we proceed to use it for I/O after relocating out of it.
I am talking aout the latter one. For SPL cases, the code is copied to some type of volatile memory and the core boots from there. I am not sure if we can turn on cache for all cases. Probably yes.
York