Re: [flashrom] A quick hack to support AMD Family 16h SOC

[Carl-Daniel Hailfinger]

Hello Wei Hu,

thanks for your patch!

Am 27.06.2013 02:34 schrieb Wei Hu:
> In the past I posted a question at
> http://www.flashrom.org/pipermail/flashrom/2013-April/010924.html
> about failure to run flashrom on the new AMD SOC. It turns out the SPI
> flash interface has introduced incompatible changes from SB600. I have
> created a quick hack and made it available at
> http://blogs.coreboot.org/blog/2013/06/26/gsoc-2013-flashrom-week-1-while1/.
> Based on the discussion there I'm posting the patch here as well. Hope
> it will be helpful.
>
> A quick hack to support AMD Family 16h SOC (Kabini).
> This patch is just a proof of concept and incompatible with previous generations of AMD chipset.
> Tested reading and writing on ASRock IMB-A180.
>
> Signed-off-by: Wei Hu

I have tried to change the patch in a way which keeps compatibility with
older AMD chipsets.
Please test with and without USE_THE_EXTENDED_SPIDataFifoPtr_REGISTER
#defined. If any errors crop up, please send them to the mailing list in
reply to this mail.

The biggest problem was the missing register reference guide (RRG) for
AMD FCH. I had to guess the way to identify affected chipsets, and I now
test for PCI device ID 0x438d (Hudson FCH, probably too broad match). I
really hope this is the right ID. It would be great if you could confirm
that the FIFO pointer management change affects all FCH with this PCI
device ID.

Is there a way to get access to the RRG for Kabini or the integrated
FCH? Right now my changes are guesses

Signed-off-by: Wei Hu 
Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-kabini/sb600spi.c
===================================================================
--- flashrom-kabini/sb600spi.c	(Revision 1704)
+++ flashrom-kabini/sb600spi.c	(Arbeitskopie)
@@ -45,6 +45,8 @@

static uint8_t *sb600_spibar = NULL;

+static int hudson_method = 0;
+
 static void reset_internal_fifo_pointer(void)
 {
    mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2);
@@ -58,14 +60,17 @@
 {
    uint8_t tmp;

+#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 +104,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 +123,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 (hudson_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 +182,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 Hudson, but it does so on all
+	 * chipsets before Hudson.
+	 */
+	if (hudson_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 Hudson, but it does so on all
+	 * chipsets before Hudson.
+	 */
+	if (hudson_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 (hudson_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 Hudson 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;
 }

@@ -310,6 +352,14 @@
    tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3;
    msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]);

+	/* AMD Hudson apparently requires use of a different SPI registers for read/write counts. */
+	if (dev->device_id == 0x438d) {
+		msg_pdbg("Using AMD Hudson SPI access method.\n");
+		hudson_method = 1;
+	} else {
+		hudson_method = 0;
+	}
+
    /* Look for the SMBus device. */
    smbus_dev = pci_dev_find(0x1002, 0x4385);