[M] Change in flashrom[master]: tree/: Replace NULL-case of programmer_delay() with internal_delay - flashrom-gerrit

17 Oct 2022

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68434 )
Change subject: tree/: Replace NULL-case of programmer_delay() with internal_delay
......................................................................
tree/: Replace NULL-case of programmer_delay() with internal_delay
Replace `programmer_delay(NULL, [..])` calls with direct
`internal_delay([..])` dispatches explicitly. Custom driver
delays remain hooked as well as core flashrom logic. The
NULL base case of 'programmer_delay()' then becomes a condition
to validate for layering violations or invalid flash contexts.
Change-Id: I1da230804d5e8f47a6e281feb66f381514dc6861
Signed-off-by: Edward O'Callaghan quasisec@google.com
---
M amd_imc.c
M atavia.c
M bitbang_spi.c
M dediprog.c
M dummyflasher.c
M flashrom.c
M ichspi.c
M it87spi.c
M nicintel_eeprom.c
M pony_spi.c
M raiden_debug_spi.c
M wbsio_spi.c
12 files changed, 60 insertions(+), 32 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/68434/1

diff --git a/amd_imc.c b/amd_imc.c
index b11eca3..2ee8332 100644
--- a/amd_imc.c
+++ b/amd_imc.c
@@ -60,7 +60,7 @@
    		msg_pwarn("IMC MBOX: Timeout!\n");
    		return 1;
    	}
-		programmer_delay(NULL, 1000);
+		internal_delay(1000);
    }
    return 0;
 }
diff --git a/atavia.c b/atavia.c
index b392999..33f6067 100644
--- a/atavia.c
+++ b/atavia.c
@@ -90,7 +90,7 @@
    		ready = true;
    		break;
    	} else {
-			programmer_delay(NULL, 1);
+			internal_delay(1);
    		continue;
    	}
    }
@@ -170,7 +170,7 @@
/* Test if a flash chip is attached. */
    pci_write_long(dev, PCI_ROM_ADDRESS, (uint32_t)PCI_ROM_ADDRESS_MASK);
-	programmer_delay(NULL, 90);
+	internal_delay(90);
    uint32_t base = pci_read_long(dev, PCI_ROM_ADDRESS);
    msg_pdbg2("BROM base=0x%08x\n", base);
    if ((base & PCI_ROM_ADDRESS_MASK) == 0) {
diff --git a/bitbang_spi.c b/bitbang_spi.c
index 10c1087..bd7bf1a 100644
--- a/bitbang_spi.c
+++ b/bitbang_spi.c
@@ -76,10 +76,10 @@
    		bitbang_spi_set_sck_set_mosi(master, 0, 0, spi_data);
    	else
    		bitbang_spi_set_sck(master, 0, spi_data);
-		programmer_delay(NULL, master->half_period);
+		internal_delay(master->half_period);
    	ret <<= 1;
    	ret |= bitbang_spi_set_sck_get_miso(master, 1, spi_data);
-		programmer_delay(NULL, master->half_period);
+		internal_delay(master->half_period);
    }
    return ret;
 }
@@ -90,9 +90,9 @@
for (i = 7; i >= 0; i--) {
    	bitbang_spi_set_sck_set_mosi(master, 0, (val >> i) & 1, spi_data);
-		programmer_delay(NULL, master->half_period);
+		internal_delay(master->half_period);
    	bitbang_spi_set_sck(master, 1, spi_data);
-		programmer_delay(NULL, master->half_period);
+		internal_delay(master->half_period);
    }
 }
@@ -122,9 +122,9 @@
    	readarr[i] = bitbang_spi_read_byte(master, data->spi_data);
bitbang_spi_set_sck(master, 0, data->spi_data);
-	programmer_delay(NULL, master->half_period);
+	internal_delay(master->half_period);
    bitbang_spi_set_cs(master, 1, data->spi_data);
-	programmer_delay(NULL, master->half_period);
+	internal_delay(master->half_period);
    /* FIXME: Run bitbang_spi_release_bus here or in programmer init? */
    bitbang_spi_release_bus(master, data->spi_data);
diff --git a/dediprog.c b/dediprog.c
index b4279d8..6c41f28 100644
--- a/dediprog.c
+++ b/dediprog.c
@@ -316,7 +316,7 @@
if (voltage_selector == 0) {
    	/* Wait some time as the original driver does. */
-		programmer_delay(NULL, 200 * 1000);
+		internal_delay(200 * 1000);
    }
    ret = dediprog_write(dediprog_handle, CMD_SET_VCC, voltage_selector, 0, NULL, 0);
    if (ret != 0x0) {
@@ -326,7 +326,7 @@
    }
    if (voltage_selector != 0) {
    	/* Wait some time as the original driver does. */
-		programmer_delay(NULL, 200 * 1000);
+		internal_delay(200 * 1000);
    }
    return 0;
 }
diff --git a/dummyflasher.c b/dummyflasher.c
index 110a88c..8a3f089 100644
--- a/dummyflasher.c
+++ b/dummyflasher.c
@@ -897,7 +897,7 @@
    	msg_pspew(" 0x%02x", readarr[i]);
    msg_pspew("\n");
-	programmer_delay(NULL, (writecnt + readcnt) * emu_data->delay_us);
+	internal_delay((writecnt + readcnt) * emu_data->delay_us);
    return 0;
 }
diff --git a/flashrom.c b/flashrom.c
index 47096d8..3218d9e 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -258,8 +258,20 @@
    if (usecs == 0)
    	return;
-	if (!flash)
+	/**
+	 * Drivers should either use internal_delay() directly or their
+	 * own custom delay. Only core flashrom logic calls programmer_delay()
+	 * which should always have a valid flash context. A NULL context
+	 * more than likely indicates a layering violation or BUG however
+	 * for now dispatch a internal_delay() as a safe default for the NULL
+	 * base case.
+	 */
+	if (!flash) {
+		msg_perr("%s called with NULL flash context. "
+			 "Please report a bug at flashrom@flashrom.org\n",
+			 __func__);
    	return internal_delay(usecs);
+	}
if (flash->mst->buses_supported & BUS_SPI) {
    	if (flash->mst->spi.delay)
diff --git a/ichspi.c b/ichspi.c
index 6d3535c..14c0dd8 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -875,7 +875,7 @@
timeout = 100 * 60;	/* 60 ms are 9.6 million cycles at 16 MHz. */
    while ((REGREAD16(ICH7_REG_SPIS) & SPIS_SCIP) && --timeout) {
-		programmer_delay(NULL, 10);
+		internal_delay(10);
    }
    if (!timeout) {
    	msg_perr("Error: SCIP never cleared!\n");
@@ -951,7 +951,7 @@
    /* Wait for Cycle Done Status or Flash Cycle Error. */
    while (((REGREAD16(ICH7_REG_SPIS) & (SPIS_CDS | SPIS_FCERR)) == 0) &&
           --timeout) {
-		programmer_delay(NULL, 10);
+		internal_delay(10);
    }
    if (!timeout) {
    	msg_perr("timeout, ICH7_REG_SPIS=0x%04x\n", REGREAD16(ICH7_REG_SPIS));
@@ -991,7 +991,7 @@
timeout = 100 * 60;	/* 60 ms are 9.6 million cycles at 16 MHz. */
    while ((REGREAD8(swseq_data.reg_ssfsc) & SSFS_SCIP) && --timeout) {
-		programmer_delay(NULL, 10);
+		internal_delay(10);
    }
    if (!timeout) {
    	msg_perr("Error: SCIP never cleared!\n");
@@ -1071,7 +1071,7 @@
    /* Wait for Cycle Done Status or Flash Cycle Error. */
    while (((REGREAD32(swseq_data.reg_ssfsc) & (SSFS_FDONE | SSFS_FCERR)) == 0) &&
           --timeout) {
-		programmer_delay(NULL, 10);
+		internal_delay(10);
    }
    if (!timeout) {
    	msg_perr("timeout, REG_SSFS=0x%08x\n", REGREAD32(swseq_data.reg_ssfsc));
@@ -1319,7 +1319,7 @@
    while ((((hsfs = REGREAD16(ICH9_REG_HSFS)) &
    	 (HSFS_FDONE | HSFS_FCERR)) == 0) &&
           --timeout_us) {
-		programmer_delay(NULL, 8);
+		internal_delay(8);
    }
    REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS));
    if (!timeout_us) {
diff --git a/it87spi.c b/it87spi.c
index a022041..fc29e51 100644
--- a/it87spi.c
+++ b/it87spi.c
@@ -146,7 +146,7 @@
    	if((status & SPI_SR_WIP) == 0)
    		return 0;
-		programmer_delay(NULL, 1000);
+		internal_delay(1000);
    }
    return 0;
 }
diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c
index d93f8e0..3078684 100644
--- a/nicintel_eeprom.c
+++ b/nicintel_eeprom.c
@@ -213,7 +213,7 @@
    eewr |= BIT(EEWR_CMDV);
    pci_mmio_writel(eewr, eebar + EEWR);
-	programmer_delay(NULL, 5);
+	internal_delay(5);
    int i;
    for (i = 0; i < MAX_ATTEMPTS; i++)
    	if (pci_mmio_readl(eebar + EEWR) & BIT(EEWR_DONE))
@@ -338,7 +338,7 @@
    	nicintel_ee_bitbang(eebar, 0x00, &rdsr);
nicintel_ee_bitset(eebar, EEC, EE_CS, 1);
-		programmer_delay(NULL, 1);
+		internal_delay(1);
    	if (!(rdsr & SPI_SR_WIP)) {
    		return 0;
    	}
@@ -379,7 +379,7 @@
    	nicintel_ee_bitset(eebar, EEC, EE_CS, 0);
    	nicintel_ee_bitbang(eebar, JEDEC_WREN, NULL);
    	nicintel_ee_bitset(eebar, EEC, EE_CS, 1);
-		programmer_delay(NULL, 1);
+		internal_delay(1);
/* data */
    	nicintel_ee_bitset(eebar, EEC, EE_CS, 0);
@@ -394,7 +394,7 @@
    			break;
    	}
    	nicintel_ee_bitset(eebar, EEC, EE_CS, 1);
-		programmer_delay(NULL, 1);
+		internal_delay(1);
    	if (nicintel_ee_ready(eebar))
    		goto out;
    }
diff --git a/pony_spi.c b/pony_spi.c
index fc27fc4..408fa12 100644
--- a/pony_spi.c
+++ b/pony_spi.c
@@ -244,7 +244,7 @@
    	for (i = 1; i <= 10; i++) {
    		data_out = i & 1;
    		sp_set_pin(PIN_RTS, data_out);
-			programmer_delay(NULL, 1000);
+			internal_delay(1000);
/* If DSR does not change, we are not connected to what we think */
    		if (data_out != sp_get_pin(PIN_DSR)) {
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c
index fb068c8..80b660d 100644
--- a/raiden_debug_spi.c
+++ b/raiden_debug_spi.c
@@ -888,7 +888,7 @@
    			/* Reattempting will not result in a recovery. */
    			return status;
    		}
-			programmer_delay(NULL, RETRY_INTERVAL_US);
+			internal_delay(RETRY_INTERVAL_US);
    		continue;
    	}
@@ -923,7 +923,7 @@
    			/* Reattempting will not result in a recovery. */
    			return status;
    		}
-			programmer_delay(NULL, RETRY_INTERVAL_US);
+			internal_delay(RETRY_INTERVAL_US);
    	}
    }
@@ -958,7 +958,7 @@
    		         "    config attempt = %d\n"
    		         "    status         = 0x%05x\n",
    		         config_attempt + 1, status);
-			programmer_delay(NULL, RETRY_INTERVAL_US);
+			internal_delay(RETRY_INTERVAL_US);
    		continue;
    	}
@@ -968,7 +968,7 @@
    		         "    config attempt = %d\n"
    		         "    status         = 0x%05x\n",
    		         config_attempt + 1, status);
-			programmer_delay(NULL, RETRY_INTERVAL_US);
+			internal_delay(RETRY_INTERVAL_US);
    		continue;
    	}
@@ -1012,7 +1012,7 @@
    	         config_attempt + 1,
    	         rsp_config.packet_v2.packet_id,
    	         rsp_config.packet_size);
-		programmer_delay(NULL, RETRY_INTERVAL_US);
+		internal_delay(RETRY_INTERVAL_US);
    }
    return USB_SPI_HOST_INIT_FAILURE;
 }
@@ -1236,7 +1236,7 @@
    			/* Reattempting will not result in a recovery. */
    			return status;
    		}
-			programmer_delay(NULL, RETRY_INTERVAL_US);
+			internal_delay(RETRY_INTERVAL_US);
    		continue;
    	}
    	for (read_attempt = 0; read_attempt < READ_RETRY_ATTEMPTS;
@@ -1273,7 +1273,7 @@
    			}
    			/* Device needs to reset its transmit index. */
    			restart_response_v2(ctx_data);
-				programmer_delay(NULL, RETRY_INTERVAL_US);
+				internal_delay(RETRY_INTERVAL_US);
    		}
    	}
    }
diff --git a/wbsio_spi.c b/wbsio_spi.c
index 1aa8729..b643eb4 100644
--- a/wbsio_spi.c
+++ b/wbsio_spi.c
@@ -155,7 +155,7 @@
OUTB(writearr[0], data->spibase);
    OUTB(mode, data->spibase + 1);
-	programmer_delay(NULL, 10);
+	internal_delay(10);
if (!readcnt)
    	return 0;
-- 
To view, visit https://review.coreboot.org/c/flashrom/+/68434
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1da230804d5e8f47a6e281feb66f381514dc6861
Gerrit-Change-Number: 68434
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan quasisec@chromium.org
Gerrit-MessageType: newchange



    