Attention is currently required from: Patrick Rudolph, Julius Werner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61960 )
Change subject: bsd/cb_err: Add error code for UEFI variable store
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
File src/commonlib/bsd/include/commonlib/bsd/cb_err.h:
https://review.coreboot.org/c/coreboot/+/61960/comment/e35c395c_b1af682d
PS2, Line 19: TO
TOO
https://review.coreboot.org/c/coreboot/+/61960/comment/236102cf_f3d73ccd
PS2, Line 46: CB_EFI_FVH_INVALID = -500, /**< UEFI FVH is corrupted */
> please, no space before tabs
Please fix.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61960
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6baea9fb138d1a2755d22a3d587105793adb9c90
Gerrit-Change-Number: 61960
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Joey Madafferi <joeymadafferi(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Patrick Rudolph
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Patrick Rudolph
Gerrit-Comment-Date: Tue, 15 Mar 2022 13:43:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61959 )
Change subject: vendorcode: Add VariableFormat.h
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
Is there a way to avoid the UDK2017 dependency? If this is the only edk2 header needed to implement EFI variable store support in coreboot, I'd rather include it directly from the driver.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61959
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibe44925555a7d1d2361dd48c0325b840bd68e0ca
Gerrit-Change-Number: 61959
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 15 Mar 2022 13:42:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jeff Daly, Mariusz Szafrański, Jonathan Zhang, Arthur Heymans, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, Nico Huber, Anjaneya "Reddy" Chagam, Johnny Lin, Tim Wawrzynczak, Suresh Bellampalli, Christian Walter, Vanessa Eusebio, Lean Sheng Tan, Michal Motyl, Werner Zeh, Tim Chu.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62064 )
Change subject: soc/intel: Move `pmc_clear_pmcon_sts()` into IA common code
......................................................................
Patch Set 10:
(6 comments)
File src/soc/intel/common/block/pmc/pmclib.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144008):
https://review.coreboot.org/c/coreboot/+/62064/comment/a8686113_a2dc64a9
PS10, Line 633: if (CONFIG(SOC_INTEL_COMMON_BLOCK_PMC_DISCOVERABLE) && (GEN_PMCON_A <= 0xff))
suspect code indent for conditional statements (8, 8)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144008):
https://review.coreboot.org/c/coreboot/+/62064/comment/2ceaa97a_22c34eef
PS10, Line 634: pmc_clear_pmcon_sts_pci();
code indent should use tabs where possible
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144008):
https://review.coreboot.org/c/coreboot/+/62064/comment/246e2297_3e794258
PS10, Line 634: pmc_clear_pmcon_sts_pci();
please, no spaces at the start of a line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144008):
https://review.coreboot.org/c/coreboot/+/62064/comment/50c1ab54_e69a19d3
PS10, Line 635: else
suspect code indent for conditional statements (8, 8)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144008):
https://review.coreboot.org/c/coreboot/+/62064/comment/e13e2c04_03e2df68
PS10, Line 636: pmc_clear_pmcon_sts_mmio();
code indent should use tabs where possible
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-144008):
https://review.coreboot.org/c/coreboot/+/62064/comment/b7f13d11_a568c9f1
PS10, Line 636: pmc_clear_pmcon_sts_mmio();
please, no spaces at the start of a line
--
To view, visit https://review.coreboot.org/c/coreboot/+/62064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8d15f421c128630f928a1b6a7e2840056d68d7b1
Gerrit-Change-Number: 62064
Gerrit-PatchSet: 10
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: 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: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: siemens-bot
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Tue, 15 Mar 2022 13:23:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jeff Daly, Mariusz Szafrański, Jonathan Zhang, Arthur Heymans, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, Nico Huber, Anjaneya "Reddy" Chagam, Johnny Lin, Tim Wawrzynczak, Suresh Bellampalli, Christian Walter, Vanessa Eusebio, Lean Sheng Tan, Michal Motyl, Werner Zeh, Tim Chu.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62064 )
Change subject: soc/intel: Move `pmc_clear_pmcon_sts()` into IA common code
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS9:
> OK, in this case I can confirm that the OS was booted successfully and that there are no errors in the coreboot log.
Thanks Werner for the confirmation.
--
To view, visit https://review.coreboot.org/c/coreboot/+/62064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8d15f421c128630f928a1b6a7e2840056d68d7b1
Gerrit-Change-Number: 62064
Gerrit-PatchSet: 10
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: 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: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: siemens-bot
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Tue, 15 Mar 2022 13:06:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: comment
Attention is currently required from: Shelley Chen, Felix Singer, Ravi kumar, Paul Menzel, mturney mturney, Ravi Kumar Bokka, Julius Werner, Angel Pons, Arthur Heymans.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59193 )
Change subject: libpayload: Parse DDR Information using coreboot tables
......................................................................
Patch Set 40:
(3 comments)
File payloads/libpayload/libc/coreboot.c:
https://review.coreboot.org/c/coreboot/+/59193/comment/37b15944_1c52c6aa
PS37, Line 263: _INFO
> hi shelly, i have taken care this comments
Ack
File src/commonlib/bsd/include/commonlib/bsd/mem_chip_info.h:
https://review.coreboot.org/c/coreboot/+/59193/comment/ec2550b7_b9243d23
PS40, Line 7: DDR3 = 0x30,
I don't think Julius meant to remove the MEM_CHIP_ prefix.
https://review.coreboot.org/c/coreboot/+/59193/comment/01742e55_b202b111
PS40, Line 8: 38
0x38
--
To view, visit https://review.coreboot.org/c/coreboot/+/59193
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieca7e9fc0e1a018fcb2e9315aebee088edac858e
Gerrit-Change-Number: 59193
Gerrit-PatchSet: 40
Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-CC: Xixi Chen <xixi.chen(a)mediatek.corp-partner.google.com>
Gerrit-CC: mturney mturney <quic_mturney(a)quicinc.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: mturney mturney <mturney(a)codeaurora.org>
Gerrit-Attention: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: mturney mturney <quic_mturney(a)quicinc.com>
Gerrit-Comment-Date: Tue, 15 Mar 2022 13:05:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shelley Chen <shchen(a)google.com>
Comment-In-Reply-To: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Jeff Daly, Mariusz Szafrański, Jonathan Zhang, Arthur Heymans, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, Nico Huber, Anjaneya "Reddy" Chagam, Subrata Banik, Johnny Lin, Tim Wawrzynczak, Suresh Bellampalli, Christian Walter, Vanessa Eusebio, Lean Sheng Tan, Michal Motyl, Tim Chu.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62064 )
Change subject: soc/intel: Move `pmc_clear_pmcon_sts()` into IA common code
......................................................................
Patch Set 10: Code-Review+1
(1 comment)
Patchset:
PS9:
> > I let our testbot have a try and it looks good so far (boot-tested with mc_apl1 and mc_apl4). […]
OK, in this case I can confirm that the OS was booted successfully and that there are no errors in the coreboot log.
--
To view, visit https://review.coreboot.org/c/coreboot/+/62064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8d15f421c128630f928a1b6a7e2840056d68d7b1
Gerrit-Change-Number: 62064
Gerrit-PatchSet: 10
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: 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: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: siemens-bot
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Tue, 15 Mar 2022 12:57:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: comment
Attention is currently required from: Jeff Daly, Mariusz Szafrański, Jonathan Zhang, Arthur Heymans, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, Nico Huber, Anjaneya "Reddy" Chagam, Johnny Lin, Tim Wawrzynczak, Suresh Bellampalli, Christian Walter, Vanessa Eusebio, Lean Sheng Tan, Michal Motyl, Werner Zeh, Tim Chu.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62064 )
Change subject: soc/intel: Move `pmc_clear_pmcon_sts()` into IA common code
......................................................................
Patch Set 9:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62064/comment/ef55061f_c8113fe5
PS9, Line 19: not
> 'not visible' or 'not discoverable'?
Ack
Patchset:
PS9:
> I let our testbot have a try and it looks good so far (boot-tested with mc_apl1 and mc_apl4). Any portion of the coreboot log you are interested in (which I would love to provide)?
Thanks Werner for your help. I guess booting to the OS without any hang is good evidence that things are good.
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/62064/comment/debce909_1d16ec7c
PS9, Line 586: pmc_clear_pmcon_sts_using_mmio
> Maybe just 'pmc_clear_pmcon_sts_mmio' and 'pmc_clear_pmcon_sts_pci' to keep function names short?
Ack
https://review.coreboot.org/c/coreboot/+/62064/comment/773ac26f_f7161f44
PS9, Line 601: struct device *dev = pcidev_path_on_root(PCH_DEVFN_PMC);
> Add a check for dev being assigned? Would avoid cases where, for some reason, the device is not ther […]
Ack
https://review.coreboot.org/c/coreboot/+/62064/comment/aabf21e2_f6e3d582
PS9, Line 603: reg_val = pci_read_config32(dev, GEN_PMCON_A);
: reg_val &= ~(MS4V);
:
: pci_write_config32(dev, GEN_PMCON_A, reg_val);
> Would 'pci_and_config32(dev, GEN_PMCON_A, ~(MS4V));' be helpful here?
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/62064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8d15f421c128630f928a1b6a7e2840056d68d7b1
Gerrit-Change-Number: 62064
Gerrit-PatchSet: 9
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: 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: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: siemens-bot
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Tue, 15 Mar 2022 12:55:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: comment
Attention is currently required from: Jeff Daly, Mariusz Szafrański, Jonathan Zhang, Arthur Heymans, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, Nico Huber, Anjaneya "Reddy" Chagam, Subrata Banik, Johnny Lin, Tim Wawrzynczak, Suresh Bellampalli, Christian Walter, Vanessa Eusebio, Lean Sheng Tan, Michal Motyl, Tim Chu.
Hello build bot (Jenkins), Jeff Daly, Mariusz Szafrański, Jonathan Zhang, Angel Pons, Arthur Heymans, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, Anjaneya "Reddy" Chagam, Johnny Lin, Christian Walter, Suresh Bellampalli, Tim Wawrzynczak, Vanessa Eusebio, Nick Vaccaro, siemens-bot, Lean Sheng Tan, Michal Motyl, Werner Zeh, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/62064
to look at the new patch set (#10).
Change subject: soc/intel: Move `pmc_clear_pmcon_sts()` into IA common code
......................................................................
soc/intel: Move `pmc_clear_pmcon_sts()` into IA common code
This patch moves `pmc_clear_pmcon_sts` function into common code and
remove SoC specific instances.
Accessing PMC GEN_PMCON_A register differs between different Intel
chipsets. Typically, there are two possible ways to perform GEN_PMCON_A
register programming (like `pmc_clear_pmcon_sts()`) as:
1. Using PCI configuration space when GEN_PMCON_A is a PCI configuration
register. And, the PMC device is visible over the PCI bus, so the SoC
user selects `SOC_INTEL_COMMON_BLOCK_PMC_DISCOVERABLE` Kconfig.
2. Using MMIO access when GEN_PMCON_A is a memory mapped register.
And, the PMC device is not visible over the PCI bus and hidden by
the FSP.
Unfortunately, there is a 3rd case as well with Apollolake platform
where SoC selects SOC_INTEL_COMMON_BLOCK_PMC_DISCOVERABLE Kconfig and
GEN_PMCON_A is memory mapped, and this scenario can't be fit into either
of the two cases above, unless provided with one more check to identify
if GEN_PMCON_A is a PCI configuration space register (offset within 256
bytes) or memory mapped. Hence, added a check along with
SOC_INTEL_COMMON_BLOCK_PMC_DISCOVERABLE Kconfig to see if the register
(i.e., GEN_PMCON_A) actually belongs to the PCI config space.
BUG=b:211954778
TEST=Able to build brya.
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Change-Id: I8d15f421c128630f928a1b6a7e2840056d68d7b1
---
M src/soc/intel/alderlake/include/soc/pm.h
M src/soc/intel/alderlake/pmutil.c
M src/soc/intel/apollolake/include/soc/pm.h
M src/soc/intel/apollolake/pmutil.c
M src/soc/intel/cannonlake/include/soc/pm.h
M src/soc/intel/cannonlake/pmutil.c
M src/soc/intel/common/block/include/intelblocks/pmclib.h
M src/soc/intel/common/block/pmc/pmc.c
M src/soc/intel/common/block/pmc/pmclib.c
M src/soc/intel/denverton_ns/include/soc/pm.h
M src/soc/intel/denverton_ns/pmutil.c
M src/soc/intel/elkhartlake/include/soc/pm.h
M src/soc/intel/elkhartlake/pmutil.c
M src/soc/intel/icelake/include/soc/pm.h
M src/soc/intel/icelake/pmutil.c
M src/soc/intel/jasperlake/include/soc/pm.h
M src/soc/intel/jasperlake/pmutil.c
M src/soc/intel/skylake/include/soc/pm.h
M src/soc/intel/skylake/pmutil.c
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/pmutil.c
M src/soc/intel/xeon_sp/include/soc/pm.h
M src/soc/intel/xeon_sp/pmutil.c
23 files changed, 61 insertions(+), 173 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/62064/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/62064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8d15f421c128630f928a1b6a7e2840056d68d7b1
Gerrit-Change-Number: 62064
Gerrit-PatchSet: 10
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: 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: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: siemens-bot
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jeff Daly, Mariusz Szafrański, Jonathan Zhang, Arthur Heymans, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, Nico Huber, Anjaneya "Reddy" Chagam, Subrata Banik, Johnny Lin, Tim Wawrzynczak, Suresh Bellampalli, Christian Walter, Vanessa Eusebio, Lean Sheng Tan, Michal Motyl, Tim Chu.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62064 )
Change subject: soc/intel: Move `pmc_clear_pmcon_sts()` into IA common code
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
I let our testbot have a try and it looks good so far (boot-tested with mc_apl1 and mc_apl4). Any portion of the coreboot log you are interested in (which I would love to provide)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/62064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8d15f421c128630f928a1b6a7e2840056d68d7b1
Gerrit-Change-Number: 62064
Gerrit-PatchSet: 9
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: 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: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: siemens-bot
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Tue, 15 Mar 2022 12:43:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: David Wu, Ren Kuo, Tim Wawrzynczak, Nick Vaccaro.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62810 )
Change subject: mb/google/brya/var/volmar: Disable thunderbolt
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
woops, wrong account
--
To view, visit https://review.coreboot.org/c/coreboot/+/62810
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic1bba80707b1d4a97c486e22f79feccf6241865e
Gerrit-Change-Number: 62810
Gerrit-PatchSet: 2
Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Tue, 15 Mar 2022 12:40:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment