coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3293 discussions

[NoOp] Change in coreboot[main]: src/mb: Rename new Makefile.inc files to Makefile.mak
by Elyes Haouas (Code Review) 15 Apr '24

15 Apr '24

Attention is currently required from: Eric Lai, Felix Held, Felix Singer, Martin L Roth, ron minnich. Elyes Haouas has uploaded a new patch set (#2) to the change originally created by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/81856?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by Eric Lai Change subject: src/mb: Rename new Makefile.inc files to Makefile.mak ...................................................................... src/mb: Rename new Makefile.inc files to Makefile.mak These files were added after the switch. Change-Id: I1986e4f921e0e56fe5255433d4b9216dc7c4dc59 Signed-off-by: Martin Roth <gaumless(a)gmail.com> --- R src/mainboard/sifive/hifive-unmatched/Makefile.mk R src/soc/sifive/fu740/Makefile.mk 2 files changed, 0 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/81856/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81856?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1986e4f921e0e56fe5255433d4b9216dc7c4dc59 Gerrit-Change-Number: 81856 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: util/lint: Add lint rule to avoid duplicated includes
by Elyes Haouas (Code Review) 15 Apr '24

15 Apr '24

Elyes Haouas has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81907?usp=email ) Change subject: util/lint: Add lint rule to avoid duplicated includes ...................................................................... util/lint: Add lint rule to avoid duplicated includes Change-Id: Ia14c73f0bffa6ee4b73254d71836f793af8771c4 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- A util/lint/lint-stable-031-includes.sh 1 file changed, 76 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/81907/1 diff --git a/util/lint/lint-stable-031-includes.sh b/util/lint/lint-stable-031-includes.sh new file mode 100644 index 0000000..222f6a4 --- /dev/null +++ b/util/lint/lint-stable-031-includes.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env sh +# SPDX-License-Identifier: GPL-2.0-or-later +# +# DESCR: Verify that included files are not duplicated +# +LINTDIR="$( + cd -- "$(dirname "$0")" > /dev/null 2>&1 || return + pwd -P +)" + +# shellcheck source=helper_functions.sh +. "${LINTDIR}/helper_functions.sh" +# Exit if the code isn't in a git repo +if [ "${IN_GIT_TREE}" -eq 0 ]; then + exit 0 +fi + +# Check for duplicated <device/pci_{def,type}.h> +DUPLICATED="grep '<device/pci_$def\|type$.h>' $(grep -l '<device/pci.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <device/pci.h> is supposed to provide <device/pci_{def,type}.h>" +fi + +# Check for duplicated <commonlib/bsd/cb_err.h>, <limits.h>, <stdbool.h>, <stdint.h> and <stddef.h> +DUPLICATED="grep '<commonlib/bsd/cb_err.h>\|<limits.h>\|<stdbool.h>\|<stdint.h>\|<stddef.h>' $(grep -l '<types.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <types.h> is supposed to provide <commonlib/bsd/cb_err.h>, <limits.h>, <stdbool.h>, <stdint.h> and <stddef.h>" +fi + +# Check for duplicated <stdarg.h> and <stdio.h> +DUPLICATED="grep '<stdarg.h>\|<stdio.h>' $(grep -l '<string.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <string.h> is supposed to provide <stdarg.h> and <stdio.h>" +fi + +# Check for duplicated <arch/cpuid.h> +DUPLICATED="grep '<arch/cpuid.h>' $(grep -l '<arch/cpu.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <arch/cpu.h> is supposed to provide <arch/cpuid.h>" +fi + +# Check for duplicated <commonlib/bsd/cbmem_id.h> +DUPLICATED="grep '<commonlib/bsd/cbmem_id.h>' $(grep -l '<cbmem.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <cbmem.h> is supposed to provide <commonlib/bsd/cbmem_id.h>" +fi + +# Check for duplicated <commonlib/loglevel.h> +DUPLICATED="grep '<commonlib/loglevel.h>' $(grep -l '<console/console.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <console/console.h> is supposed to provide <commonlib/loglevel.h>" +fi + +# Check for duplicated <arch/cpu.h> +DUPLICATED="grep '<arch/cpu.h>' $(grep -l '<cpu/cpu.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <cpu/cpu.h> is supposed to provide <arch/cpu.h>" +fi + +# Check for duplicated <cpu/x86/msr_access.h> +DUPLICATED="grep '<cpu/x86/msr_access.h>' $(grep -l '<cpu/x86/msr.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <cpu/x86/msr.h> is supposed to provide <cpu/x86/msr_access.h>" +fi + +# Check for duplicated <arch/mmio.h> +DUPLICATED="grep '<arch/mmio.h>' $(grep -l '<device/mmio.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <device/mmio.h> is supposed to provide <arch/mmio.h>" +fi + +# Check for duplicated <arch/pci_ops.h> +DUPLICATED="grep '<arch/pci_ops.h>' $(grep -l '<device/pci_ops.h>')" +if [ -n "${DUPLICATED}" ]; then + echo "Warning: <device/pci_ops.h> is supposed to provide <arch/pci_ops.h>" +fi -- To view, visit https://review.coreboot.org/c/coreboot/+/81907?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia14c73f0bffa6ee4b73254d71836f793af8771c4 Gerrit-Change-Number: 81907 Gerrit-PatchSet: 1 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: drivers/i2c/rt5645: Add RT5645 amp driver
by Jianeng Ceng (Code Review) 15 Apr '24

15 Apr '24

Attention is currently required from: Eric Lai, Paul Menzel, Subrata Banik. Jianeng Ceng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81773?usp=email ) Change subject: drivers/i2c/rt5645: Add RT5645 amp driver ...................................................................... Patch Set 12: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81773/comment/fdc9a161_1e2e4d66 : PS9, Line 9: Add RT5645 AMP support. > As it’s C code, can’t you guard the device specific parts by an if statement? 5650 and 5645 are in the same series, different from 5663. Commit Message: https://review.coreboot.org/c/coreboot/+/81773/comment/a83cb5cb_14fcfd10 : PS11, Line 11: ALC5650 > So why name the directory *rt5645*? Please add a comment in the commit message. The kernel driver of 5650 is written in rt5645.c. -- To view, visit https://review.coreboot.org/c/coreboot/+/81773?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I602fcc4dd8576043943f6e20884edc4703350320 Gerrit-Change-Number: 81773 Gerrit-PatchSet: 12 Gerrit-Owner: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dolan Liu <liuyong5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.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: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 15 Apr 2024 09:36:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/i2c/rt5645: Add RT5645 amp driver
by Jianeng Ceng (Code Review) 15 Apr '24

15 Apr '24

Attention is currently required from: Eric Lai, Jianeng Ceng, Subrata Banik. Hello Dolan Liu, Eric Lai, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81773?usp=email to look at the new patch set (#12). Change subject: drivers/i2c/rt5645: Add RT5645 amp driver ...................................................................... drivers/i2c/rt5645: Add RT5645 amp driver Add RT5645 AMP support.The kernel driver of 5650 is written in rt5645.c. Add acpi name cbj-sleeve-gpios for power gate GPIO. ALC5650 DataSheet Rev 0.93 Realtek upstream link: https://lore.kernel.org/all/20240404035747.118064-1-derek.fang@realtek.com/ BUG=None TEST=verified in anraggar and probe device rt5650 succeed ``` \_SB.PCI0.I2C3.RT58: Realtek RT5650 ``` Change-Id: I602fcc4dd8576043943f6e20884edc4703350320 Signed-off-by: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com> --- A src/drivers/i2c/rt5645/Kconfig A src/drivers/i2c/rt5645/Makefile.mk A src/drivers/i2c/rt5645/chip.h A src/drivers/i2c/rt5645/rt5645.c 4 files changed, 141 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/81773/12 -- To view, visit https://review.coreboot.org/c/coreboot/+/81773?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I602fcc4dd8576043943f6e20884edc4703350320 Gerrit-Change-Number: 81773 Gerrit-PatchSet: 12 Gerrit-Owner: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dolan Liu <liuyong5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.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: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jianeng Ceng <cengjianeng(a)huaqin.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

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

15 Apr '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Nico Huber, Sean Rhodes, Subrata Banik. Hello Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, 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 (#7). 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 AlderLake, RaptorLake and MeteorLake. 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 CometLake, 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 RaptorLake: ``` 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 1 file changed, 87 insertions(+), 38 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/81638/7 -- 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 Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 7 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: 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> 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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

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

15 Apr '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Nico Huber, Sean Rhodes, Subrata Banik. Hello Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, 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 (#6). 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 AlderLake, RaptorLake and MeteorLake. 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 CometLake, 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 RaptorLake: ``` 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 1 file changed, 87 insertions(+), 38 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/81638/6 -- 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 Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 6 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: 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> 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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: intel/alderlake: Add helper functions for Power Management
by Eric Lai (Code Review) 15 Apr '24

15 Apr '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Nico Huber, Sean Rhodes, Subrata Banik. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email ) Change subject: intel/alderlake: Add helper functions for Power Management ...................................................................... Patch Set 5: (1 comment) File src/soc/intel/alderlake/chip.h: https://review.coreboot.org/c/coreboot/+/81638/comment/75a958e5_18181ade : PS4, Line 799: }; > My theory was this patch, hook it up to the option API, move to common code, tidy the enums - just d […] Why not update src/soc/intel/common/block/include/intelblocks/pcie_rp.h? Also the value in device tree will not map to enum after this patch. -- 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 Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 5 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: 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> 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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Mon, 15 Apr 2024 09:09:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

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

15 Apr '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Nico Huber, Subrata Banik. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email ) Change subject: intel/alderlake: Add helper functions for Power Management ...................................................................... Patch Set 5: (2 comments) File src/soc/intel/alderlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/81638/comment/50df8d19_63c7e4af : PS3, Line 481: CONFIG(SOC_INTEL_COMPLIANCE_TEST_MODE) > please keep this config for EA testing purpose. Done File src/soc/intel/alderlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/81638/comment/56c17701_5d351caa : PS4, Line 504: default "auto" > Sorry, my bad, should have explained better. To coreboot, default (e.g. […] Done -- 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 Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 5 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: 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: 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: Nico Huber <nico.h(a)gmx.de> 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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Mon, 15 Apr 2024 08:58:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <ericllai(a)google.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Gerrit-MessageType: comment

1 0

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

15 Apr '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Nico Huber, Subrata Banik. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email ) Change subject: intel/alderlake: Add helper functions for Power Management ...................................................................... Patch Set 5: (2 comments) File src/soc/intel/alderlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/81638/comment/2693ce90_daa6f1cb : PS4, Line 506: s_cfg->PcieRpAspm[index] = rp_cfg->pcie_rp_aspm; > Gotcha, so wouldn't it make it easier to read to have the below so we can just use the value as is? […] Done https://review.coreboot.org/c/coreboot/+/81638/comment/f4219b3f_763971a1 : PS4, Line 517: s_cfg->PcieRpL1Substates[index] = rp_cfg->PcieRpL1Substates; > Same here. Done -- 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 Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 5 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: 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: 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: Nico Huber <nico.h(a)gmx.de> 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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Mon, 15 Apr 2024 08:58:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

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

15 Apr '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Nico Huber, Subrata Banik. Hello Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, 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 (#5). The following approvals got outdated and were removed: 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 AlderLake, RaptorLake and MeteorLake. 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 CometLake, 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 RaptorLake: ``` 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/chip.h M src/soc/intel/alderlake/fsp_params.c 2 files changed, 104 insertions(+), 38 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/81638/5 -- 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 Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 5 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: 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: 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: Nico Huber <nico.h(a)gmx.de> 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: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024