coreboot-gerrit July 2024

coreboot-gerrit@coreboot.org

1 participants
3025 discussions

[S] Change in coreboot[main]: i2c/drivers/generic: Add support for including a CDM
by Angel Pons (Code Review) 29 Jul '24

29 Jul '24

Attention is currently required from: Matt DeVillier, Paul Menzel, Sean Rhodes. Angel Pons has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email ) Change subject: i2c/drivers/generic: Add support for including a CDM ...................................................................... Patch Set 9: (1 comment) File src/drivers/i2c/generic/chip.h: https://review.coreboot.org/c/coreboot/+/81409/comment/23d1cfc5_32f33fe5?us… : PS9, Line 98: bool has_cdm; : int cdm_index; > What's the reason for specifying the values? Redundancy. I wasn't feeling great when writing that. I would keep `CDM_NOT_PRESENT = 0,` but the rest of numbers can be omitted. -- To view, visit https://review.coreboot.org/c/coreboot/+/81409?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: If70c25288d835df7064b4051c43abeb2d6531f3b Gerrit-Change-Number: 81409 Gerrit-PatchSet: 9 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Sun, 28 Jul 2024 20:04:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[M] Change in coreboot[main]: intel/alderlake: Add helper functions for Power Management
by Sean Rhodes (Code Review) 29 Jul '24

29 Jul '24

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eric Lai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Nico Huber, Paul Menzel, Subrata Banik. Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email ) Change subject: intel/alderlake: Add helper functions for Power Management ...................................................................... Patch Set 23: (4 comments) File src/soc/intel/alderlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/81638/comment/7299afa8_3f0e43ab?us… : PS22, Line 494: unsigned int > `enum ASPM_control` to make it clear we're converting from that to something else. […] Done https://review.coreboot.org/c/coreboot/+/81638/comment/099eaa41_43685a95?us… : PS22, Line 507: unsigned int > `enum ... […] Done File src/soc/intel/common/block/include/intelblocks/pcie_rp.h: https://review.coreboot.org/c/coreboot/+/81638/comment/8f3b0315_a4d6ebfc?us… : PS22, Line 40: are zero-indexed > To me this doesn't explain much, our enums are also zero-indexed. I'd rather […] Done https://review.coreboot.org/c/coreboot/+/81638/comment/d6a79742_fee07d14?us… : PS22, Line 45: FSP_DEFAULT > Probably for another patch: […] Technically right for all other SOCs atm - will do this, then move to common code and tidy -- To view, visit https://review.coreboot.org/c/coreboot/+/81638?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: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 23 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <ericllai(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 28 Jul 2024 19:35:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>

1 0

[S] Change in coreboot[main]: soc/intel/alderlake: Hook up PCI Power Management to option API
by Sean Rhodes (Code Review) 29 Jul '24

29 Jul '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik. Hello Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81906?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/intel/alderlake: Hook up PCI Power Management to option API ...................................................................... soc/intel/alderlake: Hook up PCI Power Management to option API Hook up PCICLK_PM, PCIEXP_ASPM and PCIEXP_L1_SUBSTATE to the option API. Change-Id: I2b06a7c734a4fd4073e86c668742ee35e1d79956 Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> --- M src/soc/intel/alderlake/fsp_params.c 1 file changed, 34 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/81906/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/81906?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: I2b06a7c734a4fd4073e86c668742ee35e1d79956 Gerrit-Change-Number: 81906 Gerrit-PatchSet: 8 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(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>

1 0

[M] Change in coreboot[main]: intel/alderlake: Add helper functions for Power Management
by Sean Rhodes (Code Review) 29 Jul '24

29 Jul '24

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eric Lai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Nico Huber, Paul Menzel, Sean Rhodes, Subrata Banik. Hello Angel Pons, Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Nico Huber, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81638?usp=email to look at the new patch set (#23). The following approvals got outdated and were removed: Code-Review+1 by Nico Huber, Verified+1 by build bot (Jenkins) Change subject: intel/alderlake: Add helper functions for Power Management ...................................................................... intel/alderlake: Add helper functions for Power Management Clock Power Management, ASPM and L1 Substates have been configured the same way since Skylake. The main control to enable or disable is Kconfig, and then the level can be overridden in devicetree. Despite the UPDs remaining the same since Skylake, this is not the case for Alder Lake, Raptor Lake and Meteor Lake. Taking `starlabs/starbook` as an example, at the time of this commit it has PCIEXP_CLK_PM, PCIEXP_ASPM and PCIEXP_L1_SUB_STATE enabled. On Comet Lake, this results in the correct configuration, verified with the lspci command: ``` LnkCap: Port #0, Speed 5GT/s, Width x1, ASPM L1, Exit Latency L1 <8us ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+ LnkCtl: ASPM L1 Enabled; RCB 64 bytes, Disabled- CommClk+ ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt- ``` On Raptor Lake: ``` LnkCap: Port #0, Speed 16GT/s, Width x4, ASPM L1, Exit Latency L1 <64us ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp+ LnkCtl: ASPM Disabled; RCB 64 bytes, Disabled- CommClk+ ``` Clock Power Management, ASPM and L1 Substates are also not configured for CPU root ports. Add helper functions to configure these correctly based on Kconfig, but retain the capability to override the specific levels from devicetree. Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> --- M src/soc/intel/alderlake/fsp_params.c M src/soc/intel/common/block/include/intelblocks/pcie_rp.h 2 files changed, 88 insertions(+), 38 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/81638/23 -- To view, visit https://review.coreboot.org/c/coreboot/+/81638?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: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 23 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <ericllai(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> 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: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: i2c/drivers/generic: Add support for including a CDM
by Sean Rhodes (Code Review) 29 Jul '24

29 Jul '24

Attention is currently required from: Angel Pons, Matt DeVillier, Paul Menzel. Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email ) Change subject: i2c/drivers/generic: Add support for including a CDM ...................................................................... Patch Set 9: (1 comment) File src/drivers/i2c/generic/chip.h: https://review.coreboot.org/c/coreboot/+/81409/comment/20f11e92_ea9ee94e?us… : PS9, Line 98: bool has_cdm; : int cdm_index; > If a zero value is not valid, the boolean isn't needed: […] What's the reason for specifying the values? -- To view, visit https://review.coreboot.org/c/coreboot/+/81409?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: If70c25288d835df7064b4051c43abeb2d6531f3b Gerrit-Change-Number: 81409 Gerrit-PatchSet: 9 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 28 Jul 2024 19:14:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: commonlib/device_tree.c: Fix results length check
by Maximilian Brune (Code Review) 28 Jul '24

28 Jul '24

Attention is currently required from: Julius Werner. Maximilian Brune has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83085?usp=email ) Change subject: commonlib/device_tree.c: Fix results length check ...................................................................... Patch Set 4: (1 comment) File src/lib/device_tree.c: https://review.coreboot.org/c/coreboot/+/83085/comment/858c3a2a_5961f42c?us… : PS1, Line 363: if (count_results > results_len) { > Yes it is not correct. I meant to change that (forgot to set the patch to WIP). […] The current implementation should work. But I am thinking of just ditching the warning. It keeps us in the loop even if we reached the maximum number of nodes already. I would also assume that the caller knows what it is searching for and therefore knows a sane maximum value? -- To view, visit https://review.coreboot.org/c/coreboot/+/83085?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: Ic1111e8acb72ea1e9159da0d8386f40cbbdbc63f Gerrit-Change-Number: 83085 Gerrit-PatchSet: 4 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Sun, 28 Jul 2024 01:41:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>

1 0

[S] Change in coreboot[main]: commonlib/device_tree.c: Fix results length check
by Maximilian Brune (Code Review) 28 Jul '24

28 Jul '24

Attention is currently required from: Julius Werner. Maximilian Brune has uploaded a new patch set (#4). ( https://review.coreboot.org/c/coreboot/+/83085?usp=email ) Change subject: commonlib/device_tree.c: Fix results length check ...................................................................... commonlib/device_tree.c: Fix results length check Currently a warning is printed even if the maximum amount of nodes is not exceeded. Move the check into the compare statement to make sure coreboot only warns you in case it found more nodes matching the prefix than nodes actually having space in the results array. Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: Ic1111e8acb72ea1e9159da0d8386f40cbbdbc63f --- M src/commonlib/device_tree.c 1 file changed, 7 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/83085/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/83085?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: Ic1111e8acb72ea1e9159da0d8386f40cbbdbc63f Gerrit-Change-Number: 83085 Gerrit-PatchSet: 4 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>

1 0

[S] Change in coreboot[main]: commonlib/device_tree.c: Fix results length check
by Maximilian Brune (Code Review) 28 Jul '24

28 Jul '24

1 0

[S] Change in coreboot[main]: arch/riscv: Add PMP print function
by Maximilian Brune (Code Review) 28 Jul '24

28 Jul '24

Attention is currently required from: Elyes Haouas, Philipp Hug, Ron Minnich. Maximilian Brune has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83283?usp=email ) Change subject: arch/riscv: Add PMP print function ...................................................................... Patch Set 2: (1 comment) File src/arch/riscv/pmp.c: https://review.coreboot.org/c/coreboot/+/83283/comment/8b6be6a3_49eb1cac?us… : PS1, Line 3: "commonlib/bsd/helpers.h" > already included via <commonlib/helpers. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/83283?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: I6ab1531c65b14690e37aecf57ff441bf22db1ce5 Gerrit-Change-Number: 83283 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Sat, 27 Jul 2024 23:43:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[S] Change in coreboot[main]: arch/riscv: Add PMP print function
by Maximilian Brune (Code Review) 28 Jul '24

28 Jul '24

Attention is currently required from: Maximilian Brune, Philipp Hug, Ron Minnich. Hello Philipp Hug, Ron Minnich, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83283?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: arch/riscv: Add PMP print function ...................................................................... arch/riscv: Add PMP print function For easier debugging it is useful to have a function that prints the PMP regions. Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I6ab1531c65b14690e37aecf57ff441bf22db1ce5 --- M src/arch/riscv/include/arch/pmp.h M src/arch/riscv/pmp.c 2 files changed, 48 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/83283/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/83283?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: I6ab1531c65b14690e37aecf57ff441bf22db1ce5 Gerrit-Change-Number: 83283 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2024