Nico Huber would like Carl-Daniel Hailfinger to review this change.
Add timeout check to SPI bitbang bus request
Am 05.10.2010 22:42 schrieb Iain Paton:
>>> root@p7fe-64:~/flashrom# ./flashrom -p nicintel_spi -V
>>> flashrom v0.9.2-r1186 on Linux 2.6.35-dt (x86_64), built with libpci
>>> 3.1.4, GCC 4.4.3, little endian
>>> Initializing nicintel_spi programmer
>>> Found "Intel 82540EM Gigabit Ethernet Controller" (8086:100e, BDF
>>> 06:02.0).
>
> I have a few of these cards, so maybe I'll try a different one just to
> be sure there's nothing odd going on with this one.
Here is an updated timeout patch (no functional changes for
nicintel_spi). A year has passed, and I'm still trying to find someone
who can test and ack it.
GatoLoko: This new patch also addresses the SB600 timeout issue you were
seeing.
Change-Id: I304546c98441fbd2e5daccb798557d016336a505
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
---
M bitbang_spi.c
M mcp6x_spi.c
M nicintel_spi.c
M ogp_spi.c
M programmer.h
M sb600spi.c
6 files changed, 61 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/25097/1
diff --git a/bitbang_spi.c b/bitbang_spi.c
index 391fd32..a306e5f 100644
--- a/bitbang_spi.c
+++ b/bitbang_spi.c
@@ -51,10 +51,11 @@
return bitbang_spi_master->get_miso();
}
-static void bitbang_spi_request_bus(void)
+static int bitbang_spi_request_bus(void)
{
if (bitbang_spi_master->request_bus)
- bitbang_spi_master->request_bus();
+ return bitbang_spi_master->request_bus();
+ return 0;
}
static void bitbang_spi_release_bus(void)
@@ -99,7 +100,7 @@
register_spi_programmer(&spi_programmer_bitbang);
- /* FIXME: Run bitbang_spi_request_bus here or in programmer init? */
+ /* FIXME: Run bitbang_spi_request_bus here or per command? */
bitbang_spi_set_cs(1);
bitbang_spi_set_sck(0);
bitbang_spi_set_mosi(0);
@@ -144,13 +145,15 @@
static int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr)
{
- int i;
+ int i, ret;
/* FIXME: Run bitbang_spi_request_bus here or in programmer init?
* Requesting and releasing the SPI bus is handled in here to allow the
* programmer to use its own SPI engine for native accesses.
*/
- bitbang_spi_request_bus();
+ ret = bitbang_spi_request_bus();
+ if (ret)
+ return ret;
bitbang_spi_set_cs(0);
for (i = 0; i < writecnt; i++)
bitbang_spi_readwrite_byte(writearr[i]);
diff --git a/mcp6x_spi.c b/mcp6x_spi.c
index d2c31be..c30e36a 100644
--- a/mcp6x_spi.c
+++ b/mcp6x_spi.c
@@ -39,22 +39,38 @@
#define MCP6X_SPI_REQUEST 0
#define MCP6X_SPI_GRANT 8
+/* 1000000 loops are roughly 1 s. */
+#define MAX_REQUEST_LOOPS 1000000
+
void *mcp6x_spibar = NULL;
/* Cached value of last GPIO state. */
static uint8_t mcp_gpiostate;
-static void mcp6x_request_spibus(void)
+static void mcp6x_release_spibus(void);
+
+static int mcp6x_request_spibus(void)
{
+ int i = 0;
+
mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530);
mcp_gpiostate |= 1 << MCP6X_SPI_REQUEST;
mmio_writeb(mcp_gpiostate, mcp6x_spibar + 0x530);
/* Wait until we are allowed to use the SPI bus. */
- while (!(mmio_readw(mcp6x_spibar + 0x530) & (1 << MCP6X_SPI_GRANT))) ;
+ while (!(mmio_readw(mcp6x_spibar + 0x530) & (1 << MCP6X_SPI_GRANT))) {
+ if (i++ > MAX_REQUEST_LOOPS) {
+ /* Update the cache. */
+ mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530);
+ mcp6x_release_spibus();
+ msg_perr("%s: Timeout! Aborting.\n", __func__);
+ return TIMEOUT_ERROR;
+ }
+ }
/* Update the cache. */
mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530);
+ return 0;
}
static void mcp6x_release_spibus(void)
diff --git a/nicintel_spi.c b/nicintel_spi.c
index aacd68c..9f8631e 100644
--- a/nicintel_spi.c
+++ b/nicintel_spi.c
@@ -58,6 +58,9 @@
// #define FL_BUSY 30
// #define FL_ER 31
+/* 1000000 loops are roughly 1 s. */
+#define MAX_REQUEST_LOOPS 1000000
+
uint8_t *nicintel_spibar;
const struct pcidev_status nics_intel_spi[] = {
@@ -69,16 +72,27 @@
{},
};
-static void nicintel_request_spibus(void)
+static void nicintel_release_spibus(void);
+
+static int nicintel_request_spibus(void)
{
uint32_t tmp;
+ int i = 0;
tmp = pci_mmio_readl(nicintel_spibar + FLA);
tmp |= 1 << FL_REQ;
pci_mmio_writel(tmp, nicintel_spibar + FLA);
/* Wait until we are allowed to use the SPI bus. */
- while (!(pci_mmio_readl(nicintel_spibar + FLA) & (1 << FL_GNT))) ;
+ while (!(pci_mmio_readl(nicintel_spibar + FLA) & (1 << FL_GNT))) {
+ if (i++ > MAX_REQUEST_LOOPS) {
+ nicintel_release_spibus();
+ msg_perr("%s: Timeout! Aborting.\n", __func__);
+ return TIMEOUT_ERROR;
+ }
+ }
+
+ return 0;
}
static void nicintel_release_spibus(void)
diff --git a/ogp_spi.c b/ogp_spi.c
index dbaa57a..aa9d737 100644
--- a/ogp_spi.c
+++ b/ogp_spi.c
@@ -50,9 +50,13 @@
{},
};
-static void ogp_request_spibus(void)
+static int ogp_request_spibus(void)
{
+ /* FIXME: Do we have to check if any other SPI accesses are running
+ * right now? Can the request fail?
+ */
pci_mmio_writel(1, ogp_spibar + ogp_reg_sel);
+ return 0;
}
static void ogp_release_spibus(void)
diff --git a/programmer.h b/programmer.h
index b16e4f8..59c7d13 100644
--- a/programmer.h
+++ b/programmer.h
@@ -131,7 +131,7 @@
void (*set_sck) (int val);
void (*set_mosi) (int val);
int (*get_miso) (void);
- void (*request_bus) (void);
+ int (*request_bus) (void);
void (*release_bus) (void);
};
diff --git a/sb600spi.c b/sb600spi.c
index 9d82b47..7049924 100644
--- a/sb600spi.c
+++ b/sb600spi.c
@@ -80,12 +80,19 @@
return ret;
}
-static void execute_command(void)
+static int execute_command(void)
{
+ int timeout = 10000000;
+
mmio_writeb(mmio_readb(sb600_spibar + 2) | 1, sb600_spibar + 2);
- while (mmio_readb(sb600_spibar + 2) & 1)
+ while ((mmio_readb(sb600_spibar + 2) & 1) && --timeout)
;
+ if (!timeout) {
+ msg_perr("SB600: execute_command timeout!\n");
+ return TIMEOUT_ERROR;
+ }
+ return 0;
}
static int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt,
@@ -96,6 +103,7 @@
unsigned char cmd = *writearr++;
unsigned int readoffby1;
unsigned char readwrite;
+ int ret;
writecnt--;
@@ -144,7 +152,9 @@
return SPI_PROGRAMMER_ERROR;
msg_pspew("Executing: \n");
- execute_command();
+ ret = execute_command();
+ if (ret)
+ return ret;
/*
* After the command executed, we should find out the index of the
To view, visit change 25097. To unsubscribe, or for help writing mail filters, visit settings.