Change in coreboot[master]: soc/amd: factor out common AOAC definitions

[Felix Held (Code Review)]

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48182 )

Change subject: soc/amd: factor out common AOAC definitions
......................................................................

soc/amd: factor out common AOAC definitions

The register locations and bit definitions are the same for Stoneyridge
and Picasso. Since not all devices are present on all SoCs, keep those
numbers in the SoC-specific code.

Change-Id: Ib882927e399031c376738e5a35793b3d7654b9cf
Signed-off-by: Felix Held 
---
A src/soc/amd/common/block/include/amdblocks/aoac.h
M src/soc/amd/picasso/aoac.c
M src/soc/amd/picasso/include/soc/southbridge.h
M src/soc/amd/stoneyridge/include/soc/southbridge.h
M src/soc/amd/stoneyridge/southbridge.c
5 files changed, 39 insertions(+), 50 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/48182/1

diff --git a/src/soc/amd/common/block/include/amdblocks/aoac.h b/src/soc/amd/common/block/include/amdblocks/aoac.h
new file mode 100644
index 0000000..81390b5
--- /dev/null
+++ b/src/soc/amd/common/block/include/amdblocks/aoac.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef AMD_BLOCK_AOAC_H
+#define AMD_BLOCK_AOAC_H
+
+#include <types.h>
+
+/* FCH AOAC Registers 0xfed81e00 */
+#define AOAC_DEV_D3_CTL(device)		(0x40 + device * 2)
+#define AOAC_DEV_D3_STATE(device)	(AOAC_DEV_D3_CTL(device) + 1)
+
+/* Bit definitions for Device D3 Control AOACx0000[40...7E] */
+#define   FCH_AOAC_TARGET_DEVICE_STATE (BIT(0) + BIT(1))
+#define     FCH_AOAC_D0_UNINITIALIZED	0
+#define     FCH_AOAC_D0_INITIALIZED	1
+#define     FCH_AOAC_D1_2_3_WARM	2
+#define     FCH_AOAC_D3_COLD		3
+#define   FCH_AOAC_DEVICE_STATE		BIT(2)
+#define   FCH_AOAC_PWR_ON_DEV		BIT(3)
+#define   FCH_AOAC_SW_PWR_ON_RSTB	BIT(4)
+#define   FCH_AOAC_SW_REF_CLK_OK	BIT(5)
+#define   FCH_AOAC_SW_RST_B		BIT(6)
+#define   FCH_AOAC_IS_SW_CONTROL	BIT(7)
+
+/* Bit definitions for Device D3 State AOACx0000[41...7f] */
+#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)
+#define   FCH_AOAC_DEV_OFF_GATING_STATE	BIT(3)
+#define   FCH_AOAC_D3COLD		BIT(4)
+#define   FCH_AOAC_CLK_OK_STATE		BIT(5)
+#define   FCH_AOAC_STAT0		BIT(6)
+#define   FCH_AOAC_STAT1		BIT(7)
+
+#endif /* AMD_BLOCK_AOAC_H */
diff --git a/src/soc/amd/picasso/aoac.c b/src/soc/amd/picasso/aoac.c
index 00f26fe..21031f7 100644
--- a/src/soc/amd/picasso/aoac.c
+++ b/src/soc/amd/picasso/aoac.c
@@ -2,6 +2,7 @@

#include <stdint.h>
 #include <amdblocks/acpimmio.h>
+#include <amdblocks/aoac.h>
 #include <soc/southbridge.h>
 #include <delay.h>

diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h
index d193661..8a2ae49 100644
--- a/src/soc/amd/picasso/include/soc/southbridge.h
+++ b/src/soc/amd/picasso/include/soc/southbridge.h
@@ -183,10 +183,7 @@
 #define   I2C_PAD_CTRL_SPARE0		BIT(17)
 #define   I2C_PAD_CTRL_SPARE1		BIT(18)

-/* FCH AOAC Registers 0xfed81e00 */
-#define AOAC_DEV_D3_CTL(device)		(0x40 + device * 2)
-#define AOAC_DEV_D3_STATE(device)	(AOAC_DEV_D3_CTL(device) + 1)
-
+/* FCH AOAC device offsets for AOAC_DEV_D3_CTL/AOAC_DEV_D3_STATE */
 #define FCH_AOAC_DEV_CLK_GEN		0
 #define FCH_AOAC_DEV_I2C2		7
 #define FCH_AOAC_DEV_I2C3		8
@@ -198,29 +195,6 @@
 #define FCH_AOAC_DEV_UART3		26
 #define FCH_AOAC_DEV_ESPI		27

-/* Bit definitions for Device D3 Control AOACx0000[40...7E] step 2 */
-#define   FCH_AOAC_TARGET_DEVICE_STATE (BIT(0) + BIT(1))
-#define     FCH_AOAC_D0_UNINITIALIZED	0
-#define     FCH_AOAC_D0_INITIALIZED	1
-#define     FCH_AOAC_D1_2_3_WARM	2
-#define     FCH_AOAC_D3_COLD		3
-#define   FCH_AOAC_DEVICE_STATE		BIT(2)
-#define   FCH_AOAC_PWR_ON_DEV		BIT(3)
-#define   FCH_AOAC_SW_PWR_ON_RSTB	BIT(4)
-#define   FCH_AOAC_SW_REF_CLK_OK	BIT(5)
-#define   FCH_AOAC_SW_RST_B		BIT(6)
-#define   FCH_AOAC_IS_SW_CONTROL	BIT(7)
-
-/* 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)
-#define   FCH_AOAC_DEV_OFF_GATING_STATE	BIT(3)
-#define   FCH_AOAC_D3COLD		BIT(4)
-#define   FCH_AOAC_CLK_OK_STATE		BIT(5)
-#define   FCH_AOAC_STAT0		BIT(6)
-#define   FCH_AOAC_STAT1		BIT(7)
-
 #define FCH_LEGACY_UART_DECODE		(ALINK_AHB_ADDRESS + 0x20) /* 0xfedc0020 */
 #define   FCH_LEGACY_UART_MAP_SHIFT	8
 #define   FCH_LEGACY_UART_MAP_SIZE	2
diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h
index 9450a92..59843c1 100644
--- a/src/soc/amd/stoneyridge/include/soc/southbridge.h
+++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h
@@ -189,10 +189,7 @@
 #define   DEBUG_PORT_ENABLE		  BIT(18)
 #define   DEBUG_PORT_MASK		(BIT(16) | BIT(17) | BIT(18))

-/* FCH AOAC Registers 0xfed81e00 */
-#define AOAC_DEV_D3_CTL(device)		(0x40 + device * 2)
-#define AOAC_DEV_D3_STATE(device)	(AOAC_DEV_D3_CTL(device) + 1)
-
+/* FCH AOAC device offsets for AOAC_DEV_D3_CTL/AOAC_DEV_D3_STATE */
 #define FCH_AOAC_DEV_CLK_GEN		0
 #define FCH_AOAC_DEV_I2C0		5
 #define FCH_AOAC_DEV_I2C1		6
@@ -204,25 +201,6 @@
 #define FCH_AOAC_DEV_USB2		18
 #define FCH_AOAC_DEV_USB3		23

-/* Bit definitions for all FCH_AOAC_D3_CONTROL_* Registers */
-#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)
-#define   FCH_AOAC_SW_PWR_ON_RSTB	BIT(4)
-#define   FCH_AOAC_SW_REF_CLK_OK	BIT(5)
-#define   FCH_AOAC_SW_RST_B		BIT(6)
-#define   FCH_AOAC_IS_SW_CONTROL	BIT(7)
-
-/* Bit definitions for all FCH_AOAC_D3_STATE_* Registers */
-#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)
-#define   FCH_AOAC_DEV_OFF_GATING_STATE	BIT(3)
-#define   FCH_AOAC_D3COLD		BIT(4)
-#define   FCH_AOAC_CLK_OK_STATE		BIT(5)
-#define   FCH_AOAC_STAT0		BIT(6)
-#define   FCH_AOAC_STAT1		BIT(7)
-
 #define PM1_LIMIT			16
 #define GPE0_LIMIT			28
 #define TOTAL_BITS(a)			(8 * sizeof(a))
diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c
index 05f3072..b427c18 100644
--- a/src/soc/amd/stoneyridge/southbridge.c
+++ b/src/soc/amd/stoneyridge/southbridge.c
@@ -12,6 +12,7 @@
 #include <acpi/acpi_gnvs.h>
 #include <amdblocks/amd_pci_util.h>
 #include <amdblocks/agesawrapper.h>
+#include <amdblocks/aoac.h>
 #include <amdblocks/reset.h>
 #include <amdblocks/acpimmio.h>
 #include <amdblocks/lpc.h>

[Marshall Dawson (Code Review)]

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

Change subject: soc/amd: factor out common AOAC definitions
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48182/1/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/aoac.h:

https://review.coreboot.org/c/coreboot/+/48182/1/src/soc/amd/common/block/in... 
PS1, Line 12: AOACx0000[40...7E]
Maybe [40...7E:step2].  Same on l.25.  The BKDGs used to use similar indicators but the PPRs are much harder to read IMO.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48182 )

Change subject: soc/amd: factor out common AOAC definitions
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48182/1/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/aoac.h:

https://review.coreboot.org/c/coreboot/+/48182/1/src/soc/amd/common/block/in... 
PS1, Line 12: AOACx0000[40...7E]
> Maybe [40...7E:step2].  Same on l.25. […]

ah, that's what the step 2 I removed there is about. I'll re-add that

[Felix Held (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: soc/amd: factor out common AOAC definitions
......................................................................

soc/amd: factor out common AOAC definitions

The register locations and bit definitions are the same for Stoneyridge
and Picasso. Since not all devices are present on all SoCs, keep those
numbers in the SoC-specific code.

Change-Id: Ib882927e399031c376738e5a35793b3d7654b9cf
Signed-off-by: Felix Held 
---
A src/soc/amd/common/block/include/amdblocks/aoac.h
M src/soc/amd/picasso/aoac.c
M src/soc/amd/picasso/include/soc/southbridge.h
M src/soc/amd/stoneyridge/include/soc/southbridge.h
M src/soc/amd/stoneyridge/southbridge.c
5 files changed, 39 insertions(+), 50 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/48182/2

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48182 )

Change subject: soc/amd: factor out common AOAC definitions
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48182/1/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/aoac.h:

https://review.coreboot.org/c/coreboot/+/48182/1/src/soc/amd/common/block/in... 
PS1, Line 12: AOACx0000[40...7E]
> ah, that's what the step 2 I removed there is about. […]

i ended up using even/odd byte addresses there which is imho easier to understand

[Marshall Dawson (Code Review)]

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

Change subject: soc/amd: factor out common AOAC definitions
......................................................................

Patch Set 2: Code-Review+2

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48182 )

Change subject: soc/amd: factor out common AOAC definitions
......................................................................

soc/amd: factor out common AOAC definitions

The register locations and bit definitions are the same for Stoneyridge
and Picasso. Since not all devices are present on all SoCs, keep those
numbers in the SoC-specific code.

Change-Id: Ib882927e399031c376738e5a35793b3d7654b9cf
Signed-off-by: Felix Held 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48182
Tested-by: build bot (Jenkins) 
Reviewed-by: Marshall Dawson 
---
A src/soc/amd/common/block/include/amdblocks/aoac.h
M src/soc/amd/picasso/aoac.c
M src/soc/amd/picasso/include/soc/southbridge.h
M src/soc/amd/stoneyridge/include/soc/southbridge.h
M src/soc/amd/stoneyridge/southbridge.c
5 files changed, 39 insertions(+), 50 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Marshall Dawson: Looks good to me, approved

diff --git a/src/soc/amd/common/block/include/amdblocks/aoac.h b/src/soc/amd/common/block/include/amdblocks/aoac.h
new file mode 100644
index 0000000..f4f58e6
--- /dev/null
+++ b/src/soc/amd/common/block/include/amdblocks/aoac.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef AMD_BLOCK_AOAC_H
+#define AMD_BLOCK_AOAC_H
+
+#include <types.h>
+
+/* FCH AOAC Registers 0xfed81e00 */
+#define AOAC_DEV_D3_CTL(device)		(0x40 + device * 2)
+#define AOAC_DEV_D3_STATE(device)	(AOAC_DEV_D3_CTL(device) + 1)
+
+/* Bit definitions for Device D3 Control AOACx0000[40...7E; even byte addresses] */
+#define   FCH_AOAC_TARGET_DEVICE_STATE (BIT(0) + BIT(1))
+#define     FCH_AOAC_D0_UNINITIALIZED	0
+#define     FCH_AOAC_D0_INITIALIZED	1
+#define     FCH_AOAC_D1_2_3_WARM	2
+#define     FCH_AOAC_D3_COLD		3
+#define   FCH_AOAC_DEVICE_STATE		BIT(2)
+#define   FCH_AOAC_PWR_ON_DEV		BIT(3)
+#define   FCH_AOAC_SW_PWR_ON_RSTB	BIT(4)
+#define   FCH_AOAC_SW_REF_CLK_OK	BIT(5)
+#define   FCH_AOAC_SW_RST_B		BIT(6)
+#define   FCH_AOAC_IS_SW_CONTROL	BIT(7)
+
+/* Bit definitions for Device D3 State AOACx0000[41...7f; odd byte addresses] */
+#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)
+#define   FCH_AOAC_DEV_OFF_GATING_STATE	BIT(3)
+#define   FCH_AOAC_D3COLD		BIT(4)
+#define   FCH_AOAC_CLK_OK_STATE		BIT(5)
+#define   FCH_AOAC_STAT0		BIT(6)
+#define   FCH_AOAC_STAT1		BIT(7)
+
+#endif /* AMD_BLOCK_AOAC_H */
diff --git a/src/soc/amd/picasso/aoac.c b/src/soc/amd/picasso/aoac.c
index 00f26fe..21031f7 100644
--- a/src/soc/amd/picasso/aoac.c
+++ b/src/soc/amd/picasso/aoac.c
@@ -2,6 +2,7 @@

#include <stdint.h>
 #include <amdblocks/acpimmio.h>
+#include <amdblocks/aoac.h>
 #include <soc/southbridge.h>
 #include <delay.h>

diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h
index d193661..8a2ae49 100644
--- a/src/soc/amd/picasso/include/soc/southbridge.h
+++ b/src/soc/amd/picasso/include/soc/southbridge.h
@@ -183,10 +183,7 @@
 #define   I2C_PAD_CTRL_SPARE0		BIT(17)
 #define   I2C_PAD_CTRL_SPARE1		BIT(18)

-/* FCH AOAC Registers 0xfed81e00 */
-#define AOAC_DEV_D3_CTL(device)		(0x40 + device * 2)
-#define AOAC_DEV_D3_STATE(device)	(AOAC_DEV_D3_CTL(device) + 1)
-
+/* FCH AOAC device offsets for AOAC_DEV_D3_CTL/AOAC_DEV_D3_STATE */
 #define FCH_AOAC_DEV_CLK_GEN		0
 #define FCH_AOAC_DEV_I2C2		7
 #define FCH_AOAC_DEV_I2C3		8
@@ -198,29 +195,6 @@
 #define FCH_AOAC_DEV_UART3		26
 #define FCH_AOAC_DEV_ESPI		27

-/* Bit definitions for Device D3 Control AOACx0000[40...7E] step 2 */
-#define   FCH_AOAC_TARGET_DEVICE_STATE (BIT(0) + BIT(1))
-#define     FCH_AOAC_D0_UNINITIALIZED	0
-#define     FCH_AOAC_D0_INITIALIZED	1
-#define     FCH_AOAC_D1_2_3_WARM	2
-#define     FCH_AOAC_D3_COLD		3
-#define   FCH_AOAC_DEVICE_STATE		BIT(2)
-#define   FCH_AOAC_PWR_ON_DEV		BIT(3)
-#define   FCH_AOAC_SW_PWR_ON_RSTB	BIT(4)
-#define   FCH_AOAC_SW_REF_CLK_OK	BIT(5)
-#define   FCH_AOAC_SW_RST_B		BIT(6)
-#define   FCH_AOAC_IS_SW_CONTROL	BIT(7)
-
-/* 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)
-#define   FCH_AOAC_DEV_OFF_GATING_STATE	BIT(3)
-#define   FCH_AOAC_D3COLD		BIT(4)
-#define   FCH_AOAC_CLK_OK_STATE		BIT(5)
-#define   FCH_AOAC_STAT0		BIT(6)
-#define   FCH_AOAC_STAT1		BIT(7)
-
 #define FCH_LEGACY_UART_DECODE		(ALINK_AHB_ADDRESS + 0x20) /* 0xfedc0020 */
 #define   FCH_LEGACY_UART_MAP_SHIFT	8
 #define   FCH_LEGACY_UART_MAP_SIZE	2
diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h
index 9450a92..59843c1 100644
--- a/src/soc/amd/stoneyridge/include/soc/southbridge.h
+++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h
@@ -189,10 +189,7 @@
 #define   DEBUG_PORT_ENABLE		  BIT(18)
 #define   DEBUG_PORT_MASK		(BIT(16) | BIT(17) | BIT(18))

-/* FCH AOAC Registers 0xfed81e00 */
-#define AOAC_DEV_D3_CTL(device)		(0x40 + device * 2)
-#define AOAC_DEV_D3_STATE(device)	(AOAC_DEV_D3_CTL(device) + 1)
-
+/* FCH AOAC device offsets for AOAC_DEV_D3_CTL/AOAC_DEV_D3_STATE */
 #define FCH_AOAC_DEV_CLK_GEN		0
 #define FCH_AOAC_DEV_I2C0		5
 #define FCH_AOAC_DEV_I2C1		6
@@ -204,25 +201,6 @@
 #define FCH_AOAC_DEV_USB2		18
 #define FCH_AOAC_DEV_USB3		23

-/* Bit definitions for all FCH_AOAC_D3_CONTROL_* Registers */
-#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)
-#define   FCH_AOAC_SW_PWR_ON_RSTB	BIT(4)
-#define   FCH_AOAC_SW_REF_CLK_OK	BIT(5)
-#define   FCH_AOAC_SW_RST_B		BIT(6)
-#define   FCH_AOAC_IS_SW_CONTROL	BIT(7)
-
-/* Bit definitions for all FCH_AOAC_D3_STATE_* Registers */
-#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)
-#define   FCH_AOAC_DEV_OFF_GATING_STATE	BIT(3)
-#define   FCH_AOAC_D3COLD		BIT(4)
-#define   FCH_AOAC_CLK_OK_STATE		BIT(5)
-#define   FCH_AOAC_STAT0		BIT(6)
-#define   FCH_AOAC_STAT1		BIT(7)
-
 #define PM1_LIMIT			16
 #define GPE0_LIMIT			28
 #define TOTAL_BITS(a)			(8 * sizeof(a))
diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c
index 05f3072..b427c18 100644
--- a/src/soc/amd/stoneyridge/southbridge.c
+++ b/src/soc/amd/stoneyridge/southbridge.c
@@ -12,6 +12,7 @@
 #include <acpi/acpi_gnvs.h>
 #include <amdblocks/amd_pci_util.h>
 #include <amdblocks/agesawrapper.h>
+#include <amdblocks/aoac.h>
 #include <amdblocks/reset.h>
 #include <amdblocks/acpimmio.h>
 #include <amdblocks/lpc.h>