[U-Boot] PPC405EX CHIP_21 erratum

[U-Boot] PPC405EX CHIP_21 erratum

[Steven A. Falco]

We've been bitten by the PPC405EX CHIP_21 erratum.  I've looked through the
U-Boot code, but it doesn't appear that there is a work-around for this one.

The following patch is my adaptation of AMCC's suggestion as to the fix.  But
I have to say that I don't care for it, if for no other reason that it will
break if a rev E comes out.

Also, I made it #ifdef'd on CONFIG_405EX, but it will not work on a 405EXr.
And I have seen a 405EX report its PVR as if it were a 405EXr.  It's not
looking good for the suggested work-around.

AMCC claims this can be fixed in hardware, by always doing a double reset.
Naturally, that is hard to implement and would mean massive rework of existing
boards.

So I am contemplating a different work-around, whereby software always resets
the board on a cold boot (if such a thing can be reliably detected).  That would
hopefully be the equivalent of the hardware double reset, and would not be
dependent on specific PVR values, making it more "future proof".

Has anyone else run across this?  Do you have similar concerns about the patch?

Patch below is for discussion only.  Do not apply.

Signed-off-by: Steve Falco <sfalco@harris.com>

---

--- /home/sfalco/start.S.orig	2011-04-29 12:38:20.000000000 -0400
+++ arch/powerpc/cpu/ppc4xx/start.S	2011-04-29 12:38:49.000000000 -0400
@@ -1184,6 +1184,31 @@
 #else
 	GET_GOT			/* initialize GOT access			*/
 
+#ifdef CONFIG_405EX
+/* Errata CHIP_21 */
+	mfspr r3, PVR
+	lis r0,0x1291
+	ori r0,r0,0x1475
+	cmpw r0,r3,r0
+	beq r0,..GoodPVR /*Is it a 405EX REV D with security? */
+	lis r0,0x1291
+	ori r0,r0,0x1473
+	cmpw r0,r3,r0
+	beq r0,..GoodPVR /*Is it a 405EX REV D with without security? */
+	lis r0,0x1291
+	ori r0,r0,0x147F
+	cmpw r0,r3,r0
+	beq r0,..GoodPVR /*Is it a 405EX REV C with security? */
+	lis r0,0x1291
+	ori r0,r0,0x147D
+	cmpw r0,r3,r0
+	beq r0,..GoodPVR /*Is it a 405EX REV C with without security? */
+	lis r3,0x3000 /*DBCR0[RST]=0b11 */
+	ori r3,r3,0x0000
+	mtspr SPRN_DBCR0,r3
+..GoodPVR:
+#endif
+
 	bl	cpu_init_f	/* run low-level CPU init code	   (from Flash) */
 
 	bl	board_init_f	/* run first part of init code (from Flash)	*/

[U-Boot] PPC405EX CHIP_21 erratum

[Stefan Roese]

Hi Steve,

On Friday 29 April 2011 18:54:02 Steven A. Falco wrote:
> We've been bitten by the PPC405EX CHIP_21 erratum.

How did it affect you exactly? Was an incorrect PVR detected? Did this result 
in some problems (Linux etc)?

> I've looked through the
> U-Boot code, but it doesn't appear that there is a work-around for this
> one.

Correct, there currently is no workaround.
 
> The following patch is my adaptation of AMCC's suggestion as to the fix. 
> But I have to say that I don't care for it, if for no other reason that it
> will break if a rev E comes out.
> 
> Also, I made it #ifdef'd on CONFIG_405EX, but it will not work on a 405EXr.
> And I have seen a 405EX report its PVR as if it were a 405EXr.  It's not
> looking good for the suggested work-around.
>
> AMCC claims this can be fixed in hardware, by always doing a double reset.
> Naturally, that is hard to implement and would mean massive rework of
> existing boards.

Yes, there are many 405EX(r) boards in the field already. Such an hardware 
workaround should really be avoided if possible.

> So I am contemplating a different work-around, whereby software always
> resets the board on a cold boot (if such a thing can be reliably
> detected).  That would hopefully be the equivalent of the hardware double
> reset, and would not be dependent on specific PVR values, making it more
> "future proof".
> 
> Has anyone else run across this?  Do you have similar concerns about the
> patch?

No, I have not seen this problem before. And yes, I would also prefer your 
"alternative suggestion", with the always reboot after powerup optionally 
built into the U-Boot image. But as you already mentioned, we would have to 
find a way to reliably detect the powerup reset, so that we don't end in an 
reset-loop.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] PPC405EX CHIP_21 erratum

[Steven A. Falco]

> Hi Steve,
> 
> On Friday 29 April 2011 18:54:02 Steven A. Falco wrote:
>> We've been bitten by the PPC405EX CHIP_21 erratum.
> 
> How did it affect you exactly? Was an incorrect PVR detected? Did this result 
> in some problems (Linux etc)?

The problem goes way beyond the PVR value.  We have been getting "hangs" at
the point that U-Boot is printing out the results of its PCIe discovery.

We would get "PCIE1: successfully set as root-complex" and the board would hang
forever.  We traced this to an infinite loop in 4xx_pcie.c, which we "fixed" as
follows:

@ -724,8 +724,21 @@ int __ppc4xx_init_pcie_port_hw(int port, int rootport)
 		SDR_WRITE(SDRN_PESDR_RCSSET(port), 0x01101000);
 
 	/* poll for phy !reset */
-	while (!(SDR_READ(SDRN_PESDR_PHYSTA(port)) & 0x00001000))
-		;
+    {
+        int i = 0;
+
+        while (!(SDR_READ(SDRN_PESDR_PHYSTA(port)) & 0x00001000))
+        {
+            udelay(1000);
+
+            if (i++ > 2000)
+            {
+                printf("PCIE%d phy reset timed out, PHYSTA = 0x%08x\n",
+                       port, SDR_READ(SDRN_PESDR_PHYSTA(port)));
+                break;
+            }
+        }
+    }
 
 	/* deassert the PE0_gpl_utl_reset */
 	SDR_WRITE(SDRN_PESDR_RCSSET(port), 0x00101000);

Basically, when we see the PVR issue, we also see that the PCIe phy never
becomes ready.  With the patch above, we would not hang in U-Boot.  Instead
we'd move on to load Linux, which would crash with an Oops.  The board
would then reboot, and begin running normally.  This behavior is consistent
with CHIP_21, which implies that on the second reboot the chip works properly.

Note that this only happens with a few of our boards, and not 100%.  We
have some boards that are particularly bad, and others that always work
perfectly.

We contacted AMCC support.  Their reply included the following:

	there was an errata for REVD in 405EX which required soft reset
	for PCIE to work correctly. Maybe this is a reason. The errata is

	CHIP_21: Incorrect Value read from the SDR0_ECID0:3 and PVR registers.

	And it affects PCIE reset as well.

So apparently, this is a serious bug that really needs a workaround.  Now the
question is what is the best way to work around it.

> 
>> I've looked through the
>> U-Boot code, but it doesn't appear that there is a work-around for this
>> one.
> 
> Correct, there currently is no workaround.
>  
>> The following patch is my adaptation of AMCC's suggestion as to the fix. 
>> But I have to say that I don't care for it, if for no other reason that it
>> will break if a rev E comes out.
>> 
>> Also, I made it #ifdef'd on CONFIG_405EX, but it will not work on a 405EXr.
>> And I have seen a 405EX report its PVR as if it were a 405EXr.  It's not
>> looking good for the suggested work-around.
>>
>> AMCC claims this can be fixed in hardware, by always doing a double reset.
>> Naturally, that is hard to implement and would mean massive rework of
>> existing boards.
> 
> Yes, there are many 405EX(r) boards in the field already. Such an hardware 
> workaround should really be avoided if possible.
> 
>> So I am contemplating a different work-around, whereby software always
>> resets the board on a cold boot (if such a thing can be reliably
>> detected).  That would hopefully be the equivalent of the hardware double
>> reset, and would not be dependent on specific PVR values, making it more
>> "future proof".
>> 
>> Has anyone else run across this?  Do you have similar concerns about the
>> patch?
> 
> No, I have not seen this problem before. And yes, I would also prefer your 
> "alternative suggestion", with the always reboot after powerup optionally 
> built into the U-Boot image. But as you already mentioned, we would have to 
> find a way to reliably detect the powerup reset, so that we don't end in an 
> reset-loop.

I have not been able to find a way to distinguish a power-up versus a reboot.
So for now, I've implemented the AMCC bug fix more or less as written.  To do
it right, we'd have to add entries in every U-Boot config file for the
405 EX/EXr to definitively specify the CPU type expected, and to put the
list of allowed PVRs in the code.

This means that the PVR is essentially useless to distinguish between the
EX and EXr, because we have seen one misbehaving EX report as an EXr!
If the PVR had been garbage, we could maybe use it, but reporting "wrong yet
legal" values makes it worthless.

All our boards use the EX, so we are able to live with this, but I realize
it is not a solution for the larger community.  That is why I have not
proposed a definitive patch.

	Steve

> 
> Cheers,
> Stefan
> 
> --
> DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de
> 


-- 
A: Because it makes the logic of the discussion difficult to follow.
Q: Why shouldn't I top post?
A: No.
Q: Should I top post?

[U-Boot] PPC405EX CHIP_21 erratum

[Stefan Roese]

On Monday 02 May 2011 23:44:58 Steven A. Falco wrote:
> I have not been able to find a way to distinguish a power-up versus a
> reboot. So for now, I've implemented the AMCC bug fix more or less as
> written.  To do it right, we'd have to add entries in every U-Boot config
> file for the 405 EX/EXr to definitively specify the CPU type expected, and
> to put the list of allowed PVRs in the code.
> 
> This means that the PVR is essentially useless to distinguish between the
> EX and EXr, because we have seen one misbehaving EX report as an EXr!
> If the PVR had been garbage, we could maybe use it, but reporting "wrong
> yet legal" values makes it worthless.
> 
> All our boards use the EX, so we are able to live with this, but I realize
> it is not a solution for the larger community.  That is why I have not
> proposed a definitive patch.

I see. Thanks for the explanation.

IMHO, it would be best to provide a default list of PVR's that are 
allowed/correct for 405EX(r). And this list can be overridden by a board 
specific version (as needed in your case). Not sure how to handle this 
elegantly in assembler though.

We might move this to cpu_init_f(). This is the first C function called via 
start.S. It already handles similar issues for some 4xx SoC variants 
(reconfigure_pll()) which require a system reboot.

What do you think? Would you be able to prepare a patch with such a CHIP_21 
fix?

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de