[U-Boot] [PATCH] POST: Added ECC memory test for mpc83xx.

[Michael Zaidman]

On Fri, Apr 16, 2010 at 1:25 AM, Kim Phillips
<kim.phillips at freescale.com> wrote:
> On Thu, 8 Apr 2010 10:37:08 +0200
> Joakim Tjernlund <joakim.tjernlund at transmode.se> wrote:
>
>> Kim Phillips <kim.phillips at freescale.com> wrote on 2010-04-08 10:27:03:
>> >
>> > The documentation is confusing: the e300c2 has its FPU chopped off -
>> > the FP registers are simply not there.
>> >
>> > this is a good catch by Jocke - it would be best if generic 83xx code
>> > didn't depend on the ppcDW* accessors.
>>
>> That or one could impl. ppcDW* using normal load/store insns for 832x.
>> Either way the ppcDW* should be inlined as the overhead for doing
>> function calls isn't something you want when looking for speed.
>
> another good point, but it seems they were added primarily for code
> density benefits.  I think we can do something like this in the
> meantime:
>
> From 686d3bb7a732ec36beec169c4eaf4882382d3aa2 Mon Sep 17 00:00:00 2001
> From: Kim Phillips <kim.phillips at freescale.com>
> Date: Thu, 8 Apr 2010 18:22:13 -0500
> Subject: [PATCH] mpc83xx: implement ppcDW{load,store} accessors for e300c2
>
> e300c2 core based processors (MPC832x) don't have an FPU: provide
> alternative, gpr based accessor functions for code compatibility.
>
> Suggested-by: Joakim Tjernlund <joakim.tjernlund at transmode.se>
> Signed-off-by: Kim Phillips <kim.phillips at freescale.com>
> ---
>  arch/ppc/cpu/mpc83xx/start.S |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/arch/ppc/cpu/mpc83xx/start.S b/arch/ppc/cpu/mpc83xx/start.S
> index 68bb620..6bfce57 100644
> --- a/arch/ppc/cpu/mpc83xx/start.S
> +++ b/arch/ppc/cpu/mpc83xx/start.S
> @@ -139,14 +139,28 @@ get_pvr:
>
>        .globl  ppcDWstore
>  ppcDWstore:
> +#if !defined(CONFIG_MPC832x)
>        lfd     1, 0(r4)
>        stfd    1, 0(r3)
> +#else
> +       lwz     r5, 0(r4)
> +       stw     r5, 0(r3)
> +       lwz     r5, 4(r4)
> +       stw     r5, 4(r3)
> +#endif
>        blr
>
>        .globl  ppcDWload
>  ppcDWload:
> +#if !defined(CONFIG_MPC832x)
>        lfd     1, 0(r3)
>        stfd    1, 0(r4)
> +#else
> +       lwz     r5, 0(r3)
> +       stw     r5, 0(r4)
> +       lwz     r5, 4(r3)
> +       stw     r5, 4(r4)
> +#endif
>        blr
>
>  #ifndef CONFIG_DEFAULT_IMMR
> --
> 1.7.0.5
>

Although this is good for most of the cases it does not fit in the
algorithm implemented in the post_ecc_test. The first stw in
ppcDWstore will generate ecc error (due to read-modify-write) so ecc
capture data registers will capture only first word as it was written
with flipped injected error bit or without depending on its position
in the data_error_inject_hi or data_error_inject_lo injection mask
registers. The second ecc capture data word will hold the data that
was in the memory right before the ppcDWstore call. Thus, the test
validation while working for stfd will fail for stw x 2. So, the
algorithm need to be reworked...

I also agree with Joakim regarding the routine call overhead and
replacing it by inline macro. Please review this code.