From 49bcb78006d88189bb13513982f3fa734b441641 Mon Sep 17 00:00:00 2001 From: Nikolai Artemiev Date: Wed, 2 Nov 2022 11:30:57 +1100 Subject: writeprotect,ichspi,spi25: handle register access constraints Make the spi25 register read/write functions return SPI_INVALID_OPCODE if the programmer blocks the read/write opcode for the register. Likewise, make ichspi read/write register functions return SPI_INVALID_OPCODE for registers >SR1 as they cannot be accessd. Make writeprotect ignore SPI_INVALID_OPCODE unless it is trying to read/write SR1, which should always be supported. BUG=b:253715389,b:253713774,b:240229722 BRANCH=none TEST=flashrom --wp-{enable,disable,range,list,status} on dedede Change-Id: I2145749dcc51f4556550650dab5aa1049f879c45 Signed-off-by: Nikolai Artemiev Reviewed-on: https://review.coreboot.org/c/flashrom/+/69129 Tested-by: build bot (Jenkins) Reviewed-by: Edward O'Callaghan --- ichspi.c | 16 ++++++++++++---- spi25_statusreg.c | 11 +++++++++++ writeprotect.c | 29 +++++++++++++++++++++++++---- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/ichspi.c b/ichspi.c index 4588502a..2b6c7ae5 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1400,8 +1400,12 @@ static int ich_hwseq_read_status(const struct flashctx *flash, enum flash_reg re const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); if (reg != STATUS1) { - msg_perr("%s: only supports STATUS1\n", __func__); - return -1; + msg_pdbg("%s: only supports STATUS1\n", __func__); + /* + * Return SPI_INVALID_OPCODE to be consistent with spi_read_register() + * and make error handling simpler even though this isn't a SPI master. + */ + return SPI_INVALID_OPCODE; } msg_pdbg("Reading Status register\n"); @@ -1421,8 +1425,12 @@ static int ich_hwseq_write_status(const struct flashctx *flash, enum flash_reg r const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); if (reg != STATUS1) { - msg_perr("%s: only supports STATUS1\n", __func__); - return -1; + msg_pdbg("%s: only supports STATUS1\n", __func__); + /* + * Return SPI_INVALID_OPCODE to be consistent with spi_write_register() + * and make error handling simpler even though this isn't a SPI master. + */ + return SPI_INVALID_OPCODE; } msg_pdbg("Writing status register\n"); diff --git a/spi25_statusreg.c b/spi25_statusreg.c index b178b2e3..726ca8cc 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -19,6 +19,7 @@ #include "flash.h" #include "chipdrivers.h" +#include "programmer.h" #include "spi.h" /* === Generic functions === */ @@ -129,6 +130,11 @@ int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t return 1; } + if (!flash->mst->spi.probe_opcode(flash, write_cmd[0])) { + msg_pdbg("%s: write to register %d not supported by programmer, ignoring.\n", __func__, reg); + return SPI_INVALID_OPCODE; + } + uint8_t enable_cmd; if (feature_bits & FEATURE_WRSR_WREN) { enable_cmd = JEDEC_WREN; @@ -238,6 +244,11 @@ int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t return 1; } + if (!flash->mst->spi.probe_opcode(flash, read_cmd)) { + msg_pdbg("%s: read from register %d not supported by programmer.\n", __func__, reg); + return SPI_INVALID_OPCODE; + } + /* FIXME: No workarounds for driver/hardware bugs in generic code. */ /* JEDEC_RDSR_INSIZE=1 but wbsio needs 2 */ uint8_t readarr[2]; diff --git a/writeprotect.c b/writeprotect.c index 49586b4b..d8ffa689 100644 --- a/writeprotect.c +++ b/writeprotect.c @@ -19,6 +19,7 @@ #include #include +#include "spi.h" #include "flash.h" #include "libflashrom.h" #include "chipdrivers.h" @@ -30,18 +31,38 @@ */ static int wp_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value) { + int ret; if ((flash->mst->buses_supported & BUS_PROG) && flash->mst->opaque.write_register) { - return flash->mst->opaque.write_register(flash, reg, value); + ret = flash->mst->opaque.write_register(flash, reg, value); + } else { + ret = spi_write_register(flash, reg, value); + } + + /* Writing SR1 should always be supported, ignore errors for other registers. */ + if (ret == SPI_INVALID_OPCODE && reg != STATUS1) { + msg_pdbg("%s: write to register %d not supported by programmer, ignoring.\n", __func__, reg); + ret = 0; } - return spi_write_register(flash, reg, value); + return ret; } static int wp_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value) { + int ret; if ((flash->mst->buses_supported & BUS_PROG) && flash->mst->opaque.read_register) { - return flash->mst->opaque.read_register(flash, reg, value); + ret = flash->mst->opaque.read_register(flash, reg, value); + } else { + ret = spi_read_register(flash, reg, value); } - return spi_read_register(flash, reg, value); + + /* Reading SR1 should always be supported, ignore errors for other registers. */ + if (ret == SPI_INVALID_OPCODE && reg != STATUS1) { + msg_pdbg("%s: read from register %d not is supported by programmer, " + "writeprotect operations will assume it contains 0x00.\n", __func__, reg); + *value = 0; + ret = 0; + } + return ret; } /** Read and extract a single bit from the chip's registers */ -- cgit v1.2.3