Nikolai Artemiev has uploaded this change for review.

View 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
BRANCH=none
TEST=builds / todo

Change-Id: I2145749dcc51f4556550650dab5aa1049f879c45
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
---
M ichspi.c
M spi25_statusreg.c
M writeprotect.c
3 files changed, 67 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/69129/1

diff --git a/ichspi.c b/ichspi.c
index 6d3535c..feee968 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1394,8 +1394,8 @@
 	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;
 	}
 	msg_pdbg("Reading Status register\n");
 
@@ -1415,8 +1415,8 @@
 	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;
 	}
 	msg_pdbg("Writing status register\n");
 
diff --git a/spi25_statusreg.c b/spi25_statusreg.c
index d0ce859..f42bcd4 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 === */
@@ -105,6 +106,12 @@
 		return 1;
 	}
 
+	/* Some SPI masters like ICH7/ICH9 block opcodes for writing SR2/SR3/etc. */
+	if (!flash->mst->spi.probe_opcode((struct flashctx *) 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;
@@ -200,6 +207,12 @@
 		return 1;
 	}
 
+	/* Some SPI masters like ICH7/ICH9 block opcodes for reading SR2/SR3/etc. */
+	if (!flash->mst->spi.probe_opcode((struct flashctx *) 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 f72c362..1cde16a 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,40 @@
  */
 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);
 	}
-	return spi_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 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);
 	}
-	return spi_read_register(flash, reg, value);
+	else {
+		ret = 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.