Edward O'Callaghan submitted this change.
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 <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69129
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
---
M ichspi.c
M spi25_statusreg.c
M writeprotect.c
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/ichspi.c b/ichspi.c
index 4588502..2b6c7ae 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1400,8 +1400,12 @@
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 @@
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 b178b2e..726ca8c 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 @@
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 @@
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 49586b4..d8ffa68 100644
--- a/writeprotect.c
+++ b/writeprotect.c
@@ -19,6 +19,7 @@
#include <stdlib.h>
#include <string.h>
+#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);
}
- return 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 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 */
To view, visit change 69129. To unsubscribe, or for help writing mail filters, visit settings.