February 2018 - flashrom-gerrit

Change in flashrom[master]: Add support for reading the current flash contents from a file
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/23263 ) Change subject: Add support for reading the current flash contents from a file ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1091/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/927/ : 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: 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 Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 03 Feb 2018 23:26:58 +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) 03 Feb '18

03 Feb '18

Hello Paul Menzel, 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 (#3). 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> --- M cli_classic.c M flash.h M flashrom.c M libflashrom.h 4 files changed, 60 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/23263/3 -- 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: 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 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 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: Add support for reading the current flash contents from a file ...................................................................... Patch Set 2: (13 comments) A general question: Would it make sense to update the given reference file after a successful flash? https://review.coreboot.org/#/c/23263/2/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23263/2/cli_classic.c@60 PS2, Line 60: " --ifd read layout from an Intel Firmware Descriptor\n" This line is too long now for a 80-columns terminal. Though, I think we can just avoid to shift all these lines, see below. https://review.coreboot.org/#/c/23263/2/cli_classic.c@63 PS2, Line 63: referencefile Maybe just shorten it, e.g. <ref-file>, and leave the other lines as they were. https://review.coreboot.org/#/c/23263/2/cli_classic.c@63 PS2, Line 63: -C I'm not sure about the `-C` maybe only add the long option for now? https://review.coreboot.org/#/c/23263/2/cli_classic.c@363 PS2, Line 363: contents "reference" https://review.coreboot.org/#/c/23263/2/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23263/2/flashrom.c@2348 PS2, Line 2348: * @param buffer_len Size of source buffer in bytes. reference parameter should be mentioned here https://review.coreboot.org/#/c/23263/2/flashrom.c@2355 PS2, Line 2355: int flashrom_image_write(struct flashctx *const flashctx, void *const buffer, const size_t buffer_len, const char *referencefile) I just realized that this is already part of the libflashrom interface which should be kept free of file i/o handling. You should pass another buffer here, already filled with the contents of the reference file, e.g. `refcontents` to align with `oldcontents`/`newcontents`. https://review.coreboot.org/#/c/23263/2/flashrom.c@2400 PS2, Line 2400: */ This justification makes sense in the commit message but doesn't belong here. Please condense it to a single line, e.g. /* If given, assume flash contains same data as `refcontents`. */ https://review.coreboot.org/#/c/23263/2/flashrom.c@2401 PS2, Line 2401: spurious empty line https://review.coreboot.org/#/c/23263/2/flashrom.c@2402 PS2, Line 2402: if (referencefile) { : msg_cinfo("Reading old flash chip contents from file... "); : if (read_buf_from_file(oldcontents, flash_size, referencefile)) { : ret = 1; : msg_cinfo("FAILED.\n"); : goto _finalize_ret; : } move into do_write() please https://review.coreboot.org/#/c/23263/2/flashrom.c@2410 PS2, Line 2410: spurious empty line https://review.coreboot.org/#/c/23263/2/flashrom.c@2418 PS2, Line 2418: spurious empty line https://review.coreboot.org/#/c/23263/2/flashrom.c@2438 PS2, Line 2438: if (verify_all) { : msg_cerr("Checking if anything has changed.\n"); : msg_cinfo("Reading current flash chip contents... "); : if (!flashctx->chip->read(flashctx, curcontents, 0, flash_size)) { : msg_cinfo("done.\n"); : if (!memcmp(oldcontents, curcontents, flash_size)) { : nonfatal_help_message(); : goto _finalize_ret; : } : msg_cerr("Apparently at least some data has changed.\n"); : } else : msg_cerr("Can't even read anymore!\n"); misaligned by accident? https://review.coreboot.org/#/c/23263/2/flashrom.c@2580 PS2, Line 2580: goto _free_ret; Same as `filename`, `referencefile` should be read here if given. -- 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: 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: Sat, 03 Feb 2018 19:49:09 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: 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: Add support for selecting the erased bit value with a flag ...................................................................... Patch Set 3: > (didn't retest KB9012 because the Bus Pirate is much slower with KB9012 > than other programmers) This might be fixed now, with 23057. -- 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 19:18:14 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add a SPI command class to `struct flashchip`
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/23262 ) Change subject: Add a SPI command class to `struct flashchip` ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1090/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/926/ : SUCCESS -- 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: 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 19:11:29 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: Add a SPI command class to `struct flashchip`
by Nico Huber (Code Review) 03 Feb '18

03 Feb '18

Nico Huber has uploaded a new patch set (#3) to the change originally created by Mike Banon. ( https://review.coreboot.org/23262 ) Change subject: Add a SPI command class to `struct flashchip` ...................................................................... Add a SPI command class to `struct flashchip` By default, we want to probe for SPI25 chips only. Other SPI use cases, like the ENE/EDI protocol, might use commands that can confuse these common chips. Now, flashrom will probe for a chip only if one of these conditions is true: 1) no chip has been specified AND the chip uses the SPI25 commands 2) this chip has been specified by -c | --chip <chipname> Change-Id: I89a53ccaef2791a2ac32904d7ab813da7478a6f0 Signed-off-by: Mike Banon <mikebdp2(a)gmail.com> Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flash.h M flashrom.c 2 files changed, 14 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/23262/3 -- 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: newpatchset Gerrit-Change-Id: I89a53ccaef2791a2ac32904d7ab813da7478a6f0 Gerrit-Change-Number: 23262 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]: Add support for reading the current flash contents from a file
by Mike Banon (Code Review) 03 Feb '18

03 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 2: > >> 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). > 1 - renamed contentsfile into referencefile, 2 - manpage update will do slightly later > > > 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? > "free-as-in-freedom" 's freedoms mean a lot to me, especially freedom to study (I'm a bit paranoid and its very important to me to have the ability to study the source code to verify that there aren't any backdoors) . yes I'm highlighting the importance of these patches, but thats because the creation of KB9012 firmware replacement is indeed very important: all the letters (including passwords) typed on laptop's keyboard are going through this chip -- 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: 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: Sat, 03 Feb 2018 13:29:17 +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) 03 Feb '18

03 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 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1089/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/925/ : 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: 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: Sat, 03 Feb 2018 13:09:43 +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) 03 Feb '18

03 Feb '18

Hello Paul Menzel, 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 (#2). 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> --- M cli_classic.c M flash.h M flashrom.c M libflashrom.h 4 files changed, 82 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/23263/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: newpatchset Gerrit-Change-Id: Idf153b6955f37779ae9bfb228a434ed10c304947 Gerrit-Change-Number: 23263 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>

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

Mike Banon 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: > > Thanks for the great work on this series! I had planned on > > submitting follow-up patches to this (I have not given up) so as > > Nico mentioned, it would be nice that I am still listed as author > > and have a signed-off-by line in the commits that get landed. I > > would be more than happy to add your signed-off line as well to > > show that you were also involved with this effort! > > I think the easiest way to do things would be that I submit my > rebased commit with your signed-off-by line so we can have the > right author. Just to rebase your commits - will not work because flashrom source code has been significantly changed (most importantly "Kill doit()") - thats why the additional modifications (included to my commits above) are required to make them work. So I am going to try to follow Nico Huber instruction to change the author to your name -- 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 12:53:29 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2018