[coreboot-gerrit] Change in coreboot[master]: mb/adlrvp: Fix DDR5 Boot issue

[Subrata Banik (Code Review)]

Attention is currently required from: Bora Guvendik, Furquan Shaikh, Selma Bensaid, Maulik V Vaghela, Tim Wawrzynczak, Paul Menzel, Meera Ravindranath, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50996 )

Change subject: mb/adlrvp: Fix DDR5 Boot issue
......................................................................

Patch Set 17:

(3 comments)

File src/soc/intel/alderlake/meminit.c:

https://review.coreboot.org/c/coreboot/+/50996/comment/a2e9e417_bf8b232a 
PS17, Line 222: {
will this is more readable 
   const int spd_array[] = { 0xA0, 0xA2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
                              0xA4, 0xA6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0
                           };

or

#define MAX_SPD_ADDRESS_TABLE_ELEMENT 0x10

static void mem_init_smbus_fill_upd(FSP_M_CONFIG *mem_cfg)
    static const int spd_array[MAX_SPD_ADDRESS_TABLE_ELEMENT] = {
                    [0] = 0xA0,
                    [1] = 0xA2,
                    [8] = 0xA4,
                    [9] = 0xA6,
    };

for (int i = 0; i < MAX_SPD_ADDRESS_TABLE_ELEMENT; i++)
            mem_cfg->SpdAddressTable[i] = spd_array[i];
}

I personally prefer the later one

https://review.coreboot.org/c/coreboot/+/50996/comment/50bbbcf6_9e71d8c4 
PS17, Line 222: static 
you don't need static here.

https://review.coreboot.org/c/coreboot/+/50996/comment/a5e201dc_816f6f40 
PS17, Line 271: 		mem_init_dq_upds(mem_cfg, &data, mb_cfg, dq_dqs_auto_detect);
              : 		mem_init_dqs_upds(mem_cfg, &data, mb_cfg, dq_dqs_auto_detect);
shouldn't this resides outside the loop ? we want to fill dq/dqs irrespective of you are passing the SMBUS address or
SPD pointer directly to SPD ?