powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

PATCH Superseded Prabhakar Kushwaha 3 participants 9 messages Jun 13, 2013 – Jun 25, 2013 PowerPC

lore

Share & tools

lore

Prabhakar Kushwaha

author

2013-06-13 04:44 UTC #0 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

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 <[email protected]>

---
changes for v2: Fixed checkpatch warning.

 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |    4 ++++
 1 file changed, 4 insertions(+)

arch/powerpc/cpu/mpc85xx/cpu_init_early.c +4 -0

index f4403c2..340b063 100644
@@ -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
+
 	init_tlbs();
 }

Signature · 1 line

1.7.9.5

Andy Fleming

Applied

2013-06-21 20:38 UTC #1 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 5 lines · Prabhakar Kushwaha

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 <[email protected]>

Applied, with fixes.

arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 185

Quoted 11 lines

arch/powerpc/cpu/mpc85xx/cpu_init_early.c +3 -0

index f4403c2..340b063 100644
@@ -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

Scott Wood

2013-06-21 20:59 UTC #2 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 16 lines · Andy Fleming

Quoted 5 lines · Prabhakar Kushwaha
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 <[email protected]>
Applied, with fixes.
Review
Quoted 1 line
(quoted diff) +0 -0
b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
Quoted 5 lines
index f4403c2..340b063 100644
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +0 -0
@@ -180,5 +180,9 @@ void cpu_init_early_f(void)
 

Quoted 8 lines

arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void)
Quoted 3 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0
 	invalidate_tlb(1);
 
+#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&  
!defined(CONFIG_SPL_BUILD)
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 185
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -0
+	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.

-Scott

Fleming Andy-AFLEMING

2013-06-21 21:05 UTC #3 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 14 lines · Scott Wood

Quoted 13 lines · Andy Fleming
Quoted 5 lines · Prabhakar Kushwaha
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 <[email protected]>
Applied, with fixes.
Quoted 6 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +0 -0
index f4403c2..340b063 100644
@@ -180,5 +180,9 @@ void cpu_init_early_f(void)
 

Quoted 8 lines

Quoted 6 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 185
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +3 -0
 	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. It may be that the error lies the other way around, but this fixed my build problem.

Andy

Scott Wood

2013-06-21 21:12 UTC #4 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 23 lines · Fleming Andy-AFLEMING

Quoted 3 lines · Scott Wood
Quoted 2 lines · Andy Fleming
Quoted 1 line · Prabhakar Kushwaha
init_tlbs() initialize all the TLB entries required for the
system.
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
So disable DEBUG TLB entry before TLB entries initialization.
Signed-off-by Prabhakar Kushwaha <[email protected]>
Applied, with fixes.
Review
Quoted 1 line
(quoted diff) +0 -0
b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
Quoted 8 lines
Quoted 8 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 183
Quoted 8 lines
index f4403c2..340b063 100644
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0
@@ -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)

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 185
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -0
+	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.

-Scott

Prabhakar Kushwaha

author

2013-06-24 03:14 UTC #5 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 27 lines · Scott Wood

Quoted 26 lines · Fleming Andy-AFLEMING
Quoted 9 lines · Scott Wood
Quoted 8 lines · Andy Fleming
Quoted 5 lines · Prabhakar Kushwaha
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 <[email protected]>
Applied, with fixes.
Review
Quoted 1 line
(quoted diff) +0 -0
b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
Quoted 8 lines
Quoted 8 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 183
Quoted 8 lines
index f4403c2..340b063 100644
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0
@@ -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)
Quoted 4 lines
Quoted 3 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 185
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -0
+    disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
+#endif
Had to add CONFIG_NAND_SPL here, as well, just for future reference

Quoted 8 lines

Quoted 3 lines
Quoted 1 line
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?

Regards,
Prabhakar

Prabhakar Kushwaha

author

2013-06-24 06:17 UTC #6 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 37 lines · Prabhakar Kushwaha

Quoted 28 lines · Scott Wood
Quoted 27 lines · Fleming Andy-AFLEMING
Quoted 3 lines · Scott Wood
Quoted 2 lines · Andy Fleming
Quoted 1 line · Prabhakar Kushwaha
init_tlbs() initialize all the TLB entries required for the
system.
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
So disable DEBUG TLB entry before TLB entries initialization.
Signed-off-by Prabhakar Kushwaha <[email protected]>
Applied, with fixes.
Review
Quoted 1 line
(quoted diff) +0 -0
b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
Quoted 8 lines
Quoted 8 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 183
Quoted 8 lines
index f4403c2..340b063 100644
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0
@@ -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)
Quoted 4 lines
Quoted 3 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 185
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -0
+    disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
+#endif
Had to add CONFIG_NAND_SPL here, as well, just for future reference
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
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.

Quoted 9 lines

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?

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.

Regards,
Prabhakar

Scott Wood

2013-06-24 17:41 UTC #7 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 42 lines · Prabhakar Kushwaha

Quoted 41 lines · Prabhakar Kushwaha
Quoted 40 lines · Scott Wood
Quoted 26 lines · Fleming Andy-AFLEMING
Quoted 2 lines · Scott Wood
Quoted 1 line · Andy Fleming
On Thu, Jun 13, 2013 at 10:14:00AM +0530, Prabhakar Kushwaha
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
init_tlbs() initialize all the TLB entries required for the
system.
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
So disable DEBUG TLB entry before TLB entries initialization.
Signed-off-by Prabhakar Kushwaha <[email protected]>
Applied, with fixes.
Review
Quoted 1 line
(quoted diff) +0 -0
b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
Quoted 8 lines
Quoted 8 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 183
Quoted 8 lines
index f4403c2..340b063 100644
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0
@@ -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)
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -0
+    disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
+#endif
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Had to add CONFIG_NAND_SPL here, as well, just for future
reference
Quoted 2 lines
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.

Quoted 7 lines

Quoted 7 lines
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)

Quoted 4 lines

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?

-Scott

Prabhakar Kushwaha

author

2013-06-25 04:37 UTC #8 lore raw

PATCH v2 1/1 powerpc/mpc85xx:Disable Debug TLB entry before init_tlbs

Quoted 56 lines · Scott Wood

Quoted 41 lines · Prabhakar Kushwaha
Quoted 40 lines · Prabhakar Kushwaha
Quoted 39 lines · Scott Wood
Quoted 26 lines · Fleming Andy-AFLEMING
Quoted 2 lines · Scott Wood
Quoted 1 line · Andy Fleming
On Thu, Jun 13, 2013 at 10:14:00AM +0530, Prabhakar Kushwaha
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
init_tlbs() initialize all the TLB entries required for the
system.
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
So disable DEBUG TLB entry before TLB entries initialization.
Signed-off-by Prabhakar Kushwaha <[email protected]>
Applied, with fixes.
Review
Quoted 1 line
(quoted diff) +0 -0
b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
Quoted 8 lines
Quoted 8 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c void cpu_init_early_f(void) · line 183
Quoted 8 lines
index f4403c2..340b063 100644
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0
@@ -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)
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -0
+ disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
+#endif
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Had to add CONFIG_NAND_SPL here, as well, just for future
reference
Quoted 2 lines
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.
Quoted 7 lines
Quoted 7 lines
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)

Quoted 7 lines

Quoted 4 lines
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