Change in ...coreboot[master]: mb/google/poppy: Ensure SPD index is non-zero

[Jacob Garber (Code Review)]

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33183

Change subject: mb/google/poppy: Ensure SPD index is non-zero
......................................................................

mb/google/poppy: Ensure SPD index is non-zero

Memory id's are 1-indexed, so we need to check that the SPD index is
non-zero before converting it to the 0-indexed value in the bitmap.

Change-Id: Icc542239d91c39b89c23f31856c28e7c20b2fc4d
Signed-off-by: Jacob Garber 
Found-by: Coverity CID 1387028
---
M src/mainboard/google/poppy/variants/nami/memory.c
1 file changed, 5 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/33183/1

diff --git a/src/mainboard/google/poppy/variants/nami/memory.c b/src/mainboard/google/poppy/variants/nami/memory.c
index b7f5185..b6e3d2c 100644
--- a/src/mainboard/google/poppy/variants/nami/memory.c
+++ b/src/mainboard/google/poppy/variants/nami/memory.c
@@ -66,8 +66,12 @@
    p->type = MEMORY_DDR4;
    p->use_sec_spd = 0;

+	int spd_index = variant_memory_sku();
+	if (spd_index == 0)
+		die("SPD index is 0\n");
+
    /* Rcomp resistor values are different for SDP and DDP. */
-	if (ddp_bitmap & MEM_ID(variant_memory_sku())) {
+	if (ddp_bitmap & MEM_ID(spd_index)) {
    	p->rcomp_resistor = rcomp_resistor_ddp;
    	p->rcomp_resistor_size = sizeof(rcomp_resistor_ddp);
    } else {

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/poppy: Ensure SPD index is non-zero
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/33183/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33183/1//COMMIT_MSG@7 
PS1, Line 7: :
/var/nami:

since this change is specifically for nami.

https://review.coreboot.org/#/c/33183/1//COMMIT_MSG@9 
PS1, Line 9: ,
for DDR4

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Jacob Garber (Code Review)]

Hello Shelley Chen, build bot (Jenkins),

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

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

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

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

mb/google/poppy/var/nami: Ensure SPD index is non-zero

Memory id's are 1-indexed for DRR4, so we need to check that the SPD
index is non-zero before converting it to the 0-indexed value in the
bitmap.

Change-Id: Icc542239d91c39b89c23f31856c28e7c20b2fc4d
Signed-off-by: Jacob Garber 
Found-by: Coverity CID 1387028
---
M src/mainboard/google/poppy/variants/nami/memory.c
1 file changed, 5 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/33183/2

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33183 )

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/#/c/33183/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33183/1//COMMIT_MSG@7 
PS1, Line 7: :
> /var/nami: […]

Done

https://review.coreboot.org/#/c/33183/1//COMMIT_MSG@9 
PS1, Line 9: ,
> for DDR4

Done

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

Patch Set 2: Code-Review+2

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33183 )

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/33183/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33183/2//COMMIT_MSG@9 
PS2, Line 9: DRR4
DDR4

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Jacob Garber (Code Review)]

Hello David Hendricks, Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,

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

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

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

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

mb/google/poppy/var/nami: Ensure SPD index is non-zero

Memory id's are 1-indexed for DDR4, so we need to check that the SPD
index is non-zero before converting it to the 0-indexed value in the
bitmap.

Change-Id: Icc542239d91c39b89c23f31856c28e7c20b2fc4d
Signed-off-by: Jacob Garber 
Found-by: Coverity CID 1387028
---
M src/mainboard/google/poppy/variants/nami/memory.c
1 file changed, 5 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/33183/3

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33183 )

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/33183/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33183/2//COMMIT_MSG@9 
PS2, Line 9: DRR4
> DDR4

Oops

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33183 )

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

Patch Set 3: Code-Review+2

Change in ...coreboot[master]: mb/google/poppy/var/nami: Ensure SPD index is non-zero

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33183 )

Change subject: mb/google/poppy/var/nami: Ensure SPD index is non-zero
......................................................................

mb/google/poppy/var/nami: Ensure SPD index is non-zero

Memory id's are 1-indexed for DDR4, so we need to check that the SPD
index is non-zero before converting it to the 0-indexed value in the
bitmap.

Change-Id: Icc542239d91c39b89c23f31856c28e7c20b2fc4d
Signed-off-by: Jacob Garber 
Found-by: Coverity CID 1387028
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33183
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
---
M src/mainboard/google/poppy/variants/nami/memory.c
1 file changed, 5 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/mainboard/google/poppy/variants/nami/memory.c b/src/mainboard/google/poppy/variants/nami/memory.c
index b7f5185..b6e3d2c 100644
--- a/src/mainboard/google/poppy/variants/nami/memory.c
+++ b/src/mainboard/google/poppy/variants/nami/memory.c
@@ -66,8 +66,12 @@
    p->type = MEMORY_DDR4;
    p->use_sec_spd = 0;

+	int spd_index = variant_memory_sku();
+	if (spd_index == 0)
+		die("SPD index is 0\n");
+
    /* Rcomp resistor values are different for SDP and DDP. */
-	if (ddp_bitmap & MEM_ID(variant_memory_sku())) {
+	if (ddp_bitmap & MEM_ID(spd_index)) {
    	p->rcomp_resistor = rcomp_resistor_ddp;
    	p->rcomp_resistor_size = sizeof(rcomp_resistor_ddp);
    } else {