Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32879
Change subject: soc/fsp_baytrail: fix flashconsole on platform ......................................................................
soc/fsp_baytrail: fix flashconsole on platform
Include spi.c in romstage. Since FSP 1.0 can't use NO_CAR_GLOBAL_MIGRATION, adjust global variables in spi.c to use CAR_GLOBAL.
Adapted from early versions of CB:21107
Change-Id: I3487fb8ac317ce920bf1c3ef9d89590051932378 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/soc/intel/fsp_baytrail/Makefile.inc M src/soc/intel/fsp_baytrail/spi.c 2 files changed, 44 insertions(+), 39 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/32879/1
diff --git a/src/soc/intel/fsp_baytrail/Makefile.inc b/src/soc/intel/fsp_baytrail/Makefile.inc index 5ed635d..ca2b353 100644 --- a/src/soc/intel/fsp_baytrail/Makefile.inc +++ b/src/soc/intel/fsp_baytrail/Makefile.inc @@ -35,6 +35,7 @@ postcar-y += tsc_freq.c smm-$(CONFIG_HAVE_SMI_HANDLER) += tsc_freq.c ramstage-y += spi.c +romstage-y += spi.c smm-$(CONFIG_HAVE_SMI_HANDLER) += spi.c ramstage-y += chip.c ramstage-y += iosf.c diff --git a/src/soc/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c index 979ee57..9375d19 100644 --- a/src/soc/intel/fsp_baytrail/spi.c +++ b/src/soc/intel/fsp_baytrail/spi.c @@ -15,7 +15,7 @@ */
/* This file is derived from the flashrom project. */ - +#include <arch/early_variables.h> #include <stdint.h> #include <stdlib.h> #include <commonlib/helpers.h> @@ -33,7 +33,7 @@
typedef struct spi_slave ich_spi_slave;
-static int ichspi_lock = 0; +static int g_ichspi_lock CAR_GLOBAL = 0;
typedef struct ich9_spi_regs { uint32_t bfpr; @@ -81,7 +81,7 @@ uint16_t *control; } ich_spi_controller;
-static ich_spi_controller cntlr; +static ich_spi_controller g_cntlr CAR_GLOBAL;
enum { SPIS_SCIP = 0x0001, @@ -239,18 +239,19 @@
void spi_init(void) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); ich9_spi_regs *ich9_spi = spi_regs();
- 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.preop = &ich9_spi->preop; + car_set_var(g_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->preop = &ich9_spi->preop; }
typedef struct spi_transaction { @@ -311,17 +312,18 @@
static int spi_setup_opcode(spi_transaction *trans) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); uint16_t optypes; - uint8_t opmenu[cntlr.menubytes]; + uint8_t opmenu[cntlr->menubytes];
trans->opcode = trans->out[0]; spi_use_out(trans, 1); - if (!ichspi_lock) { + if (!car_get_var(g_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. */ @@ -332,20 +334,20 @@ 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 && @@ -391,14 +393,15 @@ */ static int ich_status_poll(uint16_t bitmask, int wait_til_set) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); int timeout = 40000; /* This will result in 400 ms */ uint16_t status = 0;
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); @@ -412,6 +415,7 @@ static int spi_ctrlr_xfer(const struct spi_slave *slave, const void *dout, size_t bytesout, void *din, size_t bytesin) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); uint16_t control; int16_t opcode_index; int with_address; @@ -437,7 +441,7 @@ if (ich_status_poll(SPIS_SCIP, 0) == -1) return -1;
- writew_(SPIS_CDS | SPIS_FCERR, cntlr.status); + writew_(SPIS_CDS | SPIS_FCERR, cntlr->status);
spi_setup_type(&trans); if ((opcode_index = spi_setup_opcode(&trans)) < 0) @@ -445,13 +449,13 @@ if ((with_address = spi_setup_offset(&trans)) < 0) return -1;
- if (!ichspi_lock && trans.opcode == SPI_OPCODE_WREN) { + if (!car_get_var(g_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; }
@@ -459,13 +463,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 @@ -473,7 +477,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); @@ -495,7 +499,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; @@ -509,28 +513,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); @@ -543,7 +547,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; @@ -552,7 +556,7 @@
spi_xfer_exit: /* Clear atomic preop now that xfer is done */ - writew_(0, cntlr.preop); + writew_(0, cntlr->preop);
return 0; }
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32879 )
Change subject: soc/fsp_baytrail: fix flashconsole on platform ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32879 )
Change subject: soc/fsp_baytrail: fix flashconsole on platform ......................................................................
soc/fsp_baytrail: fix flashconsole on platform
Include spi.c in romstage. Since FSP 1.0 can't use NO_CAR_GLOBAL_MIGRATION, adjust global variables in spi.c to use CAR_GLOBAL.
Adapted from early versions of CB:21107
Change-Id: I3487fb8ac317ce920bf1c3ef9d89590051932378 Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32879 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/fsp_baytrail/Makefile.inc M src/soc/intel/fsp_baytrail/spi.c 2 files changed, 44 insertions(+), 39 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/soc/intel/fsp_baytrail/Makefile.inc b/src/soc/intel/fsp_baytrail/Makefile.inc index 5ed635d..ca2b353 100644 --- a/src/soc/intel/fsp_baytrail/Makefile.inc +++ b/src/soc/intel/fsp_baytrail/Makefile.inc @@ -35,6 +35,7 @@ postcar-y += tsc_freq.c smm-$(CONFIG_HAVE_SMI_HANDLER) += tsc_freq.c ramstage-y += spi.c +romstage-y += spi.c smm-$(CONFIG_HAVE_SMI_HANDLER) += spi.c ramstage-y += chip.c ramstage-y += iosf.c diff --git a/src/soc/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c index 979ee57..9375d19 100644 --- a/src/soc/intel/fsp_baytrail/spi.c +++ b/src/soc/intel/fsp_baytrail/spi.c @@ -15,7 +15,7 @@ */
/* This file is derived from the flashrom project. */ - +#include <arch/early_variables.h> #include <stdint.h> #include <stdlib.h> #include <commonlib/helpers.h> @@ -33,7 +33,7 @@
typedef struct spi_slave ich_spi_slave;
-static int ichspi_lock = 0; +static int g_ichspi_lock CAR_GLOBAL = 0;
typedef struct ich9_spi_regs { uint32_t bfpr; @@ -81,7 +81,7 @@ uint16_t *control; } ich_spi_controller;
-static ich_spi_controller cntlr; +static ich_spi_controller g_cntlr CAR_GLOBAL;
enum { SPIS_SCIP = 0x0001, @@ -239,18 +239,19 @@
void spi_init(void) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); ich9_spi_regs *ich9_spi = spi_regs();
- 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.preop = &ich9_spi->preop; + car_set_var(g_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->preop = &ich9_spi->preop; }
typedef struct spi_transaction { @@ -311,17 +312,18 @@
static int spi_setup_opcode(spi_transaction *trans) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); uint16_t optypes; - uint8_t opmenu[cntlr.menubytes]; + uint8_t opmenu[cntlr->menubytes];
trans->opcode = trans->out[0]; spi_use_out(trans, 1); - if (!ichspi_lock) { + if (!car_get_var(g_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. */ @@ -332,20 +334,20 @@ 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 && @@ -391,14 +393,15 @@ */ static int ich_status_poll(uint16_t bitmask, int wait_til_set) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); int timeout = 40000; /* This will result in 400 ms */ uint16_t status = 0;
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); @@ -412,6 +415,7 @@ static int spi_ctrlr_xfer(const struct spi_slave *slave, const void *dout, size_t bytesout, void *din, size_t bytesin) { + ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); uint16_t control; int16_t opcode_index; int with_address; @@ -437,7 +441,7 @@ if (ich_status_poll(SPIS_SCIP, 0) == -1) return -1;
- writew_(SPIS_CDS | SPIS_FCERR, cntlr.status); + writew_(SPIS_CDS | SPIS_FCERR, cntlr->status);
spi_setup_type(&trans); if ((opcode_index = spi_setup_opcode(&trans)) < 0) @@ -445,13 +449,13 @@ if ((with_address = spi_setup_offset(&trans)) < 0) return -1;
- if (!ichspi_lock && trans.opcode == SPI_OPCODE_WREN) { + if (!car_get_var(g_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; }
@@ -459,13 +463,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 @@ -473,7 +477,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); @@ -495,7 +499,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; @@ -509,28 +513,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); @@ -543,7 +547,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; @@ -552,7 +556,7 @@
spi_xfer_exit: /* Clear atomic preop now that xfer is done */ - writew_(0, cntlr.preop); + writew_(0, cntlr->preop);
return 0; }
-
Kyösti Mälkki (Code Review) -
Matt DeVillier (Code Review)
-
Patrick Georgi (Code Review)