Attention is currently required from: Jamie Ryu, Subrata Banik.
Saurabh Mishra has posted comments on this change by Jamie Ryu. ( https://review.coreboot.org/c/coreboot/+/83784?usp=email )
Change subject: soc/intel/cmn/pmc: Add API to dump silicon QDF information
......................................................................
Patch Set 11:
(1 comment)
Patchset:
PS11:
> why this CL is part of the PTL SoC code recipe ? […]
Hi Subrata, this new API change is added for Intel Silicons. During PTL SOC Upstream, the common code changes are identified, and if any chnage is interdependent to PTL SOC recipe, we are keeping it aligned for recipe.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83784?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I927da1a97e6dad4ee54c4d2256fea5813a0ce43d
Gerrit-Change-Number: 83784
Gerrit-PatchSet: 11
Gerrit-Owner: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Thu, 08 Aug 2024 06:26:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Jamie Ryu, Saurabh Mishra.
Subrata Banik has posted comments on this change by Jamie Ryu. ( https://review.coreboot.org/c/coreboot/+/83784?usp=email )
Change subject: soc/intel/cmn/pmc: Add API to dump silicon QDF information
......................................................................
Patch Set 11:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83784/comment/c445a419_1b1e35b1?us… :
PS6, Line 13:
> thanks for comments. […]
Acknowledged
File src/soc/intel/common/block/include/intelblocks/pmc_ipc.h:
https://review.coreboot.org/c/coreboot/+/83784/comment/7de7079f_d6554ad0?us… :
PS6, Line 45: /* IPC command for accessing SoC registers */
: #define PMC_IPC_CMD_SOC_REG_ACC 0xAA
: #define PMC_IPC_CMD_SUBCMD_SOC_REG_RD 0x00
: #if CONFIG(SOC_QDF_DYNAMIC_READ_PMC)
: #define PMC_IPC_CMD_REGID_SOC_QDF 0x03
: #endif /* SOC_QDF_DYNAMIC_READ_PMC */
> Sure, done by patchset#10
` Also, if there is no other users of these macros, then it can go into pmclib.c file itself` still not addressed. I don't think any other code would like to use these macros ? apart from ur newly added API hence, make sense to keep those into pmclib.c
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/83784/comment/b1451ef4_d1d32aec?us… :
PS11, Line 913: if (rsp.buf[0]) {
```suggestion
if (r < 0 || rsp.buf[0] == 0) {
printk(BIOS_ERR, "%s: pmc_send_ipc_cmd failed.\n", __func__);
return;
}
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83784?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I927da1a97e6dad4ee54c4d2256fea5813a0ce43d
Gerrit-Change-Number: 83784
Gerrit-PatchSet: 11
Gerrit-Owner: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Comment-Date: Thu, 08 Aug 2024 06:19:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jamie Ryu <jamie.m.ryu(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Jamie Ryu, Saurabh Mishra.
Subrata Banik has posted comments on this change by Jamie Ryu. ( https://review.coreboot.org/c/coreboot/+/83784?usp=email )
Change subject: soc/intel/cmn/pmc: Add API to dump silicon QDF information
......................................................................
Patch Set 11:
(1 comment)
Patchset:
PS11:
why this CL is part of the PTL SoC code recipe ?
I don't see a need to do that. You are just adding a new API. And if any PTL code would like to use this API that can be done as part of PTL SoC recipe code later (and nothing needs to be addressed now itself while prep for PTL SoC code)
--
To view, visit https://review.coreboot.org/c/coreboot/+/83784?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I927da1a97e6dad4ee54c4d2256fea5813a0ce43d
Gerrit-Change-Number: 83784
Gerrit-PatchSet: 11
Gerrit-Owner: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Comment-Date: Thu, 08 Aug 2024 06:16:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Angel Pons, Dinesh Gehlot, Jayvik Desai, Julius Werner, Paul Menzel.
Subrata Banik has posted comments on this change by Jayvik Desai. ( https://review.coreboot.org/c/coreboot/+/83705?usp=email )
Change subject: vc/google/chromeos: Enable eSOL config with libgfx and uGOP
......................................................................
Patch Set 23: -Code-Review
(1 comment)
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/83705/comment/88695a69_138e0f27?us… :
PS20, Line 105: config CHROMEOS_ENABLE_ESOL
> So are you saying that the Kconfig only exists so that you can extract the `config` file from CBFS and check for it? I guess that's fine, but it is a very unusual use of Kconfig so you should definitely explain that in the `help` text here. If you just create an unused Kconfig and don't explain why you need it someone might go and delete it again.
valid point and adding a meaningful help text is always desirable. But with https://review.coreboot.org/c/coreboot/+/83769/comment/35b91b25_aaf1754c/ suggested changes, I believe this Kconfig is no more a unused Kconfig even in coreboot space.
also, you could change below code to use newly added Kconfig https://github.com/coreboot/coreboot/blob/main/src/soc/intel/alderlake/roms…
```
if (!CONFIG(CHROMEOS_ENABLE_ESOL) ||
!early_graphics_init()) {
timestamp_add_now(TS_ESOL_END);
return false;
}
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83705?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic8fe4ca5234de7f8e579f950f6ccbf750f4c7950
Gerrit-Change-Number: 83705
Gerrit-PatchSet: 23
Gerrit-Owner: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 08 Aug 2024 06:15:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Jayvik Desai <jayvik(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Tongtong Pan.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83797?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified-1 by build bot (Jenkins)
Change subject: mb/google/dedede:Add fw_config control to wifi driver
......................................................................
mb/google/dedede:Add fw_config control to wifi driver
BUG=b:351968527
Change-Id: I4d79051222a797d8a7805815092915e59b605468
Signed-off-by: Tongtong Pan <pantongtong(a)huaqin.corp-partner.google.com>
---
M src/mainboard/google/dedede/variants/awasuki/overridetree.cb
M src/mainboard/google/dedede/variants/awasuki/ramstage.c
2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/83797/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83797?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4d79051222a797d8a7805815092915e59b605468
Gerrit-Change-Number: 83797
Gerrit-PatchSet: 2
Gerrit-Owner: Tongtong Pan <pantongtong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tongtong Pan <pantongtong(a)huaqin.corp-partner.google.com>
Attention is currently required from: Ashish Kumar Mishra, Cliff Huang, Dinesh Gehlot, Elyes Haouas, Eran Mitrani, Felix Singer, Jakub Czapiga, Jamie Ryu, Jérémy Compostella, Kapil Porwal, Ravishankar Sarawadi, Saurabh Mishra, Tarun, Wonkyu Kim.
Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83354?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock
......................................................................
Patch Set 62:
(1 comment)
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/b2fb85a5_793d66a6?us… :
PS48, Line 72: #define P2SB2_BAR CONFIG_P2SB_2_PCR_BASE_ADDRESS
> This name was suggested by Subrata, we had resolved this query earlier to be using "P2SB_2_PCR_BASE_ […]
PCR means private configuration register space which doesn't depict the actual picture that there are two P2SB IP in each die hence, we need two different BARs which is PCR of P2SB. I think P2SB2 is more meaningful than PCR2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83354?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibcfe71eec27cebf04f10ec343a73dd92f1272aca
Gerrit-Change-Number: 83354
Gerrit-PatchSet: 62
Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Thu, 08 Aug 2024 06:06:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com>