Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48434 )
Change subject: soc/amd/common/block/smbus: refactor fch_smbus_init ......................................................................
soc/amd/common/block/smbus: refactor fch_smbus_init
Move the setup of the base address to a separate function and explicitly set the SMBUS and ASF I/O port decode even though it is expected to already be set after reset.
Change-Id: I8072ab78985021d19b6528100c674ecdd777e62e Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/common/block/smbus/smbus_early_fch.c 2 files changed, 14 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/48434/1
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h index 3887f73..865ad48 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -15,12 +15,12 @@ * newer SoCs, but not for the generations with separate FCH or Kabini. */ #define PM_DECODE_EN 0x00 +#define SMBUS_ASF_IO_BASE_SHIFT 8 +#define SMBUS_ASF_IO_BASE_MASK (0xff << SMBUS_ASF_IO_BASE_SHIFT) #define SMBUS_ASF_IO_EN (1 << 4) #define CF9_IO_EN (1 << 1) #define LEGACY_IO_EN (1 << 0)
-#define SMB_ASF_IO_BASE 0x01 /* part of PM_DECODE_EN */ - /* * 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 diff --git a/src/soc/amd/common/block/smbus/smbus_early_fch.c b/src/soc/amd/common/block/smbus/smbus_early_fch.c index 3d04778..2b20425 100644 --- a/src/soc/amd/common/block/smbus/smbus_early_fch.c +++ b/src/soc/amd/common/block/smbus/smbus_early_fch.c @@ -3,14 +3,24 @@ #include <stdint.h> #include <amdblocks/acpimmio.h> #include <amdblocks/smbus.h> -#include <soc/southbridge.h> +#include <soc/iomap.h> + +static void fch_smbus_enable_decode(uint16_t base) +{ + uint32_t val = pm_read32(PM_DECODE_EN); + /* Configure upper byte of the I/O address; lower byte is always 0 */ + val = (val & ~SMBUS_ASF_IO_BASE_MASK) | (base & SMBUS_ASF_IO_BASE_MASK); + /* Set enable decode bit even though it should already be set */ + val |= SMBUS_ASF_IO_EN; + pm_write32(PM_DECODE_EN, val); +}
void fch_smbus_init(void) { /* 400 kHz smbus speed. */ const uint8_t smbus_speed = (66000000 / (400000 * 4));
- pm_write8(SMB_ASF_IO_BASE, SMB_BASE_ADDR >> 8); + fch_smbus_enable_decode(SMB_BASE_ADDR); smbus_write8(SMBTIMING, smbus_speed); /* Clear all SMBUS status bits */ smbus_write8(SMBHSTSTAT, SMBHST_STAT_CLEAR);
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48434 )
Change subject: soc/amd/common/block/smbus: refactor fch_smbus_init ......................................................................
Patch Set 1: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48434 )
Change subject: soc/amd/common/block/smbus: refactor fch_smbus_init ......................................................................
soc/amd/common/block/smbus: refactor fch_smbus_init
Move the setup of the base address to a separate function and explicitly set the SMBUS and ASF I/O port decode even though it is expected to already be set after reset.
Change-Id: I8072ab78985021d19b6528100c674ecdd777e62e Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/48434 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/include/amdblocks/acpimmio.h M src/soc/amd/common/block/smbus/smbus_early_fch.c 2 files changed, 14 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Marshall Dawson: 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 3887f73..865ad48 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h +++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h @@ -15,12 +15,12 @@ * newer SoCs, but not for the generations with separate FCH or Kabini. */ #define PM_DECODE_EN 0x00 +#define SMBUS_ASF_IO_BASE_SHIFT 8 +#define SMBUS_ASF_IO_BASE_MASK (0xff << SMBUS_ASF_IO_BASE_SHIFT) #define SMBUS_ASF_IO_EN (1 << 4) #define CF9_IO_EN (1 << 1) #define LEGACY_IO_EN (1 << 0)
-#define SMB_ASF_IO_BASE 0x01 /* part of PM_DECODE_EN */ - /* * 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 diff --git a/src/soc/amd/common/block/smbus/smbus_early_fch.c b/src/soc/amd/common/block/smbus/smbus_early_fch.c index 3d04778..2b20425 100644 --- a/src/soc/amd/common/block/smbus/smbus_early_fch.c +++ b/src/soc/amd/common/block/smbus/smbus_early_fch.c @@ -3,14 +3,24 @@ #include <stdint.h> #include <amdblocks/acpimmio.h> #include <amdblocks/smbus.h> -#include <soc/southbridge.h> +#include <soc/iomap.h> + +static void fch_smbus_enable_decode(uint16_t base) +{ + uint32_t val = pm_read32(PM_DECODE_EN); + /* Configure upper byte of the I/O address; lower byte is always 0 */ + val = (val & ~SMBUS_ASF_IO_BASE_MASK) | (base & SMBUS_ASF_IO_BASE_MASK); + /* Set enable decode bit even though it should already be set */ + val |= SMBUS_ASF_IO_EN; + pm_write32(PM_DECODE_EN, val); +}
void fch_smbus_init(void) { /* 400 kHz smbus speed. */ const uint8_t smbus_speed = (66000000 / (400000 * 4));
- pm_write8(SMB_ASF_IO_BASE, SMB_BASE_ADDR >> 8); + fch_smbus_enable_decode(SMB_BASE_ADDR); smbus_write8(SMBTIMING, smbus_speed); /* Clear all SMBUS status bits */ smbus_write8(SMBHSTSTAT, SMBHST_STAT_CLEAR);
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48434 )
Change subject: soc/amd/common/block/smbus: refactor fch_smbus_init ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48434/2/src/soc/amd/common/block/sm... File src/soc/amd/common/block/smbus/smbus_early_fch.c:
https://review.coreboot.org/c/coreboot/+/48434/2/src/soc/amd/common/block/sm... PS2, Line 23: fch_smbus_enable_decode(SMB_BASE_ADDR); Since CB:29258 smbus_writeX() below is MMIO, while function above is for IO decode.
-
Felix Held (Code Review) -
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)