On Tue, 2013-08-20 at 13:26 -0700, York Sun wrote:
On 08/20/2013 01:03 PM, Scott Wood wrote:
On Tue, 2013-08-20 at 12:57 -0700, York Sun wrote:
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.
That's how I'd interpret it as well, but then the comment that "this entry replaces..." doesn't make sense.
The default TLB is TLB1 entry 0. This is the same TLB.
It's the same ESEL, but the entry itself should have been cleared by now (see "Clear out any other TLB entries..." in arch/powerpc/cpu/mpc85xx/start.S).
In any case, the entry that we're describing in the comments should be the new entry which does not cover the boot page. We're no longer using the boot page at this point (on the boot core).
-Scott