Usha P has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37308 )
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
soc/intel/cannonlake: Refactor pch_early_init() code
This patch keeps required pch_early_init() function like ABASE programming, GPE and RTC init into bootblock and moves remaining functions like TCO configuration and SMBUS init into romstage/pch.c in order to maintain only required chipset programming for bootblock and verstage.
Renamed the pch_init function to bootblock_pch_init and romstage_pch_init according to the stage it is defined in.
TEST=Able to build and boot hatch successfully.
Change-Id: Idf7b04edc3fce147f7857561ce7d5b0cd05f43fe Signed-off-by: Usha P usha.p@intel.com --- M src/soc/intel/cannonlake/bootblock/bootblock.c M src/soc/intel/cannonlake/bootblock/pch.c M src/soc/intel/cannonlake/include/soc/bootblock.h M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/Makefile.inc A src/soc/intel/cannonlake/romstage/pch.c M src/soc/intel/cannonlake/romstage/romstage.c 7 files changed, 36 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/37308/1
diff --git a/src/soc/intel/cannonlake/bootblock/bootblock.c b/src/soc/intel/cannonlake/bootblock/bootblock.c index 9f85397..08c1242 100644 --- a/src/soc/intel/cannonlake/bootblock/bootblock.c +++ b/src/soc/intel/cannonlake/bootblock/bootblock.c @@ -74,5 +74,5 @@ */ gpi_clear_int_cfg(); report_platform_info(); - pch_early_init(); + bootblock_pch_init(); } diff --git a/src/soc/intel/cannonlake/bootblock/pch.c b/src/soc/intel/cannonlake/bootblock/pch.c index 39433a2..a27b507 100644 --- a/src/soc/intel/cannonlake/bootblock/pch.c +++ b/src/soc/intel/cannonlake/bootblock/pch.c @@ -2,7 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2014 Google Inc. - * Copyright (C) 2017-2018 Intel Corporation. + * Copyright (C) 2017-2019 Intel Corporation. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -25,8 +25,6 @@ #include <intelblocks/pcr.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> -#include <intelblocks/smbus.h> -#include <intelblocks/tco.h> #include <soc/bootblock.h> #include <soc/iomap.h> #include <soc/lpc.h> @@ -35,7 +33,6 @@ #include <soc/pci_devs.h> #include <soc/pcr_ids.h> #include <soc/pm.h> -#include <soc/smbus.h>
#define PCR_PSF3_TO_SHDW_PMC_REG_BASE_CNP_LP 0x1400 #define PCR_PSF3_TO_SHDW_PMC_REG_BASE_CNP_H 0x0980 @@ -180,7 +177,7 @@ pch_enable_lpc(); }
-void pch_early_init(void) +void bootblock_pch_init(void) { /* * Enabling ABASE for accessing PM1_STS, PM1_EN, PM1_CNT, @@ -188,12 +185,6 @@ */ soc_config_acpibase();
- /* Programming TCO_BASE_ADDRESS and TCO Timer Halt */ - tco_configure(); - - /* Program SMBUS_BASE_ADDRESS and Enable it */ - smbus_common_init(); - /* Set up GPE configuration */ pmc_gpe_init();
diff --git a/src/soc/intel/cannonlake/include/soc/bootblock.h b/src/soc/intel/cannonlake/include/soc/bootblock.h index a5c3c32..efc837e 100644 --- a/src/soc/intel/cannonlake/include/soc/bootblock.h +++ b/src/soc/intel/cannonlake/include/soc/bootblock.h @@ -23,7 +23,7 @@ void bootblock_pch_early_init(void);
/* Bootblock post console init programming */ -void pch_early_init(void); +void bootblock_pch_init(void); void pch_early_iorange_init(void); void report_platform_info(void);
diff --git a/src/soc/intel/cannonlake/include/soc/romstage.h b/src/soc/intel/cannonlake/include/soc/romstage.h index 643105a..ab20ee7 100644 --- a/src/soc/intel/cannonlake/include/soc/romstage.h +++ b/src/soc/intel/cannonlake/include/soc/romstage.h @@ -24,6 +24,7 @@ /* Provide a callback to allow mainboard to override the DRAM part number. */ void mainboard_get_dram_part_num(const char **part_num, size_t *len); void systemagent_early_init(void); +void romstage_pch_init(void);
/* Board type */ enum board_type { diff --git a/src/soc/intel/cannonlake/romstage/Makefile.inc b/src/soc/intel/cannonlake/romstage/Makefile.inc index 33d9629..ff3d73d 100644 --- a/src/soc/intel/cannonlake/romstage/Makefile.inc +++ b/src/soc/intel/cannonlake/romstage/Makefile.inc @@ -17,3 +17,4 @@ romstage-y += romstage.c romstage-y += fsp_params.c romstage-y += systemagent.c +romstage-y += pch.c diff --git a/src/soc/intel/cannonlake/romstage/pch.c b/src/soc/intel/cannonlake/romstage/pch.c new file mode 100644 index 0000000..388ad77 --- /dev/null +++ b/src/soc/intel/cannonlake/romstage/pch.c @@ -0,0 +1,28 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <intelblocks/smbus.h> +#include <intelblocks/tco.h> +#include <soc/romstage.h> + +void romstage_pch_init(void) +{ + /* Programming TCO_BASE_ADDRESS and TCO Timer Halt */ + tco_configure(); + + /* Program SMBUS_BASE_ADDRESS and Enable it */ + smbus_common_init(); +} + diff --git a/src/soc/intel/cannonlake/romstage/romstage.c b/src/soc/intel/cannonlake/romstage/romstage.c index f782f63..2505683 100644 --- a/src/soc/intel/cannonlake/romstage/romstage.c +++ b/src/soc/intel/cannonlake/romstage/romstage.c @@ -132,6 +132,8 @@
/* Program MCHBAR, DMIBAR, GDXBAR and EDRAMBAR */ systemagent_early_init(); + /* Program PCH init */ + romstage_pch_init(); /* initialize Heci interface */ heci_init(HECI1_BASE_ADDRESS);
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37308
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
soc/intel/cannonlake: Refactor pch_early_init() code
This patch keeps required pch_early_init() function like ABASE programming, GPE and RTC init into bootblock and moves remaining functions like TCO configuration and SMBUS init into romstage/pch.c in order to maintain only required chipset programming for bootblock and verstage.
Renamed the pch_init function to bootblock_pch_init and romstage_pch_init according to the stage it is defined in.
TEST=Able to build and boot hatch successfully.
Change-Id: Idf7b04edc3fce147f7857561ce7d5b0cd05f43fe Signed-off-by: Usha P usha.p@intel.com --- M src/soc/intel/cannonlake/bootblock/bootblock.c M src/soc/intel/cannonlake/bootblock/pch.c M src/soc/intel/cannonlake/include/soc/bootblock.h M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/Makefile.inc A src/soc/intel/cannonlake/romstage/pch.c M src/soc/intel/cannonlake/romstage/romstage.c 7 files changed, 35 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/37308/2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37308 )
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37308 )
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG@11 PS2, Line 11: TCO Fits on the line above.
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG@11 PS2, Line 11: SMBUS SMBus
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG@14 PS2, Line 14: Renamed Rename
https://review.coreboot.org/c/coreboot/+/37308/2/src/soc/intel/cannonlake/ro... File src/soc/intel/cannonlake/romstage/pch.c:
https://review.coreboot.org/c/coreboot/+/37308/2/src/soc/intel/cannonlake/ro... PS2, Line 22: Programming Program
https://review.coreboot.org/c/coreboot/+/37308/2/src/soc/intel/cannonlake/ro... PS2, Line 25: Enable enable
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37308
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
soc/intel/cannonlake: Refactor pch_early_init() code
This patch keeps required pch_early_init() function like ABASE programming, GPE and RTC init into bootblock and moves remaining functions like TCO configuration and SMBus init into romstage/pch.c in order to maintain only required chipset programming for bootblock and verstage.
Rename the pch_init function to bootblock_pch_init and romstage_pch_init according to the stage it is defined in.
TEST=Able to build and boot hatch successfully.
Change-Id: Idf7b04edc3fce147f7857561ce7d5b0cd05f43fe Signed-off-by: Usha P usha.p@intel.com --- M src/soc/intel/cannonlake/bootblock/bootblock.c M src/soc/intel/cannonlake/bootblock/pch.c M src/soc/intel/cannonlake/include/soc/bootblock.h M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/Makefile.inc A src/soc/intel/cannonlake/romstage/pch.c M src/soc/intel/cannonlake/romstage/romstage.c 7 files changed, 35 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/37308/3
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37308 )
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG@11 PS2, Line 11: TCO
Fits on the line above.
Done
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG@11 PS2, Line 11: SMBUS
SMBus
Done
https://review.coreboot.org/c/coreboot/+/37308/2//COMMIT_MSG@14 PS2, Line 14: Renamed
Rename
Done
https://review.coreboot.org/c/coreboot/+/37308/2/src/soc/intel/cannonlake/ro... File src/soc/intel/cannonlake/romstage/pch.c:
https://review.coreboot.org/c/coreboot/+/37308/2/src/soc/intel/cannonlake/ro... PS2, Line 22: Programming
Program
Done
https://review.coreboot.org/c/coreboot/+/37308/2/src/soc/intel/cannonlake/ro... PS2, Line 25: Enable
enable
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37308 )
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37308 )
Change subject: soc/intel/cannonlake: Refactor pch_early_init() code ......................................................................
soc/intel/cannonlake: Refactor pch_early_init() code
This patch keeps required pch_early_init() function like ABASE programming, GPE and RTC init into bootblock and moves remaining functions like TCO configuration and SMBus init into romstage/pch.c in order to maintain only required chipset programming for bootblock and verstage.
Rename the pch_init function to bootblock_pch_init and romstage_pch_init according to the stage it is defined in.
TEST=Able to build and boot hatch successfully.
Change-Id: Idf7b04edc3fce147f7857561ce7d5b0cd05f43fe Signed-off-by: Usha P usha.p@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37308 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/bootblock/bootblock.c M src/soc/intel/cannonlake/bootblock/pch.c M src/soc/intel/cannonlake/include/soc/bootblock.h M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/Makefile.inc A src/soc/intel/cannonlake/romstage/pch.c M src/soc/intel/cannonlake/romstage/romstage.c 7 files changed, 35 insertions(+), 13 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/bootblock/bootblock.c b/src/soc/intel/cannonlake/bootblock/bootblock.c index 6a6dd8b..4cc15fc 100644 --- a/src/soc/intel/cannonlake/bootblock/bootblock.c +++ b/src/soc/intel/cannonlake/bootblock/bootblock.c @@ -74,5 +74,5 @@ */ gpi_clear_int_cfg(); report_platform_info(); - pch_early_init(); + bootblock_pch_init(); } diff --git a/src/soc/intel/cannonlake/bootblock/pch.c b/src/soc/intel/cannonlake/bootblock/pch.c index 9ad7e86..a6e9f9d 100644 --- a/src/soc/intel/cannonlake/bootblock/pch.c +++ b/src/soc/intel/cannonlake/bootblock/pch.c @@ -2,7 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2014 Google Inc. - * Copyright (C) 2017-2018 Intel Corporation. + * Copyright (C) 2017-2019 Intel Corporation. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -25,8 +25,6 @@ #include <intelblocks/pcr.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> -#include <intelblocks/smbus.h> -#include <intelblocks/tco.h> #include <soc/bootblock.h> #include <soc/gpio.h> #include <soc/iomap.h> @@ -36,7 +34,6 @@ #include <soc/pci_devs.h> #include <soc/pcr_ids.h> #include <soc/pm.h> -#include <soc/smbus.h>
#define PCR_PSF3_TO_SHDW_PMC_REG_BASE_CNP_LP 0x1400 #define PCR_PSF3_TO_SHDW_PMC_REG_BASE_CNP_H 0x0980 @@ -181,7 +178,7 @@ pch_enable_lpc(); }
-void pch_early_init(void) +void bootblock_pch_init(void) { /* * Enabling ABASE for accessing PM1_STS, PM1_EN, PM1_CNT, @@ -189,12 +186,6 @@ */ soc_config_acpibase();
- /* Programming TCO_BASE_ADDRESS and TCO Timer Halt */ - tco_configure(); - - /* Program SMBUS_BASE_ADDRESS and Enable it */ - smbus_common_init(); - /* Set up GPE configuration */ pmc_gpe_init();
diff --git a/src/soc/intel/cannonlake/include/soc/bootblock.h b/src/soc/intel/cannonlake/include/soc/bootblock.h index a5c3c32..efc837e 100644 --- a/src/soc/intel/cannonlake/include/soc/bootblock.h +++ b/src/soc/intel/cannonlake/include/soc/bootblock.h @@ -23,7 +23,7 @@ void bootblock_pch_early_init(void);
/* Bootblock post console init programming */ -void pch_early_init(void); +void bootblock_pch_init(void); void pch_early_iorange_init(void); void report_platform_info(void);
diff --git a/src/soc/intel/cannonlake/include/soc/romstage.h b/src/soc/intel/cannonlake/include/soc/romstage.h index 643105a..ab20ee7 100644 --- a/src/soc/intel/cannonlake/include/soc/romstage.h +++ b/src/soc/intel/cannonlake/include/soc/romstage.h @@ -24,6 +24,7 @@ /* Provide a callback to allow mainboard to override the DRAM part number. */ void mainboard_get_dram_part_num(const char **part_num, size_t *len); void systemagent_early_init(void); +void romstage_pch_init(void);
/* Board type */ enum board_type { diff --git a/src/soc/intel/cannonlake/romstage/Makefile.inc b/src/soc/intel/cannonlake/romstage/Makefile.inc index 33d9629..ff3d73d 100644 --- a/src/soc/intel/cannonlake/romstage/Makefile.inc +++ b/src/soc/intel/cannonlake/romstage/Makefile.inc @@ -17,3 +17,4 @@ romstage-y += romstage.c romstage-y += fsp_params.c romstage-y += systemagent.c +romstage-y += pch.c diff --git a/src/soc/intel/cannonlake/romstage/pch.c b/src/soc/intel/cannonlake/romstage/pch.c new file mode 100644 index 0000000..8e783da --- /dev/null +++ b/src/soc/intel/cannonlake/romstage/pch.c @@ -0,0 +1,27 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <intelblocks/smbus.h> +#include <intelblocks/tco.h> +#include <soc/romstage.h> + +void romstage_pch_init(void) +{ + /* Program TCO_BASE_ADDRESS and TCO Timer Halt */ + tco_configure(); + + /* Program SMBUS_BASE_ADDRESS and enable it */ + smbus_common_init(); +} diff --git a/src/soc/intel/cannonlake/romstage/romstage.c b/src/soc/intel/cannonlake/romstage/romstage.c index f782f63..2505683 100644 --- a/src/soc/intel/cannonlake/romstage/romstage.c +++ b/src/soc/intel/cannonlake/romstage/romstage.c @@ -132,6 +132,8 @@
/* Program MCHBAR, DMIBAR, GDXBAR and EDRAMBAR */ systemagent_early_init(); + /* Program PCH init */ + romstage_pch_init(); /* initialize Heci interface */ heci_init(HECI1_BASE_ADDRESS);
-
Patrick Georgi (Code Review) -
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)
-
Usha P (Code Review)