Attention is currently required from: Angel Pons, Dinesh Gehlot, Jayvik Desai.
Paul Menzel has posted comments on this change by Jayvik Desai. ( https://review.coreboot.org/c/coreboot/+/83705?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: vc/google/chromeos: Enable eSOL config with libgfx and uGOP
......................................................................
Patch Set 16:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83705/comment/55368166_9f8e0501?us… :
PS16, Line 9: enables
Isn’t it just adding a Kconfig symbol?
https://review.coreboot.org/c/coreboot/+/83705/comment/2a5e018d_d007b53f?us… :
PS16, Line 10: when
Fits on the line above.
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/83705/comment/19265ddd_b326c62f?us… :
PS16, Line 108: sign of life
Sign-of-Life
--
To view, visit https://review.coreboot.org/c/coreboot/+/83705?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: Ic8fe4ca5234de7f8e579f950f6ccbf750f4c7950
Gerrit-Change-Number: 83705
Gerrit-PatchSet: 16
Gerrit-Owner: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 06:05:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Andrey Petrov, Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Jayvik Desai has posted comments on this change by Jayvik Desai. ( https://review.coreboot.org/c/coreboot/+/83769?usp=email )
Change subject: soc/intel/mtl: enable FSP uGOP config in MTL for eSOL
......................................................................
Patch Set 12:
(10 comments)
Patchset:
PS1:
> split this into two cls […]
Created the CL's.
Patchset:
PS2:
> this CL should be the base CL IMO.
Yes agreed
Commit Message:
https://review.coreboot.org/c/coreboot/+/83769/comment/ece05ccb_94929226?us… :
PS9, Line 9: This patch enables the FSP uGOP eSOL feature for meteorlake SOC
> Please inform that you are replacing the earlier config with a more generic uGOP config. […]
Updated in patchset#12
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/83769/comment/c6dda008_ebcd783e?us… :
PS1, Line 463: help
> ``` […]
Added
https://review.coreboot.org/c/coreboot/+/83769/comment/6114ceb0_e287d9b1?us… :
PS1, Line 464: Enable the FSP-M Sign-of-Life feature to display a
: configurable text message on screen during memory training
: and CSME update.
> you didn't mention the underlying technology (uGOP) that Intel uses to enable eSOL as part of FSP-M
Acknowledged
https://review.coreboot.org/c/coreboot/+/83769/comment/10526a52_1dbdf9c5?us… :
PS1, Line 468:
> empty line
Removed
File src/soc/intel/meteorlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83769/comment/d0dfa141_9ad5621c?us… :
PS1, Line 25: select VBT_CBFS_COMPRESSION_DEFAULT_LZ4 if FSP_UGOP_EARLY_SIGN_OF_LIFE
> please follow order
Acknowledged
File src/soc/intel/meteorlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83769/comment/8277f514_dd363bd9?us… :
PS2, Line 25: select VBT_CBFS_COMPRESSION_DEFAULT_LZ4 if FSP_UGOP_EARLY_SIGN_OF_LIFE
@subratabanik@google.com, Can you please suggest a proper way to select this config ?
File src/soc/intel/meteorlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83769/comment/2eaf79e6_73a520c4?us… :
PS8, Line 23: select FSP_USES_CB_DEBUG_EVENT_HANDLER
: select FSP_UGOP_EARLY_SIGN_OF_LIFE if !SOC_INTEL_METEORLAKE_PRE_PRODUCTION_SILICON
> ```suggestion […]
Rewrote in alphabetical order
https://review.coreboot.org/c/coreboot/+/83769/comment/7b22267c_dbddda89?us… :
PS8, Line 24: FSP_UGOP_EARLY_SIGN_OF_LIFE
> move after line 22
Fixed in patch set-9.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83769?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: Ib4589f52080229b1c83915b51272a042b7ac32cd
Gerrit-Change-Number: 83769
Gerrit-PatchSet: 12
Gerrit-Owner: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 05:37:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Dinesh Gehlot <digehlot(a)google.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Andrey Petrov, Dinesh Gehlot, Paul Menzel, Ronak Kanabar, Subrata Banik.
Jayvik Desai has posted comments on this change by Jayvik Desai. ( https://review.coreboot.org/c/coreboot/+/83770?usp=email )
Change subject: drivers/intel/fsp2: Add config for FSP uGOP eSOL
......................................................................
Patch Set 6:
(1 comment)
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/83770/comment/e9f110a8_d72b930b?us… :
PS4, Line 475: a configurable
: text message
> Where can it be configured?
This text message can be updated in ````fill_fspm_sign_of_life```` function, which is only called if this config is enabled.
But you are right it's not configurable but hardcoded at the moment so updating this statement.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83770?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: Ic0426ff7974a141ae9188b0098677b4cc97aee36
Gerrit-Change-Number: 83770
Gerrit-PatchSet: 6
Gerrit-Owner: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 05:37:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Angel Pons, Dinesh Gehlot, Paul Menzel.
Jayvik Desai has posted comments on this change by Jayvik Desai. ( https://review.coreboot.org/c/coreboot/+/83705?usp=email )
Change subject: vc/google/chromeos: Enable eSOL config with libgfx and uGOP
......................................................................
Patch Set 16:
(10 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83705/comment/c75e4a83_90243163?us… :
PS9, Line 7: src/device
> vc/google/chromeos
Updated
Commit Message:
https://review.coreboot.org/c/coreboot/+/83705/comment/309baab0_f1938338?us… :
PS13, Line 9: This patch enables the eSOL config option when libgfx or uGOP is enabled
: for early graphics initialization.
> *early sign of life* could be spelled out once.
Spelled out. Thanks
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/83705/comment/a2345d51_5a4b8aa2?us… :
PS5, Line 83: config CHROMEOS_ENABLES_ESOL
> nit: Kconfig symbol names often use "imperative mood" (I forget what it's called), e.g. […]
Done
https://review.coreboot.org/c/coreboot/+/83705/comment/5591cfb5_f2e45d53?us… :
PS5, Line 85: depends on MAINBOARD_HAS_EARLY_LIBGFXINIT || SOC_INTEL_METEORLAKE_SIGN_OF_LIFE
> I don't like repeating the condition here and in the `select` statement in `config CHROMEOS`, but `C […]
Changed, cleaned it up!
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/83705/comment/39035dd1_d7cfe0d5?us… :
PS6, Line 105: menu "CHROMEOS_ESOL"
> we don't need this
Acknowledged
https://review.coreboot.org/c/coreboot/+/83705/comment/97f26a21_0d3d38ea?us… :
PS6, Line 109: SOC_INTEL_METEORLAKE_SIGN_OF_LIFE
> FSP_UGOP_ESOL
Added
https://review.coreboot.org/c/coreboot/+/83705/comment/4aa83b0e_e4203ce8?us… :
PS6, Line 114: endmenu
> same
Ack
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/83705/comment/063c89f6_9d17bc93?us… :
PS9, Line 106: bool
: default y if FSP_UGOP_EARLY_SIGN_OF_LIFE || MAINBOARD_HAS_EARLY_LIBGFXINIT
: default n
> nit: I think this is a less verbose equivalent: […]
Updated
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/83705/comment/99f89478_f4052cfd?us… :
PS11, Line 111:
> one empty line
Acknowledged
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/83705/comment/8ea5a7d7_ceb4560e?us… :
PS12, Line 106: def_bool y if FSP_UGOP_EARLY_SIGN_OF_LIFE || MAINBOARD_HAS_EARLY_LIBGFXINIT
: def_bool n
> I suggested setting the default directly using a boolean expression. Did this not work properly? […]
it works properly, i misunderstood your previous comment, updated!
--
To view, visit https://review.coreboot.org/c/coreboot/+/83705?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: Ic8fe4ca5234de7f8e579f950f6ccbf750f4c7950
Gerrit-Change-Number: 83705
Gerrit-PatchSet: 16
Gerrit-Owner: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 05:36:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Bora Guvendik, Hannah Williams, Saurabh Mishra, Subrata Banik.
Anil Kumar K has posted comments on this change by Bora Guvendik. ( https://review.coreboot.org/c/coreboot/+/83786?usp=email )
Change subject: device/pci_ids: Add new Intel PTL device IDs for Tracehub
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/83786?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: Ifa1a0a57c504e06d686e7e0826547251b456cc8b
Gerrit-Change-Number: 83786
Gerrit-PatchSet: 3
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 04:50:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Kun Liu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83793?usp=email )
Change subject: mb/google/brox/var/lotso: Enable wifi sar
......................................................................
mb/google/brox/var/lotso: Enable wifi sar
wifi.SetTXPower test fail, so enable wifi sar.
BUG=b:351698478
TEST=emerge-brox coreboot
Change-Id: Ibf5425e72eddc45e376ef4e2d077180dab502200
Signed-off-by: Kun Liu <liukun11(a)huaqin.corp-partner.google.com>
---
M src/mainboard/google/brox/Kconfig
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/83793/1
diff --git a/src/mainboard/google/brox/Kconfig b/src/mainboard/google/brox/Kconfig
index 5044695..567c663 100644
--- a/src/mainboard/google/brox/Kconfig
+++ b/src/mainboard/google/brox/Kconfig
@@ -76,6 +76,7 @@
config BOARD_GOOGLE_LOTSO
select BOARD_GOOGLE_BASEBOARD_BROX
+ select CHROMEOS_WIFI_SAR if CHROMEOS
select USE_UNIFIED_AP_FIRMWARE_FOR_UFS_AND_NON_UFS
config BOARD_GOOGLE_GREENBAYUPOC
--
To view, visit https://review.coreboot.org/c/coreboot/+/83793?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: Ibf5425e72eddc45e376ef4e2d077180dab502200
Gerrit-Change-Number: 83793
Gerrit-PatchSet: 1
Gerrit-Owner: Kun Liu <liukun11(a)huaqin.corp-partner.google.com>
Attention is currently required from: Elyes Haouas, Felix Singer, Jérémy Compostella, Shuo Liu.
Hello Jérémy Compostella, Shuo Liu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83321?usp=email
to look at the new patch set (#10).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC
......................................................................
soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC
Change-Id: I32ad836dfaaff0d1816eac41e5a7d19ece11080f
Signed-off-by: Yuchi Chen <yuchi.chen(a)intel.com>
---
A src/soc/intel/snowridge/Kconfig
A src/soc/intel/snowridge/Makefile.mk
A src/soc/intel/snowridge/acpi.c
A src/soc/intel/snowridge/acpi/hostbridges.asl
A src/soc/intel/snowridge/acpi/ith.asl
A src/soc/intel/snowridge/acpi/lpc.asl
A src/soc/intel/snowridge/acpi/pch_irqs.asl
A src/soc/intel/snowridge/acpi/pci_irqs.asl
A src/soc/intel/snowridge/acpi/pcie.asl
A src/soc/intel/snowridge/acpi/pcie_port.asl
A src/soc/intel/snowridge/acpi/pmc.asl
A src/soc/intel/snowridge/acpi/sata0.asl
A src/soc/intel/snowridge/acpi/sata2.asl
A src/soc/intel/snowridge/acpi/smbus.asl
A src/soc/intel/snowridge/acpi/southcluster.asl
A src/soc/intel/snowridge/acpi/uncore.asl
A src/soc/intel/snowridge/bootblock/bootblock.c
A src/soc/intel/snowridge/bootblock/bootblock.h
A src/soc/intel/snowridge/bootblock/early_uart_init.c
A src/soc/intel/snowridge/chip.c
A src/soc/intel/snowridge/chip.h
A src/soc/intel/snowridge/common/fsp_hob.c
A src/soc/intel/snowridge/common/fsp_hob.h
A src/soc/intel/snowridge/common/gpio.c
A src/soc/intel/snowridge/common/hob_display.c
A src/soc/intel/snowridge/common/kti_cache.c
A src/soc/intel/snowridge/common/kti_cache.h
A src/soc/intel/snowridge/common/pmclib.c
A src/soc/intel/snowridge/common/reset.c
A src/soc/intel/snowridge/common/spi.c
A src/soc/intel/snowridge/common/uart8250mem.c
A src/soc/intel/snowridge/common/uart8250mem.h
A src/soc/intel/snowridge/common/upd_display.c
A src/soc/intel/snowridge/cpu.c
A src/soc/intel/snowridge/finalize.c
A src/soc/intel/snowridge/heci.c
A src/soc/intel/snowridge/hob_iiouds.h
A src/soc/intel/snowridge/hqm.c
A src/soc/intel/snowridge/include/soc/acpi.h
A src/soc/intel/snowridge/include/soc/cpu.h
A src/soc/intel/snowridge/include/soc/gpio.h
A src/soc/intel/snowridge/include/soc/gpio_defs.h
A src/soc/intel/snowridge/include/soc/gpio_snr.h
A src/soc/intel/snowridge/include/soc/gpmr.h
A src/soc/intel/snowridge/include/soc/iomap.h
A src/soc/intel/snowridge/include/soc/irq.h
A src/soc/intel/snowridge/include/soc/itss.h
A src/soc/intel/snowridge/include/soc/lpc.h
A src/soc/intel/snowridge/include/soc/msr.h
A src/soc/intel/snowridge/include/soc/nvs.h
A src/soc/intel/snowridge/include/soc/p2sb.h
A src/soc/intel/snowridge/include/soc/pci_devs.h
A src/soc/intel/snowridge/include/soc/pci_ids.h
A src/soc/intel/snowridge/include/soc/pcr_ids.h
A src/soc/intel/snowridge/include/soc/pm.h
A src/soc/intel/snowridge/include/soc/pmc.h
A src/soc/intel/snowridge/include/soc/sata.h
A src/soc/intel/snowridge/include/soc/smbus.h
A src/soc/intel/snowridge/include/soc/soc_chip.h
A src/soc/intel/snowridge/include/soc/systemagent.h
A src/soc/intel/snowridge/lockdown.c
A src/soc/intel/snowridge/lpc.c
A src/soc/intel/snowridge/memmap.c
A src/soc/intel/snowridge/nis.c
A src/soc/intel/snowridge/qat.c
A src/soc/intel/snowridge/ramstage.h
A src/soc/intel/snowridge/romstage/gpio_snr.c
A src/soc/intel/snowridge/romstage/romstage.c
A src/soc/intel/snowridge/sata.c
A src/soc/intel/snowridge/smihandler.c
A src/soc/intel/snowridge/sriov.c
A src/soc/intel/snowridge/systemagent.c
72 files changed, 5,848 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/83321/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/83321?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: I32ad836dfaaff0d1816eac41e5a7d19ece11080f
Gerrit-Change-Number: 83321
Gerrit-PatchSet: 10
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Attention is currently required from: Felix Singer, Jérémy Compostella, Shuo Liu.
yuchi.chen(a)intel.com has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83317?usp=email )
Change subject: soc/intel/common/block/gpmr: Allow soc to have specific gpmr definition
......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/gpmr.h:
https://review.coreboot.org/c/coreboot/+/83317/comment/ebbeec7e_9584d9f2?us… :
PS3, Line 10: #include <soc/gpmr.h>
> are IOC GPMR and PCR GPMR are totally exclusive?
From the help message in src/soc/intel/common/block/ioc/Kconfig, IOC replaces DMI interface starting from MTL because there is no PCH die, thus I guess they are totally exclusive.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83317?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: I94797a72af75fc96ab2cacb1d46b581605a15387
Gerrit-Change-Number: 83317
Gerrit-PatchSet: 8
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 03:04:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Jakub Czapiga, Julius Werner.
Yu-Ping Wu has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/83765?usp=email )
Change subject: lib/string: Add strncat() and strcat() functions
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS3:
> This is fine, but if you have extra time I think it would also be a good idea to start merging the s […]
I'll leave it for now because of the license issue (don't have time to re-write the inefficient libpayload functions atm).
--
To view, visit https://review.coreboot.org/c/coreboot/+/83765?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: If02fce0eafb4f6fa01d8bab17d87a32360f4ac83
Gerrit-Change-Number: 83765
Gerrit-PatchSet: 4
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 07 Aug 2024 02:35:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Jakub Czapiga, Julius Werner.
Yu-Ping Wu has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/83765?usp=email )
Change subject: lib/string: Add strncat() and strcat() functions
......................................................................
Patch Set 4:
(1 comment)
File src/lib/string.c:
https://review.coreboot.org/c/coreboot/+/83765/comment/ae9fa85d_67e9e060?us… :
PS3, Line 107: strncpy(dst + strlen(dst), src, count);
> This isn't really a great implementation (walks the string twice). Better would be something like: […]
Does it walk the string twice? `strlen(dst)` walks `dst` once, and `strncpy` walks `src` once. BTW libpayload's implementation really walks `src` twice.
However I just found 2 problems in this implementation:
1. `strncpy` may not append a null byte when `count` is small.
2. Unlike `strncpy`, `dst` doesn't need to be padded with zeros (up to `count` bytes).
Therefore I decided to not use `strncpy` here. Also modified the test to catch the problem #1 above.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83765?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: If02fce0eafb4f6fa01d8bab17d87a32360f4ac83
Gerrit-Change-Number: 83765
Gerrit-PatchSet: 4
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 07 Aug 2024 02:33:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>