[flashrom] [PATCH 3/6] sbxxx: Cleanup.
Stefan Tauner
stefan.tauner at student.tuwien.ac.at
Sat Aug 10 03:45:54 CEST 2013
- Move programmer definition to the top.
- Rewrite array accesses to use indices instead of using pointer arithmetic.
- Move length check and opcode extraction to a function.
- Move IMC parameter handling into existing IMC handling function.
- Split comparing and resetting the FIFO pointer.
Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
---
sb600spi.c | 186 +++++++++++++++++++++++++++++--------------------------------
1 file changed, 88 insertions(+), 98 deletions(-)
diff --git a/sb600spi.c b/sb600spi.c
index 2c1d400..ff7dbee 100644
--- a/sb600spi.c
+++ b/sb600spi.c
@@ -54,6 +54,22 @@ enum amd_chipset {
};
static enum amd_chipset amd_gen = CHIPSET_AMD_UNKNOWN;
+#define FIFO_SIZE_OLD 8
+
+static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt,
+ const unsigned char *writearr, unsigned char *readarr);
+
+static struct spi_programmer spi_programmer_sb600 = {
+ .type = SPI_CONTROLLER_SB600,
+ .max_data_read = FIFO_SIZE_OLD,
+ .max_data_write = FIFO_SIZE_OLD - 3,
+ .command = sb600_spi_send_command,
+ .multicommand = default_spi_send_multicommand,
+ .read = default_spi_read,
+ .write_256 = default_spi_write_256,
+ .write_aai = default_spi_write_aai,
+};
+
static void determine_generation(struct pci_dev *dev)
{
amd_gen = CHIPSET_AMD_UNKNOWN;
@@ -133,38 +149,45 @@ static void reset_internal_fifo_pointer(void)
static int compare_internal_fifo_pointer(uint8_t want)
{
- uint8_t tmp;
-
- tmp = mmio_readb(sb600_spibar + 0xd) & 0x07;
- want &= 0x7;
- if (want != tmp) {
- msg_perr("FIFO pointer corruption! Pointer is %d, wanted %d\n", tmp, want);
- msg_perr("Something else is accessing the flash chip and "
- "causes random corruption.\nPlease stop all "
- "applications and drivers and IPMI which access the "
- "flash chip.\n");
+ uint8_t have = mmio_readb(sb600_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);
+ 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");
return 1;
} else {
- msg_pspew("SB600 FIFO pointer is %d, wanted %d\n", tmp, want);
+ msg_pspew("AMD SPI FIFO pointer is %d, wanted %d\n", have, want);
return 0;
}
}
-static int reset_compare_internal_fifo_pointer(uint8_t want)
+/* Check the number of bytes to be transmitted and extract opcode. */
+static int check_readwritecnt(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt)
{
- int ret;
+ unsigned int maxwritecnt = flash->pgm->spi.max_data_write + 3;
+ if (writecnt > maxwritecnt) {
+ msg_pinfo("%s: SPI controller can not send %d bytes, it is limited to %d bytes\n",
+ __func__, writecnt, maxwritecnt);
+ return SPI_INVALID_LENGTH;
+ }
- ret = compare_internal_fifo_pointer(want);
- reset_internal_fifo_pointer();
- return ret;
+ unsigned int maxreadcnt = flash->pgm->spi.max_data_read + 3;
+ if (readcnt > maxreadcnt) {
+ msg_pinfo("%s: SPI controller can not receive %d bytes, it is limited to %d bytes\n",
+ __func__, readcnt, maxreadcnt);
+ return SPI_INVALID_LENGTH;
+ }
+ return 0;
}
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)
;
+ msg_pspew("done\n");
}
static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt,
@@ -172,60 +195,45 @@ static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt,
const unsigned char *writearr,
unsigned char *readarr)
{
- int count;
- /* First byte is cmd which can not being sent through FIFO. */
+ /* First byte is cmd which can not be sent through the FIFO. */
unsigned char cmd = *writearr++;
- unsigned int readoffby1;
- unsigned char readwrite;
-
writecnt--;
+ msg_pspew("%s, cmd=0x%02x, writecnt=%d, readcnt=%d\n", __func__, cmd, writecnt, readcnt);
+ mmio_writeb(cmd, sb600_spibar + 0);
- msg_pspew("%s, cmd=%x, writecnt=%x, readcnt=%x\n",
- __func__, cmd, writecnt, readcnt);
-
- if (readcnt > 8) {
- msg_pinfo("%s, SB600 SPI controller can not receive %d bytes, "
- "it is limited to 8 bytes\n", __func__, readcnt);
- return SPI_INVALID_LENGTH;
- }
-
- if (writecnt > 8) {
- msg_pinfo("%s, SB600 SPI controller can not send %d bytes, "
- "it is limited to 8 bytes\n", __func__, writecnt);
- return SPI_INVALID_LENGTH;
- }
+ int ret = check_readwritecnt(flash, writecnt, readcnt);
+ if (ret != 0)
+ return ret;
- /* This is a workaround for a bug in SB600 and SB700. If we only send
+ /* This is a workaround for a bug in SPI controller. If we only send
* an opcode and no additional data/address, the SPI controller will
* read one byte too few from the chip. Basically, the last byte of
* the chip response is discarded and will not end up in the FIFO.
* It is unclear if the CS# line is set high too early as well.
*/
- readoffby1 = (writecnt) ? 0 : 1;
- readwrite = (readcnt + readoffby1) << 4 | (writecnt);
+ unsigned int readoffby1 = (writecnt > 0) ? 0 : 1;
+ uint8_t readwrite = (readcnt + readoffby1) << 4 | (writecnt);
mmio_writeb(readwrite, sb600_spibar + 1);
- mmio_writeb(cmd, sb600_spibar + 0);
- /* Before we use the FIFO, reset it first. */
reset_internal_fifo_pointer();
-
- /* Send the write byte to FIFO. */
- msg_pspew("Writing: ");
- for (count = 0; count < writecnt; count++, writearr++) {
- msg_pspew("[%02x]", *writearr);
- mmio_writeb(*writearr, sb600_spibar + 0xC);
+ msg_pspew("Filling FIFO: ");
+ int count;
+ for (count = 0; count < writecnt; count++) {
+ msg_pspew("[%02x]", writearr[count]);
+ mmio_writeb(writearr[count], sb600_spibar + 0xC);
}
msg_pspew("\n");
+ if (compare_internal_fifo_pointer(writecnt))
+ return SPI_PROGRAMMER_ERROR;
/*
- * We should send the data by sequence, which means we need to reset
+ * We should send the data in sequence, which means we need to reset
* the FIFO pointer to the first byte we want to send.
*/
- if (reset_compare_internal_fifo_pointer(writecnt))
- return SPI_PROGRAMMER_ERROR;
-
- msg_pspew("Executing: \n");
+ reset_internal_fifo_pointer();
execute_command();
+ if (compare_internal_fifo_pointer(writecnt + readcnt))
+ return SPI_PROGRAMMER_ERROR;
/*
* After the command executed, we should find out the index of the
@@ -238,34 +246,30 @@ static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt,
* the opcode, the FIFO already stores the response from the chip.
* Usually, the chip will respond with 0x00 or 0xff.
*/
- if (reset_compare_internal_fifo_pointer(writecnt + readcnt))
- return SPI_PROGRAMMER_ERROR;
+ reset_internal_fifo_pointer();
/* Skip the bytes we sent. */
msg_pspew("Skipping: ");
for (count = 0; count < writecnt; count++) {
- cmd = mmio_readb(sb600_spibar + 0xC);
- msg_pspew("[%02x]", cmd);
+ msg_pspew("[%02x]", mmio_readb(sb600_spibar + 0xC));
}
msg_pspew("\n");
if (compare_internal_fifo_pointer(writecnt))
return SPI_PROGRAMMER_ERROR;
- msg_pspew("Reading: ");
- for (count = 0; count < readcnt; count++, readarr++) {
- *readarr = mmio_readb(sb600_spibar + 0xC);
- msg_pspew("[%02x]", *readarr);
+ msg_pspew("Reading FIFO: ");
+ for (count = 0; count < readcnt; count++) {
+ readarr[count] = mmio_readb(sb600_spibar + 0xC);
+ msg_pspew("[%02x]", readarr[count]);
}
msg_pspew("\n");
- if (reset_compare_internal_fifo_pointer(readcnt + writecnt))
+ if (compare_internal_fifo_pointer(writecnt+readcnt))
return SPI_PROGRAMMER_ERROR;
if (mmio_readb(sb600_spibar + 1) != readwrite) {
- msg_perr("Unexpected change in SB600 read/write count!\n");
- msg_perr("Something else is accessing the flash chip and "
- "causes random corruption.\nPlease stop all "
- "applications and drivers and IPMI which access the "
- "flash chip.\n");
+ 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");
return SPI_PROGRAMMER_ERROR;
}
@@ -327,12 +331,28 @@ static int handle_speed(struct pci_dev *dev)
return set_speed(dev, &spispeeds[spispeed_idx]);
}
-static int sb600_handle_imc(struct pci_dev *dev, bool amd_imc_force)
+static int handle_imc(struct pci_dev *dev)
{
/* Handle IMC everywhere but sb600 which does not have one. */
if (amd_gen == CHIPSET_SB6XX)
return 0;
+ bool amd_imc_force = false;
+ char *arg = extract_programmer_param("amd_imc_force");
+ if (arg && !strcmp(arg, "yes")) {
+ amd_imc_force = true;
+ msg_pspew("amd_imc_force enabled.\n");
+ } else if (arg && !strlen(arg)) {
+ msg_perr("Missing argument for amd_imc_force.\n");
+ free(arg);
+ return 1;
+ } else if (arg) {
+ msg_perr("Unknown argument for amd_imc_force: \"%s\" (not \"yes\").\n", arg);
+ free(arg);
+ return 1;
+ }
+ free(arg);
+
/* TODO: we should not only look at IntegratedImcPresent (LPC Dev 20, Func 3, 40h) but also at
* IMCEnable(Strap) and Override EcEnable(Strap) (sb8xx, sb9xx?, a50: Misc_Reg: 80h-87h;
* sb7xx, sp5100: PM_Reg: B0h-B1h) etc. */
@@ -360,38 +380,11 @@ static int sb600_handle_imc(struct pci_dev *dev, bool amd_imc_force)
return amd_imc_shutdown(dev);
}
-static const struct spi_programmer spi_programmer_sb600 = {
- .type = SPI_CONTROLLER_SB600,
- .max_data_read = 8,
- .max_data_write = 5,
- .command = sb600_spi_send_command,
- .multicommand = default_spi_send_multicommand,
- .read = default_spi_read,
- .write_256 = default_spi_write_256,
- .write_aai = default_spi_write_aai,
-};
-
int sb600_probe_spi(struct pci_dev *dev)
{
struct pci_dev *smbus_dev;
uint32_t tmp;
uint8_t reg;
- bool amd_imc_force = false;
-
- char *arg = extract_programmer_param("amd_imc_force");
- if (arg && !strcmp(arg, "yes")) {
- amd_imc_force = true;
- msg_pspew("amd_imc_force enabled.\n");
- } else if (arg && !strlen(arg)) {
- msg_perr("Missing argument for amd_imc_force.\n");
- free(arg);
- return ERROR_FATAL;
- } else if (arg) {
- msg_perr("Unknown argument for amd_imc_force: \"%s\" (not \"yes\").\n", arg);
- free(arg);
- return ERROR_FATAL;
- }
- free(arg);
/* Read SPI_BaseAddr */
tmp = pci_read_long(dev, 0xa0);
@@ -540,12 +533,9 @@ int sb600_probe_spi(struct pci_dev *dev)
if (handle_speed(dev) != 0)
return ERROR_FATAL;
- if (sb600_handle_imc(dev, amd_imc_force) != 0)
+ if (handle_imc(dev) != 0)
return ERROR_FATAL;
- /* Bring the FIFO to a clean state. */
- reset_internal_fifo_pointer();
-
register_spi_programmer(&spi_programmer_sb600);
return 0;
}
--
Kind regards, Stefan Tauner
More information about the flashrom
mailing list