powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

PATCH New Prabhakar Kushwaha 2 participants 8 messages Jun 25, 2013 – Jul 1, 2013 PowerPC

lore

Share & tools

lore

Prabhakar Kushwaha

author

2013-06-25 05:03 UTC #0 lore raw

PATCH 1/1 powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

CONFIG_SPL_BUILD creates debug TLB entry, so disable it before init_tlbs.

CONFIG_SPL_INIT_MINIMAL never creates any debug TLB entry, so no need of disable_tlb().

Signed-off-by Prabhakar Kushwaha <[email protected]>

---
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -1

index 837c034..0cb2717 100644
@@ -180,7 +180,8 @@ void cpu_init_early_f(void)
 
 	invalidate_tlb(1);
 
-#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NAND_SPL)
+#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && \
+		!defined(CONFIG_SPL_INIT_MINIMAL) && !defined(CONFIG_NAND_SPL)
 	disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
 #endif

Signature · 1 line

1.7.9.5

Scott Wood

2013-06-25 15:08 UTC #1 lore raw

PATCH 1/1 powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

Quoted 26 lines · Prabhakar Kushwaha

CONFIG_SPL_BUILD creates debug TLB entry, so disable it before init_tlbs.
CONFIG_SPL_INIT_MINIMAL never creates any debug TLB entry, so no need of disable_tlb().
Signed-off-by Prabhakar Kushwaha <[email protected]>
---
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -1
index 837c034..0cb2717 100644
@@ -180,7 +180,8 @@ void cpu_init_early_f(void)
 
 	invalidate_tlb(1);
 
-#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&  
 !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NAND_SPL)
+#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && \
+		!defined(CONFIG_SPL_INIT_MINIMAL) &&  
 !defined(CONFIG_NAND_SPL)
 	disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);

Quoted 1 line

#endif

This fails to clear the debug TLB in non-SPL_BUILD portion of a build that has CONFIG_SPL_INIT_MINIMAL set.

-Scott

Scott Wood

2013-06-26 19:06 UTC #3 lore raw

PATCH 1/1 powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

Quoted 37 lines · Prabhakar Kushwaha

Quoted 22 lines · Scott Wood
Quoted 21 lines · Prabhakar Kushwaha
CONFIG_SPL_BUILD creates debug TLB entry, so disable it before init_tlbs.
CONFIG_SPL_INIT_MINIMAL never creates any debug TLB entry, so no need
of disable_tlb().
Signed-off-by Prabhakar Kushwaha <[email protected]>
---
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +0 -0
index 837c034..0cb2717 100644
@@ -180,7 +180,8 @@ void cpu_init_early_f(void)
 
     invalidate_tlb(1);
 
Quoted 10 lines
Quoted 7 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -1
-#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&  
 !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NAND_SPL)
+#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && \
+        !defined(CONFIG_SPL_INIT_MINIMAL) &&  
 !defined(CONFIG_NAND_SPL)
     disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
#endif
This fails to clear the debug TLB in non-SPL_BUILD portion of a build that has CONFIG_SPL_INIT_MINIMAL setoh..
oh.. lots of cases :(
then it should be like this. slightly complex.
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&

Quoted 7 lines

!(defined(CONFIG_NAND_SPL) || \
    (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL)))

or
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && (CONFIG_SYS_RAMBOOT) ||

\
    (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_INIT_MINIMAL)))

The former:

http://lists.denx.de/pipermail/u-boot/2013-June/157201.html

Ignore the line wrapping.

The latter will only clear the debug TLB when CONFIG_SYS_RAMBOOT is set or in a non-minimal SPL (i.e. not in an ordinary NOR boot), which I don't think is what you meant. Check your parentheses. :-)

If you meant to exclude CONFIG_SYS_RAMBOOT, why?

-Scott

Prabhakar Kushwaha

author

2013-06-28 09:05 UTC #4 lore raw

PATCH 1/1 powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

Quoted 40 lines · Scott Wood

Quoted 39 lines · Prabhakar Kushwaha
Quoted 21 lines · Scott Wood
Quoted 20 lines · Prabhakar Kushwaha
CONFIG_SPL_BUILD creates debug TLB entry, so disable it before init_tlbs.
CONFIG_SPL_INIT_MINIMAL never creates any debug TLB entry, so no need of disable_tlb().
Signed-off-by Prabhakar Kushwaha <[email protected]>
---
 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +0 -0
index 837c034..0cb2717 100644
@@ -180,7 +180,8 @@ void cpu_init_early_f(void)
 
     invalidate_tlb(1);
 
Quoted 10 lines
Quoted 7 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +2 -1
-#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && 
 !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NAND_SPL)
+#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && \
+        !defined(CONFIG_SPL_INIT_MINIMAL) && 
 !defined(CONFIG_NAND_SPL)
     disable_tlb(CONFIG_SYS_PPC_E500_DEBUG_TLB);
#endif
This fails to clear the debug TLB in non-SPL_BUILD portion of a build that has CONFIG_SPL_INIT_MINIMAL setoh..
oh.. lots of cases :(
then it should be like this. slightly complex.
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
!(defined(CONFIG_NAND_SPL) || \
    (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL)))

Quoted 8 lines

Quoted 3 lines
or
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && (CONFIG_SYS_RAMBOOT) || \
    (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_INIT_MINIMAL)))
The former:
http://lists.denx.de/pipermail/u-boot/2013-June/157201.html

The defines mentioned at this link and earlier written by me will not work as they are not taking care of SD boot and NOR boot scenario.

Below define will take care of all possible combination
- NOR , SD, SPI boot
- NAND_SPL
- SPL with SPL_INIT_MINIMAL
- SPL without SPL_INIT_MINIMAL

#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && !defined(CONFIG_NAND_SPL) && \
     (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_INIT_MINIMAL))

Took lot of time to figure out :)

Regards,
Prabhakar

Scott Wood

2013-06-28 16:19 UTC #5 lore raw

PATCH 1/1 powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

Quoted 13 lines · Prabhakar Kushwaha

Quoted 12 lines · Scott Wood
Quoted 10 lines · Prabhakar Kushwaha
then it should be like this. slightly complex.
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
!(defined(CONFIG_NAND_SPL) || \
    (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL)))
or
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && (CONFIG_SYS_RAMBOOT)
|| \
    (defined(CONFIG_SPL_BUILD) &&  
!defined(CONFIG_SPL_INIT_MINIMAL)))

Quoted 6 lines

Quoted 4 lines
The former:
http://lists.denx.de/pipermail/u-boot/2013-June/157201.html
The defines mentioned at this link and earlier written by me will not work as they are not taking care of SD boot and NOR boot scenario.

How so?

Quoted 11 lines

Below define will take care of all possible combination
- NOR , SD, SPI boot
- NAND_SPL
- SPL with SPL_INIT_MINIMAL
- SPL without SPL_INIT_MINIMAL
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
!defined(CONFIG_NAND_SPL) && \
    (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_INIT_MINIMAL))
Took lot of time to figure out :)

That's equivalent to what I suggested in the above link. You just replaced !(x && y) with (!x || !y).

Or relative to your original suggestion above, you replaced !(x || (y && z)) with !x && (!y || !z).

They're all logically equivalent[1]. The question is just which one is most readable/intuitive.

-Scott

[1] http://en.wikipedia.org/wiki/De_Morgan%27s_laws

Prabhakar Kushwaha

author

2013-07-01 08:56 UTC #6 lore raw

PATCH 1/1 powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

Quoted 34 lines · Scott Wood

Quoted 12 lines · Prabhakar Kushwaha
Quoted 11 lines · Scott Wood
Quoted 9 lines · Prabhakar Kushwaha
then it should be like this. slightly complex.
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
!(defined(CONFIG_NAND_SPL) || \
    (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL)))
or
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && (CONFIG_SYS_RAMBOOT)
|| \
    (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_INIT_MINIMAL)))
Quoted 6 lines
Quoted 4 lines
The former:
http://lists.denx.de/pipermail/u-boot/2013-June/157201.html
The defines mentioned at this link and earlier written by me will not work as they are not taking care of SD boot and NOR boot scenario.
How so?
Quoted 11 lines
Below define will take care of all possible combination
- NOR , SD, SPI boot
- NAND_SPL
- SPL with SPL_INIT_MINIMAL
- SPL without SPL_INIT_MINIMAL
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
!defined(CONFIG_NAND_SPL) && \
    (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_INIT_MINIMAL))
Took lot of time to figure out :)

Quoted 8 lines

That's equivalent to what I suggested in the above link. You just replaced !(x && y) with (!x || !y).
Or relative to your original suggestion above, you replaced !(x || (y && z)) with !x && (!y || !z).
They're all logically equivalent[1]. The question is just which one is most readable/intuitiv

We have below mentioned issues with defines :-

A) Earlier suggested by me
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && !(defined(CONFIG_NAND_SPL)

|| \
     (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL)))

This will cause compile failure in SPL build.
in SPL build, CONFIG_NAND_SPL is not defined hence "1" making disable_tlb() present. As SPL + SPL_INIT_MINIMAL does not have TLB related functions means compilation error.

B) Define suggested at this link has "http://lists.denx.de/pipermail/u-boot/2013-June/157201.html "

#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && \
     !(defined(CONFIG_SPL_INIT_MINIMAL) && defined(CONFIG_SPL_BUILD)) && \
     !defined(CONFIG_NAND_SPL)

here, during SD & NOR boot, CONFIG_SPL_BUILD is not defined. hece complete define = 0. this make disable_tlb not compile which is wrong.

Only below define solve all the use cases.
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && !defined(CONFIG_NAND_SPL)

&& \
     (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_INIT_MINIMAL))

Please suggest.

Regards,
Prabhakar

Scott Wood

2013-07-01 17:14 UTC #7 lore raw

PATCH 1/1 powerpc/mpc85xx:Disable Debug TLB entry for non-minimal SPL

Quoted 53 lines · Prabhakar Kushwaha

Quoted 38 lines · Scott Wood
Quoted 14 lines · Prabhakar Kushwaha
Quoted 13 lines · Scott Wood
Quoted 11 lines · Prabhakar Kushwaha
then it should be like this. slightly complex.
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
!(defined(CONFIG_NAND_SPL) || \
    (defined(CONFIG_SPL_BUILD) &&  
defined(CONFIG_SPL_INIT_MINIMAL)))
or
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
(CONFIG_SYS_RAMBOOT) || \
    (defined(CONFIG_SPL_BUILD) &&  
!defined(CONFIG_SPL_INIT_MINIMAL)))
Quoted 7 lines
Quoted 4 lines
The former:
http://lists.denx.de/pipermail/u-boot/2013-June/157201.html
The defines mentioned at this link and earlier written by me will not work as they are not taking care of SD boot and NOR boot scenario.
How so?
Quoted 6 lines
Below define will take care of all possible combination
- NOR , SD, SPI boot
- NAND_SPL
- SPL with SPL_INIT_MINIMAL
- SPL without SPL_INIT_MINIMAL
Quoted 6 lines
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&
!defined(CONFIG_NAND_SPL) && \
    (!defined(CONFIG_SPL_BUILD) ||  
!defined(CONFIG_SPL_INIT_MINIMAL))
Took lot of time to figure out :)
Quoted 8 lines
That's equivalent to what I suggested in the above link. You just replaced !(x && y) with (!x || !y).
Or relative to your original suggestion above, you replaced !(x || (y && z)) with !x && (!y || !z).
They're all logically equivalent[1]. The question is just which one is most readable/intuitiv
We have below mentioned issues with defines :-
A) Earlier suggested by me
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) &&

Quoted 7 lines

!(defined(CONFIG_NAND_SPL) || \
    (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL)))
This will cause compile failure in SPL build.
in SPL build, CONFIG_NAND_SPL is not defined hence "1" making disable_tlb() present. As SPL + SPL_INIT_MINIMAL does not have TLB related functions means compilation error.

In that case CONFIG_SPL_BUILD and CONFIG_SPL_INIT_MINIMAL will be defined, which means (defined(CONFIG_NAND_SPL) || (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_INIT_MINIMAL))) will be true, which means the negation will be false, which means the toplevel conjunction will be false, and disable_tlb() will not be called.

Again, unless I'm misreading something, this is 100% logically equivalent to your final suggestion. I tried copy-and-pasting the above and had no problem building P2020RDB-PC_NAND.

Quoted 10 lines

B) Define suggested at this link has "http://lists.denx.de/pipermail/u-boot/2013-June/157201.html "
#if defined(CONFIG_SYS_PPC_E500_DEBUG_TLB) && \
    !(defined(CONFIG_SPL_INIT_MINIMAL) && defined(CONFIG_SPL_BUILD))  
&& \
!defined(CONFIG_NAND_SPL)
here, during SD & NOR boot, CONFIG_SPL_BUILD is not defined. hece complete define = 0. this make disable_tlb not compile which is wrong.

If CONFIG_SPL_BUILD is not defined, then !(anything && defined(CONFIG_SPL_BUILD)) will be true. Since the other terms of the toplevel conjunction will also be true, disable_tlb will be compiled.

-Scott