| Message ID | 1312555480-13401-8-git-send-email-galak@kernel.crashing.org |
|---|---|
| State | Accepted |
| Commit | bc6bbd6be85973359e89f53e3bfbba2a3549da09 |
| Delegated to: | Kumar Gala |
| Headers | show |
Dear Kumar Gala, In message <1312555480-13401-8-git-send-email-galak@kernel.crashing.org> you wrote: > From: Poonam Aggrwal <poonam.aggrwal@freescale.com> > > Issue: Address masking doesn't work properly. > When sum of the base address, defined by BA, and memory bank size, > defined by AM, exceeds 4GB (0xffff_ffff) then AMASKn[AM] doesn't mask > CSPRn[BA] bits. > > Impact: > This will impact booting when we are reprogramming CSPR0(BA) and > AMASK0(AMASK) while executing from NOR Flash. > > Workaround: > Re-programming of CSPR(BA) and AMASK is done while not executing from NOR > Flash. The code which programs the BA and AMASK is executed from L2-SRAM. > > Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org> This commit introdces new build warnings for the following boards: P1010RDB_36BIT_NOR P1010RDB_NOR P1010RDB_36BIT_NOR_SECBOOT P1010RDB_NOR_SECBOOT For example: Configuring for P1010RDB_NOR - Board: P1010RDB, Options: P1010RDB cpu_init_early.c: In function 'cpu_init_early_f': cpu_init_early.c:74: warning: 'l2srbar' may be used uninitialized in this function Please fix! Kumar, Poonam - I'm really p*ssed off. Both of you have more than enough of experience to know that you should not submit untested patches. especially here, where I already had to reject this patch because it did not even pass checkpatch: I wrote in message <20110804212403.3D53221C695@gemini.denx.de>: | Dear Kumar Gala, | | In message <08144324-BE32-4A54-BC2D-B920F18F3D43@kernel.crashing.org> | you wrote: | > | > > Kumar, could you __please__ get used to running your patches | > > throuch | > > checkpatch __before__ submitting? Thanks. | > | > I try to, but not all of them are by me ;) | | I know. But you submitted them, so you are responsible. This level of neglect is really disappointing. Wolfgang Denk
On Oct 18, 2011, at 1:35 AM, Wolfgang Denk wrote: > Dear Kumar Gala, > > In message <1312555480-13401-8-git-send-email-galak@kernel.crashing.org> you wrote: >> From: Poonam Aggrwal <poonam.aggrwal@freescale.com> >> >> Issue: Address masking doesn't work properly. >> When sum of the base address, defined by BA, and memory bank size, >> defined by AM, exceeds 4GB (0xffff_ffff) then AMASKn[AM] doesn't mask >> CSPRn[BA] bits. >> >> Impact: >> This will impact booting when we are reprogramming CSPR0(BA) and >> AMASK0(AMASK) while executing from NOR Flash. >> >> Workaround: >> Re-programming of CSPR(BA) and AMASK is done while not executing from NOR >> Flash. The code which programs the BA and AMASK is executed from L2-SRAM. >> >> Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com> >> Signed-off-by: Kumar Gala <galak@kernel.crashing.org> > > This commit introdces new build warnings for the following boards: > > P1010RDB_36BIT_NOR P1010RDB_NOR > P1010RDB_36BIT_NOR_SECBOOT P1010RDB_NOR_SECBOOT > > For example: > > Configuring for P1010RDB_NOR - Board: P1010RDB, Options: P1010RDB > cpu_init_early.c: In function 'cpu_init_early_f': > cpu_init_early.c:74: warning: 'l2srbar' may be used uninitialized in this function > > > Please fix! > > > Kumar, Poonam - I'm really p*ssed off. Both of you have more than > enough of experience to know that you should not submit > untested patches. especially here, where I already had to reject this > patch because it did not even pass checkpatch: > > I wrote in message <20110804212403.3D53221C695@gemini.denx.de>: > > | Dear Kumar Gala, > | > | In message <08144324-BE32-4A54-BC2D-B920F18F3D43@kernel.crashing.org> > | you wrote: > | > > | > > Kumar, could you __please__ get used to running your patches > | > > throuch > | > > checkpatch __before__ submitting? Thanks. > | > > | > I try to, but not all of them are by me ;) > | > | I know. But you submitted them, so you are responsible. > > > This level of neglect is really disappointing. > > > Wolfgang Denk If you look at the code I have NO IDEA how to fix this for older GCC. Gripping at me about this isn't fair. I'm sure if I hack something to make gcc-4.2 happy I'm going to piss off gcc-4.6. We can't win. At some point we have to move off gcc-4.2 as the baseline compiler w/regards to warning and code generation. - k
Kumar, Wolfgang I think this is my mistake, I did not take care of checkpatch. Please let me know , I can submit it again. Regards Poonam > -----Original Message----- > From: Kumar Gala [mailto:galak@kernel.crashing.org] > Sent: Tuesday, October 18, 2011 5:18 PM > To: Wolfgang Denk > Cc: u-boot@lists.denx.de; Aggrwal Poonam-B10812 > Subject: Re: [U-Boot] [PATCH 7/7][v2] fsl_ifc: Add the workaround for > erratum IFC A-003399(enabled on P1010) > > > On Oct 18, 2011, at 1:35 AM, Wolfgang Denk wrote: > > > Dear Kumar Gala, > > > > In message <1312555480-13401-8-git-send-email- > galak@kernel.crashing.org> you wrote: > >> From: Poonam Aggrwal <poonam.aggrwal@freescale.com> > >> > >> Issue: Address masking doesn't work properly. > >> When sum of the base address, defined by BA, and memory bank size, > >> defined by AM, exceeds 4GB (0xffff_ffff) then AMASKn[AM] doesn't mask > >> CSPRn[BA] bits. > >> > >> Impact: > >> This will impact booting when we are reprogramming CSPR0(BA) and > >> AMASK0(AMASK) while executing from NOR Flash. > >> > >> Workaround: > >> Re-programming of CSPR(BA) and AMASK is done while not executing from > >> NOR Flash. The code which programs the BA and AMASK is executed from > L2-SRAM. > >> > >> Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com> > >> Signed-off-by: Kumar Gala <galak@kernel.crashing.org> > > > > This commit introdces new build warnings for the following boards: > > > > P1010RDB_36BIT_NOR P1010RDB_NOR > > P1010RDB_36BIT_NOR_SECBOOT P1010RDB_NOR_SECBOOT > > > > For example: > > > > Configuring for P1010RDB_NOR - Board: P1010RDB, Options: P1010RDB > > cpu_init_early.c: In function 'cpu_init_early_f': > > cpu_init_early.c:74: warning: 'l2srbar' may be used uninitialized in > > this function > > > > > > Please fix! > > > > > > Kumar, Poonam - I'm really p*ssed off. Both of you have more than > > enough of experience to know that you should not submit untested > > patches. especially here, where I already had to reject this patch > > because it did not even pass checkpatch: > > > > I wrote in message <20110804212403.3D53221C695@gemini.denx.de>: > > > > | Dear Kumar Gala, > > | > > | In message > > | <08144324-BE32-4A54-BC2D-B920F18F3D43@kernel.crashing.org> > > | you wrote: > > | > > > | > > Kumar, could you __please__ get used to running your patches > > | > > throuch checkpatch __before__ submitting? Thanks. > > | > > > | > I try to, but not all of them are by me ;) > > | > > | I know. But you submitted them, so you are responsible. > > > > > > This level of neglect is really disappointing. > > > > > > Wolfgang Denk > > If you look at the code I have NO IDEA how to fix this for older GCC. > Gripping at me about this isn't fair. I'm sure if I hack something to > make gcc-4.2 happy I'm going to piss off gcc-4.6. We can't win. > > At some point we have to move off gcc-4.2 as the baseline compiler > w/regards to warning and code generation. > > - k
Dear Kumar, In message <C9025279-8F82-453E-8B43-A5D2270A0BCA@kernel.crashing.org> you wrote: > > If you look at the code I have NO IDEA how to fix this for older GCC. Maybe you have to explain the code to me. LIke the compiler, I wonder where l2srbar gets initialized: Here is the declaration: ... 74 u32 *l2srbar, *dst, *src; ... First use of this variable is here: ... 139 for (i = 0; i < 1024; i++) 140 *l2srbar++ = *src++; ... Where is the initialization? Best regards, Wolfgang Denk
On Oct 18, 2011, at 3:18 PM, Wolfgang Denk wrote: > Dear Kumar, > > In message <C9025279-8F82-453E-8B43-A5D2270A0BCA@kernel.crashing.org> you wrote: >> >> If you look at the code I have NO IDEA how to fix this for older GCC. > > Maybe you have to explain the code to me. LIke the compiler, I wonder > where l2srbar gets initialized: > > Here is the declaration: > > ... > 74 u32 *l2srbar, *dst, *src; > ... > > First use of this variable is here: > > ... > 139 for (i = 0; i < 1024; i++) > 140 *l2srbar++ = *src++; > ... > > Where is the initialization? I apologize, now that I look at this I question what in the world is going on. Poonam, What's going on whit this code: dst = (u32 *) SRAM_BASE_ADDR; src = (u32 *) setup_ifc; for (i = 0; i < 1024; i++) *l2srbar++ = *src++; we don't use 'dst' as far as I can tell and 'l2srbar' is never initialized so what in the world are we writing to? - k
> -----Original Message----- > From: Kumar Gala [mailto:galak@kernel.crashing.org] > Sent: Wednesday, October 19, 2011 11:52 AM > To: Aggrwal Poonam-B10812 > Cc: u-boot@lists.denx.de List; Wolfgang Denk > Subject: Re: [U-Boot] [PATCH 7/7][v2] fsl_ifc: Add the workaround for > erratum IFC A-003399(enabled on P1010) > > > On Oct 18, 2011, at 3:18 PM, Wolfgang Denk wrote: > > > Dear Kumar, > > > > In message <C9025279-8F82-453E-8B43-A5D2270A0BCA@kernel.crashing.org> > you wrote: > >> > >> If you look at the code I have NO IDEA how to fix this for older GCC. > > > > Maybe you have to explain the code to me. LIke the compiler, I wonder > > where l2srbar gets initialized: > > > > Here is the declaration: > > > > ... > > 74 u32 *l2srbar, *dst, *src; > > ... > > > > First use of this variable is here: > > > > ... > > 139 for (i = 0; i < 1024; i++) > > 140 *l2srbar++ = *src++; > > ... > > > > Where is the initialization? > > I apologize, now that I look at this I question what in the world is > going on. > > Poonam, > > What's going on whit this code: > > dst = (u32 *) SRAM_BASE_ADDR; > src = (u32 *) setup_ifc; > for (i = 0; i < 1024; i++) > *l2srbar++ = *src++; > > we don't use 'dst' as far as I can tell and 'l2srbar' is never > initialized so what in the world are we writing to? > Hello Kumar, Wolfgang This is a mistake, I admit. Extremely sorry for this. I will send a new patch. > - k
diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c index c2fb5b8..0478ec1 100644 --- a/arch/powerpc/cpu/mpc85xx/cmd_errata.c +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -93,6 +93,9 @@ static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #ifdef CONFIG_SYS_FSL_ERRATUM_P1010_A003549 puts("Work-around for Erratum P1010-A003549 enabled\n"); #endif +#ifdef CONFIG_SYS_FSL_ERRATUM_IFC_A003399 + puts("Work-around for Erratum IFC A-003399 enabled\n"); +#endif return 0; } diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c index c42efb1..a04f5c1 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c +++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c @@ -25,6 +25,42 @@ DECLARE_GLOBAL_DATA_PTR; +#if defined(CONFIG_SYS_FSL_ERRATUM_IFC_A003399) && !defined(CONFIG_SYS_RAMBOOT) +void setup_ifc(void) +{ + struct fsl_ifc *ifc_regs = (void *)CONFIG_SYS_IFC_ADDR; + u32 _mas0, _mas1, _mas2, _mas3, _mas7; + phys_addr_t flash_phys = CONFIG_SYS_FLASH_BASE_PHYS; + + /* + * Adjust the TLB we were running out of to match the phys addr of the + * chip select we are adjusting and will return to. + */ + flash_phys += (~CONFIG_SYS_AMASK0) + 1 - 4*1024*1024; + + _mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(15); + _mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | MAS1_IPROT | + MAS1_TSIZE(BOOKE_PAGESZ_4M); + _mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_TEXT_BASE, MAS2_I|MAS2_G); + _mas3 = FSL_BOOKE_MAS3(flash_phys, 0, MAS3_SW|MAS3_SR|MAS3_SX); + _mas7 = FSL_BOOKE_MAS7(flash_phys); + + mtspr(MAS0, _mas0); + mtspr(MAS1, _mas1); + mtspr(MAS2, _mas2); + mtspr(MAS3, _mas3); + mtspr(MAS7, _mas7); + + asm volatile("isync;msync;tlbwe;isync"); + + out_be32(&(ifc_regs->cspr_cs[0].cspr), CONFIG_SYS_CSPR0); + out_be32(&(ifc_regs->csor_cs[0].csor), CONFIG_SYS_CSOR0); + out_be32(&(ifc_regs->amask_cs[0].amask), CONFIG_SYS_AMASK0); + + return ; +} +#endif + /* We run cpu_init_early_f in AS = 1 */ void cpu_init_early_f(void) { @@ -33,6 +69,11 @@ void cpu_init_early_f(void) #ifdef CONFIG_SYS_FSL_ERRATUM_P1010_A003549 ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); #endif +#if defined(CONFIG_SYS_FSL_ERRATUM_IFC_A003399) && !defined(CONFIG_SYS_RAMBOOT) + ccsr_l2cache_t *l2cache = (void *)CONFIG_SYS_MPC85xx_L2_ADDR; + u32 *l2srbar, *dst, *src; + void (*setup_ifc_sram)(void); +#endif /* Pointer is writable since we allocated a register for it */ gd = (gd_t *) (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET); @@ -62,6 +103,51 @@ void cpu_init_early_f(void) #endif init_laws(); + +/* + * Work Around for IFC Erratum A003399, issue will hit only when execution + * from NOR Flash + */ +#if defined(CONFIG_SYS_FSL_ERRATUM_IFC_A003399) && !defined(CONFIG_SYS_RAMBOOT) +#define SRAM_BASE_ADDR (0x00000000) + /* TLB for SRAM */ + mas0 = MAS0_TLBSEL(1) | MAS0_ESEL(9); + mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_TS | + MAS1_TSIZE(BOOKE_PAGESZ_1M); + mas2 = FSL_BOOKE_MAS2(SRAM_BASE_ADDR, MAS2_I); + mas3 = FSL_BOOKE_MAS3(SRAM_BASE_ADDR, 0, MAS3_SX|MAS3_SW|MAS3_SR); + mas7 = FSL_BOOKE_MAS7(0); + + write_tlb(mas0, mas1, mas2, mas3, mas7); + + out_be32(&l2cache->l2srbar0, SRAM_BASE_ADDR); + + out_be32(&l2cache->l2errdis, + (MPC85xx_L2ERRDIS_MBECC | MPC85xx_L2ERRDIS_SBECC)); + + out_be32(&l2cache->l2ctl, + (MPC85xx_L2CTL_L2E | MPC85xx_L2CTL_L2SRAM_ENTIRE)); + + /* + * Copy the code in setup_ifc to L2SRAM. Do a word copy + * because NOR Flash on P1010 does not support byte + * access (Erratum IFC-A002769) + */ + setup_ifc_sram = (void *)SRAM_BASE_ADDR; + dst = (u32 *) SRAM_BASE_ADDR; + src = (u32 *) setup_ifc; + for (i = 0; i < 1024; i++) + *l2srbar++ = *src++; + + setup_ifc_sram(); + + /* CLEANUP */ + clrbits_be32(&l2cache->l2ctl, + (MPC85xx_L2CTL_L2E | + MPC85xx_L2CTL_L2SRAM_ENTIRE)); + out_be32(&l2cache->l2srbar0, 0x0); +#endif + invalidate_tlb(1); init_tlbs(); } diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c b/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c index 6d01479..f33db02 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c +++ b/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c @@ -43,12 +43,14 @@ void cpu_init_f(void) #endif #endif #ifdef CONFIG_FSL_IFC +#ifndef CONFIG_SYS_FSL_ERRATUM_IFC_A003399 #if defined(CONFIG_SYS_CSPR0) && defined(CONFIG_SYS_CSOR0) set_ifc_cspr(IFC_CS0, CONFIG_SYS_CSPR0); set_ifc_amask(IFC_CS0, CONFIG_SYS_AMASK0); set_ifc_csor(IFC_CS0, CONFIG_SYS_CSOR0); #endif #endif +#endif #if defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SYS_INIT_L2_ADDR) ccsr_l2cache_t *l2cache = (void *)CONFIG_SYS_MPC85xx_L2_ADDR; diff --git a/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c b/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c index e794821..6682496 100644 --- a/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c +++ b/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c @@ -43,10 +43,12 @@ void init_early_memctl_regs(void) set_ifc_ftim(IFC_CS0, IFC_FTIM2, CONFIG_SYS_CS0_FTIM2); set_ifc_ftim(IFC_CS0, IFC_FTIM3, CONFIG_SYS_CS0_FTIM3); +#if !defined(CONFIG_SYS_FSL_ERRATUM_IFC_A003399) || defined(CONFIG_SYS_RAMBOOT) set_ifc_cspr(IFC_CS0, CONFIG_SYS_CSPR0); set_ifc_amask(IFC_CS0, CONFIG_SYS_AMASK0); set_ifc_csor(IFC_CS0, CONFIG_SYS_CSOR0); #endif +#endif #if defined(CONFIG_SYS_CSPR1) && defined(CONFIG_SYS_CSOR1) set_ifc_ftim(IFC_CS1, IFC_FTIM0, CONFIG_SYS_CS1_FTIM0); diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h index 61c19ea..f9bf80d 100644 --- a/arch/powerpc/include/asm/config_mpc85xx.h +++ b/arch/powerpc/include/asm/config_mpc85xx.h @@ -114,6 +114,7 @@ #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY #define CONFIG_SYS_FSL_ERRATUM_IFC_A002769 #define CONFIG_SYS_FSL_ERRATUM_P1010_A003549 +#define CONFIG_SYS_FSL_ERRATUM_IFC_A003399 /* P1011 is single core version of P1020 */ #elif defined(CONFIG_P1011) @@ -164,6 +165,7 @@ #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY #define CONFIG_SYS_FSL_ERRATUM_IFC_A002769 #define CONFIG_SYS_FSL_ERRATUM_P1010_A003549 +#define CONFIG_SYS_FSL_ERRATUM_IFC_A003399 /* P1015 is single core version of P1024 */ #elif defined(CONFIG_P1015)