August 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: ich_descriptors: Fix off-by-one error
by Nico Huber (Code Review) Aug. 10, 2017

Aug. 10, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20937 ) Change subject: ich_descriptors: Fix off-by-one error ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/20937/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/20937/1//COMMIT_MSG@9 PS1, Line 9: The "number of masters" (NM) reflects the value that begins at 1. Seems that this is right for Lewisburg, awesome! more mess for us to handle. I've never found a definition if it starts at 0 or 1 and past practice was to count from 0 (== 1 master)! All descriptors in the wild that I've encountered were for platforms with 3 masters and always had NM set to 2 ;) Now the SPI guide for Lewisburg states it should be 6 (and there are only 6 masters mentioned), hmmm. I guess the lack of an explicit definition confused Intel's developers even more than us and now they disagree. No prob- lem for them, as they make one (binary) program for each platform anyway (I suppose). Two solutions come to mind: Take the guessed platform into account (Lewisburg would need its own constant). Or we could ignore a too high NM and just return MAX_NUM_MASTERS (and drop a lot of sanity checks as we could return an unsigned value). https://review.coreboot.org/#/c/20937/1//COMMIT_MSG@17 PS1, Line 17: even more wrong than it was before. I don't see any loop doing it wrong, nor any caller adding 1 to the return value. -- To view, visit https://review.coreboot.org/20937 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I92627332265cf79720b98241d73bc36b1f6a7167 Gerrit-Change-Number: 20937 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 10 Aug 2017 11:55:24 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: ich_descriptors: Modify limits for C620/Lewisburg PCH
by Nico Huber (Code Review) Aug. 10, 2017

Aug. 10, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20936 ) Change subject: ich_descriptors: Modify limits for C620/Lewisburg PCH ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/20936 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ic8adc4b87993e65096166fa6d665432697070b4c Gerrit-Change-Number: 20936 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 10 Aug 2017 11:34:49 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: ich_descriptors: Fix off-by-one error
by Paul Menzel (Code Review) Aug. 10, 2017

Aug. 10, 2017

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/20937 ) Change subject: ich_descriptors: Fix off-by-one error ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/20937 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I92627332265cf79720b98241d73bc36b1f6a7167 Gerrit-Change-Number: 20937 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 10 Aug 2017 09:43:38 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: ich_descriptors: Modify limits for C620/Lewisburg PCH
by Paul Menzel (Code Review) Aug. 10, 2017

Aug. 10, 2017

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/20936 ) Change subject: ich_descriptors: Modify limits for C620/Lewisburg PCH ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/20936 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ic8adc4b87993e65096166fa6d665432697070b4c Gerrit-Change-Number: 20936 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 10 Aug 2017 09:43:03 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: ich_descriptors: Fix off-by-one error
by build bot (Jenkins) (Code Review) Aug. 10, 2017

Aug. 10, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20937 ) Change subject: ich_descriptors: Fix off-by-one error ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/505/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/467/ : SUCCESS -- To view, visit https://review.coreboot.org/20937 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I92627332265cf79720b98241d73bc36b1f6a7167 Gerrit-Change-Number: 20937 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 10 Aug 2017 05:44:18 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: ich_descriptors: Modify limits for C620/Lewisburg PCH
by build bot (Jenkins) (Code Review) Aug. 10, 2017

Aug. 10, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20936 ) Change subject: ich_descriptors: Modify limits for C620/Lewisburg PCH ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/504/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/466/ : SUCCESS -- To view, visit https://review.coreboot.org/20936 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ic8adc4b87993e65096166fa6d665432697070b4c Gerrit-Change-Number: 20936 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 10 Aug 2017 05:44:13 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: ich_descriptors: Fix off-by-one error
by David Hendricks (Code Review) Aug. 10, 2017

Aug. 10, 2017

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/20937 Change subject: ich_descriptors: Fix off-by-one error ...................................................................... ich_descriptors: Fix off-by-one error The "number of masters" (NM) reflects the value that begins at 1. This value is compared with the constant MAX_NUM_MASTERS, which also begins with 1. So we really do want the raw value of NM to be used in the comparison. This fixes problems that arise when the actual NM value and MAX_NUM_MASTERS are equal. Also, the return value then gets used as an upper limit for for loops with counters beginning at 0. So adding 1 to the return value makes it even more wrong than it was before. Change-Id: I92627332265cf79720b98241d73bc36b1f6a7167 Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M ich_descriptors.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/20937/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index a2f8edf..542e1ec 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -66,8 +66,8 @@ ssize_t ich_number_of_masters(const enum ich_chipset cs, const struct ich_desc_content *const cont) { - if (cont->NM < MAX_NUM_MASTERS) - return cont->NM + 1; + if (cont->NM <= MAX_NUM_MASTERS) + return cont->NM; else return -1; } -- To view, visit https://review.coreboot.org/20937 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newchange Gerrit-Change-Id: I92627332265cf79720b98241d73bc36b1f6a7167 Gerrit-Change-Number: 20937 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>

1 0

Change in flashrom[staging]: ich_descriptors: Modify limits for C620/Lewisburg PCH
by David Hendricks (Code Review) Aug. 10, 2017

Aug. 10, 2017

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/20936 Change subject: ich_descriptors: Modify limits for C620/Lewisburg PCH ...................................................................... ich_descriptors: Modify limits for C620/Lewisburg PCH Change-Id: Ic8adc4b87993e65096166fa6d665432697070b4c Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M ich_descriptors.h 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/20936/1 diff --git a/ich_descriptors.h b/ich_descriptors.h index 8c313d1..574bc5f 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -165,7 +165,7 @@ }; }; -#define MAX_NUM_FLREGS 10 /* 16 on unsupported Lewisburg PCH */ +#define MAX_NUM_FLREGS 16 struct ich_desc_region { /* * Number of entries and width differ on various generations: @@ -174,6 +174,7 @@ * ICH8 .. Panther Point/7 5 13 * Lynx Point/8 .. Wildcat Point/9 7 15 * Sunrise Point/100 .. 10 15 + * Lewisburg/100 .. 16 15 */ union { uint32_t FLREGs[MAX_NUM_FLREGS]; /* Flash Descriptor Regions */ @@ -188,7 +189,7 @@ }; }; -#define MAX_NUM_MASTERS 5 /* 6 on unsupported Lewisburg PCH */ +#define MAX_NUM_MASTERS 6 /* 5 prior to C620/Lewisburg PCH */ struct ich_desc_master { union { uint32_t FLMSTRs[MAX_NUM_MASTERS]; /* Flash Masters */ -- To view, visit https://review.coreboot.org/20936 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newchange Gerrit-Change-Id: Ic8adc4b87993e65096166fa6d665432697070b4c Gerrit-Change-Number: 20936 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>

1 0

Change in flashrom[staging]: chipset_enable: Mark Braswell as tested
by build bot (Jenkins) (Code Review) Aug. 9, 2017

Aug. 9, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20923 ) Change subject: chipset_enable: Mark Braswell as tested ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/465/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/503/ : SUCCESS -- To view, visit https://review.coreboot.org/20923 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Iaf7558af8737af36401f577ca7aba9fd7114a3df Gerrit-Change-Number: 20923 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 09 Aug 2017 17:14:00 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: chispet_enable: Add PCI IDs for C620-series PCHs
by build bot (Jenkins) (Code Review) Aug. 9, 2017

Aug. 9, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20922 ) Change subject: chispet_enable: Add PCI IDs for C620-series PCHs ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/464/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/502/ : SUCCESS -- To view, visit https://review.coreboot.org/20922 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I96c89bc28bdfcd953229c17679f2c28f8b874d0b Gerrit-Change-Number: 20922 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 09 Aug 2017 17:13:48 +0000 Gerrit-HasComments: No

1 0