From mboxrd@z Thu Jan  1 00:00:00 1970
From: Prabhakar Kushwaha <prabhakar@freescale.com>
Date: Tue, 25 Jun 2013 10:07:10 +0530
Subject: [U-Boot] [U-Boot,
 v2] powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs
In-Reply-To: <1372095700.8183.2@snotra>
References: <1372095700.8183.2@snotra>
Message-ID: <51C91E76.2000504@freescale.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 06/24/2013 11:11 PM, Scott Wood wrote:
> On 06/24/2013 01:17:11 AM, Prabhakar Kushwaha wrote:
>> On 06/24/2013 08:44 AM, Prabhakar Kushwaha wrote:
>>> On 06/22/2013 02:42 AM, Scott Wood wrote:
>>>> On 06/21/2013 04:05:37 PM, Fleming Andy-AFLEMING wrote:
>>>>>
>>>>> On Jun 21, 2013, at 3:59 PM, Scott Wood wrote:
>>>>>
>>>>> > On 06/21/2013 03:38:48 PM, Andy Fleming wrote:
>>>>> >> On Thu, Jun 13, 2013 at 10:14:00AM +0530, Prabhakar Kushwaha 
>>>>> wrote:
>>>>> >> > init_tlbs() initialize all the TLB entries required for the 
>>>>> system.
>>>>> >> >
>>>>> >> > So disable DEBUG TLB entry before TLB entries initialization.
>>>>> >> >
>>>>> >> > Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
>>>>> >> Applied, with fixes.
>>>>> >> > diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c 
>>>>> b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>>>> >> > index f4403c2..340b063 100644
>>>>> >> > --- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>>>> >> > +++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>>>> >> > @@ -180,5 +180,9 @@ void cpu_init_early_f(void)
>>>>> >> >
>>>>> >> >      invalidate_tlb(1);
>>>>> >> >
>>>>> >> > +#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && 
>>>>> !defined(CONFIG_SPL_BUILD)
>>>>> >> > + disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
>>>>> >> > +#endif
>>>>> >> Had to add CONFIG_NAND_SPL here, as well, just for future 
>>>>> reference
>>>>> >
>>>>> > Why exclude all SPLs?  Only minimal SPLs skip creating the debug 
>>>>> TLB.
>>>>>
>>>>> The definition of disable_tlb() is excluded when NAND_SPL is defined.
>>>>
>>>> I'm talking about new-SPL, not NAND_SPL.  My comment is about the 
>>>> original patch (I made the comment before internally), not your 
>>>> edit to it.
>>>>
>>> as per comment from Scott, we should have below define. It takes 
>>> care of SPL minimal + nand_spl
>>>
>>> #if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && 
>>> !defined(CONFIG_SPL_INIT_MINIMAL) && !defined(CONFIG_NAND_SPL)
>>>
>>> May I resend this patch?
>
> It would need to be:
>
> #if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && \
>     !(defined(CONFIG_SPL_INIT_MINIMAL) && defined(CONFIG_SPL_BUILD)) && \
>     !defined(CONFIG_NAND_SPL)
>
>> just a afterthought, current modified* patch is correct i.e. " #if 
>> defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && !defined(CONFIG_SPL_BUILD) 
>> && !defined(CONFIG_NAND_SPL)"
>> DEBUG TLB never been created for SPL & NAND_SPL so no need to disable.
>
> Why would the debug TLB not be created for a non-minimal-init SPL?
>
Looks like my assumption is *wrong*.  as per this define

#if defined(CONFIG_NAND_SPL) || \
     (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL))
#define MINIMAL_SPL
#endif
MINIMAL_SPL is defined only with CONFIG_SPL_INIT_MINIMAL. for 
MINIMAL_SPL interrupt vectors are never relocated. hence,  no need of  
DEBUG TLB entry.

for rest, interrupt vectors are relocated, so debug tlb entry is must.

I will post a patch upstream to fix this problem.

Thanks,
Prabhakar