Change in coreboot[master]: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Mike Banon (Code Review)

17 Apr 2020 17 Apr '20

10:47 a.m.

Mike Banon has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40484 )

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

nb/amd/agesa: read 256 bytes to SPD buffer instead of 128

Required for adding the XMP profiles support. SPD buffer is already 256 bytes at AMD AGESA vendorcode, so this is fine.

Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f --- M src/northbridge/amd/agesa/family14/dimmSpd.c M src/northbridge/amd/agesa/family15tn/dimmSpd.c M src/northbridge/amd/agesa/family16kb/dimmSpd.c 3 files changed, 3 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/40484/1

diff --git a/src/northbridge/amd/agesa/family14/dimmSpd.c b/src/northbridge/amd/agesa/family14/dimmSpd.c index e9eeb49..fedf53c 100644 --- a/src/northbridge/amd/agesa/family14/dimmSpd.c +++ b/src/northbridge/amd/agesa/family14/dimmSpd.c @@ -41,7 +41,7 @@ if (spdAddress == 0) return AGESA_ERROR;

- int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 128); + int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 256); if (err) return AGESA_ERROR; return AGESA_SUCCESS; diff --git a/src/northbridge/amd/agesa/family15tn/dimmSpd.c b/src/northbridge/amd/agesa/family15tn/dimmSpd.c index 761b8b8..498488a 100644 --- a/src/northbridge/amd/agesa/family15tn/dimmSpd.c +++ b/src/northbridge/amd/agesa/family15tn/dimmSpd.c @@ -40,7 +40,7 @@ if (spdAddress == 0) return AGESA_ERROR;

- int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 128); + int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 256); if (err) return AGESA_ERROR; return AGESA_SUCCESS; diff --git a/src/northbridge/amd/agesa/family16kb/dimmSpd.c b/src/northbridge/amd/agesa/family16kb/dimmSpd.c index 6c751f9..3a6192c 100644 --- a/src/northbridge/amd/agesa/family16kb/dimmSpd.c +++ b/src/northbridge/amd/agesa/family16kb/dimmSpd.c @@ -40,7 +40,7 @@ if (spdAddress == 0) return AGESA_ERROR;

- int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 128); + int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 256); if (err) return AGESA_ERROR; return AGESA_SUCCESS;

Attachments:

attachment.htm (text/html — 4.2 KB)

Show replies by date

Michał Żygowski (Code Review)

17 Apr 17 Apr

12:12 p.m.

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40484 )

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family14/dimmSpd.c:

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... PS1, Line 44: int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 256); Shouldn't it be tied to CONFIG_DIMM_SPD_SIZE? If you would like to use XMP profile, you may add a Kconfig option to enable XMP profiles which will modify the DIMM_SPD_SIZE for your boards.

-- To view, visit https://review.coreboot.org/c/coreboot/+/40484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Gerrit-Change-Number: 40484 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Banon mikebdp2@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Fri, 17 Apr 2020 12:12:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Mike Banon (Code Review)

12:29 p.m.

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40484 )

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family14/dimmSpd.c:

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... PS1, Line 44: int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 256);

...

Shouldn't it be tied to CONFIG_DIMM_SPD_SIZE? If you would like to use XMP profile, you may add a Kc […]

Maybe CONFIG_DIMM_SPD_SIZE isn't needed: regardless of if a particular DDR3 RAM stick provides the XMP profiles or not, its' SPD memory is always 256 bytes according to JEDEC specification - https://github.com/mikebdp2/ddr3spd/blob/master/JEDEC_DDR3_SPD_4_01_02_11R24... . It wouldn't hurt to read the extra 128 bytes from SPD even if you wouldn't use the XMP profile stored in it.

-- To view, visit https://review.coreboot.org/c/coreboot/+/40484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Gerrit-Change-Number: 40484 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Banon mikebdp2@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Fri, 17 Apr 2020 12:29:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-MessageType: comment

Michał Żygowski (Code Review)

1:36 p.m.

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40484 )

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family14/dimmSpd.c:

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... PS1, Line 44: int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 256);

...

Maybe CONFIG_DIMM_SPD_SIZE isn't needed: regardless of if a particular DDR3 RAM stick provides the X […]

Some of the boards may rely on 128 bytes of SPD. Have you checked those?

-- To view, visit https://review.coreboot.org/c/coreboot/+/40484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Gerrit-Change-Number: 40484 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Banon mikebdp2@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Fri, 17 Apr 2020 13:36:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski michal.zygowski@3mdeb.com Comment-In-Reply-To: Mike Banon mikebdp2@gmail.com Gerrit-MessageType: comment

Mike Banon (Code Review)

5:24 p.m.

Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40484 )

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family14/dimmSpd.c:

https://review.coreboot.org/c/coreboot/+/40484/1/src/northbridge/amd/agesa/f... PS1, Line 44: int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 256);

...

Some of the boards may rely on 128 bytes of SPD. […]

I can't find any boards which have CONFIG_DIMM_SPD_SIZE defined as 128, but even if those existed (contrary to JEDEC spec), the extra read bytes will be simply unused and won't hurt.

-- To view, visit https://review.coreboot.org/c/coreboot/+/40484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Gerrit-Change-Number: 40484 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Banon mikebdp2@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Fri, 17 Apr 2020 17:24:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski michal.zygowski@3mdeb.com Comment-In-Reply-To: Mike Banon mikebdp2@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

14 Aug 14 Aug

3:42 p.m.

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

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40484/2/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family16kb/dimmSpd.c:

https://review.coreboot.org/c/coreboot/+/40484/2/src/northbridge/amd/agesa/f... PS2, Line 42: 256 for another patch: #define this number somewhere

-- To view, visit https://review.coreboot.org/c/coreboot/+/40484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Gerrit-Change-Number: 40484 Gerrit-PatchSet: 2 Gerrit-Owner: Mike Banon mikebdp2@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 14 Aug 2020 15:42:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

15 Aug 15 Aug

1 p.m.

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

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40484/2/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family16kb/dimmSpd.c:

https://review.coreboot.org/c/coreboot/+/40484/2/src/northbridge/amd/agesa/f... PS2, Line 42: 256

...

for another patch: #define this number somewhere

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/40484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Gerrit-Change-Number: 40484 Gerrit-PatchSet: 3 Gerrit-Owner: Mike Banon mikebdp2@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 15 Aug 2020 13:00:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

17 Aug 17 Aug

7:13 a.m.

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

Change subject: nb/amd/agesa: read 256 bytes to SPD buffer instead of 128 ......................................................................

nb/amd/agesa: read 256 bytes to SPD buffer instead of 128

Required for adding the XMP profiles support. SPD buffer is already 256 bytes at AMD AGESA vendorcode, so this is fine.

Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Reviewed-on: https://review.coreboot.org/c/coreboot/+/40484 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/amd/agesa/family14/dimmSpd.c M src/northbridge/amd/agesa/family15tn/dimmSpd.c M src/northbridge/amd/agesa/family16kb/dimmSpd.c 3 files changed, 3 insertions(+), 3 deletions(-)

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

diff --git a/src/northbridge/amd/agesa/family14/dimmSpd.c b/src/northbridge/amd/agesa/family14/dimmSpd.c index ac6a38d..e4d1eab 100644 --- a/src/northbridge/amd/agesa/family14/dimmSpd.c +++ b/src/northbridge/amd/agesa/family14/dimmSpd.c @@ -40,7 +40,7 @@ if (spdAddress == 0) return AGESA_ERROR;

- int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 128); + int err = smbus_readSpd(spdAddress, (void *) info->Buffer, 256); if (err) return AGESA_ERROR; return AGESA_SUCCESS; diff --git a/src/northbridge/amd/agesa/family15tn/dimmSpd.c b/src/northbridge/amd/agesa/family15tn/dimmSpd.c index 17e8aa8..6504d14 100644 --- a/src/northbridge/amd/agesa/family15tn/dimmSpd.c +++ b/src/northbridge/amd/agesa/family15tn/dimmSpd.c @@ -39,7 +39,7 @@ if (spdAddress == 0) return AGESA_ERROR;

- int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 128); + int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 256); if (err) return AGESA_ERROR; return AGESA_SUCCESS; diff --git a/src/northbridge/amd/agesa/family16kb/dimmSpd.c b/src/northbridge/amd/agesa/family16kb/dimmSpd.c index b0cac6f..b20216f 100644 --- a/src/northbridge/amd/agesa/family16kb/dimmSpd.c +++ b/src/northbridge/amd/agesa/family16kb/dimmSpd.c @@ -39,7 +39,7 @@ if (spdAddress == 0) return AGESA_ERROR;

- int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 128); + int err = hudson_readSpd(spdAddress, (void *) info->Buffer, 256); if (err) return AGESA_ERROR; return AGESA_SUCCESS;

-- To view, visit https://review.coreboot.org/c/coreboot/+/40484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7340b110477a4cc1ecb1c239181436e51952568f Gerrit-Change-Number: 40484 Gerrit-PatchSet: 4 Gerrit-Owner: Mike Banon mikebdp2@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1591

days inactive

1713

days old

coreboot-gerrit@coreboot.org

7 comments

4 participants

tags (0)

participants (4)

Angel Pons (Code Review)
Michał Żygowski (Code Review)
Mike Banon (Code Review)
Patrick Georgi (Code Review)