February 2018 - flashrom-gerrit

Change in flashrom[master]: [v4, 6/6] Add support for reading the current flash contents from a file
by Nico Huber (Code Review) Feb. 2, 2018

Feb. 2, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23263 ) Change subject: [v4,6/6] Add support for reading the current flash contents from a file ...................................................................... Patch Set 1: >> Reading is faster in many cases now, is this still needed / wanted? > > Despite the small memory size (just 128 KB), all the operations > with KB9012 internal flash through EDI interface are much much > slower than for the regular flash chips. Sadly it takes 1 hour for > the complete read/erase/write/read(verify) cycle. I see. The patch generally looks good. Two things, though: 1. `contents file` sounds ambiguous, how about `reference file`? 2. The manpage needs an update. It should also be noted when the data from the reference file is used (currently only at the begin- ning, not later, e.g. when flash content is reread after a failed erase). > This patch tries > to migitate this problem, it is essential for the comfortable > development of KB9012 free-as-in-freedom firmware replacement Does "free-as-in-freedom" mean anything to you, or are you only trying to emphasize that this is important? -- To view, visit https://review.coreboot.org/23263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf153b6955f37779ae9bfb228a434ed10c304947 Gerrit-Change-Number: 23263 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Mike Banon <mikebdp2(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: Fri, 02 Feb 2018 22:29:45 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: [v4, 4/6] ENE EDI - print debug info like others while probing for ENE...
by Nico Huber (Code Review) Feb. 2, 2018

Feb. 2, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23261 ) Change subject: [v4,4/6] ENE EDI - print debug info like others while probing for ENE chips ...................................................................... Patch Set 2: Code-Review+2 (1 comment) https://review.coreboot.org/#/c/23261/2//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23261/2//COMMIT_MSG@13 PS2, Line 13: (no newline here) You can leave a long line in a commit message if it's a quote. (If it doesn't fit here, it probably also doesn't fit in a terminal.) -- To view, visit https://review.coreboot.org/23261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8e62bc9f6785b4bf0be0aaf0f74c8120d77c0d4 Gerrit-Change-Number: 23261 Gerrit-PatchSet: 2 Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Mike Banon <mikebdp2(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: Fri, 02 Feb 2018 22:14:58 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: [v4, 3/6] ENE EDI - add dummy read to ensure proper detection of ENE c...
by Nico Huber (Code Review) Feb. 2, 2018

Feb. 2, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23260 ) Change subject: [v4,3/6] ENE EDI - add dummy read to ensure proper detection of ENE chips ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/23260/1/edi.c File edi.c: https://review.coreboot.org/#/c/23260/1/edi.c@500 PS1, Line 500: edi_detect(flash); > Yes, edi_detect is a function that includes probing I don't see that. I only see a dummy read. -- To view, visit https://review.coreboot.org/23260 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I69ee71674649cd8ba4fc635f889cb39a1cd204b9 Gerrit-Change-Number: 23260 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Mike Banon <mikebdp2(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: Fri, 02 Feb 2018 22:08:10 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: [v4, 1/6] Add support for selecting the erased bit value with a flag
by Nico Huber (Code Review) Feb. 2, 2018

Feb. 2, 2018

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23258 ) Change subject: [v4,1/6] Add support for selecting the erased bit value with a flag ...................................................................... Patch Set 2: (3 comments) >> Code looks good, how well was the rebase tested? > > After I've tested the code and submitted these ported patches, the > only rebase was because of a small typo fix at one of the commit > messages, so the results are still positive - fully tested on the > real hardware I meant the original rebase of Paul's work. What is "the real hard- ware"? It should be primarily tested on previously supported pro- grammers and flash chips to make sure nothing regresses. https://review.coreboot.org/#/c/23258/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23258/1//COMMIT_MSG@2 PS1, Line 2: Author: Mike Banon <mikebdp2(a)gmail.com> > All my commit messages which contain Paul's code - are giving the credit to Paul by including the no […] Well, that's why there are two fields "Author" and "Committer". If you insist, I'll have to find out or make up what is allowed for this project. https://review.coreboot.org/#/c/23258/1//COMMIT_MSG@7 PS1, Line 7: [v4,1/6] > After these tags are removed, would it still be convenient to merge these patches in the correct ord […] The order is clear in Git. These tags are usually used in emails but not in a repository. (The order is wrong anyway, we need the option to exclude EDI from probing first.) https://review.coreboot.org/#/c/23258/1//COMMIT_MSG@17 PS1, Line 17: Signed-off-by: Mike Banon <mikebdp2(a)gmail.com> > These patches have been modified to make them compatible with the latest flashrom, not taken as-is. […] Ok that's fine. You are always free to take full responsibility. -- To view, visit https://review.coreboot.org/23258 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b0de8568e31f9bf263ba0ad6b051e837477b6b Gerrit-Change-Number: 23258 Gerrit-PatchSet: 2 Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Mike Banon <mikebdp2(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: Fri, 02 Feb 2018 22:04:38 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0