Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: [WIP] soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
[WIP] soc/amd/common: Remove guards on ACPIMMIO utils
If one wishes to use the functions guarded here, he has to have datasheet open anyways. It should be clear from there which regions are supported and which are not.
Change-Id: I0c1f0c9c9a6711532c5078c08cdf9e6612f3bc9c Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/google/kahlee/variants/baseboard/include/baseboard/acpi/audio.asl 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/gpio_banks.h M src/soc/amd/picasso/acpi/sb_fch.asl M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/stoneyridge/acpi/sb_fch.asl M src/soc/amd/stoneyridge/include/soc/iomap.h 9 files changed, 5 insertions(+), 188 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/37210/1
diff --git a/src/mainboard/google/kahlee/variants/baseboard/include/baseboard/acpi/audio.asl b/src/mainboard/google/kahlee/variants/baseboard/include/baseboard/acpi/audio.asl index 87890da..2fcf1ff 100644 --- a/src/mainboard/google/kahlee/variants/baseboard/include/baseboard/acpi/audio.asl +++ b/src/mainboard/google/kahlee/variants/baseboard/include/baseboard/acpi/audio.asl @@ -12,7 +12,7 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ -#include <soc/iomap.h> +#include <amdblocks/acpimmio_map.h>
/* Grunt specific I2S machine driver */ Device (I2S) diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c index 7fad456..f844ea4 100644 --- a/src/soc/amd/common/block/acpimmio/mmio_util.c +++ b/src/soc/amd/common/block/acpimmio/mmio_util.c @@ -65,7 +65,6 @@ pm_io_write16(reg + sizeof(uint16_t), value & 0xffff); }
-#if SUPPORTS_ACPIMMIO_SM_PCI_BASE /* smbus pci read/write - access registers at 0xfed80000 */
u8 sm_pci_read8(u8 reg) @@ -97,10 +96,6 @@ { write32((void *)(ACPIMMIO_SM_PCI_BASE + reg), value); } -#endif - -#if SUPPORTS_ACPIMMIO_SMI_BASE -/* smi read/write - access registers at 0xfed80200 */
uint8_t smi_read8(uint8_t reg) { @@ -131,10 +126,6 @@ { write32((void *)(ACPIMMIO_SMI_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_SMI_BASE */ - -#if SUPPORTS_ACPIMMIO_PMIO_BASE -/* pm read/write - access registers at 0xfed80300 */
u8 pm_read8(u8 reg) { @@ -165,14 +156,6 @@ { write32((void *)(ACPIMMIO_PMIO_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_PMIO_BASE */ - -#if SUPPORTS_ACPIMMIO_PMIO2_BASE -/* pm2 read/write - access registers at 0xfed80400 - currently unused by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_BIOSRAM_BASE -/* biosram read/write - access registers at 0xfed80500 */
uint8_t biosram_read8(uint8_t reg) { @@ -208,18 +191,6 @@ value >>= 16; biosram_write16(reg + sizeof(uint16_t), value & 0xffff); } -#endif /* SUPPORTS_ACPIMMIO_BIOSRAM_BASE */ - -#if SUPPORTS_ACPIMMIO_CMOSRAM_BASE -/* cmosram read/write - access registers at 0xfed80600 - currently unused by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_CMOS_BASE -/* cmos read/write - access registers at 0xfed80700 - currently unused by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_ACPI_BASE -/* acpi read/write - access registers at 0xfed80800 */
u8 acpi_read8(u8 reg) { @@ -250,10 +221,6 @@ { write32((void *)(ACPIMMIO_ACPI_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_ACPI_BASE */ - -#if SUPPORTS_ACPIMMIO_ASF_BASE -/* asf read/write - access registers at 0xfed80900 */
u8 asf_read8(u8 reg) { @@ -274,10 +241,6 @@ { write16((void *)(ACPIMMIO_ASF_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_ASF_BASE */ - -#if SUPPORTS_ACPIMMIO_SMBUS_BASE -/* smbus read/write - access registers at 0xfed80a00 */
u8 smbus_read8(u8 reg) { @@ -298,18 +261,6 @@ { write16((void *)(ACPIMMIO_SMBUS_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_SMBUS_BASE */ - -#if SUPPORTS_ACPIMMIO_WDT_BASE -/* wdt read/write - access registers at 0xfed80b00 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_HPET_BASE -/* hpet read/write - access registers at 0xfed80c00 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_IOMUX_BASE -/* iomux read/write - access registers at 0xfed80d00 */
u8 iomux_read8(u8 reg) { @@ -340,10 +291,6 @@ { write32((void *)(ACPIMMIO_IOMUX_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_IOMUX_BASE */ - -#if SUPPORTS_ACPIMMIO_MISC_BASE -/* misc read/write - access registers at 0xfed80e00 */
u8 misc_read8(u8 reg) { @@ -374,26 +321,6 @@ { write32((void *)(ACPIMMIO_MISC_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_MISC_BASE */ - -#if SUPPORTS_ACPIMMIO_DPVGA_BASE -/* dpvga read/write - access registers at 0xfed81400 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_GPIO0_BASE || SUPPORTS_ACPIMMIO_GPIO1_BASE \ - || SUPPORTS_ACPIMMIO_GPIO2_BASE -/* - * No helpers are currently in use however common/block//gpio.c accesses - * the registers directly. - */ - -/* gpio bk 0 read/write - access registers at 0xfed81500 */ -/* gpio bk 1 read/write - access registers at 0xfed81600 */ -/* gpio bk 2 read/write - access registers at 0xfed81700 */ -#endif - -#if SUPPORTS_ACPIMMIO_XHCIPM_BASE -/* xhci_pm read/write - access registers at 0xfed81c00 */
uint8_t xhci_pm_read8(uint8_t reg) { @@ -424,14 +351,6 @@ { write32((void *)(ACPIMMIO_XHCIPM_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_XHCIPM_BASE */ - -#if SUPPORTS_ACPIMMIO_ACDCTMR_BASE -/* acdc_tmr read/write - access registers at 0xfed81d00 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_AOAC_BASE -/* aoac read/write - access registers at 0xfed81e00 */
u8 aoac_read8(u8 reg) { @@ -442,4 +361,3 @@ { write8((void *)(ACPIMMIO_AOAC_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_AOAC_BASE */ diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c index 17e3de0..76c2021 100644 --- a/src/soc/amd/common/block/gpio_banks/gpio.c +++ b/src/soc/amd/common/block/gpio_banks/gpio.c @@ -20,6 +20,7 @@ #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> diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index ca57cf5..b395cdb 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -19,71 +19,6 @@ #define __AMDBLOCKS_ACPIMMIO_H__
#include <stdint.h> -/* iomap.h must indicate if the device uses a block, optional if unused. */ -#include <soc/iomap.h> -#ifndef SUPPORTS_ACPIMMIO_SM_PCI_BASE - #define SUPPORTS_ACPIMMIO_SM_PCI_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_SMI_BASE - #define SUPPORTS_ACPIMMIO_SMI_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_PMIO_BASE - #define SUPPORTS_ACPIMMIO_PMIO_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_PMIO2_BASE - #define SUPPORTS_ACPIMMIO_PMIO2_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_BIOSRAM_BASE - #define SUPPORTS_ACPIMMIO_BIOSRAM_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_CMOSRAM_BASE - #define SUPPORTS_ACPIMMIO_CMOSRAM_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_CMOS_BASE - #define SUPPORTS_ACPIMMIO_CMOS_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_ACPI_BASE - #define SUPPORTS_ACPIMMIO_ACPI_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_ASF_BASE - #define SUPPORTS_ACPIMMIO_ASF_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_SMBUS_BASE - #define SUPPORTS_ACPIMMIO_SMBUS_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_WDT_BASE - #define SUPPORTS_ACPIMMIO_WDT_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_HPET_BASE - #define SUPPORTS_ACPIMMIO_HPET_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_IOMUX_BASE - #define SUPPORTS_ACPIMMIO_IOMUX_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_MISC_BASE - #define SUPPORTS_ACPIMMIO_MISC_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_DPVGA_BASE - #define SUPPORTS_ACPIMMIO_DPVGA_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_GPIO0_BASE - #define SUPPORTS_ACPIMMIO_GPIO0_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_GPIO1_BASE - #define SUPPORTS_ACPIMMIO_GPIO1_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_GPIO2_BASE - #define SUPPORTS_ACPIMMIO_GPIO2_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_XHCIPM_BASE - #define SUPPORTS_ACPIMMIO_XHCIPM_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_ACDCTMR_BASE - #define SUPPORTS_ACPIMMIO_ACDCTMR_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_AOAC_BASE - #define SUPPORTS_ACPIMMIO_AOAC_BASE 0 -#endif
/* * The following AcpiMmio register block mapping represents definitions 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 6427cb6..2206e35 100644 --- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h +++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h @@ -18,6 +18,7 @@
#include <stdint.h> #include <stddef.h> +#include <amdblocks/acpimmio_map.h>
struct soc_amd_gpio { uint8_t gpio; diff --git a/src/soc/amd/picasso/acpi/sb_fch.asl b/src/soc/amd/picasso/acpi/sb_fch.asl index 680f496..ca8d175 100644 --- a/src/soc/amd/picasso/acpi/sb_fch.asl +++ b/src/soc/amd/picasso/acpi/sb_fch.asl @@ -15,6 +15,7 @@
#include <soc/gpio.h> #include <soc/iomap.h> +#include <amdblocks/acpimmio_map.h>
Device (AAHB) { diff --git a/src/soc/amd/picasso/include/soc/iomap.h b/src/soc/amd/picasso/include/soc/iomap.h index 5037a1c..b1d4bff 100644 --- a/src/soc/amd/picasso/include/soc/iomap.h +++ b/src/soc/amd/picasso/include/soc/iomap.h @@ -27,26 +27,6 @@ #endif #define HPET_BASE_ADDRESS 0xfed00000
-/* - * AcpiMmio blocks are at fixed offsets from FED8_0000h and enabled in PMx04[1]. - * All ranges not specified as supported below may, or may not, be listed in - * any documentation but should be considered reserved through FED8_1FFFh. - */ -#include <amdblocks/acpimmio_map.h> -#define SUPPORTS_ACPIMMIO_SM_PCI_BASE 1 /* 0xfed80000 */ -#define SUPPORTS_ACPIMMIO_SMI_BASE 1 /* 0xfed80100 */ -#define SUPPORTS_ACPIMMIO_PMIO_BASE 1 /* 0xfed80300 */ -#define SUPPORTS_ACPIMMIO_BIOSRAM_BASE 1 /* 0xfed80500 */ -#define SUPPORTS_ACPIMMIO_ACPI_BASE 1 /* 0xfed80800 */ -#define SUPPORTS_ACPIMMIO_ASF_BASE 1 /* 0xfed80900 */ -#define SUPPORTS_ACPIMMIO_SMBUS_BASE 1 /* 0xfed80a00 */ -#define SUPPORTS_ACPIMMIO_IOMUX_BASE 1 /* 0xfed80d00 */ -#define SUPPORTS_ACPIMMIO_MISC_BASE 1 /* 0xfed80e00 */ -#define SUPPORTS_ACPIMMIO_GPIO0_BASE 1 /* 0xfed81500 */ -#define SUPPORTS_ACPIMMIO_GPIO1_BASE 1 /* 0xfed81800 */ -#define SUPPORTS_ACPIMMIO_GPIO2_BASE 1 /* 0xfed81700 */ -#define SUPPORTS_ACPIMMIO_AOAC_BASE 1 /* 0xfed81e00 */ - #define ALINK_AHB_ADDRESS 0xfedc0000
/* Reserved 0xfecd1000-0xfedc3fff */ diff --git a/src/soc/amd/stoneyridge/acpi/sb_fch.asl b/src/soc/amd/stoneyridge/acpi/sb_fch.asl index e7975f8..897c9ec 100644 --- a/src/soc/amd/stoneyridge/acpi/sb_fch.asl +++ b/src/soc/amd/stoneyridge/acpi/sb_fch.asl @@ -15,6 +15,7 @@
#include <soc/gpio.h> #include <soc/iomap.h> +#include <amdblocks/acpimmio_map.h>
Device (AAHB) { diff --git a/src/soc/amd/stoneyridge/include/soc/iomap.h b/src/soc/amd/stoneyridge/include/soc/iomap.h index 612b6e8..02997cc 100644 --- a/src/soc/amd/stoneyridge/include/soc/iomap.h +++ b/src/soc/amd/stoneyridge/include/soc/iomap.h @@ -22,26 +22,6 @@ #define SPI_BASE_ADDRESS 0xfec10000 #define IO_APIC2_ADDR 0xfec20000
-/* - * AcpiMmio blocks are at fixed offsets from FED8_0000h and enabled in PMx04[1]. - * All ranges not specified as supported below may, or may not, be listed in - * any documentation but should be considered reserved through FED8_1FFFh. - */ -#include <amdblocks/acpimmio_map.h> -#define SUPPORTS_ACPIMMIO_SMI_BASE 1 /* 0xfed80100 */ -#define SUPPORTS_ACPIMMIO_PMIO_BASE 1 /* 0xfed80300 */ -#define SUPPORTS_ACPIMMIO_BIOSRAM_BASE 1 /* 0xfed80500 */ -#define SUPPORTS_ACPIMMIO_ACPI_BASE 1 /* 0xfed80800 */ -#define SUPPORTS_ACPIMMIO_ASF_BASE 1 /* 0xfed80900 */ -#define SUPPORTS_ACPIMMIO_SMBUS_BASE 1 /* 0xfed80a00 */ -#define SUPPORTS_ACPIMMIO_IOMUX_BASE 1 /* 0xfed80d00 */ -#define SUPPORTS_ACPIMMIO_MISC_BASE 1 /* 0xfed80e00 */ -#define SUPPORTS_ACPIMMIO_GPIO0_BASE 1 /* 0xfed81500 */ -#define SUPPORTS_ACPIMMIO_GPIO1_BASE 1 /* 0xfed81800 */ -#define SUPPORTS_ACPIMMIO_GPIO2_BASE 1 /* 0xfed81700 */ -#define SUPPORTS_ACPIMMIO_XHCIPM_BASE 1 /* 0xfed81c00 */ -#define SUPPORTS_ACPIMMIO_AOAC_BASE 1 /* 0xfed81e00 */ - #define ALINK_AHB_ADDRESS 0xfedc0000
/* I2C fixed address */
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: [WIP] soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 1:
(1 comment)
What would happen if there was a new SOC that did not supported one of these MMIO? Or if it supported a different one? I agree that apparently Picasso and Stoney Ridge support the same set... but the guards where established for this situation and I'm not sure if it's wise to remove them.
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... PS1, Line 21: acpimmio_map Why the need for this, being that nothing was added to this header? I can see it's wide spread, but I don't understand why.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: [WIP] soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
(1 comment)
What would happen if there was a new SOC that did not supported one of these MMIO? Or if it supported a different one? I agree that apparently Picasso and Stoney Ridge support the same set... but the guards where established for this situation and I'm not sure if it's wise to remove them.
The comments already say that for each bank, it is not guaranteed register definitions remain the same across platform generations. So developer needs to go through these individually anyways and verify what banks are implemented.
Anyways, I have a plan to mask the set of banks per platform, but it will not address the above statement about incompatible banks located at same address.
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... PS1, Line 21: acpimmio_map
Why the need for this, being that nothing was added to this header? I can see it's wide spread, but […]
ACPIMMIO_GPIOx in this file. There was indirect includes.
Hello Aaron Durbin, Marshall Dawson, Richard Spiegel, build bot (Jenkins), Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37210
to look at the new patch set (#2).
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
soc/amd/common: Remove guards on ACPIMMIO utils
If one wishes to use the functions guarded here, he has to have datasheet open anyways. It should be clear from there which regions are supported and which are not.
TEST=Reproducible build of google/aleena.
Change-Id: I0c1f0c9c9a6711532c5078c08cdf9e6612f3bc9c Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/common/block/acpimmio/mmio_util.c M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/stoneyridge/include/soc/iomap.h 4 files changed, 0 insertions(+), 187 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/37210/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... PS1, Line 21: acpimmio_map
ACPIMMIO_GPIOx in this file. There was indirect includes.
Split the patch to get a reproducible build of google/aleena.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 2:
If you ask me, CB:32934 was merged with insufficient review. I did not read all of CB:32649 arguments by Furquan.
I have followup to have selected regions blocked without preprocessor and Kconfig use.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
On the basis that AMD won't deprecate ACPIMMIO...
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:
https://review.coreboot.org/c/coreboot/+/37210/1/src/soc/amd/common/block/in... PS1, Line 21: acpimmio_map
Split the patch to get a reproducible build of google/aleena.
Ack
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37210/3/src/soc/amd/common/block/ac... File src/soc/amd/common/block/acpimmio/mmio_util.c:
https://review.coreboot.org/c/coreboot/+/37210/3/src/soc/amd/common/block/ac... PS3, Line 68: /* smbus pci read/write - access registers at 0xfed80000 */ Everywhere else the comment with purpose and base address is removed. Is there any special reason why this comment was left untouched?
Hello Aaron Durbin, Arthur Heymans, Marshall Dawson, Richard Spiegel, build bot (Jenkins), Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37210
to look at the new patch set (#4).
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
soc/amd/common: Remove guards on ACPIMMIO utils
If one wishes to use the functions guarded here, he has to have datasheet open anyways. It should be clear from there which regions are supported and which are not.
TEST=Reproducible build of google/aleena.
Change-Id: I0c1f0c9c9a6711532c5078c08cdf9e6612f3bc9c Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/common/block/acpimmio/mmio_util.c M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/stoneyridge/include/soc/iomap.h 4 files changed, 0 insertions(+), 189 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/37210/4
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37210/3/src/soc/amd/common/block/ac... File src/soc/amd/common/block/acpimmio/mmio_util.c:
https://review.coreboot.org/c/coreboot/+/37210/3/src/soc/amd/common/block/ac... PS3, Line 68: /* smbus pci read/write - access registers at 0xfed80000 */
Everywhere else the comment with purpose and base address is removed. […]
Done
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
Patch Set 4: Code-Review+2
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37210 )
Change subject: soc/amd/common: Remove guards on ACPIMMIO utils ......................................................................
soc/amd/common: Remove guards on ACPIMMIO utils
If one wishes to use the functions guarded here, he has to have datasheet open anyways. It should be clear from there which regions are supported and which are not.
TEST=Reproducible build of google/aleena.
Change-Id: I0c1f0c9c9a6711532c5078c08cdf9e6612f3bc9c Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37210 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/soc/amd/common/block/acpimmio/mmio_util.c M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/stoneyridge/include/soc/iomap.h 4 files changed, 0 insertions(+), 189 deletions(-)
Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved
diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c index 7fad456..1fc5fd4 100644 --- a/src/soc/amd/common/block/acpimmio/mmio_util.c +++ b/src/soc/amd/common/block/acpimmio/mmio_util.c @@ -65,9 +65,6 @@ pm_io_write16(reg + sizeof(uint16_t), value & 0xffff); }
-#if SUPPORTS_ACPIMMIO_SM_PCI_BASE -/* smbus pci read/write - access registers at 0xfed80000 */ - u8 sm_pci_read8(u8 reg) { return read8((void *)(ACPIMMIO_SM_PCI_BASE + reg)); @@ -97,10 +94,6 @@ { write32((void *)(ACPIMMIO_SM_PCI_BASE + reg), value); } -#endif - -#if SUPPORTS_ACPIMMIO_SMI_BASE -/* smi read/write - access registers at 0xfed80200 */
uint8_t smi_read8(uint8_t reg) { @@ -131,10 +124,6 @@ { write32((void *)(ACPIMMIO_SMI_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_SMI_BASE */ - -#if SUPPORTS_ACPIMMIO_PMIO_BASE -/* pm read/write - access registers at 0xfed80300 */
u8 pm_read8(u8 reg) { @@ -165,14 +154,6 @@ { write32((void *)(ACPIMMIO_PMIO_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_PMIO_BASE */ - -#if SUPPORTS_ACPIMMIO_PMIO2_BASE -/* pm2 read/write - access registers at 0xfed80400 - currently unused by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_BIOSRAM_BASE -/* biosram read/write - access registers at 0xfed80500 */
uint8_t biosram_read8(uint8_t reg) { @@ -208,18 +189,6 @@ value >>= 16; biosram_write16(reg + sizeof(uint16_t), value & 0xffff); } -#endif /* SUPPORTS_ACPIMMIO_BIOSRAM_BASE */ - -#if SUPPORTS_ACPIMMIO_CMOSRAM_BASE -/* cmosram read/write - access registers at 0xfed80600 - currently unused by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_CMOS_BASE -/* cmos read/write - access registers at 0xfed80700 - currently unused by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_ACPI_BASE -/* acpi read/write - access registers at 0xfed80800 */
u8 acpi_read8(u8 reg) { @@ -250,10 +219,6 @@ { write32((void *)(ACPIMMIO_ACPI_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_ACPI_BASE */ - -#if SUPPORTS_ACPIMMIO_ASF_BASE -/* asf read/write - access registers at 0xfed80900 */
u8 asf_read8(u8 reg) { @@ -274,10 +239,6 @@ { write16((void *)(ACPIMMIO_ASF_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_ASF_BASE */ - -#if SUPPORTS_ACPIMMIO_SMBUS_BASE -/* smbus read/write - access registers at 0xfed80a00 */
u8 smbus_read8(u8 reg) { @@ -298,18 +259,6 @@ { write16((void *)(ACPIMMIO_SMBUS_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_SMBUS_BASE */ - -#if SUPPORTS_ACPIMMIO_WDT_BASE -/* wdt read/write - access registers at 0xfed80b00 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_HPET_BASE -/* hpet read/write - access registers at 0xfed80c00 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_IOMUX_BASE -/* iomux read/write - access registers at 0xfed80d00 */
u8 iomux_read8(u8 reg) { @@ -340,10 +289,6 @@ { write32((void *)(ACPIMMIO_IOMUX_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_IOMUX_BASE */ - -#if SUPPORTS_ACPIMMIO_MISC_BASE -/* misc read/write - access registers at 0xfed80e00 */
u8 misc_read8(u8 reg) { @@ -374,26 +319,6 @@ { write32((void *)(ACPIMMIO_MISC_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_MISC_BASE */ - -#if SUPPORTS_ACPIMMIO_DPVGA_BASE -/* dpvga read/write - access registers at 0xfed81400 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_GPIO0_BASE || SUPPORTS_ACPIMMIO_GPIO1_BASE \ - || SUPPORTS_ACPIMMIO_GPIO2_BASE -/* - * No helpers are currently in use however common/block//gpio.c accesses - * the registers directly. - */ - -/* gpio bk 0 read/write - access registers at 0xfed81500 */ -/* gpio bk 1 read/write - access registers at 0xfed81600 */ -/* gpio bk 2 read/write - access registers at 0xfed81700 */ -#endif - -#if SUPPORTS_ACPIMMIO_XHCIPM_BASE -/* xhci_pm read/write - access registers at 0xfed81c00 */
uint8_t xhci_pm_read8(uint8_t reg) { @@ -424,14 +349,6 @@ { write32((void *)(ACPIMMIO_XHCIPM_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_XHCIPM_BASE */ - -#if SUPPORTS_ACPIMMIO_ACDCTMR_BASE -/* acdc_tmr read/write - access registers at 0xfed81d00 - not currently used by any soc */ -#endif - -#if SUPPORTS_ACPIMMIO_AOAC_BASE -/* aoac read/write - access registers at 0xfed81e00 */
u8 aoac_read8(u8 reg) { @@ -442,4 +359,3 @@ { write8((void *)(ACPIMMIO_AOAC_BASE + reg), value); } -#endif /* SUPPORTS_ACPIMMIO_AOAC_BASE */ diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index ca57cf5..b395cdb 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -19,71 +19,6 @@ #define __AMDBLOCKS_ACPIMMIO_H__
#include <stdint.h> -/* iomap.h must indicate if the device uses a block, optional if unused. */ -#include <soc/iomap.h> -#ifndef SUPPORTS_ACPIMMIO_SM_PCI_BASE - #define SUPPORTS_ACPIMMIO_SM_PCI_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_SMI_BASE - #define SUPPORTS_ACPIMMIO_SMI_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_PMIO_BASE - #define SUPPORTS_ACPIMMIO_PMIO_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_PMIO2_BASE - #define SUPPORTS_ACPIMMIO_PMIO2_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_BIOSRAM_BASE - #define SUPPORTS_ACPIMMIO_BIOSRAM_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_CMOSRAM_BASE - #define SUPPORTS_ACPIMMIO_CMOSRAM_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_CMOS_BASE - #define SUPPORTS_ACPIMMIO_CMOS_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_ACPI_BASE - #define SUPPORTS_ACPIMMIO_ACPI_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_ASF_BASE - #define SUPPORTS_ACPIMMIO_ASF_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_SMBUS_BASE - #define SUPPORTS_ACPIMMIO_SMBUS_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_WDT_BASE - #define SUPPORTS_ACPIMMIO_WDT_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_HPET_BASE - #define SUPPORTS_ACPIMMIO_HPET_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_IOMUX_BASE - #define SUPPORTS_ACPIMMIO_IOMUX_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_MISC_BASE - #define SUPPORTS_ACPIMMIO_MISC_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_DPVGA_BASE - #define SUPPORTS_ACPIMMIO_DPVGA_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_GPIO0_BASE - #define SUPPORTS_ACPIMMIO_GPIO0_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_GPIO1_BASE - #define SUPPORTS_ACPIMMIO_GPIO1_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_GPIO2_BASE - #define SUPPORTS_ACPIMMIO_GPIO2_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_XHCIPM_BASE - #define SUPPORTS_ACPIMMIO_XHCIPM_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_ACDCTMR_BASE - #define SUPPORTS_ACPIMMIO_ACDCTMR_BASE 0 -#endif -#ifndef SUPPORTS_ACPIMMIO_AOAC_BASE - #define SUPPORTS_ACPIMMIO_AOAC_BASE 0 -#endif
/* * The following AcpiMmio register block mapping represents definitions diff --git a/src/soc/amd/picasso/include/soc/iomap.h b/src/soc/amd/picasso/include/soc/iomap.h index 5037a1c..b1d4bff 100644 --- a/src/soc/amd/picasso/include/soc/iomap.h +++ b/src/soc/amd/picasso/include/soc/iomap.h @@ -27,26 +27,6 @@ #endif #define HPET_BASE_ADDRESS 0xfed00000
-/* - * AcpiMmio blocks are at fixed offsets from FED8_0000h and enabled in PMx04[1]. - * All ranges not specified as supported below may, or may not, be listed in - * any documentation but should be considered reserved through FED8_1FFFh. - */ -#include <amdblocks/acpimmio_map.h> -#define SUPPORTS_ACPIMMIO_SM_PCI_BASE 1 /* 0xfed80000 */ -#define SUPPORTS_ACPIMMIO_SMI_BASE 1 /* 0xfed80100 */ -#define SUPPORTS_ACPIMMIO_PMIO_BASE 1 /* 0xfed80300 */ -#define SUPPORTS_ACPIMMIO_BIOSRAM_BASE 1 /* 0xfed80500 */ -#define SUPPORTS_ACPIMMIO_ACPI_BASE 1 /* 0xfed80800 */ -#define SUPPORTS_ACPIMMIO_ASF_BASE 1 /* 0xfed80900 */ -#define SUPPORTS_ACPIMMIO_SMBUS_BASE 1 /* 0xfed80a00 */ -#define SUPPORTS_ACPIMMIO_IOMUX_BASE 1 /* 0xfed80d00 */ -#define SUPPORTS_ACPIMMIO_MISC_BASE 1 /* 0xfed80e00 */ -#define SUPPORTS_ACPIMMIO_GPIO0_BASE 1 /* 0xfed81500 */ -#define SUPPORTS_ACPIMMIO_GPIO1_BASE 1 /* 0xfed81800 */ -#define SUPPORTS_ACPIMMIO_GPIO2_BASE 1 /* 0xfed81700 */ -#define SUPPORTS_ACPIMMIO_AOAC_BASE 1 /* 0xfed81e00 */ - #define ALINK_AHB_ADDRESS 0xfedc0000
/* Reserved 0xfecd1000-0xfedc3fff */ diff --git a/src/soc/amd/stoneyridge/include/soc/iomap.h b/src/soc/amd/stoneyridge/include/soc/iomap.h index 612b6e8..02997cc 100644 --- a/src/soc/amd/stoneyridge/include/soc/iomap.h +++ b/src/soc/amd/stoneyridge/include/soc/iomap.h @@ -22,26 +22,6 @@ #define SPI_BASE_ADDRESS 0xfec10000 #define IO_APIC2_ADDR 0xfec20000
-/* - * AcpiMmio blocks are at fixed offsets from FED8_0000h and enabled in PMx04[1]. - * All ranges not specified as supported below may, or may not, be listed in - * any documentation but should be considered reserved through FED8_1FFFh. - */ -#include <amdblocks/acpimmio_map.h> -#define SUPPORTS_ACPIMMIO_SMI_BASE 1 /* 0xfed80100 */ -#define SUPPORTS_ACPIMMIO_PMIO_BASE 1 /* 0xfed80300 */ -#define SUPPORTS_ACPIMMIO_BIOSRAM_BASE 1 /* 0xfed80500 */ -#define SUPPORTS_ACPIMMIO_ACPI_BASE 1 /* 0xfed80800 */ -#define SUPPORTS_ACPIMMIO_ASF_BASE 1 /* 0xfed80900 */ -#define SUPPORTS_ACPIMMIO_SMBUS_BASE 1 /* 0xfed80a00 */ -#define SUPPORTS_ACPIMMIO_IOMUX_BASE 1 /* 0xfed80d00 */ -#define SUPPORTS_ACPIMMIO_MISC_BASE 1 /* 0xfed80e00 */ -#define SUPPORTS_ACPIMMIO_GPIO0_BASE 1 /* 0xfed81500 */ -#define SUPPORTS_ACPIMMIO_GPIO1_BASE 1 /* 0xfed81800 */ -#define SUPPORTS_ACPIMMIO_GPIO2_BASE 1 /* 0xfed81700 */ -#define SUPPORTS_ACPIMMIO_XHCIPM_BASE 1 /* 0xfed81c00 */ -#define SUPPORTS_ACPIMMIO_AOAC_BASE 1 /* 0xfed81e00 */ - #define ALINK_AHB_ADDRESS 0xfedc0000
/* I2C fixed address */
-
Kyösti Mälkki (Code Review) -
Michał Żygowski (Code Review)
-
Richard Spiegel (Code Review)