Attention is currently required from: Anil Kumar K, Bora Guvendik, Hannah Williams, Jamie Ryu, Kapil Porwal, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra, Subrata Banik, Wonkyu Kim.
Cliff Huang has posted comments on this change by Ravishankar Sarawadi. ( https://review.coreboot.org/c/coreboot/+/83772?usp=email )
Change subject: soc/intel/ptl: Add SoC ACPI directory for Panther Lake
......................................................................
Patch Set 41:
(1 comment)
File src/soc/intel/pantherlake/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/c88ddbca_f35e6014?us… :
PS23, Line 5: INTC1041
> what is the source of this HID ? […]
INTC1041 is used in additional to INT3400 in int3400 driver (https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/head…). However, INTC 1041 is for ADL, for PTL, it should be INTC10D4.
In addition, INTC1046 and INTC1048 are for ADL, too. I will check 34xx driver is used in PTL and make the correction.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83772?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: Ia5cf899b049cb8eb27b4ea30c7f3ce7a14884f15
Gerrit-Change-Number: 83772
Gerrit-PatchSet: 41
Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(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: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
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: 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: Subrata Banik <subratabanik(a)google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(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: 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: Thu, 15 Aug 2024 19:32:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Jakub Czapiga, Jon Murphy, Julius Werner, Karthik Ramasubramanian, Maximilian Brune, Yu-Ping Wu.
Felix Singer has posted comments on this change by Jon Murphy. ( https://review.coreboot.org/c/coreboot/+/83930?usp=email )
Change subject: Revert "commonlib/bsd: Add strlen() and strnlen() functions"
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83930/comment/b37c2f29_47594e64?us… :
PS3, Line 11: Reason for revert: b/359951393 breaking builds affecting many devices
> Tried to add more context. […]
Okay. My point just is, using the numbers in the commit description suggests to the reader that the number is used at coreboot in some way, which is not the case and so it's confusing. It's only used within Google. For people outside of Google it's still not visible what that number is. I mean, I know but others might not.
So, how about using a header line as we have for others like Sign-off or Change-ID? Then it's crystal clear. I think that should work for your case too?
Example:
```
ChromeOS-Bug-ID: b/359951393
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83930?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: Ic145ed88c683ecbb9f7cefa1a40cdf20c4518d03
Gerrit-Change-Number: 83930
Gerrit-PatchSet: 6
Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 19:10:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com>
Attention is currently required from: Felix Held.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/74619?usp=email )
Change subject: include/cpu/amd/mtrr: rename TOP_MEM(2) and remove workaround
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/74619?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: Ibad72dac17bd0b05734709d42c6802b7c8a87455
Gerrit-Change-Number: 74619
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: coreboot org <coreboot.org(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 15 Aug 2024 18:53:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Raul Rangel.
Martin Roth has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/75441?usp=email )
Change subject: soc/amd/common/cpu: Don't assume endianness of host
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
Patchset:
PS1:
Needs manual rebase.
File src/soc/amd/common/block/cpu/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/75441/comment/713deb11_4e204f3a?us… :
PS1, Line 24: hexdump
> I used `od` in https://review.coreboot. […]
The issue is that hexdump isn't posix compliant. I guess the change is no worse than the already committed code though. I'd say let's just merge this and try to fix it later.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75441?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: I16b0e6898ce808f12b8277d0fd6a09ff5c44ca15
Gerrit-Change-Number: 75441
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(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: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 18:42:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Fred Reitberger <reitbergerfred(a)gmail.com>
Attention is currently required from: Maximilian Brune, Yu-Ping Wu.
Julius Werner has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/83914?usp=email )
Change subject: commonlib/bsd/string: Fix pointer overflow for strnlen()
......................................................................
Patch Set 1:
(1 comment)
File src/commonlib/bsd/string.c:
https://review.coreboot.org/c/coreboot/+/83914/comment/1d1787be_bfaad2e2?us… :
PS1, Line 19: str + maxlen + 1
> That's algorithmically incorrect in some edge cases, for example when `str = UINTPTR_MAX` and `maxle […]
Hmm... I don't think anyone should ever call this with a string beginning at the last byte of the address space, that just seems wrong to begin with (and that's the only flaw I can see in my suggestion). Even with your code it would still dereference address 0 to look for the terminating '\0' in that case. (In fact, your code generally dereferences once more byte that specified -- shouldn't the `len < maxlen` check come first?)
But okay, this seems to be breaking a lot of things so let's get at least some fix landed already while we can continue to discuss edge cases.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83914?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: Ic9d983b11391f5e05c2bceb262682aced5206f94
Gerrit-Change-Number: 83914
Gerrit-PatchSet: 1
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 18:16:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Nick Vaccaro, Rishika Raj.
Hello Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83932?usp=email
to look at the new patch set (#4).
Change subject: mb/goog/brya: Don't lock GPP_F15 (FPMCU_INT_L)
......................................................................
mb/goog/brya: Don't lock GPP_F15 (FPMCU_INT_L)
Locking GPP_F15 causes DUTs with fingerprint sensor to not be able to
correctly power down and stay powered down. This pin does not need to
be locked.
BUG=b:359692570, b:356750516
BRANCH=firmware-brya-14505.B
TEST=`FW_NAME=gimble emerge-brya coreboot chromeos-bootimage`, flash and
boot gimble into developer mode, then reboot into dev screen and select
the "Power off" button and verify gimble powers off and does not power
itself back up.
Change-Id: I1c73035b02021b0d1268cd46dcd0841621556ad5
Signed-off-by: Nick Vaccaro <nvaccaro(a)google.com>
---
M src/mainboard/google/brya/variants/baseboard/brya/gpio.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/83932/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/83932?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: I1c73035b02021b0d1268cd46dcd0841621556ad5
Gerrit-Change-Number: 83932
Gerrit-PatchSet: 4
Gerrit-Owner: Nick Vaccaro <nvaccaro(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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: 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: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Attention is currently required from: Felix Singer, Jakub Czapiga, Jon Murphy, Julius Werner, Karthik Ramasubramanian, Yu-Ping Wu.
Subrata Banik has posted comments on this change by Jon Murphy. ( https://review.coreboot.org/c/coreboot/+/83931?usp=email )
Change subject: Revert "commonlib/bsd: Add strcat() and strncat() functions"
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83931?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: I32d79fceca462e363c7d7b17a08d6c60c2bb8aff
Gerrit-Change-Number: 83931
Gerrit-PatchSet: 3
Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.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: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 17:56:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes