[coreboot-gerrit] Change in coreboot[master]: soc/amd/common/block/spi/fch_spi_ctrl.c: Fix SPI vendor id code

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35589 )

Change subject: soc/amd/common/block/spi/fch_spi_ctrl.c: Fix SPI vendor id code
......................................................................

soc/amd/common/block/spi/fch_spi_ctrl.c: Fix SPI vendor id code

All solid state devices have vendor id defined by JEDEC specification JEP106,
which originally allocated only 7 bits for it plus parity. When number of
vendors exploded beyond 126, a banking proposition came maintaining
compatibility with older vendors while allowing for 4 extra bits (16 banks)
through the introduction of the concept "Continuation code", denoted by the
byte value of 0x7f.
Examples:
0xfe, 0x60, 0x18, 0x00, 0x00 => vendor 0xfe of bank o
0x7f, 0x7f, 0xfe, 0x60, 0x18 => vendor 0xfe of bank 2

BUG=b:141535133
TEST=Build and boot grunt.

Change-Id: I16c5df70b8ba65017d1a45c79e90a76d1f78550c
Signed-off-by: Richard Spiegel 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35589
Tested-by: build bot (Jenkins) 
Reviewed-by: Edward O'Callaghan 
---
M src/soc/amd/common/block/include/amdblocks/fch_spi.h
M src/soc/amd/common/block/spi/fch_spi_ctrl.c
M src/soc/amd/common/block/spi/fch_spi_table.c
3 files changed, 27 insertions(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Edward O'Callaghan: Looks good to me, approved

diff --git a/src/soc/amd/common/block/include/amdblocks/fch_spi.h b/src/soc/amd/common/block/include/amdblocks/fch_spi.h
index cfbdf19..8e28828 100644
--- a/src/soc/amd/common/block/include/amdblocks/fch_spi.h
+++ b/src/soc/amd/common/block/include/amdblocks/fch_spi.h
@@ -23,7 +23,14 @@
 #define WORD_TO_DWORD_UPPER(x)		((x << 16) & 0xffff0000)
 #define SPI_PAGE_WRITE			0x02
 #define SPI_WRITE_ENABLE		0x06
-#define IDCODE_CONT_LEN			0
+/*
+ * IDCODE_CONT_LEN may be redefined if a device needs to declare a
+ * larger "shift" value.  IDCODE_PART_LEN generally shouldn't be
+ * changed.  This is the max number of bytes probe functions may
+ * examine when looking up part-specific identification info.
+ */
+#define IDCODE_CONT_CODE		0x7f
+#define IDCODE_CONT_LEN			1 /* currently support only bank 0 */
 #define IDCODE_PART_LEN			5
 #define IDCODE_LEN (IDCODE_CONT_LEN + IDCODE_PART_LEN)

diff --git a/src/soc/amd/common/block/spi/fch_spi_ctrl.c b/src/soc/amd/common/block/spi/fch_spi_ctrl.c
index 4299e4f..a7d9c77 100644
--- a/src/soc/amd/common/block/spi/fch_spi_ctrl.c
+++ b/src/soc/amd/common/block/spi/fch_spi_ctrl.c
@@ -278,12 +278,26 @@
    	printk(BIOS_SPEW, "\n");
    }

-	/* count the number of continuation bytes */
-	for (shift = 0, idp = idcode; shift < IDCODE_CONT_LEN && *idp == 0x7f;
-	     ++shift, ++idp)
-		continue;
+	/*
+	 * All solid state devices have vendor id defined by JEDEC specification JEP106,
+	 * which originally allocated only 7 bits for it plus parity. When number of
+	 * vendors exploded beyond 126, a banking proposition came maintaining
+	 * compatibility with older vendors while allowing for 4 extra bits (16 banks)
+	 * through the introduction of the concept "Continuation Code", denoted by the
+	 * byte value of 0x7f.
+	 * Examples:
+	 * 0xfe, 0x60, 0x18, 0x00, 0x00, 0x00 => vendor 0xfe of bank o
+	 * 0x7f, 0x7f, 0xfe, 0x60, 0x18, 0x00 => vendor 0xfe of bank 2
+	 * count the number of continuation code bytes
+	 */
+	for (shift = 0, idp = idcode; *idp == IDCODE_CONT_CODE; ++shift, ++idp) {
+		if (shift < IDCODE_CONT_LEN)
+			continue;
+		printk(BIOS_ERR, "unsupported ID code bank\n");
+		return -1;
+	}

-	printk(BIOS_INFO, "Manufacturer: %02x\n", *idp);
+	printk(BIOS_INFO, "Manufacturer: %02x on bank %d\n", *idp, shift);

/* search the table for matches in shift and id */
    for (i = 0; i < table_size; ++i) {
diff --git a/src/soc/amd/common/block/spi/fch_spi_table.c b/src/soc/amd/common/block/spi/fch_spi_table.c
index 8c0108a..acea241 100644
--- a/src/soc/amd/common/block/spi/fch_spi_table.c
+++ b/src/soc/amd/common/block/spi/fch_spi_table.c
@@ -30,11 +30,6 @@
  * Several matching entries are permitted, they will be tried
  * in sequence until a probe function returns non NULL.
  *
- * IDCODE_CONT_LEN may be redefined if a device needs to declare a
- * larger "shift" value.  IDCODE_PART_LEN generally shouldn't be
- * changed.  This is the max number of bytes probe functions may
- * examine when looking up part-specific identification info.
- *
  * Probe functions will be given the idcode buffer starting at their
  * manu id byte (the "idcode" in the table below).  In other words,
  * all of the continuation bytes will be skipped (the "shift" below).