Change in coreboot[master]: vendorcode/google: consolidate strong mainboard_get_dram_part_num rou...
Nick Vaccaro has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45636 )
Change subject: vendorcode/google: consolidate strong mainboard_get_dram_part_num routines ......................................................................
vendorcode/google: consolidate strong mainboard_get_dram_part_num routines
Move mainboard_get_dram_part_num() into src/vendor/google/chromeos to allow mainboards to use it without having to duplicate that code.
BUG=b:168724473 TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer to kernel and verify that the BIOS log shows a part name when logging SPD information:
SPD: module part number is K4U6E3S4AA-MGCL
I also built coreboot for hatch, dedede, and deltaur and verified that the build succeeds without error.
Change-Id: I49b7b2be7b46b5ffd6377d6caabd5e3e6c0a8d61 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/romstage.c M src/soc/intel/elkhartlake/romstage/romstage.c M src/vendorcode/google/chromeos/Makefile.inc A src/vendorcode/google/chromeos/spd_part_num_override.c 4 files changed, 25 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/45636/1
diff --git a/src/mainboard/google/volteer/romstage.c b/src/mainboard/google/volteer/romstage.c index 552648b..720ab7f 100644 --- a/src/mainboard/google/volteer/romstage.c +++ b/src/mainboard/google/volteer/romstage.c @@ -28,17 +28,3 @@
meminit_ddr(mem_cfg, board_cfg, &spd_info, half_populated); } - -bool mainboard_get_dram_part_num(const char **part_num, size_t *len) -{ - static char part_num_store[DIMM_INFO_PART_NUMBER_SIZE]; - - if (google_chromeec_cbi_get_dram_part_num(part_num_store, - sizeof(part_num_store)) < 0) { - printk(BIOS_ERR, "ERROR: Couldn't obtain DRAM part number from CBI\n"); - return false; - } - *part_num = part_num_store; - *len = strlen(part_num_store); - return true; -} diff --git a/src/soc/intel/elkhartlake/romstage/romstage.c b/src/soc/intel/elkhartlake/romstage/romstage.c index d68d2da..913237b 100644 --- a/src/soc/intel/elkhartlake/romstage/romstage.c +++ b/src/soc/intel/elkhartlake/romstage/romstage.c @@ -21,12 +21,6 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
-bool mainboard_get_dram_part_num(const char **part_num, size_t *len) -{ - /* Default implementation, no need to override part number. */ - return false; -} - /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { diff --git a/src/vendorcode/google/chromeos/Makefile.inc b/src/vendorcode/google/chromeos/Makefile.inc index e17236d..67e7b28 100644 --- a/src/vendorcode/google/chromeos/Makefile.inc +++ b/src/vendorcode/google/chromeos/Makefile.inc @@ -16,3 +16,5 @@ verstage-y += watchdog.c romstage-y += watchdog.c ramstage-y += watchdog.c + +romstage-$(CONFIG_CHROMEOS_DRAM_PART_NUMBER_IN_CBI) += spd_part_num_override.c diff --git a/src/vendorcode/google/chromeos/spd_part_num_override.c b/src/vendorcode/google/chromeos/spd_part_num_override.c new file mode 100644 index 0000000..b4782ae --- /dev/null +++ b/src/vendorcode/google/chromeos/spd_part_num_override.c @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <ec/google/chromeec/ec.h> +#include <spd_bin.h> +#include <string.h> + +bool mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + static char part_num_store[DIMM_INFO_PART_NUMBER_SIZE]; + + /* If we already have the string, return a pointer to it */ + if (!strlen(part_num_store) && + google_chromeec_cbi_get_dram_part_num(part_num_store, + sizeof(part_num_store)) < 0) { + printk(BIOS_ERR, "ERROR: Couldn't obtain DRAM part number" + "from CBI\n"); + return false; + } + *part_num = part_num_store; + *len = strlen(part_num_store); + return true; +}
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45636 )
Change subject: vendorcode/google: consolidate strong mainboard_get_dram_part_num routines ......................................................................
Patch Set 23:
This change is ready for review.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45636
to look at the new patch set (#24).
Change subject: vendorcode/google: consolidate strong mainboard_get_dram_part_num routines ......................................................................
vendorcode/google: consolidate strong mainboard_get_dram_part_num routines
Move mainboard_get_dram_part_num() into common location in vendorcode/google/chromeos/dram_part_num_override.c.
Change mainboard_get_dram_part_num() to take no input parameters and to return a pointer to a null-terminated string if override name is found.
Remove strong declarations of mainboard_get_dram_part_num in lieu of new common version.
Enable CHROMEOS_DRAM_PART_NUMBER_IN_CBI for hatch, volteer, and dedede.
BUG=b:168724473 TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer to kernel and verify that the BIOS log shows a part name when logging SPD information:
SPD: module part number is K4U6E3S4AA-MGCL
I also built coreboot for hatch, dedede, and deltaur and verified that the build succeeds without error.
Change-Id: I49b7b2be7b46b5ffd6377d6caabd5e3e6c0a8d61 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/include/memory_info.h M src/lib/spd_bin.c M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/romstage.c M src/mainboard/google/hatch/Kconfig M src/mainboard/google/hatch/romstage_spd_cbfs.c M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/romstage.c M src/soc/intel/alderlake/romstage/romstage.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/elkhartlake/romstage/romstage.c M src/soc/intel/jasperlake/romstage/romstage.c M src/soc/intel/tigerlake/romstage/romstage.c M src/vendorcode/google/chromeos/Makefile.inc A src/vendorcode/google/chromeos/dram_part_num_override.c 15 files changed, 60 insertions(+), 89 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/45636/24
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45636
to look at the new patch set (#25).
Change subject: vendorcode/google: consolidate strong mainboard_get_dram_part_num routines ......................................................................
vendorcode/google: consolidate strong mainboard_get_dram_part_num routines
Move mainboard_get_dram_part_num() into common location in vendorcode/google/chromeos/dram_part_num_override.c.
Change mainboard_get_dram_part_num() to take no input parameters and to return a pointer to a null-terminated string if override name is found.
Remove strong declarations of mainboard_get_dram_part_num in lieu of new common version.
Enable CHROMEOS_DRAM_PART_NUMBER_IN_CBI for hatch, volteer, and dedede.
BUG=b:168724473 TEST="emerge-volteer coreboot chromeos-bootimage" and verify it builds sucessfully,
Change-Id: I49b7b2be7b46b5ffd6377d6caabd5e3e6c0a8d61 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/include/memory_info.h M src/lib/spd_bin.c M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/romstage.c M src/mainboard/google/hatch/Kconfig M src/mainboard/google/hatch/romstage_spd_cbfs.c M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/romstage.c M src/soc/intel/alderlake/romstage/romstage.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/elkhartlake/romstage/romstage.c M src/soc/intel/jasperlake/romstage/romstage.c M src/soc/intel/tigerlake/romstage/romstage.c M src/vendorcode/google/chromeos/Makefile.inc A src/vendorcode/google/chromeos/dram_part_num_override.c 15 files changed, 60 insertions(+), 89 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/45636/25
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45636 )
Change subject: vendorcode/google: consolidate strong mainboard_get_dram_part_num routines ......................................................................
Patch Set 25:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/alderlake/ro... File src/soc/intel/alderlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/alderlake/ro... PS25, Line 61: dram_part_num = mainboard_get_dram_part_num(); I think dram_part_num_len should be set here instead of inside the loop.
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/elkhartlake/... File src/soc/intel/elkhartlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/elkhartlake/... PS25, Line 62: dram_part_num = mainboard_get_dram_part_num(); Same comment about dram_part_num_len as in alderlake.
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/jasperlake/r... File src/soc/intel/jasperlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/jasperlake/r... PS25, Line 62: dram_part_num = mainboard_get_dram_part_num(); Same here for dram_part_num_len.
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/tigerlake/ro... File src/soc/intel/tigerlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/45636/25/src/soc/intel/tigerlake/ro... PS25, Line 62: dram_part_num = mainboard_get_dram_part_num(); dram_part_num_len should be added here instead of inside the loop.
https://review.coreboot.org/c/coreboot/+/45636/25/src/vendorcode/google/chro... File src/vendorcode/google/chromeos/dram_part_num_override.c:
PS25: This file should go in the previous CL that adds the Kconfig for CHROMEOS_DRAM_PART_NUMBER_IN_CBI
Nick Vaccaro has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45636 )
Change subject: vendorcode/google: consolidate strong mainboard_get_dram_part_num routines ......................................................................
Abandoned
-
Furquan Shaikh (Code Review) -
Nick Vaccaro (Code Review)