February 2018 - flashrom-gerrit

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

05 Feb '18

Paul Kocialkowski 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 6: (4 comments) Oh well, I did have a few more suggestions in the end. https://review.coreboot.org/#/c/23263/6/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23263/6/cli_classic.c@130 PS6, Line 130: {"ifd", 0, NULL, 0x0100}, wouldn't it make more sense to use the values from the enum here? https://review.coreboot.org/#/c/23263/6/cli_classic.c@132 PS6, Line 132: {"flash-contents", 1, NULL, 0x0101}, ditto https://review.coreboot.org/#/c/23263/6/cli_classic.c@253 PS6, Line 253: case 0x0101: ditto, and there's probably a 0x0100 that's around but not shown in the context that should also be changed. https://review.coreboot.org/#/c/23263/6/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23263/6/flashrom.c@2395 PS6, Line 2395: if (refcontents && oldcontents) { Extra padding should be removed. -- 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: 6 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: Mon, 05 Feb 2018 13:16:38 +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 Paul Kocialkowski (Code Review) 05 Feb '18

05 Feb '18

Paul Kocialkowski 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 6: (1 comment) Thanks for the great work! One small change left as far as I can see. https://review.coreboot.org/#/c/23263/6//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23263/6//COMMIT_MSG@21 PS6, Line 21: Now it has been ported to the latest source code of flashrom master branch Just like with other commits, I think we can drop this block now. -- 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: 6 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: Mon, 05 Feb 2018 13:12:16 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: ENE EDI - print debug info like others while probing for ENE chips
by Paul Kocialkowski (Code Review) 05 Feb '18

05 Feb '18

Paul Kocialkowski has posted comments on this change. ( https://review.coreboot.org/23261 ) Change subject: ENE EDI - print debug info like others while probing for ENE chips ...................................................................... Patch Set 5: (3 comments) Thanks for the work! We're getting there :) https://review.coreboot.org/#/c/23261/5//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23261/5//COMMIT_MSG@7 PS5, Line 7: ENE EDI - print debug info like others while probing for ENE chips Like with the other commit, is that the regular way to prefix commits in flashrom? I suggest "edi: Print debug info like others while probing for ENE chips" instead. https://review.coreboot.org/#/c/23261/5/edi.c File edi.c: https://review.coreboot.org/#/c/23261/5/edi.c@154 PS5, Line 154: msg_cdbg("%s: reading HWVERSION failed\n", __func__); Please use lowercase so that it's consistent with the message printed on success. https://review.coreboot.org/#/c/23261/5/edi.c@160 PS5, Line 160: msg_cdbg("%s: reading EDIID failed\n", __func__); ditto -- 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: 5 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: Mon, 05 Feb 2018 13:09:39 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: ENE EDI - add dummy read to ensure proper detection of ENE chips
by Paul Kocialkowski (Code Review) 05 Feb '18

05 Feb '18

Paul Kocialkowski has posted comments on this change. ( https://review.coreboot.org/23260 ) Change subject: ENE EDI - add dummy read to ensure proper detection of ENE chips ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/#/c/23260/3//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23260/3//COMMIT_MSG@7 PS3, Line 7: ENE EDI - add dummy read to ensure proper detection of ENE chips Is that the regular way to prefix commits in flashrom? I think making the title: "edi: Add dummy read to ensure proper detection of ENE chips" would be more consistent with the rest. -- 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: Mon, 05 Feb 2018 13:06:50 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: ENE EDI - add dummy read to ensure proper detection of ENE chips
by Paul Kocialkowski (Code Review) 05 Feb '18

05 Feb '18

Paul Kocialkowski has posted comments on this change. ( https://review.coreboot.org/23260 ) Change subject: ENE 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) Nico, I don't really agree here (sorry for the delay in replying): this is really not about waking up the chip but about the chip detecting that there is activity and enabling EDI. This is why "detect" felt like a good fit: the chip detects activity, but I agree that it's not very good since it can also be understood as "flashrom detecting the chip" which is not what's happening. What about moving this read to edi_probe_kb9012 directly (under the comment block)? I guess it doesn't need its own function anyway. -- 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: Mon, 05 Feb 2018 13:05:42 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: ENE Embedded Debug Interface EDI and ENE KB9012 EC flashing support
by Paul Kocialkowski (Code Review) 05 Feb '18

05 Feb '18

Paul Kocialkowski has posted comments on this change. ( https://review.coreboot.org/23259 ) Change subject: ENE Embedded Debug Interface EDI and ENE KB9012 EC flashing support ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/#/c/23259/4//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23259/4//COMMIT_MSG@7 PS4, Line 7: ENE Embedded Debug Interface EDI and ENE KB9012 EC flashing support Oh by the way, if you like it better, this can be changed to: "Add support for the ENE Embedded Debug Interface EDI and KB9012 EC" I stopped using non-verbal phrases for git commits and use imperative forms nowadays. -- 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: 4 Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com> 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: Mon, 05 Feb 2018 13:01:27 +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 Mike Banon (Code Review) 04 Feb '18

04 Feb '18

Mike Banon 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 6: > (7 comments) > > We are close :) though, flashrom_image_write() seems to be too > convoluted to be extended easily and needs some more attention. Sorry for double patch commits (forgot to add enum the first time) -- 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: 6 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: Sun, 04 Feb 2018 17:52:04 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support for reading the current flash contents from a file
by build bot (Jenkins) (Code Review) 04 Feb '18

04 Feb '18

build bot (Jenkins) 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 6: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1100/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/930/ : SUCCESS -- 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: 6 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: Sun, 04 Feb 2018 17:48:52 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

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

04 Feb '18

Hello Paul Kocialkowski, Paul Menzel, David Hendricks, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/23263 to look at the new patch set (#6). Change subject: Add support for reading the current flash contents from a file ...................................................................... Add support for reading the current flash contents from a file When developing software that has to be flashed to a flash chip to be executed, it often takes a long time to read the current flash contents (for flashrom to know what pages to erase and reprogram) each time when writing the new image. However, when the flash was just reprogrammed, its current state is known to be the previous image that was flashed (assuming it was verified). Thus, it makes sense to provide that image as a file for the flash contents instead of wasting valuable time read the whole flash each time. Original patch has been created by Paul Kocialkowski, the previous version: http://patchwork.coreboot.org/patch/4414/ Now it has been ported to the latest source code of flashrom master branch Change-Id: Idf153b6955f37779ae9bfb228a434ed10c304947 Signed-off-by: Mike Banon <mikebdp2(a)gmail.com> Signed-off-by: Paul Kocialkowski <contact(a)paulk.fr> --- M cli_classic.c M flash.h M flashrom.c M libflashrom.h 4 files changed, 63 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/23263/6 -- 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: newpatchset Gerrit-Change-Id: Idf153b6955f37779ae9bfb228a434ed10c304947 Gerrit-Change-Number: 23263 Gerrit-PatchSet: 6 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>

1 0

Change in flashrom[master]: Add support for reading the current flash contents from a file
by build bot (Jenkins) (Code Review) 04 Feb '18

04 Feb '18

build bot (Jenkins) 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 5: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1099/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/929/ : SUCCESS -- 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: 5 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: Sun, 04 Feb 2018 17:45:13 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2018