- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: Add support for selecting the erased bit value with a flag
by build bot (Jenkins) (Code Review) 03 Feb '18

03 Feb '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23258 ) Change subject: Add support for selecting the erased bit value with a flag ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1084/ : SUCCESS -- 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: 3 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 Kocialkowski <contact(a)paulk.fr> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 03 Feb 2018 11:55:12 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Add support for selecting the erased bit value with a flag
by Mike Banon (Code Review) 03 Feb '18

03 Feb '18

Hello Paul Kocialkowski, Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/23258 to look at the new patch set (#3). Change subject: Add support for selecting the erased bit value with a flag ...................................................................... Add support for selecting the erased bit value with a flag Most flash chips are erased to ones and programmed to zeros. However, some other chips, such as the ENE KB9012 internal flash, work the opposite way. Original patch has been created by Paul Kocialkowski, the previous version: http://patchwork.coreboot.org/patch/4412/ Now it has been ported to the latest source code of flashrom master branch Change-Id: Ia7b0de8568e31f9bf263ba0ad6b051e837477b6b Signed-off-by: Mike Banon <mikebdp2(a)gmail.com> --- M flash.h M flashrom.c 2 files changed, 26 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/58/23258/3 -- 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: newpatchset Gerrit-Change-Id: Ia7b0de8568e31f9bf263ba0ad6b051e837477b6b Gerrit-Change-Number: 23258 Gerrit-PatchSet: 3 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 Kocialkowski <contact(a)paulk.fr> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: [v4, 5/6] Add the hidden-from-probing chips category and make KB9012 h...
by Nico Huber (Code Review) 03 Feb '18

03 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23262 ) Change subject: [v4,5/6] Add the hidden-from-probing chips category and make KB9012 hidden ...................................................................... Patch Set 1: IMHO, taking shortcuts is one of the problems that made flashrom less maintainable and makes merging of new patches much harder. Usually, the time you spend to do something right from the beginning pays off later. If you want, I can take this one commit over and you'd only have to rebase the others on it. -- To view, visit https://review.coreboot.org/23262 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: I89a53ccaef2791a2ac32904d7ab813da7478a6f0 Gerrit-Change-Number: 23262 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 Kocialkowski <contact(a)paulk.fr> 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:33:16 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

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

03 Feb '18

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) 03 Feb '18

03 Feb '18

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) 03 Feb '18

03 Feb '18

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) 03 Feb '18

03 Feb '18

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

Change in flashrom[master]: [v4, 5/6] Add the hidden-from-probing chips category and make KB9012 h...
by Mike Banon (Code Review) 31 Jan '18

31 Jan '18

Mike Banon has posted comments on this change. ( https://review.coreboot.org/23262 ) Change subject: [v4,5/6] Add the hidden-from-probing chips category and make KB9012 hidden ...................................................................... Patch Set 1: > (1 comment) > > This should be done before adding the EDI driver. > > AFAIR, Paul had something more flexible in mind: Instead of the > `hidden` flag a grouping of the chips. Basically the same what you > did but instead of 0/1 values a default of JEDEC and alternatively > EDI and whatever might come in the future. How about that? I agree that grouping is a better idea. To be honest I'm not familiar with flashrom codebase, and at this moment it was a much simpler solution to just add this ".hidden" flag, partially because I didn't understand what would be the proper good way of introducing the groups... Are there any other problematic chips supported by flashrom like KB9012, probing for which could cause problems for other chips? If not, maybe it's still possible to use that ".hidden" flag as the quick initial solution (just to avoid KB9012 probing when its not needed) and upgrade it to the groups later, when other "hidden chips" arrive? -- To view, visit https://review.coreboot.org/23262 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: I89a53ccaef2791a2ac32904d7ab813da7478a6f0 Gerrit-Change-Number: 23262 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 Kocialkowski <contact(a)paulk.fr> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 30 Jan 2018 23:35:19 +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 build bot (Jenkins) (Code Review) 31 Jan '18

31 Jan '18

build bot (Jenkins) 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: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/923/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/1083/ : SUCCESS -- 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: Tue, 30 Jan 2018 23:19:27 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: [v4, 4/6] ENE EDI - print debug info like others while probing for ENE...
by Mike Banon (Code Review) 31 Jan '18

31 Jan '18

Mike Banon 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: (4 comments) https://review.coreboot.org/#/c/23261/1/edi.c File edi.c: https://review.coreboot.org/#/c/23261/1/edi.c@154 PS1, Line 154: %i < > It's a signed integer, why print it in hex? sorry, my mistake https://review.coreboot.org/#/c/23261/1/edi.c@164 PS1, Line 164: if (chip->hwversion == hwversion && chip->ediid == ediid) { > No need to print them, they are 0 on this path. indeed. fixed https://review.coreboot.org/#/c/23261/1/edi.c@169 PS1, Line 169: __func__, hwversion, chip->hwversion, ediid, chip->ediid); > Please add braces here too (they are not required by syntax but help […] braces added https://review.coreboot.org/#/c/23261/1/edi.c@171 PS1, Line 171: > Actually, I'm not sure if we want this message at all. This function […] changed the debug level to msg_cdbg2, hopefully these debug messages will be useful in some cases -- 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: Tue, 30 Jan 2018 23:18:45 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit