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

[Andre Schwarz]

Kim, 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?
SPD data is nothing more than an I2C-EEPROM soldered on each memory module
containing the physical details of the memory devices.
Since I already have an I2C-EEPROM on the board I can use it for SPD 
data, i.e.
the board's memory is configured using normal detection routines.

During production (exactly: board testing) the EEPROM will be programmed 
with an SPD
table matching the soldered memory devices. This gives some flexibility 
regarding size and
speed grades ... some devices have pretty short life-cycles.

> this isn't included in the below diff, right?
yes it is - all I need is to provide the EEPROM's I2C adress and the 
offset of the SPD table.
>> 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..
yes - since the start of EEPROM memory is already occupied by HRCW.

> 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.

Currently the spd_sdram is doing *exactly* what I need.
 From my point of view there's no need to move to new code.

>> 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.
ok.
> however, it should not be being done by glittering old-83xx/spd_sdram
> with an extra #ifdef for every new parameter

ok - what about this :

#if !defined(CONFIG_SYS_DDR_MODE_ODT_VALUE)
#define CONFIG_SYS_DDR_MODE_ODT_VALUE 0x40
#endif

mode_odt_enable = CONFIG_SYS_DDR_MODE_ODT_VALUE;


I really can't think of this change being a problem for anybody.

>   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,
understood - but the effort needed to dig through the new code and
get it up and running is hardly justified by the sole need to set a 
proper ODT value.

I did not even come to speak of an increased memory footprint.
I'll have to provide a patch these days for a 8343 based board removing 
features
due binary size constantly growing ...

>   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.
ok - understood.
I'm willing to do it but not all at once.

>> 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().
ok.
>> I have taken care not to increase code size and not to break existing
>> boards.
>>
>> Any comments from your side ?
>> Is this something you might ACK ?
> not as-is - see above, and let me know what you think.

I fully understand that you want the new code to be used and I'll use it 
asap.
But right now the spd_sdram is working fine and I need this flexibility 
using
the local EEPROM.

IMHO this patch is not breaking anything and completely transparent.
Not a single byte of code increase either ....




Regards,
André


MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner