[PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B

[PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]


MPC5200 needs to have pipelining disabled for ATA to work. MPC5200B does not.
So, for the latter, don't touch the original setting from the bootloader.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
Hello Arnd,

On Mon, Aug 18, 2008 at 12:49:36PM +0200, Arnd Bergmann wrote:

> Please make this a run-time conditional instead of compile-time.
Like this?

..................................................................

This needs some testing IMHO. Most configs in U-Boot tend to enable pipelining,
which then used to be disabled by the kernel. So, on the one hand, this change
would enable what is originally wanted; on the other hand, systems may run
under a new configuration and need to be checked for regressions. Especially as
there can be puzzling effects, like for one setup here, FEC only works reliably
with pipelining enabled.

 arch/powerpc/platforms/52xx/mpc52xx_common.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: arch/powerpc/platforms/52xx/mpc52xx_common.c
===================================================================
--- arch/powerpc/platforms/52xx/mpc52xx_common.c.orig
+++ arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -99,11 +99,14 @@
 	out_be32(&xlb->master_pri_enable, 0xff);
 	out_be32(&xlb->master_priority, 0x11111111);
 
-	/* Disable XLB pipelining
+	/*
+	 * Disable XLB pipelining
 	 * (cfr errate 292. We could do this only just before ATA PIO
 	 *  transaction and re-enable it afterwards ...)
+	 * Not needed on MPC5200B.
 	 */
-	out_be32(&xlb->config, in_be32(&xlb->config) | MPC52xx_XLB_CFG_PLDIS);
+	if ((mfspr(SPRN_SVR) & MPC5200_SVR_MASK) == MPC5200_SVR)
+		out_be32(&xlb->config, in_be32(&xlb->config) | MPC52xx_XLB_CFG_PLDIS);
 
 	iounmap(xlb);
 }

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B

[Grant Likely]

On Mon, Aug 18, 2008 at 8:18 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>
> MPC5200 needs to have pipelining disabled for ATA to work. MPC5200B does not.
> So, for the latter, don't touch the original setting from the bootloader.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>

Thanks Wolfram, I'll pick this one up.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B

[roger blofeld]

----- Original Message ----

> From: Wolfram Sang <w.sang@pengutronix.de>
> To: Arnd Bergmann <arnd@arndb.de>
> Cc: linuxppc-embedded@ozlabs.org
> Sent: Monday, August 18, 2008 9:18:31 AM
> Subject: [PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B
> 
> 
> MPC5200 needs to have pipelining disabled for ATA to work. MPC5200B does not.
> So, for the latter, don't touch the original setting from the bootloader.
> 
> Signed-off-by: Wolfram Sang 
> ---
> Hello Arnd,
> 
> On Mon, Aug 18, 2008 at 12:49:36PM +0200, Arnd Bergmann wrote:
> 
> > Please make this a run-time conditional instead of compile-time.
> Like this?
> 
> ..................................................................
> 
> This needs some testing IMHO. Most configs in U-Boot tend to enable pipelining,
> which then used to be disabled by the kernel. So, on the one hand, this change
> would enable what is originally wanted; on the other hand, systems may run
> under a new configuration and need to be checked for regressions. Especially as
> there can be puzzling effects, like for one setup here, FEC only works reliably
> with pipelining enabled.
> 
> arch/powerpc/platforms/52xx/mpc52xx_common.c |    7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
> 
> Index: arch/powerpc/platforms/52xx/mpc52xx_common.c
> ===================================================================
> --- arch/powerpc/platforms/52xx/mpc52xx_common.c.orig
> +++ arch/powerpc/platforms/52xx/mpc52xx_common.c
> @@ -99,11 +99,14 @@
>     out_be32(&xlb->master_pri_enable, 0xff);
>     out_be32(&xlb->master_priority, 0x11111111);
> 
> -    /* Disable XLB pipelining
> +    /*
> +     * Disable XLB pipelining
>      * (cfr errate 292. We could do this only just before ATA PIO
>      *  transaction and re-enable it afterwards ...)
> +     * Not needed on MPC5200B.
>      */
> -    out_be32(&xlb->config, in_be32(&xlb->config) | MPC52xx_XLB_CFG_PLDIS);
> +    if ((mfspr(SPRN_SVR) & MPC5200_SVR_MASK) == MPC5200_SVR)
> +        out_be32(&xlb->config, in_be32(&xlb->config) | MPC52xx_XLB_CFG_PLDIS);
> 
>     iounmap(xlb);
> }
> 
> -- 
>   Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
> Pengutronix - Linux Solutions for Science and Industry


Hi
 Since this bug is ATA specific, shouldn't this code be conditioned by CONFIG_IDE ?

Thanks
-roger


      

Re: [PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B

[Scott Wood]

roger blofeld wrote:
> Hi
>  Since this bug is ATA specific, shouldn't this code be conditioned by CONFIG_IDE ?

And then what happens if IDE is built as a module?

-Scott

Re: [PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B

[roger blofeld]

> From: Scott Wood <scottwood@freescale.com>
> To: roger blofeld <blofeldus@yahoo.com>
> Cc: Wolfram Sang <w.sang@pengutronix.de>; Arnd Bergmann <arnd@arndb.de>; linuxppc-embedded@ozlabs.org
> Sent: Monday, August 25, 2008 10:36:48 AM
> Subject: Re: [PATCH V2] MPC52XX: Don't touch pipelining for MPC5200B
> 
> roger blofeld wrote:
> > Hi
> >  Since this bug is ATA specific, shouldn't this code be conditioned by 
> CONFIG_IDE ?
> 
> And then what happens if IDE is built as a module?
> 
> -Scott

Good point. Still it seems unfortunate to slow everything down when only ATA requires the fix. I'll just continue to disable this code on my board which has no IDE.

Thanks
-rb