Hi Tom, That seems like a good change, though I would recommend to split it into smaller patches, please, see below:
On 10/26/2011 11:13 PM, Tom Rini wrote: > This changes to making the board be responsible for providing the > memory initialization timings in SPL That probably would be one patch. > and converts the devkit 8000 > to this framework. That, would be another one. > As part of this suffix the Micron DDR settings > with their speed Next patch. > and add a few more timing values that will be needed. This can go along with the patch that uses those settings. > We also make sure that in mem_ok() we clear the values off as we may be > testing the same banks multiple times. That's should be another patch (although it has only one line). > Signed-off-by: Tom Rini <[email protected]> > --- > arch/arm/cpu/armv7/omap3/mem.c | 1 + > arch/arm/cpu/armv7/omap3/sdrc.c | 132 > +++++++++++++++------------ > arch/arm/include/asm/arch-omap3/mem.h | 58 +++++------- > arch/arm/include/asm/arch-omap3/sys_proto.h | 2 +- > board/timll/devkit8000/devkit8000.c | 24 +++++ > 5 files changed, 123 insertions(+), 94 deletions(-) > > diff --git a/arch/arm/cpu/armv7/omap3/mem.c b/arch/arm/cpu/armv7/omap3/mem.c > index a01c303..cd5fe5c 100644 > --- a/arch/arm/cpu/armv7/omap3/mem.c > +++ b/arch/arm/cpu/armv7/omap3/mem.c > @@ -86,6 +86,7 @@ u32 mem_ok(u32 cs) > writel(0x0, addr + 4); /* remove pattern off the bus */ > val1 = readl(addr + 0x400); /* get pos A value */ > val2 = readl(addr); /* get val2 */ > + writel(0x0, addr + 0x400); /* clear pos A */ > > if ((val1 != 0) || (val2 != pattern)) /* see if pos A val changed */ > return 0; > diff --git a/arch/arm/cpu/armv7/omap3/sdrc.c b/arch/arm/cpu/armv7/omap3/sdrc.c > index 0dd1955..4799787 100644 > --- a/arch/arm/cpu/armv7/omap3/sdrc.c > +++ b/arch/arm/cpu/armv7/omap3/sdrc.c > @@ -109,15 +109,56 @@ u32 get_sdr_cs_offset(u32 cs) > } > > /* > + * write_sdrc_timings - > + * - Takes CS and associated timings and initalize SDRAM > + * - Test CS to make sure it's OK for use > + */ > +static void write_sdrc_timings(u32 cs, struct sdrc_actim *sdrc_actim_base, > + u32 mcfg, u32 ctrla, u32 ctrlb, u32 rfr_ctrl, u32 mr) > +{ > + /* Setup timings we got from the board. */ > + writel(mcfg, &sdrc_base->cs[cs].mcfg); > + writel(ctrla, &sdrc_actim_base->ctrla); > + writel(ctrlb, &sdrc_actim_base->ctrlb); > + writel(rfr_ctrl, &sdrc_base->cs[cs].rfr_ctrl); > + writel(CMD_NOP, &sdrc_base->cs[cs].manual); > + writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual); > + writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual); > + writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual); > + writel(mr, &sdrc_base->cs[cs].mr); > + > + /* > + * Test ram in this bank > + * Disable if bad or not present > + */ > + if (!mem_ok(cs)) > + writel(0, &sdrc_base->cs[cs].mcfg); > +} > + > +/* > * do_sdrc_init - > - * - Initialize the SDRAM for use. > - * - code called once in C-Stack only context for CS0 and a possible 2nd > - * time depending on memory configuration from stack+global context > + * - Code called once in C-Stack only context for CS0 and with early being > + * true and a possible 2nd time depending on memory configuration from > + * stack+global context. > */ > void do_sdrc_init(u32 cs, u32 early) > { > - struct sdrc_actim *sdrc_actim_base0, *sdrc_actim_base1; > + struct sdrc_actim *sdrc_actim_base0; > + u32 mcfg, ctrla, ctrlb, rfr_ctrl, mr; > +#ifdef CONFIG_SPL_BUILD > + u32 cs_cfg; > +#endif > > + /* > + * When called in the early context this may be SPL and we will > + * need to set all of the timings. This ends up being board > + * specific so we call a helper function to take care of this > + * for us. Otherwise, to be safe, we need to copy the settings > + * from the first bank to the second. > + */ > +#ifdef CONFIG_SPL_BUILD > + get_board_mem_timings(&cs_cfg, &mcfg, &ctrla, &ctrlb, &rfr_ctrl, &mr); > +#endif > if (early) { > /* reset sdrc controller */ > writel(SOFTRESET, &sdrc_base->sysconfig); > @@ -128,73 +169,45 @@ void do_sdrc_init(u32 cs, u32 early) > /* setup sdrc to ball mux */ > writel(SDRC_SHARING, &sdrc_base->sharing); > > - /* Disable Power Down of CKE cuz of 1 CKE on combo part */ > + /* Disable Power Down of CKE because of 1 CKE on combo part */ > writel(WAKEUPPROC | SRFRONRESET | PAGEPOLICY_HIGH, > &sdrc_base->power); > > writel(ENADLL | DLLPHASE_90, &sdrc_base->dlla_ctrl); > sdelay(0x20000); > - } > - > -/* As long as V_MCFG and V_RFR_CTRL is not defined for all OMAP3 boards we > need > - * to prevent this to be build in non-SPL build */ > #ifdef CONFIG_SPL_BUILD > - /* If we use a SPL there is no x-loader nor config header so we have > - * to do the job ourselfs > - */ > - if (cs == CS0) { > - sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE; > - > - /* General SDRC config */ > - writel(V_MCFG, &sdrc_base->cs[cs].mcfg); > - writel(V_RFR_CTRL, &sdrc_base->cs[cs].rfr_ctrl); > - > - /* AC timings */ > - writel(V_ACTIMA_165, &sdrc_actim_base0->ctrla); > - writel(V_ACTIMB_165, &sdrc_actim_base0->ctrlb); > - > - /* Initialize */ > - writel(CMD_NOP, &sdrc_base->cs[cs].manual); > - writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual); > - writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual); > - writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual); > + /* We only set cs_cfg in this case */ > + writel(cs_cfg, &sdrc_base->cs_cfg); > + > + /* We need to do both banks now, in many cases. */ > + write_sdrc_timings(CS0, > + (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE, > + mcfg, ctrla, ctrlb, rfr_ctrl, mr); > + write_sdrc_timings(CS1, > + (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE, > + mcfg, ctrla, ctrlb, rfr_ctrl, mr); > +#endif > You probably can remove that empty line. > - writel(V_MR, &sdrc_base->cs[cs].mr); > } > -#endif > > /* > - * SDRC timings are set up by x-load or config header > - * We don't need to redo them here. > - * Older x-loads configure only CS0 > - * configure CS1 to handle this ommission > + * If we aren't using SPL we have been loaded by some > + * other means which may not have correctly initialized > + * both CS0 and CS1 (such as some older versions of x-loader) > + * so we may be asked now to setup CS1. > */ > if (cs == CS1) { > sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE; > - sdrc_actim_base1 = (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE; > - writel(readl(&sdrc_base->cs[CS0].mcfg), > - &sdrc_base->cs[CS1].mcfg); > - writel(readl(&sdrc_base->cs[CS0].rfr_ctrl), > - &sdrc_base->cs[CS1].rfr_ctrl); > - writel(readl(&sdrc_actim_base0->ctrla), > - &sdrc_actim_base1->ctrla); > - writel(readl(&sdrc_actim_base0->ctrlb), > - &sdrc_actim_base1->ctrlb); > - > - writel(CMD_NOP, &sdrc_base->cs[cs].manual); > - writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual); > - writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual); > - writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual); > - writel(readl(&sdrc_base->cs[CS0].mr), > - &sdrc_base->cs[CS1].mr); > - } > + mcfg = readl(&sdrc_base->cs[CS0].mcfg), > + rfr_ctrl = readl(&sdrc_base->cs[CS0].rfr_ctrl); > + ctrla = readl(&sdrc_actim_base0->ctrla), > + ctrlb = readl(&sdrc_actim_base0->ctrlb); > + mr = readl(&sdrc_base->cs[CS0].mr); > + write_sdrc_timings(cs, > + (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE, > + mcfg, ctrla, ctrlb, rfr_ctrl, mr); > same here > - /* > - * Test ram in this bank > - * Disable if bad or not present > - */ > - if (!mem_ok(cs)) > - writel(0, &sdrc_base->cs[cs].mcfg); > + } > } > > /* > @@ -208,8 +221,9 @@ int dram_init(void) > size0 = get_sdr_cs_size(CS0); > /* > * If a second bank of DDR is attached to CS1 this is > - * where it can be started. Early init code will init > - * memory on CS0. > + * where it can be found. If we have SPL that code will have Why do you need two spaces before the if? > + * initalized it already, otherwise early init code will init > + * memory on CS0 only. > */ > if ((sysinfo.mtype == DDR_COMBO) || (sysinfo.mtype == DDR_STACKED)) { > do_sdrc_init(CS1, NOT_EARLY); > diff --git a/arch/arm/include/asm/arch-omap3/mem.h > b/arch/arm/include/asm/arch-omap3/mem.h > index 8e28f77..af3504c 100644 > --- a/arch/arm/include/asm/arch-omap3/mem.h > +++ b/arch/arm/include/asm/arch-omap3/mem.h > @@ -37,12 +37,28 @@ enum { > }; > #endif /* __ASSEMBLY__ */ > > +/* Memory that can be connected to GPMC */ > +#define GPMC_NOR 0 > +#define GPMC_NAND 1 > +#define GPMC_MDOC 2 > +#define GPMC_ONENAND 3 > +#define MMC_NAND 4 > +#define MMC_ONENAND 5 > +#define GPMC_NONE 6 > +#define GPMC_ONENAND_TRY 7 > + The comment is misleading. This probably was copied from the X-Loader, and each define specifies the boot storage devices probing order, right? I think the above does not belong here as all the GPMC code in this file, we have omap-gpmc.h file in this directory for the GPMC stuff, but as the GPMC code is already here, I don't think it is right to ask you to move it... So volunteers are welcome... > #define EARLY_INIT 1 > > /* Slower full frequency range default timings for x32 operation*/ > #define SDRC_SHARING 0x00000100 > #define SDRC_MR_0_SDR 0x00000031 > > +/* optimized timings good for current shipping parts */ > +#define SDP_3430_SDRC_RFR_CTRL_100MHz 0x0002da01 > +#define SDP_3430_SDRC_RFR_CTRL_133MHz 0x0003de01 /* 7.8us/7.5ns - 50=0x3de > */ > +#define SDP_3430_SDRC_RFR_CTRL_165MHz 0x0004e201 /* 7.8us/6ns - 50=0x4e2 */ > +#define SDP_3430_SDRC_RFR_CTRL_200MHz 0x0005e601 /* 7.8us/5ns - 50=0x5e6 */ > + > #define DLL_OFFSET 0 > #define DLL_WRITEDDRCLKX2DIS 1 > #define DLL_ENADLL 1 > @@ -138,15 +154,15 @@ enum { > #define MICRON_CASWIDTH 0x5 > #define MICRON_RASWIDTH 0x2 > #define MICRON_LOCKSTATUS 0x0 > -#define MICRON_V_MCFG ((MICRON_LOCKSTATUS << 30) | (MICRON_RASWIDTH << 24) | > \ > - (MICRON_CASWIDTH << 20) | (MICRON_ADDRMUXLEGACY << 19) | \ > - (MICRON_RAMSIZE << 8) | (MICRON_BANKALLOCATION << 6) | \ > - (MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \ > - (MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE)) > +#define MICRON_V_MCFG_165 ((MICRON_LOCKSTATUS << 30) | \ The above line should be generating a white space error. There is a mixture of tabs and spaces. Also, why don't you just use one tab or space? > + (MICRON_RASWIDTH << 24) | (MICRON_CASWIDTH << 20) | \ > + (MICRON_ADDRMUXLEGACY << 19) | (MICRON_RAMSIZE << 8) | \ > + (MICRON_BANKALLOCATION << 6) | (MICRON_B32NOT16 << 4) | \ > + (MICRON_DEEPPD << 3) | (MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE)) > > -#define MICRON_ARCV 2030 > -#define MICRON_ARE 0x1 > -#define MICRON_V_RFR_CTRL ((MICRON_ARCV << 8) | (MICRON_ARE)) > +#define MICRON_ARCV_165 0x4e2 > +#define MICRON_ARE 0x1 The above values are not aligned with all the others. > +#define MICRON_V_RFR_CTRL_165 ((MICRON_ARCV_165 << 8) | (MICRON_ARE)) The MICRON_ARE macro does not need a parenthesis. > > #define MICRON_BL 0x2 > #define MICRON_SIL 0x0 > @@ -194,32 +210,6 @@ enum { > (NUMONYX_XSR_165 << 0) | (NUMONYX_TXP_165 << 8) | \ > (NUMONYX_TWTR_165 << 16)) > > -#ifdef CONFIG_OMAP3_INFINEON_DDR > -#define V_ACTIMA_165 INFINEON_V_ACTIMA_165 > -#define V_ACTIMB_165 INFINEON_V_ACTIMB_165 > -#endif > - > -#ifdef CONFIG_OMAP3_MICRON_DDR > -#define V_ACTIMA_165 MICRON_V_ACTIMA_165 > -#define V_ACTIMB_165 MICRON_V_ACTIMB_165 > -#define V_MCFG MICRON_V_MCFG > -#define V_RFR_CTRL MICRON_V_RFR_CTRL > -#define V_MR MICRON_V_MR > -#endif > - > -#ifdef CONFIG_OMAP3_NUMONYX_DDR > -#define V_ACTIMA_165 NUMONYX_V_ACTIMA_165 > -#define V_ACTIMB_165 NUMONYX_V_ACTIMB_165 > -#endif > - > -#if !defined(V_ACTIMA_165) || !defined(V_ACTIMB_165) > -#error "Please choose the right DDR type in config header" > -#endif > - > -#if defined(CONFIG_SPL_BUILD) && (!defined(V_MCFG) || !defined(V_RFR_CTRL)) > -#error "Please choose the right DDR type in config header" > -#endif > - > /* > * GPMC settings - > * Definitions is as per the following format > diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h > b/arch/arm/include/asm/arch-omap3/sys_proto.h > index 7b60051..a2c317a 100644 > --- a/arch/arm/include/asm/arch-omap3/sys_proto.h > +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h > @@ -38,6 +38,7 @@ void per_clocks_enable(void); > void memif_init(void); > void sdrc_init(void); > void do_sdrc_init(u32, u32); > +void get_board_mem_timings(u32 *, u32 *, u32 *, u32 *, u32 *, u32 *); > void emif4_init(void); > void gpmc_init(void); > void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 > base, > @@ -49,7 +50,6 @@ void set_muxconf_regs(void); > u32 get_cpu_family(void); > u32 get_cpu_rev(void); > u32 get_sku_id(void); > -u32 get_mem_type(void); I think the above will break omap2420h4 and apollon boards. Also, how is this related to that patch? > u32 get_sysboot_value(void); > u32 is_gpmc_muxed(void); > u32 get_gpmc0_type(void); > diff --git a/board/timll/devkit8000/devkit8000.c > b/board/timll/devkit8000/devkit8000.c > index f50d113..568f02e 100644 > --- a/board/timll/devkit8000/devkit8000.c > +++ b/board/timll/devkit8000/devkit8000.c > @@ -138,3 +138,27 @@ int board_eth_init(bd_t *bis) > return dm9000_initialize(bis); > } > #endif > + > +/* > + * Routine: get_board_mem_timings > + * Description: If we use SPL then there is no x-loader nor config header > + * so we have to setup the DDR timings outself on the first bank. This s/outself on/our self from/ Also two spaces before "This". > + * provides the timing values back to the function that configures > + * the memory. > + */ > +void get_board_mem_timings(u32 *cs_cfg, u32 *mcfg, u32 *ctrla, u32 *ctrlb, > + u32 *rfr_ctrl, u32 *mr) > +{ > + /* 128MiB/bank */ > + *cs_cfg = 0x1; > + > + /* General SDRC config */ > + *mcfg = MICRON_V_MCFG_165; > + *rfr_ctrl = MICRON_V_RFR_CTRL_165; > + > + /* AC timings */ > + *ctrla = MICRON_V_ACTIMA_165; > + *ctrlb = MICRON_V_ACTIMB_165; > + > + *mr = MICRON_V_MR; > +} This looks good. Regards, Igor _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
