From 7ed1337309d3fe74f5af09520970f0f1d417399a Mon Sep 17 00:00:00 2001 From: Subrata Banik Date: Thu, 4 Aug 2022 13:56:48 +0530 Subject: ichspi: Factor out common hwseq_xfer logic into helpers List of changes: 1. Add a unified `execute SPI flash transfer` function that does: - Check the SCIP bit prior initiate new operation. - Start the transfer by setting address and length for transfer, finally set FGO bit. - Wait for the transaction to get completed/failed/timed out. 2. All HW Sequencing SPI operation uses `execute SPI flash transfer` function Note: The refactoring xfer logic here assumes setting `HSFC_FDBC to 0` while performing erase operation using `ich_hwseq_block_erase()`. But it does not impact the erase operations. BUG=b:223630977 TEST=Able to perform read-status/write-status/read/write/erase operation on PCH 600 series chipset (board name: google/kano). Signed-off-by: Subrata Banik Change-Id: Ic9fd50841449e02f476a8834f4642d6ecad36dc3 Reviewed-on: https://review.coreboot.org/c/flashrom/+/62869 Reviewed-by: Edward O'Callaghan Tested-by: build bot (Jenkins) --- ichspi.c | 145 ++++++++++++++++++++++++--------------------------------------- 1 file changed, 55 insertions(+), 90 deletions(-) diff --git a/ichspi.c b/ichspi.c index f5876b7bc..fa15a953e 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1268,7 +1268,7 @@ static struct hwseq_data *get_hwseq_data_from_context(const struct flashctx *fla return flash->mst->opaque.data; } -/* Sets FLA in FADDR to (addr & hwseq_data.addr_mask) without touching other bits. */ +/* Sets FLA in FADDR to (addr & hwseq_data->addr_mask) without touching other bits. */ static void ich_hwseq_set_addr(uint32_t addr, uint32_t mask) { uint32_t addr_old = REGREAD32(ICH9_REG_FADDR) & ~mask; @@ -1344,35 +1344,63 @@ static int ich_hwseq_wait_for_cycle_complete(unsigned int len, enum ich_chipset return 0; } -static int ich_hwseq_read_status(const struct flashctx *flash, enum flash_reg reg, uint8_t *value) +/* Fire up a transfer using the hardware sequencer. */ +static void ich_start_hwseq_xfer(uint32_t hsfc_cycle, uint32_t flash_addr, size_t len, + uint32_t addr_mask) { uint16_t hsfc; - const int len = 1; - const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); - if (reg != STATUS1) { - msg_perr("%s: only supports STATUS1\n", __func__); - return -1; - } - msg_pdbg("Reading Status register\n"); - ich_hwseq_set_addr(0, hwseq_data->addr_mask); + /* Sets flash_addr in FADDR */ + ich_hwseq_set_addr(flash_addr, addr_mask); - /* clear FDONE, FCERR, AEL by writing 1 to them (if they are set) */ + /* make sure FDONE, FCERR, AEL are cleared by writing 1 to them */ REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); + /* Set up transaction parameters. */ hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~hwseq_data->hsfc_fcycle; /* set read operation */ - - /* read status register */ - hsfc |= HSFC_CYCLE_RD_STATUS; - hsfc &= ~HSFC_FDBC; /* clear byte count */ - - /* set byte count */ + hsfc &= ~g_hwseq_data.hsfc_fcycle; /* clear operation */ + hsfc |= hsfc_cycle; hsfc |= HSFC_FDBC_VAL(len - 1); hsfc |= HSFC_FGO; /* start */ + prettyprint_ich9_reg_hsfc(hsfc, ich_generation); REGWRITE16(ICH9_REG_HSFC, hsfc); +} + +static int ich_wait_for_hwseq_spi_cycle_complete(void) +{ + if (REGREAD8(ICH9_REG_HSFS) & HSFS_SCIP) { + msg_perr("Error: SCIP bit is unexpectedly set.\n"); + return 1; + } + return 0; +} + +/* Execute SPI flash transfer */ +static int ich_exec_sync_hwseq_xfer(uint32_t hsfc_cycle, uint32_t flash_addr, + size_t len, enum ich_chipset ich_gen, uint32_t addr_mask) +{ + if (ich_wait_for_hwseq_spi_cycle_complete()) { + msg_perr("SPI Transaction Timeout due to previous operation in process!\n"); + return 1; + } + + ich_start_hwseq_xfer(hsfc_cycle, flash_addr, len, addr_mask); + return ich_hwseq_wait_for_cycle_complete(len, ich_gen, addr_mask); +} + +static int ich_hwseq_read_status(const struct flashctx *flash, enum flash_reg reg, uint8_t *value) +{ + const int len = 1; + const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); + + if (reg != STATUS1) { + msg_perr("%s: only supports STATUS1\n", __func__); + return -1; + } + msg_pdbg("Reading Status register\n"); - if (ich_hwseq_wait_for_cycle_complete(len, ich_generation, hwseq_data->addr_mask)) { + if (ich_exec_sync_hwseq_xfer(HSFC_CYCLE_RD_STATUS, 0, len, ich_generation, + hwseq_data->addr_mask)) { msg_perr("Reading Status register failed\n!!"); return -1; } @@ -1383,7 +1411,6 @@ static int ich_hwseq_read_status(const struct flashctx *flash, enum flash_reg re static int ich_hwseq_write_status(const struct flashctx *flash, enum flash_reg reg, uint8_t value) { - uint16_t hsfc; const int len = 1; const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); @@ -1392,25 +1419,11 @@ static int ich_hwseq_write_status(const struct flashctx *flash, enum flash_reg r return -1; } msg_pdbg("Writing status register\n"); - ich_hwseq_set_addr(0, hwseq_data->addr_mask); - - /* clear FDONE, FCERR, AEL by writing 1 to them (if they are set) */ - REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); ich_fill_data(&value, len, ICH9_REG_FDATA0); - hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~hwseq_data->hsfc_fcycle; /* clear operation */ - - /* write status register */ - hsfc |= HSFC_CYCLE_WR_STATUS; - hsfc &= ~HSFC_FDBC; /* clear byte count */ - /* set byte count */ - hsfc |= HSFC_FDBC_VAL(len - 1); - hsfc |= HSFC_FGO; /* start */ - REGWRITE16(ICH9_REG_HSFC, hsfc); - - if (ich_hwseq_wait_for_cycle_complete(len, ich_generation, hwseq_data->addr_mask)) { + if (ich_exec_sync_hwseq_xfer(HSFC_CYCLE_WR_STATUS, 0, len, ich_generation, + hwseq_data->addr_mask)) { msg_perr("Writing Status register failed\n!!"); return -1; } @@ -1478,7 +1491,6 @@ static int ich_hwseq_block_erase(struct flashctx *flash, unsigned int addr, unsigned int len) { uint32_t erase_block; - uint16_t hsfc; const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); erase_block = ich_hwseq_get_erase_block_size(addr, hwseq_data->addr_mask, hwseq_data->only_4k); @@ -1505,25 +1517,9 @@ static int ich_hwseq_block_erase(struct flashctx *flash, unsigned int addr, } msg_pdbg("Erasing %d bytes starting at 0x%06x.\n", len, addr); - ich_hwseq_set_addr(addr, hwseq_data->addr_mask); - /* make sure FDONE, FCERR, AEL are cleared by writing 1 to them */ - REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); - - if (REGREAD8(ICH9_REG_HSFS) & HSFS_SCIP) { - msg_perr("Error: SCIP bit is unexpectedly set.\n"); - return -1; - } - - hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~hwseq_data->hsfc_fcycle; /* clear operation */ - hsfc |= HSFC_CYCLE_BLOCK_ERASE; /* set erase operation */ - hsfc |= HSFC_FGO; /* start */ - msg_pdbg("HSFC used for block erasing: "); - prettyprint_ich9_reg_hsfc(hsfc, ich_generation); - REGWRITE16(ICH9_REG_HSFC, hsfc); - - if (ich_hwseq_wait_for_cycle_complete(len, ich_generation, hwseq_data->addr_mask)) + if (ich_exec_sync_hwseq_xfer(HSFC_CYCLE_BLOCK_ERASE, addr, 0, ich_generation, + hwseq_data->addr_mask)) return -1; return 0; } @@ -1531,7 +1527,6 @@ static int ich_hwseq_block_erase(struct flashctx *flash, unsigned int addr, static int ich_hwseq_read(struct flashctx *flash, uint8_t *buf, unsigned int addr, unsigned int len) { - uint16_t hsfc; uint8_t block_len; const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); @@ -1551,23 +1546,8 @@ static int ich_hwseq_read(struct flashctx *flash, uint8_t *buf, /* as well as flash chip page borders as demanded in the Intel datasheets. */ block_len = min(block_len, 256 - (addr & 0xFF)); - ich_hwseq_set_addr(addr, hwseq_data->addr_mask); - - if (REGREAD8(ICH9_REG_HSFS) & HSFS_SCIP) { - msg_perr("Error: SCIP bit is unexpectedly set.\n"); - return -1; - } - - hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~hwseq_data->hsfc_fcycle; /* set read operation */ - hsfc &= ~HSFC_FDBC; /* clear byte count */ - hsfc |= HSFC_CYCLE_READ; /* set read operation */ - /* set byte count */ - hsfc |= HSFC_FDBC_VAL(block_len - 1); - hsfc |= HSFC_FGO; /* start */ - REGWRITE16(ICH9_REG_HSFC, hsfc); - - if (ich_hwseq_wait_for_cycle_complete(block_len, ich_generation, hwseq_data->addr_mask)) + if (ich_exec_sync_hwseq_xfer(HSFC_CYCLE_READ, addr, block_len, ich_generation, + hwseq_data->addr_mask)) return 1; ich_read_data(buf, block_len, ICH9_REG_FDATA0); addr += block_len; @@ -1579,7 +1559,6 @@ static int ich_hwseq_read(struct flashctx *flash, uint8_t *buf, static int ich_hwseq_write(struct flashctx *flash, const uint8_t *buf, unsigned int addr, unsigned int len) { - uint16_t hsfc; uint8_t block_len; const struct hwseq_data *hwseq_data = get_hwseq_data_from_context(flash); @@ -1594,28 +1573,14 @@ static int ich_hwseq_write(struct flashctx *flash, const uint8_t *buf, unsigned REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); while (len > 0) { - ich_hwseq_set_addr(addr, hwseq_data->addr_mask); /* Obey programmer limit... */ block_len = min(len, flash->mst->opaque.max_data_write); /* as well as flash chip page borders as demanded in the Intel datasheets. */ block_len = min(block_len, 256 - (addr & 0xFF)); ich_fill_data(buf, block_len, ICH9_REG_FDATA0); - if (REGREAD8(ICH9_REG_HSFS) & HSFS_SCIP) { - msg_perr("Error: SCIP bit is unexpectedly set.\n"); - return -1; - } - - hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~hwseq_data->hsfc_fcycle; /* clear operation */ - hsfc |= HSFC_CYCLE_WRITE; /* set write operation */ - hsfc &= ~HSFC_FDBC; /* clear byte count */ - /* set byte count */ - hsfc |= HSFC_FDBC_VAL(block_len - 1); - hsfc |= HSFC_FGO; /* start */ - REGWRITE16(ICH9_REG_HSFC, hsfc); - - if (ich_hwseq_wait_for_cycle_complete(block_len, ich_generation, hwseq_data->addr_mask)) + if (ich_exec_sync_hwseq_xfer(HSFC_CYCLE_WRITE, addr, block_len, ich_generation, + hwseq_data->addr_mask)) return -1; addr += block_len; buf += block_len; -- cgit v1.2.3