December 2022 - flashrom-gerrit

[S] Change in flashrom[master]: README: Add information about meson and link build instructions
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: David Hendricks, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70176 ) Change subject: README: Add information about meson and link build instructions ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7 Gerrit-Change-Number: 70176 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 07 Dec 2022 22:09:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Change chip restore data type from uint8_t to void ptr
by Sergii Dmytruk (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70349 ) Change subject: tree/: Change chip restore data type from uint8_t to void ptr ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 18:54:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

07 Dec '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Eric Lai. Nico Huber 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+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/70342/comment/4e48c80b_36b65dc5 PS1, Line 19: FT2232H > Ok interesting, I wrote a little test case: […] Many thanks for the elaborate testing! -- 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: 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 11:53:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Angel Pons (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 12: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/59696b58_79997a2e PS2, Line 7: ichspi.c: Read chip ID and use it to identify chip > Subrata can you take a look too please. There aren't that many systems with 2 flash chips, but they exist. Ivy Bridge and Haswell ThinkPads have a 8 MiB chip and a 4 MiB chip, and flashrom detects a single 12 MiB opaque flash chip. The problem with sending WP commands in such situations is that you have 2 chips to deal with. No idea how hwseq handles this. -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 12 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 10:55:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> 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

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nico Huber (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 12: (6 comments) File ichspi.c: https://review.coreboot.org/c/flashrom/+/69195/comment/229372ca_f2c15230 PS12, Line 1500: flash->chip->tested = TEST_OK_PREWB; What if ich_hwseq_get_flash_id() bailed? https://review.coreboot.org/c/flashrom/+/69195/comment/26c5d96f_52e3d538 PS12, Line 135: #define HSFC_CYCLE_RDID HSFC_FCYCLE_MASK(6) This belongs to PCH100_HSFC_FCYCLE*. It doesn't fit in the 2 bits for ICH9. (ICH9_HSFC_FCYCLE_BIT_WIDTH is a mask, not a number of bits, apparently). https://review.coreboot.org/c/flashrom/+/69195/comment/4d7ff62c_95702594 PS12, Line 1404: if ((chip->manufacture_id == mfg_id) && (chip->model_id == model_id)) Should check if it's a SPI flash and if the database numbers are for RDID. https://review.coreboot.org/c/flashrom/+/69195/comment/d7deac47_99b4b57a PS12, Line 1466: const int len = sizeof(data); Why 4? https://review.coreboot.org/c/flashrom/+/69195/comment/699cafbf_a65018c0 PS12, Line 1474: if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_RDID, 1, len, ich_generation, Should check if RDID cycle is supported (since PCH100). https://review.coreboot.org/c/flashrom/+/69195/comment/e43ed110_ed398c89 PS12, Line 1494: return -1; Does this break normal read/erase/write operation if there is no matching entry? -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 12 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 10:49:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: layout: Factor out flash_region structure from romentry
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/+/69196 ) Change subject: layout: Factor out flash_region structure from romentry ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: Someone else needs to approve as to not approve my own co-authored patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/69196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2 Gerrit-Change-Number: 69196 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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 09:31:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi: Expose flash descriptor regions through get_region()
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70127 ) Change subject: ichspi: Expose flash descriptor regions through get_region() ...................................................................... Patch Set 9: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70127 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I019f3f407f6a2a82f686a168457e0e32961ff483 Gerrit-Change-Number: 70127 Gerrit-PatchSet: 9 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 09:29:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Subrata Banik (Code Review) 07 Dec '22

07 Dec '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 12 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 07 Dec 2022 09:07:08 +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 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 serprog.c: https://review.coreboot.org/c/flashrom/+/67393/comment/d9fe24da_995d7993 PS9, Line 456: static void serprog_delay(const struct flashctx *flash, unsigned int usecs); > `serprog_delay()` uses `sp_check_opbuf_usage()` and that is below this struct. […] https://review.coreboot.org/c/flashrom/+/70443 -- 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:46:04 +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]: serprog.c: Dispense with forward decl by reshuffle
by Edward O'Callaghan (Code Review) 07 Dec '22

07 Dec '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70443 ) Change subject: serprog.c: Dispense with forward decl by reshuffle ...................................................................... serprog.c: Dispense with forward decl by reshuffle Change-Id: I0ffcd8bd03fae0d905887a88c9b2e06f61577fd4 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M serprog.c 1 file changed, 41 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/70443/1 diff --git a/serprog.c b/serprog.c index a7796ba..8d24b5a 100644 --- a/serprog.c +++ b/serprog.c @@ -453,7 +453,37 @@ return NULL; } -static void serprog_delay(const struct flashctx *flash, unsigned int usecs); +static int sp_check_opbuf_usage(int bytes_to_be_added) +{ + if (sp_device_opbuf_size <= (sp_opbuf_usage + bytes_to_be_added)) { + /* If this happens in the middle of a page load the page load will probably fail. */ + msg_pwarn(MSGHEADER "Warning: executed operation buffer due to size reasons\n"); + if (sp_execute_opbuf() != 0) + return 1; + } + return 0; +} + +static void serprog_delay(const struct flashctx *flash, unsigned int usecs) +{ + unsigned char buf[4]; + msg_pspew("%s usecs=%d\n", __func__, usecs); + if (!sp_check_commandavail(S_CMD_O_DELAY)) { + msg_pdbg2("serprog_delay used, but programmer doesn't support delays natively - emulating\n"); + internal_delay(usecs); + return; + } + if ((sp_max_write_n) && (sp_write_n_bytes)) + sp_pass_writen(); + sp_check_opbuf_usage(5); + buf[0] = ((usecs >> 0) & 0xFF); + buf[1] = ((usecs >> 8) & 0xFF); + buf[2] = ((usecs >> 16) & 0xFF); + buf[3] = ((usecs >> 24) & 0xFF); + sp_stream_buffer_op(S_CMD_O_DELAY, 4, buf); + sp_opbuf_usage += 5; + sp_prev_was_write = 0; +} static struct spi_master spi_master_serprog = { .map_flash_region = serprog_map, @@ -469,17 +499,6 @@ .delay = serprog_delay, }; -static int sp_check_opbuf_usage(int bytes_to_be_added) -{ - if (sp_device_opbuf_size <= (sp_opbuf_usage + bytes_to_be_added)) { - /* If this happens in the middle of a page load the page load will probably fail. */ - msg_pwarn(MSGHEADER "Warning: executed operation buffer due to size reasons\n"); - if (sp_execute_opbuf() != 0) - return 1; - } - return 0; -} - static void serprog_chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr) { @@ -571,27 +590,6 @@ sp_do_read_n(&(buf[addrm-addr]), addrm, lenm); // FIXME: return error } -static void serprog_delay(const struct flashctx *flash, unsigned int usecs) -{ - unsigned char buf[4]; - msg_pspew("%s usecs=%d\n", __func__, usecs); - if (!sp_check_commandavail(S_CMD_O_DELAY)) { - msg_pdbg2("serprog_delay used, but programmer doesn't support delays natively - emulating\n"); - internal_delay(usecs); - return; - } - if ((sp_max_write_n) && (sp_write_n_bytes)) - sp_pass_writen(); - sp_check_opbuf_usage(5); - buf[0] = ((usecs >> 0) & 0xFF); - buf[1] = ((usecs >> 8) & 0xFF); - buf[2] = ((usecs >> 16) & 0xFF); - buf[3] = ((usecs >> 24) & 0xFF); - sp_stream_buffer_op(S_CMD_O_DELAY, 4, buf); - sp_opbuf_usage += 5; - sp_prev_was_write = 0; -} - static const struct par_master par_master_serprog = { .map_flash_region = serprog_map, .chip_readb = serprog_chip_readb, -- To view, visit https://review.coreboot.org/c/flashrom/+/70443 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0ffcd8bd03fae0d905887a88c9b2e06f61577fd4 Gerrit-Change-Number: 70443 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2022