Change in ...coreboot[master]: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30888

Change subject: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu
......................................................................

sb/intel/i82801gx: Add a mb hook to override the SPI opmenu

By default the SPI configuration gets locked down, but the default
opmenu setting could not work with some SPI flashes.

This fixes unmodified flashrom not being able to flash the Thinkpad
X60 with CONFIG_INTEL_CHIPSET_LOCKDOWN set.

Change-Id: I43e6abcc5c761720b31bc023016cb71b860e76ed
Signed-off-by: Arthur Heymans 
---
M src/mainboard/lenovo/x60/mainboard.c
M src/southbridge/intel/i82801gx/i82801gx.h
M src/southbridge/intel/i82801gx/lpc.c
3 files changed, 126 insertions(+), 47 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/30888/1

diff --git a/src/mainboard/lenovo/x60/mainboard.c b/src/mainboard/lenovo/x60/mainboard.c
index 5cf0d26..f770cf9 100644
--- a/src/mainboard/lenovo/x60/mainboard.c
+++ b/src/mainboard/lenovo/x60/mainboard.c
@@ -22,10 +22,12 @@
 #include <arch/io.h>
 #include <ec/acpi/ec.h>
 #include <northbridge/intel/i945/i945.h>
+#include <southbridge/intel/i82801gx/i82801gx.h>
 #include "dock.h"
 #include <drivers/intel/gma/int15.h>
 #include <drivers/lenovo/lenovo.h>
 #include <arch/acpigen.h>
+#include <spi_flash.h>

#define PANEL INT15_5F35_CL_DISPLAY_DEFAULT

@@ -68,6 +70,60 @@
    return ARRAY_SIZE(cst_entries);
 }

+/*
+ * SPI lockdown configuration for SST25VF016B.
+ */
+#define SPI_OPMENU_0 0x01 /* WRSR: Write Status Register */
+#define SPI_OPTYPE_0 0x01 /* Write, no address */
+
+#define SPI_OPMENU_1 0x02 /* BYPR: Byte Program */
+#define SPI_OPTYPE_1 0x03 /* Write, address required */
+
+#define SPI_OPMENU_2 0x03 /* READ: Read Data */
+#define SPI_OPTYPE_2 0x02 /* Read, address required */
+
+#define SPI_OPMENU_3 0x05 /* RDSR: Read Status Register */
+#define SPI_OPTYPE_3 0x00 /* Read, no address */
+
+#define SPI_OPMENU_4 0x20 /* SE20: Sector Erase 0x20 */
+#define SPI_OPTYPE_4 0x03 /* Write, address required */
+
+#define SPI_OPMENU_5 0x9f /* RDID: Read ID */
+#define SPI_OPTYPE_5 0x00 /* Read, no address */
+
+#define SPI_OPMENU_6 0xad /* Auto Address Increment Word Program */
+#define SPI_OPTYPE_6 0x01 /* Write, address required */
+
+#define SPI_OPMENU_7 0x04 /* Write Disable */
+#define SPI_OPTYPE_7 0x01 /* Write, no address */
+
+#define SPI_OPPREFIX ((0x50 << 8) | 0x06) /* EWSR and WREN */
+#define SPI_OPTYPE ((SPI_OPTYPE_7 << 14) | (SPI_OPTYPE_6 << 12) | \
+		    (SPI_OPTYPE_5 << 10) | (SPI_OPTYPE_4 <<  8) | \
+		    (SPI_OPTYPE_3 <<  6) | (SPI_OPTYPE_2 <<  4) | \
+		    (SPI_OPTYPE_1 <<  2) | (SPI_OPTYPE_0 <<  0))
+#define SPI_OPMENU_UPPER ((SPI_OPMENU_7 << 24) | (SPI_OPMENU_6 << 16) | \
+			  (SPI_OPMENU_5 <<  8) | (SPI_OPMENU_4 <<  0))
+#define SPI_OPMENU_LOWER ((SPI_OPMENU_3 << 24) | (SPI_OPMENU_2 << 16) | \
+			  (SPI_OPMENU_1 <<  8) | (SPI_OPMENU_0 <<  0))
+#define SPI_VSCC (WG_64_BYTE | EO(0x20) | BES_4_KB)
+
+static const struct spi_config spi_config_sst = {
+	.preop = SPI_OPPREFIX,
+	.optype = SPI_OPTYPE,
+	.opmenu = { SPI_OPMENU_LOWER, SPI_OPMENU_UPPER },
+};
+
+
+void mainboard_override_spi_config(struct spi_config *cfg)
+{       
+	const struct spi_flash *flash = boot_device_spi_flash();
+
+	if (strcmp(flash->name, "SST25VF016B") == 0) {
+		memcpy(cfg, &spi_config_sst, sizeof(*cfg));
+	}
+}
+
 static void mainboard_init(struct device *dev)
 {
    struct device *idedev, *sdhci_dev;
diff --git a/src/southbridge/intel/i82801gx/i82801gx.h b/src/southbridge/intel/i82801gx/i82801gx.h
index 9fd9fd6..0a32201 100644
--- a/src/southbridge/intel/i82801gx/i82801gx.h
+++ b/src/southbridge/intel/i82801gx/i82801gx.h
@@ -364,51 +364,16 @@
 #define C3_RES		0x54
 #define TCO1_CNT	0x68

-/* SPIBAR
- *
- * SPI Opcode Menu setup for SPIBAR lockdown
- * should support most common flash chips.
+/*
+ * SPI lockdown configuration.
  */
+struct spi_config {
+	uint16_t preop;
+	uint16_t optype;
+	uint32_t opmenu[2];
+};

-#define PREOP		0x54
-#define OPTYPE		0x56
-#define OPMENU		0x58
-
-#define SPI_OPMENU_0 0x01 /* WRSR: Write Status Register */
-#define SPI_OPTYPE_0 0x01 /* Write, no address */
-
-#define SPI_OPMENU_1 0x02 /* BYPR: Byte Program */
-#define SPI_OPTYPE_1 0x03 /* Write, address required */
-
-#define SPI_OPMENU_2 0x03 /* READ: Read Data */
-#define SPI_OPTYPE_2 0x02 /* Read, address required */
-
-#define SPI_OPMENU_3 0x05 /* RDSR: Read Status Register */
-#define SPI_OPTYPE_3 0x00 /* Read, no address */
-
-#define SPI_OPMENU_4 0x20 /* SE20: Sector Erase 0x20 */
-#define SPI_OPTYPE_4 0x03 /* Write, address required */
-
-#define SPI_OPMENU_5 0x9f /* RDID: Read ID */
-#define SPI_OPTYPE_5 0x00 /* Read, no address */
-
-#define SPI_OPMENU_6 0xd8 /* BED8: Block Erase 0xd8 */
-#define SPI_OPTYPE_6 0x03 /* Write, address required */
-
-#define SPI_OPMENU_7 0x0b /* FAST: Fast Read */
-#define SPI_OPTYPE_7 0x02 /* Read, address required */
-
-#define SPI_OPMENU_UPPER ((SPI_OPMENU_7 << 24) | (SPI_OPMENU_6 << 16) | \
-			  (SPI_OPMENU_5 << 8) | SPI_OPMENU_4)
-#define SPI_OPMENU_LOWER ((SPI_OPMENU_3 << 24) | (SPI_OPMENU_2 << 16) | \
-			  (SPI_OPMENU_1 << 8) | SPI_OPMENU_0)
-
-#define SPI_OPTYPE ((SPI_OPTYPE_7 << 14) | (SPI_OPTYPE_6 << 12) | \
-		    (SPI_OPTYPE_5 << 10) | (SPI_OPTYPE_4 << 8) |  \
-		    (SPI_OPTYPE_3 << 6) | (SPI_OPTYPE_2 << 4) |	  \
-		    (SPI_OPTYPE_1 << 2) | (SPI_OPTYPE_0))
-
-#define SPI_OPPREFIX ((0x50 << 8) | 0x06) /* EWSR and WREN */
+void mainboard_override_spi_config(struct spi_config *cfg);

#endif /* __ACPI__ */
 #endif				/* SOUTHBRIDGE_INTEL_I82801GX_I82801GX_H */
diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c
index e8cfc74..175b33a 100644
--- a/src/southbridge/intel/i82801gx/lpc.c
+++ b/src/southbridge/intel/i82801gx/lpc.c
@@ -645,21 +645,79 @@
    }
 }

+/* SPIBAR
+ *
+ * SPI Opcode Menu setup for SPIBAR lockdown
+ * should support most common flash chips.
+ */
+
+#define PREOP		0x54
+#define OPTYPE		0x56
+#define OPMENU		0x58
+
+#define SPI_OPMENU_0 0x01 /* WRSR: Write Status Register */
+#define SPI_OPTYPE_0 0x01 /* Write, no address */
+
+#define SPI_OPMENU_1 0x02 /* BYPR: Byte Program */
+#define SPI_OPTYPE_1 0x03 /* Write, address required */
+
+#define SPI_OPMENU_2 0x03 /* READ: Read Data */
+#define SPI_OPTYPE_2 0x02 /* Read, address required */
+
+#define SPI_OPMENU_3 0x05 /* RDSR: Read Status Register */
+#define SPI_OPTYPE_3 0x00 /* Read, no address */
+
+#define SPI_OPMENU_4 0x20 /* SE20: Sector Erase 0x20 */
+#define SPI_OPTYPE_4 0x03 /* Write, address required */
+
+#define SPI_OPMENU_5 0x9f /* RDID: Read ID */
+#define SPI_OPTYPE_5 0x00 /* Read, no address */
+
+#define SPI_OPMENU_6 0xd8 /* BED8: Block Erase 0xd8 */
+#define SPI_OPTYPE_6 0x03 /* Write, address required */
+
+#define SPI_OPMENU_7 0x0b /* FAST: Fast Read */
+#define SPI_OPTYPE_7 0x02 /* Read, address required */
+
+#define SPI_OPMENU_UPPER ((SPI_OPMENU_7 << 24) | (SPI_OPMENU_6 << 16) | \
+			  (SPI_OPMENU_5 << 8) | SPI_OPMENU_4)
+#define SPI_OPMENU_LOWER ((SPI_OPMENU_3 << 24) | (SPI_OPMENU_2 << 16) | \
+			  (SPI_OPMENU_1 << 8) | SPI_OPMENU_0)
+
+#define SPI_OPTYPE ((SPI_OPTYPE_7 << 14) | (SPI_OPTYPE_6 << 12) | \
+		    (SPI_OPTYPE_5 << 10) | (SPI_OPTYPE_4 << 8) |  \
+		    (SPI_OPTYPE_3 << 6) | (SPI_OPTYPE_2 << 4) |	  \
+		    (SPI_OPTYPE_1 << 2) | (SPI_OPTYPE_0))
+
+#define SPI_OPPREFIX ((0x50 << 8) | 0x06) /* EWSR and WREN */
+
 #define SPIBAR16(x) RCBA16(0x3020 + x)
 #define SPIBAR32(x) RCBA32(0x3020 + x)

+void __weak mainboard_override_spi_config(struct spi_config *cfg)
+{
+}
+
 static void lpc_final(struct device *dev)
 {
    u16 tco1_cnt;
+	struct spi_config spi_cfg;

if (!IS_ENABLED(CONFIG_INTEL_CHIPSET_LOCKDOWN))
    	return;

-	SPIBAR16(PREOP) = SPI_OPPREFIX;
+	spi_cfg.preop = SPI_OPPREFIX;
+	spi_cfg.optype = SPI_OPTYPE;
+	spi_cfg.opmenu[0] = SPI_OPMENU_LOWER;
+	spi_cfg.opmenu[1] = SPI_OPMENU_UPPER;
+
+	mainboard_override_spi_config(&spi_cfg);
+
+	SPIBAR16(PREOP) = spi_cfg.preop;
    /* Set SPI opcode menu */
-	SPIBAR16(OPTYPE) = SPI_OPTYPE;
-	SPIBAR32(OPMENU) = SPI_OPMENU_LOWER;
-	SPIBAR32(OPMENU + 4) = SPI_OPMENU_UPPER;
+	SPIBAR16(OPTYPE) = spi_cfg.optype;
+	SPIBAR32(OPMENU) = spi_cfg.opmenu[0];
+	SPIBAR32(OPMENU + 4) = spi_cfg.opmenu[1];

/* Lock SPIBAR */
    SPIBAR16(0) = SPIBAR16(0) | (1 << 15);

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30888 )

Change subject: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/30888/1/src/mainboard/lenovo/x60/mainboard.c 
File src/mainboard/lenovo/x60/mainboard.c:

https://review.coreboot.org/#/c/30888/1/src/mainboard/lenovo/x60/mainboard.c... 
PS1, Line 119: {       
trailing whitespace

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30888 )

Change subject: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu
......................................................................

Patch Set 1:

It might be better to have flashrom deal with this properly.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30888 )

Change subject: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu
......................................................................

Patch Set 1:
> It might be better to have flashrom deal with this properly.

How does coreboot handle it? AFAICS, the chips in question would
either have to be programmed byte-wise or need us to patch the
opmenu on the fly. I assume coreboot does the latter?

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30888 )

Change subject: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu
......................................................................

Patch Set 1:

Can the trailing space issue be fixed?

Is this also relevant for the Lenovo T60?

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30888 )

Change subject: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu
......................................................................

Patch Set 1:
> Patch Set 1:
> > It might be better to have flashrom deal with this properly.
>
> How does coreboot handle it? AFAICS, the chips in question would
> either have to be programmed byte-wise or need us to patch the
> opmenu on the fly. I assume coreboot does the latter?

Coreboot indeed does the latter. This is not very mainboard specific, but more intel+SPI-flash specific, so IMO it does not really belong here, but could good-enough for now...

Change in coreboot[master]: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu

[Arthur Heymans (Code Review)]

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/30888 )

Change subject: sb/intel/i82801gx: Add a mb hook to override the SPI opmenu
......................................................................

Abandoned

Done