[U-Boot] [PATCH] powerpc/85xx: Add workaround for errata USB-14 (enable on P204x/P3041/P50x0)

On P204x/P304x/P50x0 Rev1.0, USB transmit will result in false internal multi-bit ECC errors, which has impact on performance, so software should disable all ECC reporting from USB1 and USB2 by setting bits 16 and 17 to 1 in the register at DCSRBASE + 0x0002_0520.
In formal release document, the errata number should be USB14 instead of USB138.
Signed-off-by: xulei Lei.Xu@freescale.com Signed-off-by: Roy Zang tie-fei.zang@freescale.com Signed-off-by: Kumar Gala galak@kernel.crashing.org --- arch/powerpc/cpu/mpc85xx/cmd_errata.c | 3 +++ arch/powerpc/cpu/mpc85xx/cpu_init.c | 13 +++++++++++++ arch/powerpc/include/asm/config_mpc85xx.h | 5 ++++- 3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c index 5d72f4c..422782c 100644 --- a/arch/powerpc/cpu/mpc85xx/cmd_errata.c +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -255,6 +255,9 @@ static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #ifdef CONFIG_SYS_P4080_ERRATUM_PCIE_A003 puts("Work-around for Erratum PCIe-A003 enabled\n"); #endif +#ifdef CONFIG_SYS_FSL_ERRATUM_USB14 + puts("Work-around for Erratum USB14 enabled\n"); +#endif return 0; }
diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c index de9d916..72c5328 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c @@ -623,6 +623,19 @@ skip_l2: } #endif
+ /* On P204x/P304x/P50x0 Rev1.0, USB transmit will result internal + * multi-bit ECC errors, which has impact on performance, so software + * should disable all ECC reporting from USB1 and USB2 by setting bits + * 16 and 17 to 1 in the register at DCSRBASE + 0x0002_0520. + */ +#ifdef CONFIG_SYS_FSL_ERRATUM_USB14 + if (IS_SVR_REV(get_svr(), 1, 0)) { + void *p; + p = (void *)CONFIG_SYS_DCSRBAR + 0x20520; + setbits_be32(p, 3 << (31 - 17)); + } +#endif + #ifdef CONFIG_FMAN_ENET fman_enet_init(); #endif diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h index d57c178..4236835 100644 --- a/arch/powerpc/include/asm/config_mpc85xx.h +++ b/arch/powerpc/include/asm/config_mpc85xx.h @@ -333,6 +333,7 @@ #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY #define CONFIG_SYS_FSL_ERRATUM_ESDHC111 #define CONFIG_SYS_FSL_ERRATUM_NMG_CPU_A011 +#define CONFIG_SYS_FSL_ERRATUM_USB14 #define CONFIG_SYS_FSL_ERRATUM_CPU_A003999 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003474 #define CONFIG_SYS_FSL_SRIO_PCIE_BOOT_MASTER @@ -365,6 +366,7 @@ #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY #define CONFIG_SYS_FSL_ERRATUM_ESDHC111 #define CONFIG_SYS_FSL_ERRATUM_NMG_CPU_A011 +#define CONFIG_SYS_FSL_ERRATUM_USB14 #define CONFIG_SYS_FSL_ERRATUM_CPU_A003999 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003474 #define CONFIG_SYS_FSL_SRIO_PCIE_BOOT_MASTER @@ -442,6 +444,7 @@ #define CONFIG_SYS_FSL_USB2_PHY_ENABLE #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY #define CONFIG_SYS_FSL_ERRATUM_ESDHC111 +#define CONFIG_SYS_FSL_ERRATUM_USB14 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003474 #define CONFIG_SYS_FSL_SRIO_PCIE_BOOT_MASTER #define CONFIG_SYS_FSL_SRIO_MAX_PORTS 2 @@ -473,7 +476,7 @@ #define CONFIG_SYS_FSL_USB2_PHY_ENABLE #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY #define CONFIG_SYS_FSL_ERRATUM_ESDHC111 -#define CONFIG_SYS_FSL_ERRATUM_USB138 +#define CONFIG_SYS_FSL_ERRATUM_USB14 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003474 #define CONFIG_SYS_FSL_ERRATUM_A004699

Dear xulei,
In message 1362368146-738-1-git-send-email-B33228@freescale.com you wrote:
- /* On P204x/P304x/P50x0 Rev1.0, USB transmit will result internal
* multi-bit ECC errors, which has impact on performance, so software
* should disable all ECC reporting from USB1 and USB2 by setting bits
* 16 and 17 to 1 in the register at DCSRBASE + 0x0002_0520.
*/
+#ifdef CONFIG_SYS_FSL_ERRATUM_USB14
- if (IS_SVR_REV(get_svr(), 1, 0)) {
void *p;
p = (void *)CONFIG_SYS_DCSRBAR + 0x20520;
setbits_be32(p, 3 << (31 - 17));
- }
+#endif
Please move the comment inside the #ifdef block.
Best regards,
Wolfgang Denk

Hello, Wolfgang
Thank you, will modify it in the next version.
Regards Lei
On Monday, 2013-03-04 at 10:24 +0100, Wolfgang Denk wrote:
Dear xulei,
In message 1362368146-738-1-git-send-email-B33228@freescale.com you wrote:
- /* On P204x/P304x/P50x0 Rev1.0, USB transmit will result internal
* multi-bit ECC errors, which has impact on performance, so software
* should disable all ECC reporting from USB1 and USB2 by setting bits
* 16 and 17 to 1 in the register at DCSRBASE + 0x0002_0520.
*/
+#ifdef CONFIG_SYS_FSL_ERRATUM_USB14
- if (IS_SVR_REV(get_svr(), 1, 0)) {
void *p;
p = (void *)CONFIG_SYS_DCSRBAR + 0x20520;
setbits_be32(p, 3 << (31 - 17));
- }
+#endif
Please move the comment inside the #ifdef block.
Best regards,
Wolfgang Denk

Dear xulei,
In message 1362368146-738-1-git-send-email-B33228@freescale.com you wrote:
On P204x/P304x/P50x0 Rev1.0, USB transmit will result in false internal multi-bit ECC errors, which has impact on performance, so software should disable all ECC reporting from USB1 and USB2 by setting bits 16 and 17 to 1 in the register at DCSRBASE + 0x0002_0520.
...
void *p;
p = (void *)CONFIG_SYS_DCSRBAR + 0x20520;
Um... and don't we have a proper C struct that describes the registers in this area?
We don't allow base address + offset addressing like this.
Best regards,
Wolfgang Denk

Hello, Wolfgang
Thank you and I agree with you. It is a little ugly but because these registers info are not publicly, so I did not use C struct to describe them, then for this case it it ok or other method such as define a struct but keep all other registers and bits in this register reserved? or any method better? Thank you.
Regards Lei
On Monday, 2013-03-04 at 10:26 +0100, Wolfgang Denk wrote:
Dear xulei,
In message 1362368146-738-1-git-send-email-B33228@freescale.com you wrote:
On P204x/P304x/P50x0 Rev1.0, USB transmit will result in false internal multi-bit ECC errors, which has impact on performance, so software should disable all ECC reporting from USB1 and USB2 by setting bits 16 and 17 to 1 in the register at DCSRBASE + 0x0002_0520.
...
void *p;
p = (void *)CONFIG_SYS_DCSRBAR + 0x20520;
Um... and don't we have a proper C struct that describes the registers in this area?
We don't allow base address + offset addressing like this.
Best regards,
Wolfgang Denk
participants (2)
-
Wolfgang Denk
-
xulei