Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Rizwan Qureshi, build bot (Jenkins), Lijian Zhao, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31902
to look at the new patch set (#6).
Change subject: soc/intel/cannonlake: Clear PMCON status bits
......................................................................
soc/intel/cannonlake: Clear PMCON status bits
The prev_sleep_state value was showing 5 even after warm reboot, once the
SUS_PWR_FLR bit is being set. This bit was not being cleared.
Hence clearing the PMCON status bits.
BUG=b:128482282
BRANCH=None
TEST=In cbmem logs, check for value of “prev_sleep_state” using command
cbmem –c | grep “prev_sleep_state”
For cold reboot, "prev_sleep_state 5"
For warm reboot, "prev_sleep_state 0"
Change-Id: If9863d52ed3c61b6a160df53f023b0787eaaed68
Signed-off-by: Krishna Prasad Bhat <krishna.p.bhat.d(a)intel.com>
---
M src/soc/intel/cannonlake/finalize.c
M src/soc/intel/cannonlake/include/soc/pm.h
M src/soc/intel/cannonlake/pmutil.c
3 files changed, 19 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/31902/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/31902
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If9863d52ed3c61b6a160df53f023b0787eaaed68
Gerrit-Change-Number: 31902
Gerrit-PatchSet: 6
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31902 )
Change subject: soc/intel/cannonlake: Clear PMCON status bits
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/31902/5/src/soc/intel/cannonlake/pmutil.c
File src/soc/intel/cannonlake/pmutil.c:
https://review.coreboot.org/#/c/31902/5/src/soc/intel/cannonlake/pmutil.c@1…
PS5, Line 152: /* Don't clear bits that are write-1-to-clear */
you are actually clearing some bits which are write 1 to clear.
Mention what you are clearing here.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31902
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If9863d52ed3c61b6a160df53f023b0787eaaed68
Gerrit-Change-Number: 31902
Gerrit-PatchSet: 5
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Fri, 22 Mar 2019 07:32:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Hello Patrick Rudolph, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30289
to look at the new patch set (#12).
Change subject: soc/intel/common: Remove common chip config use_fsp_mp_init
......................................................................
soc/intel/common: Remove common chip config use_fsp_mp_init
This patch ensures to make use of common MP Init Kconfig to
choose desire method to peform MP initialization for platform.
Change-Id: I4ee51276026748e8daf154f89e57095e8fe50280
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/mainboard/intel/saddlebrook/Kconfig
M src/mainboard/intel/saddlebrook/devicetree.cb
M src/soc/intel/apollolake/chip.c
M src/soc/intel/cannonlake/romstage/fsp_params.c
M src/soc/intel/common/block/chip/chip.c
M src/soc/intel/common/block/cpu/mp_init.c
M src/soc/intel/common/block/include/intelblocks/chip.h
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip_fsp20.c
9 files changed, 9 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/30289/12
--
To view, visit https://review.coreboot.org/c/coreboot/+/30289
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4ee51276026748e8daf154f89e57095e8fe50280
Gerrit-Change-Number: 30289
Gerrit-PatchSet: 12
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newpatchset
Hello Patrick Rudolph, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31841
to look at the new patch set (#3).
Change subject: Documentation/soc/intel: Add MP Initialization document
......................................................................
Documentation/soc/intel: Add MP Initialization document
This patch provides documentation for MP initialization
option available in coreboot.
Change-Id: I055808e2ddf03663e1ec5d3d423054d1caa911cb
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M Documentation/soc/intel/index.md
A Documentation/soc/intel/mp_init/mp_init.md
2 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/31841/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/31841
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I055808e2ddf03663e1ec5d3d423054d1caa911cb
Gerrit-Change-Number: 31841
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newpatchset
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30287
to look at the new patch set (#11).
Change subject: soc/intel/common: Add Kconfig option to choose desired MP Init for platform
......................................................................
soc/intel/common: Add Kconfig option to choose desired MP Init for platform
mainboard users can select correct MP Init Kconfig in order to
perform MP initialization.
1. Native coreboot MP Init.
2. FSP to do MP Init.
3. FSP to make use of coreboot MP service PPI to perform MP Initialization
Change-Id: Ifbea463fdaf97d68c21a759c37f49492d58a056b
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/soc/intel/common/block/cpu/Kconfig
1 file changed, 27 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/30287/11
--
To view, visit https://review.coreboot.org/c/coreboot/+/30287
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifbea463fdaf97d68c21a759c37f49492d58a056b
Gerrit-Change-Number: 30287
Gerrit-PatchSet: 11
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-MessageType: newpatchset
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31902 )
Change subject: soc/intel/cannonlake: Clear PMCON status bits
......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/31902/4/src/soc/intel/cannonlake/finalize.c
File src/soc/intel/cannonlake/finalize.c:
https://review.coreboot.org/#/c/31902/4/src/soc/intel/cannonlake/finalize.c…
PS4, Line 56: pmc_clear_pmcon_sts
> Shouldn't this function live in pmutil. […]
I had earlier placed it in pmutil.c. Changed it based on your suggestion here https://review.coreboot.org/c/coreboot/+/31902/3/src/soc/intel/cannonlake/p….
https://review.coreboot.org/#/c/31902/4/src/soc/intel/cannonlake/finalize.c…
PS4, Line 65: reg_val |= (SUS_PWR_FLR | GBL_RST_STS | HOST_RST_STS | PWR_FLR);
> Why is this required? Since you are reading GEN_PMCON_A, writing back the same value & ~(MS4V) shoul […]
Yes. Makes sense.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31902
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If9863d52ed3c61b6a160df53f023b0787eaaed68
Gerrit-Change-Number: 31902
Gerrit-PatchSet: 5
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Fri, 22 Mar 2019 07:10:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Rizwan Qureshi, Lijian Zhao, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31902
to look at the new patch set (#5).
Change subject: soc/intel/cannonlake: Clear PMCON status bits
......................................................................
soc/intel/cannonlake: Clear PMCON status bits
The prev_sleep_state value was showing 5 even after warm reboot, once the
SUS_PWR_FLR bit is being set. This bit was not being cleared.
Hence clearing the PMCON status bits.
BUG=b:128482282
BRANCH=None
TEST=In cbmem logs, check for value of “prev_sleep_state” using command
cbmem –c | grep “prev_sleep_state”
For cold reboot, "prev_sleep_state 5"
For warm reboot, "prev_sleep_state 0"
Change-Id: If9863d52ed3c61b6a160df53f023b0787eaaed68
Signed-off-by: Krishna Prasad Bhat <krishna.p.bhat.d(a)intel.com>
---
M src/soc/intel/cannonlake/finalize.c
M src/soc/intel/cannonlake/include/soc/pm.h
M src/soc/intel/cannonlake/pmutil.c
3 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/31902/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/31902
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If9863d52ed3c61b6a160df53f023b0787eaaed68
Gerrit-Change-Number: 31902
Gerrit-PatchSet: 5
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30289 )
Change subject: soc/intel/common: Remove common chip config use_fsp_mp_init
......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/#/c/30289/11/src/soc/intel/common/block/cpu/mp_…
File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/#/c/30289/11/src/soc/intel/common/block/cpu/mp_…
PS11, Line 141: if (CONFIG(USE_INTEL_FSP_MP_INIT))
> I don't think this needs to be a separate function, "USE_INTEL_FSP_MP_INIT" is rather self-explanato […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/30289
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4ee51276026748e8daf154f89e57095e8fe50280
Gerrit-Change-Number: 30289
Gerrit-PatchSet: 11
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Comment-Date: Fri, 22 Mar 2019 06:45:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: comment
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31841 )
Change subject: Documentation/soc/intel: Add MP Initialization document
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31841/2/Documentation/soc/intel/mp_init/mp_…
File Documentation/soc/intel/mp_init/mp_init.md:
https://review.coreboot.org/#/c/31841/2/Documentation/soc/intel/mp_init/mp_…
PS2, Line 26: Considering about facts we are having
> "Considering these facts there are"
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/31841
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I055808e2ddf03663e1ec5d3d423054d1caa911cb
Gerrit-Change-Number: 31841
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Comment-Date: Fri, 22 Mar 2019 06:42:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: comment
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30287 )
Change subject: soc/intel/common: Add Kconfig option to choose desired MP Init for platform
......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/#/c/30287/10/src/soc/intel/common/block/cpu/Kco…
File src/soc/intel/common/block/cpu/Kconfig:
https://review.coreboot.org/#/c/30287/10/src/soc/intel/common/block/cpu/Kco…
PS10, Line 58: help
> If this is to be user selectable, you need to add a title to this so it shows up in menuconfig etc. […]
Done
https://review.coreboot.org/#/c/30287/10/src/soc/intel/common/block/cpu/Kco…
PS10, Line 68: USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI
> that's quite a mouthful and I still don't know what it's supposed to do: install the PPI if this fla […]
yes Patrick.
Do you think help string is not enough to understand this ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/30287
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifbea463fdaf97d68c21a759c37f49492d58a056b
Gerrit-Change-Number: 30287
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Comment-Date: Fri, 22 Mar 2019 06:40:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Comment-In-Reply-To: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: comment