October 2017 - flashrom-gerrit

Change in flashrom[stable]: Convert flashrom to git
by Stefan Tauner (Code Review) 12 Oct '17

12 Oct '17

Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/21923 to look at the new patch set (#2). Change subject: Convert flashrom to git ...................................................................... Convert flashrom to git - Drop support for Subversion in the getrevision script and Makefile. - Refinement of getrevision to have a single function determining the new flashrom version string (based on git describe). - Add .gitignore and .gitattributes file (the latter to limit exports). - Restore modification dates of the exported files from the VCS. - Stop exporting VCS log dumps to CHANGELOG. This makes no sense. - Export VCS metadata needed by the Makefile to versioninfo.inc - Do not export the pre-"compiled" manpage. It can be generated like anything else from the code dump with versioninfo.inc - Add some initial client-side git hooks that get installed automatically on make runs: * pre-commit: - test for whitespace errors via git diff-index --check * commit-msg: - Use the up to date hook from gerrit to add Change-Id tags. Unlike the upstream hook, add them to fixup/squash changes too to allow for our staging process. - Test for duplicate sign-offs/acks. * When pushing to one of the upstream repositories check mandatory rules: - existing signoffs and acks in all new commits - no deletions of branches - no new branches other than versioned branches (e.g., 0.0.x) - no history rewriting of branches - Various other small refinements, bikesheddings and cosmetic changes to slow down progress of flashrom and humanity as a whole. Original-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Original-Reviewed-on: https://review.coreboot.org/19206 Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Original-Reviewed-by: Philippe Mathieu-Daudé <f4bug(a)amsat.org> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I325edb7d9890c412a44ecacb5161268d3dbccc57 Signed-off-by: Stefan Tauner <stefan.tauner(a)gmx.at> Signed-off-by: David Hendricks <dhendricks(a)fb.com> Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- A .gitattributes A .gitignore M Makefile M flashrom.c M util/getrevision.sh A util/git-hooks/applypatch-msg A util/git-hooks/commit-msg A util/git-hooks/install.sh A util/git-hooks/pre-applypatch A util/git-hooks/pre-commit A util/git-hooks/pre-push A util/git-hooks/wrapper.sh 12 files changed, 440 insertions(+), 132 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/21923/2 -- To view, visit https://review.coreboot.org/21923 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newpatchset Gerrit-Change-Id: I325edb7d9890c412a44ecacb5161268d3dbccc57 Gerrit-Change-Number: 21923 Gerrit-PatchSet: 2 Gerrit-Owner: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: 4BA: Flashrom integration for the 4-bytes addressing extensions
by Nico Huber (Code Review) 11 Oct '17

11 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20505 ) Change subject: 4BA: Flashrom integration for the 4-bytes addressing extensions ...................................................................... Patch Set 3: > Will write a follow-up, not sure if any line will stay. Nope, I won't. After reading through all the patches I'm failing to see the light at the end of the tunnel. IMO, this adds 4BA as an alien instead of integrating it where plausible. Doing the whole thing with proper abstractions from the beginning (the function pointer issue + the 10 times copied implementations with felt only single bytes changing) seems to be less work for me now than bringing these patches to a state I could imagine they would have after a decent reviewing process. Any thoughts where to go from here? I can start over reviewing the whole set if somebody wants to adapt these commits. Alternatively we can submit this to staging (I don't see anything broken, it's just leaving flashrom in a less maintainable state*). And I can write my own implementation for stable... * We already have way too much redundant code and layering violations, IMHO. If we increase that maintenance burden we'll have to pay double later during development and reviewing. -- To view, visit https://review.coreboot.org/20505 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib051cfc93bd4aa7580519e0e6206d025f3ca8049 Gerrit-Change-Number: 20505 Gerrit-PatchSet: 3 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, 11 Oct 2017 15:23:38 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: 4BA: Flashrom integration for the 4-bytes addressing extensions
by Nico Huber (Code Review) 11 Oct '17

11 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20505 ) Change subject: 4BA: Flashrom integration for the 4-bytes addressing extensions ...................................................................... Patch Set 2: The major issue that I see is that the added function pointers are all for naught. They are redundant information that can already be sta- tically decided by the feature bits (if we keep the _DIRECT features in the end). -- To view, visit https://review.coreboot.org/20505 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib051cfc93bd4aa7580519e0e6206d025f3ca8049 Gerrit-Change-Number: 20505 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, 11 Oct 2017 14:25:49 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: 4BA: Flashrom integration for the 4-bytes addressing extensions
by Nico Huber (Code Review) 11 Oct '17

11 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20505 ) Change subject: 4BA: Flashrom integration for the 4-bytes addressing extensions ...................................................................... Patch Set 3: (6 comments) Rather clumsy. I don't like to do invasive edits on other people's commits. Will write a follow-up, not sure if any line will stay. https://review.coreboot.org/#/c/20505/3/flash.h File flash.h: https://review.coreboot.org/#/c/20505/3/flash.h@176 PS3, Line 176: } four_bytes_addr_funcs; This makes 4BA look like an alien... https://review.coreboot.org/#/c/20505/3/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/20505/3/flashrom.c@2227 PS3, Line 2227: if(flash->chip->feature_bits & FEATURE_4BA_SUPPORT) { missing space after `if` https://review.coreboot.org/#/c/20505/3/flashrom.c@2240 PS3, Line 2240: if(flash->chip->four_bytes_addr_funcs.enter_4ba(flash)) { missing space https://review.coreboot.org/#/c/20505/3/flashrom.c@2253 PS3, Line 2253: { Hmmm, what if another master expects the chip to be in 3BA mode? https://review.coreboot.org/#/c/20505/3/spi.c File spi.c: https://review.coreboot.org/#/c/20505/3/spi.c@115 PS3, Line 115: if (!(flash->chip->feature_bits & FEATURE_4BA_SUPPORT) && What if 4BA is supported? Shouldn't we do the same check with `1 << 32`? (I can't tell. The whole `addrbase` thing looks like a layering violation to me that should be handled by programmers that actually do map the chip.) https://review.coreboot.org/#/c/20505/3/spi25.c File spi25.c: https://review.coreboot.org/#/c/20505/3/spi25.c@1019 PS3, Line 1019: rc = (flash->chip->feature_bits & FEATURE_4BA_SUPPORT) == 0 The condition should be checked in the called function, IMHO. This is too ugly... -- To view, visit https://review.coreboot.org/20505 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ib051cfc93bd4aa7580519e0e6206d025f3ca8049 Gerrit-Change-Number: 20505 Gerrit-PatchSet: 3 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, 11 Oct 2017 14:01:04 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: fixup! 4BA: Basic support for 4-bytes addressing mode extensions
by Nico Huber (Code Review) 11 Oct '17

11 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/21947 ) Change subject: fixup! 4BA: Basic support for 4-bytes addressing mode extensions ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/21947/1/spi4ba.c File spi4ba.c: https://review.coreboot.org/#/c/21947/1/spi4ba.c@185 PS1, Line 185: Erases `Erase`? the other comments are in imperative, too. -- To view, visit https://review.coreboot.org/21947 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I4faaa036fea744135fa37f405686fb9fd0882806 Gerrit-Change-Number: 21947 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 11 Oct 2017 13:30:02 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: 4BA: Basic support for 4-bytes addressing mode extensions
by Nico Huber (Code Review) 11 Oct '17

11 Oct '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20513 ) Change subject: 4BA: Basic support for 4-bytes addressing mode extensions ...................................................................... Patch Set 2: Code-Review+2 pushed a minor fixup -- To view, visit https://review.coreboot.org/20513 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ie72e2a89cd75fb4d09f48e81c4c1d927c317b7a7 Gerrit-Change-Number: 20513 Gerrit-PatchSet: 2 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: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 11 Oct 2017 10:14:26 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: fixup! 4BA: Basic support for 4-bytes addressing mode extensions
by build bot (Jenkins) (Code Review) 11 Oct '17

11 Oct '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21947 ) Change subject: fixup! 4BA: Basic support for 4-bytes addressing mode extensions ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/756/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/684/ : SUCCESS -- To view, visit https://review.coreboot.org/21947 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I4faaa036fea744135fa37f405686fb9fd0882806 Gerrit-Change-Number: 21947 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 11 Oct 2017 10:13:55 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: fixup! 4BA: Basic support for 4-bytes addressing mode extensions
by Nico Huber (Code Review) 11 Oct '17

11 Oct '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21947 Change subject: fixup! 4BA: Basic support for 4-bytes addressing mode extensions ...................................................................... fixup! 4BA: Basic support for 4-bytes addressing mode extensions Fix some whitespace, and braces. Remove sector size from comments that I could not verify. Change-Id: I4faaa036fea744135fa37f405686fb9fd0882806 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flash.h M spi4ba.c 2 files changed, 15 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/21947/1 diff --git a/flash.h b/flash.h index 611450f..56c0b4f 100644 --- a/flash.h +++ b/flash.h @@ -120,8 +120,8 @@ #define FEATURE_OTP (1 << 8) #define FEATURE_QPI (1 << 9) /* Feature bits used for 4-bytes addressing mode */ -#define FEATURE_4BA_SUPPORT (1 << 10) -#define FEATURE_4BA_ONLY (1 << 11) +#define FEATURE_4BA_SUPPORT (1 << 10) +#define FEATURE_4BA_ONLY (1 << 11) enum test_state { OK = 0, diff --git a/spi4ba.c b/spi4ba.c index 72df874..ea9bc76 100644 --- a/spi4ba.c +++ b/spi4ba.c @@ -43,7 +43,7 @@ msg_trace("-> %s\n", __func__); - /* Switch to 4-bytes addressing mode */ + /* Switch to 4-bytes addressing mode */ return spi_send_command(flash, sizeof(cmd), 0, cmd, NULL); } @@ -71,17 +71,15 @@ msg_trace("-> %s\n", __func__); - /* Switch to 4-bytes addressing mode */ + /* Switch to 4-bytes addressing mode */ result = spi_send_multicommand(flash, cmds); - if (result) { + if (result) msg_cerr("%s failed during command execution\n", __func__); - } return result; } /* Program one flash byte from 4-bytes addressing mode */ -int spi_byte_program_4ba(struct flashctx *flash, unsigned int addr, - uint8_t databyte) +int spi_byte_program_4ba(struct flashctx *flash, unsigned int addr, uint8_t databyte) { int result; struct spi_command cmds[] = { @@ -112,16 +110,13 @@ msg_trace("-> %s (0x%08X)\n", __func__, addr); result = spi_send_multicommand(flash, cmds); - if (result) { - msg_cerr("%s failed during command execution at address 0x%x\n", - __func__, addr); - } + if (result) + msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr); return result; } /* Program flash bytes from 4-bytes addressing mode */ -int spi_nbyte_program_4ba(struct flashctx *flash, unsigned int addr, - const uint8_t *bytes, unsigned int len) +int spi_nbyte_program_4ba(struct flashctx *flash, unsigned int addr, const uint8_t *bytes, unsigned int len) { int result; unsigned char cmd[(JEDEC_BYTE_PROGRAM_OUTSIZE + 1) - 1 + 256] = { @@ -171,8 +166,7 @@ } /* Read flash bytes from 4-bytes addressing mode */ -int spi_nbyte_read_4ba(struct flashctx *flash, unsigned int addr, - uint8_t *bytes, unsigned int len) +int spi_nbyte_read_4ba(struct flashctx *flash, unsigned int addr, uint8_t *bytes, unsigned int len) { const unsigned char cmd[JEDEC_READ_OUTSIZE + 1] = { JEDEC_READ, @@ -188,9 +182,8 @@ return spi_send_command(flash, sizeof(cmd), len, cmd, bytes); } -/* Erases 4 KB of flash from 4-bytes addressing mode */ -int spi_block_erase_20_4ba(struct flashctx *flash, unsigned int addr, - unsigned int blocklen) +/* Erases one sector of flash from 4-bytes addressing mode */ +int spi_block_erase_20_4ba(struct flashctx *flash, unsigned int addr, unsigned int blocklen) { int result; struct spi_command cmds[] = { @@ -234,9 +227,8 @@ return 0; } -/* Erases 32 KB of flash from 4-bytes addressing mode */ -int spi_block_erase_52_4ba(struct flashctx *flash, unsigned int addr, - unsigned int blocklen) +/* Erases one sector of flash from 4-bytes addressing mode */ +int spi_block_erase_52_4ba(struct flashctx *flash, unsigned int addr, unsigned int blocklen) { int result; struct spi_command cmds[] = { @@ -280,7 +272,7 @@ return 0; } -/* Erases 64 KB of flash from 4-bytes addressing mode */ +/* Erases one sector of flash from 4-bytes addressing mode */ int spi_block_erase_d8_4ba(struct flashctx *flash, unsigned int addr, unsigned int blocklen) { -- To view, visit https://review.coreboot.org/21947 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newchange Gerrit-Change-Id: I4faaa036fea744135fa37f405686fb9fd0882806 Gerrit-Change-Number: 21947 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: Add modification date to manpage header instead of the section
by build bot (Jenkins) (Code Review) 10 Oct '17

10 Oct '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21941 ) Change subject: Add modification date to manpage header instead of the section ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/755/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/683/ : SUCCESS -- To view, visit https://review.coreboot.org/21941 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: I45c5d6a2316c51a57a49fd010682dc3f0f915382 Gerrit-Change-Number: 21941 Gerrit-PatchSet: 1 Gerrit-Owner: 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, 10 Oct 2017 16:59:09 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: nicintel_spi: Support for I210/I211 cards
by build bot (Jenkins) (Code Review) 10 Oct '17

10 Oct '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21939 ) Change subject: nicintel_spi: Support for I210/I211 cards ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/754/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/682/ : SUCCESS -- To view, visit https://review.coreboot.org/21939 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: Ie1e5127cd3efdfef3489ef85a5f48a58deb37f8a Gerrit-Change-Number: 21939 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 10 Oct 2017 16:58:49 +0000 Gerrit-HasComments: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2017