soc/amd/picasso: Refactor AOAC enabling

Replace the raw register definitions with device numbers and macros
for determining the register offsets.  Rewrite the source to refer
to AOAC device numbers instead of a structure.

Remove the calculated offset for the console UART.  Picasso's UARTs
are not contiguous so handle them separately.

Change-Id: Iffc87f39ebe38394a56d41bb0940e9701fd05db9
Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35296
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>

2 files changed, 57 insertions, 41 deletions

diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h
index 803c638164..dad4358a8f 100644
--- a/src/soc/amd/picasso/include/soc/southbridge.h
+++ b/src/soc/amd/picasso/include/soc/southbridge.h
@@ -206,14 +206,18 @@
 #define   I2C_PAD_CTRL_SPARE1		BIT(18)
 
 /* FCH AOAC Registers 0xfed81e00 */
-#define FCH_AOAC_D3_CONTROL_CLK_GEN	0x40
-#define FCH_AOAC_D3_CONTROL_I2C2	0x4e
-#define FCH_AOAC_D3_CONTROL_I2C3	0x50
-#define FCH_AOAC_D3_CONTROL_I2C4	0x52
-#define FCH_AOAC_D3_CONTROL_UART0	0x56
-#define FCH_AOAC_D3_CONTROL_UART1	0x58
-#define FCH_AOAC_D3_CONTROL_AMBA	0x62
-/* Bit definitions for all FCH_AOAC_D3_CONTROL_* Registers */
+#define AOAC_DEV_D3_CTL(device)		(0x40 + device * 2)
+#define AOAC_DEV_D3_STATE(device)	(AOAC_DEV_D3_CTL(device) + 1)
+
+#define FCH_AOAC_DEV_CLK_GEN		0
+#define FCH_AOAC_DEV_I2C2		7
+#define FCH_AOAC_DEV_I2C3		8
+#define FCH_AOAC_DEV_I2C4		9
+#define FCH_AOAC_DEV_UART0		11
+#define FCH_AOAC_DEV_UART1		12
+#define FCH_AOAC_DEV_AMBA		17
+
+/* Bit definitions for Device D3 Control AOACx0000[40...7E] step 2 */
 #define   FCH_AOAC_TARGET_DEVICE_STATE (BIT(0) + BIT(1))
 #define   FCH_AOAC_DEVICE_STATE		BIT(2)
 #define   FCH_AOAC_PWR_ON_DEV		BIT(3)
@@ -222,14 +226,7 @@
 #define   FCH_AOAC_SW_RST_B		BIT(6)
 #define   FCH_AOAC_IS_SW_CONTROL	BIT(7)
 
-#define FCH_AOAC_D3_STATE_CLK_GEN	0x41
-#define FCH_AOAC_D3_STATE_I2C2		0x4f
-#define FCH_AOAC_D3_STATE_I2C3		0x51
-#define FCH_AOAC_D3_STATE_I2C4		0x53
-#define FCH_AOAC_D3_STATE_UART0		0x57
-#define FCH_AOAC_D3_STATE_UART1		0x59
-#define FCH_AOAC_D3_STATE_AMBA		0x63
-/* Bit definitions for all FCH_AOAC_D3_STATE_* Registers */
+/* Bit definitions for Device D3 State AOACx0000[41...7f] step 2 */
 #define   FCH_AOAC_PWR_RST_STATE	BIT(0)
 #define   FCH_AOAC_RST_CLK_OK_STATE	BIT(1)
 #define   FCH_AOAC_RST_B_STATE		BIT(2)
@@ -307,11 +304,6 @@
 /* IO 0xf0 NCP Error */
 #define   NCP_WARM_BOOT			BIT(7) /* Write-once */
 
-struct picasso_aoac {
-	int enable;
-	int status;
-};
-
 typedef struct aoac_devs {
 	unsigned int :7;
 	unsigned int ic2e:1; /* 7: I2C2 */
diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c
index 582034a6a7..e6fc0dedc2 100644
--- a/src/soc/amd/picasso/southbridge.c
+++ b/src/soc/amd/picasso/southbridge.c
@@ -36,19 +36,25 @@
 #include <soc/nvs.h>
 #include <types.h>
 
+#define FCH_AOAC_UART_FOR_CONSOLE \
+		(CONFIG_UART_FOR_CONSOLE == 0 ? FCH_AOAC_DEV_UART0 \
+		: CONFIG_UART_FOR_CONSOLE == 1 ? FCH_AOAC_DEV_UART1 \
+		: -1)
+#if FCH_AOAC_UART_FOR_CONSOLE == -1
+# error Unsupported UART_FOR_CONSOLE chosen
+#endif
+
 /*
  * Table of devices that need their AOAC registers enabled and waited
  * upon (usually about .55 milliseconds). Instead of individual delays
  * waiting for each device to become available, a single delay will be
- * executed.
+ * executed.  The console UART is handled separately from this table.
  */
-const static struct picasso_aoac aoac_devs[] = {
-	{ (FCH_AOAC_D3_CONTROL_UART0 + CONFIG_UART_FOR_CONSOLE * 2),
-		(FCH_AOAC_D3_STATE_UART0 + CONFIG_UART_FOR_CONSOLE * 2) },
-	{ FCH_AOAC_D3_CONTROL_AMBA, FCH_AOAC_D3_STATE_AMBA },
-	{ FCH_AOAC_D3_CONTROL_I2C2, FCH_AOAC_D3_STATE_I2C2 },
-	{ FCH_AOAC_D3_CONTROL_I2C3, FCH_AOAC_D3_STATE_I2C3 },
-	{ FCH_AOAC_D3_CONTROL_I2C4, FCH_AOAC_D3_STATE_I2C4 }
+const static int aoac_devs[] = {
+	FCH_AOAC_DEV_AMBA,
+	FCH_AOAC_DEV_I2C2,
+	FCH_AOAC_DEV_I2C3,
+	FCH_AOAC_DEV_I2C4,
 };
 
 /*
@@ -101,21 +107,21 @@ const struct irq_idx_name *sb_get_apic_reg_association(size_t *size)
 	return irq_association;
 }
 
-static void power_on_aoac_device(int aoac_device_control_register)
+static void power_on_aoac_device(int dev)
 {
 	uint8_t byte;
 
 	/* Power on the UART and AMBA devices */
-	byte = aoac_read8(aoac_device_control_register);
+	byte = aoac_read8(AOAC_DEV_D3_CTL(dev));
 	byte |= FCH_AOAC_PWR_ON_DEV;
-	aoac_write8(aoac_device_control_register, byte);
+	aoac_write8(AOAC_DEV_D3_CTL(dev), byte);
 }
 
-static bool is_aoac_device_enabled(int aoac_device_status_register)
+static bool is_aoac_device_enabled(int dev)
 {
 	uint8_t byte;
 
-	byte = aoac_read8(aoac_device_status_register);
+	byte = aoac_read8(AOAC_DEV_D3_STATE(dev));
 	byte &= (FCH_AOAC_PWR_RST_STATE | FCH_AOAC_RST_CLK_OK_STATE);
 	if (byte == (FCH_AOAC_PWR_RST_STATE | FCH_AOAC_RST_CLK_OK_STATE))
 		return true;
@@ -123,20 +129,38 @@ static bool is_aoac_device_enabled(int aoac_device_status_register)
 		return false;
 }
 
+static void enable_aoac_console_uart(void)
+{
+	if (!CONFIG(PICASSO_UART))
+		return;
+
+	power_on_aoac_device(FCH_AOAC_UART_FOR_CONSOLE);
+}
+
+static bool is_aoac_console_uart_enabled(void)
+{
+	if (!CONFIG(PICASSO_UART))
+		return true;
+
+	return is_aoac_device_enabled(FCH_AOAC_UART_FOR_CONSOLE);
+}
+
 void enable_aoac_devices(void)
 {
 	bool status;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(aoac_devs); i++)
-		power_on_aoac_device(aoac_devs[i].enable);
+		power_on_aoac_device(aoac_devs[i]);
+	enable_aoac_console_uart();
 
 	/* Wait for AOAC devices to indicate power and clock OK */
 	do {
 		udelay(100);
 		status = true;
 		for (i = 0; i < ARRAY_SIZE(aoac_devs); i++)
-			status &= is_aoac_device_enabled(aoac_devs[i].status);
+			status &= is_aoac_device_enabled(aoac_devs[i]);
+		status &= is_aoac_console_uart_enabled();
 	} while (!status);
 }
 
@@ -513,11 +537,11 @@ static void set_sb_final_nvs(void)
 	if (gnvs == NULL)
 		return;
 
-	gnvs->aoac.ic2e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C2);
-	gnvs->aoac.ic3e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C3);
-	gnvs->aoac.ic4e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C4);
-	gnvs->aoac.ut0e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_UART0);
-	gnvs->aoac.ut1e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_UART1);
+	gnvs->aoac.ic2e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C2);
+	gnvs->aoac.ic3e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C3);
+	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);
 	/* Rely on these being in sync with devicetree */
 	sata = pcidev_path_on_root(SATA_DEVFN);
 	gnvs->aoac.st_e = sata && sata->enabled ? 1 : 0;