On Aug 29, 2008, at 5:16 PM, Peter Tyser wrote:
> Hello,
>
> [snip]
>
>> +
>> +/* DDR SDRAM control configuration 2 (DDR_SDRAM_CFG_2) */
>> +static void set_ddr_sdram_cfg_2(fsl_memctl_config_regs_t *ddr,
>> + const memctl_options_t *popts)
>> +{
>> + unsigned int frc_sr = 0; /* Force self refresh */
>> + unsigned int sr_ie = 0; /* Self-refresh interrupt enable */
>> + unsigned int dll_rst_dis; /* DLL reset disable */
>> + unsigned int dqs_cfg; /* DQS configuration */
>> + unsigned int odt_cfg; /* ODT configuration */
>> + unsigned int num_pr; /* Number of posted refreshes */
>> + unsigned int obc_cfg; /* On-The-Fly Burst Chop Cfg */
>> + unsigned int ap_en; /* Address Parity Enable */
>> + unsigned int d_init; /* DRAM data initialization */
>> + unsigned int rcw_en = 0; /* Register Control Word Enable */
>> + unsigned int md_en = 0; /* Mirrored DIMM Enable */
>> +
>> + dll_rst_dis = 1; /* Make this configurable */
>> + dqs_cfg = popts->DQS_config;
>> + if (popts->cs_local_opts[0].odt_rd_cfg
>> + || popts->cs_local_opts[0].odt_wr_cfg) {
>> + /* FIXME */
>> + odt_cfg = 2;
>> + } else {
>> + odt_cfg = 0;
>> + }
>> +
>> + num_pr = 1; /* Make this configurable */
>> +
>> + /*
>> + * 8572 manual says
>> + * {TIMING_CFG_1[PRETOACT]
>> + * + [DDR_SDRAM_CFG_2[NUM_PR]
>> + * * ({EXT_REFREC || REFREC} + 8 + 2)]}
>> + * << DDR_SDRAM_INTERVAL[REFINT]
>> + */
>> +
>> + obc_cfg = 0; /* Make this configurable? */
>> + ap_en = 0; /* Make this configurable? */
>> +
>> +#if defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
>> + /* Use the DDR controller to auto initialize memory. */
>> + d_init = 1;
>> + ddr->ddr_data_init = CONFIG_MEM_INIT_VALUE;
>> + debug("DDR: ddr_data_init = 0x%08x\n", ddr->ddr_data_init);
>> +#else
>> + /* Memory will be initialized via DMA, or not at all. */
>> + d_init = 0;
>> +#endif
>> +
>
> I'm using the current head
> (33aa4eac66b71c797bbc13b3afe432a2132947d4) on
> a mpc8572-based board. It uses DDR2 and has support for ECC. While
> enabling ECC support, I noticed that the "old"
> CONFIG_ECC_INIT_VIA_DDRCONTROLLER define is still being used to enable
> memory initialization on bootup while a new ECC_init_using_memctl
> field
> in the memctl_options_s structure is also present.
>
> Currently it looks like ECC_init_using_memctl doesn't do anything
> and is
> only referenced in one location below:
>
>> + /* Pick ECC modes */
>> +#ifdef CONFIG_DDR_ECC
>> + popts->ECC_mode = 1; /* 0 = disabled, 1 = enabled */
>> +#else
>> + popts->ECC_mode = 0; /* 0 = disabled, 1 = enabled */
>> +#endif
>> + popts->ECC_init_using_memctl = 1; /* 0 = use DMA, 1 = use memctl */
>
> The intended functionality of CONFIG_ECC_INIT_VIA_DDRCONTROLLER and
> ECC_init_using_memctl seem to be the same, but only one of them is
> being
> used, and they currently are unrelated which is a bit confusing.
>
> Should the ECC_init_using_memctl field be removed, or is the intention
> to replace CONFIG_ECC_INIT_VIA_DDRCONTROLLER functionality with
> ECC_init_using_memctl as some point?
Now that the code is unified its will be easier to clean this up. I
need to look at the ECC bits a little more to provide a better answer.
- k
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
