9 comments:
File src/mainboard/intel/adlrvp/board_id.h:
Patch Set #7, Line 6: #include <stdint.h>
not required
Ack
File src/mainboard/intel/adlrvp/board_id.c:
uint8_t buffer[2];
size_t index;
if (send_ec_command(EC_FAB_ID_CMD) == 0) {
for (index = 0; index < sizeof(buffer); index++)
buffer[index] = recv_ec_data();
id = (buffer[0] << 8) | buffer[1];
}
suggestion: I would rewrite this buffer and the loop: […]
Ack
symbolic constant here would be nice, even if it's BOARD_ID_MASK 😊
Ack
File src/mainboard/intel/adlrvp/romstage_fsp_params.c:
Patch Set #7, Line 13: mainboard_get_spd_index
nit: this function is a "local" function, no need for the `mainboard` prefix
Ack
Patch Set #7, Line 20: 0x1F) & 0x7;
symbolic constants would be nice
Ack
const and can go up at the top w/ the other locals
Ack
File src/mainboard/intel/adlrvp/variants/adlrvp_p/devicetree.cb:
Patch Set #7, Line 8: register "SaGv" = "SaGv_Disabled"
nit: this is the default value
Tim, Default is subject to change over FSP version, in latest code I'm seeing default is 0x5 enable but for early SOC i would like to make Sagv disable
Patch Set #7, Line 43: disable
disabled
Ack
File src/mainboard/intel/adlrvp/variants/baseboard/include/baseboard/variants.h:
adl_p_lp4_1 = 0x10,
adl_p_lp4_2 = 0x11,
/* ADL-P DDR4 RVPs */
adl_p_ddr4_1 = 0x14,
adl_p_ddr4_2 = 0x3F,
typically prefer uppercase enum values
Ack
To view, visit change 46091. To unsubscribe, or for help writing mail filters, visit settings.