Hello Maulik V Vaghela,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/37670
to review the following change.
Change subject: soc/intel/tigerlake: Pick correct pmc base reg from pch type ......................................................................
soc/intel/tigerlake: Pick correct pmc base reg from pch type
Update PMC shadow register base address for Jasperlake Correct PCH detection logic based on PCH ids and return correct base address based on PCH detected since our code supports both tgl and jsl.
Change-Id: Iea3311b3dc8dc3ee5ea54db1148f386c2a5dd563 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.corp-partner.google.com --- M src/soc/intel/tigerlake/bootblock/pch.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pch.h 3 files changed, 31 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/37670/1
diff --git a/src/soc/intel/tigerlake/bootblock/pch.c b/src/soc/intel/tigerlake/bootblock/pch.c index 1ef4928..8599423 100644 --- a/src/soc/intel/tigerlake/bootblock/pch.c +++ b/src/soc/intel/tigerlake/bootblock/pch.c @@ -37,7 +37,8 @@ #include <soc/pcr_ids.h> #include <soc/pm.h>
-#define PCR_PSF3_TO_SHDW_PMC_REG_BASE 0x0600 +#define PCR_PSF3_TO_SHDW_PMC_REG_BASE_TGP 0x0600 +#define PCR_PSF3_TO_SHDW_PMC_REG_BASE_JSP 0x0980 #define PCR_PSFX_TO_SHDW_BAR0 0 #define PCR_PSFX_TO_SHDW_BAR1 0x4 #define PCR_PSFX_TO_SHDW_BAR2 0x8 @@ -57,6 +58,20 @@ #define PCR_DMI_LPCIOD 0x2770 #define PCR_DMI_LPCIOE 0x2774
+static uint32_t get_pmc_reg_base(void) +{ + uint8_t pch_series; + + pch_series = get_pch_series(); + + if (pch_series == PCH_TGP) + return PCR_PSF3_TO_SHDW_PMC_REG_BASE_TGP; + else if (pch_series == PCH_JSP) + return PCR_PSF3_TO_SHDW_PMC_REG_BASE_JSP; + else + return 0; +} + static void soc_config_pwrmbase(void) { uint32_t reg32; @@ -99,22 +114,23 @@ static void soc_config_acpibase(void) { uint32_t pmc_reg_value; + uint32_t pmc_base_reg;
- pmc_reg_value = pcr_read32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_TO_SHDW_BAR4); + pmc_base_reg = get_pmc_reg_base(); + if (!pmc_base_reg) + die_with_post_code(POST_HW_INIT_FAILURE, "Invalid PMC base address\n"); + + pmc_reg_value = pcr_read32(PID_PSF3, pmc_base_reg + PCR_PSFX_TO_SHDW_BAR4);
if (pmc_reg_value != 0xffffffff) { /* Disable Io Space before changing the address */ - pcr_rmw32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_T0_SHDW_PCIEN, + pcr_rmw32(PID_PSF3, pmc_base_reg + PCR_PSFX_T0_SHDW_PCIEN, ~PCR_PSFX_TO_SHDW_PCIEN_IOEN, 0); /* Program ABASE in PSF3 PMC space BAR4*/ - pcr_write32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_TO_SHDW_BAR4, + pcr_write32(PID_PSF3, pmc_base_reg + PCR_PSFX_TO_SHDW_BAR4, ACPI_BASE_ADDRESS); /* Enable IO Space */ - pcr_rmw32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_T0_SHDW_PCIEN, + pcr_rmw32(PID_PSF3, pmc_base_reg + PCR_PSFX_T0_SHDW_PCIEN, ~0, PCR_PSFX_TO_SHDW_PCIEN_IOEN); } } diff --git a/src/soc/intel/tigerlake/espi.c b/src/soc/intel/tigerlake/espi.c index 932f760..d07a582 100644 --- a/src/soc/intel/tigerlake/espi.c +++ b/src/soc/intel/tigerlake/espi.c @@ -81,10 +81,10 @@ */ lpc_did_hi_byte = pci_read_config8(PCH_DEV_ESPI, PCI_DEVICE_ID + 1);
- if (lpc_did_hi_byte == 0x9D) - return PCH_LP; - else if (lpc_did_hi_byte == 0xA3) - return PCH_H; + if (lpc_did_hi_byte == 0xA0) + return PCH_TGP; + else if (lpc_did_hi_byte == 0x38) + return PCH_JSP; else return PCH_UNKNOWN_SERIES; } diff --git a/src/soc/intel/tigerlake/include/soc/pch.h b/src/soc/intel/tigerlake/include/soc/pch.h index 57ddeaf..ae8e310 100644 --- a/src/soc/intel/tigerlake/include/soc/pch.h +++ b/src/soc/intel/tigerlake/include/soc/pch.h @@ -18,8 +18,8 @@
#include <stdint.h>
-#define PCH_H 1 -#define PCH_LP 2 +#define PCH_TGP 1 +#define PCH_JSP 2 #define PCH_UNKNOWN_SERIES 0xFF
#define PCIE_CLK_NOTUSED 0xFF
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37670 )
Change subject: soc/intel/tigerlake: Pick correct pmc base reg from pch type ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37670 )
Change subject: soc/intel/tigerlake: Pick correct pmc base reg from pch type ......................................................................
soc/intel/tigerlake: Pick correct pmc base reg from pch type
Update PMC shadow register base address for Jasperlake Correct PCH detection logic based on PCH ids and return correct base address based on PCH detected since our code supports both tgl and jsl.
Change-Id: Iea3311b3dc8dc3ee5ea54db1148f386c2a5dd563 Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37670 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/tigerlake/bootblock/pch.c M src/soc/intel/tigerlake/espi.c M src/soc/intel/tigerlake/include/soc/pch.h 3 files changed, 31 insertions(+), 15 deletions(-)
Approvals: build bot (Jenkins): Verified Aamir Bohra: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/bootblock/pch.c b/src/soc/intel/tigerlake/bootblock/pch.c index 1ef4928..8599423 100644 --- a/src/soc/intel/tigerlake/bootblock/pch.c +++ b/src/soc/intel/tigerlake/bootblock/pch.c @@ -37,7 +37,8 @@ #include <soc/pcr_ids.h> #include <soc/pm.h>
-#define PCR_PSF3_TO_SHDW_PMC_REG_BASE 0x0600 +#define PCR_PSF3_TO_SHDW_PMC_REG_BASE_TGP 0x0600 +#define PCR_PSF3_TO_SHDW_PMC_REG_BASE_JSP 0x0980 #define PCR_PSFX_TO_SHDW_BAR0 0 #define PCR_PSFX_TO_SHDW_BAR1 0x4 #define PCR_PSFX_TO_SHDW_BAR2 0x8 @@ -57,6 +58,20 @@ #define PCR_DMI_LPCIOD 0x2770 #define PCR_DMI_LPCIOE 0x2774
+static uint32_t get_pmc_reg_base(void) +{ + uint8_t pch_series; + + pch_series = get_pch_series(); + + if (pch_series == PCH_TGP) + return PCR_PSF3_TO_SHDW_PMC_REG_BASE_TGP; + else if (pch_series == PCH_JSP) + return PCR_PSF3_TO_SHDW_PMC_REG_BASE_JSP; + else + return 0; +} + static void soc_config_pwrmbase(void) { uint32_t reg32; @@ -99,22 +114,23 @@ static void soc_config_acpibase(void) { uint32_t pmc_reg_value; + uint32_t pmc_base_reg;
- pmc_reg_value = pcr_read32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_TO_SHDW_BAR4); + pmc_base_reg = get_pmc_reg_base(); + if (!pmc_base_reg) + die_with_post_code(POST_HW_INIT_FAILURE, "Invalid PMC base address\n"); + + pmc_reg_value = pcr_read32(PID_PSF3, pmc_base_reg + PCR_PSFX_TO_SHDW_BAR4);
if (pmc_reg_value != 0xffffffff) { /* Disable Io Space before changing the address */ - pcr_rmw32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_T0_SHDW_PCIEN, + pcr_rmw32(PID_PSF3, pmc_base_reg + PCR_PSFX_T0_SHDW_PCIEN, ~PCR_PSFX_TO_SHDW_PCIEN_IOEN, 0); /* Program ABASE in PSF3 PMC space BAR4*/ - pcr_write32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_TO_SHDW_BAR4, + pcr_write32(PID_PSF3, pmc_base_reg + PCR_PSFX_TO_SHDW_BAR4, ACPI_BASE_ADDRESS); /* Enable IO Space */ - pcr_rmw32(PID_PSF3, PCR_PSF3_TO_SHDW_PMC_REG_BASE + - PCR_PSFX_T0_SHDW_PCIEN, + pcr_rmw32(PID_PSF3, pmc_base_reg + PCR_PSFX_T0_SHDW_PCIEN, ~0, PCR_PSFX_TO_SHDW_PCIEN_IOEN); } } diff --git a/src/soc/intel/tigerlake/espi.c b/src/soc/intel/tigerlake/espi.c index 932f760..d07a582 100644 --- a/src/soc/intel/tigerlake/espi.c +++ b/src/soc/intel/tigerlake/espi.c @@ -81,10 +81,10 @@ */ lpc_did_hi_byte = pci_read_config8(PCH_DEV_ESPI, PCI_DEVICE_ID + 1);
- if (lpc_did_hi_byte == 0x9D) - return PCH_LP; - else if (lpc_did_hi_byte == 0xA3) - return PCH_H; + if (lpc_did_hi_byte == 0xA0) + return PCH_TGP; + else if (lpc_did_hi_byte == 0x38) + return PCH_JSP; else return PCH_UNKNOWN_SERIES; } diff --git a/src/soc/intel/tigerlake/include/soc/pch.h b/src/soc/intel/tigerlake/include/soc/pch.h index 57ddeaf..ae8e310 100644 --- a/src/soc/intel/tigerlake/include/soc/pch.h +++ b/src/soc/intel/tigerlake/include/soc/pch.h @@ -18,8 +18,8 @@
#include <stdint.h>
-#define PCH_H 1 -#define PCH_LP 2 +#define PCH_TGP 1 +#define PCH_JSP 2 #define PCH_UNKNOWN_SERIES 0xFF
#define PCIE_CLK_NOTUSED 0xFF