Attached is a log from flashrom -VVV -p internal -o /tmp/flashrom.log. Please let me know what else you want to collect.


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>

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);
@@ -58,14 +60,29 @@
 {
        uint8_t tmp;

+       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);
+       }
+
+#ifdef USE_THE_EXTENDED_SPIDataFifoPtr_REGISTER
+       mmio_writeb(7, sb600_spibar + 0x1e);
+       tmp = mmio_readb(sb600_spibar + 0x1f);
+#else  /* either way works */
        tmp = mmio_readb(sb600_spibar + 0xd) & 0x07;
        want &= 0x7;
+#endif
        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");
+               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);
@@ -99,7 +116,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 +135,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. */
@@ -171,27 +194,58 @@
                msg_pspew("[%02x]", cmd);
        }
        msg_pspew("\n");
-       if (compare_internal_fifo_pointer(writecnt))
-               return SPI_PROGRAMMER_ERROR;

+       /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Kabini, but it does so on all
+        * chipsets before Kabini.
+        */
+       if (kabini_method) {
+               if (compare_internal_fifo_pointer(0))
+                       return SPI_PROGRAMMER_ERROR;
+       } else {
+               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("\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;
+       /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Kabini, but it does so on all
+        * chipsets before Kabini.
+        */
+       if (kabini_method) {
+               if (compare_internal_fifo_pointer(0))
+                       return SPI_PROGRAMMER_ERROR;
+       } else {
+               if (reset_compare_internal_fifo_pointer(readcnt + writecnt))
+                       return SPI_PROGRAMMER_ERROR;
        }

+       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 +299,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 +335,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 +378,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);

--
http://www.hailfinger.org/