Attention is currently required from: Jakub Czapiga, Yu-Ping Wu.
Julius Werner has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/83765?usp=email )
Change subject: commonlib/bsd: Add strcat() and strncat() functions
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83765?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: If02fce0eafb4f6fa01d8bab17d87a32360f4ac83
Gerrit-Change-Number: 83765
Gerrit-PatchSet: 6
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 19:29:37 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Jakub Czapiga, Yu-Ping Wu.
Julius Werner has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/83830?usp=email )
Change subject: commonlib/bsd: Add strlen() and strnlen() functions
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83830?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: I1203ec9affabe493bd14b46662d212b08240cced
Gerrit-Change-Number: 83830
Gerrit-PatchSet: 2
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 19:28:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Felix Held, Felix Singer, Fred Reitberger, Jason Glenesk, Jeff Daly, Johnny Lin, Jonathan Zhang, Lance Zhao, Lean Sheng Tan, Matt DeVillier, Nico Huber, Patrick Rudolph, Raul Rangel, Tim Chu, Tim Wawrzynczak, Vanessa Eusebio.
Martin Roth has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/76128?usp=email )
Change subject: acpi: Move HPET generation to a common location
......................................................................
Patch Set 13:
(2 comments)
File src/acpi/Kconfig:
https://review.coreboot.org/c/coreboot/+/76128/comment/3bbd14d3_7073a419?us… :
PS13, Line 134: depends on HAVE_ACPI_TABLES
This doesn't apply if it's enabled with a select. The select forces it on. If you want this to matter, you'd set it to defaulting it to on in each place it's being selected now.
File src/southbridge/intel/i82371eb/Kconfig:
https://review.coreboot.org/c/coreboot/+/76128/comment/7df2600f_589c00a6?us… :
PS13, Line 6: select ACPI_HPET if HAVE_ACPI_TABLES
It seems like either "if HAVE_ACPI_TABLES" should be needed in more places or we should set it to ignore ACPI_HPET if HAVE_ACPI_TABLES is disabled so that the if isn't needed here.
It just seems odd that this is the only place it would be needed.
--
To view, visit https://review.coreboot.org/c/coreboot/+/76128?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: I38b9962a8d568267356256a3edc51348b9417e20
Gerrit-Change-Number: 76128
Gerrit-PatchSet: 13
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 18:54:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Krystian Hebel, Michał Kopeć, Michał Żygowski, Sergii Dmytruk.
Maximilian Brune has posted comments on this change by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/82639?usp=email )
Change subject: mb/qemu-{i440fx,q35}: reduce default ROM size to 8 MiB
......................................................................
Patch Set 6: Code-Review+2
(1 comment)
File src/mainboard/emulation/qemu-q35/Kconfig:
https://review.coreboot.org/c/coreboot/+/82639/comment/e5b80e4d_ec200e14?us… :
PS6, Line 13: select BOARD_ROMSIZE_KB_8192
> It is still possible, just add `CONFIG_COREBOOT_ROMSIZE_KB_16384=y` to defconfig. […]
My bad. I didn't realize we already have user configurable configurable `CONFIG_COREBOOT_ROMSIZE_KB_...` options for that.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82639?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: If36cb754a8e75e23bce49ff568dd88e5db279bb8
Gerrit-Change-Number: 82639
Gerrit-PatchSet: 6
Gerrit-Owner: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 18:45:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Attention is currently required from: Maximilian Brune, Michał Kopeć, Michał Żygowski, Sergii Dmytruk.
Krystian Hebel has posted comments on this change by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/82639?usp=email )
Change subject: mb/qemu-{i440fx,q35}: reduce default ROM size to 8 MiB
......................................................................
Patch Set 6:
(1 comment)
File src/mainboard/emulation/qemu-q35/Kconfig:
https://review.coreboot.org/c/coreboot/+/82639/comment/a4674d2c_54975770?us… :
PS6, Line 13: select BOARD_ROMSIZE_KB_8192
> I regularly use QEMU-Q35 in combination with Linuxboot. […]
It is still possible, just add `CONFIG_COREBOOT_ROMSIZE_KB_16384=y` to defconfig. `CONFIG_CBFS_SIZE` will be set automatically to 16MB as well, unless specified otherwise in defconfig.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82639?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: If36cb754a8e75e23bce49ff568dd88e5db279bb8
Gerrit-Change-Number: 82639
Gerrit-PatchSet: 6
Gerrit-Owner: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 18:41:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Attention is currently required from: Krystian Hebel, Michał Kopeć, Michał Żygowski, Sergii Dmytruk.
Maximilian Brune has posted comments on this change by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/82639?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/qemu-{i440fx,q35}: reduce default ROM size to 8 MiB
......................................................................
Patch Set 6:
(1 comment)
File src/mainboard/emulation/qemu-q35/Kconfig:
https://review.coreboot.org/c/coreboot/+/82639/comment/05bfc339_c55fe577?us… :
PS6, Line 13: select BOARD_ROMSIZE_KB_8192
I regularly use QEMU-Q35 in combination with Linuxboot. 8MB is not enough space for that (at least not without serious effort).
As far as I understood it 16MB is still possible using the `max-fw-size` option and for the QEMU `-bios` option 16MB also works fine. So I would at least like to have a Kconfig option that lets me choose between 8MB and 16MB. I don't mind defaulting to 8MB though as long as I can set an option in my defconfig to keep using 16MB.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82639?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: If36cb754a8e75e23bce49ff568dd88e5db279bb8
Gerrit-Change-Number: 82639
Gerrit-PatchSet: 6
Gerrit-Owner: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 18:34:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Varun Upadhyay.
Hello Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Varun Upadhyay,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83898?usp=email
to look at the new patch set (#2).
Change subject: soc/intel/alderlake: Correct ISH partition availability check
......................................................................
soc/intel/alderlake: Correct ISH partition availability check
The previous implementation incorrectly assumed that the presence of a
UFS device implied the availability of the ISH partition. This is not
always true, especially on Alder Lake platforms where ISH may be
enabled by default even without UFS.
This patch fixes the issue by directly checking for the presence of the
ISH device to determine if the ISH partition is available.
BUG=b:359440547
TEST=1. Able to dump the ISH version with UFS device:
```
tirwen-rev3 ~ # cbmem -c -1 | grep ISH
[DEBUG] ISH version: 5.4.2.7780
```
2. Able to dump the ISH version with eMMC device:
```
trulo-rev1 ~ # cbmem -c | grep ISH
[DEBUG] ISH version: 5.4.2.7780
```
Change-Id: I411e36606c0697f91050af40e0636f7c64810e95
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/chip.c
1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/83898/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83898?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: I411e36606c0697f91050af40e0636f7c64810e95
Gerrit-Change-Number: 83898
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Varun Upadhyay <varun.upadhyay(a)intel.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Varun Upadhyay <varun.upadhyay(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83898?usp=email )
Change subject: soc/intel/alderlake: Correct ISH partition availability check
......................................................................
soc/intel/alderlake: Correct ISH partition availability check
The previous implementation incorrectly assumed that the presence of a UFS
device implied the availability of the ISH partition. This is not always true,
especially on Alder Lake platforms where ISH may be enabled by default even
without UFS.
This patch fixes the issue by directly checking for the presence of the ISH
device to determine if the ISH partition is available.
BUG=b:359440547
TEST=Able to dump the ISH version with UFS device
tirwen-rev3 ~ # cbmem -c -1 | grep ISH
[DEBUG] ISH version: 5.4.2.7780
Able to dump the ISH version with eMMC device
trulo-rev1 ~ # cbmem -c | grep ISH
[DEBUG] ISH version: 5.4.2.7780
Change-Id: I411e36606c0697f91050af40e0636f7c64810e95
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/chip.c
1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/83898/1
diff --git a/src/soc/intel/alderlake/chip.c b/src/soc/intel/alderlake/chip.c
index 45fb39f..8979ae0f6 100644
--- a/src/soc/intel/alderlake/chip.c
+++ b/src/soc/intel/alderlake/chip.c
@@ -167,16 +167,16 @@
/*
* SoC override API to identify if ISH Firmware existed inside CSE FPT.
*
- * SoC with UFS enabled would like to keep ISH enabled as well, hence
- * identifying the UFS enabled device is enough to conclude that the ISH
- * partition also is available.
+ * Identifying the ISH enabled device is required to conclude that the ISH
+ * partition also is available (because ISH may be default enabled for non-UFS
+ * platforms as well starting with Alder Lake).
*/
bool soc_is_ish_partition_enabled(void)
{
- struct device *ufs = pcidev_path_on_root(PCH_DEVFN_UFS);
- uint16_t ufs_pci_id = ufs ? pci_read_config16(ufs, PCI_DEVICE_ID) : 0xFFFF;
+ struct device *ish = pcidev_path_on_root(PCH_DEVFN_ISH);
+ uint16_t ish_pci_id = ish ? pci_read_config16(ish, PCI_DEVICE_ID) : 0xFFFF;
- if (ufs_pci_id == 0xFFFF)
+ if (ish_pci_id == 0xFFFF)
return false;
return true;
--
To view, visit https://review.coreboot.org/c/coreboot/+/83898?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I411e36606c0697f91050af40e0636f7c64810e95
Gerrit-Change-Number: 83898
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Jakub Czapiga, Julius Werner, Yu-Ping Wu.
Maximilian Brune has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/83830?usp=email )
Change subject: commonlib/bsd: Add strlen() and strnlen() functions
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
File src/commonlib/bsd/string.c:
https://review.coreboot.org/c/coreboot/+/83830/comment/6b2c4fd2_df595fc3?us… :
PS1, Line 12: return len;
> Done
just for the record:
On RISC-V this also results in one less jump (and also avoids the extra counter).
https://review.coreboot.org/c/coreboot/+/83830/comment/a1f6b5b0_ba8d8807?us… :
PS1, Line 20: return len;
> Done
just for the record:
On RISC-V this also results in one less jump (and also avoids the extra counter).
--
To view, visit https://review.coreboot.org/c/coreboot/+/83830?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: I1203ec9affabe493bd14b46662d212b08240cced
Gerrit-Change-Number: 83830
Gerrit-PatchSet: 2
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 18:01:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jérémy Compostella, Kapil Porwal, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra, Subrata Banik, Wonkyu Kim.
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83789?usp=email )
Change subject: soc/intel/ptl: Add GPIOs for Panther Lake SOC
......................................................................
Patch Set 28:
(5 comments)
File src/soc/intel/pantherlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83789/comment/ee847d54_cf351574?us… :
PS24, Line 261: config SOC_INTEL_ACPI_GPIO_PINCTRL_COMPACT
> > when true, only GPP_[group]_[n] pads will be included in pincrtl. […]
Sure. I will remove this flag. Let me contact the pinctrl owner. It does not make sense to me to include those non GPP PADs.
File src/soc/intel/pantherlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/83789/comment/70e34ca7_05aab118?us… :
PS17, Line 38: GPP_ASPIO
> > The GPIO group names come from Intel's GPIO HAS document. GPP_ASPI0 contains GPP_A and SPI0 pads. […]
agree. thx for the explanation! I will make the change.
https://review.coreboot.org/c/coreboot/+/83789/comment/3771fb3d_0e815aa6?us… :
PS17, Line 177: { PMC_GPP_E, GPP_E },
> > will modify the this order accordingly. […]
sure. will make the changes.
File src/soc/intel/pantherlake/include/soc/gpio_defs.h:
https://review.coreboot.org/c/coreboot/+/83789/comment/88b64ea1_bf2ae8df?us… :
PS28, Line 275:
> drop one empty line
Acknowledged
File src/soc/intel/pantherlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/83789/comment/1a599a48_85388d25?us… :
PS28, Line 174: /* 48 */
> Do you genuinely require these comments? I believe that lines #164 and #171 are sufficient for anyon […]
sure. I will remove these comments.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83789?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: Iae1bc072841214efaec7a10719dbc742f2da795b
Gerrit-Change-Number: 83789
Gerrit-PatchSet: 28
Gerrit-Owner: Cliff Huang <cliff.huang(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: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-CC: Ronak Kanabar <ronak.kanabar(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-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Tue, 13 Aug 2024 17:42:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>