Change in coreboot[master]: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols

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

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

[WIP] soc/amd/common: Access ACPI MMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addesses implemented in
the platform.

TBD: Check for bad aliasing effects when immediate value changes
to memory reference.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
3 files changed, 125 insertions(+), 73 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/1

diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c
index 04d5e4a..51e661d 100644
--- a/src/soc/amd/common/block/acpimmio/mmio_util.c
+++ b/src/soc/amd/common/block/acpimmio/mmio_util.c
@@ -18,6 +18,28 @@
 #include <amdblocks/acpimmio_map.h>
 #include <amdblocks/acpimmio.h>

+uint8_t *const acpimmio_sm_pci	= (void *)ACPIMMIO_SM_PCI_BASE;
+uint8_t *const acpimmio_smi	= (void *)ACPIMMIO_SMI_BASE;
+uint8_t *const acpimmio_pmio	= (void *)ACPIMMIO_PMIO_BASE;
+uint8_t *const acpimmio_pmio2	= (void *)ACPIMMIO_PMIO2_BASE;
+uint8_t *const acpimmio_biosram	= (void *)ACPIMMIO_BIOSRAM_BASE;
+uint8_t *const acpimmio_cmosram	= (void *)ACPIMMIO_CMOSRAM_BASE;
+uint8_t *const acpimmio_cmos	= (void *)ACPIMMIO_CMOS_BASE;
+uint8_t *const acpimmio_acpi	= (void *)ACPIMMIO_ACPI_BASE;
+uint8_t *const acpimmio_asf	= (void *)ACPIMMIO_ASF_BASE;
+uint8_t *const acpimmio_smbus	= (void *)ACPIMMIO_SMBUS_BASE;
+uint8_t *const acpimmio_wdt	= (void *)ACPIMMIO_WDT_BASE;
+uint8_t *const acpimmio_hpet	= (void *)ACPIMMIO_HPET_BASE;
+uint8_t *const acpimmio_iomux	= (void *)ACPIMMIO_IOMUX_BASE;
+uint8_t *const acpimmio_misc	= (void *)ACPIMMIO_MISC_BASE;
+uint8_t *const acpimmio_dpvga	= (void *)ACPIMMIO_DPVGA_BASE;
+uint8_t *const acpimmio_gpio0	= (void *)ACPIMMIO_GPIO0_BASE;
+uint8_t *const acpimmio_gpio1	= (void *)ACPIMMIO_GPIO1_BASE;
+uint8_t *const acpimmio_gpio2	= (void *)ACPIMMIO_GPIO2_BASE;
+uint8_t *const acpimmio_xhci_pm	= (void *)ACPIMMIO_XHCIPM_BASE;
+uint8_t *const acpimmio_acdc_tmr	= (void *)ACPIMMIO_ACDCTMR_BASE;
+uint8_t *const acpimmio_aoac	= (void *)ACPIMMIO_AOAC_BASE;
+
 void enable_acpimmio_decode(void)
 {
    uint32_t dw;
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
index 57d24db..9df47d1 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
@@ -121,272 +121,272 @@

static inline uint8_t sm_pci_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_SM_PCI_BASE + reg));
+	return read8(acpimmio_sm_pci + reg);
 }

static inline uint16_t sm_pci_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_SM_PCI_BASE + reg));
+	return read16(acpimmio_sm_pci + reg);
 }

static inline uint32_t sm_pci_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_SM_PCI_BASE + reg));
+	return read32(acpimmio_sm_pci + reg);
 }

static inline void sm_pci_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_SM_PCI_BASE + reg), value);
+	write8(acpimmio_sm_pci + reg, value);
 }

static inline void sm_pci_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_SM_PCI_BASE + reg), value);
+	write16(acpimmio_sm_pci + reg, value);
 }

static inline void sm_pci_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_SM_PCI_BASE + reg), value);
+	write32(acpimmio_sm_pci + reg, value);
 }

static inline uint8_t smi_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_SMI_BASE + reg));
+	return read8(acpimmio_smi + reg);
 }

static inline uint16_t smi_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_SMI_BASE + reg));
+	return read16(acpimmio_smi + reg);
 }

static inline uint32_t smi_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_SMI_BASE + reg));
+	return read32(acpimmio_smi + reg);
 }

static inline void smi_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_SMI_BASE + reg), value);
+	write8(acpimmio_smi + reg, value);
 }

static inline void smi_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_SMI_BASE + reg), value);
+	write16(acpimmio_smi + reg, value);
 }

static inline void smi_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_SMI_BASE + reg), value);
+	write32(acpimmio_smi + reg, value);
 }

static inline uint8_t pm_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_PMIO_BASE + reg));
+	return read8(acpimmio_pmio + reg);
 }

static inline uint16_t pm_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_PMIO_BASE + reg));
+	return read16(acpimmio_pmio + reg);
 }

static inline uint32_t pm_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_PMIO_BASE + reg));
+	return read32(acpimmio_pmio + reg);
 }

static inline void pm_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_PMIO_BASE + reg), value);
+	write8(acpimmio_pmio + reg, value);
 }

static inline void pm_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_PMIO_BASE + reg), value);
+	write16(acpimmio_pmio + reg, value);
 }

static inline void pm_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_PMIO_BASE + reg), value);
+	write32(acpimmio_pmio + reg, value);
 }

static inline uint8_t biosram_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_BIOSRAM_BASE + reg));
+	return read8(acpimmio_biosram + reg);
 }

static inline void biosram_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_BIOSRAM_BASE + reg), value);
+	write8(acpimmio_biosram + reg, value);
 }

static inline uint8_t acpi_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_ACPI_BASE + reg));
+	return read8(acpimmio_acpi + reg);
 }

static inline uint16_t acpi_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_ACPI_BASE + reg));
+	return read16(acpimmio_acpi + reg);
 }

static inline uint32_t acpi_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_ACPI_BASE + reg));
+	return read32(acpimmio_acpi + reg);
 }

static inline void acpi_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_ACPI_BASE + reg), value);
+	write8(acpimmio_acpi + reg, value);
 }

static inline void acpi_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_ACPI_BASE + reg), value);
+	write16(acpimmio_acpi + reg, value);
 }

static inline void acpi_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_ACPI_BASE + reg), value);
+	write32(acpimmio_acpi + reg, value);
 }

static inline uint8_t asf_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_ASF_BASE + reg));
+	return read8(acpimmio_asf + reg);
 }

static inline uint16_t asf_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_ASF_BASE + reg));
+	return read16(acpimmio_asf + reg);
 }

static inline void asf_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_ASF_BASE + reg), value);
+	write8(acpimmio_asf + reg, value);
 }

static inline void asf_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_ASF_BASE + reg), value);
+	write16(acpimmio_asf + reg, value);
 }

static inline uint8_t smbus_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_SMBUS_BASE + reg));
+	return read8(acpimmio_smbus + reg);
 }

static inline uint16_t smbus_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_SMBUS_BASE + reg));
+	return read16(acpimmio_smbus + reg);
 }

static inline void smbus_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_SMBUS_BASE + reg), value);
+	write8(acpimmio_smbus + reg, value);
 }

static inline void smbus_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_SMBUS_BASE + reg), value);
+	write16(acpimmio_smbus + reg, value);
 }

static inline uint8_t iomux_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_IOMUX_BASE + reg));
+	return read8(acpimmio_iomux + reg);
 }

static inline uint16_t iomux_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_IOMUX_BASE + reg));
+	return read16(acpimmio_iomux + reg);
 }

static inline uint32_t iomux_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_IOMUX_BASE + reg));
+	return read32(acpimmio_iomux + reg);
 }

static inline void iomux_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
+	write8(acpimmio_iomux + reg, value);
 }

static inline void iomux_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
+	write16(acpimmio_iomux + reg, value);
 }

static inline void iomux_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
+	write32(acpimmio_iomux + reg, value);
 }

static inline uint8_t misc_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_MISC_BASE + reg));
+	return read8(acpimmio_misc + reg);
 }

static inline uint16_t misc_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_MISC_BASE + reg));
+	return read16(acpimmio_misc + reg);
 }

static inline uint32_t misc_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_MISC_BASE + reg));
+	return read32(acpimmio_misc + reg);
 }

static inline void misc_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_MISC_BASE + reg), value);
+	write8(acpimmio_misc + reg, value);
 }

static inline void misc_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_MISC_BASE + reg), value);
+	write16(acpimmio_misc + reg, value);
 }

static inline void misc_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_MISC_BASE + reg), value);
+	write32(acpimmio_misc + reg, value);
 }

static inline uint8_t xhci_pm_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_XHCIPM_BASE + reg));
+	return read8(acpimmio_xhci_pm + reg);
 }

static inline uint16_t xhci_pm_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_XHCIPM_BASE + reg));
+	return read16(acpimmio_xhci_pm + reg);
 }

static inline uint32_t xhci_pm_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_XHCIPM_BASE + reg));
+	return read32(acpimmio_xhci_pm + reg);
 }

static inline void xhci_pm_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
+	write8(acpimmio_xhci_pm + reg, value);
 }

static inline void xhci_pm_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
+	write16(acpimmio_xhci_pm + reg, value);
 }

static inline void xhci_pm_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
+	write32(acpimmio_xhci_pm + reg, value);
 }

static inline uint8_t aoac_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_AOAC_BASE + reg));
+	return read8(acpimmio_aoac + reg);
 }

static inline void aoac_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_AOAC_BASE + reg), value);
+	write8(acpimmio_aoac + reg, value);
 }

#endif /* __AMDBLOCKS_ACPIMMIO_H__ */
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
index 755af52..4f64ee6 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
@@ -42,26 +42,56 @@
  */

#define AMD_SB_ACPI_MMIO_ADDR		0xfed80000
-#define ACPIMMIO_SM_PCI_BASE		0xfed80000
-#define ACPIMMIO_SMI_BASE		0xfed80200
-#define ACPIMMIO_PMIO_BASE		0xfed80300
-#define ACPIMMIO_PMIO2_BASE		0xfed80400
-#define ACPIMMIO_BIOSRAM_BASE		0xfed80500
-#define ACPIMMIO_CMOSRAM_BASE		0xfed80600
-#define ACPIMMIO_CMOS_BASE		0xfed80700
-#define ACPIMMIO_ACPI_BASE		0xfed80800
-#define ACPIMMIO_ASF_BASE		0xfed80900
-#define ACPIMMIO_SMBUS_BASE		0xfed80a00
-#define ACPIMMIO_WDT_BASE		0xfed80b00
-#define ACPIMMIO_HPET_BASE		0xfed80c00
-#define ACPIMMIO_IOMUX_BASE		0xfed80d00
+
+#ifndef __ACPI__
+#define ACPIMMIO_SM_PCI_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0000)
+#define ACPIMMIO_SMI_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0200)
+#define ACPIMMIO_PMIO_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0300)
+#define ACPIMMIO_PMIO2_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0400)
+#define ACPIMMIO_BIOSRAM_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0500)
+#define ACPIMMIO_CMOSRAM_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0600)
+#define ACPIMMIO_CMOS_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0700)
+#define ACPIMMIO_ACPI_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0800)
+#define ACPIMMIO_ASF_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0900)
+#define ACPIMMIO_SMBUS_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0a00)
+#define ACPIMMIO_WDT_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0b00)
+#define ACPIMMIO_HPET_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0c00)
+#define ACPIMMIO_IOMUX_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0d00)
+#define ACPIMMIO_MISC_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x0e00)
+#define ACPIMMIO_DPVGA_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x1400)
+#define ACPIMMIO_GPIO0_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x1500)
+#define ACPIMMIO_GPIO1_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x1600)
+#define ACPIMMIO_GPIO2_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x1700)
+#define ACPIMMIO_XHCIPM_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x1c00)
+#define ACPIMMIO_ACDCTMR_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x1d00)
+#define ACPIMMIO_AOAC_BASE		(AMD_SB_ACPI_MMIO_ADDR + 0x1e00)
+
+extern uint8_t *const acpimmio_sm_pci;
+extern uint8_t *const acpimmio_smi;
+extern uint8_t *const acpimmio_pmio;
+extern uint8_t *const acpimmio_pmio2;
+extern uint8_t *const acpimmio_biosram;
+extern uint8_t *const acpimmio_cmosram;
+extern uint8_t *const acpimmio_cmos;
+extern uint8_t *const acpimmio_acpi;
+extern uint8_t *const acpimmio_asf;
+extern uint8_t *const acpimmio_smbus;
+extern uint8_t *const acpimmio_wdt;
+extern uint8_t *const acpimmio_hpet;
+extern uint8_t *const acpimmio_iomux;
+extern uint8_t *const acpimmio_misc;
+extern uint8_t *const acpimmio_dpvga;
+extern uint8_t *const acpimmio_gpio0;
+extern uint8_t *const acpimmio_gpio1;
+extern uint8_t *const acpimmio_gpio2;
+extern uint8_t *const acpimmio_xhci_pm;
+extern uint8_t *const acpimmio_acdc_tmr;
+extern uint8_t *const acpimmio_aoac;
+#else
+
+/* Damn, ASL fails on additions. */
 #define ACPIMMIO_MISC_BASE		0xfed80e00
-#define ACPIMMIO_DPVGA_BASE		0xfed81400
 #define ACPIMMIO_GPIO0_BASE		0xfed81500
-#define ACPIMMIO_GPIO1_BASE		0xfed81600
-#define ACPIMMIO_GPIO2_BASE		0xfed81700
-#define ACPIMMIO_XHCIPM_BASE		0xfed81c00
-#define ACPIMMIO_ACDCTMR_BASE		0xfed81d00
-#define ACPIMMIO_AOAC_BASE		0xfed81e00
+#endif

#endif /* __AMDBLOCKS_ACPIMMIO_MAP_H__ */

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/37324/1//COMMIT_MSG@10 
PS1, Line 10: addesses
addresses

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

Hello build bot (Jenkins),

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

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

[WIP] soc/amd/common: Access ACPI MMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addesses implemented in
the platform.

TBD: Check for bad aliasing effects when immediate value changes
to memory reference.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
3 files changed, 163 insertions(+), 102 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/2

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

Hello build bot (Jenkins),

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

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

[WIP] soc/amd/common: Access ACPI MMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addesses implemented in
the platform.

TBD: Check for bad aliasing effects when immediate value changes
to memory reference.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/picasso/psp.c
7 files changed, 180 insertions(+), 116 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/3

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

Hello build bot (Jenkins),

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

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

[WIP] soc/amd/common: Access ACPI MMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addesses implemented in
the platform.

TBD: Check for bad aliasing effects when immediate value changes
to memory reference.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/google/kahlee/mainboard.c
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/picasso/psp.c
8 files changed, 183 insertions(+), 120 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/4

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 4: Code-Review+1

(2 comments)

Haven't looked at it in detail, but I think it's a good approach.

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/ac... 
File src/soc/amd/common/block/acpimmio/mmio_util.c:

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/ac... 
PS4, Line 28: 	
Indent is off

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/in... 
PS4, Line 42: Damn, 
Remove the swear

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 4: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/in... 
PS4, Line 42: Damn,
> Remove the swear

Or maybe use a minced oath like "Darn" or "Dang" which are not so charged?

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

Hello build bot (Jenkins), Raul Rangel, Angel Pons, Felix Held,

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

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

[WIP] soc/amd/common: Access ACPI MMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addesses implemented in
the platform.

TBD: Check for bad aliasing effects when immediate value changes
to memory reference.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/google/kahlee/mainboard.c
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/picasso/psp.c
8 files changed, 183 insertions(+), 120 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/5

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/ac... 
File src/soc/amd/common/block/acpimmio/mmio_util.c:

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/ac... 
PS4, Line 28:
> Indent is off

Like gpio_100, no room for whitespace unless I realign them all. Immediate followup touches these lines so this eyesore goes away.

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/4/src/soc/amd/common/block/in... 
PS4, Line 42: Damn,
> Or maybe use a minced oath like "Darn" or "Dang" which are not so charged?

Done

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 5:

(2 comments)

Only downside to this is that we're doing and extra read for every access to the bank instead of constants in the instruction stream.

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
PS5, Line 48: #define ACPIMMIO_SM_PCI_BANK		0x0000
These are offsets. Should we name the macros accordingly?

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
PS5, Line 22: #define GPIO_BANK0_CONTROL(gpio) ((uintptr_t)acpimmio_gpio0 + ((gpio) * 4))
nit: add a '- 0' for the offset for consistency with the others?

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
PS5, Line 22: #define GPIO_BANK0_CONTROL(gpio) ((uintptr_t)acpimmio_gpio0 + ((gpio) * 4))
> nit: add a '- 0' for the offset for consistency with the others?

also maybe align the numerical constants to the right:

#define GPIO_BANK0_CONTROL(gpio) ((uintptr_t)acpimmio_gpio0 + (((gpio) -   0) * 4))
 #define GPIO_BANK1_CONTROL(gpio) ((uintptr_t)acpimmio_gpio1 + (((gpio) -  64) * 4))
 #define GPIO_BANK2_CONTROL(gpio) ((uintptr_t)acpimmio_gpio2 + (((gpio) - 128) * 4))

It makes sense to align numbers like that, because ones, tens, hundreds, etc. are all on the same column.

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 5:
> Patch Set 5:
>
> (2 comments)
>
> Only downside to this is that we're doing and extra read for every access to the bank instead of constants in the instruction stream.

Is it a downside? These sequences often do read-modify-writes so the address will appear multiple times in the instruction stream vs loading the base once into a register and using relative offseting. Depends of course of the arch how well it works but constants are not choice for PSP here anyway.

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 5:
> >
> > (2 comments)
> >
> > Only downside to this is that we're doing and extra read for every access to the bank instead of constants in the instruction stream.
>
> Is it a downside? These sequences often do read-modify-writes so the address will appear multiple times in the instruction stream vs loading the base once into a register and using relative offseting. Depends of course of the arch how well it works but constants are not choice for PSP here anyway.

Sure. Depends on the compiler. I should have said "potential downside". Analysis would need to be done, but I'd also expect constant propagation to be used for immediates as well. In the end, I was just commenting on potential downsides. I don't have anything to suggest this is a bad change, though.

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 5:
> > > Patch Set 5:
> > >
> > > (2 comments)
> > >
> > > Only downside to this is that we're doing and extra read for every access to the bank instead of constants in the instruction stream.
> >
> > Is it a downside? These sequences often do read-modify-writes so the address will appear multiple times in the instruction stream vs loading the base once into a register and using relative offseting. Depends of course of the arch how well it works but constants are not choice for PSP here anyway.
>
> Sure. Depends on the compiler. I should have said "potential downside". Analysis would need to be done, but I'd also expect constant propagation to be used for immediates as well. In the end, I was just commenting on potential downsides. I don't have anything to suggest this is a bad change, though.

Indeed. From what I understand psp-verstage user app is now in a state that it builds so I can have a look at objdump -r -d outputs.

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

Hello build bot (Jenkins), Raul Rangel, Angel Pons, Felix Held,

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

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

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

Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols
......................................................................

[WIP] soc/amd/common: Access ACPI MMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addresses implemented in
the platform.

TBD: Check for bad aliasing effects when immediate value changes
to memory reference.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/google/kahlee/mainboard.c
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/picasso/psp.c
8 files changed, 183 insertions(+), 120 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/6

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Set Ready For Review

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 12:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/g... 
File src/soc/amd/common/block/gpio_banks/gpio.c:

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/g... 
PS12, Line 183: 	inter_master = (void *)acpimmio_gpio0 + GPIO_MASTER_SWITCH;
i'm being pedantic, but we can we please not do arithmetic on void * pointers?

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio.h:

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/i... 
PS12, Line 15: this newer method
What is the newer method?

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 12:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37324/12//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/37324/12//COMMIT_MSG@10 
PS12, Line 10: addesses
add*r*esses

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/g... 
File src/soc/amd/common/block/gpio_banks/gpio.c:

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/g... 
PS12, Line 183: 	inter_master = (void *)acpimmio_gpio0 + GPIO_MASTER_SWITCH;
> i'm being pedantic, but we can we please not do arithmetic on void * pointers?

if acpimmio has a fixed format, how about using a memory-mapped struct? Or would that be too cursed?

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

Hello build bot (Jenkins), Raul Rangel, Angel Pons, Felix Held,

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

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

soc/amd/common: Access ACPIMMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addresses implemented for
the specific platform and executing stage.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/amd/inagua/BiosCallOuts.c
M src/mainboard/amd/persimmon/BiosCallOuts.c
M src/mainboard/amd/south_station/BiosCallOuts.c
M src/mainboard/amd/union_station/BiosCallOuts.c
M src/mainboard/asrock/e350m1/BiosCallOuts.c
M src/mainboard/elmex/pcm205400/BiosCallOuts.c
M src/mainboard/google/kahlee/mainboard.c
M src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/common/block/smbus/smbus.c
M src/soc/amd/picasso/psp.c
M src/soc/amd/stoneyridge/smbus_spd.c
17 files changed, 285 insertions(+), 226 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/13

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13:

(3 comments)

https://review.coreboot.org/c/coreboot/+/37324/12//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/37324/12//COMMIT_MSG@10 
PS12, Line 10: addesses
> add*r*esses

Done

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/g... 
File src/soc/amd/common/block/gpio_banks/gpio.c:

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/g... 
PS12, Line 183: 	inter_master = (void *)acpimmio_gpio0 + GPIO_MASTER_SWITCH;
> i'm being pedantic, but we can we please not do arithmetic on void * pointers?

Done

As for memory mapped struct, see CB:42692.

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio.h:

https://review.coreboot.org/c/coreboot/+/37324/12/src/soc/amd/common/block/i... 
PS12, Line 15: this newer method
> What is the newer method?

Done

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13: Code-Review+2

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13: Code-Review+2

(2 comments)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
File src/soc/amd/common/block/acpimmio/mmio_util.c:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
PS13, Line 9:  
Is the indentation off or is it just gerrit?

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 99: ASL fails on additions
What fails specifically?

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 117: ACPIMMIO_WDT_BANK
Why the rename from BASE to BANK? The term BANK is only used for the GPIO banks.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Marshall Dawson (Code Review)]

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
File src/soc/amd/common/block/acpimmio/mmio_util.c:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
PS13, Line 9:
> Is the indentation off or is it just gerrit?

Looks like a space where all other lines use a tab.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
File src/soc/amd/common/block/acpimmio/mmio_util.c:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
PS13, Line 9:
> Is the indentation off or is it just gerrit?

Seems off indeed

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 117: ACPIMMIO_WDT_BANK
> Why the rename from BASE to BANK? The term BANK is only used for the GPIO banks.

These are not base addresses

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 117: ACPIMMIO_WDT_BANK
> These are not base addresses

How about _OFFSET?

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Martin Roth (Code Review)]

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 117: ACPIMMIO_WDT_BANK
> How about _OFFSET?

Huh? How are these not base addresses?  They're not PCI bars, sure, but they are absolutely base addresses.  You have a base and add an offset.  These are the base addresses for the register blocks associated with them.

"In computing, a base address is an address serving as a reference point ("base") for other addresses. Related addresses can be accessed using an addressing scheme."

https://en.wikipedia.org/wiki/Base_address

I mean, I don't really care about base vs bank, but saying that they're not base addresses isn't right.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 13:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 99: ASL fails on additions
> What fails specifically?

Last time I tried, this:

(AMD_SB_ACPI_MMIO_ADDR + ACPIMMIO_MISC_BANK)

CPP will not do that sum before iasl.

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 117: ACPIMMIO_WDT_BANK
> Huh? How are these not base addresses?  They're not PCI bars, sure, but they are absolutely base add […]

In my vision, these terms relate to how hardware is built.

Something _BASE_ needs to include the most-significant-bit as it used to generate a 'device select' signal when decoding memory transactions from the bus. As for _OFFSET_ my mind expands that implicitly to 'register offset', something that would contain the least-significant-bit and it steps in adjacent memory locations. Common terms for the in-between parts would be _PAGE_ or _BANK_. I associate former with memory and latter more to MMIO where different banks provide different functionality.

Resolve or find an agreement. I don't plan to change this.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

Hello build bot (Jenkins), Raul Rangel, Angel Pons, Aaron Durbin, Felix Held,

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

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

soc/amd/common: Access ACPIMMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addresses implemented for
the specific platform and executing stage.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/amd/inagua/BiosCallOuts.c
M src/mainboard/amd/persimmon/BiosCallOuts.c
M src/mainboard/amd/south_station/BiosCallOuts.c
M src/mainboard/amd/union_station/BiosCallOuts.c
M src/mainboard/asrock/e350m1/BiosCallOuts.c
M src/mainboard/elmex/pcm205400/BiosCallOuts.c
M src/mainboard/google/kahlee/mainboard.c
M src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/common/block/smbus/smbus.c
M src/soc/amd/picasso/psp.c
M src/soc/amd/stoneyridge/smbus_spd.c
17 files changed, 285 insertions(+), 226 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/14

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

Hello build bot (Jenkins), Raul Rangel, Angel Pons, Aaron Durbin, Felix Held,

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

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

soc/amd/common: Access ACPIMMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addresses implemented for
the specific platform and executing stage.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/amd/inagua/BiosCallOuts.c
M src/mainboard/amd/persimmon/BiosCallOuts.c
M src/mainboard/amd/south_station/BiosCallOuts.c
M src/mainboard/amd/union_station/BiosCallOuts.c
M src/mainboard/asrock/e350m1/BiosCallOuts.c
M src/mainboard/elmex/pcm205400/BiosCallOuts.c
M src/mainboard/google/kahlee/mainboard.c
M src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/common/block/smbus/smbus.c
M src/soc/amd/picasso/psp.c
M src/soc/amd/stoneyridge/smbus_spd.c
17 files changed, 285 insertions(+), 226 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/37324/15

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 117: ACPIMMIO_WDT_BANK
> In my vision, these terms relate to how hardware is built. […]

@Martin sorry, I meant *absolute* base addresses. They are indeed base addresses, but relative to ACPIMMIO.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 15: Code-Review+2

(2 comments)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 99: ASL fails on additions
> Last time I tried, this: […]

Strange. I've used addition: https://source.corp.google.com/chromeos_public/src/third_party/coreboot/src/...

marking resolved for now.

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 117: ACPIMMIO_WDT_BANK
> @Martin sorry, I meant *absolute* base addresses. […]

Ack

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/i... 
PS13, Line 99: ASL fails on additions
> Strange. I've used addition: https://source.corp.google. […]

Works now, so I must have typoed something. Probably some mixup with _BASE vs _BANK in the names.

I push followup to change this later.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
File src/soc/amd/common/block/acpimmio/mmio_util.c:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
PS13, Line 9:
> Seems off indeed

An already-approved followup CB:42073 fixes this.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 15:

(3 comments)

https://review.coreboot.org/c/coreboot/+/37324/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/37324/1//COMMIT_MSG@10 
PS1, Line 10: addesses
> addresses

Done

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
File src/soc/amd/common/block/acpimmio/mmio_util.c:

https://review.coreboot.org/c/coreboot/+/37324/13/src/soc/amd/common/block/a... 
PS13, Line 9:
> An already-approved followup CB:42073 fixes this.

Done

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
PS5, Line 22: #define GPIO_BANK0_CONTROL(gpio) ((uintptr_t)acpimmio_gpio0 + ((gpio) * 4))
> also maybe align the numerical constants to the right: […]

Removed with followup.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

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

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

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:

https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... 
PS5, Line 48: #define ACPIMMIO_SM_PCI_BANK		0x0000
> These are offsets. […]

Followup work.

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

Patch Set 15: Code-Review+2

Change in coreboot[master]: soc/amd/common: Access ACPIMMIO via proper symbols

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37324 )

Change subject: soc/amd/common: Access ACPIMMIO via proper symbols
......................................................................

soc/amd/common: Access ACPIMMIO via proper symbols

Using proper symbols for base addresses, it is possible to
only define the symbols for base addresses implemented for
the specific platform and executing stage.

Change-Id: Ib8599ee93bfb1c2d6d9b4accfca1ebbefe758e09
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37324
Tested-by: build bot (Jenkins) 
Reviewed-by: Raul Rangel 
Reviewed-by: Furquan Shaikh 
---
M src/mainboard/amd/inagua/BiosCallOuts.c
M src/mainboard/amd/persimmon/BiosCallOuts.c
M src/mainboard/amd/south_station/BiosCallOuts.c
M src/mainboard/amd/union_station/BiosCallOuts.c
M src/mainboard/asrock/e350m1/BiosCallOuts.c
M src/mainboard/elmex/pcm205400/BiosCallOuts.c
M src/mainboard/google/kahlee/mainboard.c
M src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c
M src/soc/amd/common/block/acpimmio/biosram.c
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
M src/soc/amd/common/block/smbus/smbus.c
M src/soc/amd/picasso/psp.c
M src/soc/amd/stoneyridge/smbus_spd.c
17 files changed, 285 insertions(+), 226 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Raul Rangel: Looks good to me, approved

diff --git a/src/mainboard/amd/inagua/BiosCallOuts.c b/src/mainboard/amd/inagua/BiosCallOuts.c
index 16058b6..56a7f53 100644
--- a/src/mainboard/amd/inagua/BiosCallOuts.c
+++ b/src/mainboard/amd/inagua/BiosCallOuts.c
@@ -38,7 +38,7 @@
    MemData = ConfigPtr;

Status  = AGESA_SUCCESS;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;

Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG178);
    Data8 &= ~BIT5;
@@ -109,7 +109,7 @@

FcnData   = Data;
    ResetInfo = ConfigPtr;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;
    Status = AGESA_UNSUPPORTED;
    switch (ResetInfo->ResetId) {
    case 4:
diff --git a/src/mainboard/amd/persimmon/BiosCallOuts.c b/src/mainboard/amd/persimmon/BiosCallOuts.c
index 1734bc9..bee2f9e 100644
--- a/src/mainboard/amd/persimmon/BiosCallOuts.c
+++ b/src/mainboard/amd/persimmon/BiosCallOuts.c
@@ -45,7 +45,7 @@
    FcnData = Data;
    ResetInfo = ConfigPtr;
    Status = AGESA_UNSUPPORTED;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;
    switch (ResetInfo->ResetId)
    {
    case 46:	// GPIO50 = SBGPIO_PCIE_RST# affects LAN0, LAN1, PCIe slot
diff --git a/src/mainboard/amd/south_station/BiosCallOuts.c b/src/mainboard/amd/south_station/BiosCallOuts.c
index a4438f5..6216a3f 100644
--- a/src/mainboard/amd/south_station/BiosCallOuts.c
+++ b/src/mainboard/amd/south_station/BiosCallOuts.c
@@ -38,7 +38,7 @@
    MemData = ConfigPtr;

Status = AGESA_SUCCESS;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;

Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG178);
    Data8 &= ~BIT5;
@@ -106,7 +106,7 @@
    UINT32                 GpioMmioAddr;
    UINT8                  Data8;

-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;

FcnData = Data;
    ResetInfo = ConfigPtr;
diff --git a/src/mainboard/amd/union_station/BiosCallOuts.c b/src/mainboard/amd/union_station/BiosCallOuts.c
index deb6176..c324fd6 100644
--- a/src/mainboard/amd/union_station/BiosCallOuts.c
+++ b/src/mainboard/amd/union_station/BiosCallOuts.c
@@ -38,7 +38,7 @@
    MemData = ConfigPtr;

Status = AGESA_SUCCESS;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;

Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG178);
    Data8 &= ~BIT5;
@@ -106,7 +106,7 @@
    UINT32                  GpioMmioAddr;
    UINT8                   Data8;

-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;

FcnData = Data;
    ResetInfo = ConfigPtr;
diff --git a/src/mainboard/asrock/e350m1/BiosCallOuts.c b/src/mainboard/asrock/e350m1/BiosCallOuts.c
index a3d5337..6bd2f98 100644
--- a/src/mainboard/asrock/e350m1/BiosCallOuts.c
+++ b/src/mainboard/asrock/e350m1/BiosCallOuts.c
@@ -36,7 +36,7 @@
    MemData = ConfigPtr;

Status = AGESA_SUCCESS;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;

Data8 = Read64Mem8(GpioMmioAddr+SB_GPIO_REG178);
    Data8 &= ~BIT5;
diff --git a/src/mainboard/elmex/pcm205400/BiosCallOuts.c b/src/mainboard/elmex/pcm205400/BiosCallOuts.c
index 1734bc9..bee2f9e 100644
--- a/src/mainboard/elmex/pcm205400/BiosCallOuts.c
+++ b/src/mainboard/elmex/pcm205400/BiosCallOuts.c
@@ -45,7 +45,7 @@
    FcnData = Data;
    ResetInfo = ConfigPtr;
    Status = AGESA_UNSUPPORTED;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;
    switch (ResetInfo->ResetId)
    {
    case 46:	// GPIO50 = SBGPIO_PCIE_RST# affects LAN0, LAN1, PCIe slot
diff --git a/src/mainboard/google/kahlee/mainboard.c b/src/mainboard/google/kahlee/mainboard.c
index 239887e..5e1bdf4 100644
--- a/src/mainboard/google/kahlee/mainboard.c
+++ b/src/mainboard/google/kahlee/mainboard.c
@@ -134,13 +134,13 @@
    pm_write8(PM_PCIB_CFG, pm_read8(PM_PCIB_CFG) | PM_GENINT_DISABLE);

/* Set low-power mode for BayHub eMMC bridge's PCIe clock. */
-	clrsetbits32((uint32_t *)(ACPIMMIO_MISC_BASE + GPP_CLK_CNTRL),
+	clrsetbits32(acpimmio_misc + GPP_CLK_CNTRL,
    	     GPP_CLK2_REQ_MAP_MASK,
    	     GPP_CLK2_REQ_MAP_CLK_REQ2 <<
    	     GPP_CLK2_REQ_MAP_SHIFT);

/* Same for the WiFi */
-	clrsetbits32((uint32_t *)(ACPIMMIO_MISC_BASE + GPP_CLK_CNTRL),
+	clrsetbits32(acpimmio_misc + GPP_CLK_CNTRL,
    	     GPP_CLK0_REQ_MAP_MASK,
    	     GPP_CLK0_REQ_MAP_CLK_REQ0 <<
    	     GPP_CLK0_REQ_MAP_SHIFT);
diff --git a/src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c b/src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c
index 2129509..64ba978 100644
--- a/src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c
+++ b/src/mainboard/jetway/nf81-t56n-lf/BiosCallOuts.c
@@ -48,7 +48,7 @@
    FcnData = Data;
    ResetInfo = ConfigPtr;
    Status = AGESA_UNSUPPORTED;
-	GpioMmioAddr = ACPIMMIO_GPIO_BASE_100;
+	GpioMmioAddr = (uintptr_t)acpimmio_gpio_100;
    switch (ResetInfo->ResetId)
    {
    case 46:	/* GPIO50 = SBGPIO_PCIE_RST# affects LAN0, LAN1, PCIe slot */
diff --git a/src/soc/amd/common/block/acpimmio/biosram.c b/src/soc/amd/common/block/acpimmio/biosram.c
index a7bb9ff..b01c196 100644
--- a/src/soc/amd/common/block/acpimmio/biosram.c
+++ b/src/soc/amd/common/block/acpimmio/biosram.c
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

-#include <amdblocks/acpimmio_map.h>
+#include <amdblocks/acpimmio.h>
 #include <amdblocks/biosram.h>
 #include <cbmem.h>
 #include <device/mmio.h>
@@ -14,12 +14,12 @@

static uint8_t biosram_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_BIOSRAM_BASE + reg));
+	return read8(acpimmio_biosram + reg);
 }

static void biosram_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_BIOSRAM_BASE + reg), value);
+	write8(acpimmio_biosram + reg, value);
 }

static uint16_t biosram_read16(uint8_t reg) /* Must be 1 byte at a time */
diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c
index 7126851..b3d3332 100644
--- a/src/soc/amd/common/block/acpimmio/mmio_util.c
+++ b/src/soc/amd/common/block/acpimmio/mmio_util.c
@@ -5,6 +5,29 @@
 #include <amdblocks/acpimmio_map.h>
 #include <amdblocks/acpimmio.h>

+uint8_t *const acpimmio_sm_pci	= ACPIMMIO_BASE(SM_PCI);
+uint8_t *const acpimmio_gpio_100 = ACPIMMIO_BASE(GPIO_100);
+uint8_t *const acpimmio_smi	= ACPIMMIO_BASE(SMI);
+uint8_t *const acpimmio_pmio	= ACPIMMIO_BASE(PMIO);
+uint8_t *const acpimmio_pmio2	= ACPIMMIO_BASE(PMIO2);
+uint8_t *const acpimmio_biosram	= ACPIMMIO_BASE(BIOSRAM);
+uint8_t *const acpimmio_cmosram	= ACPIMMIO_BASE(CMOSRAM);
+uint8_t *const acpimmio_cmos	= ACPIMMIO_BASE(CMOS);
+uint8_t *const acpimmio_acpi	= ACPIMMIO_BASE(ACPI);
+uint8_t *const acpimmio_asf	= ACPIMMIO_BASE(ASF);
+uint8_t *const acpimmio_smbus	= ACPIMMIO_BASE(SMBUS);
+uint8_t *const acpimmio_wdt	= ACPIMMIO_BASE(WDT);
+uint8_t *const acpimmio_hpet	= ACPIMMIO_BASE(HPET);
+uint8_t *const acpimmio_iomux	= ACPIMMIO_BASE(IOMUX);
+uint8_t *const acpimmio_misc	= ACPIMMIO_BASE(MISC);
+uint8_t *const acpimmio_dpvga	= ACPIMMIO_BASE(DPVGA);
+uint8_t *const acpimmio_gpio0	= ACPIMMIO_BASE(GPIO0);
+uint8_t *const acpimmio_gpio1	= ACPIMMIO_BASE(GPIO1);
+uint8_t *const acpimmio_gpio2	= ACPIMMIO_BASE(GPIO2);
+uint8_t *const acpimmio_xhci_pm	= ACPIMMIO_BASE(XHCIPM);
+uint8_t *const acpimmio_acdc_tmr	= ACPIMMIO_BASE(ACDCTMR);
+uint8_t *const acpimmio_aoac	= ACPIMMIO_BASE(AOAC);
+
 void enable_acpimmio_decode_pm24(void)
 {
    uint32_t dw;
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index 81ea725..d67e2d0 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -5,7 +5,6 @@
 #include <console/console.h>
 #include <gpio.h>
 #include <amdblocks/acpimmio.h>
-#include <amdblocks/acpimmio_map.h>
 #include <soc/gpio.h>
 #include <soc/smi.h>
 #include <assert.h>
@@ -181,8 +180,7 @@
    const struct soc_amd_event *gev_tbl;
    size_t gev_items;

-	inter_master = (uint32_t *)(uintptr_t)(ACPIMMIO_GPIO0_BASE
-					       + GPIO_MASTER_SWITCH);
+	inter_master = (void *)(acpimmio_gpio0 + GPIO_MASTER_SWITCH);
    direction = 0;
    edge_level = 0;
    mask = 0;
@@ -270,14 +268,10 @@
    mem_read_write32(inter_master, GPIO_INTERRUPT_EN, GPIO_INTERRUPT_EN);

/* Set all SCI trigger direction (high/low) */
-	mem_read_write32((uint32_t *)
-			(uintptr_t)(ACPIMMIO_SMI_BASE + SMI_SCI_TRIG),
-					direction, mask);
+	mem_read_write32((void *)(acpimmio_smi + SMI_SCI_TRIG), direction, mask);

/* Set all SCI trigger level (edge/level) */
-	mem_read_write32((uint32_t *)
-			(uintptr_t)(ACPIMMIO_SMI_BASE + SMI_SCI_LEVEL),
-					edge_level, mask);
+	mem_read_write32((void *)(acpimmio_smi + SMI_SCI_LEVEL), edge_level, mask);
 }

int gpio_interrupt_status(gpio_t gpio)
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
index 84a9360..3d0cf06 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
@@ -5,86 +5,46 @@

#include <device/mmio.h>
 #include <types.h>
-#include <amdblocks/acpimmio_map.h>
+
+/* IO index/data for accessing PMIO prior to enabling MMIO decode */
+#define PM_INDEX			0xcd6
+#define PM_DATA				0xcd7

/*
- * The following AcpiMmio register block mapping represents definitions
- * that have been documented in AMD publications.  All blocks aren't
- * implemented in all products, so the caller should be careful not to
- * inadvertently access a non-existent block.  The definitions within
- * each block are also subject to change across products.  Please refer
- * to the appropriate RRG, the BKDG, or PPR for the product.
- *
- * The base address is configurable in older products, but defaults to
- * 0xfed80000.  The address is fixed at 0xfed80000 in newer products.
- *
- * +---------------------------------------------------------------------------+
- * |0x000    SMBus PCI space                                                   |
- * |           * Dual-mapped to PCI configuration header of D14F0              |
- * +---------------------------------------------------------------------------+
- * |0x100    GPIO configuration registers                                      |
- * |           * old style, never implemented with newer style                 |
- * +---------------------------------------------------------------------------+
- * |0x200    SMI configuration registers                                       |
- * +---------------------------------------------------------------------------+
- * |0x300    Power Management registers                                        |
- * |           * Dual-mapped via IO Index/Data 0xcd6/0xcd7 (byte access only)  |
- * +---------------------------------------------------------------------------+
- * |0x400    Power Management 2 registers                                      |
- * +---------------------------------------------------------------------------+
- * |0x500    BIOS RAM                                                          |
- * |           * General-purpose storage in S3 domain                          |
- * |           * Byte access only                                              |
- * +---------------------------------------------------------------------------+
- * |0x600    CMOS RAM                                                          |
- * |           * Dual-mapped to storage at Alt RTC Index/Data (0x72/0x73)      |
- * |           * Byte access only                                              |
- * +---------------------------------------------------------------------------+
- * |0x700    CMOS                                                              |
- * |           * Dual-mapped to storage at RTC Index/Data (0x70/0x71)          |
- * |           * Byte access only                                              |
- * +---------------------------------------------------------------------------+
- * |0x800    Standard ACPI registers                                           |
- * |           * Dual-mapped to I/O ACPI registers                             |
- * +---------------------------------------------------------------------------+
- * |0x900    ASF controller registers                                          |
- * |           * Dual-mapped to I/O ASF controller registers                   |
- * +---------------------------------------------------------------------------+
- * |0xa00    SMBus controller registers                                        |
- * |           * Dual-mapped to I/O SMBus controller registers                 |
- * +---------------------------------------------------------------------------+
- * |0xb00    WDT registers                                                     |
- * |           * Dual-mapped to WDT registers, typ. enabled at 0xfeb00000      |
- * +---------------------------------------------------------------------------+
- * |0xc00    HPET registers                                                    |
- * |           * Dual-mapped to HPET registers, typ. enabled at 0xfed00000     |
- * +---------------------------------------------------------------------------+
- * |0xd00    MUX configuration registers for GPIO signals                      |
- * +---------------------------------------------------------------------------+
- * |0xe00    Miscellaneous registers                                           |
- * +---------------------------------------------------------------------------+
- * |0x1000   Serial debug bus                                                  |
- * +---------------------------------------------------------------------------+
- * |0x1400   DP-VGA                                                            |
- * +---------------------------------------------------------------------------+
- * |0x1500   GPIO configuration registers bank 0                               |
- * |           * new style, never implemented with older style                 |
- * +---------------------------------------------------------------------------+
- * |0x1600   GPIO configuration registers bank 1                               |
- * |           * new style, never implemented with older style                 |
- * +---------------------------------------------------------------------------+
- * |0x1700   GPIO configuration registers bank 2                               |
- * |           * new style, never implemented with older style                 |
- * +---------------------------------------------------------------------------+
- * |0x1c00   xHCI Power Management registers                                   |
- * +---------------------------------------------------------------------------+
- * |0x1d00   Wake device (AC DC timer)                                         |
- * +---------------------------------------------------------------------------+
- * |0x1e00   Always On Always Connect registers                                |
- * +---------------------------------------------------------------------------+
+ * Earlier devices enable the ACPIMMIO bank decodes in PMx24. All discrete FCHs
+ * and the Kabini SoC fall into this category. Kabini's successor, Mullins, uses
+ * this newer method of enable in PMx04.
  */

-/* Enable the AcpiMmio range at 0xfed80000 */
+#define ACPIMMIO_DECODE_REGISTER_24 0x24
+#define   PM_24_ACPIMMIO_DECODE_EN	BIT(0)
+
+#define ACPIMMIO_DECODE_REGISTER_04 0x4
+#define   PM_04_BIOSRAM_DECODE_EN	BIT(0)
+#define   PM_04_ACPIMMIO_DECODE_EN	BIT(1)
+
+extern uint8_t *const acpimmio_gpio_100;
+extern uint8_t *const acpimmio_sm_pci;
+extern uint8_t *const acpimmio_smi;
+extern uint8_t *const acpimmio_pmio;
+extern uint8_t *const acpimmio_pmio2;
+extern uint8_t *const acpimmio_biosram;
+extern uint8_t *const acpimmio_cmosram;
+extern uint8_t *const acpimmio_cmos;
+extern uint8_t *const acpimmio_acpi;
+extern uint8_t *const acpimmio_asf;
+extern uint8_t *const acpimmio_smbus;
+extern uint8_t *const acpimmio_wdt;
+extern uint8_t *const acpimmio_hpet;
+extern uint8_t *const acpimmio_iomux;
+extern uint8_t *const acpimmio_misc;
+extern uint8_t *const acpimmio_dpvga;
+extern uint8_t *const acpimmio_gpio0;
+extern uint8_t *const acpimmio_gpio1;
+extern uint8_t *const acpimmio_gpio2;
+extern uint8_t *const acpimmio_xhci_pm;
+extern uint8_t *const acpimmio_acdc_tmr;
+extern uint8_t *const acpimmio_aoac;

/* For older discrete FCHs */
 void enable_acpimmio_decode_pm24(void);
@@ -102,417 +62,417 @@

static inline uint8_t sm_pci_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_SM_PCI_BASE + reg));
+	return read8(acpimmio_sm_pci + reg);
 }

static inline uint16_t sm_pci_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_SM_PCI_BASE + reg));
+	return read16(acpimmio_sm_pci + reg);
 }

static inline uint32_t sm_pci_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_SM_PCI_BASE + reg));
+	return read32(acpimmio_sm_pci + reg);
 }

static inline void sm_pci_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_SM_PCI_BASE + reg), value);
+	write8(acpimmio_sm_pci + reg, value);
 }

static inline void sm_pci_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_SM_PCI_BASE + reg), value);
+	write16(acpimmio_sm_pci + reg, value);
 }

static inline void sm_pci_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_SM_PCI_BASE + reg), value);
+	write32(acpimmio_sm_pci + reg, value);
 }

static inline uint8_t smi_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_SMI_BASE + reg));
+	return read8(acpimmio_smi + reg);
 }

static inline uint16_t smi_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_SMI_BASE + reg));
+	return read16(acpimmio_smi + reg);
 }

static inline uint32_t smi_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_SMI_BASE + reg));
+	return read32(acpimmio_smi + reg);
 }

static inline void smi_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_SMI_BASE + reg), value);
+	write8(acpimmio_smi + reg, value);
 }

static inline void smi_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_SMI_BASE + reg), value);
+	write16(acpimmio_smi + reg, value);
 }

static inline void smi_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_SMI_BASE + reg), value);
+	write32(acpimmio_smi + reg, value);
 }

static inline uint8_t pm_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_PMIO_BASE + reg));
+	return read8(acpimmio_pmio + reg);
 }

static inline uint16_t pm_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_PMIO_BASE + reg));
+	return read16(acpimmio_pmio + reg);
 }

static inline uint32_t pm_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_PMIO_BASE + reg));
+	return read32(acpimmio_pmio + reg);
 }

static inline void pm_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_PMIO_BASE + reg), value);
+	write8(acpimmio_pmio + reg, value);
 }

static inline void pm_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_PMIO_BASE + reg), value);
+	write16(acpimmio_pmio + reg, value);
 }

static inline void pm_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_PMIO_BASE + reg), value);
+	write32(acpimmio_pmio + reg, value);
 }

static inline uint8_t pm2_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_PMIO2_BASE + reg));
+	return read8(acpimmio_pmio2 + reg);
 }

static inline uint16_t pm2_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_PMIO2_BASE + reg));
+	return read16(acpimmio_pmio2 + reg);
 }

static inline uint32_t pm2_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_PMIO2_BASE + reg));
+	return read32(acpimmio_pmio2 + reg);
 }

static inline void pm2_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_PMIO2_BASE + reg), value);
+	write8(acpimmio_pmio2 + reg, value);
 }

static inline void pm2_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_PMIO2_BASE + reg), value);
+	write16(acpimmio_pmio2 + reg, value);
 }

static inline void pm2_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_PMIO2_BASE + reg), value);
+	write32(acpimmio_pmio2 + reg, value);
 }

static inline uint8_t acpi_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_ACPI_BASE + reg));
+	return read8(acpimmio_acpi + reg);
 }

static inline uint16_t acpi_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_ACPI_BASE + reg));
+	return read16(acpimmio_acpi + reg);
 }

static inline uint32_t acpi_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_ACPI_BASE + reg));
+	return read32(acpimmio_acpi + reg);
 }

static inline void acpi_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_ACPI_BASE + reg), value);
+	write8(acpimmio_acpi + reg, value);
 }

static inline void acpi_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_ACPI_BASE + reg), value);
+	write16(acpimmio_acpi + reg, value);
 }

static inline void acpi_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_ACPI_BASE + reg), value);
+	write32(acpimmio_acpi + reg, value);
 }

static inline uint8_t asf_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_ASF_BASE + reg));
+	return read8(acpimmio_asf + reg);
 }

static inline uint16_t asf_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_ASF_BASE + reg));
+	return read16(acpimmio_asf + reg);
 }

static inline void asf_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_ASF_BASE + reg), value);
+	write8(acpimmio_asf + reg, value);
 }

static inline void asf_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_ASF_BASE + reg), value);
+	write16(acpimmio_asf + reg, value);
 }

static inline uint8_t smbus_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_SMBUS_BASE + reg));
+	return read8(acpimmio_smbus + reg);
 }

static inline uint16_t smbus_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_SMBUS_BASE + reg));
+	return read16(acpimmio_smbus + reg);
 }

static inline void smbus_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_SMBUS_BASE + reg), value);
+	write8(acpimmio_smbus + reg, value);
 }

static inline void smbus_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_SMBUS_BASE + reg), value);
+	write16(acpimmio_smbus + reg, value);
 }

static inline uint8_t iomux_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_IOMUX_BASE + reg));
+	return read8(acpimmio_iomux + reg);
 }

static inline uint16_t iomux_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_IOMUX_BASE + reg));
+	return read16(acpimmio_iomux + reg);
 }

static inline uint32_t iomux_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_IOMUX_BASE + reg));
+	return read32(acpimmio_iomux + reg);
 }

static inline void iomux_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
+	write8(acpimmio_iomux + reg, value);
 }

static inline void iomux_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
+	write16(acpimmio_iomux + reg, value);
 }

static inline void iomux_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
+	write32(acpimmio_iomux + reg, value);
 }

static inline uint8_t misc_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_MISC_BASE + reg));
+	return read8(acpimmio_misc + reg);
 }

static inline uint16_t misc_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_MISC_BASE + reg));
+	return read16(acpimmio_misc + reg);
 }

static inline uint32_t misc_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_MISC_BASE + reg));
+	return read32(acpimmio_misc + reg);
 }

static inline void misc_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_MISC_BASE + reg), value);
+	write8(acpimmio_misc + reg, value);
 }

static inline void misc_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_MISC_BASE + reg), value);
+	write16(acpimmio_misc + reg, value);
 }

static inline void misc_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_MISC_BASE + reg), value);
+	write32(acpimmio_misc + reg, value);
 }

/* Old GPIO configuration registers */
 static inline uint8_t gpio_100_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_GPIO_BASE_100 + reg));
+	return read8(acpimmio_gpio_100 + reg);
 }

static inline uint16_t gpio_100_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_GPIO_BASE_100 + reg));
+	return read16(acpimmio_gpio_100 + reg);
 }

static inline uint32_t gpio_100_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_GPIO_BASE_100 + reg));
+	return read32(acpimmio_gpio_100 + reg);
 }

static inline void gpio_100_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value);
+	write8(acpimmio_gpio_100 + reg, value);
 }

static inline void gpio_100_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value);
+	write16(acpimmio_gpio_100 + reg, value);
 }

static inline void gpio_100_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value);
+	write32(acpimmio_gpio_100 + reg, value);
 }

/* New GPIO banks configuration registers */
 /* GPIO bank 0 */
 static inline uint8_t gpio0_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_GPIO0_BASE + reg));
+	return read8(acpimmio_gpio0 + reg);
 }

static inline uint16_t gpio0_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_GPIO0_BASE + reg));
+	return read16(acpimmio_gpio0 + reg);
 }

static inline uint32_t gpio0_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_GPIO0_BASE + reg));
+	return read32(acpimmio_gpio0 + reg);
 }

static inline void gpio0_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value);
+	write8(acpimmio_gpio0 + reg, value);
 }

static inline void gpio0_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value);
+	write16(acpimmio_gpio0 + reg, value);
 }

static inline void gpio0_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value);
+	write32(acpimmio_gpio0 + reg, value);
 }

/* GPIO bank 1 */
 static inline uint8_t gpio1_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_GPIO1_BASE + reg));
+	return read8(acpimmio_gpio1 + reg);
 }

static inline uint16_t gpio1_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_GPIO1_BASE + reg));
+	return read16(acpimmio_gpio1 + reg);
 }

static inline uint32_t gpio1_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_GPIO1_BASE + reg));
+	return read32(acpimmio_gpio1 + reg);
 }

static inline void gpio1_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value);
+	write8(acpimmio_gpio1 + reg, value);
 }

static inline void gpio1_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value);
+	write16(acpimmio_gpio1 + reg, value);
 }

static inline void gpio1_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value);
+	write32(acpimmio_gpio1 + reg, value);
 }

/* GPIO bank 2 */
 static inline uint8_t gpio2_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_GPIO2_BASE + reg));
+	return read8(acpimmio_gpio2 + reg);
 }

static inline uint16_t gpio2_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_GPIO2_BASE + reg));
+	return read16(acpimmio_gpio2 + reg);
 }

static inline uint32_t gpio2_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_GPIO2_BASE + reg));
+	return read32(acpimmio_gpio2 + reg);
 }

static inline void gpio2_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value);
+	write8(acpimmio_gpio2 + reg, value);
 }

static inline void gpio2_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value);
+	write16(acpimmio_gpio2 + reg, value);
 }

static inline void gpio2_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value);
+	write32(acpimmio_gpio2 + reg, value);
 }

static inline uint8_t xhci_pm_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_XHCIPM_BASE + reg));
+	return read8(acpimmio_xhci_pm + reg);
 }

static inline uint16_t xhci_pm_read16(uint8_t reg)
 {
-	return read16((void *)(ACPIMMIO_XHCIPM_BASE + reg));
+	return read16(acpimmio_xhci_pm + reg);
 }

static inline uint32_t xhci_pm_read32(uint8_t reg)
 {
-	return read32((void *)(ACPIMMIO_XHCIPM_BASE + reg));
+	return read32(acpimmio_xhci_pm + reg);
 }

static inline void xhci_pm_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
+	write8(acpimmio_xhci_pm + reg, value);
 }

static inline void xhci_pm_write16(uint8_t reg, uint16_t value)
 {
-	write16((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
+	write16(acpimmio_xhci_pm + reg, value);
 }

static inline void xhci_pm_write32(uint8_t reg, uint32_t value)
 {
-	write32((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
+	write32(acpimmio_xhci_pm + reg, value);
 }

static inline uint8_t aoac_read8(uint8_t reg)
 {
-	return read8((void *)(ACPIMMIO_AOAC_BASE + reg));
+	return read8(acpimmio_aoac + reg);
 }

static inline void aoac_write8(uint8_t reg, uint8_t value)
 {
-	write8((void *)(ACPIMMIO_AOAC_BASE + reg), value);
+	write8(acpimmio_aoac + reg, value);
 }

#endif /* __AMDBLOCKS_ACPIMMIO_H__ */
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
index 9e44a85..d230d54 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h
@@ -3,24 +3,85 @@
 #ifndef __AMDBLOCKS_ACPIMMIO_MAP_H__
 #define __AMDBLOCKS_ACPIMMIO_MAP_H__

-/* IO index/data for accessing PMIO prior to enabling MMIO decode */
-#define PM_INDEX			0xcd6
-#define PM_DATA				0xcd7
-
-/* Earlier devices enable the decode in PMx24 instead.  All discrete FCHs and
- * the Kabini SoC fall into this category.  Kabini's successor, Mullins, uses
- * this newer method.
+/*
+ * The following AcpiMmio register block mapping represents definitions
+ * that have been documented in AMD publications.  All blocks aren't
+ * implemented in all products, so the caller should be careful not to
+ * inadvertently access a non-existent block.  The definitions within
+ * each block are also subject to change across products.  Please refer
+ * to the appropriate RRG, the BKDG, or PPR for the product.
+ *
+ * The base address is configurable in older products, but defaults to
+ * 0xfed80000.  The address is fixed at 0xfed80000 in newer products.
+ *
+ * +---------------------------------------------------------------------------+
+ * |0x000    SMBus PCI space                                                   |
+ * |           * Dual-mapped to PCI configuration header of D14F0              |
+ * +---------------------------------------------------------------------------+
+ * |0x100    GPIO configuration registers                                      |
+ * |           * old style, never implemented with newer style                 |
+ * |           * discrete controller hubs and Family 16h Models 00h-0Fh.       |
+ * +---------------------------------------------------------------------------+
+ * |0x200    SMI configuration registers                                       |
+ * +---------------------------------------------------------------------------+
+ * |0x300    Power Management registers                                        |
+ * |           * Dual-mapped via IO Index/Data 0xcd6/0xcd7 (byte access only)  |
+ * +---------------------------------------------------------------------------+
+ * |0x400    Power Management 2 registers                                      |
+ * +---------------------------------------------------------------------------+
+ * |0x500    BIOS RAM                                                          |
+ * |           * General-purpose storage in S3 domain                          |
+ * |           * Byte access only                                              |
+ * +---------------------------------------------------------------------------+
+ * |0x600    CMOS RAM                                                          |
+ * |           * Dual-mapped to storage at Alt RTC Index/Data (0x72/0x73)      |
+ * |           * Byte access only                                              |
+ * +---------------------------------------------------------------------------+
+ * |0x700    CMOS                                                              |
+ * |           * Dual-mapped to storage at RTC Index/Data (0x70/0x71)          |
+ * |           * Byte access only                                              |
+ * +---------------------------------------------------------------------------+
+ * |0x800    Standard ACPI registers                                           |
+ * |           * Dual-mapped to I/O ACPI registers                             |
+ * +---------------------------------------------------------------------------+
+ * |0x900    ASF controller registers                                          |
+ * |           * Dual-mapped to I/O ASF controller registers                   |
+ * +---------------------------------------------------------------------------+
+ * |0xa00    SMBus controller registers                                        |
+ * |           * Dual-mapped to I/O SMBus controller registers                 |
+ * +---------------------------------------------------------------------------+
+ * |0xb00    WDT registers                                                     |
+ * |           * Dual-mapped to WDT registers, typ. enabled at 0xfeb00000      |
+ * +---------------------------------------------------------------------------+
+ * |0xc00    HPET registers                                                    |
+ * |           * Dual-mapped to HPET registers, typ. enabled at 0xfed00000     |
+ * +---------------------------------------------------------------------------+
+ * |0xd00    MUX configuration registers for GPIO signals                      |
+ * +---------------------------------------------------------------------------+
+ * |0xe00    Miscellaneous registers                                           |
+ * +---------------------------------------------------------------------------+
+ * |0x1000   Serial debug bus                                                  |
+ * +---------------------------------------------------------------------------+
+ * |0x1400   DP-VGA                                                            |
+ * +---------------------------------------------------------------------------+
+ * |0x1500   GPIO configuration registers bank 0                               |
+ * |           * new style, never implemented with older style                 |
+ * |           * SoCs starting with Family 16h Models 30h-3Fh                  |
+ * +---------------------------------------------------------------------------+
+ * |0x1600   GPIO configuration registers bank 1 (following bank 0)            |
+ * +---------------------------------------------------------------------------+
+ * |0x1700   GPIO configuration registers bank 2 (following bank 1)            |
+ * +---------------------------------------------------------------------------+
+ * |0x1c00   xHCI Power Management registers                                   |
+ * +---------------------------------------------------------------------------+
+ * |0x1d00   Wake device (AC DC timer)                                         |
+ * +---------------------------------------------------------------------------+
+ * |0x1e00   Always On Always Connect registers                                |
+ * +---------------------------------------------------------------------------+
  */

-#define ACPIMMIO_DECODE_REGISTER_24 0x24
-#define   PM_24_ACPIMMIO_DECODE_EN	BIT(0)
-
-#define ACPIMMIO_DECODE_REGISTER_04 0x4
-#define   PM_04_BIOSRAM_DECODE_EN	BIT(0)
-#define   PM_04_ACPIMMIO_DECODE_EN	BIT(1)
-
-
-/* MMIO register blocks are at fixed offsets from 0xfed80000 and are enabled
+/*
+ * MMIO register blocks are at fixed offsets from 0xfed80000 and are enabled
  * in PMx24[0] (older implementations) and PMx04[1] (newer implementations).
  * PM registers are also accessible via IO CD6/CD7.
  *
@@ -32,27 +93,46 @@
  */

#define AMD_SB_ACPI_MMIO_ADDR		0xfed80000
-#define ACPIMMIO_SM_PCI_BASE		0xfed80000
-#define ACPIMMIO_GPIO_BASE_100		0xfed80100
-#define ACPIMMIO_SMI_BASE		0xfed80200
-#define ACPIMMIO_PMIO_BASE		0xfed80300
-#define ACPIMMIO_PMIO2_BASE		0xfed80400
-#define ACPIMMIO_BIOSRAM_BASE		0xfed80500
-#define ACPIMMIO_CMOSRAM_BASE		0xfed80600
-#define ACPIMMIO_CMOS_BASE		0xfed80700
-#define ACPIMMIO_ACPI_BASE		0xfed80800
-#define ACPIMMIO_ASF_BASE		0xfed80900
-#define ACPIMMIO_SMBUS_BASE		0xfed80a00
-#define ACPIMMIO_WDT_BASE		0xfed80b00
-#define ACPIMMIO_HPET_BASE		0xfed80c00
-#define ACPIMMIO_IOMUX_BASE		0xfed80d00
+
+#ifdef __ACPI__
+
+/* ASL fails on additions. */
 #define ACPIMMIO_MISC_BASE		0xfed80e00
-#define ACPIMMIO_DPVGA_BASE		0xfed81400
 #define ACPIMMIO_GPIO0_BASE		0xfed81500
-#define ACPIMMIO_GPIO1_BASE		0xfed81600
-#define ACPIMMIO_GPIO2_BASE		0xfed81700
-#define ACPIMMIO_XHCIPM_BASE		0xfed81c00
-#define ACPIMMIO_ACDCTMR_BASE		0xfed81d00
 #define ACPIMMIO_AOAC_BASE		0xfed81e00

+#else
+
+#define ACPIMMIO_SM_PCI_BANK		0x0000
+#define ACPIMMIO_GPIO_100_BANK		0x0100
+#define ACPIMMIO_SMI_BANK		0x0200
+#define ACPIMMIO_PMIO_BANK		0x0300
+#define ACPIMMIO_PMIO2_BANK		0x0400
+#define ACPIMMIO_BIOSRAM_BANK		0x0500
+#define ACPIMMIO_CMOSRAM_BANK		0x0600
+#define ACPIMMIO_CMOS_BANK		0x0700
+#define ACPIMMIO_ACPI_BANK		0x0800
+#define ACPIMMIO_ASF_BANK		0x0900
+#define ACPIMMIO_SMBUS_BANK		0x0a00
+#define ACPIMMIO_WDT_BANK		0x0b00
+#define ACPIMMIO_HPET_BANK		0x0c00
+#define ACPIMMIO_IOMUX_BANK		0x0d00
+#define ACPIMMIO_MISC_BANK		0x0e00
+#define ACPIMMIO_DPVGA_BANK		0x1400
+#define ACPIMMIO_GPIO0_BANK		0x1500
+#define ACPIMMIO_GPIO1_BANK		0x1600
+#define ACPIMMIO_GPIO2_BANK		0x1700
+#define ACPIMMIO_XHCIPM_BANK		0x1c00
+#define ACPIMMIO_ACDCTMR_BANK		0x1d00
+#define ACPIMMIO_AOAC_BANK		0x1e00
+
+#define ACPIMMIO_BASE(x) \
+	(void *)(AMD_SB_ACPI_MMIO_ADDR + ACPIMMIO_ ## x ## _BANK)
+
+/* FIXME: Passing host base for SMBUS is not long-term solution. */
+#define ACPIMMIO_ASF_BASE	(AMD_SB_ACPI_MMIO_ADDR + ACPIMMIO_ASF_BANK)
+#define ACPIMMIO_SMBUS_BASE	(AMD_SB_ACPI_MMIO_ADDR + ACPIMMIO_SMBUS_BANK)
+
+#endif
+
 #endif /* __AMDBLOCKS_ACPIMMIO_MAP_H__ */
diff --git a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
index 572e639..f4288aa 100644
--- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
+++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
@@ -5,7 +5,7 @@

#include <stdint.h>
 #include <stddef.h>
-#include <amdblocks/acpimmio_map.h>
+#include <amdblocks/acpimmio.h>

struct soc_amd_gpio {
    uint8_t gpio;
@@ -19,9 +19,9 @@
    uint8_t event;
 };

-#define GPIO_BANK0_CONTROL(gpio) (ACPIMMIO_GPIO0_BASE + ((gpio) * 4))
-#define GPIO_BANK1_CONTROL(gpio) (ACPIMMIO_GPIO1_BASE + (((gpio) - 64) * 4))
-#define GPIO_BANK2_CONTROL(gpio) (ACPIMMIO_GPIO2_BASE + (((gpio) - 128) * 4))
+#define GPIO_BANK0_CONTROL(gpio) ((uintptr_t)acpimmio_gpio0 + ((gpio) * 4))
+#define GPIO_BANK1_CONTROL(gpio) ((uintptr_t)acpimmio_gpio1 + (((gpio) - 64) * 4))
+#define GPIO_BANK2_CONTROL(gpio) ((uintptr_t)acpimmio_gpio2 + (((gpio) - 128) * 4))

#define GPIO_MASTER_SWITCH	0xFC
 #define   GPIO_MASK_STS_EN	BIT(28)
diff --git a/src/soc/amd/common/block/smbus/smbus.c b/src/soc/amd/common/block/smbus/smbus.c
index b5db56b..e304483 100644
--- a/src/soc/amd/common/block/smbus/smbus.c
+++ b/src/soc/amd/common/block/smbus/smbus.c
@@ -4,6 +4,7 @@
 #include <console/console.h>
 #include <device/smbus_host.h>
 #include <amdblocks/acpimmio.h>
+#include <amdblocks/acpimmio_map.h>
 #include <soc/southbridge.h>

/*
diff --git a/src/soc/amd/picasso/psp.c b/src/soc/amd/picasso/psp.c
index 0229139..e40d395 100644
--- a/src/soc/amd/picasso/psp.c
+++ b/src/soc/amd/picasso/psp.c
@@ -3,7 +3,7 @@
 #include <console/console.h>
 #include <cpu/x86/msr.h>
 #include <soc/smi.h>
-#include <amdblocks/acpimmio_map.h>
+#include <amdblocks/acpimmio.h>
 #include <amdblocks/psp.h>

#define PSP_MAILBOX_OFFSET		0x10570
@@ -27,7 +27,7 @@
    if (!trig)
    	return;

-	trig->address = 0xfed802a8;
+	trig->address = (uintptr_t)acpimmio_smi + SMI_REG_CONTROL2;
    trig->address_type = SMM_TRIGGER_MEM;
    trig->value_width = SMM_TRIGGER_DWORD;
    trig->value_and_mask = 0xfdffffff;
@@ -39,19 +39,19 @@
    if (!reg)
    	return;

-	reg->smi_enb.address = ACPIMMIO_SMI_BASE + SMI_REG_SMITRIG0;
+	reg->smi_enb.address = (uintptr_t)acpimmio_smi + SMI_REG_SMITRIG0;
    reg->smi_enb.address_type = SMM_TRIGGER_MEM;
    reg->smi_enb.value_width = SMM_TRIGGER_DWORD;
    reg->smi_enb.reg_bit_mask = SMITRG0_SMIENB;
    reg->smi_enb.expect_value = 0;

-	reg->eos.address = ACPIMMIO_SMI_BASE + SMI_REG_SMITRIG0;
+	reg->eos.address = (uintptr_t)acpimmio_smi + SMI_REG_SMITRIG0;
    reg->eos.address_type = SMM_TRIGGER_MEM;
    reg->eos.value_width = SMM_TRIGGER_DWORD;
    reg->eos.reg_bit_mask = SMITRG0_EOS;
    reg->eos.expect_value = SMITRG0_EOS;

-	reg->psp_smi_en.address = ACPIMMIO_SMI_BASE + SMI_REG_CONTROL0;
+	reg->psp_smi_en.address = (uintptr_t)acpimmio_smi + SMI_REG_CONTROL0;
    reg->psp_smi_en.address += sizeof(uint32_t) * SMITYPE_PSP / 16;
    reg->psp_smi_en.address_type = SMM_TRIGGER_MEM;
    reg->psp_smi_en.value_width = SMM_TRIGGER_DWORD;
diff --git a/src/soc/amd/stoneyridge/smbus_spd.c b/src/soc/amd/stoneyridge/smbus_spd.c
index bc9a9d9..7c54d8d 100644
--- a/src/soc/amd/stoneyridge/smbus_spd.c
+++ b/src/soc/amd/stoneyridge/smbus_spd.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <amdblocks/agesawrapper.h>
+#include <amdblocks/acpimmio_map.h>
 #include <console/console.h>
 #include <device/pci_def.h>
 #include <device/device.h>