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?