Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
amdblocks/acpimmio: add missing MMIO functions
Add missing Power Management 2, old and new GPIO functions to modify the contents of these MMIO blocks.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: Ie4db6a4d12d9122ea5b87147adbf7b632ac2b311 --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h 2 files changed, 156 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/37813/1
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index c441ab8..7194853 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -214,6 +214,36 @@ write32((void *)(ACPIMMIO_PMIO_BASE + reg), value); }
+static inline uint8_t pm2_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_PMIO2_BASE + reg)); +} + +static inline uint16_t pm2_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_PMIO2_BASE + reg)); +} + +static inline uint32_t pm2_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_PMIO2_BASE + reg)); +} + +static inline void pm2_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_PMIO2_BASE + reg), value); +} + +static inline void pm2_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_PMIO2_BASE + reg), value); +} + +static inline void pm2_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_PMIO2_BASE + reg), value); +} + static inline uint8_t biosram_read8(uint8_t reg) { return read8((void *)(ACPIMMIO_BIOSRAM_BASE + reg)); @@ -354,6 +384,131 @@ write32((void *)(ACPIMMIO_MISC_BASE + reg), value); }
+/* Old GPIO configuration registers */ +static inline uint8_t gpio_100_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_GPIO_BASE_100 + reg)); +} + +static inline uint16_t gpio_100_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO_BASE_100 + reg)); +} + +static inline uint32_t gpio_100_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO_BASE_100 + reg)); +} + +static inline void gpio_100_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value); +} + +static inline void gpio_100_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value); +} + +static inline void gpio_100_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO_BASE_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)); +} + +static inline uint16_t gpio0_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); +} + +static inline uint32_t gpio0_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); +} + +static inline void gpio0_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); +} + +static inline void gpio0_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); +} + +static inline void gpio0_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); +} + +/* GPIO bank 1 */ +static inline uint8_t gpio1_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); +} + +static inline uint16_t gpio1_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); +} + +static inline uint32_t gpio1_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); +} + +static inline void gpio1_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); +} + +static inline void gpio1_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); +} + +static inline void gpio1_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); +} + +/* GPIO bank 2 */ +static inline uint8_t gpio2_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); +} + +static inline uint16_t gpio2_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); +} + +static inline uint32_t gpio2_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); +} + +static inline void gpio2_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); +} + +static inline void gpio2_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); +} + +static inline void gpio2_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); +} + static inline uint8_t xhci_pm_read8(uint8_t reg) { return read8((void *)(ACPIMMIO_XHCIPM_BASE + reg)); 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 9a15840..4d62b39 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h @@ -48,6 +48,7 @@
#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
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37813/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37813/1//COMMIT_MSG@9 PS1, Line 9: new I don't agree on adding new GPIO. There are functions already for them.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 217: static inline uint8_t pm2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline uint16_t pm2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline uint32_t pm2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline void pm2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : : static inline void pm2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : : static inline void pm2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : : These functions should be protected by SUPPORTS_ACPIMMIO_PMIO2_BASE. Even better, declare them at soc/amd/common/block/acpimmio/mmio_util.c and only leave the prototypes here, still protected by SUPPORTS_ACPIMMIO_PMIO2_BASE (and don't use inline or static).
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 387: /* Old GPIO configuration registers */ new SOC don't implement these registers, old SOC manipulated them through AGESA. What is the point of implementing these functions, unless you plan to support old SOC GPIO without using AGESA (for clarity and speed?).
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 417: : /* New GPIO banks configuration registers */ : /* GPIO bank 0 */ : static inline uint8_t gpio0_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint16_t gpio0_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint32_t gpio0_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline void gpio0_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : /* GPIO bank 1 */ : static inline uint8_t gpio1_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint16_t gpio1_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint32_t gpio1_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline void gpio1_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : /* GPIO bank 2 */ : static inline uint8_t gpio2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint16_t gpio2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint32_t gpio2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline void gpio2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } I don't see value on these. GPIO configuration is quite complicated, accomplished by soc/amd/common/block/gpio_banks/gpio.c using macros and tables. At most I would leave the read functions, I don't want anyone messing with these registers without knowing what they are doing... but it would be better to fully hide, thus I would also remove the read functions.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 217: static inline uint8_t pm2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline uint16_t pm2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline uint32_t pm2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline void pm2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : : static inline void pm2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : : static inline void pm2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : :
These functions should be protected by SUPPORTS_ACPIMMIO_PMIO2_BASE. […]
We have intentionally dropped the guards, and they are inlined for better code density.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 387: /* Old GPIO configuration registers */
new SOC don't implement these registers, old SOC manipulated them through AGESA. […]
Keeping VBOOT in mind, bootblock will not be allowed to call AGESA, specially the blobbed one.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 417: : /* New GPIO banks configuration registers */ : /* GPIO bank 0 */ : static inline uint8_t gpio0_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint16_t gpio0_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint32_t gpio0_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline void gpio0_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : /* GPIO bank 1 */ : static inline uint8_t gpio1_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint16_t gpio1_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint32_t gpio1_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline void gpio1_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : /* GPIO bank 2 */ : static inline uint8_t gpio2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint16_t gpio2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint32_t gpio2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline void gpio2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : }
I don't see value on these. […]
I kind of agree about the hiding but we are not there yet and things are open-coded currently. I believe we will identify some of these direct GPIO manipulations will translate into composite enable 48MHz clock function calls.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 51: #define ACPIMMIO_GPIO_BASE_100 0xfed80100 How was it called int the specs?
_LEGACY_GPIO_ ?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 387: /* Old GPIO configuration registers */
Keeping VBOOT in mind, bootblock will not be allowed to call AGESA, specially the blobbed one.
As hinted below, selecting and supporting GENERIC_GPIO_LIB on a platform may be more useful.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 417: : /* New GPIO banks configuration registers */ : /* GPIO bank 0 */ : static inline uint8_t gpio0_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint16_t gpio0_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint32_t gpio0_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline void gpio0_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : /* GPIO bank 1 */ : static inline uint8_t gpio1_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint16_t gpio1_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint32_t gpio1_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline void gpio1_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : /* GPIO bank 2 */ : static inline uint8_t gpio2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint16_t gpio2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint32_t gpio2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline void gpio2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : }
I kind of agree about the hiding but we are not there yet and things are open-coded currently. […]
I had contemplated adding these, if nothing else, for completeness, however they weren't being used currently and I couldn't foresee how they would ever be useful. Selecting and supporting GENERIC_GPIO_LIB was much more relevant for modern features.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 51: #define ACPIMMIO_GPIO_BASE_100 0xfed80100
How was it called int the specs? […]
IIRC the specs simply called the block "GPIO". Then AMD changed the GPIO design -- these went away, and the banked ones starting at +500 appeared. I was careful to name the newer ones "SOC_..._BANKED_GPIOS" in Kconfig and common//gpio_banks/gpio.c in the event support for the older design was added to this code.
BTW for Family 16h integrated FCHs, Kabini was the old design and Mullins the banked design. All discrete FCHs (i.e. attached to Ontario, Trinity, etc.) were the old design. The Family 15h integrated FCHs (e.g. Carrizo and Stoney Ridge) implemented the GPIO banks.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 417: : /* New GPIO banks configuration registers */ : /* GPIO bank 0 */ : static inline uint8_t gpio0_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint16_t gpio0_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint32_t gpio0_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline void gpio0_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : /* GPIO bank 1 */ : static inline uint8_t gpio1_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint16_t gpio1_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint32_t gpio1_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline void gpio1_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : /* GPIO bank 2 */ : static inline uint8_t gpio2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint16_t gpio2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint32_t gpio2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline void gpio2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : }
I had contemplated adding these, if nothing else, for completeness, however they weren't being used […]
In my vision, the long-term solution is that each of these regions will get assumed ownership in a "module" and these direct MMIO accessors are not exposed. Seemed like BIOSRAM would be ready for this already: CB:37862.
That GENERIC_GPIO_LIB is for exposing actual general-purpose IOs to ACPI, right? I believe most of the change we are currently rolling is the configuration of pins to purpose other than general-purpose.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 387: /* Old GPIO configuration registers */
As hinted below, selecting and supporting GENERIC_GPIO_LIB on a platform may be more useful.
Agree, for the old GPIO block at 0x100 it may be a more appropriate way.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 417: : /* New GPIO banks configuration registers */ : /* GPIO bank 0 */ : static inline uint8_t gpio0_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint16_t gpio0_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint32_t gpio0_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline void gpio0_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : /* GPIO bank 1 */ : static inline uint8_t gpio1_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint16_t gpio1_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint32_t gpio1_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline void gpio1_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : /* GPIO bank 2 */ : static inline uint8_t gpio2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint16_t gpio2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint32_t gpio2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline void gpio2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : }
In my vision, the long-term solution is that each of these regions will get assumed ownership in a " […]
My goal was to unify the GPIO configuration for mainboards. Currently, they use various methods to manipulate their configuration: volatile pointers, readXX/writeXX and typically whole bytes are written, not decoding what it actually does. 48MHz clock is only one thing.
On the other hand, there is a nice AMD GPIO block as Richard pointed, but there again I would have to cope with "soc/*" includes. I just wonder whether a transfer from cpu/sb/nb to soc is a worthy effort for older families.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 217: static inline uint8_t pm2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline uint16_t pm2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline uint32_t pm2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_PMIO2_BASE + reg)); : } : : static inline void pm2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : : static inline void pm2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : : static inline void pm2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_PMIO2_BASE + reg), value); : } : :
We have intentionally dropped the guards, and they are inlined for better code density.
Ack
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 387: /* Old GPIO configuration registers */
Agree, for the old GPIO block at 0x100 it may be a more appropriate way.
But are you planning on supporting bootblock GPIO programming on old platforms? It has not been necessary so far, and most platforms can wait some milliseconds until romstage is reached and than programming GPIO.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 417: : /* New GPIO banks configuration registers */ : /* GPIO bank 0 */ : static inline uint8_t gpio0_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint16_t gpio0_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint32_t gpio0_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline void gpio0_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : /* GPIO bank 1 */ : static inline uint8_t gpio1_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint16_t gpio1_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint32_t gpio1_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline void gpio1_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : /* GPIO bank 2 */ : static inline uint8_t gpio2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint16_t gpio2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint32_t gpio2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline void gpio2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : }
My goal was to unify the GPIO configuration for mainboards. […]
Older families that implement this GPIO and are not SOC included will implement GPIO through AGESA, normally at romstage and sometimes at a later stage. And yes, in this situation you have to be an expert to know the bytes to be placed on the table that is fed to AGESA. I would really prefer to not have these functions available, it opens a big can of worms with possibility of mistakes done by those that don't fully understand (final client/user) what they are doing.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 1: Code-Review+2
I am not saying the approach with _gpio100 here is a final one, but it's easier to proceed and see the big picture after we get CB:37400 and CB:37401 in. Please rebase and take CB:37862 on the same branch.
Hello Kyösti Mälkki, Marshall Dawson, Richard Spiegel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37813
to look at the new patch set (#2).
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
amdblocks/acpimmio: add missing MMIO functions
Add missing Power Management 2, old and new GPIO functions to modify the contents of these MMIO blocks.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: Ie4db6a4d12d9122ea5b87147adbf7b632ac2b311 --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h 2 files changed, 166 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/37813/2
Hello Kyösti Mälkki, Marshall Dawson, Richard Spiegel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37813
to look at the new patch set (#3).
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
amdblocks/acpimmio: add missing MMIO functions
Add missing Power Management 2, old and new GPIO functions to modify the contents of these MMIO blocks.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: Ie4db6a4d12d9122ea5b87147adbf7b632ac2b311 --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h 2 files changed, 156 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/37813/3
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37813/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37813/1//COMMIT_MSG@9 PS1, Line 9: new
I don't agree on adding new GPIO. There are functions already for them.
We'll get there and see how much of existing GPIO block is usable.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 387: /* Old GPIO configuration registers */
But are you planning on supporting bootblock GPIO programming on old platforms? It has not been nece […]
It's a bit nasty to leave some GPIO programming as not available in bootblock. In theory, there could be RS232/RS485 transceiver controls behind GPIOs that need to be programmed to get early UART.
More importatly, this enables a fair amount of cleanup, while maybe not final solution.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 417: : /* New GPIO banks configuration registers */ : /* GPIO bank 0 */ : static inline uint8_t gpio0_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint16_t gpio0_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline uint32_t gpio0_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); : } : : static inline void gpio0_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : static inline void gpio0_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); : } : : /* GPIO bank 1 */ : static inline uint8_t gpio1_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint16_t gpio1_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline uint32_t gpio1_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); : } : : static inline void gpio1_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : static inline void gpio1_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); : } : : /* GPIO bank 2 */ : static inline uint8_t gpio2_read8(uint8_t reg) : { : return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint16_t gpio2_read16(uint8_t reg) : { : return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline uint32_t gpio2_read32(uint8_t reg) : { : return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); : } : : static inline void gpio2_write8(uint8_t reg, uint8_t value) : { : write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write16(uint8_t reg, uint16_t value) : { : write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : } : : static inline void gpio2_write32(uint8_t reg, uint32_t value) : { : write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); : }
Older families that implement this GPIO and are not SOC included will implement GPIO through AGESA, […]
Easiest thing is not to call them. The process of copy-pasting old platform sources with incomplete review, specially across chipset generations, is doomed anyways. And GPIO programming calls have to be under mainboard/ anyways, and you should never-ever copy-paste GPIO programming from one board to another due the possibility of configuring two opposite ends of signal trace both as push-pull outputs and permanently bricking hardware.
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio_map.h:
https://review.coreboot.org/c/coreboot/+/37813/1/src/soc/amd/common/block/in... PS1, Line 51: #define ACPIMMIO_GPIO_BASE_100 0xfed80100
IIRC the specs simply called the block "GPIO". […]
Ack
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
Patch Set 3: Code-Review+2
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37813 )
Change subject: amdblocks/acpimmio: add missing MMIO functions ......................................................................
amdblocks/acpimmio: add missing MMIO functions
Add missing Power Management 2, old and new GPIO functions to modify the contents of these MMIO blocks.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: Ie4db6a4d12d9122ea5b87147adbf7b632ac2b311 Reviewed-on: https://review.coreboot.org/c/coreboot/+/37813 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/common/block/include/amdblocks/acpimmio_map.h 2 files changed, 156 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index 46d088f..b4a4b50 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -206,6 +206,36 @@ write32((void *)(ACPIMMIO_PMIO_BASE + reg), value); }
+static inline uint8_t pm2_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_PMIO2_BASE + reg)); +} + +static inline uint16_t pm2_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_PMIO2_BASE + reg)); +} + +static inline uint32_t pm2_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_PMIO2_BASE + reg)); +} + +static inline void pm2_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_PMIO2_BASE + reg), value); +} + +static inline void pm2_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_PMIO2_BASE + reg), value); +} + +static inline void pm2_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_PMIO2_BASE + reg), value); +} + static inline uint8_t acpi_read8(uint8_t reg) { return read8((void *)(ACPIMMIO_ACPI_BASE + reg)); @@ -336,6 +366,131 @@ write32((void *)(ACPIMMIO_MISC_BASE + reg), value); }
+/* Old GPIO configuration registers */ +static inline uint8_t gpio_100_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_GPIO_BASE_100 + reg)); +} + +static inline uint16_t gpio_100_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO_BASE_100 + reg)); +} + +static inline uint32_t gpio_100_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO_BASE_100 + reg)); +} + +static inline void gpio_100_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value); +} + +static inline void gpio_100_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO_BASE_100 + reg), value); +} + +static inline void gpio_100_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO_BASE_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)); +} + +static inline uint16_t gpio0_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO0_BASE + reg)); +} + +static inline uint32_t gpio0_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO0_BASE + reg)); +} + +static inline void gpio0_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO0_BASE + reg), value); +} + +static inline void gpio0_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO0_BASE + reg), value); +} + +static inline void gpio0_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO0_BASE + reg), value); +} + +/* GPIO bank 1 */ +static inline uint8_t gpio1_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_GPIO1_BASE + reg)); +} + +static inline uint16_t gpio1_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO1_BASE + reg)); +} + +static inline uint32_t gpio1_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO1_BASE + reg)); +} + +static inline void gpio1_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO1_BASE + reg), value); +} + +static inline void gpio1_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO1_BASE + reg), value); +} + +static inline void gpio1_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO1_BASE + reg), value); +} + +/* GPIO bank 2 */ +static inline uint8_t gpio2_read8(uint8_t reg) +{ + return read8((void *)(ACPIMMIO_GPIO2_BASE + reg)); +} + +static inline uint16_t gpio2_read16(uint8_t reg) +{ + return read16((void *)(ACPIMMIO_GPIO2_BASE + reg)); +} + +static inline uint32_t gpio2_read32(uint8_t reg) +{ + return read32((void *)(ACPIMMIO_GPIO2_BASE + reg)); +} + +static inline void gpio2_write8(uint8_t reg, uint8_t value) +{ + write8((void *)(ACPIMMIO_GPIO2_BASE + reg), value); +} + +static inline void gpio2_write16(uint8_t reg, uint16_t value) +{ + write16((void *)(ACPIMMIO_GPIO2_BASE + reg), value); +} + +static inline void gpio2_write32(uint8_t reg, uint32_t value) +{ + write32((void *)(ACPIMMIO_GPIO2_BASE + reg), value); +} + static inline uint8_t xhci_pm_read8(uint8_t reg) { return read8((void *)(ACPIMMIO_XHCIPM_BASE + reg)); 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 9a15840..4d62b39 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio_map.h @@ -48,6 +48,7 @@
#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
-
Kyösti Mälkki (Code Review) -
Marshall Dawson (Code Review)
-
Michał Żygowski (Code Review)
-
Richard Spiegel (Code Review)