Sam McNally 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: (2 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@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. */` […] Why is this being done?
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 /* .. […] It's just the register being read. There probably should be more generic i2c register read/write functions that don't hard-code the i2c address for cases like this.
-- 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:59:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment