Shiyu Sun has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41779 ) Change subject: lspcon_i2c_spi.c: Add debug message that shows the active block. ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/41779/1/lspcon_i2c_spi.c File lspcon_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/41779/1/lspcon_i2c_spi.c@30 PS1, Line 30: #define ACTIVE_BLOCK_ADDRESS (0x9a >> 1) will fix this indentation -- To view, visit https://review.coreboot.org/c/flashrom/+/41779 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b3022542f98f114097c6f39254ef4c2cf188c90 Gerrit-Change-Number: 41779 Gerrit-PatchSet: 1 Gerrit-Owner: Shiyu Sun <sshiyu@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Wed, 27 May 2020 13:33:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/41779 to look at the new patch set (#2). Change subject: lspcon_i2c_spi.c: Add debug message that shows the active block ...................................................................... lspcon_i2c_spi.c: Add debug message that shows the active block This is all for debug propose to verify A/B update work as expected. BUG=b:148746232 BRANCH=none TEST=flashrom -p lspcon_i2c_spi:bus=7 --flash-size -V verified the related message appears. Signed-off-by: Shiyu Sun <sshiyu@chromium.org> Change-Id: I1b3022542f98f114097c6f39254ef4c2cf188c90 --- M lspcon_i2c_spi.c 1 file changed, 25 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/41779/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/41779 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b3022542f98f114097c6f39254ef4c2cf188c90 Gerrit-Change-Number: 41779 Gerrit-PatchSet: 2 Gerrit-Owner: Shiyu Sun <sshiyu@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41779 ) Change subject: lspcon_i2c_spi.c: Add debug message that shows the active block ...................................................................... Patch Set 2: (7 comments) https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c File lspcon_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c@476 PS2, Line 476: static void lspcon_i2c_spi_boot_block_info(int fd) move this function after `lspcon_i2c_spi_map_page(int fd, unsigned int offset)` https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c@480 PS2, Line 480: ret |= lspcon_i2c_spi_write_register(fd, 0xbc, 0xc0); : ret |= lspcon_i2c_spi_write_register(fd, 0xbc, 0x40); The first two magics programmed here are equiv to `lspcon_i2c_spi_reset_mpu_stop(int fd) {}` and so not needed. https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c@482 PS2, Line 482: ret |= lspcon_i2c_spi_write_register(fd, 0x8e, 0x00); : ret |= lspcon_i2c_spi_write_register(fd, 0x8f, 0x00); Put a comment above, `/* Set rom addr to beginning. */` and, ``` #define ROMADDR_BYTE1 0x8e #define ROMADDR_BYTE2 0x8f ``` so you can just use `lspcon_i2c_spi_map_page(int fd, unsigned int offset)` https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c@484 PS2, Line 484: struct timespec wait_10ms = { 0, (unsigned)1e7 }; I suspect this wait is for the unnecessary MPU reset above, so you can get rid of it here. https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c@487 PS2, Line 487: 0x0e Perhaps this should be a #define or have a inline comment /* .. */ that has the name of what the vendor calls this special command magic. https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c@490 PS2, Line 490: { {} unnecessary for single lines. https://review.coreboot.org/c/flashrom/+/41779/2/lspcon_i2c_spi.c@491 PS2, Line 491: pdbg("F msg_perr() and use `%s: Failed to..` with __func__. As mentioned before, consider yourself as a user seeing the word "Error/Failed" in a log - what are you going to grep for? -- To view, visit https://review.coreboot.org/c/flashrom/+/41779 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b3022542f98f114097c6f39254ef4c2cf188c90 Gerrit-Change-Number: 41779 Gerrit-PatchSet: 2 Gerrit-Owner: Shiyu Sun <sshiyu@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz@google.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Fri, 29 May 2020 08:31:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41779 ) Change subject: lspcon_i2c_spi.c: Add debug message that shows the active block ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/flashrom/+/41779/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/41779/2//COMMIT_MSG@9 PS2, Line 9: work works -- To view, visit https://review.coreboot.org/c/flashrom/+/41779 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b3022542f98f114097c6f39254ef4c2cf188c90 Gerrit-Change-Number: 41779 Gerrit-PatchSet: 2 Gerrit-Owner: Shiyu Sun <sshiyu@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz@google.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Fri, 29 May 2020 12:39:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/41779?usp=email ) Change subject: lspcon_i2c_spi.c: Add debug message that shows the active block ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/flashrom/+/41779?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1b3022542f98f114097c6f39254ef4c2cf188c90 Gerrit-Change-Number: 41779 Gerrit-PatchSet: 2 Gerrit-Owner: Shiyu Sun <sshiyu@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz@google.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org>