[U-Boot] [PATCH 2/2 v2] powerpc/8xxx: Refactor fsl_ddr_get_spd into common code from board

[Timur Tabi]

On Mon, Jan 31, 2011 at 11:28 PM, Kumar Gala <galak at kernel.crashing.org> wrote:
> +#if defined(SPD_EEPROM_ADDRESS) || \
> +    defined(SPD_EEPROM_ADDRESS1) || defined(SPD_EEPROM_ADDRESS2) || \
> +    defined(SPD_EEPROM_ADDRESS3) || defined(SPD_EEPROM_ADDRESS4)
> +#if (CONFIG_NUM_DDR_CONTROLLERS == 1) && (CONFIG_DIMM_SLOTS_PER_CTLR == 1)
> +u8 spd_i2c_addr[CONFIG_NUM_DDR_CONTROLLERS][CONFIG_DIMM_SLOTS_PER_CTLR] = {
> +       [0][0] = SPD_EEPROM_ADDRESS,
> +};
> +#endif
> +#if (CONFIG_NUM_DDR_CONTROLLERS == 2) && (CONFIG_DIMM_SLOTS_PER_CTLR == 1)
> +u8 spd_i2c_addr[CONFIG_NUM_DDR_CONTROLLERS][CONFIG_DIMM_SLOTS_PER_CTLR] = {
> +       [0][0] = SPD_EEPROM_ADDRESS1,   /* controller 1 */
> +       [1][0] = SPD_EEPROM_ADDRESS2,   /* controller 2 */
> +};
> +#endif
> +#if (CONFIG_NUM_DDR_CONTROLLERS == 2) && (CONFIG_DIMM_SLOTS_PER_CTLR == 2)
> +u8 spd_i2c_addr[CONFIG_NUM_DDR_CONTROLLERS][CONFIG_DIMM_SLOTS_PER_CTLR] = {
> +       [0][0] = SPD_EEPROM_ADDRESS1,   /* controller 1 */
> +       [0][1] = SPD_EEPROM_ADDRESS2,   /* controller 1 */
> +       [1][0] = SPD_EEPROM_ADDRESS3,   /* controller 2 */
> +       [1][1] = SPD_EEPROM_ADDRESS4,   /* controller 2 */
> +};
> +#endif

Wouldn't it be easier if we just temporarily defined values for
SPD_EEPROM_ADDRESSx?  Like this:

#ifndef SPD_EEPROM_ADDRESS2
#define SPD_EEPROM_ADDRESS2 0
#endif
...

u8 spd_i2c_addr[2][2] = {


> +static void __get_spd(generic_spd_eeprom_t *spd, u8 i2c_address)
> +{
> +       int ret = i2c_read(i2c_address, 0, 1, (uchar *)spd,
> +                               sizeof(generic_spd_eeprom_t));
> +
> +       if (ret) {
> +               debug("DDR: failed to read SPD from address %u\n", i2c_address);

This should probably be a printf() instead.

-- 
Timur Tabi
Linux kernel developer at Freescale