From mboxrd@z Thu Jan 1 00:00:00 1970 From: York Sun Date: Tue, 20 Aug 2013 13:26:04 -0700 Subject: [U-Boot] [PATCH v1 8/8] mpc85xx: introduce the kmp204x reference design support In-Reply-To: <1377028987.3370.41.camel@snotra.buserror.net> References: <1374832955-4544-1-git-send-email-valentin.longchamp@keymile.com> <1374832955-4544-9-git-send-email-valentin.longchamp@keymile.com> <1376429921.20487.144.camel@snotra.buserror.net> <52123EBD.80709@keymile.com> <1376959689.31636.39960101@freescale.com> <1377028987.3370.41.camel@snotra.buserror.net> Message-ID: <5213D0DC.5070408@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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: >>>>>>> >>>>>>> >>>>>>> >>>>>>>>>>> + /* 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. Along the booting process, we switch to AS=1 and replace the default TLB with this one, then switch back. That's why I said "replace". I am sure you are very familiar with this process. > > This entry is for L3SRAM which is 1 MiB at 0xf00000000 which is nowhere > near the boot page. It maps from CONFIG_SYS_INIT_L3_ADDR ( == CONFIG_RAMBOOT_TEXT_BASE == CONFIG_SYS_TEXT_BASE == 0xfff80000) to CONFIG_SYS_INIT_L3_ADDR_PHYS ( == 0xf00000000ull | CONFIG_RAMBOOT_TEXT_BASE). > >>>>>> >>>>>> 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. > > That entry would have already been replaced by asm code. No argument here. I was pointing out one case "I" and "G" don't go together. York