Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Angel Pons, Sridhar Siricilla, Werner Zeh, Andrey Petrov, Patrick Rudolph, EricR Lai.
Hello build bot (Jenkins), Maulik V Vaghela, Tim Wawrzynczak, Angel Pons, Werner Zeh, Andrey Petrov, Patrick Rudolph, EricR Lai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/61444
to look at the new patch set (#6).
Change subject: soc/intel/apollolake: Use PCR write to disable HECI1
......................................................................
soc/intel/apollolake: Use PCR write to disable HECI1
Set the SOC_INTEL_COMMON_BLOCK_HECI1_DISABLE_USING_PCR config for
Apollo Lake to disable HECI1 device using PCR writes.
BUG=none
TEST=None
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Change-Id: I8df9544296f0bea095c5415805a596cb5b36885e
---
M src/soc/intel/apollolake/Kconfig
M src/soc/intel/apollolake/cse.c
2 files changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/61444/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/61444
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8df9544296f0bea095c5415805a596cb5b36885e
Gerrit-Change-Number: 61444
Gerrit-PatchSet: 6
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: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.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-MessageType: newpatchset
Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Sridhar Siricilla, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61455 )
Change subject: soc/intel/cannonlake: Drop unnecessary guard for HECI1 disable FSP UPD
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/61455/comment/fb7f23f5_80325213
PS3, Line 589: Heci1
> Writing Heci1 in the code should be ok, but comment should be HECI1! WDYT?
Don't know about any rule thats support this argument "Writing Heci1 in the code should be ok, but comment should be HECI1!". If you are specific about following the acronym then why not adhere the same all across in my point.
Also, if I just look for your recommendation then also there are ample conflicts. I would prefer to have action that unifies those as well.
src//soc/intel/elkhartlake/romstage/romstage.c:133: /* initialize Heci interface */
src//soc/intel/cannonlake/chip.h:217: /* Heci related */
src//soc/intel/cannonlake/romstage/romstage.c:127: /* initialize Heci interface */
> I can update remaining places wherever applicable.
Sure
--
To view, visit https://review.coreboot.org/c/coreboot/+/61455
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia8908c080ca9991e7a71e795ccb8fc76d99514f8
Gerrit-Change-Number: 61455
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: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
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: Sun, 30 Jan 2022 07:36:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Maulik V Vaghela, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61455 )
Change subject: soc/intel/cannonlake: Drop unnecessary guard for HECI1 disable FSP UPD
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/61455/comment/34ebf4d5_12512014
PS3, Line 589: Heci1
> Sure, I can change Heci1 to HECI1 but IMO, we should follow a rule that applies to all. […]
Writing Heci1 in the code should be ok, but comment should be HECI1! WDYT? I can update remaining places wherever applicable.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61455
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia8908c080ca9991e7a71e795ccb8fc76d99514f8
Gerrit-Change-Number: 61455
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: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: 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: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: Sun, 30 Jan 2022 07:30:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Matt DeVillier, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai.
Hello build bot (Jenkins), Matt DeVillier, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/61433
to look at the new patch set (#7).
Change subject: soc/intel/skylake: Use PCR write to disable HECI1
......................................................................
soc/intel/skylake: Use PCR write to disable HECI1
Set the SOC_INTEL_COMMON_BLOCK_HECI1_DISABLE_USING_PCR config for
Skylake to disable HECI1 device using PCR writes.
BUG=none
TEST=None
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Change-Id: Ib6bfa7c48660a6df8d0944de675a4f30fe248d1b
---
M src/soc/intel/skylake/Kconfig
M src/soc/intel/skylake/finalize.c
2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/61433/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/61433
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib6bfa7c48660a6df8d0944de675a4f30fe248d1b
Gerrit-Change-Number: 61433
Gerrit-PatchSet: 7
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: Matt DeVillier <matt.devillier(a)gmail.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: 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: 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-MessageType: newpatchset
Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Sridhar Siricilla, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61455 )
Change subject: soc/intel/cannonlake: Drop unnecessary guard for HECI1 disable FSP UPD
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/61455/comment/443d4736_cf429c8f
PS3, Line 589: Heci1
Sure, I can change Heci1 to HECI1 but IMO, we should follow a rule that applies to all.
In that case, will you be able to fix all Heci to HECI in a follow up patch.
>
subratabanik-macbookpro:coreboot subratabanik$ grep -rsn "Heci" src/
src//soc/intel/alderlake/fsp_params.c:642: s_cfg->DisableD0I3SettingForHeci = 1;
src//soc/intel/elkhartlake/romstage/fsp_params.c:92: m_cfg->HeciCommunication2 = config->Heci2Enable;
src//soc/intel/elkhartlake/romstage/romstage.c:133: /* initialize Heci interface */
src//soc/intel/elkhartlake/fsp_params.c:312: params->Heci3Enabled = config->Heci3Enable;
src//soc/intel/elkhartlake/chip.h:241: uint8_t Heci2Enable;
src//soc/intel/elkhartlake/chip.h:242: uint8_t Heci3Enable;
src//soc/intel/denverton_ns/upd_display.c:46: DISPLAY_UPD(PcdMeHeciCommunication);
src//soc/intel/denverton_ns/romstage.c:148: if (mupd->FspmConfig.PcdMeHeciCommunication == 0) {
src//soc/intel/cannonlake/romstage/fsp_params.c:132: if (config->DisableHeciRetry)
src//soc/intel/cannonlake/romstage/fsp_params.c:133: tconfig->DisableHeciRetry = config->DisableHeciRetry;
src//soc/intel/cannonlake/romstage/fsp_params.c:151: m_cfg->Heci1BarAddress = HECI1_BASE_ADDRESS;
src//soc/intel/cannonlake/romstage/romstage.c:127: /* initialize Heci interface */
src//soc/intel/cannonlake/fsp_params.c:588: params->Heci3Enabled = is_devfn_enabled(PCH_DEVFN_CSE_3);
src//soc/intel/cannonlake/fsp_params.c:590: params->Heci1Disabled = CONFIG(DISABLE_HECI1_AT_PRE_BOOT);
src//soc/intel/cannonlake/chip.h:217: /* Heci related */
src//soc/intel/cannonlake/chip.h:218: uint8_t DisableHeciRetry;
src//soc/intel/skylake/bootblock/pch.c:144: /* initialize Heci interface */
src//soc/intel/skylake/finalize.c:61: /* we should disable Heci1 based on the config */
src//soc/intel/skylake/chip.c:365: params->Heci3Enabled = is_devfn_enabled(PCH_DEVFN_CSE_3);
src//soc/intel/icelake/romstage/romstage.c:116: /* initialize Heci interface */
>
Moreover why don't you file a bug for FSP to update all Heci related UPDs to adhere to the acronyms HECI, for example:
>
src//vendorcode/intel/fsp/fsp2_0/alderlake/FspmUpd.h:752: UINT8 HeciTimeouts;
src//vendorcode/intel/fsp/fsp2_0/alderlake/FspmUpd.h:757: UINT32 Heci1BarAddress;
src//vendorcode/intel/fsp/fsp2_0/alderlake/FspmUpd.h:762: UINT32 Heci2BarAddress;
src//vendorcode/intel/fsp/fsp2_0/alderlake/FspmUpd.h:767: UINT32 Heci3BarAddress;
>
--
To view, visit https://review.coreboot.org/c/coreboot/+/61455
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia8908c080ca9991e7a71e795ccb8fc76d99514f8
Gerrit-Change-Number: 61455
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: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
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: Sun, 30 Jan 2022 07:25:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-MessageType: comment