Hello Varun Joshi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/38560
to review the following change.
Change subject: Common code changes for skylake ......................................................................
Common code changes for skylake
Signed-off-by: Varun Joshi varun.joshi@intel.corp-partner.google.com Change-Id: Ibd0dac2de74388e732da16aff3d6af6111ea075e --- M src/soc/intel/skylake/romstage/romstage.c 1 file changed, 45 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/38560/1
diff --git a/src/soc/intel/skylake/romstage/romstage.c b/src/soc/intel/skylake/romstage/romstage.c index 51428df..3295136 100644 --- a/src/soc/intel/skylake/romstage/romstage.c +++ b/src/soc/intel/skylake/romstage/romstage.c @@ -50,6 +50,22 @@ CHANNEL_PRESENT };
+static const struct romstage_ops_skylk_rs_ops = { + &romstage_cmn_soc_init, + &romstage_cmn_pch_init, + &romstage_cmn_cpu_init, + &romstage_cmn_is_s3wake, + &skylk_soc_mem_init_params, + &mainboard_memory_init_params, + &skylk_soc_post_mem_init, + &romstage_cmn_mb_post_mem_init +}; + +struct romstage_ops *soc_get_ops(void) +{ + return (struct romstage_ops *)&skylk_rs_ops; +} + /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { @@ -139,18 +155,9 @@ printk(BIOS_DEBUG, "%d DIMMs found\n", mem_info->dimm_cnt); }
-void mainboard_romstage_entry(void) +static void skylk_soc_post_mem_init(void) { - bool s3wake; - struct chipset_power_state *ps; - - /* Program MCHBAR, DMIBAR, GDXBAR and EDRAMBAR */ - systemagent_early_init(); - /* Program PCH init */ - romstage_pch_init(); - ps = pmc_get_power_state(); - s3wake = pmc_fill_power_state(ps) == ACPI_S3; - fsp_memory_init(s3wake); + struct romstage_ops *rs_ops = soc_get_ops(); pmc_set_disb(); if (!s3wake) save_dimm_info(); @@ -223,37 +230,6 @@ } }
-static void soc_memory_init_params(FSP_M_CONFIG *m_cfg, - const struct soc_intel_skylake_config *config) -{ - int i; - uint32_t mask = 0; - - m_cfg->MmioSize = 0x800; /* 2GB in MB */ - m_cfg->TsegSize = CONFIG_SMM_TSEG_SIZE; - m_cfg->IedSize = CONFIG_IED_REGION_SIZE; - m_cfg->ProbelessTrace = config->ProbelessTrace; - m_cfg->SaGv = config->SaGv; - m_cfg->UserBd = BOARD_TYPE_ULT_ULX; - m_cfg->RMT = config->Rmt; - m_cfg->CmdTriStateDis = config->CmdTriStateDis; - m_cfg->DdrFreqLimit = config->DdrFreqLimit; - m_cfg->VmxEnable = CONFIG(ENABLE_VMX); - m_cfg->PrmrrSize = get_prmrr_size(); - for (i = 0; i < ARRAY_SIZE(config->PcieRpEnable); i++) { - if (config->PcieRpEnable[i]) - mask |= (1<<i); - } - m_cfg->PcieRpEnableMask = mask; - - cpu_flex_override(m_cfg); - - /* HPET BDF already handled in coreboot code, so tell FSP to ignore UPDs */ - m_cfg->PchHpetBdfValid = 0; - - m_cfg->HyperThreading = CONFIG(FSP_HYPERTHREADING); -} - static void soc_primary_gfx_config_params(FSP_M_CONFIG *m_cfg, const struct soc_intel_skylake_config *config) { @@ -282,15 +258,40 @@ m_cfg->PrimaryDisplay = config->PrimaryDisplay; }
-void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) +static void skylk_soc_mem_init_params(FSPM_UPD *mupd) { const struct soc_intel_skylake_config *config; + int i; + uint32_t mask = 0; FSP_M_CONFIG *m_cfg = &mupd->FspmConfig; FSP_M_TEST_CONFIG *m_t_cfg = &mupd->FspmTestConfig;
config = config_of_soc();
- soc_memory_init_params(m_cfg, config); + m_cfg->MmioSize = 0x800; /* 2GB in MB */ + m_cfg->TsegSize = CONFIG_SMM_TSEG_SIZE; + m_cfg->IedSize = CONFIG_IED_REGION_SIZE; + m_cfg->ProbelessTrace = config->ProbelessTrace; + m_cfg->SaGv = config->SaGv; + m_cfg->UserBd = BOARD_TYPE_ULT_ULX; + m_cfg->RMT = config->Rmt; + m_cfg->CmdTriStateDis = config->CmdTriStateDis; + m_cfg->DdrFreqLimit = config->DdrFreqLimit; + m_cfg->VmxEnable = CONFIG(ENABLE_VMX); + m_cfg->PrmrrSize = get_prmrr_size(); + for (i = 0; i < ARRAY_SIZE(config->PcieRpEnable); i++) { + if (config->PcieRpEnable[i]) + mask |= (1<<i); + } + m_cfg->PcieRpEnableMask = mask; + + cpu_flex_override(m_cfg); + + /* HPET BDF already handled in coreboot code, so tell FSP to ignore UPDs */ + m_cfg->PchHpetBdfValid = 0; + + m_cfg->HyperThreading = CONFIG(FSP_HYPERTHREADING); + soc_peg_init_params(m_cfg, m_t_cfg, config);
/* Skip creating Management Engine MBP HOB */ @@ -315,8 +316,6 @@ /* Set primary graphic device */ soc_primary_gfx_config_params(m_cfg, config); m_t_cfg->SkipExtGfxScan = config->SkipExtGfxScan; - - mainboard_memory_init_params(mupd); }
void soc_update_memory_params_for_mma(FSP_M_CONFIG *memory_cfg,
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38560 )
Change subject: Common code changes for skylake ......................................................................
Patch Set 1:
(40 comments)
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... File src/soc/intel/skylake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 265: uint32_t mask = 0; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 265: uint32_t mask = 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 271: m_cfg->MmioSize = 0x800; /* 2GB in MB */ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 271: m_cfg->MmioSize = 0x800; /* 2GB in MB */ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 272: m_cfg->TsegSize = CONFIG_SMM_TSEG_SIZE; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 272: m_cfg->TsegSize = CONFIG_SMM_TSEG_SIZE; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 273: m_cfg->IedSize = CONFIG_IED_REGION_SIZE; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 273: m_cfg->IedSize = CONFIG_IED_REGION_SIZE; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 274: m_cfg->ProbelessTrace = config->ProbelessTrace; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 274: m_cfg->ProbelessTrace = config->ProbelessTrace; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 275: m_cfg->SaGv = config->SaGv; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 275: m_cfg->SaGv = config->SaGv; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 276: m_cfg->UserBd = BOARD_TYPE_ULT_ULX; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 276: m_cfg->UserBd = BOARD_TYPE_ULT_ULX; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 277: m_cfg->RMT = config->Rmt; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 277: m_cfg->RMT = config->Rmt; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 278: m_cfg->CmdTriStateDis = config->CmdTriStateDis; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 278: m_cfg->CmdTriStateDis = config->CmdTriStateDis; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 279: m_cfg->DdrFreqLimit = config->DdrFreqLimit; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 279: m_cfg->DdrFreqLimit = config->DdrFreqLimit; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 280: m_cfg->VmxEnable = CONFIG(ENABLE_VMX); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 280: m_cfg->VmxEnable = CONFIG(ENABLE_VMX); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 281: m_cfg->PrmrrSize = get_prmrr_size(); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 281: m_cfg->PrmrrSize = get_prmrr_size(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 282: for (i = 0; i < ARRAY_SIZE(config->PcieRpEnable); i++) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 282: for (i = 0; i < ARRAY_SIZE(config->PcieRpEnable); i++) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 283: if (config->PcieRpEnable[i]) code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 283: if (config->PcieRpEnable[i]) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 284: mask |= (1<<i); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 284: mask |= (1<<i); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 285: } code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 285: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 288: cpu_flex_override(m_cfg); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 288: cpu_flex_override(m_cfg); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 290: /* HPET BDF already handled in coreboot code, so tell FSP to ignore UPDs */ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 291: m_cfg->PchHpetBdfValid = 0; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 291: m_cfg->PchHpetBdfValid = 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 293: m_cfg->HyperThreading = CONFIG(FSP_HYPERTHREADING); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 293: m_cfg->HyperThreading = CONFIG(FSP_HYPERTHREADING); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38560/1/src/soc/intel/skylake/romst... PS1, Line 294: trailing whitespace
Varun Joshi has removed Patrick Rudolph from this change. ( https://review.coreboot.org/c/coreboot/+/38560 )
Change subject: Common code changes for skylake ......................................................................
Removed reviewer Patrick Rudolph.
-
build bot (Jenkins) (Code Review) -
Varun Joshi (Code Review)