diff --git a/chipdrivers.h b/chipdrivers.h index e1d6aa9e7..ea8d480d5 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -62,8 +62,10 @@ 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_write_status_register(const struct flashctx *flash, int status); +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); int spi_prettyprint_status_register_plain(struct flashctx *flash); int spi_prettyprint_status_register_default_welwip(struct flashctx *flash); diff --git a/flash.h b/flash.h index 654cdee6d..f1a8b340e 100644 --- a/flash.h +++ b/flash.h @@ -166,6 +166,13 @@ struct flashrom_flashctx; #define flashctx flashrom_flashctx /* TODO: Agree on a name and convert all occurences. */ typedef int (erasefunc_t)(struct flashctx *flash, unsigned int addr, unsigned int blocklen); +enum flash_reg { + INVALID_REG = 0, + STATUS1, + STATUS2, + MAX_REGISTERS +}; + struct flashchip { const char *vendor; const char *name; diff --git a/spi25_statusreg.c b/spi25_statusreg.c index a0b0fcf5e..0d7bc25b9 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -22,24 +22,23 @@ #include "spi.h" /* === Generic functions === */ -static int spi_write_status_register_flag(const struct flashctx *flash, int status, const unsigned char enable_opcode) +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) { - int result; - int i = 0; /* - * WRSR 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. + * 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 = (const unsigned char[]){ enable_opcode }, + .writearr = &enable_opcode, .readcnt = 0, .readarr = NULL, }, { - .writecnt = JEDEC_WRSR_OUTSIZE, - .writearr = (const unsigned char[]){ JEDEC_WRSR, (unsigned char) status }, + .writecnt = write_cmd_len, + .writearr = write_cmd, .readcnt = 0, .readarr = NULL, }, { @@ -49,69 +48,117 @@ static int spi_write_status_register_flag(const struct flashctx *flash, int stat .readarr = NULL, }}; - result = spi_send_multicommand(flash, cmds); + 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 + /* + * 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. + + /* + * 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. */ - programmer_delay(100 * 1000); - while (spi_read_status_register(flash) & SPI_SR_WIP) { - if (++i > 490) { - msg_cerr("Error: WIP bit after WRSR never cleared\n"); - return TIMEOUT_ERROR; - } + 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); } - return 0; + + msg_cerr("Error: WIP bit after WRSR never cleared\n"); + return TIMEOUT_ERROR; } -int spi_write_status_register(const struct flashctx *flash, int status) +int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value) { int feature_bits = flash->chip->feature_bits; - int ret = 1; + + uint8_t write_cmd[3]; + size_t write_cmd_len = 0; + + /* + * Create SPI write command sequence based on the destination register + * and the chip's supported command set. + */ + switch (reg) { + case STATUS1: + write_cmd[0] = JEDEC_WRSR; + write_cmd[1] = value; + write_cmd_len = JEDEC_WRSR_OUTSIZE; + break; + default: + msg_cerr("Cannot write register: unknown register\n"); + return 1; + } if (!(feature_bits & (FEATURE_WRSR_WREN | FEATURE_WRSR_EWSR))) { msg_cdbg("Missing status register write definition, assuming " "EWSR is needed\n"); feature_bits |= FEATURE_WRSR_EWSR; } + + int ret = 1; if (feature_bits & FEATURE_WRSR_WREN) - ret = spi_write_status_register_flag(flash, status, JEDEC_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_status_register_flag(flash, status, JEDEC_EWSR); + ret = spi_write_register_flag(flash, JEDEC_EWSR, write_cmd, write_cmd_len, reg); return ret; } +int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value) +{ + uint8_t read_cmd; + + switch (reg) { + case STATUS1: + read_cmd = JEDEC_RDSR; + break; + default: + msg_cerr("Cannot read register: unknown register\n"); + return 1; + } + + /* FIXME: No workarounds for driver/hardware bugs in generic code. */ + /* JEDEC_RDSR_INSIZE=1 but wbsio needs 2 */ + uint8_t readarr[2]; + + int ret = spi_send_command(flash, sizeof(read_cmd), sizeof(readarr), &read_cmd, readarr); + if (ret) { + msg_cerr("Register read failed!\n"); + return ret; + } + + *value = readarr[0]; + return 0; +} + uint8_t spi_read_status_register(const struct flashctx *flash) { - static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR }; - /* FIXME: No workarounds for driver/hardware bugs in generic code. */ - unsigned char readarr[2]; /* JEDEC_RDSR_INSIZE=1 but wbsio needs 2 */ - int ret; - - /* Read Status Register */ - ret = spi_send_command(flash, sizeof(cmd), sizeof(readarr), cmd, readarr); - if (ret) { - msg_cerr("RDSR failed!\n"); - /* FIXME: We should propagate the error. */ - return 0; - } - - return readarr[0]; + 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); - return spi_write_status_register(flash, status); + return spi_write_register(flash, STATUS1, status); } /* A generic block protection disable. @@ -156,9 +203,9 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m return 1; } /* All bits except the register lock bit (often called SPRL, SRWD, WPEN) are readonly. */ - result = spi_write_status_register(flash, status & ~lock_mask); + result = spi_write_register(flash, STATUS1, status & ~lock_mask); if (result) { - msg_cerr("spi_write_status_register failed.\n"); + msg_cerr("Could not write status register 1.\n"); return result; } status = spi_read_status_register(flash); @@ -169,9 +216,9 @@ static int spi_disable_blockprotect_generic(struct flashctx *flash, uint8_t bp_m msg_cdbg("done.\n"); } /* Global unprotect. Make sure to mask the register lock bit as well. */ - result = spi_write_status_register(flash, status & ~(bp_mask | lock_mask) & unprotect_mask); + result = spi_write_register(flash, STATUS1, status & ~(bp_mask | lock_mask) & unprotect_mask); if (result) { - msg_cerr("spi_write_status_register failed.\n"); + msg_cerr("Could not write status register 1.\n"); return result; } status = spi_read_status_register(flash);