Attention is currently required from: Angel Pons, Arthur Heymans, Jérémy Compostella.
Nico Huber has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84040?usp=email )
Change subject: ext_stage_cache: Make sure variables are initialized
......................................................................
Patch Set 7: Code-Review+1
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84040/comment/9dca9c8b_655e226d?us… :
PS7, Line 10:
Maybe add: `Also exit gracefully from stage-cache code if no smm region is found.`
Actually, I'd prefer if we can solve this puzzle semantically, though. i.e.
make sure `smm_subregion(SMM_SUBREGION_CACHE, ...` can't fail.
File src/cpu/x86/smm/tseg_region.c:
https://review.coreboot.org/c/coreboot/+/84040/comment/0cbed9da_f34c5768?us… :
PS7, Line 34: return -1;
Hmmm, do we ever link this path? Because here start/size would
indeed have been uninitialized.
If not we could put `dead_code();` here.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84040?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: Ib1851295646258e97c489dc7402b9df3fcf092c1
Gerrit-Change-Number: 84040
Gerrit-PatchSet: 7
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 12:05:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Hung-Te Lin, Xi Chen, Yu-Ping Wu.
Yidi Lin has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/84053?usp=email )
Change subject: soc/mediatek: Require MCU and DRAM blobs to exist
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84053?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: I755e7c5a70b34b0c3d3915ab339c65263688aad7
Gerrit-Change-Number: 84053
Gerrit-PatchSet: 4
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 11:55:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Jakub Czapiga, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Nico Huber has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84039?usp=email )
Change subject: cbmem.h: Change return type of cbmem_get_region
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84039/comment/df136086_954a6f99?us… :
PS6, Line 13: checked for NULL later.
So it realized thanks to LTO that we didn't consistently initialize
variables, is that right? That would make this change actually fix
something and worth to mention.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84039?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: Ib3e09a75380faf9f533601368993261f042422ef
Gerrit-Change-Number: 84039
Gerrit-PatchSet: 7
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 11:51:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
SH Kim has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84058?usp=email )
Change subject: Revert "mb/google/brya/var/xol: Change touchpad I2C interrupt type to GPIO_INT"
......................................................................
Revert "mb/google/brya/var/xol: Change touchpad I2C interrupt type to GPIO_INT"
This reverts commit aa6865291a7ddfae4c67fcfc55ebd0c13a376807.
Reason for revert: We applied this patch for touchpad stuttering issue
for XOl, but the same touchpad problem was reported. So we would revert
this change and apply kernel patch (crrev/c/5808335) to avoid the
touchpad issue.
Change-Id: I78139932e76dbd4128fb325dd70b7dcff3bcc81c
Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.corp-partner.google.com>
---
M src/mainboard/google/brya/variants/xol/gpio.c
M src/mainboard/google/brya/variants/xol/overridetree.cb
2 files changed, 2 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/84058/1
diff --git a/src/mainboard/google/brya/variants/xol/gpio.c b/src/mainboard/google/brya/variants/xol/gpio.c
index 9e49784..39478a1 100644
--- a/src/mainboard/google/brya/variants/xol/gpio.c
+++ b/src/mainboard/google/brya/variants/xol/gpio.c
@@ -119,13 +119,6 @@
PAD_NC(GPP_F12, NONE),
/* F13 : GSXDOUT ==> NC */
PAD_NC(GPP_F13, NONE),
- /* F14 : GSXDIN ==> TCHPAD_INT_ODL */
- /*
- * FIXME: Change back the interrupt type to IRQxAPIC if possible after investigating
- * the reason why Xol shows touchpad stuttering issue with IRQxAPIC
- * configuration but not GPI_INT.
- */
- PAD_CFG_GPI_INT(GPP_F14, NONE, PWROK, LEVEL),
/* F15 : GSXSRESET# ==> PU 100K 3.3V */
PAD_CFG_GPI(GPP_F15, NONE, DEEP),
/* F16 : GSXCLK ==> NC */
diff --git a/src/mainboard/google/brya/variants/xol/overridetree.cb b/src/mainboard/google/brya/variants/xol/overridetree.cb
index 529cb37..914940b 100644
--- a/src/mainboard/google/brya/variants/xol/overridetree.cb
+++ b/src/mainboard/google/brya/variants/xol/overridetree.cb
@@ -341,11 +341,8 @@
chip drivers/i2c/hid
register "generic.hid" = ""ZNT0000""
register "generic.desc" = ""Zinitix Touchpad""
- # FIXME: Change back the interrupt type to IRQxAPIC if possible
- # after investigating the reason why Xol shows touchpad
- # stuttering issue with IRQxAPIC configuration but not
- # GPI_INT.
- register "generic.irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW_WAKE(GPP_F14)"
+ register "generic.irq" = "ACPI_IRQ_WAKE_LEVEL_LOW(GPP_F14_IRQ)"
+ register "generic.wake" = "GPE0_DW2_14"
register "generic.detect" = "1"
register "hid_desc_reg_offset" = "0xE"
device i2c 40 on end
--
To view, visit https://review.coreboot.org/c/coreboot/+/84058?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: I78139932e76dbd4128fb325dd70b7dcff3bcc81c
Gerrit-Change-Number: 84058
Gerrit-PatchSet: 1
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Attention is currently required from: Jérémy Compostella, Nico Huber.
Arthur Heymans has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84037?usp=email )
Change subject: cpu/x86: Don't do partial linking
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> Tested yet?
yes this works on qemu iirc.!
--
To view, visit https://review.coreboot.org/c/coreboot/+/84037?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: If2c186eb87072e0b80c7e8998b2a0d9bdfddf740
Gerrit-Change-Number: 84037
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 11:44:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84057?usp=email )
Change subject: cpu/x86/64bit: Compile and link separately
......................................................................
cpu/x86/64bit: Compile and link separately
When clang supports linking bare metal targets it has troubles doing so
for page tables. So instead compile and link in separate phases.
Change-Id: I66fb374a456ea752a97a41426c5a98e6747f3a92
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/cpu/x86/64bit/Makefile.mk
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/84057/1
diff --git a/src/cpu/x86/64bit/Makefile.mk b/src/cpu/x86/64bit/Makefile.mk
index 1fda087..e1d6f32 100644
--- a/src/cpu/x86/64bit/Makefile.mk
+++ b/src/cpu/x86/64bit/Makefile.mk
@@ -13,9 +13,10 @@
# Add --defsym=_start=0 to suppress a linker warning.
$(objcbfs)/pt: $(dir)/$(PAGETABLE_SRC) $(obj)/config.h
- $(CC_bootblock) $(CFLAGS_bootblock) $(CPPFLAGS_bootblock) -o $@.tmp $< -Wl,--section-start=.rodata=$(CONFIG_ARCH_X86_64_PGTBL_LOC),--defsym=_start=0
+ $(CC_bootblock) $(CFLAGS_bootblock) $(CPPFLAGS_bootblock) -c -o $@.o $<
+ $(LD_bootblock) $@.o -o $@.tmp --section-start=.rodata=$(CONFIG_ARCH_X86_64_PGTBL_LOC) --defsym=_start=0
$(OBJCOPY_ramstage) -Obinary -j .rodata $@.tmp $@
- rm $@.tmp
+ rm $@.o
cbfs-files-$(CONFIG_PAGE_TABLES_IN_CBFS) += pagetables
pagetables-file := $(objcbfs)/pt
--
To view, visit https://review.coreboot.org/c/coreboot/+/84057?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: I66fb374a456ea752a97a41426c5a98e6747f3a92
Gerrit-Change-Number: 84057
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Arthur Heymans.
Nico Huber has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84038?usp=email )
Change subject: lib/rmodules: Add support for LTO
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84038?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: I9cdda036f330486370e8c4120be5b6a0fd982e99
Gerrit-Change-Number: 84038
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 11:37:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Jérémy Compostella.
Nico Huber has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84037?usp=email )
Change subject: cpu/x86: Don't do partial linking
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84037/comment/299b067b_693e9c38?us… :
PS2, Line 7: cpu/x86
`cpu/x86/smm:`
--
To view, visit https://review.coreboot.org/c/coreboot/+/84037?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: If2c186eb87072e0b80c7e8998b2a0d9bdfddf740
Gerrit-Change-Number: 84037
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 11:36:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Angel Pons, Arthur Heymans.
Nico Huber has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84011?usp=email )
Change subject: nvramcui: Fix main function signature
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
File payloads/nvramcui/nvramcui.c:
https://review.coreboot.org/c/coreboot/+/84011/comment/fae44483_422b81df?us… :
PS1, Line 162: *argv
> > I've seen both, I prefer `char *argv[]` because it better conveys intent (`argv` is an array of st […]
Now I found it, it's declared in .c files (libpayload/arch/<arch>/main.c)... Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/84011?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: Ia0c50224bd70503e884573fedf3bf33c134bba00
Gerrit-Change-Number: 84011
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 11:03:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>