Attention is currently required from: Subrata Banik, Angel Pons, Werner Zeh, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61519 )
Change subject: soc/intel/common/cse: Add `cse_send_end_of_post()` as a public function
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/61519
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I837c5723eca766d21b191b98e39eb52889498bfc
Gerrit-Change-Number: 61519
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:32:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61451 )
Change subject: soc/intel/cannonlake: Use SBI msg to disable HECI1
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8:
> this appears to cause a hang during finalization on google/hatch (akemi variant); reverting it allows normal booting
Interesting, On CML platform, we had validated both more actually.
Can you please select SOC_INTEL_COMMON_BLOCK_HECI1_DISABLE_USING_PMC_IPC instead HECI_SMM and see if any luck from SoC Kconfig.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61451
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6882b619506d1bf4131f68c2c9a32ef4f7d6f6d7
Gerrit-Change-Number: 61451
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wonkyu Kim
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:22:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier, Tim Wawrzynczak, Sridhar Siricilla, Michael Niewöhner, Lean Sheng Tan, Patrick Rudolph, EricR Lai.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61650 )
Change subject: soc/intel/skylake: Add function to clear PMCON status bits
......................................................................
Patch Set 3:
(2 comments)
File src/soc/intel/skylake/pmc.c:
https://review.coreboot.org/c/coreboot/+/61650/comment/94499377_4b25d259
PS3, Line 99: pci_or_config32(dev, GEN_PMCON_A, 0);
> well...
This code is not honouring the bit 18 which may not be still relevant for Chrome platform as S4 is not POR but ideally it should.
File src/soc/intel/skylake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/61650/comment/a4eb9c76_9109aef2
PS3, Line 269: void pmc_clear_pmcon_sts(void)
> this function is all the same for all platforms using common code, so why exactly do we have to copy-pasta it once again instead of finally moving it to soc/intel/common?
There are few SOCs which eventually uses the IA PMC common code but the SOC EDS does have exact register details. I've tried creating the common code which eventually need to resolve the register definitions from SoC layer as GEN_PMCON_A offset is not same across all SoC. But due to documentation limitation issue, I just avoided pursuing that path.
> btw. this probably should be done in soc_pmc_init instead of finalize.
The reason this is being done as part of finalize is to ensure that prior booting to OS, those bits are cleared.
Note: FSP can even request a global reset during FSP notify (which is post soc_pmc_init) hence, after global reset, this corresponding bit would have set. And, we might need to clear it prior to boot. There is no harm if you would like to call this more than once.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie786e6ba2daf88accb5d70be33de0abe593f8c53
Gerrit-Change-Number: 61650
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:19:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61649 )
Change subject: soc/intel/*/pmc: Add `finalize` operation for pmc
......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/cannonlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/61649/comment/1f8f8745_751a3b09
PS8, Line 146: pmc_clear_pmcon_sts();
this probably should be done in soc_pmc_init instead of finalize. compare CB:58019
--
To view, visit https://review.coreboot.org/c/coreboot/+/61649
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0a0b869849d5d8c76031b8999f3d28817ac69247
Gerrit-Change-Number: 61649
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:09:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Matt DeVillier, Tim Wawrzynczak, Sridhar Siricilla, Lean Sheng Tan, Patrick Rudolph, EricR Lai.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61650 )
Change subject: soc/intel/skylake: Add function to clear PMCON status bits
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/skylake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/61650/comment/a846f40b_914adfe9
PS3, Line 269: void pmc_clear_pmcon_sts(void)
> this function is all the same for all platforms using common code, so why exactly do we have to copy […]
btw. this probably should be done in soc_pmc_init instead of finalize. compare CB:58019
--
To view, visit https://review.coreboot.org/c/coreboot/+/61650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie786e6ba2daf88accb5d70be33de0abe593f8c53
Gerrit-Change-Number: 61650
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:07:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61451 )
Change subject: soc/intel/cannonlake: Use SBI msg to disable HECI1
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8:
this appears to cause a hang during finalization on google/hatch (akemi variant); reverting it allows normal booting
--
To view, visit https://review.coreboot.org/c/coreboot/+/61451
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6882b619506d1bf4131f68c2c9a32ef4f7d6f6d7
Gerrit-Change-Number: 61451
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wonkyu Kim
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:05:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Marc Jones, Anjaneya "Reddy" Chagam, Subrata Banik, Johnny Lin, Christian Walter, Angel Pons, Arthur Heymans, Nick Vaccaro, Patrick Rudolph, EricR Lai, Tim Chu.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61652 )
Change subject: soc/intel/xeon_sp: Add function to clear PMCON status bits
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/xeon_sp/pmutil.c:
https://review.coreboot.org/c/coreboot/+/61652/comment/f4e9bb95_3329c669
PS3, Line 183: void pmc_clear_pmcon_sts(void)
copypastacopypasta see https://review.coreboot.org/c/coreboot/+/61650/3/src/soc/intel/skylake/pmut…
--
To view, visit https://review.coreboot.org/c/coreboot/+/61652
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I22650f539a1646f93f2c6494cbf54b8ca785d6ad
Gerrit-Change-Number: 61652
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:02:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Angel Pons, Werner Zeh, Andrey Petrov, Patrick Rudolph, EricR Lai.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61651 )
Change subject: soc/intel/apollolake: Add function to clear PMCON status bits
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/apollolake/pmutil.c:
https://review.coreboot.org/c/coreboot/+/61651/comment/88bac3eb_6ebdd2df
PS3, Line 241: void pmc_clear_pmcon_sts(void)
copypastacopypasta see https://review.coreboot.org/c/coreboot/+/61650/3/src/soc/intel/skylake/pmut…
--
To view, visit https://review.coreboot.org/c/coreboot/+/61651
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I982f669b13f25d1d0e6dfaec2fbf50d3200f74fe
Gerrit-Change-Number: 61651
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 20:02:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment