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.