Attention is currently required from: Jamie Ryu, Saurabh Mishra.
Hello Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams, Jérémy Compostella, Ravishankar Sarawadi, Saurabh Mishra, Subrata Banik, Wonkyu Kim, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83784?usp=email
to look at the new patch set (#12).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/intel/cmn/pmc: Add API to dump silicon QDF information
......................................................................
soc/intel/cmn/pmc: Add API to dump silicon QDF information
This adds pmc_dump_soc_qdf_info function and PMC_IPC_CMD_SOC_REG_ACC
PMC IPC Command to read and print Intel SoC QDF information using PMC
interface if SOC_QDF_DYNAMIC_READ_PMC is enabled. QDF read command is
supported from Panther Lake SoC.
QDF is a four digit code that can be used to identify enabled features
and capabilities. This information will be useful to debug issues
found during the development phase and in the field as well.
Change-Id: I927da1a97e6dad4ee54c4d2256fea5813a0ce43d
Signed-off-by: Jamie Ryu <jamie.m.ryu(a)intel.com>
---
M src/soc/intel/common/block/include/intelblocks/pmclib.h
M src/soc/intel/common/block/pmc/Kconfig
M src/soc/intel/common/block/pmc/pmclib.c
3 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/83784/12
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I927da1a97e6dad4ee54c4d2256fea5813a0ce43d
Gerrit-Change-Number: 83784
Gerrit-PatchSet: 12
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>
Attention is currently required from: Ashish Kumar Mishra, Cliff Huang, Krishna P Bhat D, Saurabh Mishra.
Ronak Kanabar has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83732?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: vc/intel/fsp/fsp2_0/ptl: Add placeholder FSP headers to compile
......................................................................
Patch Set 15:
(3 comments)
File src/vendorcode/intel/fsp/fsp2_0/pantherlake/FirmwareVersionInfo.h:
https://review.coreboot.org/c/coreboot/+/83732/comment/a2a5d5d8_306c6004?us… :
PS15, Line 5:
: * This software is released under the MIT License.
: * https://opensource.org/licenses/MIT
: *
: * Permission is hereby granted, free of charge, to any person obtaining a copy
: * of this software and associated documentation files (the "Software"), to deal
: * in the Software without restriction, including without limitation the rights
: * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
: * copies of the Software, and to permit persons to whom the Software is
: * furnished to do so, subject to the following conditions:
: *
: * The above copyright notice and this permission notice shall be included in all
: * copies or substantial portions of the Software.
: *
: * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
: * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
: * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
: * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
: * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
: * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
: * SOFTWARE.
: */
need change
ref : https://github.com/coreboot/coreboot/blob/main/src/vendorcode/intel/fsp/fsp…
File src/vendorcode/intel/fsp/fsp2_0/pantherlake/FspProducerDataHeader.h:
https://review.coreboot.org/c/coreboot/+/83732/comment/1a6d893d_68e1e6fe?us… :
PS15, Line 8: Permission is hereby granted, free of charge, to any person obtaining a copy
: * of this software and associated documentation files (the "Software"), to deal
: * in the Software without restriction, including without limitation the rights
: * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
: * copies of the Software, and to permit persons to whom the Software is
: * furnished to do so, subject to the following conditions:
: *
: * The above copyright notice and this permission notice shall be included in all
: * copies or substantial portions of the Software.
: *
: * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
: * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
: * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
: * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
: * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
: * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
: * SOFTWARE.
: */
need change
https://github.com/coreboot/coreboot/blob/main/src/vendorcode/intel/fsp/fsp…
check this for the ref
File src/vendorcode/intel/fsp/fsp2_0/pantherlake/MemInfoHob.h:
https://review.coreboot.org/c/coreboot/+/83732/comment/a79a86e8_942b4e78?us… :
PS15, Line 3: Memory S3 Save data, Memory Info data and Memory Platform
: * data hobs.
: *
: * Copyright (c) 2024, Intel Corporation. All rights reserved.<BR>
: *
: * This software is released under the MIT License.
: * https://opensource.org/licenses/MIT
: *
: * Permission is hereby granted, free of charge, to any person obtaining a copy
: * of this software and associated documentation files (the "Software"), to deal
: * in the Software without restriction, including without limitation the rights
: * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
: * copies of the Software, and to permit persons to whom the Software is
: * furnished to do so, subject to the following conditions:
: *
: * The above copyright notice and this permission notice shall be included in all
: * copies or substantial portions of the Software.
: *
: * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
: * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
: * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
: * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
: * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
: * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
: * SOFTWARE.
need change
ref : https://github.com/coreboot/coreboot/blob/main/src/vendorcode/intel/fsp/fsp…
--
To view, visit https://review.coreboot.org/c/coreboot/+/83732?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: I4c069ba64f487259ce746dc52296618d91209602
Gerrit-Change-Number: 83732
Gerrit-PatchSet: 15
Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Appukuttan V K <appukuttan.vk(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: 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: Krishna P Bhat D <krishna.p.bhat.d(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: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Comment-Date: Thu, 08 Aug 2024 07:30:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Felix Singer, Jérémy Compostella, yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83318?usp=email )
Change subject: soc/intel/common/systemagent: Improve systemagent
......................................................................
Patch Set 10:
(5 comments)
File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/83318/comment/92bc4f5b_5a9eec6c?us… :
PS10, Line 146: value = ALIGN_DOWN(value, 1 * MiB);
If a register is with low 20+ bits reserved (cannot be written into), the align down to 1MB operation is actually not effective. If a register is with low 20- bits reserved, the align down to 1MB operation is actually changing the value.
At the moment, when we are not clear on the direct impact of the aligning down on specific SoCs, we cannot remove it. But we can add an assert here as reminder in logs.
https://review.coreboot.org/c/coreboot/+/83318/comment/c9a5c4a0_104065c3?us… :
PS10, Line 78: return;
Can you move ln77 to ln84?
https://review.coreboot.org/c/coreboot/+/83318/comment/b7acd2db_bb2c7c26?us… :
PS10, Line 135: .align = CONFIG_TOUUD_ALIGNMENT,
No need to add align here
https://review.coreboot.org/c/coreboot/+/83318/comment/26c0ce6a_912bd8cb?us… :
PS10, Line 178: value = ALIGN_DOWN(value, entry->align);
if (entry->is_limit) {
probing granularity by writing 1s and read back.
value = (value | (1 < granularity - 1)) + 1;
} else {
value = ALIGN_DOWN(value, 1 * MiB); -> keep as is
}
And the +1 operation is quite confusing, it had better to be moved to where the value is used.
File src/soc/intel/common/block/systemagent/systemagent_def.h:
https://review.coreboot.org/c/coreboot/+/83318/comment/a11866c4_69a8538a?us… :
PS3, Line 73: * IS_LIMIT = If registers/offset indicates address limit or address limit plus 1.
> That doesn't make sense since all values here should be aligned. […]
Rename is_limit to fix_up_limit?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83318?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: If32c2a6524c9d55ce7f9c3dd203bcf85cab76c2c
Gerrit-Change-Number: 83318
Gerrit-PatchSet: 10
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Thu, 08 Aug 2024 07:29:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Nicholas Chin, Paul Menzel.
Pablo has posted comments on this change by Pablo. ( https://review.coreboot.org/c/coreboot/+/83817?usp=email )
Change subject: Documentation: getting-started: faq: Remove line break in URL breaking link
......................................................................
Patch Set 5:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83817/comment/3abed3f8_80620215?us… :
PS2, Line 7: Fix space that breaked link on documentation rendering
> Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/83817/comment/6eea2285_4098672b?us… :
PS2, Line 10:
> Please add a Signed-off-by line.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83817?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: I3f950af4201486cd90e5fa61a4657ab7ae643825
Gerrit-Change-Number: 83817
Gerrit-PatchSet: 5
Gerrit-Owner: Pablo <Pablo(a)Iranzo.io>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Thu, 08 Aug 2024 07:21:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Nicholas Chin, Pablo, Paul Menzel.
Hello Nicholas Chin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83817?usp=email
to look at the new patch set (#6).
Change subject: Documentation: getting-started: faq: Remove line break in URL breaking link
......................................................................
Documentation: getting-started: faq: Remove line break in URL breaking link
Change-Id: I3f950af4201486cd90e5fa61a4657ab7ae643825
Signed-off-by: Pablo Iranzo Gómez <Pablo.Iranzo(a)gmail.com>
---
M Documentation/getting_started/faq.md
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/83817/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/83817?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: I3f950af4201486cd90e5fa61a4657ab7ae643825
Gerrit-Change-Number: 83817
Gerrit-PatchSet: 6
Gerrit-Owner: Pablo <Pablo(a)Iranzo.io>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Pablo <Pablo(a)Iranzo.io>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>