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. :-) > >> > >> 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... > >> > >>> +#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. -Scott _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
