[U-Boot] [PATCH] powerpc/mpc85xx: Make NMG_CPU_A011 workaround conditional

[U-Boot] [PATCH] powerpc/mpc85xx: Make NMG_CPU_A011 workaround conditional

[York Sun]

Workaround for erratum NMG_CPU_A011 can be enabled by hwconfig with syntax

fsl_cpu_a011:enable

This workaround is not enabled by default. Enabling this workaround may
degrade performance. P4080 erratum CPU22 shares the same workaround. So it
is always enabled for P4080.

Signed-off-by: York Sun <yorksun@freescale.com>
---
 arch/powerpc/cpu/mpc85xx/cmd_errata.c |    6 +++++-
 arch/powerpc/cpu/mpc85xx/cpu_init.c   |   24 +++++++++++++++++++++++-
 arch/powerpc/cpu/mpc85xx/release.S    |   15 +++++++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c
index 4e1a54a..858b3f8 100644
--- a/arch/powerpc/cpu/mpc85xx/cmd_errata.c
+++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c
@@ -27,6 +27,9 @@
 
 static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
+#ifdef CONFIG_SYS_FSL_ERRATUM_NMG_CPU_A011
+	extern int enable_cpu_a011_workaround;
+#endif
 	__maybe_unused u32 svr = get_svr();
 
 #if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001)
@@ -56,8 +59,9 @@ static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	/*
 	 * NMG_CPU_A011 applies to P4080 rev 1.0, 2.0, fixed in 3.0
 	 * also applies to P3041 rev 1.0, 1.1, P2041 rev 1.0, 1.1
+	 * The SVR has been checked by cpu_init_r().
 	 */
-	if (SVR_SOC_VER(svr) != SVR_P4080 || SVR_MAJ(svr) < 3)
+	if (enable_cpu_a011_workaround)
 		puts("Work-around for Erratum CPU-A011 enabled\n");
 #endif
 #if defined(CONFIG_SYS_FSL_ERRATUM_CPU_A003999)
diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
index 4f03d25..10f142e 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
@@ -38,6 +38,7 @@
 #include <asm/fsl_law.h>
 #include <asm/fsl_serdes.h>
 #include <asm/fsl_srio.h>
+#include <hwconfig.h>
 #include <linux/compiler.h>
 #include "mp.h"
 #ifdef CONFIG_SYS_QE_FMAN_FW_IN_NAND
@@ -47,6 +48,8 @@
 
 #include "../../../../drivers/block/fsl_sata.h"
 
+#define HWCONFIG_BUFFER_SIZE 128
+
 DECLARE_GLOBAL_DATA_PTR;
 
 #ifdef CONFIG_QE
@@ -311,11 +314,30 @@ int cpu_init_r(void)
 #if defined(CONFIG_SYS_P4080_ERRATUM_CPU22) || \
 	defined(CONFIG_SYS_FSL_ERRATUM_NMG_CPU_A011)
 	/*
+	 * CPU22 and NMG_CPU_A011 share the same workaround.
 	 * CPU22 applies to P4080 rev 1.0, 2.0, fixed in 3.0
 	 * NMG_CPU_A011 applies to P4080 rev 1.0, 2.0, fixed in 3.0
 	 * also applies to P3041 rev 1.0, 1.1, P2041 rev 1.0, 1.1
+	 * NMG_CPU_A011 is activated by hwconfig with syntax:
+	 * fsl_cpu_a011:enable
 	 */
-	if (SVR_SOC_VER(svr) != SVR_P4080 || SVR_MAJ(svr) < 3) {
+	extern int enable_cpu_a011_workaround;
+#ifdef CONFIG_SYS_P4080_ERRATUM_CPU22
+	enable_cpu_a011_workaround =
+		(SVR_SOC_VER(svr) != SVR_P4080 || SVR_MAJ(svr) < 3);
+#else
+	char buffer[HWCONFIG_BUFFER_SIZE];
+	char *buf = NULL;
+
+	if (getenv_f("hwconfig", buffer, sizeof(buffer)) > 0)
+		buf = buffer;
+
+	if (hwconfig_arg_cmp_f("fsl_cpu_a011", "enable", buf) > 0) {
+		enable_cpu_a011_workaround =
+			(SVR_SOC_VER(svr) != SVR_P4080 || SVR_MAJ(svr) < 3);
+	}
+#endif
+	if (enable_cpu_a011_workaround) {
 		flush_dcache();
 		mtspr(L1CSR2, (mfspr(L1CSR2) | L1CSR2_DCWS));
 		sync();
diff --git a/arch/powerpc/cpu/mpc85xx/release.S b/arch/powerpc/cpu/mpc85xx/release.S
index 1860684..b8d4a37 100644
--- a/arch/powerpc/cpu/mpc85xx/release.S
+++ b/arch/powerpc/cpu/mpc85xx/release.S
@@ -163,6 +163,12 @@ __secondary_start_page:
 	cmpw    r3,r5
 	bge     2f
 1:
+#ifdef	CONFIG_SYS_FSL_ERRATUM_NMG_CPU_A011
+	lis	r3,toreset(enable_cpu_a011_workaround)@ha
+	lwz	r3,toreset(enable_cpu_a011_workaround)@l(r3)
+	cmpwi	r3,0
+	beq	2f
+#endif
 	mfspr	r3,L1CSR2
 	oris	r3,r3,(L1CSR2_DCWS)@h
 	mtspr	L1CSR2,r3
@@ -346,6 +352,15 @@ __bootpg_addr:
 __spin_table:
 	.space CONFIG_MAX_CPUS*ENTRY_SIZE
 
+	/*
+	 * This variable is set by cpu_init_r() after parsing hwconfig
+	 * to enable workaround for erratum NMG_CPU_A011.
+	 */
+	.align L1_CACHE_SHIFT
+	.global enable_cpu_a011_workaround
+enable_cpu_a011_workaround:
+	.long	0
+
 	/* Fill in the empty space.  The actual reset vector is
 	 * the last word of the page */
 __secondary_start_code_end:
-- 
1.7.0.4

[U-Boot] [PATCH] powerpc/mpc85xx: Make NMG_CPU_A011 workaround conditional

[Andy Fleming]

Please copy me on any 85xx U-Boot patches.


>
> +#define HWCONFIG_BUFFER_SIZE 128


[...]

> +       char buffer[HWCONFIG_BUFFER_SIZE];
> +       char *buf = NULL;
> +
> +       if (getenv_f("hwconfig", buffer, sizeof(buffer)) > 0)
> +               buf = buffer;


This seems fragile. I suppose there's no way to ensure that this
succeeds, but we should at least check to see if the buffer we pass in
was big enough, and let the user know that we were looking for the
A011 erratum setting, and may not have found it because the hwconfig
string was more than 128 characters.


> +
> +       if (hwconfig_arg_cmp_f("fsl_cpu_a011", "enable", buf) > 0) {
> +               enable_cpu_a011_workaround =
> +                       (SVR_SOC_VER(svr) != SVR_P4080 || SVR_MAJ(svr) < 3);


Please merge this patch with its follow-on that changes the default to
be enabled.

Andy

[U-Boot] [PATCH] powerpc/mpc85xx: Make NMG_CPU_A011 workaround conditional

[York Sun]

On Wed, 2012-08-08 at 15:09 -0500, Andy Fleming wrote:
> Please copy me on any 85xx U-Boot patches.
> 
> 
> >
> > +#define HWCONFIG_BUFFER_SIZE 128
> 
> 
> [...]
> 
> > +       char buffer[HWCONFIG_BUFFER_SIZE];
> > +       char *buf = NULL;
> > +
> > +       if (getenv_f("hwconfig", buffer, sizeof(buffer)) > 0)
> > +               buf = buffer;
> 
> 
> This seems fragile. I suppose there's no way to ensure that this
> succeeds, but we should at least check to see if the buffer we pass in
> was big enough, and let the user know that we were looking for the
> A011 erratum setting, and may not have found it because the hwconfig
> string was more than 128 characters.
> 

If the buffer isn't big enough, getenv_f() will print a message saying
that. Is that enough? Or do you prefer another warning when fsl_cpu_a011
is not detected? It seems a little bit redundant.

York

[U-Boot] [PATCH] powerpc/mpc85xx: Make NMG_CPU_A011 workaround conditional

[Andy Fleming]

On Wed, Aug 8, 2012 at 6:16 PM, York Sun <yorksun@freescale.com> wrote:
> On Wed, 2012-08-08 at 15:09 -0500, Andy Fleming wrote:
>> Please copy me on any 85xx U-Boot patches.
>>
>>
>> >
>> > +#define HWCONFIG_BUFFER_SIZE 128
>>
>>
>> [...]
>>
>> > +       char buffer[HWCONFIG_BUFFER_SIZE];
>> > +       char *buf = NULL;
>> > +
>> > +       if (getenv_f("hwconfig", buffer, sizeof(buffer)) > 0)
>> > +               buf = buffer;
>>
>>
>> This seems fragile. I suppose there's no way to ensure that this
>> succeeds, but we should at least check to see if the buffer we pass in
>> was big enough, and let the user know that we were looking for the
>> A011 erratum setting, and may not have found it because the hwconfig
>> string was more than 128 characters.
>>
>
> If the buffer isn't big enough, getenv_f() will print a message saying
> that. Is that enough? Or do you prefer another warning when fsl_cpu_a011
> is not detected? It seems a little bit redundant.

Well, I think that the user will want to know that it's fsl_cpu_a011
that isn't being found. The code is able to know that, whereas the
user would have to dig around for a while to figure out what was
wrong. Especially if the user adds random stuff to the beginning, and
that causes the erratum variable to fall off the end.

Andy

[U-Boot] [PATCH] powerpc/mpc85xx: Make NMG_CPU_A011 workaround conditional

[Wolfgang Denk]

Dear York Sun,

In message <1344467781.18960.41.camel@oslab-l1> you wrote:
>
> If the buffer isn't big enough, getenv_f() will print a message saying

... but only if it can (i. e. if he call is _after_ the initialization
of the console driver has been completed (which is actually pretty
late).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Of course there's no reason for it, it's just our policy.