February 2018 - flashrom-gerrit

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 (#5). 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, 59 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/23263/5 -- 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: 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>

1 0

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

04 Feb '18

Mike Banon 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: (1 comment) https://review.coreboot.org/#/c/23261/4/edi.c File edi.c: https://review.coreboot.org/#/c/23261/4/edi.c@162 PS4, Line 162: } > Thanks for adding this, it's indeed useful to be a bit more verbose than I was initially! […] Thank you for the feedback, I've implemented your suggestions -- 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: Sun, 04 Feb 2018 16:45:11 +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 build bot (Jenkins) (Code Review) 04 Feb '18

04 Feb '18

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

1 0

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

04 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/23261 to look at the new patch set (#5). Change subject: ENE EDI - print debug info like others while probing for ENE chips ...................................................................... ENE EDI - print debug info like others while probing for ENE chips Instead of just "Probing for ENE KB9012 (EDI), 128 kB:", lets print some debug info - like it is currently being printed for other chips: Probing for ENE KB9012 (EDI), 128 kB: edi_chip_probe: rc1 0, rc2 0; hwversion 0xc3, ediid 0x04 Change-Id: Id8e62bc9f6785b4bf0be0aaf0f74c8120d77c0d4 Signed-off-by: Mike Banon <mikebdp2(a)gmail.com> Signed-off-by: Paul Kocialkowski <contact(a)paulk.fr> --- M edi.c 1 file changed, 12 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/23261/5 -- 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: newpatchset 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>

1 0

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

04 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 4: (1 comment) https://review.coreboot.org/#/c/23261/4/edi.c File edi.c: https://review.coreboot.org/#/c/23261/4/edi.c@162 PS4, Line 162: } Thanks for adding this, it's indeed useful to be a bit more verbose than I was initially! Naming variables rc1 and rc2 does not feel very consistent with the rest of the code in this file though, so I suggest that you keep using a single rc variable. Most importantly, I don't think it makes a lot of sense to print "rc1" and "rc2" in the error string. Instead, I think it would make more sense to print something like "reading EDIID failed" and "reading HWVERSION failed". What do you think? -- 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: 4 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: Sun, 04 Feb 2018 14:31:49 +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) 04 Feb '18

04 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 4: (7 comments) We are close :) though, flashrom_image_write() seems to be too convoluted to be extended easily and needs some more attention. https://review.coreboot.org/#/c/23263/4/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23263/4/cli_classic.c@114 PS4, Line 114: C: You can just drop the C: and it should work. The 'C' below only tells getopt_long() what to return in case of --flash-contents which could be any distinct number. Though, better would be to use a non-character number like --ifd does, e.g. 0x0101. Ideally, we would use an enum for these numbers like this: enum { OPTION_IFD = 0x0100, OPTION_FLASH_CONTENTS, }; https://review.coreboot.org/#/c/23263/4/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23263/4/flashrom.c@2349 PS4, Line 2349: * @param refbuffer If given, assume flash contains same data as `refbuffer` Full stop please, as the lines above have that too. https://review.coreboot.org/#/c/23263/4/flashrom.c@2349 PS4, Line 2349: flash `flash chip` please, for consistent wording. https://review.coreboot.org/#/c/23263/4/flashrom.c@2357 PS4, Line 2357: void Can be (and therefore should be) const. https://review.coreboot.org/#/c/23263/4/flashrom.c@2397 PS4, Line 2397: oldcontents = refcontents; I would rather copy the data here. Would allow to keep the surrounding code as is. You'd have to keep track of where `oldcontents` points to, otherwise, e.g. to make sure you don't free() `refbuffer`. Also, I just realized that you need to copy the data into `curcontents` as well. `oldcontents` is only optionally used for verification later and not for the checks which parts of the flash chip have to be updated. Basically, after this if/else block, `curcontents` has to contain valid data at least for all included regions in the layout, and `oldcontents` has to contain valid data for the whole flash if `verify_all` is set. With that written down, you can/should only copy into `oldcontents` if `verify_all` is set (in other words if `oldcontents != NULL`). https://review.coreboot.org/#/c/23263/4/flashrom.c@2561 PS4, Line 2561: uint8_t *refcontents = NULL; Please move up to the general declarations above (`newcontents` is only declared later because it couldn't be `const` otherwise, stupid language if you ask me, or stupid me because I always try to make as much as possible `const`). https://review.coreboot.org/#/c/23263/4/flashrom.c@2582 PS4, Line 2582: _free_ret_refcontents: You only need a single `_free_ret` as `free(NULL);` is valid. -- 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: 4 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: Sun, 04 Feb 2018 12:03:15 +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 Mike Banon (Code Review) 04 Feb '18

04 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 4: > 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! Paul, thank you very much for your original KB9012 work! I have amended 4 patches (original version of which has been created by you) to set you as the author, and also added your signed-off-by line to all 6 patches. Hopefully, once these patches are improved and accepted, we could go ahead to the free firmware creation -- 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: 4 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: Sun, 04 Feb 2018 00:34:35 +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 4: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1097/ : 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: 4 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: Sun, 04 Feb 2018 00:16:32 +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, 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 (#4). 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, 60 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/23263/4 -- 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: 4 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 a SPI command class to `struct flashchip`
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/23262 ) Change subject: Add a SPI command class to `struct flashchip` ...................................................................... Patch Set 4: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1096/ : 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: 4 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: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 04 Feb 2018 00:13:12 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2018