Hello Richard Spiegel, Martin Roth, Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32645
to review the following change.
Change subject: soc/amd/stoneyridge: Add aoac_ read/write functions ......................................................................
soc/amd/stoneyridge: Add aoac_ read/write functions
Add 8-bit functions to access the AOAC registers and use them in southbridge.c. At this time, there is no reason to pursue WORD or DWORD access and it's not known if those transaction sizes are supported.
Change-Id: I3a8f493625f941fb855c0b8a0eff511a9a5ddfe8 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/sb_util.c M src/soc/amd/stoneyridge/southbridge.c 3 files changed, 16 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/32645/1
diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h index aed3288..098d24e 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -533,6 +533,8 @@ uint16_t smbus_read16(uint8_t offset); void smbus_write8(uint8_t offset, uint8_t value); void smbus_write16(uint8_t offset, uint16_t value); +uint8_t aoac_read8(uint8_t reg); +void aoac_write8(uint8_t reg, uint8_t value); void bootblock_fch_early_init(void); void bootblock_fch_init(void); /** diff --git a/src/soc/amd/stoneyridge/sb_util.c b/src/soc/amd/stoneyridge/sb_util.c index 4bffdbc..530e9cd 100644 --- a/src/soc/amd/stoneyridge/sb_util.c +++ b/src/soc/amd/stoneyridge/sb_util.c @@ -271,6 +271,16 @@
/* aoac read/write - access registers at 0xfed81e00 - not currently used */
+u8 aoac_read8(u8 reg) +{ + return read8((void *)(ACPIMMIO_AOAC_BASE + reg)); +} + +void aoac_write8(u8 reg, u8 value) +{ + write8((void *)(ACPIMMIO_AOAC_BASE + reg), value); +} + uint16_t pm_acpi_pm_cnt_blk(void) { return pm_read16(PM1_CNT_BLK); diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index 66894a2..7dc27c8 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -279,20 +279,18 @@ static void power_on_aoac_device(int aoac_device_control_register) { uint8_t byte; - uint8_t *register_pointer = (uint8_t *)(uintptr_t)ACPIMMIO_AOAC_BASE - + aoac_device_control_register;
/* Power on the UART and AMBA devices */ - byte = read8(register_pointer); + byte = aoac_read8(aoac_device_control_register); byte |= FCH_AOAC_PWR_ON_DEV; - write8(register_pointer, byte); + aoac_write8(aoac_device_control_register, byte); }
static bool is_aoac_device_enabled(int aoac_device_status_register) { uint8_t byte; - byte = read8((uint8_t *)(uintptr_t)ACPIMMIO_AOAC_BASE - + aoac_device_status_register); + + byte = aoac_read8(aoac_device_status_register); byte &= (FCH_AOAC_PWR_RST_STATE | FCH_AOAC_RST_CLK_OK_STATE); if (byte == (FCH_AOAC_PWR_RST_STATE | FCH_AOAC_RST_CLK_OK_STATE)) return true;
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32645 )
Change subject: soc/amd/stoneyridge: Add aoac_ read/write functions ......................................................................
Patch Set 1: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32645 )
Change subject: soc/amd/stoneyridge: Add aoac_ read/write functions ......................................................................
Patch Set 1: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32645 )
Change subject: soc/amd/stoneyridge: Add aoac_ read/write functions ......................................................................
soc/amd/stoneyridge: Add aoac_ read/write functions
Add 8-bit functions to access the AOAC registers and use them in southbridge.c. At this time, there is no reason to pursue WORD or DWORD access and it's not known if those transaction sizes are supported.
Change-Id: I3a8f493625f941fb855c0b8a0eff511a9a5ddfe8 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32645 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Richard Spiegel richard.spiegel@silverbackltd.com Reviewed-by: Martin Roth martinroth@google.com --- M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/sb_util.c M src/soc/amd/stoneyridge/southbridge.c 3 files changed, 16 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Richard Spiegel: Looks good to me, approved
diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h index aed3288..098d24e 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -533,6 +533,8 @@ uint16_t smbus_read16(uint8_t offset); void smbus_write8(uint8_t offset, uint8_t value); void smbus_write16(uint8_t offset, uint16_t value); +uint8_t aoac_read8(uint8_t reg); +void aoac_write8(uint8_t reg, uint8_t value); void bootblock_fch_early_init(void); void bootblock_fch_init(void); /** diff --git a/src/soc/amd/stoneyridge/sb_util.c b/src/soc/amd/stoneyridge/sb_util.c index 4bffdbc..530e9cd 100644 --- a/src/soc/amd/stoneyridge/sb_util.c +++ b/src/soc/amd/stoneyridge/sb_util.c @@ -271,6 +271,16 @@
/* aoac read/write - access registers at 0xfed81e00 - not currently used */
+u8 aoac_read8(u8 reg) +{ + return read8((void *)(ACPIMMIO_AOAC_BASE + reg)); +} + +void aoac_write8(u8 reg, u8 value) +{ + write8((void *)(ACPIMMIO_AOAC_BASE + reg), value); +} + uint16_t pm_acpi_pm_cnt_blk(void) { return pm_read16(PM1_CNT_BLK); diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index 66894a2..7dc27c8 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -279,20 +279,18 @@ static void power_on_aoac_device(int aoac_device_control_register) { uint8_t byte; - uint8_t *register_pointer = (uint8_t *)(uintptr_t)ACPIMMIO_AOAC_BASE - + aoac_device_control_register;
/* Power on the UART and AMBA devices */ - byte = read8(register_pointer); + byte = aoac_read8(aoac_device_control_register); byte |= FCH_AOAC_PWR_ON_DEV; - write8(register_pointer, byte); + aoac_write8(aoac_device_control_register, byte); }
static bool is_aoac_device_enabled(int aoac_device_status_register) { uint8_t byte; - byte = read8((uint8_t *)(uintptr_t)ACPIMMIO_AOAC_BASE - + aoac_device_status_register); + + byte = aoac_read8(aoac_device_status_register); byte &= (FCH_AOAC_PWR_RST_STATE | FCH_AOAC_RST_CLK_OK_STATE); if (byte == (FCH_AOAC_PWR_RST_STATE | FCH_AOAC_RST_CLK_OK_STATE)) return true;
-
Marshall Dawson (Code Review) -
Martin Roth (Code Review)
-
Richard Spiegel (Code Review)