- flashrom-gerrit - mail.coreboot.org

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

Hello 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 (#2). Change subject: [v4,4/6] ENE EDI - print debug info like others while probing for ENE chips ...................................................................... [v4,4/6] 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 0x0, rc2 0x0; (no newline here) hwversion 0xc3, ediid 0x04 Change-Id: Id8e62bc9f6785b4bf0be0aaf0f74c8120d77c0d4 Signed-off-by: Mike Banon <mikebdp2(a)gmail.com> --- M edi.c 1 file changed, 15 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/23261/2 -- 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: 2 Gerrit-Owner: 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]: [v4, 6/6] Add support for reading the current flash contents from a file
by Mike Banon (Code Review) 31 Jan '18

31 Jan '18

Mike Banon 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. This patch tries to migitate this problem, it is essential for the comfortable development of KB9012 free-as-in-freedom firmware replacement -- 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: Tue, 30 Jan 2018 23:01:42 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: [v4, 3/6] ENE EDI - add dummy read to ensure proper detection of ENE c...
by Mike Banon (Code Review) 31 Jan '18

31 Jan '18

Mike Banon 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); > `edi_detect` sounds a little like it would already probe for it... […] Yes, edi_detect is a function that includes probing - but this function would never be called if KB9012 has not been explicitly selected by the user. One of the following patches moves KB9012 into the "hidden chips" domain - these chips would still shown as supported by flashrom as well as other chips, but a hidden chip (KB9012 in this case) isn't probed for by flashrom unless the user chooses it with -c option -- 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: Tue, 30 Jan 2018 22:45:46 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: [v4, 2/6] ENE Embedded Debug Interface EDI and ENE KB9012 EC flashing ...
by Mike Banon (Code Review) 31 Jan '18

31 Jan '18

Mike Banon has posted comments on this change. ( https://review.coreboot.org/23259 ) Change subject: [v4,2/6] ENE Embedded Debug Interface EDI and ENE KB9012 EC flashing support ...................................................................... Patch Set 2: > Patch Set 2: Commit message was updated. This small change has caused a rebase (gerrit shows two patch sets although they are identical) -- 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: 2 Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 30 Jan 2018 22:40:45 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: [v4, 1/6] Add support for selecting the erased bit value with a flag
by Mike Banon (Code Review) 31 Jan '18

31 Jan '18

Mike Banon 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: (5 comments) > (5 comments) > > Code looks good, how well was the rebase tested? > (5 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 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> > As a courtesy, you should reset the author to Paul, or use […] All my commit messages which contain Paul's code - are giving the credit to Paul by including the note "Original patch has been created by Paul Kocialkowski" with a link to it. I'm not the author of the original patches, but while porting these patches I've modified them to make them compatible with the latest flashrom. And still haven't received any comment from Paul - it would be wrong to put his signature when we still do not even know his opinion about these changed patches. Also, these modified patches could contain mistakes (although I tried not to make any) - in which case it is me the committer who will have to be blamed https://review.coreboot.org/#/c/23258/1//COMMIT_MSG@7 PS1, Line 7: [v4,1/6] > Please remove this tag. After these tags are removed, would it still be convenient to merge these patches in the correct order? (1/6->2/6->3/6->4/6->5/6->6/6) I'm just trying to follow Paul's scheme from patchwork, it seemed to me as a right approach https://review.coreboot.org/#/c/23258/1//COMMIT_MSG@17 PS1, Line 17: Signed-off-by: Mike Banon <mikebdp2(a)gmail.com> > Please also keep original Signed-off-by lines. These patches have been modified to make them compatible with the latest flashrom, not taken as-is. Not sure if the original signed-off-by still applies, because - since I've modified these patches - I should take the full responsibility for their quality https://review.coreboot.org/#/c/23258/1/flashrom.c File flashrom.c: https://review.coreboot.org/#/c/23258/1/flashrom.c@762 PS1, Line 762: static int need_erase_gran_bytes(const uint8_t *have, const uint8_t *want, unsigned int len, > line is too long (112 chars limit) fixed at new edit https://review.coreboot.org/#/c/23258/1/flashrom.c@792 PS1, Line 792: */ > line is too long fixed at new edit -- 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: Tue, 30 Jan 2018 22:38:41 +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 build bot (Jenkins) (Code Review) 31 Jan '18

31 Jan '18

build bot (Jenkins) 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: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/922/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/1082/ : 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: 2 Gerrit-Owner: 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 22:35:51 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: [v4, 1/6] Add support for selecting the erased bit value with a flag
by Mike Banon (Code Review) 31 Jan '18

31 Jan '18

Hello 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 (#2). Change subject: [v4,1/6] Add support for selecting the erased bit value with a flag ...................................................................... [v4,1/6] 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/2 -- 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: 2 Gerrit-Owner: 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]: cli_classic: Add --chip-size command
by Nico Huber (Code Review) 26 Jan '18

26 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23346 ) Change subject: cli_classic: Add --chip-size command ...................................................................... Patch Set 3: Code-Review+2 (2 comments) https://review.coreboot.org/#/c/23346/3//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/23346/3//COMMIT_MSG@9 PS3, Line 9: This adds a command to print the size of the detected flash chip (in bytes). Line is too long :-P https://review.coreboot.org/#/c/23346/2/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23346/2/cli_classic.c@129 PS2, Line 129: 0x0101 > Change this (and the one for --ifd) to use an enum? Good idea. I also thought about making the operation a single variable (with a bitmask enum), currently *_it and chip_size are all mutually exclusive booleans. -- To view, visit https://review.coreboot.org/23346 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: Ia6f334654bea73478f369207e6f4345bcb72e8d4 Gerrit-Change-Number: 23346 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(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, 26 Jan 2018 18:58:16 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: flashchips: reset a M25P10-A before a probe
by Nico Huber (Code Review) 26 Jan '18

26 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23337 ) Change subject: flashchips: reset a M25P10-A before a probe ...................................................................... Patch Set 2: I'm don't feel experienced enough to say anything about such flash chip quirks. David, any clue? -- To view, visit https://review.coreboot.org/23337 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: I19a5332756d15b3cb74609c44a8d3fed2f4a2d68 Gerrit-Change-Number: 23337 Gerrit-PatchSet: 2 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: David Hendricks <david.hendricks(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, 26 Jan 2018 18:37:54 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: digilent_spi: add a driver for the iCEblink40 development board
by Nico Huber (Code Review) 26 Jan '18

26 Jan '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23338 ) Change subject: digilent_spi: add a driver for the iCEblink40 development board ...................................................................... Patch Set 2: (15 comments) Code looks quite nice :) haven't looked at the protocol yet, though. https://review.coreboot.org/#/c/23338/2/digilent_spi.c File digilent_spi.c: https://review.coreboot.org/#/c/23338/2/digilent_spi.c@14 PS2, Line 14: version 2 of the License is this restriction intentional? I've just noticed, most of flashrom is licensed v2+ https://review.coreboot.org/#/c/23338/2/digilent_spi.c@32 PS2, Line 32: #if CONFIG_DIGILENT_SPI == 1 This is unnecessary, the Makefile already takes care of it. https://review.coreboot.org/#/c/23338/2/digilent_spi.c@58 PS2, Line 58: 0 nit, spurious 0? https://review.coreboot.org/#/c/23338/2/digilent_spi.c@103 PS2, Line 103: 0 please always set a reasonable timeout https://review.coreboot.org/#/c/23338/2/digilent_spi.c@120 PS2, Line 120: return res_len; Rather `tx_len`? how about an error message for short replies? https://review.coreboot.org/#/c/23338/2/digilent_spi.c@184 PS2, Line 184: real_speed =(res[5] << 24) |(res[4] << 16) |(res[3] << 8) | res[2]; Missing spaces after = and | operators. https://review.coreboot.org/#/c/23338/2/digilent_spi.c@241 PS2, Line 241: static int digilent_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) please break this line (112 chars limit for flashrom) https://review.coreboot.org/#/c/23338/2/digilent_spi.c@249 PS2, Line 249: buf = alloca(len); I don't know how portable it is (and it's not used yet in flashrom). I'd prefer malloc(). https://review.coreboot.org/#/c/23338/2/digilent_spi.c@253 PS2, Line 253: &buf[writecnt] `buf + writecnt` would be much clearer to me. But that could be just me. ;) https://review.coreboot.org/#/c/23338/2/digilent_spi.c@264 PS2, Line 264: if (ret != 0) Error message, please. https://review.coreboot.org/#/c/23338/2/digilent_spi.c@269 PS2, Line 269: if (ret != 0) Same here. https://review.coreboot.org/#/c/23338/2/digilent_spi.c@303 PS2, Line 303: What about the libusb handle? https://review.coreboot.org/#/c/23338/2/digilent_spi.c@356 PS2, Line 356: spiclock Other programmers call it `spispeed`. https://review.coreboot.org/#/c/23338/2/digilent_spi.c@362 PS2, Line 362: return -1; `goto close_handle;` ? https://review.coreboot.org/#/c/23338/2/digilent_spi.c@374 PS2, Line 374: if (reset_board) { : gpio_open(); : gpio_set_dir(1); : gpio_set_value(0); : } : : spi_open(); : spi_set_speed(speed_hz); : spi_set_mode(0x00); Check return values, please. -- To view, visit https://review.coreboot.org/23338 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: I7ffcd9a2db4395816f0e8b6ce6c3b0d8e930c9e6 Gerrit-Change-Number: 23338 Gerrit-PatchSet: 2 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> 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, 26 Jan 2018 18:34:36 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit