Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/58475 )
Change subject: [RFC] spi25_statusreg: make status register functions generic ......................................................................
[RFC] spi25_statusreg: make status register functions generic
This patch adds new spi_{read,write}_register() functions that take the source/destination register as an argument. Currently they still only accept the STATUS1 register, support for other registers will be added in following patches.
Since we're refactoring things, this commit also makes spi_read_register() return an error code, making it possible to identify error conditions that spi_read_status_register() concealed.
BUG=b:195381327,b:153800563 TEST=builds, tested flashrom/wp commands at end of patch chain BRANCH=none
Change-Id: I0a3951bbf993f2d8d830143b29d3ce16cc6901d7 Signed-off-by: Nikolai Artemiev nartemiev@google.com --- M chipdrivers.h M flash.h M spi25_statusreg.c 3 files changed, 80 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/58475/1
diff --git a/chipdrivers.h b/chipdrivers.h index e1d6aa9..ea8d480 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -62,8 +62,10 @@
/* 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 18e1a57..c3f99c9 100644 --- a/flash.h +++ b/flash.h @@ -168,6 +168,15 @@ #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, + CONFIG1, +}; +#define MAX_REGISTERS 4 + struct flashchip { const char *vendor; const char *name; diff --git a/spi25_statusreg.c b/spi25_statusreg.c index a0b0fcf..8e8eead 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -22,24 +22,27 @@ #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) { - 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. */ + 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,7 +52,7 @@ .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 @@ -62,6 +65,7 @@ * 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. */ + int i = 0; programmer_delay(100 * 1000); while (spi_read_status_register(flash) & SPI_SR_WIP) { if (++i > 490) { @@ -73,47 +77,83 @@ return 0; }
-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. */ + if (reg == STATUS1) { + write_cmd[0] = JEDEC_WRSR; + write_cmd[1] = value; + write_cmd_len = 2; + } + if(write_cmd_len == 0) { + msg_cerr("Cannot write register: not supported by chip\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); 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); return ret; }
-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) { - 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; + uint8_t read_cmd;
- /* 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; + if (reg == STATUS1) { + read_cmd = JEDEC_RDSR; + } + else { + msg_cerr("Cannot read register: not supported by chip\n"); + return 1; }
- return readarr[0]; + /* 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) +{ + uint8_t status; + /* 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. * Tests if a protection is enabled with the block protection mask (bp_mask) and returns success otherwise. * Tests if the register bits are locked with the lock_mask (lock_mask). @@ -156,9 +196,9 @@ 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("spi_write_register failed.\n"); return result; } status = spi_read_status_register(flash); @@ -169,9 +209,9 @@ 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("spi_write_register failed.\n"); return result; } status = spi_read_status_register(flash);