skylake: refactor flash_controller code

There's no need to add any typedefs nor guard code with
ENV_ROMSTAGE. The linker will garbage collect unused functions.
Additionally there were a few errors in the code including
the operation mask wasn't wide enough to clear out old operations
as well as component size decoding was incorrect.

The big difference in the code flow is that the operation
setup is now in one place. The stopwatch API is also used in
order to not open code time calculations.

BUG=chrome-os-partner:42115
BUG=chrome-os-partner:43522
BRANCH=None
TEST=Built and booted. Suspended and resumed. event log is populated
     for all.

Change-Id: I0ddd42f0744cf8f88da832d7d715663238209a71
Signed-off-by: Patrick Georgi <pgeorgi@chromium.org>
Original-Commit-Id: 9893fe309104c05edfb158afda6bb029801c0489
Original-Change-Id: I6468f5b9b4a73885b69ebd916861dd2e8e3746b6
Original-Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Original-Reviewed-on: https://chromium-review.googlesource.com/295980
Original-Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-on: http://review.coreboot.org/11543
Tested-by: build bot (Jenkins)
Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

2 files changed, 158 insertions, 208 deletions

diff --git a/src/soc/intel/skylake/flash_controller.c b/src/soc/intel/skylake/flash_controller.c
index ab334416c7..897956baf8 100644
--- a/src/soc/intel/skylake/flash_controller.c
+++ b/src/soc/intel/skylake/flash_controller.c
@@ -21,73 +21,135 @@
 #include <stdlib.h>
 #include <string.h>
 #include <bootstate.h>
-#include <delay.h>
-#include <device/pci_ids.h>
 #include <spi_flash.h>
-#include <spi-generic.h>
+#include <timer.h>
 #include <soc/flash_controller.h>
 #include <soc/pci_devs.h>
 #include <soc/spi.h>
 
-#if !(ENV_ROMSTAGE)
-typedef struct spi_slave pch_spi_slave;
-static struct spi_flash *spi_flash_hwseq_probe(struct spi_slave *spi);
-#endif
+static inline uint16_t spi_read_hsfs(pch_spi_regs * const regs)
+{
+	return readw_(&regs->hsfs);
+}
 
-unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len)
+static inline void spi_clear_status(pch_spi_regs * const regs)
 {
-	pch_spi_regs *spi_bar;
+	/* clear FDONE, FCERR, AEL by writing 1 to them (if they are set) */
+	writew_(spi_read_hsfs(regs), &regs->hsfs);
+}
 
-	spi_bar = get_spi_bar();
-	return min(sizeof(spi_bar->fdata), buf_len);
+static inline uint16_t spi_read_hsfc(pch_spi_regs * const regs)
+{
+	return readw_(&regs->hsfc);
 }
 
-#if !(ENV_ROMSTAGE)
-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
+static inline uint32_t spi_read_faddr(pch_spi_regs * const regs)
 {
-	pch_spi_slave *slave = malloc(sizeof(*slave));
+	return readl_(&regs->faddr) & SPIBAR_FADDR_MASK;
+}
 
-	if (!slave) {
-		printk(BIOS_DEBUG, "PCH SPI: Bad allocation\n");
-		return NULL;
+/*
+ * Polls for Cycle Done Status, Flash Cycle Error
+ * Resets all error flags in HSFS.
+ * Returns 0 if the cycle completes successfully without errors within
+ * timeout, 1 on errors.
+ */
+static int wait_for_completion(pch_spi_regs * const regs, int timeout_ms,
+				size_t len)
+{
+	uint16_t hsfs;
+	uint16_t hsfc;
+	uint32_t addr;
+	struct stopwatch sw;
+	int timeout = 0;
+
+	stopwatch_init_msecs_expire(&sw, timeout_ms);
+
+	while (!(timeout = stopwatch_expired(&sw))) {
+		hsfs = spi_read_hsfs(regs);
+
+		if ((hsfs & (HSFS_FDONE | HSFS_FCERR)))
+			break;
 	}
 
-	memset(slave, 0, sizeof(*slave));
+	if (timeout) {
+		addr = spi_read_faddr(regs);
+		hsfc = spi_read_hsfc(regs);
+		printk(BIOS_ERR, "%ld ms Transaction timeout between offset "
+			"0x%08x and 0x%08x (= 0x%08x + %zd) HSFC=%x HSFS=%x!\n",
+			stopwatch_duration_msecs(&sw), addr, addr + len - 1,
+			addr, len - 1, hsfc, hsfs);
+		return 1;
+	}
 
-	slave->bus = bus;
-	slave->cs = cs;
-	slave->force_programmer_specific = 1;
-	slave->programmer_specific_probe = spi_flash_hwseq_probe;
+	if (hsfs & HSFS_FCERR) {
+		addr = spi_read_faddr(regs);
+		hsfc = spi_read_hsfc(regs);
+		printk(BIOS_ERR, "Transaction error between offset 0x%08x and "
+		       "0x%08x (= 0x%08x + %zd) HSFC=%x HSFS=%x!\n",
+		       addr, addr + len - 1, addr, len - 1,
+		       hsfc, hsfs);
+		return 1;
+	}
 
-	return slave;
+	return 0;
 }
-#endif
 
-static u32 spi_get_flash_size(pch_spi_regs *spi_bar)
+/* Start operation returning 0 on success, non-zero on error or timeout. */
+static int spi_do_operation(int op, size_t offset, size_t size, int timeout_ms)
+{
+	uint16_t hsfc;
+	pch_spi_regs * const regs = get_spi_bar();
+
+	/* Clear status prior to operation. */
+	spi_clear_status(regs);
+
+	/* Set the FADDR */
+	writel_(offset & SPIBAR_FADDR_MASK, &regs->faddr);
+
+	hsfc = readw_(&regs->hsfc);
+	/* Clear then set the correct op. */
+	hsfc &= ~HSFC_FCYCLE_MASK;
+	hsfc |= op;
+	/* Set the size field */
+	hsfc &= ~HSFC_FDBC_MASK;
+	/* Check for sizes of confirming operations. */
+	if (size && size <= SPI_FDATA_BYTES)
+		hsfc |= ((size - 1) << HSFC_FDBC_SHIFT) & HSFC_FDBC_MASK;
+	/* start operation */
+	hsfc |= HSFC_FGO;
+	writew_(hsfc, &regs->hsfc);
+
+	return wait_for_completion(regs, timeout_ms, size);
+}
+
+unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len)
+{
+	return min(SPI_FDATA_BYTES, buf_len);
+}
+
+static size_t spi_get_flash_size(pch_spi_regs *spi_bar)
 {
 	uint32_t flcomp;
-	u32 size;
+	size_t size;
 
 	writel_(SPIBAR_FDOC_COMPONENT, &spi_bar->fdoc);
 	flcomp = readl_(&spi_bar->fdod);
-	printk(BIOS_DEBUG, "flcomp = %x\n", flcomp);
 
 	switch (flcomp & FLCOMP_C0DEN_MASK) {
 	case FLCOMP_C0DEN_8MB:
-		size = 0x100000;
+		size = 8*MiB;
 		break;
 	case FLCOMP_C0DEN_16MB:
-		size = 0x1000000;
+		size = 16*MiB;
 		break;
 	case FLCOMP_C0DEN_32MB:
-		size = 0x10000000;
+		size = 32*MiB;
 		break;
 	default:
-		size = 0x1000000;
+		size = 16*MiB;
 	}
 
-	printk(BIOS_DEBUG, "flash size 0x%x bytes\n", size);
-
 	return size;
 }
 
@@ -102,16 +164,6 @@ void spi_init(void)
 {
 	uint8_t bios_cntl;
 	device_t dev = PCH_DEV_SPI;
-	pch_spi_regs *spi_bar;
-	uint16_t hsfs;
-
-	/* Root Complex Register Block */
-	spi_bar = get_spi_bar();
-	hsfs = readw_(&spi_bar->hsfs);
-	if (hsfs & HSFS_FDV) {
-		/* Select Flash Descriptor Section Index to 1 */
-		writel_(SPIBAR_FDOC_FDSI_1, &spi_bar->fdoc);
-	}
 
 	/* Disable the BIOS write protect so write commands are allowed. */
 	pci_read_config_byte(dev, SPIBAR_BIOS_CNTL, &bios_cntl);
@@ -131,64 +183,11 @@ void spi_release_bus(struct spi_slave *slave)
 	/* Handled by PCH automatically. */
 }
 
-static void pch_hwseq_set_addr(uint32_t addr, pch_spi_regs *spi_bar)
-{
-	uint32_t addr_old = readl_(&spi_bar->faddr) & ~SPIBAR_FADDR_MASK;
-	writel_((addr & SPIBAR_FADDR_MASK) | addr_old, &spi_bar->faddr);
-}
-
-/*
- * Polls for Cycle Done Status, Flash Cycle Error or timeout in 8 us intervals.
- * Resets all error flags in HSFS.
- * Returns 0 if the cycle completes successfully without errors within
- * timeout us, 1 on errors.
- */
-static int pch_hwseq_wait_for_cycle_complete(unsigned int timeout,
-			unsigned int len, pch_spi_regs *spi_bar)
-{
-	uint16_t hsfs;
-	uint32_t addr;
-
-	timeout /= 8; /* scale timeout duration to counter */
-	while ((((hsfs = readw_(&spi_bar->hsfs)) &
-		 (HSFS_FDONE | HSFS_FCERR)) == 0) && --timeout) {
-		udelay(8);
-	}
-	writew_(readw_(&spi_bar->hsfs), &spi_bar->hsfs);
-
-	if (!timeout) {
-		uint16_t hsfc;
-		addr = readl_(&spi_bar->faddr) & SPIBAR_FADDR_MASK;
-		hsfc = readw_(&spi_bar->hsfc);
-		printk(BIOS_ERR, "Transaction timeout between offset 0x%08x \
-			and 0x%08x (= 0x%08x + %d) HSFC=%x HSFS=%x!\n",
-			addr, addr + len - 1, addr, len - 1,
-			hsfc, hsfs);
-		return 1;
-	}
-
-	if (hsfs & HSFS_FCERR) {
-		uint16_t hsfc;
-		addr = readl_(&spi_bar->faddr) & SPIBAR_FADDR_MASK;
-		hsfc = readw_(&spi_bar->hsfc);
-		printk(BIOS_ERR, "Transaction error between offset 0x%08x and \
-		       0x%08x (= 0x%08x + %d) HSFC=%x HSFS=%x!\n",
-		       addr, addr + len - 1, addr, len - 1,
-		       hsfc, hsfs);
-		return 1;
-	}
-	return 0;
-}
-
 int pch_hwseq_erase(struct spi_flash *flash, u32 offset, size_t len)
 {
 	u32 start, end, erase_size;
 	int ret;
-	uint16_t hsfc;
-	uint32_t timeout = 5000 * 1000; /* 5 s for max 64 kB */
-	pch_spi_regs *spi_bar;
 
-	spi_bar = get_spi_bar();
 	erase_size = flash->sector_size;
 	if (offset % erase_size || len % erase_size) {
 		printk(BIOS_ERR, "SF: Erase offset/length not multiple of erase size\n");
@@ -206,27 +205,13 @@ int pch_hwseq_erase(struct spi_flash *flash, u32 offset, size_t len)
 	end = start + len;
 
 	while (offset < end) {
-		/*
-		 * Make sure FDONE, FCERR, AEL are
-		 * cleared by writing 1 to them.
-		 */
-		writew_(readw_(&spi_bar->hsfs), &spi_bar->hsfs);
-
-		pch_hwseq_set_addr(offset, spi_bar);
-
-		offset += erase_size;
-
-		hsfc = readw_(&spi_bar->hsfc);
-		hsfc &= ~HSFC_FCYCLE; /* clear operation */
-		hsfc |= HSFC_FCYCLE; /* set erase operation */
-		hsfc |= HSFC_FGO; /* start */
-		writew_(hsfc, &spi_bar->hsfc);
-		if (pch_hwseq_wait_for_cycle_complete(timeout, len, spi_bar)) {
-			printk(BIOS_ERR, "SF: Erase failed at %x\n",
-				offset - erase_size);
+		if (spi_do_operation(HSFC_FCYCLE_4KE, offset, 0, 5000)) {
+			printk(BIOS_ERR, "SF: Erase failed at %x\n", offset);
 			ret = -1;
 			goto out;
 		}
+
+		offset += erase_size;
 	}
 
 	printk(BIOS_DEBUG, "SF: Successfully erased %zu bytes @ %#x\n",
@@ -237,11 +222,14 @@ out:
 	return ret;
 }
 
-static void pch_read_data(uint8_t *data, int len, pch_spi_regs *spi_bar)
+static void pch_read_data(uint8_t *data, int len)
 {
 	int i;
+	pch_spi_regs *spi_bar;
 	uint32_t temp32 = 0;
 
+	spi_bar = get_spi_bar();
+
 	for (i = 0; i < len; i++) {
 		if ((i % 4) == 0)
 			temp32 = readl_((uint8_t *)spi_bar->fdata + i);
@@ -249,16 +237,12 @@ static void pch_read_data(uint8_t *data, int len, pch_spi_regs *spi_bar)
 		data[i] = (temp32 >> ((i % 4) * 8)) & 0xff;
 	}
 }
-int pch_hwseq_read(struct spi_flash *flash,
-			  u32 addr, size_t len, void *buf)
+
+int pch_hwseq_read(struct spi_flash *flash, u32 addr, size_t len, void *buf)
 {
-	uint16_t hsfc;
-	uint16_t timeout = 100 * 60;  /* 6 mili secs timeout */
 	uint8_t block_len;
-	pch_spi_regs *spi_bar;
 
-	spi_bar = get_spi_bar();
-	if (addr + len > spi_get_flash_size(spi_bar)) {
+	if (addr + len > spi_get_flash_size(get_spi_bar())) {
 		printk(BIOS_ERR,
 			"Attempt to read %x-%x which is out of chip\n",
 			(unsigned) addr,
@@ -266,26 +250,18 @@ int pch_hwseq_read(struct spi_flash *flash,
 		return -1;
 	}
 
-	/* clear FDONE, FCERR, AEL by writing 1 to them (if they are set) */
-	writew_(readw_(&spi_bar->hsfs), &spi_bar->hsfs);
-
 	while (len > 0) {
-		block_len = min(len, sizeof(spi_bar->fdata));
+		const int timeout_ms = 6;
+
+		block_len = min(len, SPI_FDATA_BYTES);
 		if (block_len > (~addr & 0xff))
 			block_len = (~addr & 0xff) + 1;
-		pch_hwseq_set_addr(addr, spi_bar);
-		hsfc = readw_(&spi_bar->hsfc);
-		hsfc &= ~HSFC_FCYCLE; /* set read operation */
-		hsfc &= ~HSFC_FDBC; /* clear byte count */
-		/* set byte count */
-		hsfc |= (((block_len - 1) << HSFC_FDBC_SHIFT) & HSFC_FDBC);
-		hsfc |= HSFC_FGO; /* start */
-		writew_(hsfc, &spi_bar->hsfc);
-
-		if (pch_hwseq_wait_for_cycle_complete
-			(timeout, block_len, spi_bar))
+
+		if (spi_do_operation(HSFC_FCYCLE_RD, addr, block_len,
+					timeout_ms))
 			return -1;
-		pch_read_data(buf, block_len, spi_bar);
+
+		pch_read_data(buf, block_len);
 		addr += block_len;
 		buf += block_len;
 		len -= block_len;
@@ -326,8 +302,6 @@ static void pch_fill_data(const uint8_t *data, int len)
 int pch_hwseq_write(struct spi_flash *flash,
 			   u32 addr, size_t len, const void *buf)
 {
-	uint16_t hsfc;
-	uint16_t timeout = 100 * 60;   /* 6 mili secs  timeout */
 	uint8_t block_len;
 	uint32_t start = addr;
 	pch_spi_regs *spi_bar;
@@ -341,28 +315,16 @@ int pch_hwseq_write(struct spi_flash *flash,
 		return -1;
 	}
 
-	/* clear FDONE, FCERR, AEL by writing 1 to them (if they are set) */
-	writew_(readw_(&spi_bar->hsfs), &spi_bar->hsfs);
-
 	while (len > 0) {
+		const int timeout_ms = 6;
+
 		block_len = min(len, sizeof(spi_bar->fdata));
 		if (block_len > (~addr & 0xff))
 			block_len = (~addr & 0xff) + 1;
 
-		pch_hwseq_set_addr(addr, spi_bar);
-
 		pch_fill_data(buf, block_len);
-		hsfc = readw_(&spi_bar->hsfc);
-		hsfc &= ~HSFC_FCYCLE; /* clear operation */
-		hsfc |= HSFC_FCYCLE_WR; /* set write operation */
-		hsfc &= ~HSFC_FDBC; /* clear byte count */
-		/* set byte count */
-		hsfc |= (((block_len - 1) << HSFC_FDBC_SHIFT) & HSFC_FDBC);
-		hsfc |= HSFC_FGO; /* start */
-		writew_(hsfc, &spi_bar->hsfc);
-
-		if (pch_hwseq_wait_for_cycle_complete
-			(timeout, block_len, spi_bar)) {
+		if (spi_do_operation(HSFC_FCYCLE_WR, addr, block_len,
+					timeout_ms)) {
 			printk(BIOS_ERR, "SF: write failure at %x\n", addr);
 			return -1;
 		}
@@ -377,44 +339,21 @@ int pch_hwseq_write(struct spi_flash *flash,
 
 int pch_hwseq_read_status(struct spi_flash *flash, u8 *reg)
 {
-	uint16_t hsfc;
-	uint16_t timeout = 100 * 60;   /* 6 mili secs timeout */
-	uint8_t block_len = SPI_READ_STATUS_LENGTH;
-	pch_spi_regs *spi_bar;
+	size_t block_len = SPI_READ_STATUS_LENGTH;
+	const int timeout_ms = 6;
 
-	spi_bar = get_spi_bar();
-	/* clear FDONE, FCERR, AEL by writing 1 to them (if they are set) */
-	writew_(readw_(&spi_bar->hsfs), &spi_bar->hsfs);
-
-	hsfc = readw_(&spi_bar->hsfc);
-	hsfc &= ~HSFC_FCYCLE; /* set read operation */
-	/* read status register */
-	hsfc |= HSFC_FCYCLE_RS;
-	hsfc &= ~HSFC_FDBC; /* clear byte count */
-	/* set byte count */
-	hsfc |= (((block_len - 1) << HSFC_FDBC_SHIFT) & HSFC_FDBC);
-	hsfc |= HSFC_FGO; /* start */
-	writew_(hsfc, &spi_bar->hsfc);
-
-	if (pch_hwseq_wait_for_cycle_complete(timeout,
-			block_len, spi_bar))
+	if (spi_do_operation(HSFC_FCYCLE_RS, 0, block_len, timeout_ms))
 		return -1;
-	pch_read_data(reg, block_len, spi_bar);
-	/* clear read status register */
-	writew_(readw_(&spi_bar->hsfc) &
-			~HSFC_FCYCLE_RS, &spi_bar->hsfc);
+
+	pch_read_data(reg, block_len);
 
 	return 0;
 }
 
-#if !(ENV_ROMSTAGE)
 static struct spi_flash *spi_flash_hwseq_probe(struct spi_slave *spi)
 {
-	struct spi_flash *flash = NULL;
-	u32 berase;
-	pch_spi_regs *spi_bar;
+	struct spi_flash *flash;
 
-	spi_bar = get_spi_bar();
 	flash = malloc(sizeof(*flash));
 	if (!flash) {
 		printk(BIOS_WARNING, "SF: Failed to allocate memory\n");
@@ -428,31 +367,33 @@ static struct spi_flash *spi_flash_hwseq_probe(struct spi_slave *spi)
 	flash->erase = pch_hwseq_erase;
 	flash->read = pch_hwseq_read;
 	flash->status = pch_hwseq_read_status;
-	pch_hwseq_set_addr(0, spi_bar);
 
-	berase = ((readw_(&spi_bar->hsfs)) >> SPIBAR_HSFS_BERASE_OFFSET) &
-		  SPIBAR_HSFS_BERASE_MASK;
+	/* The hardware sequencing supports 4KiB or 64KiB erase. Use 4KiB. */
+	flash->sector_size = 4*KiB;
 
-	switch (berase) {
-	case 0:
-		flash->sector_size = 256;
-		break;
-	case 1:
-		flash->sector_size = 4096;
-		break;
-	case 2:
-		flash->sector_size = 8192;
-		break;
-	case 3:
-		flash->sector_size = 65536;
-		break;
+	flash->size = spi_get_flash_size(get_spi_bar());
+
+	return flash;
+}
+
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
+{
+	struct spi_slave *slave = malloc(sizeof(*slave));
+
+	if (!slave) {
+		printk(BIOS_DEBUG, "PCH SPI: Bad allocation\n");
+		return NULL;
 	}
 
-	flash->size = spi_get_flash_size(spi_bar);
+	memset(slave, 0, sizeof(*slave));
+
+	slave->bus = bus;
+	slave->cs = cs;
+	slave->force_programmer_specific = 1;
+	slave->programmer_specific_probe = spi_flash_hwseq_probe;
 
-	return flash;
+	return slave;
 }
-#endif
 
 #if ENV_RAMSTAGE
 /*
diff --git a/src/soc/intel/skylake/include/soc/flash_controller.h b/src/soc/intel/skylake/include/soc/flash_controller.h
index 25cbce9d1f..91693e3505 100644
--- a/src/soc/intel/skylake/include/soc/flash_controller.h
+++ b/src/soc/intel/skylake/include/soc/flash_controller.h
@@ -121,22 +121,31 @@ static void writel_(u32 b, void *addr)
 	pci_write_config32(dev, reg, val)
 #endif /* ENV_SMM */
 
-#define HSFC_FCYCLE		(0x3 << HSFC_FCYCLE_SHIFT)
+#define HSFC_FCYCLE_MASK	(0xf << HSFC_FCYCLE_SHIFT)
+#define HSFC_FCYCLE_RD		(0x0 << HSFC_FCYCLE_SHIFT)
 #define HSFC_FCYCLE_WR		(0x2 << HSFC_FCYCLE_SHIFT)
+#define HSFC_FCYCLE_4KE		(0x3 << HSFC_FCYCLE_SHIFT)
+#define HSFC_FCYCLE_64KE	(0x4 << HSFC_FCYCLE_SHIFT)
+#define HSFC_FCYCLE_SFDP	(0x5 << HSFC_FCYCLE_SHIFT)
+#define HSFC_FCYCLE_JEDECID	(0x6 << HSFC_FCYCLE_SHIFT)
+#define HSFC_FCYCLE_WS		(0x7 << HSFC_FCYCLE_SHIFT)
 #define HSFC_FCYCLE_RS		(0x8 << HSFC_FCYCLE_SHIFT)
-#define HSFC_FDBC		(0x3f << HSFC_FDBC_SHIFT)
+#define HSFC_FDBC_MASK		(0x3f << HSFC_FDBC_SHIFT)
 
 #define SPI_READ_STATUS_LENGTH 1 /* Read Status Register 1 */
 
 #define WPSR_MASK_SRP0_BIT 0x80
 
+#define SPI_FDATA_REGS 16
+#define SPI_FDATA_BYTES (SPI_FDATA_REGS * sizeof(uint32_t))
+
 typedef struct pch_spi_regs {
 	uint32_t bfpr;
 	uint16_t hsfs;
 	uint16_t hsfc;
 	uint32_t faddr;
-	uint32_t _reserved0;
-	uint32_t fdata[16];
+	uint32_t dlock;
+	uint32_t fdata[SPI_FDATA_REGS];
 	uint32_t frap;
 	uint32_t freg[6];
 	uint32_t _reserved1[6];