Change in flashrom[master]: flashchips,spi25: Replace `.wrea_override` with FEATURE_4BA_EAR_1716

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons.
Hello Angel Pons,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/flashrom/+/64636

to review the following change.

Change subject: flashchips,spi25: Replace `.wrea_override` with FEATURE_4BA_EAR_1716
......................................................................

flashchips,spi25: Replace `.wrea_override` with FEATURE_4BA_EAR_1716

There are two competing sets of instructions to access the extended
address register of 4BA SPI chips. Some chips even support both sets.

So far, we assumed the 0xc5/0xc8 instructions by default and allowed
to override the write instructions with the `.wrea_override` field.
This has some disadvantages:

* The additional field is easily overlooked. So when adding a new
  flash chip, one might assume only 0xc5/0xc8 are supported.

* We cannot describe flash chips completely that allow both
  instructions (and some programmers may be picky about which
  instructions can be used).

Therefore, replace the `.wrea_override` field with a feature flag,
and set it for chips known to support it (Spansion and ISSI). The
S25FL512S did not advertise EAR support at all, so we set it to
TEST_UNTESTED again.

Signed-off-by: Nico Huber 
Change-Id: I6d82f24898acd0789203516a7456fd785907bc10
Ticket: https://ticket.coreboot.org/issues/357
---
M dediprog.c
M flashchips.c
M include/flash.h
M include/spi.h
M spi25.c
5 files changed, 28 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/64636/1

diff --git a/dediprog.c b/dediprog.c
index 91a1632..9775fde 100644
--- a/dediprog.c
+++ b/dediprog.c
@@ -416,7 +416,7 @@
    		}
    	}
    } else {
-		if (flash->chip->feature_bits & FEATURE_4BA_EAR_C5C8) {
+		if (flash->chip->feature_bits & FEATURE_4BA_EAR_ANY) {
    		if (spi_set_extended_address(flash, start >> 24))
    			return 1;
    	} else if (start >> 24) {
diff --git a/flashchips.c b/flashchips.c
index d8208fe..cc80159 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -7469,7 +7469,8 @@
    	.page_size	= 256,
    	/* supports SFDP */
    	/* OTP: 1024B total; read 0x68; write 0x62, erase 0x64, read ID 0x4B */
-		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA | FEATURE_4BA_ENTER_EAR7,
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP |
+				  FEATURE_4BA | FEATURE_4BA_ENTER_EAR7 | FEATURE_4BA_EAR_1716,
    	.tested		= TEST_OK_PREW,
    	.probe		= probe_spi_rdid,
    	.probe_timing	= TIMING_ZERO,
@@ -7644,7 +7645,8 @@
    	.page_size	= 256,
    	/* supports SFDP */
    	/* OTP: 1024B total; read 0x68; write 0x62, erase 0x64, read ID 0x4B */
-		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA | FEATURE_4BA_ENTER_EAR7,
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP |
+				  FEATURE_4BA | FEATURE_4BA_ENTER_EAR7 | FEATURE_4BA_EAR_1716,
    	.tested		= TEST_OK_PREW,
    	.probe		= probe_spi_rdid,
    	.probe_timing	= TIMING_ZERO,
@@ -16672,7 +16674,8 @@
    	.total_size	= 32768,
    	.page_size	= 256,
    	/* OTP: 1024B total, 32B reserved; read 0x4B; write 0x42 */
-		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_EAR7,
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP |
+				  FEATURE_4BA_NATIVE | FEATURE_4BA_ENTER_EAR7 | FEATURE_4BA_EAR_1716,
    	.tested		= TEST_OK_PREW,
    	.probe		= probe_spi_rdid,
    	.probe_timing	= TIMING_ZERO,
@@ -16706,7 +16709,6 @@
    	.write		= spi_chip_write_256, /* Multi I/O supported */
    	.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
    	.voltage	= {2700, 3600},
-		.wrea_override	= 0x17,
    },

{
@@ -16718,8 +16720,9 @@
    	.total_size	= 65536, /* 512 Mb (=> 64 MB)) */
    	.page_size	= 256,
    	/* OTP: 1024B total, 32B reserved; read 0x4B; write 0x42 */
-		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_NATIVE,
-		.tested		= TEST_OK_PREW,
+		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP |
+				  FEATURE_4BA_NATIVE | FEATURE_4BA_ENTER_EAR7 | FEATURE_4BA_EAR_1716,
+		.tested		= TEST_UNTESTED,
    	.probe		= probe_spi_rdid,
    	.probe_timing	= TIMING_ZERO,
    	.block_erasers	=
diff --git a/include/flash.h b/include/flash.h
index 162c0ac..8203295 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -127,10 +127,12 @@
 #define FEATURE_4BA_EAR_C5C8	(1 << 13) /**< Regular 3-byte operations can be used by writing the most
    				       significant address byte into an extended address register
    				       (using 0xc5/0xc8 instructions). */
-#define FEATURE_4BA_READ	(1 << 14) /**< Native 4BA read instruction (0x13) is supported. */
-#define FEATURE_4BA_FAST_READ	(1 << 15) /**< Native 4BA fast read instruction (0x0c) is supported. */
-#define FEATURE_4BA_WRITE	(1 << 16) /**< Native 4BA byte program (0x12) is supported. */
+#define FEATURE_4BA_EAR_1716	(1 << 14) /**< Like FEATURE_4BA_EAR_C5C8 but with 0x17/0x16 instructions. */
+#define FEATURE_4BA_READ	(1 << 15) /**< Native 4BA read instruction (0x13) is supported. */
+#define FEATURE_4BA_FAST_READ	(1 << 16) /**< Native 4BA fast read instruction (0x0c) is supported. */
+#define FEATURE_4BA_WRITE	(1 << 17) /**< Native 4BA byte program (0x12) is supported. */
 /* 4BA Shorthands */
+#define FEATURE_4BA_EAR_ANY	(FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_EAR_1716)
 #define FEATURE_4BA_NATIVE	(FEATURE_4BA_READ | FEATURE_4BA_FAST_READ | FEATURE_4BA_WRITE)
 #define FEATURE_4BA		(FEATURE_4BA_ENTER | FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_NATIVE)
 #define FEATURE_4BA_WREN	(FEATURE_4BA_ENTER_WREN | FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_NATIVE)
@@ -273,9 +275,6 @@
    } voltage;
    enum write_granularity gran;

-	/* SPI specific options (TODO: Make it a union in case other bustypes get specific options.) */
-	uint8_t wrea_override; /**< override opcode for write extended address register */
-
    struct reg_bit_map {
    	/* Status register protection bit (SRP) */
    	struct reg_bit_info srp;
diff --git a/include/spi.h b/include/spi.h
index 14f71aa..ef331db 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -176,9 +176,11 @@

/* Write Extended Address Register */
 #define JEDEC_WRITE_EXT_ADDR_REG	0xC5
+#define ALT_WRITE_EXT_ADDR_REG_17	0x17

/* Read Extended Address Register */
 #define JEDEC_READ_EXT_ADDR_REG		0xC8
+#define ALT_READ_EXT_ADDR_REG_16	0x16

/* Read the memory */
 #define JEDEC_READ		0x03
diff --git a/spi25.c b/spi25.c
index f6900a9..ad218fa 100644
--- a/spi25.c
+++ b/spi25.c
@@ -351,7 +351,16 @@

static int spi_write_extended_address_register(struct flashctx *const flash, const uint8_t regdata)
 {
-	const uint8_t op = flash->chip->wrea_override ? : JEDEC_WRITE_EXT_ADDR_REG;
+	uint8_t op;
+	if (flash->chip->feature_bits & FEATURE_4BA_EAR_C5C8) {
+		op = JEDEC_WRITE_EXT_ADDR_REG;
+	} else if (flash->chip->feature_bits & FEATURE_4BA_EAR_1716) {
+		op = ALT_WRITE_EXT_ADDR_REG_17;
+	} else {
+		msg_cerr("Flash misses feature flag for extended-address register.\n");
+		return -1;
+	}
+
    struct spi_command cmds[] = {
    {
    	.readarr = 0,
@@ -394,7 +403,7 @@
    	cmd_buf[4] = (addr >>  0) & 0xff;
    	return 4;
    } else {
-		if (flash->chip->feature_bits & FEATURE_4BA_EAR_C5C8) {
+		if (flash->chip->feature_bits & FEATURE_4BA_EAR_ANY) {
    		if (spi_set_extended_address(flash, addr >> 24))
    			return -1;
    	} else if (addr >> 24) {