February 2018 - flashrom-gerrit

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Nico Huber (Code Review) 09 Feb '18

09 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 13: (8 comments) https://review.coreboot.org/#/c/23021/13/flash.h File flash.h: https://review.coreboot.org/#/c/23021/13/flash.h@303 PS13, Line 303: const Nit, `const` qualification has no meaning in the forward declaration. https://review.coreboot.org/#/c/23021/13/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23021/13/flashrom.c@1383 PS13, Line 1383: return 1; This was probably added because of my misinterpretation that we don't allow mixing a full-image file with per-region files. Should be reverted to the simple `continue;` it was. All such constraints should be handled in a higher level. https://review.coreboot.org/#/c/23021/13/flashrom.c@1470 PS13, Line 1470: write_buf_to_file Nit, rather write_buf_to_file*s*? https://review.coreboot.org/#/c/23021/13/flashrom.c@1500 PS13, Line 1500: if (ret) : goto out; mhmm? https://review.coreboot.org/#/c/23021/13/flashrom.c@1506 PS13, Line 1506: #endif oops https://review.coreboot.org/#/c/23021/13/flashrom.c@2651 PS13, Line 2651: if (get_num_include_args_with_files(get_layout(flash)) > 0) { Why check this explicitly? I'd expect read_buf_from_include_args() to do nothing by definition if the count is 0. https://review.coreboot.org/#/c/23021/13/flashrom.c@2676 PS13, Line 2676: *files missing space https://review.coreboot.org/#/c/23021/13/layout.c File layout.c: https://review.coreboot.org/#/c/23021/13/layout.c@233 PS13, Line 233: for (j = 0; j < l->num_entries; j++) { As mentioned in PS7, `j` should start at `i + 1` otherwise you will check each pair (x, y) twice, once with i=x, j=y and once with j=x and i=y. -- To view, visit https://review.coreboot.org/23021 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: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 13 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 09 Feb 2018 13:21:29 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: layout: Add -i <region>[:<file>] support.
by Nico Huber (Code Review) 08 Feb '18

08 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23021 ) Change subject: layout: Add -i <region>[:<file>] support. ...................................................................... Patch Set 13: In the light of the shy ML discussion, what do you think about my alternative syntax proposal [1]? Especially the last one would be most user-friendly, IMHO, because you just can't do confusing stuff. I assume, it's unlikely that regular users would want to mix a full flash sized file with per-region files. [1] https://flashrom.org/Per_region_file_arguments -- To view, visit https://review.coreboot.org/23021 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: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 13 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 08 Feb 2018 22:06:48 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC
by Nico Huber (Code Review) 08 Feb '18

08 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23259 ) Change subject: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC ...................................................................... Patch Set 8: >> This should be rebased on the SPI command set commit, and add and >> use >> another class. Looks very good otherwise. > > Big problem is that this change "23259" depends on already verified > "23258" and its' "FEATURE_ERASED_ZERO" . When I try rebasing it on > top of "23262" (SPI command set) I get a -1 build error from > Jenkins. I'm not sure what to do next... You need all three commits (23258, 23259, 23262) on a single branch. IMO, easiest way is to check this one out (which will include 23258 as its parent) and rebase on 23262. git fetch ssh://Nico@review.coreboot.org:29418/flashrom refs/changes/59/23259/8 && git checkout FETCH_HEAD git fetch ssh://Nico@review.coreboot.org:29418/flashrom refs/changes/62/23262/5 && git rebase FETCH_HEAD git push ... > Also its' a bit unclear to > me, what did you mean by "add and use another class" for this > commit. Please could you clarify? I meant you should extend the enum added by 23262 (e.g. add SPI_EDI) and set `.spi_cmd_set = SPI_EDI,` in the flashchips.c entry. -- To view, visit https://review.coreboot.org/23259 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: Ib8b2eb2feeef5c337d725d15ebf994a299897854 Gerrit-Change-Number: 23259 Gerrit-PatchSet: 8 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: Thu, 08 Feb 2018 16:14:42 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC
by Mike Banon (Code Review) 08 Feb '18

08 Feb '18

Mike Banon has posted comments on this change. ( https://review.coreboot.org/23259 ) Change subject: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC ...................................................................... Patch Set 8: > This should be rebased on the SPI command set commit, and add and > use > another class. Looks very good otherwise. Big problem is that this change "23259" depends on already verified "23258" and its' "FEATURE_ERASED_ZERO" . When I try rebasing it on top of "23262" (SPI command set) I get a -1 build error from Jenkins. I'm not sure what to do next... Also its' a bit unclear to me, what did you mean by "add and use another class" for this commit. Please could you clarify? -- To view, visit https://review.coreboot.org/23259 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: Ib8b2eb2feeef5c337d725d15ebf994a299897854 Gerrit-Change-Number: 23259 Gerrit-PatchSet: 8 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: Thu, 08 Feb 2018 12:49:32 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC
by build bot (Jenkins) (Code Review) 08 Feb '18

08 Feb '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23259 ) Change subject: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC ...................................................................... Patch Set 8: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1116/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/940/ : SUCCESS -- To view, visit https://review.coreboot.org/23259 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: Ib8b2eb2feeef5c337d725d15ebf994a299897854 Gerrit-Change-Number: 23259 Gerrit-PatchSet: 8 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: Thu, 08 Feb 2018 12:42:16 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC
by Mike Banon (Code Review) 08 Feb '18

08 Feb '18

Mike Banon has posted comments on this change. ( https://review.coreboot.org/23259 ) Change subject: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC ...................................................................... Patch Set 7: > Build Failed > > https://qa.coreboot.org/job/flashrom_gerrit/939/ : FAILURE > > https://qa.coreboot.org/job/flashrom-customrules/1115/ : SUCCESS got "flashchips.c:3290:19: error: 'FEATURE_ERASED_ZERO' undeclared here (not in a function); did you mean 'FEATURE_RESET_MASK'? error", will try to fix -- To view, visit https://review.coreboot.org/23259 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: Ib8b2eb2feeef5c337d725d15ebf994a299897854 Gerrit-Change-Number: 23259 Gerrit-PatchSet: 7 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: Thu, 08 Feb 2018 10:14:18 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC
by build bot (Jenkins) (Code Review) 08 Feb '18

08 Feb '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23259 ) Change subject: Add support for the ENE Embedded Debug Interface EDI and KB9012 EC ...................................................................... Patch Set 7: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom_gerrit/939/ : FAILURE https://qa.coreboot.org/job/flashrom-customrules/1115/ : SUCCESS -- To view, visit https://review.coreboot.org/23259 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: Ib8b2eb2feeef5c337d725d15ebf994a299897854 Gerrit-Change-Number: 23259 Gerrit-PatchSet: 7 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: Thu, 08 Feb 2018 10:11:06 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: edi: Add dummy read to ensure proper detection of ENE chips
by Paul Kocialkowski (Code Review) 08 Feb '18

08 Feb '18

Paul Kocialkowski has posted comments on this change. ( https://review.coreboot.org/23260 ) Change subject: edi: Add dummy read to ensure proper detection of ENE chips ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/#/c/23260/3/edi.c File edi.c: https://review.coreboot.org/#/c/23260/3/edi.c@166 PS3, Line 166: static void edi_wakeup(struct flashctx *flash) > > Theoretically […] Agreed. -- 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: 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: Thu, 08 Feb 2018 09:23:42 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

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

07 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23263 ) Change subject: Add support for reading the current flash contents from a file ...................................................................... Patch Set 11: Code-Review+2 -- 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: 11 Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(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: Wed, 07 Feb 2018 22:18:19 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: edi: Add dummy read to ensure proper detection of ENE chips
by Nico Huber (Code Review) 07 Feb '18

07 Feb '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23260 ) Change subject: edi: Add dummy read to ensure proper detection of ENE chips ...................................................................... Patch Set 6: Code-Review+2 (1 comment) Well, it's really a lot of boilerplate and commentary for something that actually could be a one-liner with a one-line comment. Your choice. https://review.coreboot.org/#/c/23260/3/edi.c File edi.c: https://review.coreboot.org/#/c/23260/3/edi.c@166 PS3, Line 166: static void edi_draw_attention(struct flashctx *flash) > Theoretically we could put every function call into its own function and comment it twice. It's really a little too much here, IMHO. -- 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: 6 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: Wed, 07 Feb 2018 22:13:45 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2018