From 005d32b7b781c355f22d04ceb532c4fa4656801a Mon Sep 17 00:00:00 2001 From: Nikolai Artemiev Date: Thu, 28 Oct 2021 16:18:28 +1100 Subject: spi25_statusreg: delete spi_read_status_register() Delete the spi_read_status_register() function because the generic spi_read_register() function can be used instead. This patch also converts all call sites over to spi_read_register(). A side effect is that error codes are now properly propagated and checked. BUG=b:195381327,b:153800563 BRANCH=none TEST=flashrom -{r,w,E} TEST=Tested with a W25Q128.W flash on a kasumi (AMD) dut. Read SR1/SR2 with --wp-status and activated various WP ranges that toggled bits in both SR1 and SR2. Change-Id: I146b4b5439872e66c5d33e156451a729d248c7da Signed-off-by: Nikolai Artemiev Reviewed-on: https://review.coreboot.org/c/flashrom/+/59529 Tested-by: build bot (Jenkins) Reviewed-by: Edward O'Callaghan Reviewed-by: Anastasia Klimchuk --- chipdrivers.h | 2 - it87spi.c | 17 ++++++- s25f.c | 10 ++-- spi25.c | 13 +++-- spi25_statusreg.c | 145 +++++++++++++++++++++++++++++++++++++++++------------- 5 files changed, 141 insertions(+), 46 deletions(-) diff --git a/chipdrivers.h b/chipdrivers.h index c223534a..0695993a 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -62,8 +62,6 @@ int spi_set_extended_address(struct flashctx *, uint8_t addr_high); /* spi25_statusreg.c */ -/* FIXME: replace spi_read_status_register() calls with spi_read_register() */ -uint8_t spi_read_status_register(const struct flashctx *flash); int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value); int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value); void spi_prettyprint_status_register_bit(uint8_t status, int bit); diff --git a/it87spi.c b/it87spi.c index 728ec70c..204816af 100644 --- a/it87spi.c +++ b/it87spi.c @@ -133,9 +133,20 @@ static int it8716f_spi_page_program(struct flashctx *flash, const uint8_t *buf, OUTB(0, data->flashport); /* Wait until the Write-In-Progress bit is cleared. * This usually takes 1-10 ms, so wait in 1 ms steps. + * + * FIXME: This should timeout after some number of retries. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (true) { + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + + if((status & SPI_SR_WIP) == 0) + return 0; + programmer_delay(1000); + } return 0; } @@ -277,7 +288,9 @@ static int it8716f_spi_chip_write_256(struct flashctx *flash, const uint8_t *buf } while (len >= chip->page_size) { - it8716f_spi_page_program(flash, buf, start); + int ret = it8716f_spi_page_program(flash, buf, start); + if (ret) + return ret; start += chip->page_size; len -= chip->page_size; buf += chip->page_size; diff --git a/s25f.c b/s25f.c index 0bd4a7cb..82ca6106 100644 --- a/s25f.c +++ b/s25f.c @@ -133,9 +133,14 @@ static int s25fs_software_reset(struct flashctx *flash) static int s25f_poll_status(const struct flashctx *flash) { - uint8_t tmp = spi_read_status_register(flash); + while (true) { + uint8_t tmp; + if (spi_read_register(flash, STATUS1, &tmp)) + return -1; + + if ((tmp & SPI_SR_WIP) == 0) + break; - while (tmp & SPI_SR_WIP) { /* * The WIP bit on S25F chips remains set to 1 if erase or * programming errors occur, so we must check for those @@ -156,7 +161,6 @@ static int s25f_poll_status(const struct flashctx *flash) } programmer_delay(1000 * 10); - tmp = spi_read_status_register(flash); } return 0; diff --git a/spi25.c b/spi25.c index 213273f2..e96a38db 100644 --- a/spi25.c +++ b/spi25.c @@ -304,12 +304,17 @@ int probe_spi_at25f(struct flashctx *flash) static int spi_poll_wip(struct flashctx *const flash, const unsigned int poll_delay) { - /* FIXME: We can't tell if spi_read_status_register() failed. */ /* FIXME: We don't time out. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (true) { + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + if (!(status & SPI_SR_WIP)) + return 0; + programmer_delay(poll_delay); - /* FIXME: Check the status register for errors. */ - return 0; + } } /** diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 249ab9a3..6b168398 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -123,7 +123,11 @@ int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t } for (; delay_ms > 0; delay_ms -= 10) { - if ((spi_read_status_register(flash) & SPI_SR_WIP) == 0) + uint8_t status; + result = spi_read_register(flash, STATUS1, &status); + if (result) + return result; + if ((status & SPI_SR_WIP) == 0) return 0; programmer_delay(10 * 1000); } @@ -168,14 +172,6 @@ int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t return 0; } -uint8_t spi_read_status_register(const struct flashctx *flash) -{ - uint8_t status = 0; - /* FIXME: We should propagate the error. */ - spi_read_register(flash, STATUS1, &status); - return status; -} - static int spi_restore_status(struct flashctx *flash, uint8_t status) { msg_cdbg("restoring chip status (0x%02x)\n", status); @@ -207,7 +203,10 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m uint8_t status; int result; - status = spi_read_status_register(flash); + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + if ((status & bp_mask) == 0) { msg_cdbg2("Block protection is disabled.\n"); return 0; @@ -229,7 +228,11 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m msg_cerr("Could not write status register 1.\n"); return result; } - status = spi_read_status_register(flash); + + ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + if ((status & lock_mask) != 0) { msg_cerr("Unsetting lock bit(s) failed.\n"); return 1; @@ -242,7 +245,11 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m msg_cerr("Could not write status register 1.\n"); return result; } - status = spi_read_status_register(flash); + + ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + if ((status & bp_mask) != 0) { msg_cerr("Block protection could not be disabled!\n"); if (flash->chip->printlock) @@ -363,7 +370,10 @@ void spi_prettyprint_status_register_bit(uint8_t status, int bit) int spi_prettyprint_status_register_plain(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); return 0; } @@ -371,7 +381,10 @@ int spi_prettyprint_status_register_plain(struct flashctx *flash) /* Print the plain hex value and the welwip bits only. */ int spi_prettyprint_status_register_default_welwip(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_welwip(status); @@ -384,7 +397,10 @@ int spi_prettyprint_status_register_default_welwip(struct flashctx *flash) */ int spi_prettyprint_status_register_bp1_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -402,7 +418,10 @@ int spi_prettyprint_status_register_bp1_srwd(struct flashctx *flash) */ int spi_prettyprint_status_register_bp2_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -419,7 +438,10 @@ int spi_prettyprint_status_register_bp2_srwd(struct flashctx *flash) */ int spi_prettyprint_status_register_bp3_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -431,7 +453,10 @@ int spi_prettyprint_status_register_bp3_srwd(struct flashctx *flash) int spi_prettyprint_status_register_bp4_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -442,7 +467,10 @@ int spi_prettyprint_status_register_bp4_srwd(struct flashctx *flash) int spi_prettyprint_status_register_bp2_bpl(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_bpl(status); @@ -455,7 +483,10 @@ int spi_prettyprint_status_register_bp2_bpl(struct flashctx *flash) int spi_prettyprint_status_register_bp2_tb_bpl(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_bpl(status); @@ -477,7 +508,10 @@ int spi_prettyprint_status_register_bp2_tb_bpl(struct flashctx *flash) int spi_prettyprint_status_register_amic_a25l032(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -533,7 +567,11 @@ static void spi_prettyprint_status_register_atmel_at25_swp(uint8_t status) int spi_prettyprint_status_register_at25df(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -556,8 +594,10 @@ int spi_prettyprint_status_register_at25df_sec(struct flashctx *flash) int spi_prettyprint_status_register_at25f(struct flashctx *flash) { uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; - status = spi_read_status_register(flash); spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -572,8 +612,10 @@ int spi_prettyprint_status_register_at25f(struct flashctx *flash) int spi_prettyprint_status_register_at25f512a(struct flashctx *flash) { uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; - status = spi_read_status_register(flash); spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -588,7 +630,10 @@ int spi_prettyprint_status_register_at25f512a(struct flashctx *flash) int spi_prettyprint_status_register_at25f512b(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -604,7 +649,10 @@ int spi_prettyprint_status_register_at25f4096(struct flashctx *flash) { uint8_t status; - status = spi_read_status_register(flash); + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -617,7 +665,10 @@ int spi_prettyprint_status_register_at25f4096(struct flashctx *flash) int spi_prettyprint_status_register_at25fs010(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -637,7 +688,10 @@ int spi_prettyprint_status_register_at25fs010(struct flashctx *flash) int spi_prettyprint_status_register_at25fs040(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -649,7 +703,10 @@ int spi_prettyprint_status_register_at25fs040(struct flashctx *flash) int spi_prettyprint_status_register_at26df081a(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -707,7 +764,10 @@ int spi_disable_blockprotect_at25fs040(struct flashctx *flash) int spi_prettyprint_status_register_en25s_wp(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -726,7 +786,10 @@ int spi_disable_blockprotect_n25q(struct flashctx *flash) int spi_prettyprint_status_register_n25q(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -751,7 +814,10 @@ int spi_disable_blockprotect_bp2_ep_srwd(struct flashctx *flash) /* Used by Intel/Numonyx S33 and Spansion S25FL-S chips */ int spi_prettyprint_status_register_bp2_ep_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -779,7 +845,10 @@ static void spi_prettyprint_status_register_sst25_common(uint8_t status) int spi_prettyprint_status_register_sst25(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_sst25_common(status); return 0; } @@ -795,7 +864,10 @@ int spi_prettyprint_status_register_sst25vf016(struct flashctx *flash) "100000H-1FFFFFH", "all", "all" }; - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_sst25_common(status); msg_cdbg("Resulting block protection : %s\n", bpt[(status & 0x1c) >> 2]); return 0; @@ -810,7 +882,10 @@ int spi_prettyprint_status_register_sst25vf040b(struct flashctx *flash) "0x40000-0x7ffff", "all blocks", "all blocks", "all blocks", "all blocks" }; - uint8_t status = spi_read_status_register(flash); + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; spi_prettyprint_status_register_sst25_common(status); msg_cdbg("Resulting block protection : %s\n", bpt[(status & 0x1c) >> 2]); return 0; -- cgit v1.2.3