January 2022 - flashrom-gerrit

Change in flashrom[master]: flashrom.c: Move do_*() helpers into cli_classic.c
by Edward O'Callaghan (Code Review) 26 Jan '22

26 Jan '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/60070 ) Change subject: flashrom.c: Move do_*() helpers into cli_classic.c ...................................................................... flashrom.c: Move do_*() helpers into cli_classic.c These helpers are only used by the CLI logic and so we localise them here to move towards cli_classic being a pure libflashrom user. BUG=b:208132085 TEST=`make` Change-Id: If1112155e2421e0178fd73f847cbb80868387433 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60070 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M flash.h M flashrom.c 3 files changed, 97 insertions(+), 101 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 3cd78d4..8c337eb 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -154,6 +154,103 @@ return 0; } +static int do_read(struct flashctx *const flash, const char *const filename) +{ + int ret; + + unsigned long size = flash->chip->total_size * 1024; + unsigned char *buf = calloc(size, sizeof(unsigned char)); + if (!buf) { + msg_gerr("Memory allocation failed!\n"); + return 1; + } + + ret = flashrom_image_read(flash, buf, size); + if (ret > 0) + goto free_out; + + if (write_buf_to_include_args(flash, buf)) { + ret = 1; + goto free_out; + } + if (filename) + ret = write_buf_to_file(buf, size, filename); + +free_out: + free(buf); + return ret; +} + +static int do_extract(struct flashctx *const flash) +{ + prepare_layout_for_extraction(flash); + return do_read(flash, NULL); +} + +static int do_write(struct flashctx *const flash, const char *const filename, const char *const referencefile) +{ + const size_t flash_size = flash->chip->total_size * 1024; + int ret = 1; + + uint8_t *const newcontents = malloc(flash_size); + uint8_t *const refcontents = referencefile ? malloc(flash_size) : NULL; + + if (!newcontents || (referencefile && !refcontents)) { + msg_gerr("Out of memory!\n"); + goto _free_ret; + } + + /* Read '-w' argument first... */ + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + /* + * ... then update newcontents with contents from files provided to '-i' + * args if needed. + */ + if (read_buf_from_include_args(flash, newcontents)) + goto _free_ret; + + if (referencefile) { + if (read_buf_from_file(refcontents, flash_size, referencefile)) + goto _free_ret; + } + + ret = flashrom_image_write(flash, newcontents, flash_size, refcontents); + +_free_ret: + free(refcontents); + free(newcontents); + return ret; +} + +static int do_verify(struct flashctx *const flash, const char *const filename) +{ + const size_t flash_size = flash->chip->total_size * 1024; + int ret = 1; + + uint8_t *const newcontents = malloc(flash_size); + if (!newcontents) { + msg_gerr("Out of memory!\n"); + goto _free_ret; + } + + /* Read '-v' argument first... */ + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + /* + * ... then update newcontents with contents from files provided to '-i' + * args if needed. + */ + if (read_buf_from_include_args(flash, newcontents)) + goto _free_ret; + + ret = flashrom_image_verify(flash, newcontents, flash_size); + +_free_ret: + free(newcontents); + return ret; +} + int main(int argc, char *argv[]) { const struct flashchip *chip = NULL; diff --git a/flash.h b/flash.h index 932419e..09c1c85 100644 --- a/flash.h +++ b/flash.h @@ -356,10 +356,6 @@ int write_buf_to_include_args(const struct flashctx *const flash, unsigned char *buf); int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); void finalize_flash_access(struct flashctx *); -int do_read(struct flashctx *, const char *filename); -int do_extract(struct flashctx *); -int do_write(struct flashctx *, const char *const filename, const char *const referencefile); -int do_verify(struct flashctx *, const char *const filename); int register_chip_restore(chip_restore_fn_cb_t func, struct flashctx *flash, uint8_t status); /* Something happened that shouldn't happen, but we can go on. */ diff --git a/flashrom.c b/flashrom.c index 8287c31..ac61259 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2189,100 +2189,3 @@ } /** @} */ /* end flashrom-ops */ - -int do_read(struct flashctx *const flash, const char *const filename) -{ - int ret; - - unsigned long size = flash->chip->total_size * 1024; - unsigned char *buf = calloc(size, sizeof(unsigned char)); - if (!buf) { - msg_gerr("Memory allocation failed!\n"); - return 1; - } - - ret = flashrom_image_read(flash, buf, size); - if (ret > 0) - goto free_out; - - if (write_buf_to_include_args(flash, buf)) { - ret = 1; - goto free_out; - } - if (filename) - ret = write_buf_to_file(buf, size, filename); - -free_out: - free(buf); - return ret; -} - -int do_extract(struct flashctx *const flash) -{ - prepare_layout_for_extraction(flash); - return do_read(flash, NULL); -} - -int do_write(struct flashctx *const flash, const char *const filename, const char *const referencefile) -{ - const size_t flash_size = flash->chip->total_size * 1024; - int ret = 1; - - uint8_t *const newcontents = malloc(flash_size); - uint8_t *const refcontents = referencefile ? malloc(flash_size) : NULL; - - if (!newcontents || (referencefile && !refcontents)) { - msg_gerr("Out of memory!\n"); - goto _free_ret; - } - - /* Read '-w' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; - /* - * ... then update newcontents with contents from files provided to '-i' - * args if needed. - */ - if (read_buf_from_include_args(flash, newcontents)) - goto _free_ret; - - if (referencefile) { - if (read_buf_from_file(refcontents, flash_size, referencefile)) - goto _free_ret; - } - - ret = flashrom_image_write(flash, newcontents, flash_size, refcontents); - -_free_ret: - free(refcontents); - free(newcontents); - return ret; -} - -int do_verify(struct flashctx *const flash, const char *const filename) -{ - const size_t flash_size = flash->chip->total_size * 1024; - int ret = 1; - - uint8_t *const newcontents = malloc(flash_size); - if (!newcontents) { - msg_gerr("Out of memory!\n"); - goto _free_ret; - } - - /* Read '-v' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; - /* - * ... then update newcontents with contents from files provided to '-i' - * args if needed. - */ - if (read_buf_from_include_args(flash, newcontents)) - goto _free_ret; - - ret = flashrom_image_verify(flash, newcontents, flash_size); - -_free_ret: - free(newcontents); - return ret; -} -- To view, visit https://review.coreboot.org/c/flashrom/+/60070 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If1112155e2421e0178fd73f847cbb80868387433 Gerrit-Change-Number: 60070 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: flashrom.c: Make {read,write}_buf_from_include_args() public
by Edward O'Callaghan (Code Review) 26 Jan '22

26 Jan '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/60069 ) Change subject: flashrom.c: Make {read,write}_buf_from_include_args() public ...................................................................... flashrom.c: Make {read,write}_buf_from_include_args() public This allows for moving all the do_*() helper functions use for the cli from flashrom.c within static local functions in cli_classic.c BUG=b:208132085 TEST=`make` Change-Id: Ia0abec655a682ca449d0e8ba620886a2d616b86d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60069 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flash.h M flashrom.c 2 files changed, 4 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flash.h b/flash.h index 11e6389..932419e 100644 --- a/flash.h +++ b/flash.h @@ -351,7 +351,9 @@ void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename); +int read_buf_from_include_args(const struct flashctx *const flash, unsigned char *buf); int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename); +int write_buf_to_include_args(const struct flashctx *const flash, unsigned char *buf); int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); void finalize_flash_access(struct flashctx *); int do_read(struct flashctx *, const char *filename); diff --git a/flashrom.c b/flashrom.c index 257112f..8287c31 100644 --- a/flashrom.c +++ b/flashrom.c @@ -931,7 +931,7 @@ * @param buf Chip-sized buffer to write data to * @return 0 on success */ -static int read_buf_from_include_args(const struct flashctx *const flash, +int read_buf_from_include_args(const struct flashctx *const flash, unsigned char *buf) { const struct flashrom_layout *const layout = get_layout(flash); @@ -1028,7 +1028,7 @@ * @param buf Chip-sized buffer to read data from * @return 0 on success */ -static int write_buf_to_include_args(const struct flashctx *const flash, +int write_buf_to_include_args(const struct flashctx *const flash, unsigned char *buf) { const struct flashrom_layout *const layout = get_layout(flash); -- To view, visit https://review.coreboot.org/c/flashrom/+/60069 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia0abec655a682ca449d0e8ba620886a2d616b86d Gerrit-Change-Number: 60069 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 26 Jan '22

26 Jan '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Nicholas Chin. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: > Now that this is tested I've found a ThinkPad T60 in one of my […] My memory tries to tell me something... back in 2012 we were first trying coreboot on a spare T60 with broken display. This might be the very first computer I've ever used flashrom on *and* the first I've flashed coreboot to!!! ^^ -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 23:43:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 26 Jan '22

26 Jan '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Nicholas Chin. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 7: Code-Review+1 (2 comments) Patchset: PS7: Now that this is tested I've found a ThinkPad T60 in one of my laptop piles /o\ didn't try to boot it up yet, and I have no idea whose it is oO File ichspi.c: https://review.coreboot.org/c/flashrom/+/58735/comment/fb5f3205_2049dfa2 PS7, Line 1771: ich_generation = ich_gen; : ich_spibar = spibar; These two are globals and also affect the ICH7 paths. I guess we missed them because they are not used for ICH7 in the original init function but only later during runtime. I've also checked all other writes to globals on the common paths of the original init function. There is only `swseq_data`, which is not used for ICH7 (although I noticed that it could be used to avoid code duplication in some cases, but that's for another day). -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 23:36:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Neill Corlett (Code Review) 26 Jan '22

26 Jan '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Neill Corlett has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 2: (3 comments) Patchset: PS1: > Ah, hmmm, I'm not sure but I think there are SMBus controller drivers that could > still advertise I2C_FUNC_I2C. Can you please confirm that this is not the case. > Or can you tell us what controller is used specifically? > > Also, is the chip aware if it is connected over SMBus or I2C, e.g. by a pin strap? I will research this and get back to you. If necessary, we can remove the SMBus code path from mainline, if that would simplify things. Only very early prototype hardware needs it. > Who is PanVision in this story? They are a vendor who has provided us with the scaler firmware running on the MST chip. > Also, where do you use these chips, integrated display or something external? > If you used SMBus, is it even on the mainboard? This is the scaler chip for the integrated display on the Google Meet Series One Desk 27. It's attached to I2C on the main x86_64 application CPU. File mediatek_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/61288/comment/dbcfc9df_5a9edf76 PS1, Line 96: args.data->block > I would use `args.data->block + 1` when targeting an array and […] Inclined to leave as-is unless there are strong objections. https://review.coreboot.org/c/flashrom/+/61288/comment/1b0b2ff1_d36d7578 PS1, Line 249: 0x426, 7 > Hmmm, I see. […] My understanding is that this is the usual, expected, line that's supposed to go to WP#. In theory it could be wired up another way. -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 2 Gerrit-Owner: Neill Corlett <corlett(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> 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-Comment-Date: Wed, 26 Jan 2022 20:33:38 +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: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: SFDP: make mandatory table length check work with newer SFDP revisions
by Nico Huber (Code Review) 26 Jan '22

26 Jan '22

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/60055 ) Change subject: SFDP: make mandatory table length check work with newer SFDP revisions ...................................................................... SFDP: make mandatory table length check work with newer SFDP revisions The JEDEC SFDP specification JESD216A (1.5) adds five new DWORDs to the Basic Flash Parameter Table. Later versions of the spec add even more fields. This increases the table being read from 36 bytes to currently 64 bytes and makes flashrom bail out for any SFDP version >= 1.5 due to a static table length check. This was discovered on a GigaDevice GD25B127DSIGR from 2021 with SFDP revision 1.6, while another flash of the same model from 2020 with SFDP revision 1.0 was detected fine by flashrom. GD25B127DSIGR - 2020 version: Probing for Unknown SFDP-capable chip, 0 kB: SFDP revision = 1.0 SFDP number of parameter headers is 2 (NPH = 1). SFDP parameter table header 0/1: ID 0x00, version 1.0 Length 36 B, Parameter Table Pointer 0x000030 GD25B127DSIGR - 2021 version: Probing for Unknown SFDP-capable chip, 0 kB: SFDP revision = 1.6 SFDP number of parameter headers is 2 (NPH = 1). SFDP parameter table header 0/1: ID 0x00, version 1.6 Length 64 B, Parameter Table Pointer 0x000030 ... Length of the mandatory JEDEC SFDP parameter table is wrong (64 B), skipping it. The specification says that changes of the minor SFDP revision will maintain compatibility. Thus, simply check for the minimal required table length, which is 16 bytes for legacy Intel pre-SFDP and 36 bytes for SFDP. Change-Id: Id84cde4ebc805d68e2984e8041fbc48d7ceebe34 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60055 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M sfdp.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/sfdp.c b/sfdp.c index 85e698d..b549417 100644 --- a/sfdp.c +++ b/sfdp.c @@ -369,7 +369,7 @@ msg_cdbg("The chip contains an unknown " "version of the JEDEC flash " "parameters table, skipping it.\n"); - } else if (len != 9 * 4 && len != 4 * 4) { + } else if (len != 4 * 4 && len < 9 * 4) { msg_cdbg("Length of the mandatory JEDEC SFDP " "parameter table is wrong (%d B), " "skipping it.\n", len); -- To view, visit https://review.coreboot.org/c/flashrom/+/60055 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id84cde4ebc805d68e2984e8041fbc48d7ceebe34 Gerrit-Change-Number: 60055 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <edward.ocallaghan(a)koparo.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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-MessageType: merged

1 0

Change in flashrom[master]: sfdp: drop redundant check of the mandatory table size
by Nico Huber (Code Review) 26 Jan '22

26 Jan '22

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/61379 ) Change subject: sfdp: drop redundant check of the mandatory table size ...................................................................... sfdp: drop redundant check of the mandatory table size Change-Id: I464856612a6d21c682f1d9ad5110fa11a0a276c2 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/61379 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M sfdp.c 1 file changed, 0 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/sfdp.c b/sfdp.c index 530588f..85e698d 100644 --- a/sfdp.c +++ b/sfdp.c @@ -131,10 +131,6 @@ int j; msg_cdbg("Parsing JEDEC flash parameter table... "); - if (len != 9 * 4 && len != 4 * 4) { - msg_cdbg("%s: len out of spec\n", __func__); - return 1; - } msg_cdbg2("\n"); /* 1. double word */ -- To view, visit https://review.coreboot.org/c/flashrom/+/61379 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I464856612a6d21c682f1d9ad5110fa11a0a276c2 Gerrit-Change-Number: 61379 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Nico Huber (Code Review) 26 Jan '22

26 Jan '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Neill Corlett. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > - We had two different board designs, one of which connects the TSUM* to SMBus and the other to raw I2C. This driver is tested on both. The finished product will be using I2C. I've left the SMBus code in as a convenience so we can continue to flash our earlier prototypes. Ah, hmmm, I'm not sure but I think there are SMBus controller drivers that could still advertise I2C_FUNC_I2C. Can you please confirm that this is not the case. Or can you tell us what controller is used specifically? Also, is the chip aware if it is connected over SMBus or I2C, e.g. by a pin strap? > - GPIO numbers are specific to TSUMOP82JUQ. My understanding is that these particular GPIO lines, for that chip, are standard for connecting to SPI WP#. Documentation that we received from PanVision just says to poke them and doesn't provide further detail. Who is PanVision in this story? Did they acquire the MSTAR chip line and provide the chips? or do they provide the board? or ...? Also, where do you use these chips, integrated display or something external? If you used SMBus, is it even on the mainboard? File mediatek_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/61288/comment/e2d50ac8_de21b96a PS1, Line 96: args.data->block > I imagine Edward would like to avoid pointer arithmetics, substituting `args. […] I would use `args.data->block + 1` when targeting an array and `&args.data->block[1]` only if I target a single member. -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 2 Gerrit-Owner: Neill Corlett <corlett(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Neill Corlett <corlett(a)google.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 15:18:17 +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: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c Invert the meaning of 'dual_output' bit
by Angel Pons (Code Review) 26 Jan '22

26 Jan '22

Attention is currently required from: Nico Huber, Werner Zeh. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61362 ) Change subject: ich_descriptors.c Invert the meaning of 'dual_output' bit ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f Gerrit-Change-Number: 61362 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 13:19:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add mediatek_i2c_spi interface
by Angel Pons (Code Review) 26 Jan '22

26 Jan '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Neill Corlett. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61288 ) Change subject: Add mediatek_i2c_spi interface ...................................................................... Patch Set 2: (3 comments) File mediatek_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/61288/comment/4c5d2b8f_a59c3ac9 PS1, Line 96: args.data->block > This is copying the data out of the i2c_smbus_data, starting at 1, using the size specified by [0]. I imagine Edward would like to avoid pointer arithmetics, substituting `args.data->block + 1` with `&args.data->block[1]`. https://review.coreboot.org/c/flashrom/+/61288/comment/1c39616f_6a30553b PS1, Line 242: GPIO line > This is specific to the revision of chip that our board uses (TSUMOP82JUQ). At the very least, I'd add an option to enable WP# control. https://review.coreboot.org/c/flashrom/+/61288/comment/6b711c64_fa9c87a6 PS1, Line 249: 0x426, 7 > The TSUMOP82JUQ has a GPIO line going to WP# on SPI, and these two registers (0x426 and 0x428) contr […] Hmmm, I see. Could other boards with the same chip use a different GPIO for WP#? -- To view, visit https://review.coreboot.org/c/flashrom/+/61288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24adb14e7b4f7160e1c3ff941774064d5a81e820 Gerrit-Change-Number: 61288 Gerrit-PatchSet: 2 Gerrit-Owner: Neill Corlett <corlett(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Neill Corlett <corlett(a)google.com> Gerrit-Comment-Date: Wed, 26 Jan 2022 13:01:37 +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: Neill Corlett <corlett(a)google.com> Gerrit-MessageType: comment

1 0

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2022