Change in ...coreboot[master]: src/soc/amd/picasso: Update UARTs

[Marshall Dawson (Code Review)]

Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33767

Change subject: src/soc/amd/picasso: Update UARTs
......................................................................

src/soc/amd/picasso: Update UARTs

Signed-off-by: Marshall Dawson 
Change-Id: I74579674544f0edd2c0e6c4963270b442668e62f
---
M src/soc/amd/picasso/include/soc/iomap.h
M src/soc/amd/picasso/uart.c
2 files changed, 14 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/33767/1

diff --git a/src/soc/amd/picasso/include/soc/iomap.h b/src/soc/amd/picasso/include/soc/iomap.h
index b9d1ab5..b06e101 100644
--- a/src/soc/amd/picasso/include/soc/iomap.h
+++ b/src/soc/amd/picasso/include/soc/iomap.h
@@ -59,8 +59,11 @@
 #endif
 #define HPET_BASE_ADDRESS		0xfed00000

-#define APU_UART0_BASE			0xfedc6000
-#define APU_UART1_BASE			0xfedc8000
+#define APU_UART0_BASE			0xfedc9000
+#define APU_UART1_BASE			0xfedca000
+#define APU_UART2_BASE			0xfedce000
+#define APU_UART3_BASE			0xfedcf000
+#define APU_UART_MAX_PORT		3

#define FLASH_BASE_ADDR			((0xffffffff - CONFIG_ROM_SIZE) + 1)

diff --git a/src/soc/amd/picasso/uart.c b/src/soc/amd/picasso/uart.c
index d5d3006..339bd27 100644
--- a/src/soc/amd/picasso/uart.c
+++ b/src/soc/amd/picasso/uart.c
@@ -16,12 +16,19 @@
 #include <console/uart.h>
 #include <soc/southbridge.h>

+static const uintptr_t uart_addresses[] = {
+	APU_UART0_BASE,
+	APU_UART1_BASE,
+	APU_UART2_BASE,
+	APU_UART3_BASE,
+};
+
 uintptr_t uart_platform_base(int idx)
 {
-	if (CONFIG_UART_FOR_CONSOLE < 0 || CONFIG_UART_FOR_CONSOLE > 1)
+	if (idx < 0 || idx > APU_UART_MAX_PORT)
    	return 0;

-	return (uintptr_t)(APU_UART0_BASE + 0x2000 * (idx & 1));
+	return uart_addresses[uart];
 }

unsigned int uart_platform_refclk(void)

Change in coreboot[master]: src/soc/amd/picasso: Update UARTs

[Marshall Dawson (Code Review)]

Hello build bot (Jenkins), Martin Roth, Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33767

to look at the new patch set (#4).

Change subject: src/soc/amd/picasso: Update UARTs
......................................................................

src/soc/amd/picasso: Update UARTs

Add a function to uart.c to ensure the right IOMux settings are
programmed for the console UART.  Update Kconfig to reflect the
new addresses.

Signed-off-by: Marshall Dawson 
Change-Id: I74579674544f0edd2c0e6c4963270b442668e62f
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/include/soc/iomap.h
M src/soc/amd/picasso/include/soc/southbridge.h
M src/soc/amd/picasso/southbridge.c
M src/soc/amd/picasso/uart.c
5 files changed, 52 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/33767/4

Change in coreboot[master]: src/soc/amd/picasso: Update UARTs

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: src/soc/amd/picasso: Update UARTs
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33767/4/src/soc/amd/picasso/Kconfig 
File src/soc/amd/picasso/Kconfig:

https://review.coreboot.org/c/coreboot/+/33767/4/src/soc/amd/picasso/Kconfig... 
PS4, Line 196: config CONSOLE_UART_BASE_ADDRESS
I know this pre-exists, but do we really need it? It looks like this is a redunant definition that could easily be replaced with a function call and array in C source, then we would not need to list the addresses in Kconfig

I can see this is passed to SeaBIOS makefile, but SeaBIOS really should pickup the same informatio from coreboot tables / LB tags.

Change in coreboot[master]: src/soc/amd/picasso: Update UARTs

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: src/soc/amd/picasso: Update UARTs
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33767/4/src/soc/amd/picasso/Kconfig 
File src/soc/amd/picasso/Kconfig:

https://review.coreboot.org/c/coreboot/+/33767/4/src/soc/amd/picasso/Kconfig... 
PS4, Line 196: config CONSOLE_UART_BASE_ADDRESS
> I know this pre-exists, but do we really need it? It looks like this is a redunant definition that c […]

It seems only used to create the SeaBIOS .config.  It looks like SeaBIOS writes debug info before the tables are located.  If I remove it, the payload wont build.

Change in coreboot[master]: src/soc/amd/picasso: Update UARTs

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: src/soc/amd/picasso: Update UARTs
......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33767/4/src/soc/amd/picasso/Kconfig 
File src/soc/amd/picasso/Kconfig:

https://review.coreboot.org/c/coreboot/+/33767/4/src/soc/amd/picasso/Kconfig... 
PS4, Line 196: config CONSOLE_UART_BASE_ADDRESS
> It seems only used to create the SeaBIOS .config. […]

Ack

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: soc/amd/picasso: Update UARTs
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
File src/soc/amd/picasso/include/soc/iomap.h:

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
PS13, Line 31: 0xfed80000
Really? For stoney it was 0xfed80a00. Also, I saw when you defined the register accessed through this MMIO it was saying offset to 0xfed80a00. Either it was wrong there, or it's wrong here (which is what I believe).
Or are we talking something completely different, as I do see an SUPPORTS_ACPIMMIO_SMBUS_BASE below.
On Stoney, 0xfed80000 was for USB legacy registers, did it change?

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: soc/amd/picasso: Update UARTs
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
File src/soc/amd/picasso/include/soc/iomap.h:

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
PS13, Line 31: 0xfed80000
> For stoney it was 0xfed80a00. Also, I saw when you defined the register accessed through this MMIO it was saying offset to 0xfed80a00.

D14F0 is the SMBus host controller, which has PCI config space and two BARs for SMBus and ASF.  AcpiMmio is a fixed region where you can access all three of these:  the config space without a config cycle, and the two controllers without needing to read either BAR.  In your ST BKDG, look at the first item in the table of PMx04[1].  That's what this is.
> On Stoney, 0xfed80000 was for USB legacy registers, did it change?

That doesn't ring a bell, and I don't know how that could be possible.  Can you point me to what you're talking about?

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: soc/amd/picasso: Update UARTs
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
File src/soc/amd/picasso/include/soc/iomap.h:

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
PS13, Line 31: 0xfed80000
> > For stoney it was 0xfed80a00. […]

55072 Rev 3.06 Apr 05, 2018
BKDG for AMD Family 15h Models 70h-7Fh Processors
pg 838
3.26.4.3
USB Legacy Registers
This register space is used to provide USB legacy support. Each of the registers is located on a 32-bit boundary.
The offsets of the registers are relative to the AcpiMMio base (FED8_0000h). Three of the operational regis-
ters (HCEx4C, HCEx44 and HCEx48) are accessible at IO060 and IO064 when emulation is enabled. Reads
and writes to the registers using IO addresses have side effects as outlined in the table below.

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: soc/amd/picasso: Update UARTs
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
File src/soc/amd/picasso/include/soc/iomap.h:

https://review.coreboot.org/c/coreboot/+/33767/13/src/soc/amd/picasso/includ... 
PS13, Line 31: 0xfed80000
Thanks.  Looking through a number of old specs, trying to determine when it showed up, was it a typo, etc., here's what I've come up with.  The lowest offset for any of those registers is 0x40 (depending on the product).

FWIW, I'd compared the MMIO version with the D14F0 header before making the change.  It looks like there are no definitions for D14F0x40-D14F0xFB.  Looking more closely, I've used HDT to try to determine how much of D14F0 responds or is sticky from 0-FF.  It's not much, so I suspect the majority of locations are unimplemented vs. undocumented.  Also, this matches perfectly the behavior I see at 0xfed80000.  Also, at 0xfed80040-50 I can see the USB registers.

In other words, I get 100% of the implemented D14F0 space at 0xfed80000, and I additionally get a handful of USB registers at 0xfed80040.  Most importantly, part of that 100% is the UART Control register at 0xfc.
> ...and two BARs for SMBus and ASF...

FYI, the ST BKDG isn't as clear as the PCO PPR, so that I said wasn't correct about the BARs.  These are read-only and are 0.  In addition to the AcpiMmio ranges, SMBus and ASF controllers may also be accessed w/IO based on PMx00[15:8] when enabled.

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: soc/amd/picasso: Update UARTs
......................................................................

Patch Set 14: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Marshall Dawson (Code Review)]

Hello Richard Spiegel, build bot (Jenkins), Martin Roth, Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33767

to look at the new patch set (#15).

Change subject: soc/amd/picasso: Update UARTs
......................................................................

soc/amd/picasso: Update UARTs

Add a function to uart.c to ensure the right IOMux settings are
programmed for the console UART.  Update Kconfig to reflect the
new addresses.

Give the user the ability to downclock the UARTs' refclock to
1.8342MHz.

Add the abiltiy to use an APU UART at a legacy I/O address.

Update the AOAC register configuration for the two additional
UARTs.

Signed-off-by: Marshall Dawson 
Change-Id: I74579674544f0edd2c0e6c4963270b442668e62f
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/acpi/globalnvs.asl
M src/soc/amd/picasso/acpi/sb_fch.asl
M src/soc/amd/picasso/acpi/sb_pci0_fch.asl
M src/soc/amd/picasso/include/soc/iomap.h
M src/soc/amd/picasso/include/soc/southbridge.h
M src/soc/amd/picasso/southbridge.c
M src/soc/amd/picasso/uart.c
8 files changed, 217 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/33767/15

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: soc/amd/picasso: Update UARTs
......................................................................

Patch Set 15: Code-Review+2

Change in coreboot[master]: soc/amd/picasso: Update UARTs

[Martin Roth (Code Review)]

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/33767 )

Change subject: soc/amd/picasso: Update UARTs
......................................................................

soc/amd/picasso: Update UARTs

Add a function to uart.c to ensure the right IOMux settings are
programmed for the console UART.  Update Kconfig to reflect the
new addresses.

Give the user the ability to downclock the UARTs' refclock to
1.8342MHz.

Add the abiltiy to use an APU UART at a legacy I/O address.

Update the AOAC register configuration for the two additional
UARTs.

Signed-off-by: Marshall Dawson 
Change-Id: I74579674544f0edd2c0e6c4963270b442668e62f
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33767
Tested-by: build bot (Jenkins) 
Reviewed-by: Martin Roth 
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/acpi/globalnvs.asl
M src/soc/amd/picasso/acpi/sb_fch.asl
M src/soc/amd/picasso/acpi/sb_pci0_fch.asl
M src/soc/amd/picasso/include/soc/iomap.h
M src/soc/amd/picasso/include/soc/southbridge.h
M src/soc/amd/picasso/southbridge.c
M src/soc/amd/picasso/uart.c
8 files changed, 217 insertions(+), 19 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved

diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 6d0a3ef..5af830c 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -157,16 +157,46 @@
    select NO_UART_ON_SUPERIO
    select UART_OVERRIDE_REFCLK
    help
-	  There are two UART controllers in Picasso.
-	  The UART registers are memory-mapped. UART
-	  controller 0 registers range from FEDC_6000h
-	  to FEDC_6FFFh. UART controller 1 registers
-	  range from FEDC_8000h to FEDC_8FFFh.
+	  There are four memory-mapped UARTs controllers in Picasso at:
+	  0: 0xfedc9000
+	  1: 0xfedca000
+	  2: 0xfedc3000
+	  3: 0xfedcf000
+
+choice PICASSO_UART_CLOCK_SOURCE
+	prompt "UART Frequency"
+	depends on PICASSO_UART
+	default PICASSO_UART_48MZ
+
+config PICASSO_UART_48MZ
+	bool "48 MHz clock"
+	help
+	  Select this option for the most compatibility.
+
+config PICASSO_UART_1_8MZ
+	bool "1.8432 MHz clock"
+	help
+	  Select this option if an old payload or Linux ttyS0 arguments
+	  require it.
+
+endchoice
+
+config PICASSO_UART_LEGACY
+	bool "Decode legacy I/O range"
+	depends on PICASSO_UART
+	help
+	  Assign I/O 3F8, 2F8, etc. to a Picasso UART.  Only a single UART may
+	  decode legacy addresses and this option enables the one used for the
+	  console.  A UART accessed with I/O does not allow all the features
+	  of MMIO.  The MMIO decode is still present when this option is used.

config CONSOLE_UART_BASE_ADDRESS
-	depends on CONSOLE_SERIAL
+	depends on CONSOLE_SERIAL && PICASSO_UART
    hex
-	default 0xfedc6000
+	default 0xfedc9000 if UART_FOR_CONSOLE = 0
+	default 0xfedca000 if UART_FOR_CONSOLE = 1
+	default 0xfedc3000 if UART_FOR_CONSOLE = 2
+	default 0xfedcf000 if UART_FOR_CONSOLE = 3

config SMM_TSEG_SIZE
    hex
diff --git a/src/soc/amd/picasso/acpi/globalnvs.asl b/src/soc/amd/picasso/acpi/globalnvs.asl
index 12480c7..a373a99 100644
--- a/src/soc/amd/picasso/acpi/globalnvs.asl
+++ b/src/soc/amd/picasso/acpi/globalnvs.asl
@@ -55,7 +55,10 @@
    , 1,
    UT0E,	1,	//        UART0, 11
    UT1E,	1,	//        UART1, 12
-	, 14,
+	, 3,
+	UT2E,	1,	//        UART2, 16
+	, 9,
+	UT23,	1,	//        UART3, 26
    ESPI,	1,	//        ESPI, 27
    /* ChromeOS stuff (0x100 -> 0xfff, size 0xeff) */
    Offset (0x100),
diff --git a/src/soc/amd/picasso/acpi/sb_fch.asl b/src/soc/amd/picasso/acpi/sb_fch.asl
index e7975f8..13b9025 100644
--- a/src/soc/amd/picasso/acpi/sb_fch.asl
+++ b/src/soc/amd/picasso/acpi/sb_fch.asl
@@ -58,7 +58,8 @@
    Name (_CRS, ResourceTemplate()
    {
    	IRQ (Edge, ActiveHigh, Exclusive) { 10 }
-		Memory32Fixed (ReadWrite, 0xFEDC6000, 0x2000)
+		Memory32Fixed (ReadWrite, APU_UART0_BASE, 0x1000)
+		Memory32Fixed (ReadWrite, APU_DMAC0_BASE, 0x1000)
    })
    Method (_STA, 0x0, NotSerialized)
    {
@@ -71,12 +72,44 @@
    Name (_UID, 0x1)
    Name (_CRS, ResourceTemplate()
    {
-			IRQ (Edge, ActiveHigh, Exclusive) { 11 }
-			Memory32Fixed (ReadWrite, 0xFEDC8000, 0x2000)
+		IRQ (Edge, ActiveHigh, Exclusive) { 11 }
+		Memory32Fixed (ReadWrite, APU_UART1_BASE, 0x1000)
+		Memory32Fixed (ReadWrite, APU_DMAC1_BASE, 0x1000)
    })
    Method (_STA, 0x0, NotSerialized)
    {
-			Return (0x0F)
+		Return (0x0F)
+	}
+}
+
+Device (FUR2)
+{
+	Name (_HID, "AMD0020")
+	Name (_UID, 0x0)
+	Name (_CRS, ResourceTemplate()
+	{
+		IRQ (Edge, ActiveHigh, Exclusive) { 15 }
+		Memory32Fixed (ReadWrite, APU_UART2_BASE, 0x1000)
+		Memory32Fixed (ReadWrite, APU_DMAC2_BASE, 0x1000)
+	})
+	Method (_STA, 0x0, NotSerialized)
+	{
+		Return (0x0F)
+	}
+}
+
+Device (FUR3) {
+	Name (_HID, "AMD0020")
+	Name (_UID, 0x1)
+	Name (_CRS, ResourceTemplate()
+	{
+		IRQ (Edge, ActiveHigh, Exclusive) { 5 }
+		Memory32Fixed (ReadWrite, APU_UART3_BASE, 0x1000)
+		Memory32Fixed (ReadWrite, APU_DMAC3_BASE, 0x1000)
+	})
+	Method (_STA, 0x0, NotSerialized)
+	{
+		Return (0x0F)
    }
 }

diff --git a/src/soc/amd/picasso/acpi/sb_pci0_fch.asl b/src/soc/amd/picasso/acpi/sb_pci0_fch.asl
index 5e16fef..bd340dd 100644
--- a/src/soc/amd/picasso/acpi/sb_pci0_fch.asl
+++ b/src/soc/amd/picasso/acpi/sb_pci0_fch.asl
@@ -236,9 +236,23 @@
    offset (0x1e59), /* UART1 D3 State */
    U1DS, 3,

+	offset (0x1e60), /* UART2 D3 Control */
+	U2TD, 2,
+	, 1,
+	U2PD, 1,
+	offset (0x1e61), /* UART2 D3 State */
+	U2DS, 3,
+
    offset (0x1e71), /* SD D3 State */
    SDDS, 3,

+	offset (0x1e74), /* UART3 D3 Control */
+	U3TD, 2,
+	, 1,
+	U3PD, 1,
+	offset (0x1e75), /* UART3 D3 State */
+	U3DS, 3,
+
    offset (0x1e80), /* Shadow Register Request */
    , 15,
    RQ15, 1,
@@ -375,6 +389,22 @@
    				Store(U1DS, Local0)
    			}
    		}
+			Case(16) {
+				Store(0x00, U2TD)
+				Store(One, U2PD)
+				Store(U2DS, Local0)
+				while(LNotEqual(Local0,0x7)) {
+					Store(U2DS, Local0)
+				}
+			}
+			Case(26) {
+				Store(0x00, U3TD)
+				Store(One, U3PD)
+				Store(U3DS, Local0)
+				while(LNotEqual(Local0,0x7)) {
+					Store(U3DS, Local0)
+				}
+			}
    	}
    } else {
    	/* put device into D3cold */
@@ -427,6 +457,22 @@
    			}
    			Store(0x03, U1TD)
    		}
+			Case(16) {
+				Store(Zero, U2PD)
+				Store(U2DS, Local0)
+				while(LNotEqual(Local0,0x0)) {
+					Store(U2DS, Local0)
+				}
+				Store(0x03, U2TD)
+			}
+			Case(26) {
+				Store(Zero, U3PD)
+				Store(U3DS, Local0)
+				while(LNotEqual(Local0,0x0)) {
+					Store(U3DS, Local0)
+				}
+				Store(0x03, U3TD)
+			}
    	}
    	if(LEqual(I1TD, 3)) {
    		if(LEqual(I2TD, 3)) {
diff --git a/src/soc/amd/picasso/include/soc/iomap.h b/src/soc/amd/picasso/include/soc/iomap.h
index ad0e7c4..344b886 100644
--- a/src/soc/amd/picasso/include/soc/iomap.h
+++ b/src/soc/amd/picasso/include/soc/iomap.h
@@ -28,6 +28,7 @@
  * any documentation but should be considered reserved through FED8_1FFFh.
  */
 #include <amdblocks/acpimmio_map.h>
+#define SUPPORTS_ACPIMMIO_SM_PCI_BASE	1 /* 0xfed80000 */
 #define SUPPORTS_ACPIMMIO_SMI_BASE	1 /* 0xfed80100 */
 #define SUPPORTS_ACPIMMIO_PMIO_BASE	1 /* 0xfed80300 */
 #define SUPPORTS_ACPIMMIO_BIOSRAM_BASE	1 /* 0xfed80500 */
@@ -60,8 +61,10 @@
 #endif
 #define HPET_BASE_ADDRESS		0xfed00000

-#define APU_UART0_BASE			0xfedc6000
-#define APU_UART1_BASE			0xfedc8000
+#define APU_UART0_BASE			0xfedc9000
+#define APU_UART1_BASE			0xfedca000
+#define APU_UART2_BASE			0xfedce000
+#define APU_UART3_BASE			0xfedcf000

#define FLASH_BASE_ADDR			((0xffffffff - CONFIG_ROM_SIZE) + 1)

diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h
index 8bd061b..0fb187d 100644
--- a/src/soc/amd/picasso/include/soc/southbridge.h
+++ b/src/soc/amd/picasso/include/soc/southbridge.h
@@ -28,6 +28,14 @@
  *  - fixed addresses offset from 0xfed80000
  */

+/* SMBus controller registers:  0xfed80000 or D14F0 */
+#define SMB_UART_CONFIG			0xfc
+#define   SMB_UART3_1_8M		BIT(31) /* defaults are 0 = 48MHz */
+#define   SMB_UART2_1_8M		BIT(30)
+#define   SMB_UART1_1_8M		BIT(29)
+#define   SMB_UART0_1_8M		BIT(28)
+#define   SMB_UART_1_8M_SHIFT		28
+
 /* Power management registers:  0xfed80300 or index/data at IO 0xcd6/cd7 */
 #define PM_DECODE_EN			0x00
 #define   SMBUS_ASF_IO_EN		BIT(4)
@@ -209,6 +217,7 @@
 #define FCH_AOAC_DEV_UART1		12
 #define FCH_AOAC_DEV_UART2		16
 #define FCH_AOAC_DEV_AMBA		17
+#define FCH_AOAC_DEV_UART3		26
 #define FCH_AOAC_DEV_ESPI		27

/* Bit definitions for Device D3 Control AOACx0000[40...7E] step 2 */
@@ -230,6 +239,11 @@
 #define   FCH_AOAC_STAT0		BIT(6)
 #define   FCH_AOAC_STAT1		BIT(7)

+#define FCH_UART_LEGACY_DECODE		0xfedc0020
+#define   FCH_LEGACY_3F8_SH		3
+#define   FCH_LEGACY_2F8_SH		1
+#define   FCH_LEGACY_3E8_SH		2
+
 #define PM1_LIMIT			16
 #define GPE0_LIMIT			28
 #define TOTAL_BITS(a)			(8 * sizeof(a))
@@ -294,7 +308,10 @@
    unsigned int :1;
    unsigned int ut0e:1; /* 11: UART0 */
    unsigned int ut1e:1; /* 12: UART1 */
-	unsigned int :14;
+	unsigned int :3;
+	unsigned int ut2e:1; /* 16: UART2 */
+	unsigned int :9;
+	unsigned int ut3e:1; /* 26: UART3 */
    unsigned int espi:1; /* 27: ESPI */
    unsigned int :4;
 } __packed aoac_devs_t;
@@ -317,6 +334,7 @@
 void sb_set_spi100(u16 norm, u16 fast, u16 alt, u16 tpm);
 void fch_pre_init(void);
 void fch_early_init(void);
+void set_uart_config(int idx);
 /**
  * @brief Save the UMA bize
  *
diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c
index 9206728..fe801d4 100644
--- a/src/soc/amd/picasso/southbridge.c
+++ b/src/soc/amd/picasso/southbridge.c
@@ -40,6 +40,8 @@
 #define FCH_AOAC_UART_FOR_CONSOLE \
    	(CONFIG_UART_FOR_CONSOLE == 0 ? FCH_AOAC_DEV_UART0 \
    	: CONFIG_UART_FOR_CONSOLE == 1 ? FCH_AOAC_DEV_UART1 \
+		: CONFIG_UART_FOR_CONSOLE == 2 ? FCH_AOAC_DEV_UART2 \
+		: CONFIG_UART_FOR_CONSOLE == 3 ? FCH_AOAC_DEV_UART3 \
    	: -1)
 #if FCH_AOAC_UART_FOR_CONSOLE == -1
 # error Unsupported UART_FOR_CONSOLE chosen
@@ -282,6 +284,8 @@
    sb_enable_legacy_io();
    enable_aoac_devices();
    sb_reset_i2c_slaves();
+	if (CONFIG(PICASSO_UART))
+		set_uart_config(CONFIG_UART_FOR_CONSOLE);
 }

static void print_num_status_bits(int num_bits, uint32_t status,
@@ -463,6 +467,8 @@
    gnvs->aoac.ic4e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C4);
    gnvs->aoac.ut0e = is_aoac_device_enabled(FCH_AOAC_DEV_UART0);
    gnvs->aoac.ut1e = is_aoac_device_enabled(FCH_AOAC_DEV_UART1);
+	gnvs->aoac.ut2e = is_aoac_device_enabled(FCH_AOAC_DEV_UART2);
+	gnvs->aoac.ut3e = is_aoac_device_enabled(FCH_AOAC_DEV_UART3);
    gnvs->aoac.espi = 1;
 }

diff --git a/src/soc/amd/picasso/uart.c b/src/soc/amd/picasso/uart.c
index d5d3006..4458624 100644
--- a/src/soc/amd/picasso/uart.c
+++ b/src/soc/amd/picasso/uart.c
@@ -1,7 +1,7 @@
 /*
  * This file is part of the coreboot project.
  *
- * Copyright (C) 2015 Advanced Micro Devices, Inc.
+ * Copyright (C) 2019 Advanced Micro Devices, Inc.
  *
  * 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
@@ -13,18 +13,77 @@
  * GNU General Public License for more details.
  */

+#include <arch/mmio.h>
 #include <console/uart.h>
+#include <commonlib/helpers.h>
+#include <amdblocks/gpio_banks.h>
+#include <amdblocks/acpimmio.h>
 #include <soc/southbridge.h>
+#include <soc/gpio.h>
+
+static const struct _uart_info {
+	uintptr_t base;
+	struct soc_amd_gpio mux[2];
+} uart_info[] = {
+	[0] = { APU_UART0_BASE, {
+			PAD_NF(GPIO_138, UART0_TXD, PULL_NONE),
+			PAD_NF(GPIO_136, UART0_RXD, PULL_NONE),
+	} },
+	[1] = { APU_UART1_BASE, {
+			PAD_NF(GPIO_143, UART1_TXD, PULL_NONE),
+			PAD_NF(GPIO_141, UART1_RXD, PULL_NONE),
+	} },
+	[2] = { APU_UART2_BASE, {
+			PAD_NF(GPIO_137, UART2_TXD, PULL_NONE),
+			PAD_NF(GPIO_135, UART2_RXD, PULL_NONE),
+	} },
+	[3] = { APU_UART3_BASE, {
+			PAD_NF(GPIO_140, UART3_TXD, PULL_NONE),
+			PAD_NF(GPIO_142, UART3_RXD, PULL_NONE),
+	} },
+};

uintptr_t uart_platform_base(int idx)
 {
-	if (CONFIG_UART_FOR_CONSOLE < 0 || CONFIG_UART_FOR_CONSOLE > 1)
+	if (idx < 0 || idx > ARRAY_SIZE(uart_info))
    	return 0;

-	return (uintptr_t)(APU_UART0_BASE + 0x2000 * (idx & 1));
+	return uart_info[idx].base;
+}
+
+void set_uart_config(int idx)
+{
+	uint32_t uart_ctrl;
+	uint16_t uart_leg;
+
+	if (idx < 0 || idx > ARRAY_SIZE(uart_info))
+		return;
+
+	program_gpios(uart_info[idx].mux, 2);
+
+	if (CONFIG(PICASSO_UART_1_8MZ)) {
+		uart_ctrl = sm_pci_read32(SMB_UART_CONFIG);
+		uart_ctrl |= 1 << (SMB_UART_1_8M_SHIFT + idx);
+		sm_pci_write32(SMB_UART_CONFIG, uart_ctrl);
+	}
+
+	if (CONFIG(PICASSO_UART_LEGACY) && idx != 3) {
+		/* Force 3F8 if idx=0, 2F8 if idx=1, 3E8 if idx=2 */
+
+		/* TODO: make clearer once PPR is updated */
+		uart_leg = (idx << 8) | (idx << 10) | (idx << 12) | (idx << 14);
+		if (idx == 0)
+			uart_leg |= 1 << FCH_LEGACY_3F8_SH;
+		else if (idx == 1)
+			uart_leg |= 1 << FCH_LEGACY_2F8_SH;
+		else if (idx == 2)
+			uart_leg |= 1 << FCH_LEGACY_3E8_SH;
+
+		write16((void *)FCH_UART_LEGACY_DECODE, uart_leg);
+	}
 }

unsigned int uart_platform_refclk(void)
 {
-	return 48000000;
+	return CONFIG(PICASSO_UART_48MZ) ? 48000000 : 115200 * 16;
 }