July 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashchips: Add MX25L12833F
by Tao Xia (Code Review) 23 Jul '21

23 Jul '21

Tao Xia has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56546 ) Change subject: flashchips: Add MX25L12833F ...................................................................... flashchips: Add MX25L12833F Just add the name to the existing entry, as usual it is supposed to be compatible. Signed-off-by: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Change-Id: I14ab7e04f5209d2bcf34b0d2de9da2c01bf32d00 --- M flashchips.c M flashchips.h 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/56546/1 diff --git a/flashchips.c b/flashchips.c index 8cd3dfd..bec16cc 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8359,7 +8359,7 @@ { .vendor = "Macronix", - .name = "MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F", + .name = "MX25L12833F/MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, .model_id = MACRONIX_MX25L12805D, diff --git a/flashchips.h b/flashchips.h index 249e7b4..e190dca 100644 --- a/flashchips.h +++ b/flashchips.h @@ -506,7 +506,7 @@ #define MACRONIX_MX25L1605 0x2015 /* MX25L1605 (64k 0x20); MX25L1605A/MX25L1606E/MX25L1608E (4k 0x20, 64k 0x52); MX25L1605D/MX25L1608D/MX25L1673E (4k 0x20) */ #define MACRONIX_MX25L3205 0x2016 /* MX25L3205, MX25L3205A (64k 0x20); MX25L3205D/MX25L3208D (4k 0x20); MX25L3206E/MX25L3208E (4k 0x20, 64k 0x52); MX25L3233F/MX25L3273E (4k 0x20, 32k 0x52) */ #define MACRONIX_MX25L6405 0x2017 /* MX25L6405, MX25L6405D (64k 0x20); MX25L6406E/MX25L6408E (4k 0x20); MX25L6436E/MX25L6445E/MX25L6465E/MX25L6473E (4k 0x20, 32k 0x52) */ -#define MACRONIX_MX25L12805D 0x2018 /* MX25L12805D (no 32k); MX25L12865E, MX25L12835F, MX25L12845E, MX25L12873F (32k 0x52) */ +#define MACRONIX_MX25L12805D 0x2018 /* MX25L12805D (no 32k); MX25L12865E, MX25L12835F, MX25L12845E, MX25L12873F, MX25L12833F (32k 0x52) */ #define MACRONIX_MX25L25635F 0x2019 /* Same as MX25L25639F, but the latter seems to not support REMS */ #define MACRONIX_MX25L1635D 0x2415 #define MACRONIX_MX25L1635E 0x2515 /* MX25L1635{E} */ -- To view, visit https://review.coreboot.org/c/flashrom/+/56546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I14ab7e04f5209d2bcf34b0d2de9da2c01bf32d00 Gerrit-Change-Number: 56546 Gerrit-PatchSet: 1 Gerrit-Owner: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ene_lpc: remove ENE LPC programmer
by Victor Ding (Code Review) 23 Jul '21

23 Jul '21

Attention is currently required from: Mike Banon, Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Victor Ding has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56475 ) Change subject: ene_lpc: remove ENE LPC programmer ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/56475/comment/b1fa11a5_f458992e PS1, Line 10: reached Auto Update Expiration (AUE) a.k.a. end-of-life. > Makes sense Anastasia, done. Thank you all for the valuable comments. I've updated the commit message to better reflect why we want to have these two programmers removed. -- To view, visit https://review.coreboot.org/c/flashrom/+/56475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f40db22c42c04ce029c4defd837e05ebb550c9b Gerrit-Change-Number: 56475 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Jul 2021 04:22:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: mec1308: remove MEC1308 EC programmer
by Victor Ding (Code Review) 23 Jul '21

23 Jul '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Victor Ding has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56476 ) Change subject: mec1308: remove MEC1308 EC programmer ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/56476/comment/816d05d7_e6583bb7 PS1, Line 9: Last known device that uses MEC1308 Embedded Controller has reached Auto : Update Expiration (AUE) a.k.a. end-of-life. : > see https://review.coreboot. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/56476 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I535b6380846734c999474519e9e60a73eb6a2ec4 Gerrit-Change-Number: 56476 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Jul 2021 04:18:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc: remove ENE LPC programmer
by Victor Ding (Code Review) 23 Jul '21

23 Jul '21

Attention is currently required from: Mike Banon, Nico Huber, Victor Ding, Angel Pons, Anastasia Klimchuk. Hello Mike Banon, build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56475 to look at the new patch set (#2). Change subject: ene_lpc: remove ENE LPC programmer ...................................................................... ene_lpc: remove ENE LPC programmer Best efforts were made to upstream older Chromebook support for good intentions for folks interested. However, we no longer have the hardware available to test and maintain the code as the hardware is now end of life. Therefore the code state has sadly fallen into a unknown state. BUG=none BRANCH=none TEST=builds and ninja test passes Signed-off-by: Victor Ding <victording(a)google.com> Change-Id: I3f40db22c42c04ce029c4defd837e05ebb550c9b --- M Makefile D ene_lpc.c M meson.build M meson_options.txt M programmer.h M programmer_table.c M tests/init_shutdown.c M tests/tests.c M tests/tests.h 9 files changed, 3 insertions(+), 681 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/56475/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f40db22c42c04ce029c4defd837e05ebb550c9b Gerrit-Change-Number: 56475 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: mec1308: remove MEC1308 EC programmer
by Victor Ding (Code Review) 23 Jul '21

23 Jul '21

Attention is currently required from: Nico Huber, Victor Ding, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56476 to look at the new patch set (#2). Change subject: mec1308: remove MEC1308 EC programmer ...................................................................... mec1308: remove MEC1308 EC programmer Best efforts were made to upstream older Chromebook support for good intentions for folks interested. However, we no longer have the hardware available to test and maintain the code as the hardware is now end of life. Therefore the code state has sadly fallen into a unknown state. BUG=none BRANCH=none TEST=builds and ninja test passes Signed-off-by: Victor Ding <victording(a)google.com> Change-Id: I535b6380846734c999474519e9e60a73eb6a2ec4 --- M Makefile D mec1308.c M meson.build M meson_options.txt M programmer.h M programmer_table.c M tests/init_shutdown.c M tests/tests.c M tests/tests.h 9 files changed, 2 insertions(+), 602 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/56476/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56476 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I535b6380846734c999474519e9e60a73eb6a2ec4 Gerrit-Change-Number: 56476 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ene_lpc: remove ENE LPC programmer
by Edward O'Callaghan (Code Review) 23 Jul '21

23 Jul '21

Attention is currently required from: Mike Banon, Nico Huber, Victor Ding, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56475 ) Change subject: ene_lpc: remove ENE LPC programmer ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/56475/comment/49d9aa33_03509cf6 PS1, Line 10: reached Auto Update Expiration (AUE) a.k.a. end-of-life. > If it comes to testing, CB:56102 needs to be cherry-picked (ene and mec won't work without it). […] Makes sense Anastasia, done. -- To view, visit https://review.coreboot.org/c/flashrom/+/56475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f40db22c42c04ce029c4defd837e05ebb550c9b Gerrit-Change-Number: 56475 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Victor Ding <victording(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Jul 2021 02:51:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi_master: Add default write_aai function to masters that have none
by Edward O'Callaghan (Code Review) 23 Jul '21

23 Jul '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56102 ) Change subject: spi_master: Add default write_aai function to masters that have none ...................................................................... spi_master: Add default write_aai function to masters that have none write_aai is required to register a spi master, if it is not set in spi_master struct then register_spi_master returns ERROR_FLASHROM_BUG. Masters in this patch did not have it set in the struct, and register_spi_master always returned an error for them. However return value of register_spi_master was ignored, so this was hard to notice. Next patch in the chain checks return value of register_spi_master. BUG=b:185191942 TEST=builds and ninja test Change-Id: I712e74e11244e1f0ab8d8e245fcd5207ce211219 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56102 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M ene_lpc.c M mec1308.c 2 files changed, 2 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/ene_lpc.c b/ene_lpc.c index b30eee6..23ba12b 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -511,6 +511,7 @@ .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, }; static int check_params(void) diff --git a/mec1308.c b/mec1308.c index 295bffb..c085ff2 100644 --- a/mec1308.c +++ b/mec1308.c @@ -404,6 +404,7 @@ .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, }; static int check_params(void) -- To view, visit https://review.coreboot.org/c/flashrom/+/56102 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I712e74e11244e1f0ab8d8e245fcd5207ce211219 Gerrit-Change-Number: 56102 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Victor Ding <victording(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ene_lpc: remove ENE LPC programmer
by Anastasia Klimchuk (Code Review) 23 Jul '21

23 Jul '21

Attention is currently required from: Mike Banon, Nico Huber, Victor Ding, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56475 ) Change subject: ene_lpc: remove ENE LPC programmer ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/56475/comment/150c674b_c85bd15d PS1, Line 10: reached Auto Update Expiration (AUE) a.k.a. end-of-life. > Nico said in an email: […] If it comes to testing, CB:56102 needs to be cherry-picked (ene and mec won't work without it). Or maybe, CB:56102 can even be submitted? It's a part of the chain, but only because that's how I found the issue. Can be submitted independently. -- To view, visit https://review.coreboot.org/c/flashrom/+/56475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f40db22c42c04ce029c4defd837e05ebb550c9b Gerrit-Change-Number: 56475 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Victor Ding <victording(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 22 Jul 2021 23:52:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc: remove ENE LPC programmer
by Angel Pons (Code Review) 22 Jul '21

22 Jul '21

Attention is currently required from: Mike Banon, Nico Huber, Victor Ding, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56475 ) Change subject: ene_lpc: remove ENE LPC programmer ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/56475/comment/b99ef68c_a02251e2 PS1, Line 10: reached Auto Update Expiration (AUE) a.k.a. end-of-life. Nico said in an email: > Ah, maybe we should ask around if ayybody has the means of external recovery. It would be nice to support that machine indeed. flashrom supports externally reflashing a KB9012 EC via EDI (ENE Debug Interface). If the ene_lpc programmer corrupts something, one could use EDI to recover. -- To view, visit https://review.coreboot.org/c/flashrom/+/56475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f40db22c42c04ce029c4defd837e05ebb550c9b Gerrit-Change-Number: 56475 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Ding <victording(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mike Banon <mikebdp2(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Victor Ding <victording(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 22 Jul 2021 09:15:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add test to erase a chip
by Angel Pons (Code Review) 22 Jul '21

22 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add test to erase a chip ...................................................................... Patch Set 1: (9 comments) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/56501/comment/eab68aaf_20dec834 PS1, Line 26: /* : * Populate buf with the value used for erase operation, this allows to pass : * verification checks and also emulates successful erase. : */ : buf = memset(buf, 0xff, len); > should this memset happen instead in the block erase fn and use a singleton state here to see if tha […] +1. Doing the erases in `block_erase_chip()` accounting for the `blockaddr` and `blocklen` parameters is much more robust. This would also allow testing partial block updates, e.g. smallest erase block size is 2 KiB, but the boundary between two contiguous regions isn't aligned to at least 2 KiB, and only one of the regions is included. Hmmm, but doesn't dummyflasher already emulate flash chips? Why not use them, then? https://review.coreboot.org/c/flashrom/+/56501/comment/93c297da_fef98dab PS1, Line 42: printf("Unlock chip called\n"); > The singleton state could also be queried with a counter to ensure a double unlock isn't done? Even better would be to prevent erases/writes if the chip hasn't been unlocked. https://review.coreboot.org/c/flashrom/+/56501/comment/7ea8c4ec_68a8a071 PS1, Line 57: char *param_dup = strdup(""); The second parameter of `programmer_init()` is of `const char *` type. You don't need `strdup()`, just use a string literal: const char *param = ""; https://review.coreboot.org/c/flashrom/+/56501/comment/5d80eee8_0b01b265 PS1, Line 60: "aklm" 😜 https://review.coreboot.org/c/flashrom/+/56501/comment/a1646ce6_167b73e3 PS1, Line 61: /* : * Total size less than 16 * 1024 to skip some steps : * in flashrom.c#prepare_flash_access. : */ Is skipping these steps an issue, though? `spi_master_no_4ba_modes()` can only return true for SPI programmers with the `SPI_MASTER_NO_4BA_MODES` flag. Currently, only Dediprog sets this flag. https://review.coreboot.org/c/flashrom/+/56501/comment/a544af9d_e9e8fe20 PS1, Line 73: {2 * 1024, 3} Huh? This is just three blocks of 2 KiB each. The flash chip size is 8 MiB (`total_size` is in KiB). If you emulate the flash chip contents for the test (zero-initialise the buffer and erase the blocks in `block_erase_chip`), you should see that verification fails. https://review.coreboot.org/c/flashrom/+/56501/comment/38370cac_1bca4647 PS1, Line 86: 0x00002000 chip->total_size * KiB https://review.coreboot.org/c/flashrom/+/56501/comment/291c939a_bc526206 PS1, Line 86: 0x00000000 0 https://review.coreboot.org/c/flashrom/+/56501/comment/0d89f67c_90d92970 PS1, Line 114: free(param_dup); If using a string literal on line 57, this must be removed. -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 22 Jul 2021 09:09:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit July 2021