Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48181 )
Change subject: soc/amd/stoneyridge: align AOAC code with Picasso ......................................................................
soc/amd/stoneyridge: align AOAC code with Picasso
In commit 09d50671e6b43c23853a91ff4d6fb26c1e7e17a1 the AOAC code was reworked for Picasso and this patch ports this back to Stoneyridge to facilitate factoring out the functionality into common code.
Change-Id: I836b91dc647987d064170fff7c8ca6ef2ee49211 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/southbridge.c 2 files changed, 36 insertions(+), 48 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/48181/1
diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h index cbd9caa..9450a92 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -190,16 +190,20 @@ #define DEBUG_PORT_MASK (BIT(16) | BIT(17) | BIT(18))
/* FCH AOAC Registers 0xfed81e00 */ -#define FCH_AOAC_D3_CONTROL_CLK_GEN 0x40 -#define FCH_AOAC_D3_CONTROL_I2C0 0x4a -#define FCH_AOAC_D3_CONTROL_I2C1 0x4c -#define FCH_AOAC_D3_CONTROL_I2C2 0x4e -#define FCH_AOAC_D3_CONTROL_I2C3 0x50 -#define FCH_AOAC_D3_CONTROL_UART0 0x56 -#define FCH_AOAC_D3_CONTROL_UART1 0x58 -#define FCH_AOAC_D3_CONTROL_AMBA 0x62 -#define FCH_AOAC_D3_CONTROL_USB2 0x64 -#define FCH_AOAC_D3_CONTROL_USB3 0x6e +#define AOAC_DEV_D3_CTL(device) (0x40 + device * 2) +#define AOAC_DEV_D3_STATE(device) (AOAC_DEV_D3_CTL(device) + 1) + +#define FCH_AOAC_DEV_CLK_GEN 0 +#define FCH_AOAC_DEV_I2C0 5 +#define FCH_AOAC_DEV_I2C1 6 +#define FCH_AOAC_DEV_I2C2 7 +#define FCH_AOAC_DEV_I2C3 8 +#define FCH_AOAC_DEV_UART0 11 +#define FCH_AOAC_DEV_UART1 12 +#define FCH_AOAC_DEV_AMBA 17 +#define FCH_AOAC_DEV_USB2 18 +#define FCH_AOAC_DEV_USB3 23 + /* Bit definitions for all FCH_AOAC_D3_CONTROL_* Registers */ #define FCH_AOAC_TARGET_DEVICE_STATE (BIT(0) + BIT(1)) #define FCH_AOAC_DEVICE_STATE BIT(2) @@ -209,16 +213,6 @@ #define FCH_AOAC_SW_RST_B BIT(6) #define FCH_AOAC_IS_SW_CONTROL BIT(7)
-#define FCH_AOAC_D3_STATE_CLK_GEN 0x41 -#define FCH_AOAC_D3_STATE_I2C0 0x4b -#define FCH_AOAC_D3_STATE_I2C1 0x4d -#define FCH_AOAC_D3_STATE_I2C2 0x4f -#define FCH_AOAC_D3_STATE_I2C3 0x51 -#define FCH_AOAC_D3_STATE_UART0 0x57 -#define FCH_AOAC_D3_STATE_UART1 0x59 -#define FCH_AOAC_D3_STATE_AMBA 0x63 -#define FCH_AOAC_D3_STATE_USB2 0x65 -#define FCH_AOAC_D3_STATE_USB3 0x6f /* Bit definitions for all FCH_AOAC_D3_STATE_* Registers */ #define FCH_AOAC_PWR_RST_STATE BIT(0) #define FCH_AOAC_RST_CLK_OK_STATE BIT(1) @@ -292,11 +286,6 @@ #define RST_CMD BIT(2) #define SYS_RST BIT(1)
-struct stoneyridge_aoac { - int enable; - int status; -}; - typedef struct aoac_devs { unsigned int :5; unsigned int ic0e:1; /* 5: I2C0 */ diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index 267f747..05f3072 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -31,14 +31,13 @@ * waiting for each device to become available, a single delay will be * executed. */ -static const struct stoneyridge_aoac aoac_devs[] = { - { (FCH_AOAC_D3_CONTROL_UART0 + CONFIG_UART_FOR_CONSOLE * 2), - (FCH_AOAC_D3_STATE_UART0 + CONFIG_UART_FOR_CONSOLE * 2) }, - { FCH_AOAC_D3_CONTROL_AMBA, FCH_AOAC_D3_STATE_AMBA }, - { FCH_AOAC_D3_CONTROL_I2C0, FCH_AOAC_D3_STATE_I2C0 }, - { FCH_AOAC_D3_CONTROL_I2C1, FCH_AOAC_D3_STATE_I2C1 }, - { FCH_AOAC_D3_CONTROL_I2C2, FCH_AOAC_D3_STATE_I2C2 }, - { FCH_AOAC_D3_CONTROL_I2C3, FCH_AOAC_D3_STATE_I2C3 } +static const unsigned int aoac_devs[] = { + FCH_AOAC_DEV_UART0 + CONFIG_UART_FOR_CONSOLE * 2, + FCH_AOAC_DEV_AMBA, + FCH_AOAC_DEV_I2C0, + FCH_AOAC_DEV_I2C1, + FCH_AOAC_DEV_I2C2, + FCH_AOAC_DEV_I2C3, };
static int is_sata_config(void) @@ -146,21 +145,21 @@ return irq_association; }
-static void power_on_aoac_device(int aoac_device_control_register) +static void power_on_aoac_device(unsigned int dev) { uint8_t byte;
/* Power on the UART and AMBA devices */ - byte = aoac_read8(aoac_device_control_register); + byte = aoac_read8(AOAC_DEV_D3_CTL(dev)); byte |= FCH_AOAC_PWR_ON_DEV; - aoac_write8(aoac_device_control_register, byte); + aoac_write8(AOAC_DEV_D3_CTL(dev), byte); }
-static bool is_aoac_device_enabled(int aoac_device_status_register) +static bool is_aoac_device_enabled(unsigned int dev) { uint8_t byte;
- byte = aoac_read8(aoac_device_status_register); + byte = aoac_read8(AOAC_DEV_D3_STATE(dev)); 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; @@ -174,14 +173,14 @@ int i;
for (i = 0; i < ARRAY_SIZE(aoac_devs); i++) - power_on_aoac_device(aoac_devs[i].enable); + power_on_aoac_device(aoac_devs[i]);
/* Wait for AOAC devices to indicate power and clock OK */ do { udelay(100); status = true; for (i = 0; i < ARRAY_SIZE(aoac_devs); i++) - status &= is_aoac_device_enabled(aoac_devs[i].status); + status &= is_aoac_device_enabled(aoac_devs[i]); } while (!status); }
@@ -545,14 +544,14 @@ if (gnvs == NULL) return;
- gnvs->aoac.ic0e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C0); - gnvs->aoac.ic1e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C1); - gnvs->aoac.ic2e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C2); - gnvs->aoac.ic3e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C3); - gnvs->aoac.ut0e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_UART0); - gnvs->aoac.ut1e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_UART1); - gnvs->aoac.ehce = is_aoac_device_enabled(FCH_AOAC_D3_STATE_USB2); - gnvs->aoac.xhce = is_aoac_device_enabled(FCH_AOAC_D3_STATE_USB3); + gnvs->aoac.ic0e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C0); + gnvs->aoac.ic1e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C1); + gnvs->aoac.ic2e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C2); + gnvs->aoac.ic3e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C3); + gnvs->aoac.ut0e = is_aoac_device_enabled(FCH_AOAC_DEV_UART0); + gnvs->aoac.ut1e = is_aoac_device_enabled(FCH_AOAC_DEV_UART1); + gnvs->aoac.ehce = is_aoac_device_enabled(FCH_AOAC_DEV_USB2); + gnvs->aoac.xhce = is_aoac_device_enabled(FCH_AOAC_DEV_USB3); /* Rely on these being in sync with devicetree */ sd = pcidev_path_on_root(SD_DEVFN); gnvs->aoac.sd_e = sd && sd->enabled ? 1 : 0;
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48181 )
Change subject: soc/amd/stoneyridge: align AOAC code with Picasso ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/48181/1/src/soc/amd/stoneyridge/sou... File src/soc/amd/stoneyridge/southbridge.c:
https://review.coreboot.org/c/coreboot/+/48181/1/src/soc/amd/stoneyridge/sou... PS1, Line 154: FCH_AOAC_PWR_ON_DEV Outside the scope of this patch but picasso clears both b0 and b1 in byte before ORing. I can't recall whether that's because the PSP was leaving some stuff in non-default states.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48181 )
Change subject: soc/amd/stoneyridge: align AOAC code with Picasso ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48181/1/src/soc/amd/stoneyridge/sou... File src/soc/amd/stoneyridge/southbridge.c:
https://review.coreboot.org/c/coreboot/+/48181/1/src/soc/amd/stoneyridge/sou... PS1, Line 154: FCH_AOAC_PWR_ON_DEV
Outside the scope of this patch but picasso clears both b0 and b1 in byte before ORing. […]
i wasn't sure if i should add this here or in the follow up patch. i went the second route; see https://review.coreboot.org/c/coreboot/+/48183/1//COMMIT_MSG#9
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48181 )
Change subject: soc/amd/stoneyridge: align AOAC code with Picasso ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48181/1/src/soc/amd/stoneyridge/sou... File src/soc/amd/stoneyridge/southbridge.c:
https://review.coreboot.org/c/coreboot/+/48181/1/src/soc/amd/stoneyridge/sou... PS1, Line 154: FCH_AOAC_PWR_ON_DEV
i wasn't sure if i should add this here or in the follow up patch. […]
Yep, saw that later. LGTM.
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48181 )
Change subject: soc/amd/stoneyridge: align AOAC code with Picasso ......................................................................
soc/amd/stoneyridge: align AOAC code with Picasso
In commit 09d50671e6b43c23853a91ff4d6fb26c1e7e17a1 the AOAC code was reworked for Picasso and this patch ports this back to Stoneyridge to facilitate factoring out the functionality into common code.
Change-Id: I836b91dc647987d064170fff7c8ca6ef2ee49211 Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/48181 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/southbridge.c 2 files changed, 36 insertions(+), 48 deletions(-)
Approvals: build bot (Jenkins): Verified Marshall Dawson: 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 cbd9caa..9450a92 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -190,16 +190,20 @@ #define DEBUG_PORT_MASK (BIT(16) | BIT(17) | BIT(18))
/* FCH AOAC Registers 0xfed81e00 */ -#define FCH_AOAC_D3_CONTROL_CLK_GEN 0x40 -#define FCH_AOAC_D3_CONTROL_I2C0 0x4a -#define FCH_AOAC_D3_CONTROL_I2C1 0x4c -#define FCH_AOAC_D3_CONTROL_I2C2 0x4e -#define FCH_AOAC_D3_CONTROL_I2C3 0x50 -#define FCH_AOAC_D3_CONTROL_UART0 0x56 -#define FCH_AOAC_D3_CONTROL_UART1 0x58 -#define FCH_AOAC_D3_CONTROL_AMBA 0x62 -#define FCH_AOAC_D3_CONTROL_USB2 0x64 -#define FCH_AOAC_D3_CONTROL_USB3 0x6e +#define AOAC_DEV_D3_CTL(device) (0x40 + device * 2) +#define AOAC_DEV_D3_STATE(device) (AOAC_DEV_D3_CTL(device) + 1) + +#define FCH_AOAC_DEV_CLK_GEN 0 +#define FCH_AOAC_DEV_I2C0 5 +#define FCH_AOAC_DEV_I2C1 6 +#define FCH_AOAC_DEV_I2C2 7 +#define FCH_AOAC_DEV_I2C3 8 +#define FCH_AOAC_DEV_UART0 11 +#define FCH_AOAC_DEV_UART1 12 +#define FCH_AOAC_DEV_AMBA 17 +#define FCH_AOAC_DEV_USB2 18 +#define FCH_AOAC_DEV_USB3 23 + /* Bit definitions for all FCH_AOAC_D3_CONTROL_* Registers */ #define FCH_AOAC_TARGET_DEVICE_STATE (BIT(0) + BIT(1)) #define FCH_AOAC_DEVICE_STATE BIT(2) @@ -209,16 +213,6 @@ #define FCH_AOAC_SW_RST_B BIT(6) #define FCH_AOAC_IS_SW_CONTROL BIT(7)
-#define FCH_AOAC_D3_STATE_CLK_GEN 0x41 -#define FCH_AOAC_D3_STATE_I2C0 0x4b -#define FCH_AOAC_D3_STATE_I2C1 0x4d -#define FCH_AOAC_D3_STATE_I2C2 0x4f -#define FCH_AOAC_D3_STATE_I2C3 0x51 -#define FCH_AOAC_D3_STATE_UART0 0x57 -#define FCH_AOAC_D3_STATE_UART1 0x59 -#define FCH_AOAC_D3_STATE_AMBA 0x63 -#define FCH_AOAC_D3_STATE_USB2 0x65 -#define FCH_AOAC_D3_STATE_USB3 0x6f /* Bit definitions for all FCH_AOAC_D3_STATE_* Registers */ #define FCH_AOAC_PWR_RST_STATE BIT(0) #define FCH_AOAC_RST_CLK_OK_STATE BIT(1) @@ -292,11 +286,6 @@ #define RST_CMD BIT(2) #define SYS_RST BIT(1)
-struct stoneyridge_aoac { - int enable; - int status; -}; - typedef struct aoac_devs { unsigned int :5; unsigned int ic0e:1; /* 5: I2C0 */ diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index 267f747..05f3072 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -31,14 +31,13 @@ * waiting for each device to become available, a single delay will be * executed. */ -static const struct stoneyridge_aoac aoac_devs[] = { - { (FCH_AOAC_D3_CONTROL_UART0 + CONFIG_UART_FOR_CONSOLE * 2), - (FCH_AOAC_D3_STATE_UART0 + CONFIG_UART_FOR_CONSOLE * 2) }, - { FCH_AOAC_D3_CONTROL_AMBA, FCH_AOAC_D3_STATE_AMBA }, - { FCH_AOAC_D3_CONTROL_I2C0, FCH_AOAC_D3_STATE_I2C0 }, - { FCH_AOAC_D3_CONTROL_I2C1, FCH_AOAC_D3_STATE_I2C1 }, - { FCH_AOAC_D3_CONTROL_I2C2, FCH_AOAC_D3_STATE_I2C2 }, - { FCH_AOAC_D3_CONTROL_I2C3, FCH_AOAC_D3_STATE_I2C3 } +static const unsigned int aoac_devs[] = { + FCH_AOAC_DEV_UART0 + CONFIG_UART_FOR_CONSOLE * 2, + FCH_AOAC_DEV_AMBA, + FCH_AOAC_DEV_I2C0, + FCH_AOAC_DEV_I2C1, + FCH_AOAC_DEV_I2C2, + FCH_AOAC_DEV_I2C3, };
static int is_sata_config(void) @@ -146,21 +145,21 @@ return irq_association; }
-static void power_on_aoac_device(int aoac_device_control_register) +static void power_on_aoac_device(unsigned int dev) { uint8_t byte;
/* Power on the UART and AMBA devices */ - byte = aoac_read8(aoac_device_control_register); + byte = aoac_read8(AOAC_DEV_D3_CTL(dev)); byte |= FCH_AOAC_PWR_ON_DEV; - aoac_write8(aoac_device_control_register, byte); + aoac_write8(AOAC_DEV_D3_CTL(dev), byte); }
-static bool is_aoac_device_enabled(int aoac_device_status_register) +static bool is_aoac_device_enabled(unsigned int dev) { uint8_t byte;
- byte = aoac_read8(aoac_device_status_register); + byte = aoac_read8(AOAC_DEV_D3_STATE(dev)); 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; @@ -174,14 +173,14 @@ int i;
for (i = 0; i < ARRAY_SIZE(aoac_devs); i++) - power_on_aoac_device(aoac_devs[i].enable); + power_on_aoac_device(aoac_devs[i]);
/* Wait for AOAC devices to indicate power and clock OK */ do { udelay(100); status = true; for (i = 0; i < ARRAY_SIZE(aoac_devs); i++) - status &= is_aoac_device_enabled(aoac_devs[i].status); + status &= is_aoac_device_enabled(aoac_devs[i]); } while (!status); }
@@ -545,14 +544,14 @@ if (gnvs == NULL) return;
- gnvs->aoac.ic0e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C0); - gnvs->aoac.ic1e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C1); - gnvs->aoac.ic2e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C2); - gnvs->aoac.ic3e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_I2C3); - gnvs->aoac.ut0e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_UART0); - gnvs->aoac.ut1e = is_aoac_device_enabled(FCH_AOAC_D3_STATE_UART1); - gnvs->aoac.ehce = is_aoac_device_enabled(FCH_AOAC_D3_STATE_USB2); - gnvs->aoac.xhce = is_aoac_device_enabled(FCH_AOAC_D3_STATE_USB3); + gnvs->aoac.ic0e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C0); + gnvs->aoac.ic1e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C1); + gnvs->aoac.ic2e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C2); + gnvs->aoac.ic3e = is_aoac_device_enabled(FCH_AOAC_DEV_I2C3); + gnvs->aoac.ut0e = is_aoac_device_enabled(FCH_AOAC_DEV_UART0); + gnvs->aoac.ut1e = is_aoac_device_enabled(FCH_AOAC_DEV_UART1); + gnvs->aoac.ehce = is_aoac_device_enabled(FCH_AOAC_DEV_USB2); + gnvs->aoac.xhce = is_aoac_device_enabled(FCH_AOAC_DEV_USB3); /* Rely on these being in sync with devicetree */ sd = pcidev_path_on_root(SD_DEVFN); gnvs->aoac.sd_e = sd && sd->enabled ? 1 : 0;
-
Felix Held (Code Review) -
Marshall Dawson (Code Review)