Change in coreboot[master]: fsp_broadwell_de: Make sure SPI driver can be used across all stages

[Werner Zeh (Code Review)]

Werner Zeh has uploaded this change for review. ( https://review.coreboot.org/29594

Change subject: fsp_broadwell_de: Make sure SPI driver can be used across all stages
......................................................................

fsp_broadwell_de: Make sure SPI driver can be used across all stages

If the SPI driver is used in romstage after the cache memory has beed
migrated to DRAM by FSP, there is no access to the fixed structure for
cntlr and ichspi_lock variables as the cache, where they have been
allocated in by the compiler, is not usable anymore.
The right way to handle this case is using car_get_var_ptr() so that
one will always have the right memory location for the given variables.

Change-Id: I148acffbe9c93b23eb21e7e704b178c187895da1
Signed-off-by: Werner Zeh 
---
M src/soc/intel/fsp_broadwell_de/spi.c
1 file changed, 58 insertions(+), 41 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/29594/1

diff --git a/src/soc/intel/fsp_broadwell_de/spi.c b/src/soc/intel/fsp_broadwell_de/spi.c
index a778aa9..c6e9371 100644
--- a/src/soc/intel/fsp_broadwell_de/spi.c
+++ b/src/soc/intel/fsp_broadwell_de/spi.c
@@ -59,7 +59,7 @@

typedef struct spi_slave ich_spi_slave;

-static int ichspi_lock CAR_GLOBAL = 0;
+static int ichspi_lock_var CAR_GLOBAL = 0;

typedef struct ich9_spi_regs {
    uint32_t bfpr;
@@ -109,7 +109,7 @@
    uint32_t *bbar;
 } ich_spi_controller;

-static ich_spi_controller cntlr CAR_GLOBAL;
+static ich_spi_controller cntlr_var CAR_GLOBAL;

enum {
    SPIS_SCIP =		0x0001,
@@ -254,11 +254,13 @@
 {
    const uint32_t bbar_mask = 0x00ffff00;
    uint32_t ichspi_bbar;
+	ich_spi_controller *cntlr;

+	cntlr = car_get_var_ptr(&cntlr);
    minaddr &= bbar_mask;
-	ichspi_bbar = readl_(cntlr.bbar) & ~bbar_mask;
+	ichspi_bbar = readl_(cntlr->bbar) & ~bbar_mask;
    ichspi_bbar |= minaddr;
-	writel_(ichspi_bbar, cntlr.bbar);
+	writel_(ichspi_bbar, cntlr->bbar);
 }

void spi_init(void)
@@ -267,6 +269,8 @@
    uint32_t rcba; /* Root Complex Base Address */
    uint8_t bios_cntl;
    ich9_spi_regs *ich9_spi;
+	ich_spi_controller *cntlr;
+	int *ichspi_lock;

#if defined(__SIMPLE_DEVICE__)
    pci_devfn_t dev = PCI_DEV(0, 31, 0);
@@ -275,21 +279,23 @@
    dev = dev_find_slot(0, PCI_DEVFN(31, 0));
 #endif

+	cntlr = car_get_var_ptr(&cntlr_var);
+	ichspi_lock = car_get_var_ptr(&ichspi_lock_var);
    pci_read_config_dword(dev, 0xf0, &rcba);
    /* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable. */
    rcrb = (uint8_t *)(rcba & 0xffffc000);
    ich9_spi = (ich9_spi_regs *)(rcrb + 0x3800);
-	ichspi_lock = readw_(&ich9_spi->hsfs) & HSFS_FLOCKDN;
-	cntlr.opmenu = ich9_spi->opmenu;
-	cntlr.menubytes = sizeof(ich9_spi->opmenu);
-	cntlr.optype = &ich9_spi->optype;
-	cntlr.addr = &ich9_spi->faddr;
-	cntlr.data = (uint8_t *)ich9_spi->fdata;
-	cntlr.databytes = sizeof(ich9_spi->fdata);
-	cntlr.status = &ich9_spi->ssfs;
-	cntlr.control = (uint16_t *)ich9_spi->ssfc;
-	cntlr.bbar = &ich9_spi->bbar;
-	cntlr.preop = &ich9_spi->preop;
+	*ichspi_lock = readw_(&ich9_spi->hsfs) & HSFS_FLOCKDN;
+	cntlr->opmenu = ich9_spi->opmenu;
+	cntlr->menubytes = sizeof(ich9_spi->opmenu);
+	cntlr->optype = &ich9_spi->optype;
+	cntlr->addr = &ich9_spi->faddr;
+	cntlr->data = (uint8_t *)ich9_spi->fdata;
+	cntlr->databytes = sizeof(ich9_spi->fdata);
+	cntlr->status = &ich9_spi->ssfs;
+	cntlr->control = (uint16_t *)ich9_spi->ssfc;
+	cntlr->bbar = &ich9_spi->bbar;
+	cntlr->preop = &ich9_spi->preop;
    ich_set_bbar(0);

/* Disable the BIOS write protect so write commands are allowed. */
@@ -357,40 +363,44 @@
 static int spi_setup_opcode(spi_transaction *trans)
 {
    uint16_t optypes;
-	uint8_t opmenu[cntlr.menubytes];
+	ich_spi_controller *cntlr;
+	int *ichspi_lock;

+	cntlr = car_get_var_ptr(&cntlr_var);
+	ichspi_lock = car_get_var_ptr(&ichspi_lock_var);
    trans->opcode = trans->out[0];
    spi_use_out(trans, 1);
-	if (!ichspi_lock) {
+	if (!(*ichspi_lock)) {
    	/* The lock is off, so just use index 0. */
-		writeb_(trans->opcode, cntlr.opmenu);
-		optypes = readw_(cntlr.optype);
+		writeb_(trans->opcode, cntlr->opmenu);
+		optypes = readw_(cntlr->optype);
    	optypes = (optypes & 0xfffc) | (trans->type & 0x3);
-		writew_(optypes, cntlr.optype);
+		writew_(optypes, cntlr->optype);
    	return 0;
    } else {
    	/* The lock is on. See if what we need is on the menu. */
    	uint8_t optype;
    	uint16_t opcode_index;
+		uint8_t opmenu[cntlr->menubytes];

/* Write Enable is handled as atomic prefix */
    	if (trans->opcode == SPI_OPCODE_WREN)
    		return 0;

-		read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
-		for (opcode_index = 0; opcode_index < cntlr.menubytes;
+		read_reg(cntlr->opmenu, opmenu, sizeof(opmenu));
+		for (opcode_index = 0; opcode_index < cntlr->menubytes;
    			opcode_index++) {
    		if (opmenu[opcode_index] == trans->opcode)
    			break;
    	}

-		if (opcode_index == cntlr.menubytes) {
+		if (opcode_index == cntlr->menubytes) {
    		printk(BIOS_DEBUG, "ICH SPI: Opcode %x not found\n",
    			trans->opcode);
    		return -1;
    	}

-		optypes = readw_(cntlr.optype);
+		optypes = readw_(cntlr->optype);
    	optype = (optypes >> (opcode_index * 2)) & 0x3;
    	if (trans->type == SPI_OPCODE_TYPE_WRITE_NO_ADDRESS &&
    		optype == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS &&
@@ -438,12 +448,15 @@
 {
    int timeout = 40000; /* This will result in 400 ms */
    uint16_t status = 0;
+	ich_spi_controller *cntlr;
+
+	cntlr = car_get_var_ptr(&cntlr_var);

while (timeout--) {
-		status = readw_(cntlr.status);
+		status = readw_(cntlr->status);
    	if (wait_til_set ^ ((status & bitmask) == 0)) {
    		if (wait_til_set)
-				writew_((status & bitmask), cntlr.status);
+				writew_((status & bitmask), cntlr->status);
    		return status;
    	}
    	udelay(10);
@@ -461,6 +474,8 @@
    int16_t opcode_index;
    int with_address;
    int status;
+	int *ichspi_lock;
+	ich_spi_controller *cntlr;

spi_transaction trans = {
    	dout, bytesout,
@@ -482,7 +497,8 @@
    if (ich_status_poll(SPIS_SCIP, 0) == -1)
    	return -1;

-	writew_(SPIS_CDS | SPIS_FCERR, cntlr.status);
+	cntlr = car_get_var_ptr(&cntlr_var);
+	writew_(SPIS_CDS | SPIS_FCERR, cntlr->status);

spi_setup_type(&trans);
    opcode_index = spi_setup_opcode(&trans);
@@ -492,13 +508,14 @@
    if (with_address < 0)
    	return -1;

-	if (!ichspi_lock && trans.opcode == SPI_OPCODE_WREN) {
+	ichspi_lock = car_get_var_ptr(&ichspi_lock_var);
+	if (!(*ichspi_lock) && trans.opcode == SPI_OPCODE_WREN) {
    	/*
    	 * Treat Write Enable as Atomic Pre-Op if possible
    	 * in order to prevent the Management Engine from
    	 * issuing a transaction between WREN and DATA.
    	 */
-		writew_(trans.opcode, cntlr.preop);
+		writew_(trans.opcode, cntlr->preop);
    	return 0;
    }

@@ -506,13 +523,13 @@
    control = SPIC_SCGO | ((opcode_index & 0x07) << 4);

/* Issue atomic preop cycle if needed */
-	if (readw_(cntlr.preop))
+	if (readw_(cntlr->preop))
    	control |= SPIC_ACS;

if (!trans.bytesout && !trans.bytesin) {
    	/* SPI addresses are 24 bit only */
    	if (with_address)
-			writel_(trans.offset & 0x00FFFFFF, cntlr.addr);
+			writel_(trans.offset & 0x00FFFFFF, cntlr->addr);

/*
    	 * This is a 'no data' command (like Write Enable), its
@@ -520,7 +537,7 @@
    	 * spi_setup_opcode() above. Tell the chip to send the
    	 * command.
    	 */
-		writew_(control, cntlr.control);
+		writew_(control, cntlr->control);

/* wait for the result */
    	status = ich_status_poll(SPIS_CDS | SPIS_FCERR, 1);
@@ -542,7 +559,7 @@
     * and followed by other SPI commands, and this sequence is controlled
     * by the SPI chip driver.
     */
-	if (trans.bytesout > cntlr.databytes) {
+	if (trans.bytesout > cntlr->databytes) {
    	printk(BIOS_DEBUG, "ICH SPI: Too much to write. Does your SPI chip driver use"
    	     " spi_crop_chunk()?\n");
    	return -1;
@@ -556,28 +573,28 @@
    	uint32_t data_length;

/* SPI addresses are 24 bit only */
-		writel_(trans.offset & 0x00FFFFFF, cntlr.addr);
+		writel_(trans.offset & 0x00FFFFFF, cntlr->addr);

if (trans.bytesout)
-			data_length = min(trans.bytesout, cntlr.databytes);
+			data_length = min(trans.bytesout, cntlr->databytes);
    	else
-			data_length = min(trans.bytesin, cntlr.databytes);
+			data_length = min(trans.bytesin, cntlr->databytes);

/* Program data into FDATA0 to N */
    	if (trans.bytesout) {
-			write_reg(trans.out, cntlr.data, data_length);
+			write_reg(trans.out, cntlr->data, data_length);
    		spi_use_out(&trans, data_length);
    		if (with_address)
    			trans.offset += data_length;
    	}

/* Add proper control fields' values */
-		control &= ~((cntlr.databytes - 1) << 8);
+		control &= ~((cntlr->databytes - 1) << 8);
    	control |= SPIC_DS;
    	control |= (data_length - 1) << 8;

/* write it */
-		writew_(control, cntlr.control);
+		writew_(control, cntlr->control);

/* Wait for Cycle Done Status or Flash Cycle Error. */
    	status = ich_status_poll(SPIS_CDS | SPIS_FCERR, 1);
@@ -590,7 +607,7 @@
    	}

if (trans.bytesin) {
-			read_reg(cntlr.data, trans.in, data_length);
+			read_reg(cntlr->data, trans.in, data_length);
    		spi_use_in(&trans, data_length);
    		if (with_address)
    			trans.offset += data_length;
@@ -599,7 +616,7 @@

spi_xfer_exit:
    /* Clear atomic preop now that xfer is done */
-	writew_(0, cntlr.preop);
+	writew_(0, cntlr->preop);

return 0;
 }