<div dir="ltr">Attached is a log from flashrom -VVV -p internal -o /tmp/flashrom.log. Please let me know what else you want to collect.<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Jul 28, 2013 at 3:38 PM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net" target="_blank">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Am <a href="tel:26.07.2013%2023" value="+12607201323">26.07.2013 23</a>:31 schrieb Stefan Tauner:<br>
<div><div class="h5">> On Fri, 26 Jul 2013 12:28:48 -0700<br>
> Wei Hu <<a href="mailto:wei@aristanetworks.com">wei@aristanetworks.com</a>> wrote:<br>
><br>
>> On Fri, Jul 26, 2013 at 12:18 AM, Carl-Daniel Hailfinger<br>
>> <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>> wrote:<br>
>>>> I'll give you modified patch a test tomorrow.<br>
>>> Please note that I expect problems with my patch. Not all of the FIFO<br>
>>> management has been changed, and this might be the cause of error<br>
>>> messages. Updated patch with more debugging at the end of this mail.<br>
>> After changing the device ID to match 0x780e, your patch works for<br>
>> both reading and writing. Writing is extremely slow though. I feel<br>
>> like on this new FCH we should ditch the SB600 interface and<br>
>> investigate the new programming interface. My current patch is more<br>
>> like a band aid workaround.<br>
> I plan to do that but for now it would be better than nothing, *but*<br>
> this will break Hudson-2... I talked to Martin Roth from sage and he<br>
> confirmed that although AMD changed PCI IDs with Hudson-1 the<br>
> interface did only really change with Kabini/Temash.<br>
><br>
> That explains also why Hudson-2 worked fine previously:<br>
> <a href="http://marc.info/?l=flashrom&m=131853263731000" target="_blank">http://marc.info/?l=flashrom&m=131853263731000</a><br>
> Wang Qing Pei has tested his Hudson patch too probably before sending<br>
> it to us but I don't know which system he used exactly.<br>
><br>
> I'll try to get the Hudson-2 datasheets, but ATM I don't have them.<br>
> We need a way to distinguish Kabini from the rest... since it is a SoC,<br>
> we could match the pci ids of the root complex<br>
> (00:00.0 Host bridge [0600]: Advanced Micro Devices [AMD] Family 16h<br>
> Processor Root Complex [1022:1536] in my case), but we would need to<br>
> maintain that for future models... Martin told me that they just read<br>
> if the new registers are (non-)0xff and infer from that if they need to<br>
> use the new interface.<br>
<br>
</div></div>New patch. Better heuristic (Martin's variant) for Kabini, more debug<br>
prints.<br>
The big challenge is to check whether this breaks Hudson variants before<br>
Kabini.<br>
<br>
Please get some -VVV logs for a simple probe. Thanks!<br>
# flashrom -o logfile.txt -VVV<br>
<div class="im"><br>
Signed-off-by: Carl-Daniel Hailfinger <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>><br>
<br>
Index: flashrom-kabini/sb600spi.c<br>
===================================================================<br>
--- flashrom-kabini/sb600spi.c  (Revision 1704)<br>
+++ flashrom-kabini/sb600spi.c  (Arbeitskopie)<br>
</div>@@ -4,7 +4,7 @@<br>
  * Copyright (C) 2008 Wang Qingpei <<a href="mailto:Qingpei.Wang@amd.com">Qingpei.Wang@amd.com</a>><br>
  * Copyright (C) 2008 Joe Bao <<a href="mailto:Zheng.Bao@amd.com">Zheng.Bao@amd.com</a>><br>
  * Copyright (C) 2008 Advanced Micro Devices, Inc.<br>
- * Copyright (C) 2009, 2010 Carl-Daniel Hailfinger<br>
+ * Copyright (C) 2009, 2010, 2013 Carl-Daniel Hailfinger<br>
  *<br>
  * This program is free software; you can redistribute it and/or modify<br>
  * it under the terms of the GNU General Public License as published by<br>
<div class="im">@@ -45,6 +45,8 @@<br>
<br>
 static uint8_t *sb600_spibar = NULL;<br>
<br>
</div>+static int kabini_method = 0;<br>
<div class="im">+<br>
 static void reset_internal_fifo_pointer(void)<br>
 {<br>
        mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2);<br>
@@ -58,14 +60,29 @@<br>
 {<br>
        uint8_t tmp;<br>
<br>
</div>+       if (kabini_method) {<br>
<div><div class="h5">+               uint8_t tmp1;<br>
+               mmio_writeb(7, sb600_spibar + 0x1e);<br>
+               tmp = mmio_readb(sb600_spibar + 0x1f);<br>
+               msg_pspew("Kabini SPIDataFifoPtr %d/%d, ", tmp, want);<br>
+               tmp = mmio_readb(sb600_spibar + 0xd) & 0x07;<br>
+               msg_pspew("FifoPtr %d/%d, ", tmp, want & 0x07);<br>
+               tmp = mmio_readb(sb600_spibar + 0x4d) & 0x7f;<br>
+               tmp1 = mmio_readb(sb600_spibar + 0x4e) & 0x7f;<br>
+               msg_pspew("FifoWrPtr/FifoRdPtr %d/%d\n", tmp, tmp1);<br>
+       }<br>
+<br>
+#ifdef USE_THE_EXTENDED_SPIDataFifoPtr_REGISTER<br>
+       mmio_writeb(7, sb600_spibar + 0x1e);<br>
+       tmp = mmio_readb(sb600_spibar + 0x1f);<br>
+#else  /* either way works */<br>
        tmp = mmio_readb(sb600_spibar + 0xd) & 0x07;<br>
        want &= 0x7;<br>
+#endif<br>
        if (want != tmp) {<br>
                msg_perr("FIFO pointer corruption! Pointer is %d, wanted %d\n", tmp, want);<br>
-               msg_perr("Something else is accessing the flash chip and "<br>
-                        "causes random corruption.\nPlease stop all "<br>
-                        "applications and drivers and IPMI which access the "<br>
-                        "flash chip.\n");<br>
+               msg_perr("Something else is accessing the flash chip and causes random corruption.\n"<br>
+                        "Please stop all applications and drivers and IPMI which access the flash chip.\n");<br>
                return 1;<br>
        } else {<br>
                msg_pspew("SB600 FIFO pointer is %d, wanted %d\n", tmp, want);<br>
@@ -99,7 +116,7 @@<br>
        /* First byte is cmd which can not being sent through FIFO. */<br>
        unsigned char cmd = *writearr++;<br>
        unsigned int readoffby1;<br>
-       unsigned char readwrite;<br>
+       unsigned char readwrite = 0;<br>
<br>
        writecnt--;<br>
<br>
@@ -118,15 +135,21 @@<br>
                return SPI_INVALID_LENGTH;<br>
        }<br>
<br>
-       /* This is a workaround for a bug in SB600 and SB700. If we only send<br>
-        * an opcode and no additional data/address, the SPI controller will<br>
-        * read one byte too few from the chip. Basically, the last byte of<br>
-        * the chip response is discarded and will not end up in the FIFO.<br>
-        * It is unclear if the CS# line is set high too early as well.<br>
-        */<br>
-       readoffby1 = (writecnt) ? 0 : 1;<br>
-       readwrite = (readcnt + readoffby1) << 4 | (writecnt);<br>
-       mmio_writeb(readwrite, sb600_spibar + 1);<br>
</div></div>+       if (kabini_method) {<br>
<div class="im">+               /* Use the extended TxByteCount and RxByteCount register. */<br>
+               mmio_writeb(writecnt, sb600_spibar + 0x48);<br>
+               mmio_writeb(readcnt, sb600_spibar + 0x4b);<br>
+       } else {<br>
+               /* This is a workaround for a bug in SB600 and SB700. If we only send<br>
+                * an opcode and no additional data/address, the SPI controller will<br>
+                * read one byte too few from the chip. Basically, the last byte of<br>
+                * the chip response is discarded and will not end up in the FIFO.<br>
+                * It is unclear if the CS# line is set high too early as well.<br>
+                */<br>
+               readoffby1 = (writecnt) ? 0 : 1;<br>
+               readwrite = (readcnt + readoffby1) << 4 | (writecnt);<br>
+               mmio_writeb(readwrite, sb600_spibar + 1);<br>
+       }<br>
        mmio_writeb(cmd, sb600_spibar + 0);<br>
<br>
        /* Before we use the FIFO, reset it first. */<br>
@@ -171,27 +194,58 @@<br>
                msg_pspew("[%02x]", cmd);<br>
        }<br>
        msg_pspew("\n");<br>
-       if (compare_internal_fifo_pointer(writecnt))<br>
-               return SPI_PROGRAMMER_ERROR;<br>
<br>
</div>+       /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Kabini, but it does so on all<br>
+        * chipsets before Kabini.<br>
+        */<br>
+       if (kabini_method) {<br>
<div class="im">+               if (compare_internal_fifo_pointer(0))<br>
+                       return SPI_PROGRAMMER_ERROR;<br>
+       } else {<br>
+               if (compare_internal_fifo_pointer(writecnt))<br>
+                       return SPI_PROGRAMMER_ERROR;<br>
+       }<br>
+<br>
        msg_pspew("Reading: ");<br>
        for (count = 0; count < readcnt; count++, readarr++) {<br>
                *readarr = mmio_readb(sb600_spibar + 0xC);<br>
                msg_pspew("[%02x]", *readarr);<br>
        }<br>
        msg_pspew("\n");<br>
-       if (reset_compare_internal_fifo_pointer(readcnt + writecnt))<br>
-               return SPI_PROGRAMMER_ERROR;<br>
<br>
-       if (mmio_readb(sb600_spibar + 1) != readwrite) {<br>
-               msg_perr("Unexpected change in SB600 read/write count!\n");<br>
-               msg_perr("Something else is accessing the flash chip and "<br>
-                        "causes random corruption.\nPlease stop all "<br>
-                        "applications and drivers and IPMI which access the "<br>
-                        "flash chip.\n");<br>
-               return SPI_PROGRAMMER_ERROR;<br>
</div>+       /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Kabini, but it does so on all<br>
+        * chipsets before Kabini.<br>
+        */<br>
+       if (kabini_method) {<br>
<div class="im">+               if (compare_internal_fifo_pointer(0))<br>
+                       return SPI_PROGRAMMER_ERROR;<br>
+       } else {<br>
+               if (reset_compare_internal_fifo_pointer(readcnt + writecnt))<br>
+                       return SPI_PROGRAMMER_ERROR;<br>
        }<br>
<br>
</div>+       if (kabini_method) {<br>
<div class="im">+               uint8_t tmp_wc = mmio_readb(sb600_spibar + 0x48);<br>
+               uint8_t tmp_rc = mmio_readb(sb600_spibar + 0x4b);<br>
+               if ((tmp_rc != readcnt) || (tmp_wc != writecnt)) {<br>
</div>+                       msg_perr("Unexpected change in Kabini read/write count: %i/%i instead of %i/%i!\n",<br>
<div class="im">+                                tmp_rc, tmp_wc, readcnt, writecnt);<br>
+                       msg_perr("Something else is accessing the flash chip and causes random corruption.\n"<br>
+                                "Please stop all applications and drivers and IPMI which access the flash "<br>
+                                "chip.\n");<br>
+                       return SPI_PROGRAMMER_ERROR;<br>
+               }<br>
+       } else {<br>
+               uint8_t tmp_rw = mmio_readb(sb600_spibar + 1);<br>
+               if (tmp_rw != readwrite) {<br>
+                       msg_perr("Unexpected change in SB600 read/write count: %i instead of %i!\n", tmp_rw,<br>
+                                readwrite);<br>
+                       msg_perr("Something else is accessing the flash chip and causes random corruption.\n"<br>
+                                "Please stop all applications and drivers and IPMI which access the flash "<br>
+                                "chip.\n");<br>
+                       return SPI_PROGRAMMER_ERROR;<br>
+               }<br>
+       }<br>
        return 0;<br>
 }<br>
<br>
</div>@@ -245,8 +299,8 @@<br>
        uint32_t tmp;<br>
        uint8_t reg;<br>
        bool amd_imc_force = false;<br>
-       static const char *const speed_names[4] = {<br>
-               "66/reserved", "33", "22", "16.5"<br>
+       static const char *const speed_names[8] = {<br>
+               "66 MHz", "33 MHz", "22 MHz", "16.6 MHz", "100 MHz", "Reserved", "Reserved", "800 kHz"<br>
        };<br>
<br>
        char *arg = extract_programmer_param("amd_imc_force");<br>
@@ -281,6 +335,30 @@<br>
         */<br>
        sb600_spibar += tmp & 0xfff;<br>
<br>
+       /* AMD Kabini has a different SPI interface. */<br>
+       kabini_method = 0;<br>
+       if (dev->device_id == 0x780e) {<br>
+               /* The PCI ID of the LPC bridge doesn't change between Hudson-2 and Kabini (Hudson-3) although<br>
+                * they use different SPI interfaces. ID 0x780e is common for all Hudson variants. This<br>
+                * heuristic accesses the SPI interface MMIO BAR at locations beyond those supported by<br>
+                * Hudson-2 and earlier in the hope of getting 0xff readback on older chipsets and non-0xff<br>
+                * readback on Kabini and newer chipsets.<br>
+                */<br>
+               int i;<br>
+               msg_pdbg("Checking for AMD Kabini or later... ");<br>
+               for (i = 0x20; i <= 0x4f; i++) {<br>
+                       if (mmio_readb(sb600_spibar + i) != 0xff) {<br>
+                               kabini_method = 1;<br>
+                               break;<br>
+                       }<br>
+               }<br>
+               if (kabini_method)<br>
+                       msg_pdbg("using AMD Kabini SPI access method.\n");<br>
+               else<br>
+                       msg_pdbg("not found.\n");<br>
+       }<br>
+<br>
+       /* FIXME: Check all accesses below for Kabini incompatibilities. */<br>
        tmp = pci_read_long(dev, 0xa0);<br>
        msg_pdbg("AltSpiCSEnable=%i, SpiRomEnable=%i, "<br>
                     "AbortEnable=%i\n", tmp & 0x1, (tmp & 0x2) >> 1,<br>
@@ -300,15 +378,28 @@<br>
         * SB700 or later, reads and writes will be corrupted. Abort in this<br>
         * case. Make sure to avoid this check on SB600.<br>
         */<br>
-       msg_pdbg("(0x%08" PRIx32 ") fastReadEnable=%u, SpiArbEnable=%i, SpiAccessMacRomEn=%i, "<br>
+       msg_pdbg("(0x%08" PRIx32 ") SpiReadMode/FastReadEnable=%i, SpiArbEnable=%i, SpiAccessMacRomEn=%i, "<br>
                     "SpiHostAccessRomEn=%i, ArbWaitCount=%i, "<br>
-                    "SpiBridgeDisable=%i, DropOneClkOnRd=%i\n",<br>
-                    tmp, (tmp >> 18) & 0x1,<br>
+                    "SpiBridgeDisable=%i, SpiClkGate/DropOneClkOnRd=%i\n",<br>
+                    tmp, ((tmp >> 28) & 0x6) | ((tmp >> 18) & 0x1),<br>
                     (tmp >> 19) & 0x1, (tmp >> 22) & 0x1,<br>
                     (tmp >> 23) & 0x1, (tmp >> 24) & 0x7,<br>
                     (tmp >> 27) & 0x1, (tmp >> 28) & 0x1);<br>
-       tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3;<br>
-       msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]);<br>
+       if (kabini_method) {<br>
+               tmp = mmio_readb(sb600_spibar + 0x20) & 0x1;<br>
+               msg_pdbg("UseSpi100 is %i (%s)\n", tmp, tmp ? "unsupported" : "OK");<br>
+               tmp = mmio_readw(sb600_spibar + 0x22);<br>
+               msg_pdbg("TpmSpeedNew is %s\n", speed_names[tmp & 0xf]);<br>
+               tmp >>= 4;<br>
+               msg_pdbg("AltSpeedNew is %s\n", speed_names[tmp & 0xf]);<br>
+               tmp >>= 4;<br>
+               msg_pdbg("FastSpeedNew is %s\n", speed_names[tmp & 0xf]);<br>
+               tmp >>= 4;<br>
+               msg_pdbg("NormSpeedNew is %s\n", speed_names[tmp & 0xf]);<br>
+       } else {<br>
+               tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3;<br>
+               msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]);<br>
+       }<br>
<br>
<div class="HOEnZb"><div class="h5">        /* Look for the SMBus device. */<br>
        smbus_dev = pci_dev_find(0x1002, 0x4385);<br>
<br>
--<br>
<a href="http://www.hailfinger.org/" target="_blank">http://www.hailfinger.org/</a><br>
<br>
</div></div></blockquote></div><br></div>