Nick Vaccaro has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fixed error in generic SPD ......................................................................
mb/google/volteer: fixed error in generic SPD
The SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex SPD contained an incorrect SDRAM Max Cycle Time (0 instead of 0xff). After fixing that error, I noticed that two generic SPDs could be collapsed into one, so I removed one of the duplicate generic SPDs (SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex), and changed Makefile to collapse volteer's DRAM ID 2 into DRAM ID 0.
BUG=156126658 TEST=Flash and boot a ripto to kernel. Also verified that ripto can boot successfully to the kernel at 4267 MT/sec with FSP built in debug mode with RMT enabled.
Change-Id: Ib52bf674ebf91854d3d078015aa640aa7ee98a6f Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/volteer/Makefile.inc D src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex 2 files changed, 3 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/41345/1
diff --git a/src/mainboard/google/volteer/variants/volteer/Makefile.inc b/src/mainboard/google/volteer/variants/volteer/Makefile.inc index faf0d89..dc94fab 100644 --- a/src/mainboard/google/volteer/variants/volteer/Makefile.inc +++ b/src/mainboard/google/volteer/variants/volteer/Makefile.inc @@ -6,8 +6,8 @@ ##
## Memory Options # DRAM ID # Part Num -SPD_SOURCES = SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267 # 0b0000 # K4U6E3S4AA-MGCL +SPD_SOURCES = SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267 # 0b0000 # K4U6E3S4AA-MGCL + # H9HCNNNBKMMLXR-NEE SPD_SOURCES += SPD_LPDDR4X_200b_8bank_2Rx16_32Gb_DDP_4267 # 0b0001 # K4UBE3D4AA-MGCL -SPD_SOURCES += SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267 # 0b0010 # H9HCNNNBKMMLXR-NEE +SPD_SOURCES += SPD_LPDDR4X_200b_2R_32Gb_QDP_4267 # 0b0010 # MT53E1G32D2NP-046 WT:A SPD_SOURCES += SPD_LPDDR4X_200b_2R_64Gb_ODP_4267 # 0b0011 # H9HCNNNFAMMLXR-NEE -SPD_SOURCES += SPD_LPDDR4X_200b_2R_32Gb_QDP_4267 # 0b0100 # MT53E1G32D2NP-046 WT:A diff --git a/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex b/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex deleted file mode 100644 index 94f258e..0000000 --- a/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex +++ /dev/null @@ -1,32 +0,0 @@ -23 11 11 0E 15 21 95 08 00 00 00 00 02 01 00 00 -48 00 04 00 92 55 00 00 8C 00 90 A8 90 C0 08 60 -04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 7F E1 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fixed error in generic SPD ......................................................................
Patch Set 3:
This change is ready for review.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41345
to look at the new patch set (#4).
Change subject: mb/google/volteer: fixed error in generic SPD ......................................................................
mb/google/volteer: fixed error in generic SPD
The SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex SPD contained an incorrect SDRAM Max Cycle Time (0 instead of 0x0f). After fixing that error, I noticed that two generic SPDs could be collapsed into one, so I removed one of the duplicate generic SPDs (SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_16Row_DDP_4267.spd.hex), and changed Makefile to collapse volteer's DRAM ID 2 into ID 0.
BUG=156126658 TEST=Flash and boot a ripto to kernel. Also verified that ripto can boot successfully to the kernel at 4267 MT/sec with FSP built in debug mode with RMT enabled.
Change-Id: Ib52bf674ebf91854d3d078015aa640aa7ee98a6f Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/volteer/Makefile.inc D src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex R src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex 5 files changed, 6 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/41345/4
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41345
to look at the new patch set (#5).
Change subject: mb/google/volteer: fixed error in generic SPD ......................................................................
mb/google/volteer: fixed error in generic SPD
The SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex SPD contained an incorrect SDRAM Max Cycle Time (0 instead of 0x0f). After fixing that error, I noticed that two generic SPDs could be collapsed into one, so I removed one of the duplicate generic SPDs (SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_16Row_DDP_4267.spd.hex), and changed Makefile to collapse volteer's DRAM ID 2 into ID 0.
BUG=b:156126658, b:156058720 TEST=Flash and boot a ripto to kernel. Also verified that ripto can boot successfully to the kernel at 4267 MT/sec with FSP built in debug mode with RMT enabled.
Change-Id: Ib52bf674ebf91854d3d078015aa640aa7ee98a6f Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/volteer/Makefile.inc D src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex R src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex 5 files changed, 6 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/41345/5
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41345
to look at the new patch set (#7).
Change subject: mb/google/volteer: fixed error in generic SPD ......................................................................
mb/google/volteer: fixed error in generic SPD
The SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex SPD contained an incorrect SDRAM Max Cycle Time (0 instead of 0x0f). After fixing that error, I noticed that two generic SPDs could be collapsed into one, so I removed one of the duplicate generic SPDs (SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_16Row_DDP_4267.spd.hex), and changed Makefile to collapse volteer's DRAM ID 2 into ID 0.
BUG=b:156126658, b:156058720 TEST=Flash and boot a ripto to kernel. Also verified that ripto can boot successfully to the kernel at 4267 MT/sec with FSP built in debug mode with RMT enabled.
Change-Id: Ib52bf674ebf91854d3d078015aa640aa7ee98a6f Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/volteer/Makefile.inc D src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex R src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex 5 files changed, 6 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/41345/7
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fixed error in generic SPD ......................................................................
Patch Set 7: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41345/7//COMMIT_MSG@7 PS7, Line 7: fixed Present tense in imperative mood: Fix
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Caveh Jalali, Paul Menzel, Duncan Laurie, Dossym Nurmukhanov,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41345
to look at the new patch set (#8).
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
mb/google/volteer: fix error in generic SPD
The SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex SPD contained an incorrect SDRAM Max Cycle Time (0 instead of 0x0f). After fixing that error, I noticed that two generic SPDs could be collapsed into one, so I removed one of the duplicate generic SPDs (SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_16Row_DDP_4267.spd.hex), and changed Makefile to collapse volteer's DRAM ID 2 into ID 0.
BUG=b:156126658, b:156058720 TEST=Flash and boot a ripto to kernel. Also verified that ripto can boot successfully to the kernel at 4267 MT/sec with FSP built in debug mode with RMT enabled.
Change-Id: Ib52bf674ebf91854d3d078015aa640aa7ee98a6f Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/volteer/Makefile.inc D src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex R src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex 5 files changed, 6 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/41345/8
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41345/7//COMMIT_MSG@7 PS7, Line 7: fixed
Present tense in imperative mood: Fix
Done
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
ping...
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 8: nit: did the white space police let this one slide?
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 8:
nit: did the white space police let this one slide?
It did.
Checking now, I notice that Hatch has tabs there. All variants of volteer appear to be using spaces. If we want to use tabs here, the change should be made in a separate CL and performed on all volteer variants. If you think we should change these, please file a bug against me and I'll make the change.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 8:
It did. […]
leading tabs in makefiles make me nervous because they have semantic significance... in most places. this is not one of them... i had to double check 😊
starting the line with '#' would be safest.
if you want to scrub white spaces as a follow-up, i'm OK with that.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 1: ## : ## : ## : ## SPDX-License-Identifier: GPL-2.0-or-later : ## nit: may as well clean this up, I think someone went through and erased all of the "This file belongs to coreboot" text.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 1: ## : ## : ## : ## SPDX-License-Identifier: GPL-2.0-or-later : ##
nit: may as well clean this up, I think someone went through and erased all of the "This file belong […]
I'll clean this up in the follow-up cleanup CL discussed in comment below.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 1: ## : ## : ## : ## SPDX-License-Identifier: GPL-2.0-or-later : ##
I'll clean this up in the follow-up cleanup CL discussed in comment below.
Tim, what should be done to clean this up? Strip lines 1 and 5 ? Looks like all of Hatch variant's Makefile.inc files inside their variants volter look similar (i.e. no other info in file header comment except SPDX license header).
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 1: ## : ## : ## : ## SPDX-License-Identifier: GPL-2.0-or-later : ##
Tim, what should be done to clean this up? Strip lines 1 and 5 ? Looks like all of Hatch variant's […]
Just the one line: ## SPDX-License-Identifier: GPL-2.0-or-later Yeah, I think somebody went through with a script and removed the "This file is part of the coreboot project" line, so it used to look like:
## ## This file is part of the coreboot project ## ## SPDX-License-Identifier: GPL-2.0-or-later ## which looked slightly more sane than what it is today.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/8/src/mainboard/google/voltee... PS8, Line 1: ## : ## : ## : ## SPDX-License-Identifier: GPL-2.0-or-later : ##
Just the one line: […]
There have been a few more clean up scripts coming in later to clean up these extra lines, but these files might have escaped that part of the effort.
Nick Vaccaro has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
mb/google/volteer: fix error in generic SPD
The SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex SPD contained an incorrect SDRAM Max Cycle Time (0 instead of 0x0f). After fixing that error, I noticed that two generic SPDs could be collapsed into one, so I removed one of the duplicate generic SPDs (SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_16Row_DDP_4267.spd.hex), and changed Makefile to collapse volteer's DRAM ID 2 into ID 0.
BUG=b:156126658, b:156058720 TEST=Flash and boot a ripto to kernel. Also verified that ripto can boot successfully to the kernel at 4267 MT/sec with FSP built in debug mode with RMT enabled.
Change-Id: Ib52bf674ebf91854d3d078015aa640aa7ee98a6f Signed-off-by: Nick Vaccaro nvaccaro@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41345 Reviewed-by: Caveh Jalali caveh@chromium.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex M src/mainboard/google/volteer/variants/volteer/Makefile.inc D src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex R src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex 5 files changed, 6 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Caveh Jalali: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex b/src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex index 94f258e..1ead4c5 100644 --- a/src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex +++ b/src/mainboard/google/volteer/variants/malefor/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex @@ -1,5 +1,5 @@ 23 11 11 0E 15 21 95 08 00 00 00 00 02 01 00 00 -48 00 04 00 92 55 00 00 8C 00 90 A8 90 C0 08 60 +48 00 04 0F 92 55 00 00 8C 00 90 A8 90 C0 08 60 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 diff --git a/src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex b/src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex index 94f258e..1ead4c5 100644 --- a/src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex +++ b/src/mainboard/google/volteer/variants/ripto/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex @@ -1,5 +1,5 @@ 23 11 11 0E 15 21 95 08 00 00 00 00 02 01 00 00 -48 00 04 00 92 55 00 00 8C 00 90 A8 90 C0 08 60 +48 00 04 0F 92 55 00 00 8C 00 90 A8 90 C0 08 60 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 diff --git a/src/mainboard/google/volteer/variants/volteer/Makefile.inc b/src/mainboard/google/volteer/variants/volteer/Makefile.inc index 5b97e8d..07dbef7 100644 --- a/src/mainboard/google/volteer/variants/volteer/Makefile.inc +++ b/src/mainboard/google/volteer/variants/volteer/Makefile.inc @@ -2,8 +2,8 @@ ##
## Memory Options # DRAM ID # Part Num -SPD_SOURCES = SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267 # 0b0000 # K4U6E3S4AA-MGCL +SPD_SOURCES = SPD_LPDDR4X_200b_1R_16Gb_DDP_4267 # 0b0000 # K4U6E3S4AA-MGCL + # H9HCNNNBKMMLXR-NEE SPD_SOURCES += SPD_LPDDR4X_200b_8bank_2Rx16_32Gb_DDP_4267 # 0b0001 # K4UBE3D4AA-MGCL -SPD_SOURCES += SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267 # 0b0010 # H9HCNNNBKMMLXR-NEE +SPD_SOURCES += SPD_LPDDR4X_200b_2R_32Gb_QDP_4267 # 0b0010 # MT53E1G32D2NP-046 WT:A SPD_SOURCES += SPD_LPDDR4X_200b_2R_64Gb_ODP_4267 # 0b0011 # H9HCNNNFAMMLXR-NEE -SPD_SOURCES += SPD_LPDDR4X_200b_2R_32Gb_QDP_4267 # 0b0100 # MT53E1G32D2NP-046 WT:A diff --git a/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex b/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex deleted file mode 100644 index 9ec70a9..0000000 --- a/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_16Row_DDP_4267.spd.hex +++ /dev/null @@ -1,32 +0,0 @@ -23 11 11 0E 15 21 95 08 00 00 00 00 02 01 00 00 -48 00 04 FF 92 55 00 00 8C 00 90 A8 90 C0 08 60 -04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 7F E1 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 diff --git a/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex b/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex similarity index 96% rename from src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex rename to src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex index 94f258e..1ead4c5 100644 --- a/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_8bank_1Rx16_16Gb_DDP_4267.spd.hex +++ b/src/mainboard/google/volteer/variants/volteer/spd/SPD_LPDDR4X_200b_1R_16Gb_DDP_4267.spd.hex @@ -1,5 +1,5 @@ 23 11 11 0E 15 21 95 08 00 00 00 00 02 01 00 00 -48 00 04 00 92 55 00 00 8C 00 90 A8 90 C0 08 60 +48 00 04 0F 92 55 00 00 8C 00 90 A8 90 C0 08 60 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 9:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3822 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3821 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3820 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3819
Please note: This test is under development and might not be accurate at all!
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 7: H9HCNNNBKMMLXR-NEE This is wrong. Why was the ID for H9HCNNNBKMMLXR-NEE changed? Isn't this ID assigned to the memory part already and configured on volteer boards?
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 9: MT53E1G32D2NP-046 WT:A Same here. Why were the IDs changed?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 5: K4U6E3S4AA-MGCL : # H9HCNNNBKMMLXR-NEE I don't think these two parts can really be combined together. See the details of memory parts here: https://review.coreboot.org/c/coreboot/+/41613/8/src/mainboard/google/voltee...
As per datasheet, K4U6E3S4AA-MGCL has 1 die per package whereas H9HCNNNBKMMLXR-NEE has 2 dies per package.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 9:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 7: H9HCNNNBKMMLXR-NEE
This is wrong. […]
No, we haven't seen that memory part yet. It's a new part that's planned for EVT.
It was changed because once I fixed an error in the DRAM ID 0 SPD, I saw that the SPD for this part was identical to the SPD for DRAM ID 0 (after the fix).
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 9: MT53E1G32D2NP-046 WT:A
Same here. […]
Numbers were collapsed to be sequential, and I notified variant projects planning to use said parts, and they have acknowledged and granted approval of the change.
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 5: K4U6E3S4AA-MGCL : # H9HCNNNBKMMLXR-NEE
I don't think these two parts can really be combined together. […]
That implies that the SPD for our Ripto memory part is incorrect, yet we haven't seen any issues with that part yet.
IIUC, this is the SPD that intel qualified for that part : https://drive.google.com/file/d/1M3kPHAEWp4J0j2GUZ50KzzDeB5SmR4hc/view
based on what I see here : https://docs.google.com/spreadsheets/d/1SJ7GNbl1heYBQI22E798cII_VZEeavLAuDLo...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 9: MT53E1G32D2NP-046 WT:A
Numbers were collapsed to be sequential, and I notified variant projects planning to use said parts, […]
Do you have all the tables captured in one place regarding what is provided to ODMs? I want to make sure that we can keep using the same IDs, but for that we need to ensure that the memory part used list (https://review.coreboot.org/c/coreboot/+/41617/8/src/mainboard/google/voltee...) matches what was assigned before. Also, do we know by when we can make changes to the ID numbers?
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 5: K4U6E3S4AA-MGCL : # H9HCNNNBKMMLXR-NEE
That implies that the SPD for our Ripto memory part is incorrect, yet we haven't seen any issues wit […]
That is not what the datasheet says though. As per datasheet, K4U6E3S4AA-MGCL is a single die package: https://drive.google.com/corp/drive/u/0/search?q=K4U6E3S4AA-MGCL
The 3S stands for monolithic package.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 9: MT53E1G32D2NP-046 WT:A
Do you have all the tables captured in one place regarding what is provided to ODMs? I want to make […]
I had updated the spread sheet as I was going along, but SPDs kept getting hung up or changed in review, so I decided to stop wasting work and wait until all SPDs were approved an in before making those changes. I will be updating that list, once I make sure DRAM IDs aor SPDs aren't going to change based on this discussion.
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 5: K4U6E3S4AA-MGCL : # H9HCNNNBKMMLXR-NEE
That is not what the datasheet says though. […]
This may be an issue of Samsung/Intel misunderstanding. For instance, the Samsung data sheet for the K4UBE3D4AA-MGCL part states 2 die, but Intel explained that it's really an equivalent of a 4-die and categorizes it as a QDP.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41345 )
Change subject: mb/google/volteer: fix error in generic SPD ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41345/9/src/mainboard/google/voltee... PS9, Line 5: K4U6E3S4AA-MGCL : # H9HCNNNBKMMLXR-NEE
This may be an issue of Samsung/Intel misunderstanding. […]
I have posted a question on b/148182234. Do you understand how the logical organization maps to SPD bytes?
-
9elements QA (Code Review) -
Caveh Jalali (Code Review)
-
Furquan Shaikh (Code Review)
-
Nick Vaccaro (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)