Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/48975 )
Change subject: realtek_mst_i2c_spi.c: Consolidate shifts to the one fn ......................................................................
realtek_mst_i2c_spi.c: Consolidate shifts to the one fn
To avoid further incorrect mappings ensure all the shifting happens within realtek_mst_i2c_spi_map_page() itself.
BUG=none BRANCH=none TEST=builds
Change-Id: I96c595b1abae044347fb0c2c91b891a60dd3675e Signed-off-by: Edward O'Callaghan quasisec@google.com Suggested-by: Angel Pons th3fanbus@gmail.com --- M realtek_mst_i2c_spi.c 1 file changed, 8 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/48975/1
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 3e84b23..418e23a 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -289,9 +289,14 @@ return ret; }
-static int realtek_mst_i2c_spi_map_page(int fd, uint8_t block_idx, uint8_t page_idx, uint8_t byte_idx) +static int realtek_mst_i2c_spi_map_page(int fd, uint32_t addr) { int ret = 0; + + uint8_t block_idx = (addr >> 16) & 0xff; + uint8_t page_idx = (addr >> 8) & 0xff; + uint8_t byte_idx = addr & 0xff; + ret |= realtek_mst_i2c_spi_write_register(fd, MAP_PAGE_BYTE2, block_idx); ret |= realtek_mst_i2c_spi_write_register(fd, MAP_PAGE_BYTE1, page_idx); ret |= realtek_mst_i2c_spi_write_register(fd, MAP_PAGE_BYTE0, byte_idx); @@ -330,10 +335,7 @@ start--; ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x46); // ** ret |= realtek_mst_i2c_spi_write_register(fd, 0x61, OPCODE_READ); - uint8_t block_idx = (start >> 16) & 0xff; - uint8_t page_idx = (start >> 8) & 0xff; - uint8_t byte_idx = start & 0xff; - ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); + ret |= realtek_mst_i2c_spi_map_page(fd, start); ret |= realtek_mst_i2c_spi_write_register(fd, 0x6a, 0x03); ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x47); // ** if (ret) @@ -384,10 +386,7 @@ uint16_t page_len = min(len - i, PAGE_SIZE); if (len - i < PAGE_SIZE) ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, page_len-1); - uint8_t block_idx = ((start + i) >> 16) & 0xff; - uint8_t page_idx = ((start + i) >> 8) & 0xff; - uint8_t byte_idx = (start + i) & 0xff; - ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); + ret |= realtek_mst_i2c_spi_map_page(fd, (start + i)); if (ret) break;
Shiyu Sun has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48975 )
Change subject: realtek_mst_i2c_spi.c: Consolidate shifts to the one fn ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48975 )
Change subject: realtek_mst_i2c_spi.c: Consolidate shifts to the one fn ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/flashrom/+/48975/1/realtek_mst_i2c_spi.c File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/48975/1/realtek_mst_i2c_spi.c@389 PS1, Line 389: (start + i) nit: braces aren't necessary anymore
Hello build bot (Jenkins), Shiyu Sun, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48975
to look at the new patch set (#2).
Change subject: realtek_mst_i2c_spi.c: Consolidate shifts to the one fn ......................................................................
realtek_mst_i2c_spi.c: Consolidate shifts to the one fn
To avoid further incorrect mappings ensure all the shifting happens within realtek_mst_i2c_spi_map_page() itself.
BUG=none BRANCH=none TEST=builds
Change-Id: I96c595b1abae044347fb0c2c91b891a60dd3675e Signed-off-by: Edward O'Callaghan quasisec@google.com Suggested-by: Angel Pons th3fanbus@gmail.com --- M realtek_mst_i2c_spi.c 1 file changed, 8 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/48975/2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48975 )
Change subject: realtek_mst_i2c_spi.c: Consolidate shifts to the one fn ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/flashrom/+/48975/1/realtek_mst_i2c_spi.c File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/48975/1/realtek_mst_i2c_spi.c@389 PS1, Line 389: (start + i)
nit: braces aren't necessary anymore
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48975 )
Change subject: realtek_mst_i2c_spi.c: Consolidate shifts to the one fn ......................................................................
Patch Set 2: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/48975 )
Change subject: realtek_mst_i2c_spi.c: Consolidate shifts to the one fn ......................................................................
realtek_mst_i2c_spi.c: Consolidate shifts to the one fn
To avoid further incorrect mappings ensure all the shifting happens within realtek_mst_i2c_spi_map_page() itself.
BUG=none BRANCH=none TEST=builds
Change-Id: I96c595b1abae044347fb0c2c91b891a60dd3675e Signed-off-by: Edward O'Callaghan quasisec@google.com Suggested-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/48975 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M realtek_mst_i2c_spi.c 1 file changed, 8 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 3e84b23..a0cd7c6 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -289,9 +289,14 @@ return ret; }
-static int realtek_mst_i2c_spi_map_page(int fd, uint8_t block_idx, uint8_t page_idx, uint8_t byte_idx) +static int realtek_mst_i2c_spi_map_page(int fd, uint32_t addr) { int ret = 0; + + uint8_t block_idx = (addr >> 16) & 0xff; + uint8_t page_idx = (addr >> 8) & 0xff; + uint8_t byte_idx = addr & 0xff; + ret |= realtek_mst_i2c_spi_write_register(fd, MAP_PAGE_BYTE2, block_idx); ret |= realtek_mst_i2c_spi_write_register(fd, MAP_PAGE_BYTE1, page_idx); ret |= realtek_mst_i2c_spi_write_register(fd, MAP_PAGE_BYTE0, byte_idx); @@ -330,10 +335,7 @@ start--; ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x46); // ** ret |= realtek_mst_i2c_spi_write_register(fd, 0x61, OPCODE_READ); - uint8_t block_idx = (start >> 16) & 0xff; - uint8_t page_idx = (start >> 8) & 0xff; - uint8_t byte_idx = start & 0xff; - ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); + ret |= realtek_mst_i2c_spi_map_page(fd, start); ret |= realtek_mst_i2c_spi_write_register(fd, 0x6a, 0x03); ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x47); // ** if (ret) @@ -384,10 +386,7 @@ uint16_t page_len = min(len - i, PAGE_SIZE); if (len - i < PAGE_SIZE) ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, page_len-1); - uint8_t block_idx = ((start + i) >> 16) & 0xff; - uint8_t page_idx = ((start + i) >> 8) & 0xff; - uint8_t byte_idx = (start + i) & 0xff; - ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); + ret |= realtek_mst_i2c_spi_map_page(fd, start + i); if (ret) break;
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Shiyu Sun (Code Review)