Attention is currently required from: Tim Chu.
Hello Tim Chu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/70009
to review the following change.
Change subject: soc/intel/xeon_sp: Move codes to support new PCH ......................................................................
soc/intel/xeon_sp: Move codes to support new PCH
New PCH has different definitions for registers which make the existing codes not suitable. Here move some soc specific code to each cpu's folder so that each soc can have their own definition for registers.
* Move soc specific codes from pch.c to soc_pch.c under each cpu's folder * Move lewisburg_pch_gpio_defs.h to each cpu's folder and rename to gpio_soc_defs.h * Move pcr_ids.h to each cpu's folder
Signed-off-by: Tim Chu Tim.Chu@quantatw.com Change-Id: I06555ed6612c632ea2ce1938d81781cd9348017a --- M src/soc/intel/xeon_sp/cpx/Makefile.inc M src/soc/intel/xeon_sp/cpx/chip.c R src/soc/intel/xeon_sp/cpx/include/soc/gpio_soc_defs.h R src/soc/intel/xeon_sp/cpx/include/soc/pcr_ids.h A src/soc/intel/xeon_sp/cpx/include/soc/soc_pch.h A src/soc/intel/xeon_sp/cpx/soc_pch.c M src/soc/intel/xeon_sp/include/soc/gpio.h M src/soc/intel/xeon_sp/include/soc/pch.h M src/soc/intel/xeon_sp/pch.c M src/soc/intel/xeon_sp/skx/Makefile.inc M src/soc/intel/xeon_sp/skx/chip.c C src/soc/intel/xeon_sp/skx/include/soc/gpio_soc_defs.h C src/soc/intel/xeon_sp/skx/include/soc/pcr_ids.h A src/soc/intel/xeon_sp/skx/include/soc/soc_pch.h A src/soc/intel/xeon_sp/skx/soc_pch.c 15 files changed, 161 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/70009/1
diff --git a/src/soc/intel/xeon_sp/cpx/Makefile.inc b/src/soc/intel/xeon_sp/cpx/Makefile.inc index d2a1583..25893de 100644 --- a/src/soc/intel/xeon_sp/cpx/Makefile.inc +++ b/src/soc/intel/xeon_sp/cpx/Makefile.inc @@ -5,11 +5,13 @@ subdirs-y += ../../../../cpu/intel/turbo subdirs-y += ../../../../cpu/intel/microcode
+bootblock-y += soc_pch.c + romstage-y += romstage.c ddr.c romstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c romstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c
-ramstage-y += chip.c cpu.c soc_util.c ramstage.c soc_acpi.c +ramstage-y += chip.c cpu.c soc_util.c ramstage.c soc_acpi.c soc_pch.c ramstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c ramstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c
diff --git a/src/soc/intel/xeon_sp/cpx/chip.c b/src/soc/intel/xeon_sp/cpx/chip.c index 4b15435..15bd5de 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.c +++ b/src/soc/intel/xeon_sp/cpx/chip.c @@ -14,6 +14,7 @@ #include <soc/chip_common.h> #include <soc/cpu.h> #include <soc/pch.h> +#include <soc/soc_pch.h> #include <soc/ramstage.h> #include <soc/p2sb.h> #include <soc/soc_util.h> diff --git a/src/soc/intel/xeon_sp/include/soc/lewisburg_pch_gpio_defs.h b/src/soc/intel/xeon_sp/cpx/include/soc/gpio_soc_defs.h similarity index 100% rename from src/soc/intel/xeon_sp/include/soc/lewisburg_pch_gpio_defs.h rename to src/soc/intel/xeon_sp/cpx/include/soc/gpio_soc_defs.h diff --git a/src/soc/intel/xeon_sp/include/soc/pcr_ids.h b/src/soc/intel/xeon_sp/cpx/include/soc/pcr_ids.h similarity index 100% rename from src/soc/intel/xeon_sp/include/soc/pcr_ids.h rename to src/soc/intel/xeon_sp/cpx/include/soc/pcr_ids.h diff --git a/src/soc/intel/xeon_sp/cpx/include/soc/soc_pch.h b/src/soc/intel/xeon_sp/cpx/include/soc/soc_pch.h new file mode 100644 index 0000000..2d87be3 --- /dev/null +++ b/src/soc/intel/xeon_sp/cpx/include/soc/soc_pch.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _SOC_SOC_PCH_H_ +#define _SOC_SOC_PCH_H_ + +void pch_lock_dmictl(void); + +#endif /* _SOC_SOC_PCH_H_ */ diff --git a/src/soc/intel/xeon_sp/cpx/soc_pch.c b/src/soc/intel/xeon_sp/cpx/soc_pch.c new file mode 100644 index 0000000..83ed20a --- /dev/null +++ b/src/soc/intel/xeon_sp/cpx/soc_pch.c @@ -0,0 +1,58 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <device/pci_ops.h> +#include <soc/pci_devs.h> +#include <soc/pcr_ids.h> +#include <intelblocks/pcr.h> +#include <intelblocks/rtc.h> +#include <intelblocks/p2sb.h> +#include <soc/bootblock.h> +#include <soc/soc_pch.h> +#include <soc/pmc.h> +#include <console/console.h> + +#define PCR_DMI_ACPIBA 0x27B4 +#define PCR_DMI_ACPIBDID 0x27B8 +#define PCR_DMI_DMICTL 0x2234 +#define PCR_DMI_DMICTL_SRLOCK (1 << 31) +#define PCR_DMI_PMBASEA 0x27AC +#define PCR_DMI_PMBASEC 0x27B0 + +static void soc_config_acpibase(void) +{ + uint32_t reg32; + + /* Disable ABASE in PMC Device first before changing Base Address */ + reg32 = pci_read_config32(PCH_DEV_PMC, ACTL); + pci_write_config32(PCH_DEV_PMC, ACTL, reg32 & ~ACPI_EN); + + /* Program ACPI Base */ + pci_write_config32(PCH_DEV_PMC, ABASE, ACPI_BASE_ADDRESS); + + /* Enable ACPI in PMC */ + pci_write_config32(PCH_DEV_PMC, ACTL, reg32 | ACPI_EN); + + uint32_t data = pci_read_config32(PCH_DEV_PMC, ABASE); + printk(BIOS_INFO, "%s : pmbase = %x\n", __func__, (int)data); + /* + * Program "ACPI Base Address" PCR[DMI] + 27B4h[23:18, 15:2, 0] + * to [0x3F, PMC PCI Offset 40h bit[15:2], 1] + */ + reg32 = (0x3f << 18) | ACPI_BASE_ADDRESS | 1; + pcr_write32(PID_DMI, PCR_DMI_ACPIBA, reg32); + pcr_write32(PID_DMI, PCR_DMI_ACPIBDID, 0x23a8); +} + +void bootblock_pch_init(void) +{ + /* + * Enabling ABASE for accessing PM1_STS, PM1_EN, PM1_CNT + */ + soc_config_acpibase(); +} + +void pch_lock_dmictl(void) +{ + uint32_t reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); + pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); +} diff --git a/src/soc/intel/xeon_sp/include/soc/gpio.h b/src/soc/intel/xeon_sp/include/soc/gpio.h index 04eb9ae..e7ffa6f 100644 --- a/src/soc/intel/xeon_sp/include/soc/gpio.h +++ b/src/soc/intel/xeon_sp/include/soc/gpio.h @@ -3,7 +3,7 @@ #ifndef _SOC_GPIO_H_ #define _SOC_GPIO_H_
-#include <soc/lewisburg_pch_gpio_defs.h> +#include <soc/gpio_soc_defs.h> #include <intelblocks/gpio.h>
/* diff --git a/src/soc/intel/xeon_sp/include/soc/pch.h b/src/soc/intel/xeon_sp/include/soc/pch.h index 156a22a..0be14ae 100644 --- a/src/soc/intel/xeon_sp/include/soc/pch.h +++ b/src/soc/intel/xeon_sp/include/soc/pch.h @@ -10,6 +10,5 @@ #endif
void override_hpet_ioapic_bdf(void); -void pch_lock_dmictl(void);
#endif /* _SOC_PCH_H_ */ diff --git a/src/soc/intel/xeon_sp/pch.c b/src/soc/intel/xeon_sp/pch.c index a8f47d3..23af6d9 100644 --- a/src/soc/intel/xeon_sp/pch.c +++ b/src/soc/intel/xeon_sp/pch.c @@ -11,46 +11,6 @@ #include <soc/pmc.h> #include <console/console.h>
-#define PCR_DMI_ACPIBA 0x27B4 -#define PCR_DMI_ACPIBDID 0x27B8 -#define PCR_DMI_DMICTL 0x2234 -#define PCR_DMI_DMICTL_SRLOCK (1 << 31) -#define PCR_DMI_PMBASEA 0x27AC -#define PCR_DMI_PMBASEC 0x27B0 - -static void soc_config_acpibase(void) -{ - uint32_t reg32; - - /* Disable ABASE in PMC Device first before changing Base Address */ - reg32 = pci_read_config32(PCH_DEV_PMC, ACTL); - pci_write_config32(PCH_DEV_PMC, ACTL, reg32 & ~ACPI_EN); - - /* Program ACPI Base */ - pci_write_config32(PCH_DEV_PMC, ABASE, ACPI_BASE_ADDRESS); - - /* Enable ACPI in PMC */ - pci_write_config32(PCH_DEV_PMC, ACTL, reg32 | ACPI_EN); - - uint32_t data = pci_read_config32(PCH_DEV_PMC, ABASE); - printk(BIOS_INFO, "%s : pmbase = %x\n", __func__, (int)data); - /* - * Program "ACPI Base Address" PCR[DMI] + 27B4h[23:18, 15:2, 0] - * to [0x3F, PMC PCI Offset 40h bit[15:2], 1] - */ - reg32 = (0x3f << 18) | ACPI_BASE_ADDRESS | 1; - pcr_write32(PID_DMI, PCR_DMI_ACPIBA, reg32); - pcr_write32(PID_DMI, PCR_DMI_ACPIBDID, 0x23a8); -} - -void bootblock_pch_init(void) -{ - /* - * Enabling ABASE for accessing PM1_STS, PM1_EN, PM1_CNT - */ - soc_config_acpibase(); -} - void override_hpet_ioapic_bdf(void) { union p2sb_bdf ioapic_bdf = { @@ -67,9 +27,3 @@ p2sb_set_ioapic_bdf(ioapic_bdf); p2sb_set_hpet_bdf(hpet_bdf); } - -void pch_lock_dmictl(void) -{ - uint32_t reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); - pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); -} diff --git a/src/soc/intel/xeon_sp/skx/Makefile.inc b/src/soc/intel/xeon_sp/skx/Makefile.inc index f7599a3..80c594b 100644 --- a/src/soc/intel/xeon_sp/skx/Makefile.inc +++ b/src/soc/intel/xeon_sp/skx/Makefile.inc @@ -5,6 +5,8 @@ subdirs-y += ../../../../cpu/intel/microcode subdirs-y += ../../../../cpu/intel/turbo
+bootblock-y += soc_pch.c + postcar-y += soc_util.c
romstage-y += soc_util.c @@ -18,6 +20,7 @@ ramstage-y += chip.c ramstage-y += soc_util.c ramstage-y += cpu.c +ramstage-y += soc_pch.c ramstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c ramstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c ramstage-y += hob_display.c diff --git a/src/soc/intel/xeon_sp/skx/chip.c b/src/soc/intel/xeon_sp/skx/chip.c index 7850787..efdeef8 100644 --- a/src/soc/intel/xeon_sp/skx/chip.c +++ b/src/soc/intel/xeon_sp/skx/chip.c @@ -8,6 +8,7 @@ #include <soc/acpi.h> #include <soc/chip_common.h> #include <soc/pch.h> +#include <soc/soc_pch.h> #include <soc/ramstage.h> #include <soc/soc_util.h> #include <soc/util.h> diff --git a/src/soc/intel/xeon_sp/include/soc/lewisburg_pch_gpio_defs.h b/src/soc/intel/xeon_sp/skx/include/soc/gpio_soc_defs.h similarity index 100% copy from src/soc/intel/xeon_sp/include/soc/lewisburg_pch_gpio_defs.h copy to src/soc/intel/xeon_sp/skx/include/soc/gpio_soc_defs.h diff --git a/src/soc/intel/xeon_sp/include/soc/pcr_ids.h b/src/soc/intel/xeon_sp/skx/include/soc/pcr_ids.h similarity index 100% copy from src/soc/intel/xeon_sp/include/soc/pcr_ids.h copy to src/soc/intel/xeon_sp/skx/include/soc/pcr_ids.h diff --git a/src/soc/intel/xeon_sp/skx/include/soc/soc_pch.h b/src/soc/intel/xeon_sp/skx/include/soc/soc_pch.h new file mode 100644 index 0000000..2d87be3 --- /dev/null +++ b/src/soc/intel/xeon_sp/skx/include/soc/soc_pch.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _SOC_SOC_PCH_H_ +#define _SOC_SOC_PCH_H_ + +void pch_lock_dmictl(void); + +#endif /* _SOC_SOC_PCH_H_ */ diff --git a/src/soc/intel/xeon_sp/skx/soc_pch.c b/src/soc/intel/xeon_sp/skx/soc_pch.c new file mode 100644 index 0000000..83ed20a --- /dev/null +++ b/src/soc/intel/xeon_sp/skx/soc_pch.c @@ -0,0 +1,58 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <device/pci_ops.h> +#include <soc/pci_devs.h> +#include <soc/pcr_ids.h> +#include <intelblocks/pcr.h> +#include <intelblocks/rtc.h> +#include <intelblocks/p2sb.h> +#include <soc/bootblock.h> +#include <soc/soc_pch.h> +#include <soc/pmc.h> +#include <console/console.h> + +#define PCR_DMI_ACPIBA 0x27B4 +#define PCR_DMI_ACPIBDID 0x27B8 +#define PCR_DMI_DMICTL 0x2234 +#define PCR_DMI_DMICTL_SRLOCK (1 << 31) +#define PCR_DMI_PMBASEA 0x27AC +#define PCR_DMI_PMBASEC 0x27B0 + +static void soc_config_acpibase(void) +{ + uint32_t reg32; + + /* Disable ABASE in PMC Device first before changing Base Address */ + reg32 = pci_read_config32(PCH_DEV_PMC, ACTL); + pci_write_config32(PCH_DEV_PMC, ACTL, reg32 & ~ACPI_EN); + + /* Program ACPI Base */ + pci_write_config32(PCH_DEV_PMC, ABASE, ACPI_BASE_ADDRESS); + + /* Enable ACPI in PMC */ + pci_write_config32(PCH_DEV_PMC, ACTL, reg32 | ACPI_EN); + + uint32_t data = pci_read_config32(PCH_DEV_PMC, ABASE); + printk(BIOS_INFO, "%s : pmbase = %x\n", __func__, (int)data); + /* + * Program "ACPI Base Address" PCR[DMI] + 27B4h[23:18, 15:2, 0] + * to [0x3F, PMC PCI Offset 40h bit[15:2], 1] + */ + reg32 = (0x3f << 18) | ACPI_BASE_ADDRESS | 1; + pcr_write32(PID_DMI, PCR_DMI_ACPIBA, reg32); + pcr_write32(PID_DMI, PCR_DMI_ACPIBDID, 0x23a8); +} + +void bootblock_pch_init(void) +{ + /* + * Enabling ABASE for accessing PM1_STS, PM1_EN, PM1_CNT + */ + soc_config_acpibase(); +} + +void pch_lock_dmictl(void) +{ + uint32_t reg32 = pcr_read32(PID_DMI, PCR_DMI_DMICTL); + pcr_write32(PID_DMI, PCR_DMI_DMICTL, reg32 | PCR_DMI_DMICTL_SRLOCK); +}
-
Shuming Chu (Shuming) (Code Review)