- flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: fixup! nicintel_eeprom: Support for I210 emulated EEprom
by build bot (Jenkins) (Code Review) 27 Sep '17

27 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21702 ) Change subject: fixup! nicintel_eeprom: Support for I210 emulated EEprom ...................................................................... Patch Set 2: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/570/ : FAILURE -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 Gerrit-PatchSet: 2 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: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 27 Sep 2017 16:28:06 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: fixup! nicintel_eeprom: Support for I210 emulated EEprom
by David Hendricks (Code Review) 27 Sep '17

27 Sep '17

Hello Stefan Tauner, Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/21702 to look at the new patch set (#2). Change subject: fixup! nicintel_eeprom: Support for I210 emulated EEprom ...................................................................... fixup! nicintel_eeprom: Support for I210 emulated EEprom A couple of C99-style variable declarations within loops are causing compilation failures on some systems (gcc 4.9.2-10 on Raspbian). This moves them to make gcc happy. Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Signed-off-by: David Hendricks <david.hendricks(a)gmail.com> --- M nicintel_eeprom.c 1 file changed, 4 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/21702/2 -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 Gerrit-PatchSet: 2 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: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: nicintel_eeprom: Move variable declarations out of loops
by Stefan Tauner (Code Review) 27 Sep '17

27 Sep '17

Stefan Tauner has posted comments on this change. ( https://review.coreboot.org/21702 ) Change subject: nicintel_eeprom: Move variable declarations out of loops ...................................................................... Patch Set 1: -Code-Review > I actually looked in the codebase for other variable declarations > in for heads and didn't see any. Perhaps there are some that you > know of? In any case, if we don't want to support non-c99 then we > should make that explicit in the Makefile. > > I suspect that this bug/error was only triggered by a recent patch, > which worked fine with Jenkins and a couple other versions of GCC. > This patch is just a quick fix, which will work no matter if we > force C99 compatibility or not. Yes, you are right. My memory betrayed me and I did not check, sorry. The C90 declarations annoyed me from the beginning and I thought they were already removed but that's wrong (probably because - as I explained on IRC - forcing C99 on compilers is more problematic than one would think). So, yes, we should fix the code, but as a fixup! of the respective patch which is 9fe1fb7 (aka nicintel_eeprom: Support for I210 emulated EEprom). This also shows why a buildbot is a precious thing ;) -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 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: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 27 Sep 2017 07:50:13 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: nicintel_eeprom: Move variable declarations out of loops
by David Hendricks (Code Review) 27 Sep '17

27 Sep '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/21702 ) Change subject: nicintel_eeprom: Move variable declarations out of loops ...................................................................... Patch Set 1: I actually looked in the codebase for other variable declarations in for heads and didn't see any. Perhaps there are some that you know of? In any case, if we don't want to support non-c99 then we should make that explicit in the Makefile. I suspect that this bug/error was only triggered by a recent patch, which worked fine with Jenkins and a couple other versions of GCC. This patch is just a quick fix, which will work no matter if we force C99 compatibility or not. Here's build_details.txt: https://paste.flashrom.org/view.php?id=3052 -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 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: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 27 Sep 2017 05:59:08 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: 4BA: Winbond W25Q256.V chip (32MB) declaration, 4-bytes addr mode
by Lijian Zhao (Code Review) 27 Sep '17

27 Sep '17

Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/20506 ) Change subject: 4BA: Winbond W25Q256.V chip (32MB) declaration, 4-bytes addr mode ...................................................................... Patch Set 3: Code-Review+1 Tested okay with both internal flash and ftdi_spi google-servo-v2. Flash with flashrom and then read back from dediprog, the content is identical. -- To view, visit https://review.coreboot.org/20506 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I90226f453f8147ae5ac7dbbef7549ee3bfacc3d6 Gerrit-Change-Number: 20506 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <f4bug(a)amsat.org> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 27 Sep 2017 01:27:32 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: nicintel_eeprom: Move variable declarations out of loops
by Stefan Tauner (Code Review) 26 Sep '17

26 Sep '17

Stefan Tauner has posted comments on this change. ( https://review.coreboot.org/21702 ) Change subject: nicintel_eeprom: Move variable declarations out of loops ...................................................................... Patch Set 1: > > We do not (want to) support non-C99 compilers and I seriously > doubt > > that this is really the issue and thus the best fix. Can you > please > > show us the compiler's diagnostics and build_details.txt? > > Well, we kept the code compatible until now and as long as we don't > specify the standard, should keep it that way. No, we didn't at all. Why do you think so? Not *forcing* a version in the makefile does not mean we support *every* compiler out there. We use lots of C99 features all around the code base... including variable declarations in for heads. The report (and patch) are lacking details... -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Sep 2017 18:01:12 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: nicintel_eeprom: Move variable declarations out of loops
by Nico Huber (Code Review) 26 Sep '17

26 Sep '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/21702 ) Change subject: nicintel_eeprom: Move variable declarations out of loops ...................................................................... Patch Set 1: Code-Review+2 > We do not (want to) support non-C99 compilers and I seriously doubt > that this is really the issue and thus the best fix. Can you please > show us the compiler's diagnostics and build_details.txt? Well, we kept the code compatible until now and as long as we don't specify the standard, should keep it that way. -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Sep 2017 17:48:44 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: nicintel_eeprom: Move variable declarations out of loops
by Stefan Tauner (Code Review) 26 Sep '17

26 Sep '17

Stefan Tauner has posted comments on this change. ( https://review.coreboot.org/21702 ) Change subject: nicintel_eeprom: Move variable declarations out of loops ...................................................................... Patch Set 1: Code-Review-1 We do not (want to) support non-C99 compilers and I seriously doubt that this is really the issue and thus the best fix. Can you please show us the compiler's diagnostics and build_details.txt? -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Sep 2017 17:36:45 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: nicintel_eeprom: Move variable declarations out of loops
by build bot (Jenkins) (Code Review) 26 Sep '17

26 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21702 ) Change subject: nicintel_eeprom: Move variable declarations out of loops ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/569/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/515/ : SUCCESS -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Sep 2017 16:57:27 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: nicintel_eeprom: Move variable declarations out of loops
by David Hendricks (Code Review) 26 Sep '17

26 Sep '17

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/21702 Change subject: nicintel_eeprom: Move variable declarations out of loops ...................................................................... nicintel_eeprom: Move variable declarations out of loops A couple of C99-style variable declarations within loops are causing compilation failures on some systems (gcc 4.9.2-10 on Raspbian). This moves them to make gcc happy. Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Signed-off-by: David Hendricks <david.hendricks(a)gmail.com> --- M nicintel_eeprom.c 1 file changed, 4 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/21702/1 diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index e2fb584..e4a91ef 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -211,7 +211,8 @@ pci_mmio_writel(eewr, nicintel_eebar + EEWR); programmer_delay(5); - for (int i = 0; i < MAX_ATTEMPTS; i++) + int i; + for (i = 0; i < MAX_ATTEMPTS; i++) if (pci_mmio_readl(nicintel_eebar + EEWR) & BIT(EEWR_DONE)) return 0; return -1; @@ -419,7 +420,8 @@ flup |= BIT(EE_FLUPD); pci_mmio_writel(flup, nicintel_eebar + EEC); - for (int i = 0; i < MAX_ATTEMPTS; i++) + int i; + for (i = 0; i < MAX_ATTEMPTS; i++) if (pci_mmio_readl(nicintel_eebar + EEC) & BIT(EE_FLUDONE)) return 0; -- To view, visit https://review.coreboot.org/21702 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newchange Gerrit-Change-Id: Ib7ad5a69244e462f84eae93df9e841716e089b31 Gerrit-Change-Number: 21702 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit