Attention is currently required from: Felix Singer, Filip Lewiński, Krystian Hebel, Maciej Pijanowski, Martin L Roth, Michał Kopeć, Paul Menzel.
Matt DeVillier has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/83385?usp=email )
Change subject: payloads/external/iPXE/Makefile: Build iPXE for EFI target if requested
......................................................................
Patch Set 3:
(3 comments)
File payloads/external/iPXE/Kconfig:
https://review.coreboot.org/c/coreboot/+/83385/comment/8bd08e88_b393641d?us… :
PS3, Line 119: config IPXE_BUILD_EFI
> Please make that a choice menu, so the difference between EFI and non-EFI is clear.
disagree, a choice is unnecessary here when there are only two options. The help text could elaborate though that not selecting the option means it is being built for legacy BIOS
https://review.coreboot.org/c/coreboot/+/83385/comment/8cb660ff_6eec5138?us… :
PS3, Line 121: default n
do we want this to depend on PAYLOAD_EDK2, or do we want it to be able to be built separately?
https://review.coreboot.org/c/coreboot/+/83385/comment/a6583809_da0b4f80?us… :
PS3, Line 123: Build iPXE for EFI target, enabling it to be executed from EDK2.
> Does EDK2 support loading it from CBFS?
no, edk2 has no concept of CBFS, only the FVs (firmware volume) created as part of its build
--
To view, visit https://review.coreboot.org/c/coreboot/+/83385?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: I7f247a59a65aeb18a67475d4d543f519af88aeb9
Gerrit-Change-Number: 83385
Gerrit-PatchSet: 3
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 14:08:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Benjamin Doron, Felix Singer, Filip Lewiński, Krystian Hebel, Lean Sheng Tan, Maciej Pijanowski, Martin L Roth, Michał Kopeć, Michał Żygowski.
Matt DeVillier has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/82721?usp=email )
Change subject: payloads/external/Makefile.mk: build iPXE for EDK2 with custom boot option name
......................................................................
Patch Set 5:
(1 comment)
File payloads/external/edk2/Makefile:
https://review.coreboot.org/c/coreboot/+/82721/comment/52c4ddf3_c30e58eb?us… :
PS3, Line 152: The Dasharo repository has the following additional options:
> Could you follow the similar pattern from MrChromebox to create this kconfig (EDK2_REPO_DASHARO or something similar), and guard this with this kconfig?
I think the last thing we need here is a proliferation of repo-specific options. Guarding with `EDK2_REPO_CUSTOM` in the Kconfig is sufficient. Plus, the MrChromebox fork does support this option as well
--
To view, visit https://review.coreboot.org/c/coreboot/+/82721?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: Ied61c7b8aa7a34261d6c6f7fd089b1affdc7d3f6
Gerrit-Change-Number: 82721
Gerrit-PatchSet: 5
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 14:05:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com>
Attention is currently required from: Bao Zheng, Jason Nien, Martin Roth, Matt DeVillier, Zheng Bao.
Matt DeVillier has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/83646?usp=email )
Change subject: mb/google/skyrim: Combine the variants function
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/skyrim/variants/baseboard/port_descriptors_combo.c:
PS1:
having two port_descriptors files in the baseboard doesn't make much sense to me. I would recommend combining into a single file, dropping the weak implementation, and using a Kconfig to select 'DXIO_DESCRIPTORS_EMMC_DEFAULT' (or something to that effect) for Markarth/Winterhold to select the variant implementation of the function
--
To view, visit https://review.coreboot.org/c/coreboot/+/83646?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: I981e9c52c8e5fa32296e2e43be47411557133691
Gerrit-Change-Number: 83646
Gerrit-PatchSet: 1
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 13:59:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Saurabh Mishra 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 16:
(3 comments)
File src/soc/intel/pantherlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83354/comment/87e663b8_61d3d4fc?us… :
PS8, Line 44: 0xfa000000
> > Due to PTL using config blocks (intermediality, UPD based FSP will be relased soon) to update to […]
Hi Subrata, agree on this. I have corrected to DCACHE size to use UPD based FSP-M/S settings.
https://review.coreboot.org/c/coreboot/+/83354/comment/08f61b5e_7c81a45c?us… :
PS8, Line 47: 0x200000
> > Due to PTL using config blocks (intermediality, UPD based FSP will be relased soon) to update to F […]
Sure, restored.
https://review.coreboot.org/c/coreboot/+/83354/comment/c777a497_f45a9192?us… :
PS8, Line 63: 0x80100
> > Due to PTL using config blocks (intermediality, UPD based FSP will be relased soon) to update to F […]
Changes done as suggested.
--
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: 16
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: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(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: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 13:42:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83354?usp=email
to look at the new patch set (#16).
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock
......................................................................
soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock
List of changes:
1. Add required Pather Lake SoC programming till bootblock.
2. Include only required headers into include/soc.
3. Include PTL related DID, BDF.
4. Ref: Processor EDS documents
vol0.51 #815002
BUG=b:348678529
TEST=verified on Panther Lake Simics PSS using google/fatcat mainboard.
Change-Id: Ibcfe71eec27cebf04f10ec343a73dd92f1272aca
Signed-off-by: Saurabh Mishra <mishra.saurabh(a)intel.com>
---
A src/soc/intel/pantherlake/Kconfig
A src/soc/intel/pantherlake/Makefile.mk
A src/soc/intel/pantherlake/bootblock/bootblock.c
A src/soc/intel/pantherlake/bootblock/pcd.c
A src/soc/intel/pantherlake/bootblock/report_platform.c
A src/soc/intel/pantherlake/espi.c
A src/soc/intel/pantherlake/include/soc/bootblock.h
A src/soc/intel/pantherlake/include/soc/iomap.h
A src/soc/intel/pantherlake/include/soc/p2sb.h
A src/soc/intel/pantherlake/include/soc/pci_devs.h
A src/soc/intel/pantherlake/include/soc/pcr_ids.h
A src/soc/intel/pantherlake/include/soc/pm.h
A src/soc/intel/pantherlake/include/soc/smbus.h
A src/soc/intel/pantherlake/include/soc/soc_info.h
A src/soc/intel/pantherlake/soc_info.c
15 files changed, 1,261 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/83354/16
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibcfe71eec27cebf04f10ec343a73dd92f1272aca
Gerrit-Change-Number: 83354
Gerrit-PatchSet: 16
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: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(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: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Attention is currently required from: Filip Lewiński, Krystian Hebel, Maciej Pijanowski, Martin L Roth, Michał Kopeć.
Felix Singer has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/83385?usp=email )
Change subject: payloads/external/iPXE/Makefile: Build iPXE for EFI target if requested
......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83385/comment/9d513b86_c0a1fecd?us… :
PS3, Line 7: Build iPXE for EFI target if requested
Suggestion:
Allow building EFI target
https://review.coreboot.org/c/coreboot/+/83385/comment/52abfe53_0c1f8642?us… :
PS3, Line 7: /Makefile
Remove "Makefile" from the title.
File payloads/external/iPXE/Kconfig:
https://review.coreboot.org/c/coreboot/+/83385/comment/4b5a591d_6a6fb312?us… :
PS3, Line 119: config IPXE_BUILD_EFI
Please make that a choice menu, so the difference between EFI and non-EFI is clear.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83385?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: I7f247a59a65aeb18a67475d4d543f519af88aeb9
Gerrit-Change-Number: 83385
Gerrit-PatchSet: 3
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 13:36:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Saurabh Mishra 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 15:
(4 comments)
File src/soc/intel/pantherlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83354/comment/827cf58e_03280c24?us… :
PS14, Line 25: select HAVE_X86_64_SUPPORT
> please use the order ?
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/1726299a_fd37b7b3?us… :
PS14, Line 32: Choose this option if your mainboard has a PTL-U (15W)
: or PTL-H 4Xe3 / 12Xe3 (25W) SoC.
> Choose this option if your mainboard has a PTL-UH SoC.
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/c758d640_ec2c37e7?us… :
PS14, Line 33: PTL-H 4Xe3
> this is pure PTL-H and not part of the PTL-UH
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/dd937367_80a05672?us… :
PS14, Line 67: ~1KiB)
> 32KiB?
Corrected to ~1KiB size.
--
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: 15
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: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(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: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 13:34:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Saurabh Mishra 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 15:
(19 comments)
File src/soc/intel/pantherlake/espi.c:
https://review.coreboot.org/c/coreboot/+/83354/comment/226f08ba_55c8edfb?us… :
PS14, Line 27: #if ENV_RAMSTAGE
> I believe this is not required. […]
Acknowledged
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/e77768d1_188334f4?us… :
PS14, Line 10: */
> Please add a new line here.
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/81e66d83_805fa979?us… :
PS14, Line 59: /*
> need white space around
Acknowledged
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/39febaca_9a6d24fb?us… :
PS8, Line 37: fc800000
> > Acknowledged […]
We can close this comment query? since no DMI3BAR for PTL Mobile.
https://review.coreboot.org/c/coreboot/+/83354/comment/d73b07f7_933b7488?us… :
PS8, Line 74: 0x3fff0800000
> > IOM should be anywhere of size 64KB. […]
Let me verify with FSP and come back on this.
File src/soc/intel/pantherlake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/a916b8c9_a077f223?us… :
PS14, Line 9: #if !defined(__SIMPLE_DEVICE__)
> Suggestion: […]
Keeping as it is, based on previous SOC platform.
https://review.coreboot.org/c/coreboot/+/83354/comment/37b531c0_4ff9bb4d?us… :
PS14, Line 165: #define PCI_DEVFN_PCIE7 _PCI_DEVFN(PCIE_1, 6)
> Please fix spaces.
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/54e5fc8d_742baa1b?us… :
PS14, Line 174:
> need two more space
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/e6387661_087d6f39?us… :
PS14, Line 211: #endif
> Suggestion: For such nested endif, please add a comment for which #if, for better readability.
Added in line:219
https://review.coreboot.org/c/coreboot/+/83354/comment/d6d2c217_93c7f828?us… :
PS14, Line 219: #endif
> Same as line 211 above.
Acknowledged
File src/soc/intel/pantherlake/include/soc/pcr_ids.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/5ef84a87_916b088f?us… :
PS14, Line 6: /*
: * Port ids
: */
> These multiline comments at file header follows is seen in earlier soc files as well, but if we use […]
Acknowledged
File src/soc/intel/pantherlake/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/81d9557b_8b375837?us… :
PS14, Line 80: #define GPE_STS_RSVD GPE_STD
> Do we need duplicating macros?
Hi Ashish, currently the macro is not used anywhere for PTL. Keeping it for future supports.
https://review.coreboot.org/c/coreboot/+/83354/comment/74843496_7aeaed9c?us… :
PS14, Line 97: #define PME_B0_EN_BIT 13
> If this macro is unused anywhere else, we could remove this and follow same (1 << x) as earlier line […]
Hi Ashish, currently the macro is not used anywhere for PTL. Keeping for future extensions for wake support.
File src/soc/intel/pantherlake/include/soc/soc_info.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/816a9884_dc999f8d?us… :
PS14, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
> please give one new line
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/03c2b562_11919f5d?us… :
PS14, Line 5: enum {
: NOT_DETECTED = 0,
: PTLU,
: PTLH,
: };
> why needed ?
Removed and followed MTL refactoring for PTL.
81111: soc/intel/mtl: Improve functions in soc_info.c | https://review.coreboot.org/c/coreboot/+/81111
File src/soc/intel/pantherlake/soc_info.c:
https://review.coreboot.org/c/coreboot/+/83354/comment/844512fd_9a92c38e?us… :
PS14, Line 2:
> ``` […]
Removed.
https://review.coreboot.org/c/coreboot/+/83354/comment/c8a499fd_14072bdd?us… :
PS14, Line 10: get_soctype
> why do we need this ? which problem this new API will solve. […]
Removed.
https://review.coreboot.org/c/coreboot/+/83354/comment/1f36fa24_88c291bf?us… :
PS14, Line 69: printk(BIOS_DEBUG, "soc_info: max_pcie_clock:%d\n", pcie_clock);
> why we need these prints ?
Acknowledged
https://review.coreboot.org/c/coreboot/+/83354/comment/2b98cb7b_e3bc0c90?us… :
PS14, Line 92: }
> can you follow mtl soc_info. […]
Removed and followed MTL refactoring for PTL.
81111: soc/intel/mtl: Improve functions in soc_info.c | https://review.coreboot.org/c/coreboot/+/81111
--
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: 15
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: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(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: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Thu, 25 Jul 2024 13:24:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>