Attention is currently required from: Martin L Roth, Nicholas Chin.
Hello Martin L Roth, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83185?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+2 by Martin L Roth, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: util/autoport: Add CC-PDDC SPDX header to empty files
......................................................................
util/autoport: Add CC-PDDC SPDX header to empty files
As per commit cf4722d317ea (src/mb: Update unlicensable files with the
CC-PDDC SPDX ID) effectively empty files should use the the Creative
Commons Public Domain Dedication and Certification (CC-PDDC) license
header.
The only empty files that autoport currently generates are ec.asl and
superio.asl on non-laptop systems, where NoEC() is used.
Change-Id: I5fdf0c80443a79c082b3d0418d16cd858782b2c9
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
---
M util/autoport/ec_none.go
M util/autoport/main.go
2 files changed, 10 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/83185/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/83185?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: I5fdf0c80443a79c082b3d0418d16cd858782b2c9
Gerrit-Change-Number: 83185
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Hung-Te Lin, Jarried Lin.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83928?usp=email )
Change subject: soc/mediatek/mt8196: Fix timer reset in BL31
......................................................................
Patch Set 23:
(1 comment)
Patchset:
PS23:
gental ping
--
To view, visit https://review.coreboot.org/c/coreboot/+/83928?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: I520986b81ca153ec3ce56558a80619448cfc0c59
Gerrit-Change-Number: 83928
Gerrit-PatchSet: 23
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Zhanzhan Ge <zhanzhan.ge(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 01:15:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Hung-Te Lin, Jarried Lin.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84025?usp=email )
Change subject: soc/mediatek/mt8196: Add unmask eint event for bootblock
......................................................................
Patch Set 14:
(1 comment)
Patchset:
PS14:
gental ping
--
To view, visit https://review.coreboot.org/c/coreboot/+/84025?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: I4bf3820a89172186b8f51591f8760787affbb7a3
Gerrit-Change-Number: 84025
Gerrit-PatchSet: 14
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 01:14:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Angel Pons.
Nicholas Chin has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/83184?usp=email )
Change subject: util/autoport: Make printing of SPDX headers generic
......................................................................
Patch Set 3:
(2 comments)
File util/autoport/main.go:
https://review.coreboot.org/c/coreboot/+/83184/comment/d28612d5_4103da9a?us… :
PS2, Line 882: gma.WriteString(`-- SPDX-License-Identifier: GPL-2.0-or-later
> Of course, the function would be renamed to `Add_License` or similar. […]
Done
https://review.coreboot.org/c/coreboot/+/83184/comment/63103b8e_9bef77b0?us… :
PS2, Line 232: func Add_gpl(f *os.File, filetype Filetype) {
: fmt.Fprintf(f, CommentFormatStrings[filetype],
: "SPDX-License-Identifier: GPL-2.0-only")
: fmt.Fprintln(f)
: }
> Acknowledged. There is already a `makeComment()` function in `main. […]
Done. Left makeComment as is for now and will probably drop it and change the Kconfig comment handling in a separate commit.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83184?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: I24a1ccd0afb7045e878bf6eaae7a23f828a9240d
Gerrit-Change-Number: 83184
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 01:09:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Angel Pons.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83184?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: util/autoport: Make printing of SPDX headers generic
......................................................................
util/autoport: Make printing of SPDX headers generic
Previously, Add_gpl() was only used with C and ASL source code files,
and was hard coded to use the C /* */ style comment, preventing it from
being used with files with other comment styles. Convert this into a
generic function for adding arbitrary SPDX license identifiers for
arbitrary filetypes. This replaces the hard coded GPL-2.0-or-later
string used in gma-mainboard.ads with a call to the new function.
This is also used to add SPDX headers to Kconfig and Makefile sources;
as previous commits added them to all such files in the tree.
Tested against logs from a Latitude E6430 (Ivy Bridge) and Precision
M6800 (Haswell) to check that license headers that were already being
generated did not change.
Change-Id: I24a1ccd0afb7045e878bf6eaae7a23f828a9240d
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
---
M util/autoport/azalia.go
M util/autoport/bd82x6x.go
M util/autoport/ec_fixme.go
M util/autoport/ec_lenovo.go
M util/autoport/ec_none.go
M util/autoport/gpio_common.go
M util/autoport/lynxpoint.go
M util/autoport/lynxpoint_lp_gpio.go
M util/autoport/main.go
9 files changed, 55 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/83184/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/83184?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: I24a1ccd0afb7045e878bf6eaae7a23f828a9240d
Gerrit-Change-Number: 83184
Gerrit-PatchSet: 4
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Jérémy Compostella, Paul Menzel, Subrata Banik.
Wonkyu Kim has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84210?usp=email )
Change subject: intel/cmm/block: use CONFIG_SMM_TSEG_SIZE in sa_get_tseg_size function
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/84210/comment/4af13985_8620136b?us… :
PS2, Line 155: sa_get_gsm_base() - sa_get_tseg_base()
> Would an assert be useful?
Assert may not help as gsm base can be not programmed if internal graphic IP is disabled.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84210?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: Ic3d5175add1f7dc6e2c9f1c38133b36ffc59e789
Gerrit-Change-Number: 84210
Gerrit-PatchSet: 3
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(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: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 10 Sep 2024 00:26:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Jérémy Compostella, Paul Menzel, Subrata Banik.
Wonkyu Kim has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84210?usp=email )
Change subject: intel/cmm/block: use CONFIG_SMM_TSEG_SIZE in sa_get_tseg_size function
......................................................................
Patch Set 2:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84210/comment/47b7b21c_bf06b1b1?us… :
PS2, Line 7: src/
> remove `src/`
Done
https://review.coreboot.org/c/coreboot/+/84210/comment/8726788b_af7ed5a5?us… :
PS2, Line 7: Update sa_get_tseg_size function
> Please be more specific.
Done
https://review.coreboot.org/c/coreboot/+/84210/comment/5deeb6c1_3607115f?us… :
PS2, Line 10: api
> API
Done
https://review.coreboot.org/c/coreboot/+/84210/comment/46ebf1b5_53073bdd?us… :
PS2, Line 10: nagative
> negative
Done
File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/84210/comment/008e022d_a14fc1b6?us… :
PS2, Line 155: return sa_get_gsm_base() - sa_get_tseg_base();
> WDYT? I assume in absence of IGD the GSM size would be zero hence, we won't get into `if` case. […]
We checked internally and got recommendation not to calculate it as CONFIG_SMM_TSEG_SIZE is passed to FSP and memory map which based on calculation can be changed later.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84210?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: Ic3d5175add1f7dc6e2c9f1c38133b36ffc59e789
Gerrit-Change-Number: 84210
Gerrit-PatchSet: 2
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(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: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 10 Sep 2024 00:19:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Pratikkumar V Prajapati, Subrata Banik.
Wonkyu Kim has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84228?usp=email )
Change subject: src/device: Add more condition to check valid PCI device id
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84228/comment/1c740478_620e5db4?us… :
PS2, Line 12: Below are invalid PCI device id cases
: VID: 0x0 or 0xffff
: DID: 0x0 or 0xffff
> > Yes, there is issue in valid VID with 0x0 DID in PSS environment. […]
I think it does not harm to add more condition and DID 0x0 and 0xffff are not valid . Do you have concern for performance by checking more? Or do you think DID 0x0 or 0xffff can be valid case?
If we're worried about performance issue, we can add Kconfig to check more like this.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84228?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: Iffabc9037a8af1b2a4ffebdf30199c4f6eae9540
Gerrit-Change-Number: 84228
Gerrit-PatchSet: 2
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(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: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(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: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 00:09:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Jérémy Compostella, Subrata Banik.
Wonkyu Kim has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84229?usp=email )
Change subject: libpayload: add more condition to check valid PCI device id
......................................................................
Patch Set 3:
(5 comments)
File payloads/libpayload/drivers/usb/usbinit.c:
https://review.coreboot.org/c/coreboot/+/84229/comment/547604e1_326c412c?us… :
PS3, Line 126: /* Check if there's a device here at all. */
: if (pci_read_config32(pci_device, REG_VENDOR_ID) == 0xffffffff)
> please specify the device where you have ran into this issue. […]
This is checking bad hardware to avoid hang issue.
And current condition is not even checking 0x0 VID.
I think it's better to check valid both VID and DID and not enumerate the PCI ID.
https://review.coreboot.org/c/coreboot/+/84229/comment/6406b33b_3c9cc0ac?us… :
PS3, Line 119: int bus, int dev, int fun
> Why can't it take a `pcidev_t` ?
We can use it as parameter.
https://review.coreboot.org/c/coreboot/+/84229/comment/0f4b995d_03f29fad?us… :
PS3, Line 130: (did == 0x0000) || (did == 0xffff)) {
> It should fit in one line.
Acknowledged
https://review.coreboot.org/c/coreboot/+/84229/comment/b25f4a4b_f0982022?us… :
PS3, Line 141:
> why ?
Acknowledged
https://review.coreboot.org/c/coreboot/+/84229/comment/df58fcb2_6f1ce982?us… :
PS3, Line 147: if ( is_valid_pci_dev(bus, dev, 0) == false)
> The implementation is other parts of the lib is as follow: […]
This is also limited checking for VID 0x0 and 0xFFFFFF.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84229?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: I772c4199c7a6c13a25590a36f1bfee17c1a44daf
Gerrit-Change-Number: 84229
Gerrit-PatchSet: 3
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(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: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 00:01:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>