Patchwork A quick hack to support AMD Family 16h SOC

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2013-07-26 07:18:03
Message ID <51F222AB.4080003@gmx.net>
Download mbox | patch
Permalink /patch/3994/
State Superseded
Headers show

Comments

Carl-Daniel Hailfinger - 2013-07-26 07:18:03
Hi Wei,

thanks for the link. I didn't search for processor BKDG, only for
chipset BKDG. That's why I missed the Family 16h BKDG which includes the
SPI controller documentation.


Am 26.07.2013 07:18 schrieb Wei Hu:
> Have you checked the public BKDG
> http://support.amd.com/us/Processor_TechDocs/48751_BKDG_Fam_16h_Mod_00h-0Fh.pdf?
> It actually does talk about the SPI registers in Section 3.26.9.2.

Thanks.

> I'm not very familiar with AMD's code names, but I think Kabini's FCH
> is called Yangtze? Hudson seems to be previous generations.

Yes, I will have to fix this.


> 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.


> On Thu, Jul 25, 2013 at 7:21 PM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006@gmx.net> wrote:
>> 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 <wei@aristanetworks.com>
>> 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.

I still have to search/replace Hudson with Yangtze, but this patch will
help a lot finding out more about the various FIFO related registers.

Signed-off-by: Wei Hu <wei@aristanetworks.com>
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Tauner - 2013-07-26 13:12:17
On Fri, 26 Jul 2013 09:18:03 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> Hi Wei,
> 
> thanks for the link. I didn't search for processor BKDG, only for
> chipset BKDG. That's why I missed the Family 16h BKDG which includes the
> SPI controller documentation.
> 
> > On Thu, Jul 25, 2013 at 7:21 PM, Carl-Daniel Hailfinger
> > <c-d.hailfinger.devel.2006@gmx.net> wrote:
> >> Am 27.06.2013 02:34 schrieb Wei Hu:
> >>> 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 <wei@aristanetworks.com>
> >> 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.
> 
> I still have to search/replace Hudson with Yangtze, but this patch will
> help a lot finding out more about the various FIFO related registers.

The code names are complicated.

Jaguar describes the CPU(APU) architecture (succeeding Bobcat), while
Kabini (embedded, htpc, laptops) and Temash (tablets) describe SoC
families with those cores.

Hudson seems to be the generic code name(!?) for all FCH aka
southbridges since sb9xx which came to be when they started with their
fusion aka APU development, hence there are multiple generations of
Hudson (3 AFAIK, where Yangtze/Salton are #3).

The integrated FCH for Kabini and Temash are codenamed "Yangtze" and
"Salton" respectively. I am not aware of the previous code names...
The only other FCH name i could find is Yuba but that was never
released because it was cancelled together with Krishna/Wichita.

Martin Roth sent me a table matching Hudson generations with model
names a few weeks ago:

"FCH  Codename
A50M Hudson-M1
A45 Hudson-D1
A55E Hudson-E1
A60M Hudson-M2
A68M Hudson-M3L
A70M Hudson-M3
A55 Hudson-D2
A68 Hudson-D3L
A75 Hudson-D3
A85X Hudson-D4

The last number is the Hudson revision that you want."

Hope that explains at least a bit. No idea (yet) regarding PCI IDs.
Wei Hu - 2013-07-26 17:29:26
On Fri, Jul 26, 2013 at 6:12 AM, Stefan Tauner
<stefan.tauner@student.tuwien.ac.at> wrote:
> The code names are complicated.
>
> Jaguar describes the CPU(APU) architecture (succeeding Bobcat), while
> Kabini (embedded, htpc, laptops) and Temash (tablets) describe SoC
> families with those cores.
>
> Hudson seems to be the generic code name(!?) for all FCH aka
> southbridges since sb9xx which came to be when they started with their
> fusion aka APU development, hence there are multiple generations of
> Hudson (3 AFAIK, where Yangtze/Salton are #3).
>
> The integrated FCH for Kabini and Temash are codenamed "Yangtze" and
> "Salton" respectively. I am not aware of the previous code names...
> The only other FCH name i could find is Yuba but that was never
> released because it was cancelled together with Krishna/Wichita.

Is it right to call out Hudson for incompatible SPI changes though? We
might need to call it hudson3 or something. What I seem to see in
coreboot is they try to bundle compatible changes into hudson and for
incompatible changes just call it yangtze.
Stefan Tauner - 2013-07-26 17:41:34
On Fri, 26 Jul 2013 09:18:03 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> I still have to search/replace Hudson with Yangtze, but this patch will
> help a lot finding out more about the various FIFO related registers.

A45/A50M/A55E (aka Hudson-1) has the same id as sb7xx-sb9xx: 0x439d
(doc id 47777, page 301).

SB6xx is using 0x438d, which Carl-Daniel uses in his patch to test for
hudson... how come? I guess because the block diagram in figure 2 in
47777 shows this, but IMHO that's wrong. 

So what about Kabini aka Hudson-3? The BKDG (48751) lists 0x780e for
the LPC bridge, which is true on my ASRock A180-H too:
00:14.3 ISA bridge [0601]: Advanced Micro Devices [AMD] FCH LPC Bridge [1022:780e] (rev 11)
	Subsystem: ASRock Incorporation Device [1849:780e]

The problem is Hudson-2... no public data sheets. :/
Also interesting: http://pci-ids.ucw.cz/read/PC/1022/780e

I have not looked at the coreboot source yet... maybe there is stuff.

I assume that all Hudson-2 and newer are 0x780e and use the new
interface. I would therefore propose to use "FCH" as a name for
anything related to this class of spi controllers.
Stefan Tauner - 2013-07-26 18:02:31
On Fri, 26 Jul 2013 19:41:34 +0200
Stefan Tauner <stefan.tauner@student.tuwien.ac.at> wrote:

> I assume that all Hudson-2 and newer are 0x780e and use the new
> interface. I would therefore propose to use "FCH" as a name for
> anything related to this class of spi controllers.

Google search results seem to confirm that.
For example Lenovo Edge E525 has an A60M:
http://www.lenovo.com/shop/americas/content/pdf/system_data/e525_tech_specs.pdf
and has the FCH ID:
http://forums.opensuse.org/english/get-technical-help-here/laptop/470415-opensuse-12-1-lenovo-edge-e525.html#post2425420
Wei Hu - 2013-07-26 18:39:19
Stefan,

AMD internal documents also suggest A45/A50M/A55E FCH (Hudson-1) use
device ID 438D. Not sure if it's wrong since I don't have a test
system.

According to AMD document A55/A60M/A68/A68M/A70M/A75/A85X use device
ID 780E. There's also this A77E FCH (codename Bolton) which also uses
780E.

(Please cc me on your future emails since I'm not subscribed to the list)

On Fri, Jul 26, 2013 at 10:29 AM, Wei Hu <wei@aristanetworks.com> wrote:
> On Fri, Jul 26, 2013 at 6:12 AM, Stefan Tauner
> <stefan.tauner@student.tuwien.ac.at> wrote:
>> The code names are complicated.
>>
>> Jaguar describes the CPU(APU) architecture (succeeding Bobcat), while
>> Kabini (embedded, htpc, laptops) and Temash (tablets) describe SoC
>> families with those cores.
>>
>> Hudson seems to be the generic code name(!?) for all FCH aka
>> southbridges since sb9xx which came to be when they started with their
>> fusion aka APU development, hence there are multiple generations of
>> Hudson (3 AFAIK, where Yangtze/Salton are #3).
>>
>> The integrated FCH for Kabini and Temash are codenamed "Yangtze" and
>> "Salton" respectively. I am not aware of the previous code names...
>> The only other FCH name i could find is Yuba but that was never
>> released because it was cancelled together with Krishna/Wichita.
>
> Is it right to call out Hudson for incompatible SPI changes though? We
> might need to call it hudson3 or something. What I seem to see in
> coreboot is they try to bundle compatible changes into hudson and for
> incompatible changes just call it yangtze.
Stefan Tauner - 2013-07-26 18:54:28
On Fri, 26 Jul 2013 11:39:19 -0700
Wei Hu <wei@aristanetworks.com> wrote:

> Stefan,
> 
> AMD internal documents also suggest A45/A50M/A55E FCH (Hudson-1) use
> device ID 438D. Not sure if it's wrong since I don't have a test
> system.

I am. :) The ASRock E350M1 uses the A50M and has 439D (it is well
supported by coreboot and it would be easy to find someone to test it):
http://www.coreboot.org/pipermail/coreboot/2011-June/065421.html

> According to AMD document A55/A60M/A68/A68M/A70M/A75/A85X use device
> ID 780E. There's also this A77E FCH (codename Bolton) which also uses
> 780E.

OK, thanks

> (Please cc me on your future emails since I'm not subscribed to the list)

sorry!
Wei Hu - 2013-07-26 19:28:48
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.
Stefan Tauner - 2013-07-26 21:31:52
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.

Patch

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,29 @@ 
 {
 	uint8_t tmp;
 
+	if (hudson_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 (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 +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 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 +364,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);