[U-Boot] [RFC] mpc83xx: add config options to spd_sdram

[York Sun]

On Wed, 2011-04-06 at 10:42 +0200, Andre Schwarz wrote:
> York,
> >>> I have made some mods to spd_sdram.c for various reason:
> >>>
> >>> 1.
> >>> use SPD setup also for soldered RAM.
> >>> This allows DDR mounting options without U-Boot change because SPD data
> >>> is written during in-circuit/boundary-scan testing.
> >> not sure I understand this - board with soldered RAM can't physically
> >> get the SPD data from RAM, yet SPD data were somehow acquired and
> >> written into some ROM, so spd_sdram() is still needed to parse&
> >> program the controller without requiring a new u-boot binary?
> >>
> >> this isn't included in the below diff, right?
> > Actually I have code for this already, pending for more broader testing.
> > I use raw timing data from datasheet of DDR chips. We have an internal
> > discussion to recommend using EEPROM to mimic SPD structure.
> sorry - I don't understand this.
> Why do you want to create something new if there's a standard
> like SPD layout ? It is working perfectly fine and the data can be
> obtained from the manufacturer without further modifications.


I am not creating anything new. For some platforms with soldered DDR
components, there is no SPD on board. We need to either create the
SPD-like structure, or use the raw timing data from datasheet.

> 
> >>> 2.
> >>> read SPD data also from extended adressing EEPROMS used for e.g. HRCW
> >>> storage.
> >>> Due to HRCW being being located at offset 0 we need an SPD data offset.
> >> and this is the SPD_EEPROM_OFFSET stuff below?  hmm..
> >>
> >> so both 1&  2 are coagulated in my mind - ideally, this would be
> >> implemented with the new ddr code in
> >> arch/powerpc/cpu/mpc8xxx/ddr/main.c - see fsl_ddr_compute(), which has
> >> a GET_SPD step that would need to be either skipped (i.e, start_step =
> >> COMPUTE_DIMM_PARMS), or changed to be able to be overridden, or,
> >> ideally, all DIMM computations would be precomputed and stored in ROM
> >> and a direct start_step to e.g., STEP_ASSIGN_ADDRESSES were made.
> > I don't 100% agree with the idea to put SPD-like data in HRCW.
> The SPD data has nothing to do with HRCW - it's just stored in the same 
> PROM.
> >   How can you be sure the data matches DDR?
> Because the SPD data gets programmed during production.
> We can use any SPD table *exactly* matching the soldered parts.

Agreed as far as the SPD data is not easily erased by mistake.

> > It would be equivalent to using fixed raw timing data, wouldn't it?
> no - it isn't.
> I want the *same* U-Boot binary running on *different* memory 
> configurations, i.e.
> *no* fixed timing at all.

As far as the SPD data is persistent.

> 
> >>> 3.
> >>> for optimized signal integrity and power consumption we need more
> >>> influence on
> >>> the on-die termination. Although the assumed default values are working they
> >>> are far from ideal.
> >> board specific things like this are perfectly acceptable, of course.
> >>
> >> however, it should not be being done by glittering old-83xx/spd_sdram
> >> with an extra #ifdef for every new parameter that is needed, especially
> >> when this is all fixed in the new ddr code.  So, in preparation for
> >> 83xx to eventually migrate to the new ddr code, I'd like this to be
> >> done in such a way so as to start to conform to the new code by at least
> >> defining ODT, etc. parameters in a board-specific
> >> ddr.c:fsl_ddr_board_options().  See board/freescale/{p1022ds,*}/ddr.c
> >> for example.
> > The ODT table is implemented in options.c and it can be override by
> > board functions. Look for the last function in options.c and  ddr.c
> > under the board folder.
> 
> To me it is a fundamental design error to rely on environment data to set
> up the hardware correctly. Especially something that critical as memory
> configuration.
> 
> Do you expect U-Boot to work with a corrupted environment, i.e. hwconfig
> value unavailable ?
> 
> Of course this might never happen on a reference design board - but 
> that's not real life.

It is not in environmental variable.

> 
> >>> 4.
> >>> CPO values depend on internal bond wire length which has significantly
> >>> high variance on MPC837x, i.e. this value also should be board specific.
> >> again, to be defined as popts->cpo_override in the board-specific
> >> ddr.c:fsl_ddr_board_options().
> > I recommend to use auto calibration. If for some reason, you want to
> > override, it belongs to board specific file, same as the above.
> >
> no - auto calibration is far from ideal regarding emc and power consumption.
> 

Then just use whatever parameter fits. Again, it is in the board ddr
file.


York