Patch set 4:Code-Review +1
6 comments:
File src/mainboard/intel/elkhartlake_crb/Kconfig:
Patch Set #4, Line 43: default 8
You have this define in Kconfig of soc already:
config MAX_CPUS
int
default 4
The Kconfig for the soc should have the higher number as it is meant for every possible SKU. Then, if needed for this particular CRB, one could adjust MAX_CPUS here with a lower value.
File src/mainboard/intel/elkhartlake_crb/board_info.txt:
Maybe "CRB" would be better here?
Patch Set #4, Line 6: Flashrom support: y
Does flashrom already have support for EHL?
File src/mainboard/intel/elkhartlake_crb/chromeos.fmd:
Patch Set #4, Line 1: FLASH@0xff000000 0x1000000 {
There is a feature in fmaptool where the definition of the offset for the regions is not needed anymore (see https://review.coreboot.org/c/coreboot/+/33276). This makes the fmd-file better readable and maintainable. I understand that this is just a copy from jasperlake but maybe you can keep that in mind when you start adjusting for Elkhartlake.
File src/mainboard/intel/elkhartlake_crb/mainboard.c:
Patch Set #4, Line 33: sku2147483647
What does this string mean? Looks strange to me but I may not have the right meaning.
File src/mainboard/intel/elkhartlake_crb/variants/baseboard/include/baseboard/variants.h:
Patch Set #4, Line 17: const struct mb_cfg *variant_memcfg_config(uint8_t board_id);
The upper comment does not apply to this function. Maybe rearrange the order so that the comment can be exclusive for the upper three functions?
To view, visit change 47707. To unsubscribe, or for help writing mail filters, visit settings.