coreboot-gerrit June 2024

coreboot-gerrit@coreboot.org

1 participants
3527 discussions

[S] Change in coreboot[main]: soc/intel/xeon_sp: Reserve MMIO range for VTd BAR dynamically
by Nico Huber (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu. Nico Huber has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/82431?usp=email ) Change subject: soc/intel/xeon_sp: Reserve MMIO range for VTd BAR dynamically ...................................................................... Patch Set 19: Code-Review+2 (3 comments) File src/soc/intel/xeon_sp/include/soc/chip_common.h: https://review.coreboot.org/c/coreboot/+/82431/comment/12094a51_cd9265da?us… : PS19, Line 93: uint32_t `size_t` please. File src/soc/intel/xeon_sp/uncore.c: https://review.coreboot.org/c/coreboot/+/82431/comment/1c8cccc8_be13a6c6?us… : PS19, Line 61: uint32_t size = (~(pci_read_config32(dev, VTD_BAR_CSR) & ((uint32_t)(-4 * KiB)))) + 1; This line has some unnecessary parentheses, which is more explicitly but can actually make it harder to read in one go. I'd only use necessary parentheses: ``` uint32_t size = ~(pci_read_config32(dev, VTD_BAR_CSR) & (uint32_t)(-4 * KiB)) + 1; ``` Also, what could help the reader would be to place the mask in a constant and use that here and above, e.g. ``` const uint32_t size_mask = (uint32_t)(-4 * KiB); ``` https://review.coreboot.org/c/coreboot/+/82431/comment/103e9d5e_db7b75e2?us… : PS19, Line 63: pci_write_config32(dev, VTD_BAR_CSR, val); I'd have guessed that the register is actually 64 bit? It wouldn't matter in practice, as we don't expect >=4GiB BARs here. If it is indeed 64 bits, a comment would be nice, though. -- To view, visit https://review.coreboot.org/c/coreboot/+/82431?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: Ie45dd29e386cbfcb136ce2152aba2ec67757ee3c Gerrit-Change-Number: 82431 Gerrit-PatchSet: 19 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:32:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms
by Derek Huang (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Kapil Porwal, Subrata Banik, Tony Huang. Derek Huang has posted comments on this change by Tony Huang. ( https://review.coreboot.org/c/coreboot/+/83206?usp=email ) Change subject: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/83206?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: I22d7800b50f6f7de9f147ae6998a5015d0dc0be9 Gerrit-Change-Number: 83206 Gerrit-PatchSet: 3 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> Gerrit-CC: Robert Chen <robert.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:27:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: acpi: Add support for DRHD size reporting
by Nico Huber (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Angel Pons, Arthur Heymans, Chen, Gang C, Christian Walter, Cliff Huang, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jeff Daly, Jincheng Li, Johnny Lin, Jonathan Zhang, Kapil Porwal, Lance Zhao, Lean Sheng Tan, Nick Vaccaro, Patrick Rudolph, Paul Menzel, Sean Rhodes, Shuo Liu, Subrata Banik, Tarun, Tim Chu, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh. Nico Huber has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/82429?usp=email ) Change subject: acpi: Add support for DRHD size reporting ...................................................................... Patch Set 16: Code-Review+1 (4 comments) File src/acpi/acpi_dmar.c: https://review.coreboot.org/c/coreboot/+/82429/comment/c11ef7a9_ab0f418d?us… : PS16, Line 60: * Refer to chapter 8.3 of https://cdrdv2.intel.com/v1/dl/getContent/671081 This is true for the the whole _drhd functions, maybe place above? Though, I'm not sure why we should mention that explicitly. We don't do it for all the other lines in this file. Also, URLs usually break, so referring to the spec by name (and optionnaly a current URL) would be better. https://review.coreboot.org/c/coreboot/+/82429/comment/c56b71d0_99cabb62?us… : PS16, Line 68: (1 << (drhd->size + 12) That's just `size`, assuming it is a power of 2, if not we already have another bug. Could also use the macro: ``` assert(IS_ALIGNED(bar, size)); ``` and optionally ``` assert(IS_POWER_OF_2(size)); ``` and if we wanted to be really thorough ``` assert(size >= 4*KiB); ``` or all `size` assertions in one line ``` assert(4*KiB <= size && size <= (1 << 15) * 4*KiB && IS_POWER_OF_2(size)); ``` NB. Assertion on the input parameters is usually placed at the top of a function. File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/82429/comment/1714956a_335c309b?us… : PS16, Line 1855: u32 Please use `size_t` for byte sizes. File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/82429/comment/4a6ae4dd_67e1bdec?us… : PS15, Line 1852: unsigned long acpi_create_dmar_drhd(unsigned long current, u8 flags, : u16 segment, u64 bar, u32 size); > To avoid having to touch all platforms, would it make sense to have a specialised acpi_create_dmar_drhd_4k function? Really no strong feelings about this, but if not touching all platforms was the goal, the function with the added parameter could be named something like acpi_create_dmar_drhd_size(). The _4k() is more explicit, though, so I don't mind touching all platforms. -- To view, visit https://review.coreboot.org/c/coreboot/+/82429?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: I49dd5de2eca257a5f6240e36d05755cabca96d1c Gerrit-Change-Number: 82429 Gerrit-PatchSet: 16 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(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: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.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: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:25:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

[M] Change in coreboot[main]: util/autoport: Extend Add_gpl to more files
by Angel Pons (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Nicholas Chin. Angel Pons has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/83184?usp=email ) Change subject: util/autoport: Extend Add_gpl to more files ...................................................................... Patch Set 3: (2 comments) File util/autoport/main.go: https://review.coreboot.org/c/coreboot/+/83184/comment/7efbf6bc_14cb57a6?us… : PS2, Line 882: gma.WriteString(`-- SPDX-License-Identifier: GPL-2.0-or-later > I kind of do, because consistency: pretty much every `gma-mainboard.ads` uses `GPL-2.0-or-later` […] Of course, the function would be renamed to `Add_License` or similar. CB:83185 would also be able to make use of it: ``` func Add_CC_PDDC(f *os.File, filetype Filetype) { Add_License(f, filetype, CC_PDDC) fmt.Fprintln(f) fmt.Fprintf(f, CommentFormatStrings[filetype], "Please update the license if adding licensable material.") } ``` https://review.coreboot.org/c/coreboot/+/83184/comment/3109ab8b_184f789c?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) : } How about adding a general helper function to write comments? ```suggestion func Add_Comment(f *os.File, filetype Filetype, body string) { fmt.Fprintf(f, CommentFormatStrings[filetype], string) } func Add_gpl(f *os.File, filetype Filetype) { Add_Comment(f, filetype, "SPDX-License-Identifier: GPL-2.0-only") fmt.Fprintln(f) } ``` -- 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: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:09:05 +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>

1 0

[XS] Change in coreboot[main]: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms
by Tony Huang (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Derek Huang, Kapil Porwal, Subrata Banik. Tony Huang has posted comments on this change by Tony Huang. ( https://review.coreboot.org/c/coreboot/+/83206?usp=email ) Change subject: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/83206/comment/baac00d7_d013656d?us… : PS2, Line 13: . > `:` Done https://review.coreboot.org/c/coreboot/+/83206/comment/7831f9c6_ee51d74f?us… : PS2, Line 16: . > `:` Done -- To view, visit https://review.coreboot.org/c/coreboot/+/83206?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: I22d7800b50f6f7de9f147ae6998a5015d0dc0be9 Gerrit-Change-Number: 83206 Gerrit-PatchSet: 3 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> Gerrit-CC: Robert Chen <robert.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:06:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Derek Huang <derekhuang(a)google.com>

1 0

[XS] Change in coreboot[main]: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms
by Tony Huang (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Kapil Porwal, Subrata Banik, Tony Huang. Hello Derek Huang, Kapil Porwal, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83206?usp=email to look at the new patch set (#3). Change subject: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms ...................................................................... soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms Some dongles require more time to be ready, this CL extedns the DP mode entry timeout from 0.5s to 1.5s and make sure the tested dongle display works. Before: [WARN ] DP not ready after 500ms. Abort. After: [INFO ] DP ready after 1211 ms BUG=b:348309582 TEST=emerge coreboot verify tested dongles and monitors display works Change-Id: I22d7800b50f6f7de9f147ae6998a5015d0dc0be9 Signed-off-by: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> --- M src/soc/intel/common/block/tcss/tcss.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/83206/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/83206?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: I22d7800b50f6f7de9f147ae6998a5015d0dc0be9 Gerrit-Change-Number: 83206 Gerrit-PatchSet: 3 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> Gerrit-CC: Robert Chen <robert.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>

1 0

[M] Change in coreboot[main]: acpi: Rename acpi_create_dmar_drhd
by Nico Huber (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Cliff Huang, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jeff Daly, Johnny Lin, Jonathan Zhang, Kapil Porwal, Lance Zhao, Lean Sheng Tan, Nick Vaccaro, Patrick Rudolph, Sean Rhodes, Shuo Liu, Subrata Banik, Tarun, Tim Chu, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh. Nico Huber has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/83202?usp=email ) Change subject: acpi: Rename acpi_create_dmar_drhd ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/83202?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: Ic0a0618aa8e46d3fec2ceac7a91742122993df91 Gerrit-Change-Number: 83202 Gerrit-PatchSet: 1 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: 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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.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: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.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: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:05:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms
by Derek Huang (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Kapil Porwal, Subrata Banik, Tony Huang. Derek Huang has posted comments on this change by Tony Huang. ( https://review.coreboot.org/c/coreboot/+/83206?usp=email ) Change subject: soc/intel/common: Extend WAIT_FOR_DP_MODE_ENTRY_TIMEOUT_MS to 1500ms ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/83206/comment/7a103f92_de592fed?us… : PS2, Line 13: . `:` https://review.coreboot.org/c/coreboot/+/83206/comment/60fa5fca_649400d2?us… : PS2, Line 16: . `:` -- To view, visit https://review.coreboot.org/c/coreboot/+/83206?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: I22d7800b50f6f7de9f147ae6998a5015d0dc0be9 Gerrit-Change-Number: 83206 Gerrit-PatchSet: 2 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> Gerrit-CC: Robert Chen <robert.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:05:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: util/autoport: Extend Add_gpl to more files
by Angel Pons (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Nicholas Chin. Angel Pons has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/83184?usp=email ) Change subject: util/autoport: Extend Add_gpl to more files ...................................................................... Patch Set 3: (1 comment) File util/autoport/main.go: https://review.coreboot.org/c/coreboot/+/83184/comment/30571138_3c0abb1e?us… : PS2, Line 882: gma.WriteString(`-- SPDX-License-Identifier: GPL-2.0-or-later > I made a note about it in the commit message. […] I kind of do, because consistency: pretty much every `gma-mainboard.ads` uses `GPL-2.0-or-later` How about parametrising the license name? You can use an enum: ``` type License string const ( GPL2_only License = "GPL-2.0-only" GPL2_or_later = "GPL-2.0-or-later" CC_PDDC = "CC-PDDC" MIT = "MIT" BSD2 = "BSD-2-Clause" BSD2_patent = "BSD-2-Clause-Patent" BSD3 = "BSD-3-Clause" ) ``` We may not need all of these, but I added them for the sake of example -- 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: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 09:03:23 +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>

1 0

[S] Change in coreboot[main]: acpi: Add support for DRHD size reporting
by Angel Pons (Code Review) 25 Jun '24

25 Jun '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, Christian Walter, Cliff Huang, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jeff Daly, Jincheng Li, Johnny Lin, Jonathan Zhang, Kapil Porwal, Lance Zhao, Lean Sheng Tan, Nick Vaccaro, Nico Huber, Patrick Rudolph, Paul Menzel, Sean Rhodes, Shuo Liu, Subrata Banik, Tarun, Tim Chu, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh. Angel Pons has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/82429?usp=email ) Change subject: acpi: Add support for DRHD size reporting ...................................................................... Patch Set 16: Code-Review+1 (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/82429/comment/0c35a242_46978c17?us… : PS15, Line 11: the register set is 2^N 4 KB pages. > When N is zero, the size would be 2^0 4KB pages, a.k.a. […] Ack, thanks for confirming. File src/acpi/acpi_dmar.c: https://review.coreboot.org/c/coreboot/+/82429/comment/99217151_f2d1cbe6?us… : PS16, Line 42: u16 segment, u64 bar) nit: ```suggestion u16 segment, u64 bar) ``` https://review.coreboot.org/c/coreboot/+/82429/comment/e82b9b96_d9f63166?us… : PS16, Line 44: 1 Since you kept the `size` parameter in bytes: ```suggestion return acpi_create_dmar_drhd(current, flags, segment, bar, 4 * KiB); ``` https://review.coreboot.org/c/coreboot/+/82429/comment/b5080575_2c65d4ca?us… : PS16, Line 48: u16 segment, u64 bar, u32 size) nit: ```suggestion u16 segment, u64 bar, u32 size) ``` File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/82429/comment/301de44b_be82648f?us… : PS15, Line 1852: unsigned long acpi_create_dmar_drhd(unsigned long current, u8 flags, : u16 segment, u64 bar, u32 size); > I made some attempts between using npages or size, and didn't get clear benefit of using npages bec […] Hmmm, okay, let's keep it as-is for now. It's not too bad given that existing code now uses `acpi_create_dmar_drhd_4k` -- To view, visit https://review.coreboot.org/c/coreboot/+/82429?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: I49dd5de2eca257a5f6240e36d05755cabca96d1c Gerrit-Change-Number: 82429 Gerrit-PatchSet: 16 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(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: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.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: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Comment-Date: Tue, 25 Jun 2024 08:53:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2024