Edward O'Callaghan has uploaded this change for review.
sb600spi.c: Don't access spibar directly
Constrain the manner in which the global state of spibar is
set and then subsequently accessed. This makes it easier to
write unit-tests and instrument the code.
Change-Id: Ic26372b9c1baebb20716eea1db1e942239ed3e48
Signed-off-by: Edward O'Callaghan <quasisec@chromium.org>
---
M sb600spi.c
1 file changed, 53 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/36433/1
diff --git a/sb600spi.c b/sb600spi.c
index 23b36ee..7bdd11f 100644
--- a/sb600spi.c
+++ b/sb600spi.c
@@ -40,7 +40,13 @@
*};
*/
-static uint8_t *sb600_spibar = NULL;
+static uint8_t *g_sb600_spibar = NULL;
+
+static inline uint8_t * get_spibar(void)
+{
+ return g_sb600_spibar;
+}
+
enum amd_chipset {
CHIPSET_AMD_UNKNOWN,
CHIPSET_SB6XX,
@@ -180,16 +186,18 @@
static void reset_internal_fifo_pointer(void)
{
- mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2);
+ uint8_t *spibar = get_spibar();
+ mmio_writeb(mmio_readb(spibar + 2) | 0x10, spibar + 2);
/* FIXME: This loop needs a timeout and a clearer message. */
- while (mmio_readb(sb600_spibar + 0xD) & 0x7)
+ while (mmio_readb(spibar + 0xD) & 0x7)
msg_pspew("reset\n");
}
static int compare_internal_fifo_pointer(uint8_t want)
{
- uint8_t have = mmio_readb(sb600_spibar + 0xd) & 0x07;
+ uint8_t *spibar = get_spibar();
+ uint8_t have = mmio_readb(spibar + 0xd) & 0x07;
want %= FIFO_SIZE_OLD;
if (have != want) {
msg_perr("AMD SPI FIFO pointer corruption! Pointer is %d, wanted %d\n", have, want);
@@ -224,8 +232,9 @@
static void execute_command(void)
{
msg_pspew("Executing... ");
- mmio_writeb(mmio_readb(sb600_spibar + 2) | 1, sb600_spibar + 2);
- while (mmio_readb(sb600_spibar + 2) & 1)
+ uint8_t *spibar = get_spibar();
+ mmio_writeb(mmio_readb(spibar + 2) | 1, spibar + 2);
+ while (mmio_readb(spibar + 2) & 1)
;
msg_pspew("done\n");
}
@@ -239,7 +248,8 @@
unsigned char cmd = *writearr++;
writecnt--;
msg_pspew("%s, cmd=0x%02x, writecnt=%d, readcnt=%d\n", __func__, cmd, writecnt, readcnt);
- mmio_writeb(cmd, sb600_spibar + 0);
+ uint8_t *spibar = get_spibar();
+ mmio_writeb(cmd, spibar + 0);
int ret = check_readwritecnt(flash, writecnt, readcnt);
if (ret != 0)
@@ -253,14 +263,14 @@
*/
unsigned int readoffby1 = (writecnt > 0) ? 0 : 1;
uint8_t readwrite = (readcnt + readoffby1) << 4 | (writecnt);
- mmio_writeb(readwrite, sb600_spibar + 1);
+ mmio_writeb(readwrite, spibar + 1);
reset_internal_fifo_pointer();
msg_pspew("Filling FIFO: ");
unsigned int count;
for (count = 0; count < writecnt; count++) {
msg_pspew("[%02x]", writearr[count]);
- mmio_writeb(writearr[count], sb600_spibar + 0xC);
+ mmio_writeb(writearr[count], spibar + 0xC);
}
msg_pspew("\n");
if (compare_internal_fifo_pointer(writecnt))
@@ -291,7 +301,7 @@
/* Skip the bytes we sent. */
msg_pspew("Skipping: ");
for (count = 0; count < writecnt; count++) {
- msg_pspew("[%02x]", mmio_readb(sb600_spibar + 0xC));
+ msg_pspew("[%02x]", mmio_readb(spibar + 0xC));
}
msg_pspew("\n");
if (compare_internal_fifo_pointer(writecnt))
@@ -299,14 +309,14 @@
msg_pspew("Reading FIFO: ");
for (count = 0; count < readcnt; count++) {
- readarr[count] = mmio_readb(sb600_spibar + 0xC);
+ readarr[count] = mmio_readb(spibar + 0xC);
msg_pspew("[%02x]", readarr[count]);
}
msg_pspew("\n");
if (compare_internal_fifo_pointer(writecnt+readcnt))
return SPI_PROGRAMMER_ERROR;
- if (mmio_readb(sb600_spibar + 1) != readwrite) {
+ if (mmio_readb(spibar + 1) != readwrite) {
msg_perr("Unexpected change in AMD SPI read/write count!\n");
msg_perr("Something else is accessing the flash chip and causes random corruption.\n"
"Please stop all applications and drivers and IPMI which access the flash chip.\n");
@@ -325,21 +335,22 @@
unsigned char cmd = *writearr++;
writecnt--;
msg_pspew("%s, cmd=0x%02x, writecnt=%d, readcnt=%d\n", __func__, cmd, writecnt, readcnt);
- mmio_writeb(cmd, sb600_spibar + 0);
+ uint8_t *spibar = get_spibar();
+ mmio_writeb(cmd, spibar + 0);
int ret = check_readwritecnt(flash, writecnt, readcnt);
if (ret != 0)
return ret;
/* Use the extended TxByteCount and RxByteCount registers. */
- mmio_writeb(writecnt, sb600_spibar + 0x48);
- mmio_writeb(readcnt, sb600_spibar + 0x4b);
+ mmio_writeb(writecnt, spibar + 0x48);
+ mmio_writeb(readcnt, spibar + 0x4b);
msg_pspew("Filling buffer: ");
unsigned int count;
for (count = 0; count < writecnt; count++) {
msg_pspew("[%02x]", writearr[count]);
- mmio_writeb(writearr[count], sb600_spibar + 0x80 + count);
+ mmio_writeb(writearr[count], spibar + 0x80 + count);
}
msg_pspew("\n");
@@ -347,7 +358,7 @@
msg_pspew("Reading buffer: ");
for (count = 0; count < readcnt; count++) {
- readarr[count] = mmio_readb(sb600_spibar + 0x80 + (writecnt + count) % FIFO_SIZE_YANGTZE);
+ readarr[count] = mmio_readb(spibar + 0x80 + (writecnt + count) % FIFO_SIZE_YANGTZE);
msg_pspew("[%02x]", readarr[count]);
}
msg_pspew("\n");
@@ -375,16 +386,17 @@
{
bool success = false;
uint8_t speed = spispeed->speed;
+ uint8_t *spibar = get_spibar();
msg_pdbg("Setting SPI clock to %s (0x%x).\n", spispeed->name, speed);
if (amd_gen >= CHIPSET_YANGTZE) {
- rmmio_writew((speed << 12) | (speed << 8) | (speed << 4) | speed, sb600_spibar + 0x22);
- uint16_t tmp = mmio_readw(sb600_spibar + 0x22);
+ rmmio_writew((speed << 12) | (speed << 8) | (speed << 4) | speed, spibar + 0x22);
+ uint16_t tmp = mmio_readw(spibar + 0x22);
success = (((tmp >> 12) & 0xf) == speed && ((tmp >> 8) & 0xf) == speed &&
((tmp >> 4) & 0xf) == speed && ((tmp >> 0) & 0xf) == speed);
} else {
- rmmio_writeb((mmio_readb(sb600_spibar + 0xd) & ~(0x3 << 4)) | (speed << 4), sb600_spibar + 0xd);
- success = (speed == ((mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3));
+ rmmio_writeb((mmio_readb(spibar + 0xd) & ~(0x3 << 4)) | (speed << 4), spibar + 0xd);
+ success = (speed == ((mmio_readb(spibar + 0xd) >> 4) & 0x3));
}
if (!success) {
@@ -396,11 +408,12 @@
static int set_mode(struct pci_dev *dev, uint8_t read_mode)
{
- uint32_t tmp = mmio_readl(sb600_spibar + 0x00);
+ uint8_t *spibar = get_spibar();
+ uint32_t tmp = mmio_readl(spibar + 0x00);
tmp &= ~(0x6 << 28 | 0x1 << 18); /* Clear mode bits */
tmp |= ((read_mode & 0x6) << 28) | ((read_mode & 0x1) << 18);
- rmmio_writel(tmp, sb600_spibar + 0x00);
- if (tmp != mmio_readl(sb600_spibar + 0x00))
+ rmmio_writel(tmp, spibar + 0x00);
+ if (tmp != mmio_readl(spibar + 0x00))
return 1;
return 0;
}
@@ -409,6 +422,7 @@
{
uint32_t tmp;
uint8_t spispeed_idx = 3; /* Default to 16.5 MHz */
+ uint8_t *spibar = get_spibar();
char *spispeed = extract_programmer_param("spispeed");
if (spispeed != NULL) {
@@ -450,7 +464,7 @@
"Normal (up to 66 MHz)", /* 6 */
"Fast Read", /* 7 (Not defined in the Bolton datasheet.) */
};
- tmp = mmio_readl(sb600_spibar + 0x00);
+ tmp = mmio_readl(spibar + 0x00);
uint8_t read_mode = ((tmp >> 28) & 0x6) | ((tmp >> 18) & 0x1);
msg_pdbg("SpiReadMode=%s (%i)\n", spireadmodes[read_mode], read_mode);
if (read_mode != 6) {
@@ -463,11 +477,11 @@
}
if (amd_gen >= CHIPSET_YANGTZE) {
- tmp = mmio_readb(sb600_spibar + 0x20);
+ tmp = mmio_readb(spibar + 0x20);
msg_pdbg("UseSpi100 is %sabled\n", (tmp & 0x1) ? "en" : "dis");
if ((tmp & 0x1) == 0) {
- rmmio_writeb(tmp | 0x1, sb600_spibar + 0x20);
- tmp = mmio_readb(sb600_spibar + 0x20) & 0x1;
+ rmmio_writeb(tmp | 0x1, spibar + 0x20);
+ tmp = mmio_readb(spibar + 0x20) & 0x1;
if (tmp == 0) {
msg_perr("Enabling Spi100 failed.\n");
return 1;
@@ -475,7 +489,7 @@
msg_pdbg("Enabling Spi100 succeeded.\n");
}
- tmp = mmio_readw(sb600_spibar + 0x22); /* SPI 100 Speed Config */
+ tmp = mmio_readw(spibar + 0x22); /* SPI 100 Speed Config */
msg_pdbg("NormSpeedNew is %s\n", spispeeds[(tmp >> 12) & 0xf].name);
msg_pdbg("FastSpeedNew is %s\n", spispeeds[(tmp >> 8) & 0xf].name);
msg_pdbg("AltSpeedNew is %s\n", spispeeds[(tmp >> 4) & 0xf].name);
@@ -483,15 +497,15 @@
}
} else {
if (amd_gen >= CHIPSET_SB89XX && amd_gen <= CHIPSET_HUDSON234) {
- bool fast_read = (mmio_readl(sb600_spibar + 0x00) >> 18) & 0x1;
+ bool fast_read = (mmio_readl(spibar + 0x00) >> 18) & 0x1;
msg_pdbg("Fast Reads are %sabled\n", fast_read ? "en" : "dis");
if (fast_read) {
msg_pdbg("Disabling them temporarily.\n");
- rmmio_writel(mmio_readl(sb600_spibar + 0x00) & ~(0x1 << 18),
- sb600_spibar + 0x00);
+ rmmio_writel(mmio_readl(spibar + 0x00) & ~(0x1 << 18),
+ spibar + 0x00);
}
}
- tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3;
+ tmp = (mmio_readb(spibar + 0xd) >> 4) & 0x3;
msg_pdbg("NormSpeed is %s\n", spispeeds[tmp].name);
}
return set_speed(dev, amd_gen, &spispeeds[spispeed_idx]);
@@ -581,14 +595,14 @@
return 0;
/* Physical memory has to be mapped at page (4k) boundaries. */
- sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000);
- if (sb600_spibar == ERROR_PTR)
+ g_sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000);
+ if (g_sb600_spibar == ERROR_PTR)
return ERROR_FATAL;
/* The low bits of the SPI base address are used as offset into
* the mapped page.
*/
- sb600_spibar += tmp & 0xfff;
+ g_sb600_spibar += tmp & 0xfff;
int amd_gen = determine_generation(dev);
if (amd_gen < 0)
@@ -649,7 +663,8 @@
*
* <1> see handle_speed
*/
- tmp = mmio_readl(sb600_spibar + 0x00);
+ uint8_t *spibar = get_spibar();
+ tmp = mmio_readl(spibar + 0x00);
msg_pdbg("(0x%08" PRIx32 ") SpiArbEnable=%i", tmp, (tmp >> 19) & 0x1);
if (amd_gen >= CHIPSET_YANGTZE)
msg_pdbg(", IllegalAccess=%i", (tmp >> 21) & 0x1);
@@ -679,7 +694,7 @@
}
if (amd_gen >= CHIPSET_SB89XX) {
- tmp = mmio_readb(sb600_spibar + 0x1D);
+ tmp = mmio_readb(spibar + 0x1D);
msg_pdbg("Using SPI_CS%d\n", tmp & 0x3);
/* FIXME: Handle SpiProtect* configuration on Yangtze. */
}
To view, visit change 36433. To unsubscribe, or for help writing mail filters, visit settings.