Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31851
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
mb/google/hatch: Provide DRAM part number from EEPROM
This change reads DRAM part number from EEPROM if available and returns it using the SoC callback (mainboard_get_dram_part_number).
BUG=b:127609572 TEST=Verify that DRAM part number from EEPROM is added to DMI table 17 (dmidecode -t 17).
Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Signed-off-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/romstage.c 1 file changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/31851/1
diff --git a/src/mainboard/google/hatch/romstage.c b/src/mainboard/google/hatch/romstage.c index 401f41f..9f1aabd 100644 --- a/src/mainboard/google/hatch/romstage.c +++ b/src/mainboard/google/hatch/romstage.c @@ -14,6 +14,9 @@ */
#include <baseboard/variants.h> +#include <console/console.h> +#include <ec/google/chromeec/ec.h> +#include <memory_info.h> #include <soc/cnl_memcfg_init.h> #include <soc/romstage.h>
@@ -27,3 +30,16 @@ cannonlake_memcfg_init(&memupd->FspmConfig, variant_memory_params(), &spd); } + +void 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[0], + sizeof(part_num_store)) < 0) { + printk(BIOS_ERR, "No DRAM part number in CBI!\n"); + } else { + *part_num = &part_num_store[0]; + *len = sizeof(part_num_store); + } +}
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31851
to look at the new patch set (#4).
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
mb/google/hatch: Provide DRAM part number from EEPROM
This change reads DRAM part number from EEPROM if available and returns it using the SoC callback (mainboard_get_dram_part_number).
BUG=b:127609572 TEST=Verify that DRAM part number from EEPROM is added to DMI table 17 (dmidecode -t 17).
Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Signed-off-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/romstage.c 1 file changed, 29 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/31851/4
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31851 )
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/31851/4/src/mainboard/google/hatch/romstage.... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/#/c/31851/4/src/mainboard/google/hatch/romstage.... PS4, Line 57: *len = sizeof(part_num_store); We don't care about it this being longer than actually used, right?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31851 )
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/31851/4/src/mainboard/google/hatch/romstage.... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/#/c/31851/4/src/mainboard/google/hatch/romstage.... PS4, Line 57: *len = sizeof(part_num_store);
We don't care about it this being longer than actually used, right?
It is NULL-terminated string, so either ways it should be fine. I can change this to strlen to make it clear.
Hello Aaron Durbin, Philip Chen, Duncan Laurie, Shelley Chen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31851
to look at the new patch set (#5).
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
mb/google/hatch: Provide DRAM part number from EEPROM
This change reads DRAM part number from EEPROM if available and returns it using the SoC callback (mainboard_get_dram_part_number).
BUG=b:127609572 TEST=Verify that DRAM part number from EEPROM is added to DMI table 17 (dmidecode -t 17).
Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Signed-off-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/romstage.c 1 file changed, 29 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/31851/5
Hello Aaron Durbin, Philip Chen, Duncan Laurie, Shelley Chen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31851
to look at the new patch set (#6).
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
mb/google/hatch: Provide DRAM part number from EEPROM
This change reads DRAM part number from EEPROM if available and returns it using the SoC callback (mainboard_get_dram_part_number).
BUG=b:127609572 TEST=Verify that DRAM part number from EEPROM is added to DMI table 17 (dmidecode -t 17).
Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Signed-off-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/romstage.c 1 file changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/31851/6
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31851 )
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/31851/6/src/mainboard/google/hatch/romstage.... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/#/c/31851/6/src/mainboard/google/hatch/romstage.... PS6, Line 58: *len = strlen(part_num_store); Does this get padded out in the other parts of the stack? Or do we need to +1 ?
Hello Aaron Durbin, Philip Chen, Duncan Laurie, Shelley Chen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31851
to look at the new patch set (#7).
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
mb/google/hatch: Provide DRAM part number from EEPROM
This change reads DRAM part number from EEPROM if available and returns it using the SoC callback (mainboard_get_dram_part_number).
BUG=b:127609572 TEST=Verify that DRAM part number from EEPROM is added to DMI table 17 (dmidecode -t 17).
Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Signed-off-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/romstage.c 1 file changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/31851/7
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31851 )
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/31851/6/src/mainboard/google/hatch/romstage.... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/#/c/31851/6/src/mainboard/google/hatch/romstage.... PS6, Line 58: *len = strlen(part_num_store);
Does this get padded out in the other parts of the stack? Or do we need to +1 ?
It ends up being passed to strncpy which would not pad it since we are passing in strlen. But, the dest to which it is being copied is memset to 0. So, ideally it should be good. But, I have done a +1 anyways in latest patchset.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31851 )
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
Patch Set 7: Code-Review+2
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31851 )
Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ......................................................................
mb/google/hatch: Provide DRAM part number from EEPROM
This change reads DRAM part number from EEPROM if available and returns it using the SoC callback (mainboard_get_dram_part_number).
BUG=b:127609572 TEST=Verify that DRAM part number from EEPROM is added to DMI table 17 (dmidecode -t 17).
Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/31851 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/mainboard/google/hatch/romstage.c 1 file changed, 30 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/romstage.c b/src/mainboard/google/hatch/romstage.c index 401f41f..429aa09 100644 --- a/src/mainboard/google/hatch/romstage.c +++ b/src/mainboard/google/hatch/romstage.c @@ -14,8 +14,12 @@ */
#include <baseboard/variants.h> +#include <console/console.h> +#include <ec/google/chromeec/ec.h> +#include <memory_info.h> #include <soc/cnl_memcfg_init.h> #include <soc/romstage.h> +#include <string.h>
void mainboard_memory_init_params(FSPM_UPD *memupd) { @@ -27,3 +31,29 @@ cannonlake_memcfg_init(&memupd->FspmConfig, variant_memory_params(), &spd); } + +void mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + static char part_num_store[DIMM_INFO_PART_NUMBER_SIZE]; + static enum { + PART_NUM_NOT_READ, + PART_NUM_AVAILABLE, + PART_NUM_NOT_IN_CBI, + } part_num_state = PART_NUM_NOT_READ; + + if (part_num_state == PART_NUM_NOT_READ) { + if (google_chromeec_cbi_get_dram_part_num(&part_num_store[0], + sizeof(part_num_store)) < 0) { + printk(BIOS_ERR, "No DRAM part number in CBI!\n"); + part_num_state = PART_NUM_NOT_IN_CBI; + } else { + part_num_state = PART_NUM_AVAILABLE; + } + } + + if (part_num_state == PART_NUM_NOT_IN_CBI) + return; + + *part_num = &part_num_store[0]; + *len = strlen(part_num_store) + 1; +}