Change in coreboot[master]: lp4x: Add new memory parts and generate SPDs

[David Wu (Code Review)]

David Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

lp4x: Add new memory parts and generate SPDs

This change adds the following memory parts to LP4x global list and
generates SPDs using gen_spd.go for TGL:
1. H9HCNNNCRMBLPR-NEE
2. H9HCNNNFBMBLPR-NEE
3. MT53D1G64D4NW-046 WT:A

BUG=b:172751925
TEST=build.

Signed-off-by: David Wu 
Change-Id: I37702770f707fe078920694468552c5db59c478f
---
M src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
M util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
2 files changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/47350/1

diff --git a/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt b/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
index 70877d2..c7e9690 100644
--- a/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
+++ b/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
@@ -13,3 +13,6 @@
 K4UBE3D4AA-MGCR,lp4x-spd-3.hex
 MT53E512M64D4NW-046 WT:E,lp4x-spd-1.hex
 MT53E1G64D8NW-046 WT:E,lp4x-spd-3.hex
+H9HCNNNCRMBLPR-NEE,lp4x-spd-1.hex
+H9HCNNNFBMBLPR-NEE,lp4x-spd-3.hex
+MT53D1G64D4NW-046 WT:A,lp4x-spd-4.hex
diff --git a/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt b/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
index 109fadb..91062d0 100644
--- a/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
+++ b/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
@@ -181,6 +181,42 @@
                 "ranksPerChannel": 2,
                 "speedMbps": 4267
             }
+        },
+        {
+            "name": "H9HCNNNCRMBLPR-NEE",
+            "attribs": {
+                "densityPerChannelGb": 8,
+                "banks": 8,
+                "channelsPerDie": 2,
+                "diesPerPackage": 2,
+                "bitWidthPerChannel": 16,
+                "ranksPerChannel": 1,
+                "speedMbps": 4267
+            }
+        },
+        {
+            "name": "H9HCNNNFBMBLPR-NEE",
+            "attribs": {
+                "densityPerChannelGb": 8,
+                "banks": 8,
+                "channelsPerDie": 2,
+                "diesPerPackage": 4,
+                "bitWidthPerChannel": 16,
+                "ranksPerChannel": 2,
+                "speedMbps": 4267
+            }
+        },
+        {
+            "name": "MT53D1G64D4NW-046 WT:A",
+            "attribs": {
+                "densityPerChannelGb": 16,
+                "banks": 8,
+                "channelsPerDie": 2,
+                "diesPerPackage": 2,
+                "bitWidthPerChannel": 16,
+                "ranksPerChannel": 1,
+                "speedMbps": 4267
+            }
         }
     ]
 }

[David Wu (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Nick Vaccaro, Zhuohao Lee, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/47350

to look at the new patch set (#2).

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

lp4x: Add new memory parts and generate SPDs

This change adds the following memory parts to LP4x global list and
generates SPDs using gen_spd.go for TGL:
1. H9HCNNNCRMBLPR-NEE
2. H9HCNNNFBMBLPR-NEE
3. MT53D1G64D4NW-046 WT:A

BUG=b:172751925,b:172781673,b:172782100,b:172781562
TEST=build.

Signed-off-by: David Wu 
Change-Id: I37702770f707fe078920694468552c5db59c478f
---
M src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
M util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
2 files changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/47350/2

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

Patch Set 2: Code-Review+1

[Caveh Jalali (Code Review)]

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

Patch Set 2: Code-Review+1

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47350/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47350/1//COMMIT_MSG@10 
PS1, Line 10: generates SPDs using gen_spd.go for TGL
This CL is not adding any new SPDs, nor are any new generic SPDs generated by adding these three parts to the global manifest. It uses three generic SPDs that already exist.  Perhaps a better summary would be something like "This change adds the following memory parts to LP4x global list of available LP4x parts and to the global JSON file containing LP4x parts and their characteristics." or something like that?

https://review.coreboot.org/c/coreboot/+/47350/1//COMMIT_MSG@16 
PS1, Line 16: build
Please include commands used to verify these changes don't break SPD generation.

e.g. cd <path_to_coreboot_src>/util/spd_tools/lp4x && ./gen_spd <path_to_coreboot_src>/src/soc/intel/tigerlake/spd ./global_lp4x_mem_parts.json.txt "TGL"

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
File util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt:

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 191: 2
Is this correct?  Page 3 of the spec attached to the bug shows the part as being a QDP.

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 203: 4
Is this correct?  Page 3 of part spec attached to bug shows this part to be an ODP.

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 215:  2
Is this correct?  Page 2 of part spec attached to bug shows this part to be a QDP.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
File util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt:

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 191: 2
> Is this correct?  Page 3 of the spec attached to the bug shows the part as being a QDP.

As per [1], "Thus, number of diesPerPackage is the number of ZQ balls on the package." As per the part datasheet, there are 2 ZQ pins (ZQ0_A and ZQ0_C), hence 2 dies per package.

[1] https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/a98f80fc..."

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 203: 4
> Is this correct?  Page 3 of part spec attached to bug shows this part to be an ODP.

Please check as per the comment above.

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 215:  2
> Is this correct?  Page 2 of part spec attached to bug shows this part to be a QDP.

Please check as per the comment above.

[David Wu (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Tim Wawrzynczak, Paul Fagerburg, Nick Vaccaro, Zhuohao Lee, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/47350

to look at the new patch set (#3).

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

lp4x: Add new memory parts and generate SPDs

This change adds the following memory parts to LP4x global list of
available LP4x parts and to the global JSON file containing LP4x parts
and their characteristics.
1. H9HCNNNCRMBLPR-NEE
2. H9HCNNNFBMBLPR-NEE
3. MT53D1G64D4NW-046 WT:A

BUG=b:172751925,b:172781673,b:172782100,b:172781562
TEST=cd <path_to_coreboot_src>/util/spd_tools/lp4x &&
./gen_spd <path_to_coreboot_src>/src/soc/intel/tigerlake/spd \
global_lp4x_mem_parts.json.txt "TGL"

Signed-off-by: David Wu 
Change-Id: I37702770f707fe078920694468552c5db59c478f
---
M src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
M util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
2 files changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/47350/3

[David Wu (Code Review)]

David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47350/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47350/1//COMMIT_MSG@10 
PS1, Line 10: generates SPDs using gen_spd.go for TGL
> This CL is not adding any new SPDs, nor are any new generic SPDs generated by adding these three par […]

Done

https://review.coreboot.org/c/coreboot/+/47350/1//COMMIT_MSG@16 
PS1, Line 16: build
> Please include commands used to verify these changes don't break SPD generation. […]

Done

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

Patch Set 3: Code-Review+2

(3 comments)

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
File util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt:

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 191: 2
> As per [1], "Thus, number of diesPerPackage is the number of ZQ balls on the package. […]

Thanks, Furquan.

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 203: 4
> Please check as per the comment above.

Done

https://review.coreboot.org/c/coreboot/+/47350/1/util/spd_tools/lp4x/global_... 
PS1, Line 215:  2
> Please check as per the comment above.

Done

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47350 )

Change subject: lp4x: Add new memory parts and generate SPDs
......................................................................

lp4x: Add new memory parts and generate SPDs

This change adds the following memory parts to LP4x global list of
available LP4x parts and to the global JSON file containing LP4x parts
and their characteristics.
1. H9HCNNNCRMBLPR-NEE
2. H9HCNNNFBMBLPR-NEE
3. MT53D1G64D4NW-046 WT:A

BUG=b:172751925,b:172781673,b:172782100,b:172781562
TEST=cd <path_to_coreboot_src>/util/spd_tools/lp4x &&
./gen_spd <path_to_coreboot_src>/src/soc/intel/tigerlake/spd \
global_lp4x_mem_parts.json.txt "TGL"

Signed-off-by: David Wu 
Change-Id: I37702770f707fe078920694468552c5db59c478f
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47350
Reviewed-by: Nick Vaccaro 
Reviewed-by: Paul Fagerburg 
Reviewed-by: Caveh Jalali 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
M util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
2 files changed, 39 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nick Vaccaro: Looks good to me, approved
  Caveh Jalali: Looks good to me, but someone else must approve
  Paul Fagerburg: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt b/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
index 70877d2..c7e9690 100644
--- a/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
+++ b/src/soc/intel/tigerlake/spd/lp4x_spd_manifest.generated.txt
@@ -13,3 +13,6 @@
 K4UBE3D4AA-MGCR,lp4x-spd-3.hex
 MT53E512M64D4NW-046 WT:E,lp4x-spd-1.hex
 MT53E1G64D8NW-046 WT:E,lp4x-spd-3.hex
+H9HCNNNCRMBLPR-NEE,lp4x-spd-1.hex
+H9HCNNNFBMBLPR-NEE,lp4x-spd-3.hex
+MT53D1G64D4NW-046 WT:A,lp4x-spd-4.hex
diff --git a/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt b/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
index 109fadb..91062d0 100644
--- a/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
+++ b/util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt
@@ -181,6 +181,42 @@
                 "ranksPerChannel": 2,
                 "speedMbps": 4267
             }
+        },
+        {
+            "name": "H9HCNNNCRMBLPR-NEE",
+            "attribs": {
+                "densityPerChannelGb": 8,
+                "banks": 8,
+                "channelsPerDie": 2,
+                "diesPerPackage": 2,
+                "bitWidthPerChannel": 16,
+                "ranksPerChannel": 1,
+                "speedMbps": 4267
+            }
+        },
+        {
+            "name": "H9HCNNNFBMBLPR-NEE",
+            "attribs": {
+                "densityPerChannelGb": 8,
+                "banks": 8,
+                "channelsPerDie": 2,
+                "diesPerPackage": 4,
+                "bitWidthPerChannel": 16,
+                "ranksPerChannel": 2,
+                "speedMbps": 4267
+            }
+        },
+        {
+            "name": "MT53D1G64D4NW-046 WT:A",
+            "attribs": {
+                "densityPerChannelGb": 16,
+                "banks": 8,
+                "channelsPerDie": 2,
+                "diesPerPackage": 2,
+                "bitWidthPerChannel": 16,
+                "ranksPerChannel": 1,
+                "speedMbps": 4267
+            }
         }
     ]
 }