Attention is currently required from: Arthur Heymans.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74501?usp=email )
Change subject: arch/arm64: Add Clang as supported target
......................................................................
Patch Set 23:
(3 comments)
Patchset:
PS21:
> @Julius: configs/config.google_trogdor.build_test makes the bootblock too tight for clang. […]
Trogdor should still have plenty of space, we just need to shuffle things around a bit. The 60K PRERAM_CBFS_CACHE was just a leftover from the (aborted) attempt of compressing QcLib with LZMA, IIRC. Since we eventually settled on LZ4 it should not need to be nearly as big and you should be able to steal 10-20K from there as needed.
File src/arch/arm64/Kconfig:
https://review.coreboot.org/c/coreboot/+/74501/comment/ad45de72_bbb657c5 :
PS23, Line 7: default y if ARCH_ARM64
Should move under the `if` below.
File src/soc/qualcomm/sc7180/Kconfig:
https://review.coreboot.org/c/coreboot/+/74501/comment/2ef56f4d_6e48b592 :
PS23, Line 27: default n if SOC_QUALCOMM_SC7180 && CBFS_VERIFICATION && TPM_MEASURED_BOOT
ditto
--
To view, visit https://review.coreboot.org/c/coreboot/+/74501?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: I940a1ccf5cc4ec7bed5b6c8be92fc47922e1e747
Gerrit-Change-Number: 74501
Gerrit-PatchSet: 23
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:55:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Paul Menzel.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69747?usp=email )
Change subject: arch/arm: Build test all arm targets with clang
......................................................................
Patch Set 20:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/69747/comment/02afd7cb_0987ab34 :
PS20, Line 7: arch/arm: Build test all arm targets with clang
This says "build test" but it looks like this CL is more about adding the ARCH_SUPPORTS_CLANG Kconfig?
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/69747/comment/297de493_74513037 :
PS20, Line 1544: default n
Should this be set for x86 somewhere as well? I thought that was already supporting clang?
File src/soc/nvidia/tegra124/Kconfig:
https://review.coreboot.org/c/coreboot/+/69747/comment/6f6ba39c_991636aa :
PS20, Line 20: default n if SOC_NVIDIA_TEGRA124 && CHROMEOS
Why not move these into the `if SOC_...` blocks below?
File src/soc/qualcomm/ipq806x/Kconfig:
https://review.coreboot.org/c/coreboot/+/69747/comment/842ec24a_7c4de807 :
PS20, Line 16: SOC_NVIDIA_TEGRA124
Wrong SoC
--
To view, visit https://review.coreboot.org/c/coreboot/+/69747?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: I88cf8ce16fb6c61c19d615e396f5871179b06fc8
Gerrit-Change-Number: 69747
Gerrit-PatchSet: 20
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:51:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Martin L Roth.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75031?usp=email )
Change subject: arch/arm/armv{4,7}/Makefile.inc: Add more finegrained ld-ccopts
......................................................................
Patch Set 11:
(3 comments)
Patchset:
PS6:
> It seems kinda silly that we separate CFLAGS and CPPFLAGS into two different variables, and then we […]
Seems to have been resolved?
File src/arch/arm/armv4/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/75031/comment/46360d16_2c06f8b5 :
PS11, Line 20: -target arm-eabi
Generally, I feel like this kind of stuff belongs in xcompile, not in the architecture Makefile. That's what xcompile is meant for, to figure out the right flags for whatever compiler the user has to do what coreboot expects. Target ABI is supposed to be a property of the compiler (and for GCC it usually is), so if clang needs an extra flag to tell it that xcompile should set it.
In fact, there already seems to be a line in xcompile that tries to do something like this:
```
CFLAGS_CLANG="-target ${clang_arch}-${TABI} $CFLAGS_CLANG"
```
Can't we make that work for this?
https://review.coreboot.org/c/coreboot/+/75031/comment/3946635b_677998db :
PS11, Line 34: verstage-ld-ccopts += -target arm-eabi
nit: this would probably look cleaner if it always came right after the generic-ccopts.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75031?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: I4340681e30059d6f18a49a49937668cd3dd39ce1
Gerrit-Change-Number: 75031
Gerrit-PatchSet: 11
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:47:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Cliff Huang, Hannah Williams, Martin L Roth, Stefan Reinauer, Zhixing Ma.
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80726?usp=email )
Change subject: payloads/depthcharge: Add DEPTHCHARGE_REPO and DEPTHCHARGE_BRANCH
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
Could you add more detail info for intention for this change like below.
This change enable to use mirrored internal depthcharge repo and branch for early SOC development (before upstreaming SOC and dephthcharge code)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80726?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: Icca10aa770b7b7a6e010f58bcf1e4f0a3401681a
Gerrit-Change-Number: 80726
Gerrit-PatchSet: 1
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:34:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80714?usp=email )
Change subject: util/amdfwtool: build amdfwtool only for all tools or AMD CPUs
......................................................................
Patch Set 2:
(1 comment)
File util/amdfwtool/Makefile:
https://review.coreboot.org/c/coreboot/+/80714/comment/7bfa4a4f_4adbfdcc :
PS2, Line 20: BUILD_ALL_TOOLS := 1
Do we need this? IIUC the build targets in this makefile here will apply when a user comes into this path and invoke `make ${TARGET}`. Please educate me if I am missing a bigger picture.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80714?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: I9021674a06d65a79e24020790d317ab947c505fe
Gerrit-Change-Number: 80714
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:33:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80639?usp=email )
Change subject: soc/qualcomm/sc7{1,2}80: Increase romstage/verstage section for clang
......................................................................
Patch Set 3:
(2 comments)
File src/soc/qualcomm/sc7180/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/80639/comment/b0b678aa_ab824e33 :
PS3, Line 27: REGION(qcsdi, 0x1469C000, 55K, 4K)
You're actually adjusting this downward here, you don't need to do that. There's intentionally some space left so the romstage can be grown if needed.
File src/soc/qualcomm/sc7280/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/80639/comment/7dac169c_beffbfb8 :
PS3, Line 30: REGION(modem_id, 0x146A7000, 4, 4)
Unfortunately modem_id is used as a communication buffer with an external blob and cannot be moved independently. I think the best thing you can do here is reduce qcsdi to the remaining size, and if anyone ever needs it again (which will likely be never) they'll have to deal with the potential fallout.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80639?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: Ide01233f209613678c5408f1afab19415c1071be
Gerrit-Change-Number: 80639
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:27:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80714?usp=email )
Change subject: util/amdfwtool: build amdfwtool only for all tools or AMD CPUs
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80714?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: I9021674a06d65a79e24020790d317ab947c505fe
Gerrit-Change-Number: 80714
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:23:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier.
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80714?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: util/amdfwtool: build amdfwtool only for all tools or AMD CPUs
......................................................................
Patch Set 2:
(2 comments)
File Makefile:
https://review.coreboot.org/c/coreboot/+/80714/comment/0c29eb02_66cceca7 :
PS2, Line 143:
: ifneq ($(filter tools, $(MAKECMDGOALS)), )
: BUILD_ALL_TOOLS:=1
: endif
What does this do? A comment about the intention would be nice
File util/amdfwtool/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/80714/comment/a51e1ff0_c6c55af0 :
PS2, Line 31: endif #($(CONFIG_USE_AMDFWTOOL),y)
#($(BUILD_ALL_TOOLS)$(CONFIG_USE_AMDFWTOOL),)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80714?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: I9021674a06d65a79e24020790d317ab947c505fe
Gerrit-Change-Number: 80714
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 23 Feb 2024 00:17:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Xi Chen, Yidi Lin, Yu-Ping Wu.
Hello Hung-Te Lin, Xi Chen, Yu-Ping Wu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80687?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/mediatek: Move `get_sdram_config` to common code
......................................................................
soc/mediatek: Move `get_sdram_config` to common code
Starting from MT8195, MediaTek platform supports "dram adaptive"
feature. So we can just pass a placeholder param blob in
`get_sdram_config` by default. The platform which does not support
"dram adaptive" can override `get_sdram_config` and implement it in the
mainboard folder.
TEST=emerge-geralt coreboot
Change-Id: I05a01b1ab13fbf19b2a908c48a540a5c2e1ccbdc
Signed-off-by: Yidi Lin <yidilin(a)chromium.org>
---
M src/mainboard/google/cherry/Makefile.mk
D src/mainboard/google/cherry/sdram_configs.c
M src/mainboard/google/corsola/Makefile.mk
D src/mainboard/google/corsola/sdram_configs.c
M src/mainboard/google/geralt/Makefile.mk
D src/mainboard/google/geralt/sdram_configs.c
M src/soc/mediatek/common/include/soc/dramc_param_common.h
A src/soc/mediatek/common/sdram_configs.c
M src/soc/mediatek/mt8186/Makefile.mk
M src/soc/mediatek/mt8186/include/soc/dramc_param.h
M src/soc/mediatek/mt8188/Makefile.mk
M src/soc/mediatek/mt8188/include/soc/dramc_param.h
M src/soc/mediatek/mt8192/include/soc/dramc_param.h
M src/soc/mediatek/mt8195/Makefile.mk
M src/soc/mediatek/mt8195/include/soc/dramc_param.h
15 files changed, 20 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/80687/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80687?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: I05a01b1ab13fbf19b2a908c48a540a5c2e1ccbdc
Gerrit-Change-Number: 80687
Gerrit-PatchSet: 2
Gerrit-Owner: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: newpatchset