Attention is currently required from: Angel Pons, Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Nico Huber, Paul Menzel, Sean Rhodes, Subrata Banik.
Matt DeVillier 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 16:
(1 comment)
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/81638/comment/e3cd8fdc_667c71b1?us… :
PS15, Line 507: if (rp_cfg->pcie_rp_aspm == 0)
: s_cfg->PcieRpAspm[index] = 4;
: else
: s_cfg->PcieRpAspm[index] = rp_cfg->pcie_rp_aspm - 1;
> Not too fond of it, personally, but let's see what others have to say. […]
I don't hate it, it's certainly much more readable. agreed on using helper functions, but I'd drop the adl_ prefix since it the code is under soc/intel/adl already
--
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: 16
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: 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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
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: 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: Wed, 24 Jul 2024 21:07:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Elyes Haouas, Eric Lai, Felix Singer, Michał Żygowski, Piotr Król.
Matt DeVillier has posted comments on this change by Felix Singer. ( https://review.coreboot.org/c/coreboot/+/83455?usp=email )
Change subject: mb/protectli/vault_cml: Drop superfluous devices from devicetree
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83455?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: Ie485684747efccb8fb0ab87f10694c52a98f3c88
Gerrit-Change-Number: 83455
Gerrit-PatchSet: 4
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: 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: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Comment-Date: Wed, 24 Jul 2024 21:03:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Saurabh Mishra, Tarun.
Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83354?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock
......................................................................
Patch Set 14:
(20 comments)
File src/soc/intel/pantherlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83354/comment/68184929_684a9083?us… :
PS8, Line 44: 0xfa000000
> Due to PTL using config blocks (intermediality, UPD based FSP will be relased soon) to update to FSP-M/S, hence the size of DCACHE was increased. This setting is reverted back to previous program settings, and validated with LNL-M SOC SKU (FSP-Debug/Non-debug both).
I don't think PTL-FSP should be using config block, we have dropped that feature from PTL hence, this argument doesn't make any sense to me.
https://review.coreboot.org/c/coreboot/+/83354/comment/477b7a54_5840107a?us… :
PS8, Line 47: 0x200000
> Due to PTL using config blocks (intermediality, UPD based FSP will be relased soon) to update to FSP-M/S, hence the size of DCACHE was increased. This setting is reverted back to previous program settings, and validated with LNL-M SOC SKU (FSP-Debug/Non-debug both).
same justification, please restore the changes w/o getting impacted by FSP config block which is non-POR for PTL
https://review.coreboot.org/c/coreboot/+/83354/comment/32c33095_9a6fe5f2?us… :
PS8, Line 63: 0x80100
> Due to PTL using config blocks (intermediality, UPD based FSP will be relased soon) to update to FSP-M/S, hence the size of DCACHE was increased. This setting is reverted back to previous program settings, and validated with LNL-M SOC SKU (FSP-Debug/Non-debug both).
same justification
File src/soc/intel/pantherlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83354/comment/a91cd180_735258d9?us… :
PS14, Line 25: select HAVE_X86_64_SUPPORT
please use the order ?
https://review.coreboot.org/c/coreboot/+/83354/comment/c26b09a4_5f151821?us… :
PS14, Line 32: Choose this option if your mainboard has a PTL-U (15W)
: or PTL-H 4Xe3 / 12Xe3 (25W) SoC.
Choose this option if your mainboard has a PTL-UH SoC.
https://review.coreboot.org/c/coreboot/+/83354/comment/85796e91_cf3d5b09?us… :
PS14, Line 33: PTL-H 4Xe3
this is pure PTL-H and not part of the PTL-UH
https://review.coreboot.org/c/coreboot/+/83354/comment/e75115b1_caeb3e6a?us… :
PS14, Line 67: ~1KiB)
32KiB?
File src/soc/intel/pantherlake/espi.c:
https://review.coreboot.org/c/coreboot/+/83354/comment/cf053ca4_29240850?us… :
PS14, Line 27: #if ENV_RAMSTAGE
I believe this is not required. CB:83637
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/28519cd3_a77ae3f0?us… :
PS14, Line 59: /*
need white space around
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/075fc440_7243ab93?us… :
PS8, Line 37: VTD_BASE_ADDRESS
> Acknowledged
I'm unable to follow why you have marked this `ack` when I'm expecting little more clarification about what is DMI3BAR ?
https://review.coreboot.org/c/coreboot/+/83354/comment/2ec14909_f8e454a6?us… :
PS8, Line 37: fc800000
> Acknowledged
why you have marked it `ack` when nothing has been changed ?
https://review.coreboot.org/c/coreboot/+/83354/comment/4c8944ca_7d8ef968?us… :
PS8, Line 74: 0x3fff0800000
> IOM should be anywhere of size 64KB.
what you mean by anywhere, this number has to be aligned between FSP and coreboot. How about coreboot programs something differently than what FSP does?
File src/soc/intel/pantherlake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/132a2d20_d1d64e43?us… :
PS14, Line 174:
need two more space
File src/soc/intel/pantherlake/include/soc/pcr_ids.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/09480f09_425f22aa?us… :
PS14, Line 6: /*
: * Port ids
: */
why multiline ?
File src/soc/intel/pantherlake/include/soc/soc_info.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/593ecaf7_7129106f?us… :
PS14, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
please give one new line
https://review.coreboot.org/c/coreboot/+/83354/comment/ab2fda0f_8606b0a9?us… :
PS14, Line 5: enum {
: NOT_DETECTED = 0,
: PTLU,
: PTLH,
: };
why needed ?
File src/soc/intel/pantherlake/soc_info.c:
https://review.coreboot.org/c/coreboot/+/83354/comment/ef1bbed0_c76f1255?us… :
PS14, Line 2:
```
#define __SIMPLE_DEVICE__
```
to avoid #if/#else in line 14-20
https://review.coreboot.org/c/coreboot/+/83354/comment/546f3027_0261b076?us… :
PS14, Line 10: get_soctype
why do we need this ? which problem this new API will solve. ideally an API should have its own comment block for caller to know when to use it
https://review.coreboot.org/c/coreboot/+/83354/comment/4979cdad_fd0141da?us… :
PS14, Line 69: printk(BIOS_DEBUG, "soc_info: max_pcie_clock:%d\n", pcie_clock);
why we need these prints ?
https://review.coreboot.org/c/coreboot/+/83354/comment/3037dc59_1baa3c63?us… :
PS14, Line 92: }
can you follow mtl soc_info.c? I guess your PTL code base is very old (atleast two quarter old because https://review.coreboot.org/c/coreboot/+/81111 actually did the code refactoring which is not present in your PTL code). Your soc_info.c code is actually the older version of MTL soc_info.c before https://review.coreboot.org/c/coreboot/+/81111 refactoring.
Please don't try to push older code base in form of PTL (it only makes things difficult and add up more work for us later).
--
To view, visit https://review.coreboot.org/c/coreboot/+/83354?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: Ibcfe71eec27cebf04f10ec343a73dd92f1272aca
Gerrit-Change-Number: 83354
Gerrit-PatchSet: 14
Gerrit-Owner: Saurabh Mishra <mishra.saurabh(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: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jul 2024 20:58:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Caveh Jalali, Forest Mittelberg.
Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83639?usp=email )
Change subject: ec/google/chromeec: Drop 'choice' selections for EC and PD firmware
......................................................................
ec/google/chromeec: Drop 'choice' selections for EC and PD firmware
Since the EC and PD firmware sources are now limited to two options -
'none' and 'external' - drop the choice selection and make the
EC and PD external options independent.
TEST=build google/lulu with external EC binary using existing defconfig
Change-Id: Ie37ff3a188b414fd099fbb344858bca4df419086
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
---
M src/ec/google/chromeec/Kconfig
1 file changed, 6 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/83639/1
diff --git a/src/ec/google/chromeec/Kconfig b/src/ec/google/chromeec/Kconfig
index e8a5d58..7a8777f 100644
--- a/src/ec/google/chromeec/Kconfig
+++ b/src/ec/google/chromeec/Kconfig
@@ -110,22 +110,10 @@
Enable support for the real-time clock on the ChromeOS EC. This
uses the EC_CMD_RTC_GET_VALUE command to read the current time.
-choice
- prompt "Chrome EC firmware source"
- default EC_GOOGLE_CHROMEEC_FIRMWARE_NONE
-
- config EC_GOOGLE_CHROMEEC_FIRMWARE_NONE
- bool "No EC firmware is included"
+config EC_GOOGLE_CHROMEEC_FIRMWARE_EXTERNAL
+ bool "Include an external EC firmware binary"
help
- No EC firmware is included in the image.
-
- config EC_GOOGLE_CHROMEEC_FIRMWARE_EXTERNAL
- bool "External EC firmware is included"
- help
- Include EC firmware binary in the image from an external source.
- It is expected to be built externally.
-
-endchoice
+ Include a precompiled EC firmware binary in the image.
config EC_GOOGLE_CHROMEEC_FIRMWARE_FILE
string "Chrome EC firmware path and filename"
@@ -133,23 +121,11 @@
help
The path and filename of the EC firmware file to use.
-choice
- prompt "Chrome EC firmware source for PD"
+config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_EXTERNAL
+ bool "Include an external PD firmware binary"
depends on EC_GOOGLE_CHROMEEC_PD
- default EC_GOOGLE_CHROMEEC_PD_FIRMWARE_NONE
-
- config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_NONE
- bool "No PD firmware is included"
help
- No PD firmware is included in the image.
-
- config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_EXTERNAL
- bool "External PD firmware is included"
- help
- Include PD firmware binary in the image from an external source.
- It is expected to be built externally.
-
-endchoice
+ Include a precompiled PD firmware binary in the image.
config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_FILE
string "Chrome EC firmware path and filename for PD"
--
To view, visit https://review.coreboot.org/c/coreboot/+/83639?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie37ff3a188b414fd099fbb344858bca4df419086
Gerrit-Change-Number: 83639
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83638?usp=email )
Change subject: ec/google/chromeec: Drop ability to build Chrome-EC, PD components
......................................................................
ec/google/chromeec: Drop ability to build Chrome-EC, PD components
In preparation for dropping the Chrome-EC submodule, remove the ability
for Chrome-EC and PD components to be built as part of coreboot.
These components have not been used or buildable for many years.
Change-Id: Ibf6bd43e755cf5b4d2aa8a42f38dc52e7023e9b3
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
---
M src/ec/google/chromeec/Kconfig
M src/ec/google/chromeec/Makefile.mk
M src/mainboard/google/foster/Kconfig
M src/mainboard/google/rambi/Kconfig
4 files changed, 2 insertions(+), 65 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/83638/1
diff --git a/src/ec/google/chromeec/Kconfig b/src/ec/google/chromeec/Kconfig
index 19790d1..e8a5d58 100644
--- a/src/ec/google/chromeec/Kconfig
+++ b/src/ec/google/chromeec/Kconfig
@@ -104,23 +104,6 @@
help
Provides common routine for reporting the skuid to ChromeOS.
-config EC_GOOGLE_CHROMEEC_BOARDNAME
- string "Chrome EC board name for EC"
- default ""
- help
- The board name used in the Chrome EC code base to build
- the EC firmware. If set, the coreboot build with also
- build the EC firmware and add it to the image.
-
-config EC_GOOGLE_CHROMEEC_PD_BOARDNAME
- depends on EC_GOOGLE_CHROMEEC_PD
- string "Chrome EC board name for PD"
- default ""
- help
- The board name used in the Chrome EC code base to build
- the PD firmware. If set, the coreboot build with also
- build the EC firmware and add it to the image.
-
config EC_GOOGLE_CHROMEEC_RTC
bool "Enable ChromeOS EC RTC"
help
@@ -129,13 +112,12 @@
choice
prompt "Chrome EC firmware source"
- default EC_GOOGLE_CHROMEEC_FIRMWARE_BUILTIN if EC_GOOGLE_CHROMEEC_BOARDNAME != ""
default EC_GOOGLE_CHROMEEC_FIRMWARE_NONE
config EC_GOOGLE_CHROMEEC_FIRMWARE_NONE
bool "No EC firmware is included"
help
- Disable building and including any EC firmware in the image.
+ No EC firmware is included in the image.
config EC_GOOGLE_CHROMEEC_FIRMWARE_EXTERNAL
bool "External EC firmware is included"
@@ -143,11 +125,6 @@
Include EC firmware binary in the image from an external source.
It is expected to be built externally.
- config EC_GOOGLE_CHROMEEC_FIRMWARE_BUILTIN
- bool "Builtin EC firmware is included"
- help
- Build and include EC firmware binary in the image.
-
endchoice
config EC_GOOGLE_CHROMEEC_FIRMWARE_FILE
@@ -159,13 +136,12 @@
choice
prompt "Chrome EC firmware source for PD"
depends on EC_GOOGLE_CHROMEEC_PD
- default EC_GOOGLE_CHROMEEC_PD_FIRMWARE_BUILTIN if EC_GOOGLE_CHROMEEC_PD_BOARDNAME != ""
default EC_GOOGLE_CHROMEEC_PD_FIRMWARE_NONE
config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_NONE
bool "No PD firmware is included"
help
- Disable building and including any PD firmware in the image.
+ No PD firmware is included in the image.
config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_EXTERNAL
bool "External PD firmware is included"
@@ -173,11 +149,6 @@
Include PD firmware binary in the image from an external source.
It is expected to be built externally.
- config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_BUILTIN
- bool "Builtin PD firmware is included"
- help
- Build and include PD firmware binary in the image.
-
endchoice
config EC_GOOGLE_CHROMEEC_PD_FIRMWARE_FILE
diff --git a/src/ec/google/chromeec/Makefile.mk b/src/ec/google/chromeec/Makefile.mk
index 1cf88f2..a8d2960 100644
--- a/src/ec/google/chromeec/Makefile.mk
+++ b/src/ec/google/chromeec/Makefile.mk
@@ -74,19 +74,6 @@
$(obj)/mainboard/$(MAINBOARDDIR)/ecrw: $(CONFIG_EC_GOOGLE_CHROMEEC_FIRMWARE_FILE)
cp $(CONFIG_EC_GOOGLE_CHROMEEC_FIRMWARE_FILE) $@
-else
-CONFIG_EC_GOOGLE_CHROMEEC_BOARDNAME := $(call strip_quotes,$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDNAME))
-
-$(obj)/mainboard/$(MAINBOARDDIR)/ecrw:
- $(MAKE) -C $(CHROMEEC_SOURCE) $(if $(CONFIG_CCACHE),,CCACHE=) \
- out=$(abspath $(obj)/external/chromeec/$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDNAME)) \
- REPRODUCIBLE_BUILD=1 \
- CC=$(GCC_CC_arm) \
- CROSS_COMPILE=$(subst -cpp,-,$(CPP_arm)) \
- HOST_CROSS_COMPILE= \
- BOARD=$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDNAME) \
- rw
- cp $(obj)/external/chromeec/$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDNAME)/RW/ec.RW.flat $@
endif
$(obj)/mainboard/$(MAINBOARDDIR)/ecrw.hash: $(obj)/mainboard/$(MAINBOARDDIR)/ecrw
@@ -113,19 +100,6 @@
$(obj)/mainboard/$(MAINBOARDDIR)/pdrw: $(CONFIG_EC_GOOGLE_CHROMEEC_PD_FIRMWARE_FILE)
cp $(CONFIG_EC_GOOGLE_CHROMEEC_PD_FIRMWARE_FILE) $@
-else
-CONFIG_EC_GOOGLE_CHROMEEC_PD_BOARDNAME := $(call strip_quotes,$(CONFIG_EC_GOOGLE_CHROMEEC_PD_BOARDNAME))
-
-$(obj)/mainboard/$(MAINBOARDDIR)/pdrw:
- $(MAKE) -C $(CHROMEEC_SOURCE) $(if $(CONFIG_CCACHE),,CCACHE=) \
- out=$(abspath $(obj)/external/chromeec/$(CONFIG_EC_GOOGLE_CHROMEEC_PD_BOARDNAME)) \
- REPRODUCIBLE_BUILD=1 \
- CC=$(GCC_CC_arm) \
- CROSS_COMPILE=$(subst -cpp,-,$(CPP_arm)) \
- HOST_CROSS_COMPILE= \
- BOARD=$(CONFIG_EC_GOOGLE_CHROMEEC_PD_BOARDNAME) \
- rw
- cp $(obj)/external/chromeec/$(CONFIG_EC_GOOGLE_CHROMEEC_PD_BOARDNAME)/RW/ec.RW.flat $@
endif
$(obj)/mainboard/$(MAINBOARDDIR)/pdrw.hash: $(obj)/mainboard/$(MAINBOARDDIR)/pdrw
diff --git a/src/mainboard/google/foster/Kconfig b/src/mainboard/google/foster/Kconfig
index 6a2d28e..cef17b1 100644
--- a/src/mainboard/google/foster/Kconfig
+++ b/src/mainboard/google/foster/Kconfig
@@ -61,10 +61,6 @@
hex
default 0x20
-#config EC_GOOGLE_CHROMEEC_BOARDNAME
-# string
-# default "nyan"
-
config VBOOT_FWID_MODEL
string
default "Nvidia_Foster"
diff --git a/src/mainboard/google/rambi/Kconfig b/src/mainboard/google/rambi/Kconfig
index 600d989..65339e0 100644
--- a/src/mainboard/google/rambi/Kconfig
+++ b/src/mainboard/google/rambi/Kconfig
@@ -143,10 +143,6 @@
config OVERRIDE_DEVICETREE
default "variants/\$(CONFIG_VARIANT_DIR)/overridetree.cb"
-config EC_GOOGLE_CHROMEEC_BOARDNAME
- string
- default ""
-
config MAINBOARD_SMBIOS_MANUFACTURER
string
default "GOOGLE"
--
To view, visit https://review.coreboot.org/c/coreboot/+/83638?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibf6bd43e755cf5b4d2aa8a42f38dc52e7023e9b3
Gerrit-Change-Number: 83638
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Nico Huber, Paul Menzel, Sean Rhodes, Subrata Banik.
Angel Pons 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 16:
(1 comment)
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/81638/comment/8a0f54d2_18eb1cd1?us… :
PS15, Line 507: if (rp_cfg->pcie_rp_aspm == 0)
: s_cfg->PcieRpAspm[index] = 4;
: else
: s_cfg->PcieRpAspm[index] = rp_cfg->pcie_rp_aspm - 1;
> How about PS16? Might be a bit too UEFI, but makes it easy to read 😊
Not too fond of it, personally, but let's see what others have to say.
Still not using the helper function (which would also be used for CPU PCIe RPs).
--
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: 16
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: 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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
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: 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-Comment-Date: Wed, 24 Jul 2024 20:38:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Christian Walter, Filip Lewiński.
Julius Werner has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/82695?usp=email )
Change subject: security/intel/txt: Handle TPM properly when vboot enabled
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
Please justify what you're doing here and why?
I don't think making vboot play nice with INIT_BOOTBLOCK is as simple as turning on IGNORE_POSTINIT. IGNORE_POSTINIT is a hack and not really a secure thing to use by default — there are good reasons why you want to make sure the TPM was properly reset on boot.
Also, you're still running `tpm_setup()` twice, that's still not good, even if you ignore the error that makes it fail. You're calling `tspi_measure_cache_to_pcr()` again and end up adding all those entries twice.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82695?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: I19dc3d910c23fcfd8732465c488f47dd86a96781
Gerrit-Change-Number: 82695
Gerrit-PatchSet: 4
Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Comment-Date: Wed, 24 Jul 2024 20:19:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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 16:
(1 comment)
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/81638/comment/7496693b_5cb88ec2?us… :
PS15, Line 507: if (rp_cfg->pcie_rp_aspm == 0)
: s_cfg->PcieRpAspm[index] = 4;
: else
: s_cfg->PcieRpAspm[index] = rp_cfg->pcie_rp_aspm - 1;
> That too
How about PS16? Might be a bit too UEFI, but makes it easy to read 😊
--
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: 16
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: 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-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: Wed, 24 Jul 2024 20:11:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
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, 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 (#16).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons, 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/fsp_params.c
M src/soc/intel/common/block/include/intelblocks/pcie_rp.h
2 files changed, 101 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/81638/16
--
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: 16
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: 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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
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: 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>