Am 30.07.2013 20:55 schrieb Wei Hu:
Attached is a log from flashrom -VVV -p internal -o /tmp/flashrom.log. Please let me know what else you want to collect.
Thanks!
On Sun, Jul 28, 2013 at 3:38 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 26.07.2013 23:31 schrieb Stefan Tauner:
On Fri, 26 Jul 2013 12:28:48 -0700 Wei Hu wei@aristanetworks.com wrote:
On Fri, Jul 26, 2013 at 12:18 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
I'll give you modified patch a test tomorrow.
Please note that I expect problems with my patch. Not all of the FIFO management has been changed, and this might be the cause of error messages. Updated patch with more debugging at the end of this mail.
After changing the device ID to match 0x780e, your patch works for both reading and writing. Writing is extremely slow though. I feel like on this new FCH we should ditch the SB600 interface and investigate the new programming interface. My current patch is more like a band aid workaround.
I plan to do that but for now it would be better than nothing, *but* this will break Hudson-2... I talked to Martin Roth from sage and he confirmed that although AMD changed PCI IDs with Hudson-1 the interface did only really change with Kabini/Temash.
That explains also why Hudson-2 worked fine previously: http://marc.info/?l=flashrom&m=131853263731000 Wang Qing Pei has tested his Hudson patch too probably before sending it to us but I don't know which system he used exactly.
I'll try to get the Hudson-2 datasheets, but ATM I don't have them. We need a way to distinguish Kabini from the rest... since it is a SoC, we could match the pci ids of the root complex (00:00.0 Host bridge [0600]: Advanced Micro Devices [AMD] Family 16h Processor Root Complex [1022:1536] in my case), but we would need to maintain that for future models... Martin told me that they just read if the new registers are (non-)0xff and infer from that if they need to use the new interface.
New patch. Better heuristic (Martin's variant) for Kabini, more debug prints. The big challenge is to check whether this breaks Hudson variants before Kabini.
Please get some -VVV logs for a simple probe. Thanks! # flashrom -o logfile.txt -VVV
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
This patch should work at normal speed and use the correct FIFO pointer registers on Kabini. The code is not exactly pretty, but functional and should (in theory) need no functional changes before commit.
Please check if read/write works at the expected speed. -VV is sufficient this time.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-kabini/sb600spi.c =================================================================== --- flashrom-kabini/sb600spi.c (Revision 1704) +++ flashrom-kabini/sb600spi.c (Arbeitskopie) @@ -4,7 +4,7 @@ * Copyright (C) 2008 Wang Qingpei Qingpei.Wang@amd.com * Copyright (C) 2008 Joe Bao Zheng.Bao@amd.com * Copyright (C) 2008 Advanced Micro Devices, Inc. - * Copyright (C) 2009, 2010 Carl-Daniel Hailfinger + * Copyright (C) 2009, 2010, 2013 Carl-Daniel Hailfinger * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -45,6 +45,8 @@
static uint8_t *sb600_spibar = NULL;
+static int kabini_method = 0; + static void reset_internal_fifo_pointer(void) { mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2); @@ -54,33 +56,67 @@ msg_pspew("reset\n"); }
-static int compare_internal_fifo_pointer(uint8_t want) +static int check_internal_fifo_pointer(uint8_t have, uint8_t want) { + 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; + } + msg_pspew("AMD SPI FIFO pointer is %d, wanted %d\n", have, want); + return 0; +} + +static int compare_internal_kabini_fifo_write_pointer(uint8_t want) +{ uint8_t tmp;
+ tmp = mmio_readb(sb600_spibar + 0x4d) & 0x7f; + return check_internal_fifo_pointer(tmp, want); +} + +static int compare_internal_kabini_fifo_read_pointer(uint8_t want) +{ + uint8_t tmp; + + tmp = mmio_readb(sb600_spibar + 0x4e) & 0x7f; + return check_internal_fifo_pointer(tmp, want); +} + +static int compare_internal_sb600_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"); - return 1; - } else { - msg_pspew("SB600 FIFO pointer is %d, wanted %d\n", tmp, want); - return 0; - } + return check_internal_fifo_pointer(tmp, want); }
+#if 0 static int reset_compare_internal_fifo_pointer(uint8_t want) { int ret;
+#if 0 + if (kabini_method) { + uint8_t tmp1; + mmio_writeb(7, sb600_spibar + 0x1e); + tmp = mmio_readb(sb600_spibar + 0x1f); + msg_pspew("Kabini SPIDataFifoPtr %d/%d, ", tmp, want); + tmp = mmio_readb(sb600_spibar + 0xd) & 0x07; + msg_pspew("FifoPtr %d/%d, ", tmp, want & 0x07); + tmp = mmio_readb(sb600_spibar + 0x4d) & 0x7f; + tmp1 = mmio_readb(sb600_spibar + 0x4e) & 0x7f; + msg_pspew("FifoWrPtr/FifoRdPtr %d/%d\n", tmp, tmp1); + } +#endif + ret = compare_internal_fifo_pointer(want); reset_internal_fifo_pointer(); return ret; } +#endif
static void execute_command(void) { @@ -99,7 +135,7 @@ /* First byte is cmd which can not being sent through FIFO. */ unsigned char cmd = *writearr++; unsigned int readoffby1; - unsigned char readwrite; + unsigned char readwrite = 0;
writecnt--;
@@ -118,15 +154,21 @@ return SPI_INVALID_LENGTH; }
- /* This is a workaround for a bug in SB600 and SB700. 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); - mmio_writeb(readwrite, sb600_spibar + 1); + if (kabini_method) { + /* Use the extended TxByteCount and RxByteCount register. */ + mmio_writeb(writecnt, sb600_spibar + 0x48); + mmio_writeb(readcnt, sb600_spibar + 0x4b); + } else { + /* This is a workaround for a bug in SB600 and SB700. 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); + mmio_writeb(readwrite, sb600_spibar + 1); + } mmio_writeb(cmd, sb600_spibar + 0);
/* Before we use the FIFO, reset it first. */ @@ -139,16 +181,29 @@ mmio_writeb(*writearr, sb600_spibar + 0xC); } msg_pspew("\n"); + if (kabini_method) { + if (compare_internal_kabini_fifo_write_pointer(writecnt)) + return SPI_PROGRAMMER_ERROR; + } else { + if (compare_internal_sb600_fifo_pointer(writecnt)) + return SPI_PROGRAMMER_ERROR; + }
/* * We should send the data by 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; + reset_internal_fifo_pointer();
msg_pspew("Executing: \n"); execute_command(); + if (kabini_method) { + if (compare_internal_kabini_fifo_write_pointer(writecnt + readcnt)) + return SPI_PROGRAMMER_ERROR; + } else { + if (compare_internal_sb600_fifo_pointer(writecnt + readcnt)) + return SPI_PROGRAMMER_ERROR; + }
/* * After the command executed, we should find out the index of the @@ -161,8 +216,7 @@ * 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: "); @@ -171,27 +225,53 @@ msg_pspew("[%02x]", cmd); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt)) - return SPI_PROGRAMMER_ERROR;
+ if (kabini_method) { + if (compare_internal_kabini_fifo_read_pointer(writecnt)) + return SPI_PROGRAMMER_ERROR; + } else { + if (compare_internal_sb600_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("\n"); - if (reset_compare_internal_fifo_pointer(readcnt + writecnt)) - 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"); - return SPI_PROGRAMMER_ERROR; + if (kabini_method) { + if (compare_internal_kabini_fifo_read_pointer(writecnt + readcnt)) + return SPI_PROGRAMMER_ERROR; + } else { + if (compare_internal_sb600_fifo_pointer(writecnt + readcnt)) + return SPI_PROGRAMMER_ERROR; } + reset_internal_fifo_pointer();
+ if (kabini_method) { + uint8_t tmp_wc = mmio_readb(sb600_spibar + 0x48); + uint8_t tmp_rc = mmio_readb(sb600_spibar + 0x4b); + if ((tmp_rc != readcnt) || (tmp_wc != writecnt)) { + msg_perr("Unexpected change in Kabini read/write count: %i/%i instead of %i/%i!\n", + tmp_rc, tmp_wc, readcnt, writecnt); + 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; + } + } else { + uint8_t tmp_rw = mmio_readb(sb600_spibar + 1); + if (tmp_rw != readwrite) { + msg_perr("Unexpected change in SB600 read/write count: %i instead of %i!\n", tmp_rw, + readwrite); + 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; + } + } return 0; }
@@ -245,8 +325,8 @@ uint32_t tmp; uint8_t reg; bool amd_imc_force = false; - static const char *const speed_names[4] = { - "66/reserved", "33", "22", "16.5" + static const char *const speed_names[8] = { + "66 MHz", "33 MHz", "22 MHz", "16.6 MHz", "100 MHz", "Reserved", "Reserved", "800 kHz" };
char *arg = extract_programmer_param("amd_imc_force"); @@ -281,6 +361,30 @@ */ sb600_spibar += tmp & 0xfff;
+ /* AMD Kabini has a different SPI interface. */ + kabini_method = 0; + if (dev->device_id == 0x780e) { + /* The PCI ID of the LPC bridge doesn't change between Hudson-2 and Kabini (Hudson-3) although + * they use different SPI interfaces. ID 0x780e is common for all Hudson variants. This + * heuristic accesses the SPI interface MMIO BAR at locations beyond those supported by + * Hudson-2 and earlier in the hope of getting 0xff readback on older chipsets and non-0xff + * readback on Kabini and newer chipsets. + */ + int i; + msg_pdbg("Checking for AMD Kabini or later... "); + for (i = 0x20; i <= 0x4f; i++) { + if (mmio_readb(sb600_spibar + i) != 0xff) { + kabini_method = 1; + break; + } + } + if (kabini_method) + msg_pdbg("using AMD Kabini SPI access method.\n"); + else + msg_pdbg("not found.\n"); + } + + /* FIXME: Check all accesses below for Kabini incompatibilities. */ tmp = pci_read_long(dev, 0xa0); msg_pdbg("AltSpiCSEnable=%i, SpiRomEnable=%i, " "AbortEnable=%i\n", tmp & 0x1, (tmp & 0x2) >> 1, @@ -300,15 +404,28 @@ * SB700 or later, reads and writes will be corrupted. Abort in this * case. Make sure to avoid this check on SB600. */ - msg_pdbg("(0x%08" PRIx32 ") fastReadEnable=%u, SpiArbEnable=%i, SpiAccessMacRomEn=%i, " + msg_pdbg("(0x%08" PRIx32 ") SpiReadMode/FastReadEnable=%i, SpiArbEnable=%i, SpiAccessMacRomEn=%i, " "SpiHostAccessRomEn=%i, ArbWaitCount=%i, " - "SpiBridgeDisable=%i, DropOneClkOnRd=%i\n", - tmp, (tmp >> 18) & 0x1, + "SpiBridgeDisable=%i, SpiClkGate/DropOneClkOnRd=%i\n", + tmp, ((tmp >> 28) & 0x6) | ((tmp >> 18) & 0x1), (tmp >> 19) & 0x1, (tmp >> 22) & 0x1, (tmp >> 23) & 0x1, (tmp >> 24) & 0x7, (tmp >> 27) & 0x1, (tmp >> 28) & 0x1); - tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3; - msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]); + if (kabini_method) { + tmp = mmio_readb(sb600_spibar + 0x20) & 0x1; + msg_pdbg("UseSpi100 is %i (%s)\n", tmp, tmp ? "unsupported" : "OK"); + tmp = mmio_readw(sb600_spibar + 0x22); + msg_pdbg("TpmSpeedNew is %s\n", speed_names[tmp & 0xf]); + tmp >>= 4; + msg_pdbg("AltSpeedNew is %s\n", speed_names[tmp & 0xf]); + tmp >>= 4; + msg_pdbg("FastSpeedNew is %s\n", speed_names[tmp & 0xf]); + tmp >>= 4; + msg_pdbg("NormSpeedNew is %s\n", speed_names[tmp & 0xf]); + } else { + tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3; + msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]); + }
/* Look for the SMBus device. */ smbus_dev = pci_dev_find(0x1002, 0x4385);