From e5389d1b8fa18932a7aa3be5fee094c5023d1251 Mon Sep 17 00:00:00 2001 From: Nikolai Artemiev Date: Mon, 22 Nov 2021 13:18:49 +1100 Subject: spi25_statusreg: inline spi_write_register_flag() Creating the entire SPI command that should be sent to the chip in spi_write_register() is simpler than splitting it across two functions that have to pass multiple parameters between them. Additionally, having separate spi_write_register_flag() function provided little benefit, as it was only ever called from spi_write_register(). 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: I4996b0848d0ed09032bad2ab13ab1f40bbfc0304 Signed-off-by: Nikolai Artemiev Reviewed-on: https://review.coreboot.org/c/flashrom/+/59528 Reviewed-by: Anastasia Klimchuk Reviewed-by: Edward O'Callaghan Tested-by: build bot (Jenkins) --- spi25_statusreg.c | 126 ++++++++++++++++++++++++------------------------------ 1 file changed, 56 insertions(+), 70 deletions(-) (limited to 'spi25_statusreg.c') diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 31d6c7698..249ab9a3e 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -22,68 +22,6 @@ #include "spi.h" /* === Generic functions === */ -static int spi_write_register_flag(const struct flashctx *flash, uint8_t enable_opcode, uint8_t *write_cmd, size_t write_cmd_len, enum flash_reg reg) -{ - /* - * Enabling register writes requires either EWSR or WREN depending on - * chip type. The code below relies on the fact hat EWSR and WREN have - * the same INSIZE and OUTSIZE. - */ - - struct spi_command cmds[] = { - { - .writecnt = JEDEC_WREN_OUTSIZE, - .writearr = &enable_opcode, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = write_cmd_len, - .writearr = write_cmd, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 0, - .writearr = NULL, - .readcnt = 0, - .readarr = NULL, - }}; - - int result = spi_send_multicommand(flash, cmds); - if (result) { - msg_cerr("%s failed during command execution\n", __func__); - /* - * No point in waiting for the command to complete if execution - * failed. - */ - return result; - } - - /* - * WRSR performs a self-timed erase before the changes take effect. - * This may take 50-85 ms in most cases, and some chips apparently - * allow running RDSR only once. Therefore pick an initial delay of - * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. - * - * Newer chips with multiple status registers (SR2 etc.) are unlikely - * to have problems with multiple RDSR commands, so only wait for the - * initial 100 ms if the register we wrote to was SR1. - */ - int delay_ms = 5000; - if (reg == STATUS1) { - programmer_delay(100 * 1000); - delay_ms -= 100; - } - - for (; delay_ms > 0; delay_ms -= 10) { - if ((spi_read_status_register(flash) & SPI_SR_WIP) == 0) - return 0; - programmer_delay(10 * 1000); - } - - msg_cerr("Error: WIP bit after WRSR never cleared\n"); - return TIMEOUT_ERROR; -} - int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value) { int feature_bits = flash->chip->feature_bits; @@ -133,18 +71,66 @@ int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t return 1; } - if (!(feature_bits & (FEATURE_WRSR_WREN | FEATURE_WRSR_EWSR))) { + uint8_t enable_cmd; + if (feature_bits & FEATURE_WRSR_WREN) { + enable_cmd = JEDEC_WREN; + } else if (feature_bits & FEATURE_WRSR_EWSR) { + enable_cmd = JEDEC_EWSR; + } else { msg_cdbg("Missing status register write definition, assuming " "EWSR is needed\n"); - feature_bits |= FEATURE_WRSR_EWSR; + enable_cmd = JEDEC_EWSR; + } + + struct spi_command cmds[] = { + { + .writecnt = JEDEC_WREN_OUTSIZE, + .writearr = &enable_cmd, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = write_cmd_len, + .writearr = write_cmd, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .writearr = NULL, + .readcnt = 0, + .readarr = NULL, + }}; + + int result = spi_send_multicommand(flash, cmds); + if (result) { + msg_cerr("%s failed during command execution\n", __func__); + return result; } - int ret = 1; - if (feature_bits & FEATURE_WRSR_WREN) - ret = spi_write_register_flag(flash, JEDEC_WREN, write_cmd, write_cmd_len, reg); - if (ret && (feature_bits & FEATURE_WRSR_EWSR)) - ret = spi_write_register_flag(flash, JEDEC_EWSR, write_cmd, write_cmd_len, reg); - return ret; + /* + * WRSR performs a self-timed erase before the changes take effect. + * This may take 50-85 ms in most cases, and some chips apparently + * allow running RDSR only once. Therefore pick an initial delay of + * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. + * + * Newer chips with multiple status registers (SR2 etc.) are unlikely + * to have problems with multiple RDSR commands, so only wait for the + * initial 100 ms if the register we wrote to was SR1. + */ + int delay_ms = 5000; + if (reg == STATUS1) { + programmer_delay(100 * 1000); + delay_ms -= 100; + } + + for (; delay_ms > 0; delay_ms -= 10) { + if ((spi_read_status_register(flash) & SPI_SR_WIP) == 0) + return 0; + programmer_delay(10 * 1000); + } + + + msg_cerr("Error: WIP bit after WRSR never cleared\n"); + return TIMEOUT_ERROR; } int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value) -- cgit v1.2.3