Anastasia Klimchuk submitted this change.

View Change



28 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
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 <subratabanik@google.com>
Change-Id: Ic9fd50841449e02f476a8834f4642d6ecad36dc3
Reviewed-on: https://review.coreboot.org/c/flashrom/+/62869
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M ichspi.c
1 file changed, 86 insertions(+), 90 deletions(-)

diff --git a/ichspi.c b/ichspi.c
index f5876b7..fa15a95 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1268,7 +1268,7 @@
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,9 +1344,52 @@
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;
+
+ /* Sets flash_addr in FADDR */
+ ich_hwseq_set_addr(flash_addr, addr_mask);
+
+ /* 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 &= ~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);

@@ -1355,24 +1398,9 @@
return -1;
}
msg_pdbg("Reading 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));
-
- 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 |= 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_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_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 @@
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 @@
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 @@
}

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_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 @@
/* 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_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 @@
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;

To view, visit change 62869. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic9fd50841449e02f476a8834f4642d6ecad36dc3
Gerrit-Change-Number: 62869
Gerrit-PatchSet: 30
Gerrit-Owner: Subrata Banik <subratabanik@google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev@google.com>
Gerrit-Reviewer: Thomas Heijligen <src@posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Felix Singer <felixsinger@posteo.net>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged