EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
soc/intel/cannonlake: memory spd data debug
Add printing SPD data for debug usage.
BUG=b:139397313 BRANCH=N/A TEST=Tested the on Hatch and checked cbmem log.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1e257a8ea6ff9c906267841819d2a4b62a9e0b9e --- M src/soc/intel/cannonlake/cnl_memcfg_init.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/35235/1
diff --git a/src/soc/intel/cannonlake/cnl_memcfg_init.c b/src/soc/intel/cannonlake/cnl_memcfg_init.c index d3e5e83..5d3db76 100644 --- a/src/soc/intel/cannonlake/cnl_memcfg_init.c +++ b/src/soc/intel/cannonlake/cnl_memcfg_init.c @@ -81,6 +81,7 @@ default: die("nonexistent memory slot"); } + print_spd_info((unsigned char *)spd_data_ptr); }
/*
Ivy Jian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 1: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/1/src/soc/intel/cannonlake/cn... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/35235/1/src/soc/intel/cannonlake/cn... PS1, Line 84: unsigned char nit: print_spd_info uses uint8_t
Hello Ivy Jian, Patrick Rudolph, Mathew King, Tim Wawrzynczak, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35235
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
soc/intel/cannonlake: memory spd data debug
Add printing SPD data for debug usage.
BUG=b:139397313 BRANCH=N/A TEST=Tested the on Hatch and checked cbmem log.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1e257a8ea6ff9c906267841819d2a4b62a9e0b9e --- M src/soc/intel/cannonlake/cnl_memcfg_init.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/35235/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/1/src/soc/intel/cannonlake/cn... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/35235/1/src/soc/intel/cannonlake/cn... PS1, Line 84: unsigned char
nit: print_spd_info uses uint8_t
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 2: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35235/2//COMMIT_MSG@13 PS2, Line 13: the remove this?
Hello Ivy Jian, Patrick Rudolph, Mathew King, Tim Wawrzynczak, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35235
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
soc/intel/cannonlake: memory spd data debug
Add printing SPD data for debug usage.
BUG=b:139397313 BRANCH=N/A TEST=Tested on Hatch and checked cbmem log.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1e257a8ea6ff9c906267841819d2a4b62a9e0b9e --- M src/soc/intel/cannonlake/cnl_memcfg_init.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/35235/3
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35235/2//COMMIT_MSG@13 PS2, Line 13: the
remove this?
Done thanks.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... PS3, Line 115: last_spd_index = spd_index; Wouldn't it be better to print here? If the same SPD is used for multiple slots, you would end up printing multiple times above.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... PS3, Line 115: last_spd_index = spd_index;
Wouldn't it be better to print here? If the same SPD is used for multiple slots, you would end up pr […]
But we don't print the slot info. If add here, I'd better print the slot as well? I had notice this, but I think it's okay.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... PS3, Line 115: last_spd_index = spd_index;
But we don't print the slot info. […]
You can print slot info, but only one slot # would get printed. Instead you can print SPD Index # if that is helpful.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... PS3, Line 115: last_spd_index = spd_index;
You can print slot info, but only one slot # would get printed. […]
Index already printed before, I would see the result for this.
Hello Ivy Jian, Patrick Rudolph, Mathew King, Tim Wawrzynczak, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35235
to look at the new patch set (#4).
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
soc/intel/cannonlake: memory spd data debug
Add printing SPD data for debug usage.
BUG=b:139397313 BRANCH=N/A TEST=Tested the on Hatch and checked cbmem log.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1e257a8ea6ff9c906267841819d2a4b62a9e0b9e --- M src/soc/intel/cannonlake/cnl_memcfg_init.c 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/35235/4
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/35235/3/src/soc/intel/cannonlake/cn... PS3, Line 115: last_spd_index = spd_index;
Index already printed before, I would see the result for this.
I think this is a clear log to me.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 4: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35235 )
Change subject: soc/intel/cannonlake: memory spd data debug ......................................................................
soc/intel/cannonlake: memory spd data debug
Add printing SPD data for debug usage.
BUG=b:139397313 BRANCH=N/A TEST=Tested the on Hatch and checked cbmem log.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1e257a8ea6ff9c906267841819d2a4b62a9e0b9e Reviewed-on: https://review.coreboot.org/c/coreboot/+/35235 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/cannonlake/cnl_memcfg_init.c 1 file changed, 2 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/cnl_memcfg_init.c b/src/soc/intel/cannonlake/cnl_memcfg_init.c index d3e5e83..6c551ad 100644 --- a/src/soc/intel/cannonlake/cnl_memcfg_init.c +++ b/src/soc/intel/cannonlake/cnl_memcfg_init.c @@ -81,6 +81,7 @@ default: die("nonexistent memory slot"); } + printk(BIOS_INFO, "memory slot: %d configuration done.\n", mem_slot); }
/* @@ -112,6 +113,7 @@
spd_data_ptr = (uintptr_t)rdev_mmap_full(&spd_rdev); last_spd_index = spd_index; + print_spd_info((unsigned char *)spd_data_ptr); }
meminit_spd_data(mem_cfg, mem_slot, spd_data_len, spd_data_ptr);
-
EricR Lai (Code Review) -
Furquan Shaikh (Code Review)
-
Ivy Jian (Code Review)
-
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)