December 2022 - flashrom-gerrit

[M] Change in flashrom[master]: tree/: Move programmer_delay() out of programmer state machine
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67393 ) Change subject: tree/: Move programmer_delay() out of programmer state machine ...................................................................... Patch Set 10: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/67393/comment/3f9d9b8a_34ffb53d PS9, Line 265: if (flash->mst->buses_supported & BUS_SPI) { : if (flash->mst->spi.delay) : flash->mst->spi.delay(flash, usecs); : } else if (flash->mst->buses_supported & BUS_PARALLEL) { : if (flash->mst->par.delay) : flash->mst->par.delay(flash, usecs); : } else : internal_delay(usecs); > Isn't this logically equivalent? Also delay functions have a void return type. Done? -- To view, visit https://review.coreboot.org/c/flashrom/+/67393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c Gerrit-Change-Number: 67393 Gerrit-PatchSet: 10 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 07 Dec 2022 06:42:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Move programmer_delay() out of programmer state machine
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, Alexander Goncharov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67393 to look at the new patch set (#10). Change subject: tree/: Move programmer_delay() out of programmer state machine ...................................................................... tree/: Move programmer_delay() out of programmer state machine Handle the special cases of both serprog and ch341a_spi. Also rewrite programmer_delay() to handle the two base cases of zero time and no valid flashctx yet before handling per master branching. Additionally, modify the custom delay function pointer signature to allow closure over the flashctx. This allows driver specific delay implementations to recover programmer specific opaque data within their delay implementations. Therefore programmer specific delay functions can avoid programmer specific globals. Change-Id: Id059abb58b31a066a408009073912da2b224d40c Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M ch341a_spi.c M flashrom.c M include/programmer.h M serprog.c 4 files changed, 65 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/67393/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/67393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c Gerrit-Change-Number: 67393 Gerrit-PatchSet: 10 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70128 ) Change subject: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions ...................................................................... Patch Set 14: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/70128/comment/e60b6e4f_2fdb4cfa PS13, Line 348: region->name = strdup(""); > I mean, set to `NULL` in this function and at the call sites of `get_flash_region()` just have; `con […] Oh I see the `strdup()` now in the previous patch to which you are referring to. OK, just insert the `free()`'s. The problem will go away upon the second iteration of this when the fixture is within the layout life-time. This `strdup()` thing is relatively minor in the grand scheme of things, avoid fixating on it in favor of realising the layout integration goal. -- To view, visit https://review.coreboot.org/c/flashrom/+/70128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Gerrit-Change-Number: 70128 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 05:36:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Move programmer_delay() out of programmer state machine
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67393 ) Change subject: tree/: Move programmer_delay() out of programmer state machine ...................................................................... Patch Set 9: (2 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/67393/comment/b32f921b_114e240a PS9, Line 265: if (flash->mst->buses_supported & BUS_SPI) { : if (flash->mst->spi.delay) : flash->mst->spi.delay(flash, usecs); : } else if (flash->mst->buses_supported & BUS_PARALLEL) { : if (flash->mst->par.delay) : flash->mst->par.delay(flash, usecs); : } else : internal_delay(usecs); > This changes the logic, it seems to me? […] Isn't this logically equivalent? Also delay functions have a void return type. File serprog.c: https://review.coreboot.org/c/flashrom/+/67393/comment/dcac7b87_063715e9 PS9, Line 456: static void serprog_delay(const struct flashctx *flash, unsigned int usecs); > I am missing something, why the static forward declaration is needed? Can it be the `serprog_delay` […] `serprog_delay()` uses `sp_check_opbuf_usage()` and that is below this struct. I am avoiding reshuffling so much of the file. -- To view, visit https://review.coreboot.org/c/flashrom/+/67393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c Gerrit-Change-Number: 67393 Gerrit-PatchSet: 9 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 07 Dec 2022 05:07:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence
by Anastasia Klimchuk (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev, Eric Lai. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70342 ) Change subject: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70342 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Gerrit-Change-Number: 70342 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 04:25:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Move programmer_delay() out of programmer state machine
by Anastasia Klimchuk (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67393 ) Change subject: tree/: Move programmer_delay() out of programmer state machine ...................................................................... Patch Set 9: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67393/comment/dfd95801_4125cad4 PS4, Line 13: > I modified the signature as above and updated the commit message to explain that programmer specific […] I re-read the discussion, and it seems resolved to me. The last question from Angel: > If the long-term goal is to remove the global state, in which struct should these currently-global variables go? These currently-global variables should move together with ex-global variables, which is going into a data struct that every registered master has already. But Edward gave a lot more more detailed explanation above. I am resolving this thread (I added two other comments, but that's a different story :) ). However, Angel, if you have more thoughts please tell us! File flashrom.c: https://review.coreboot.org/c/flashrom/+/67393/comment/2b409423_8fef924d PS9, Line 265: if (flash->mst->buses_supported & BUS_SPI) { : if (flash->mst->spi.delay) : flash->mst->spi.delay(flash, usecs); : } else if (flash->mst->buses_supported & BUS_PARALLEL) { : if (flash->mst->par.delay) : flash->mst->par.delay(flash, usecs); : } else : internal_delay(usecs); This changes the logic, it seems to me? If we need to keep the logic "internal_delay is default unless custom delay defined" then internal_delay needs to always be called unless custom delay defined. Which would be (in my mind): ``` if (flash->mst->buses_supported & BUS_SPI) if (flash->mst->spi.delay) return flash->mst->spi.delay(flash, usecs); if (flash->mst->buses_supported & BUS_PARALLEL) if (flash->mst->par.delay) return flash->mst->par.delay(flash, usecs); return internal_delay(usecs); ``` "If there is nothing custom, call internal_delay" Does this seem right to you? File serprog.c: https://review.coreboot.org/c/flashrom/+/67393/comment/0b36bcdb_1ab1c994 PS9, Line 456: static void serprog_delay(const struct flashctx *flash, unsigned int usecs); I am missing something, why the static forward declaration is needed? Can it be the `serprog_delay` itself here? -- To view, visit https://review.coreboot.org/c/flashrom/+/67393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c Gerrit-Change-Number: 67393 Gerrit-PatchSet: 9 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 04:11:03 +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

[M] Change in flashrom[master]: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70128 ) Change subject: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions ...................................................................... Patch Set 14: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/70128/comment/2a9402bb_3bf08644 PS13, Line 348: region->name = strdup(""); > Yeah but we still have the case where ichspi allocates strings as well. […] I mean, set to `NULL` in this function and at the call sites of `get_flash_region()` just have; `const char *rname = region.name ? region.name : "";` and subsequently use `rname` in the `msg_gdbg()`. -- To view, visit https://review.coreboot.org/c/flashrom/+/70128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Gerrit-Change-Number: 70128 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 03:41:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions
by Nikolai Artemiev (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Edward O'Callaghan, Angel Pons. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70128 ) Change subject: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions ...................................................................... Patch Set 13: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/70128/comment/e9c53620_6b7a86e7 PS6, Line 11: platforms with active an CSME coprocessor. > Would it help to split this up into read/write/erase/verify ops? I feel like 'erase' and 'read' are […] Ack, we can handle the cases differently. For consistency maybe erase should just use the same flag as write though. File flashrom.c: https://review.coreboot.org/c/flashrom/+/70128/comment/ab127022_c37aa188 PS13, Line 348: region->name = strdup(""); > can `region->name` not be NULL and treat that as the empty string? Yeah but we still have the case where ichspi allocates strings as well. Maybe we can drop that too but I'd prefer to find a better solution if possible. https://review.coreboot.org/c/flashrom/+/70128/comment/e9637f60_c271471e PS13, Line 579: i > use `addr` not `i`, it is more descriptive, index is prone to error. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/70128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Gerrit-Change-Number: 70128 Gerrit-PatchSet: 13 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 02:57:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Position heap alloc along side check in compare_range()
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69473 ) Change subject: flashrom.c: Position heap alloc along side check in compare_range() ...................................................................... Patch Set 1: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/69473/comment/259f53f0_5cea8f1e PS1, Line 451: uint8_t *cmpbuf = malloc(len); > Felix, from a functional standpoint, this change only swaps the order of two variable declarations ( […] That's right, I sometimes run djgpp on my personal machine (we don't have it in corp). However djgpp supports C99 in the GCC frontend fine, A ANSI C89 requirement would be a exceedingly old/obstructive/obsolescent environment to work with. -- To view, visit https://review.coreboot.org/c/flashrom/+/69473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0386ac4c09a541cb9a659b2410ce49c3292ecc6e Gerrit-Change-Number: 69473 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 02:51:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> 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

[M] Change in flashrom[master]: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions
by Nikolai Artemiev (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70128 to look at the new patch set (#14). Change subject: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions ...................................................................... flashrom: Skip read/write/erase/verify ops on inaccessable flash regions Skip flash regions that get_region() indicates are inaccessable. This fixes transaction errors due to accessing unreadable flash regions on Intel platforms with active an CSME coprocessor. BUG=b:260440773 BRANCH=none TEST=flashrom -{r,w,E,v} on dedede (JSL) Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b CoAuthored-by: Edward O'Callaghan <quasisec(a)google.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 178 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/28/70128/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/70128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Gerrit-Change-Number: 70128 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2022