On Fri, Mar 22, 2013 at 12:23 PM, York Sun yorksun@freescale.com wrote:
From: Mingkai Hu Mingkai.hu@freescale.com
Calculate reserved fields according to IFC bank count
- Move csor_ext register behind csor register and fix res offset
- move ifc bank count to config_mpc85xx.h to support 8 bank count
There's no IFC controller instead of eLBC controller on some platforms, such as MPC8536, P2041, P3041, P4080 etc, so there's no macro definition for the number of IFC chip select(CONFIG_SYS_FSL_IFC_BANK_COUNT) which is used in the IFC controller header file fsl_ifc.h on these platforms.
This paragraph is pretty confusing. Is this just explaining that IFC_BANK_COUNT isn't being defined for devices that don't use IFC? Or is it explaining why you have to guard fsl_ifc.h with a #ifdef?
Signed-off-by: Mingkai Hu Mingkai.hu@freescale.com
arch/powerpc/cpu/mpc85xx/cpu.c | 2 +- arch/powerpc/cpu/mpc8xxx/fsl_ifc.c | 58 ++++++++++++++++++++++++++++- arch/powerpc/include/asm/config_mpc85xx.h | 7 ++++ arch/powerpc/include/asm/fsl_ifc.h | 42 +++++++++++++++------ 4 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c index df2ab6d..379a7df 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu.c +++ b/arch/powerpc/cpu/mpc85xx/cpu.c @@ -34,8 +34,8 @@ #include <asm/io.h> #include <asm/mmu.h> #include <asm/fsl_ifc.h> -#include <asm/fsl_law.h> #include <asm/fsl_lbc.h> +#include <asm/fsl_law.h> #include <post.h> #include <asm/processor.h> #include <asm/fsl_ddr_sdram.h> diff --git a/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c b/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c index 56b319f..f0da355 100644 --- a/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c +++ b/arch/powerpc/cpu/mpc8xxx/fsl_ifc.c @@ -26,7 +26,7 @@ void print_ifc_regs(void) int i, j;
printf("IFC Controller Registers\n");
for (i = 0; i < FSL_IFC_BANK_COUNT; i++) {
for (i = 0; i < CONFIG_SYS_FSL_IFC_BANK_COUNT; i++) { printf("CSPR%d:0x%08X\tAMASK%d:0x%08X\tCSOR%d:0x%08X\n", i, get_ifc_cspr(i), i, get_ifc_amask(i), i, get_ifc_csor(i));@@ -94,4 +94,60 @@ void init_early_memctl_regs(void) set_ifc_amask(IFC_CS3, CONFIG_SYS_AMASK3); set_ifc_csor(IFC_CS3, CONFIG_SYS_CSOR3); #endif
+#ifdef CONFIG_SYS_CSPR4_EXT
set_ifc_cspr_ext(IFC_CS4, CONFIG_SYS_CSPR4_EXT);+#endif +#if defined(CONFIG_SYS_CSPR4) && defined(CONFIG_SYS_CSOR4)
There seem to be a large number of CONFIG options associated with each and every new bank. It's following the pattern of the existing code, so I'll accept it, but I can't help but think that some of these config options are entirely redundant (would we ever have CSPR4, and *not* CSOR4?). This is, admittedly, based only on a high-level view of the code -- I'm not familiar with the specifics of the IFC design.
[...]
diff --git a/arch/powerpc/include/asm/fsl_ifc.h b/arch/powerpc/include/asm/fsl_ifc.h index ba41b73..debcb6b 100644 --- a/arch/powerpc/include/asm/fsl_ifc.h +++ b/arch/powerpc/include/asm/fsl_ifc.h @@ -21,6 +21,7 @@ #ifndef __ASM_PPC_FSL_IFC_H #define __ASM_PPC_FSL_IFC_H
+#ifdef CONFIG_FSL_IFC #include <config.h> #include <common.h>
[...]
@@ -907,6 +910,22 @@ struct fsl_ifc_gpcm { u32 res4[0x1F3]; };
+#ifdef CONFIG_SYS_FSL_IFC_BANK_COUNT +#if (CONFIG_SYS_FSL_IFC_BANK_COUNT <= 8) +#define CONFIG_SYS_FSL_IFC_CSPR_RES \ + (0x25 - CONFIG_SYS_FSL_IFC_BANK_COUNT * 3) +#define CONFIG_SYS_FSL_IFC_AMASK_RES \ + (0x24 - CONFIG_SYS_FSL_IFC_BANK_COUNT * 3) +#define CONFIG_SYS_FSL_IFC_CSOR_RES \ + (0x24 - CONFIG_SYS_FSL_IFC_BANK_COUNT * 3) +#define CONFIG_SYS_FSL_IFC_FTIM_RES \ + (0x90 - CONFIG_SYS_FSL_IFC_BANK_COUNT * 0xc)
These aren't really config values. They are values derived from a CONFIG value, and they only have local scope (so there's no need for the very global scoping of the names).
Also... Are these correct? 0x25 is a strange amount of gap in register space.
All that aside, I think we should just define the register offsets to cover all existing (or even predicted) registers, and make the gap hard-coded as before. There's no real advantage to specifying only as many as are implemented, and this method seems ripe for potential bugs in the future.
+#else +#error IFC BANK count not vaild +#endif +#else +#error IFC BANK count not defined +#endif
/* * IFC Controller Registers @@ -918,24 +937,24 @@ struct fsl_ifc { u32 cspr_ext; u32 cspr; u32 res2; - } cspr_cs[FSL_IFC_BANK_COUNT]; - u32 res3[0x19]; + } cspr_cs[CONFIG_SYS_FSL_IFC_BANK_COUNT]; + u32 res3[CONFIG_SYS_FSL_IFC_CSPR_RES]; struct { u32 amask; u32 res4[0x2]; - } amask_cs[FSL_IFC_BANK_COUNT]; - u32 res5[0x17]; + } amask_cs[CONFIG_SYS_FSL_IFC_BANK_COUNT]; + u32 res5[CONFIG_SYS_FSL_IFC_AMASK_RES]; struct { - u32 csor_ext; u32 csor; + u32 csor_ext; u32 res6; - } csor_cs[FSL_IFC_BANK_COUNT]; - u32 res7[0x19]; + } csor_cs[CONFIG_SYS_FSL_IFC_BANK_COUNT]; + u32 res7[CONFIG_SYS_FSL_IFC_CSOR_RES]; struct { u32 ftim[4]; u32 res8[0x8]; - } ftim_cs[FSL_IFC_BANK_COUNT]; - u32 res9[0x60]; + } ftim_cs[CONFIG_SYS_FSL_IFC_BANK_COUNT]; + u32 res9[CONFIG_SYS_FSL_IFC_FTIM_RES]; u32 rb_stat; u32 res10[0x2]; u32 ifc_gcr; @@ -961,6 +980,7 @@ struct fsl_ifc { #undef CSPR_MSEL_NOR #define CSPR_MSEL_NOR CSPR_MSEL_GPCM #endif +#endif /* CONFIG_FSL_IFC */
#endif /* __ASSEMBLY__ */ #endif /* __ASM_PPC_FSL_IFC_H */ -- 1.7.9.5
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot