Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Johnny Lin, Jonathan Zhang, Lance Zhao, Lean Sheng Tan, Nico Huber, Patrick Rudolph, Shuo Liu, Tim Chu, Tim Wawrzynczak.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81652?usp=email )
Change subject: acpi: Move acpigen_write_OSC_pci_domain to Xeon-SP
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
since the acpi code seems to be part of the acpi spec (haven't checked, but the comments in the code suggest that), i'd be in favor of keeping this code in acpi/acpigen.c. i'd however like to not have weak function in the common code if that can be easily avoided which is the case here. i see two possibilities for this: pass the uint32_t osc feature flags as function argument or just remove the weak function implementations and require the soc code calling these functions to implement the get feature functions; socs hat don't call the 3 new functions won't need to implement the get feature functions, since those won't be called so the linker won't need the get feature functions to be resolvable symbols
--
To view, visit https://review.coreboot.org/c/coreboot/+/81652?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: Ia297e0a828f25b751032f18de04ec76f10131076
Gerrit-Change-Number: 81652
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.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: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.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: Wed, 03 Apr 2024 13:24:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jon Murphy, Julius Werner.
Arthur Heymans 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 26:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74501/comment/ae004968_3ca7b0b8 :
PS26, Line 13: config files to achieve the same level of build testing.
> Can you just change the sc7180 memlayout to make a bit more space instead? You can steal a few KB from PRERAM_CBFS_CACHE if necessary.
I was just not aware of the SOC details: loading a bootblock is often soc specific with sometimes size restrictions. If you say it's possible to do so I can for sure try this.
--
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: 26
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: Jon Murphy <jpmurphy(a)google.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 03 Apr 2024 13:10:56 +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: Julius Werner.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80639?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/qualcomm/sc7{1,2}80: Increase romstage/verstage section for clang
......................................................................
soc/qualcomm/sc7{1,2}80: Increase romstage/verstage section for clang
Clang builds slightly larger binaries so increase the section.
The qcsdi is used for an external blob that is currently not in use so
reducing the size is fine for now.
Change-Id: Ide01233f209613678c5408f1afab19415c1071be
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/soc/qualcomm/sc7180/memlayout.ld
M src/soc/qualcomm/sc7280/memlayout.ld
2 files changed, 3 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/80639/6
--
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: 6
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: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Julius Werner.
Arthur Heymans 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 5:
(1 comment)
File src/soc/qualcomm/sc7280/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/80639/comment/6f0a56b8_8cb560ca :
PS5, Line 29: 40192
> I think it would be more readable to write this as `39K + 256` or `39K + 0x100`
Done
--
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: 5
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: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 03 Apr 2024 13:08:18 +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: Jakub Czapiga, Julius Werner, Martin L Roth.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81081?usp=email )
Change subject: lib/device_tree: Add some FDT helper functions
......................................................................
Patch Set 18:
(2 comments)
File src/include/device_tree.h:
https://review.coreboot.org/c/coreboot/+/81081/comment/da16ab56_731cb288 :
PS18, Line 203:
> nit: Please add at least a line of documentation, particularly to clarify here what the return value […]
To all functions?
File src/lib/device_tree.c:
https://review.coreboot.org/c/coreboot/+/81081/comment/b15de1dc_ecd754b8 :
PS18, Line 175: ""
> I don't really understand the `""` here at the start, what does that represent? There shouldn't be a […]
I disagree. The root node is the same as any other node. Its name is an empty string. This function treats all paths as relative paths. Removing it would add quite some boilerplate code (in `fdt_find_node_by_path`) to skip the root node and returns its offset if the user requested "/". It would treat the root node special (even though it isn't).
Using
```
fdt_find_node(blob, root_node_offset, { NULL }, ...
```
will not work, because it would strcmp NULL. I have to put the NULL check after the strcmp in order to return the correct offset.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81081?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: I2fb1d93c5b3e1cb2f7d9584db52bbce3767b63d8
Gerrit-Change-Number: 81081
Gerrit-PatchSet: 18
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
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: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 03 Apr 2024 13:07:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79576?usp=email )
(
7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: vendorcode/amd/opensil: Add CPP args to all stages
......................................................................
vendorcode/amd/opensil: Add CPP args to all stages
It does not hurt to do this and makes it possible to link romstage
sources into bootblock.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: Ic7edfdac43c2d71ee3dcbd9d8f59c9799595e7f8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79576
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/vendorcode/amd/opensil/genoa_poc/Makefile.mk
1 file changed, 1 insertion(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Martin L Roth: Looks good to me, approved
diff --git a/src/vendorcode/amd/opensil/genoa_poc/Makefile.mk b/src/vendorcode/amd/opensil/genoa_poc/Makefile.mk
index 70bf44b..ca58868 100644
--- a/src/vendorcode/amd/opensil/genoa_poc/Makefile.mk
+++ b/src/vendorcode/amd/opensil/genoa_poc/Makefile.mk
@@ -2,8 +2,7 @@
subdirs-y += mpio
-CPPFLAGS_ramstage += -I$(opensil_dir)/Include -I$(opensil_dir)/xUSL -I$(opensil_dir)/xUSL/Include -I$(opensil_dir)/xUSL/FCH -I$(opensil_dir)/xUSL/FCH/Common -I$(opensil_dir)/xSIM -I$(opensil_dir)/xPRF
-CPPFLAGS_romstage += -I$(opensil_dir)/Include -I$(opensil_dir)/xUSL -I$(opensil_dir)/xUSL/Include -I$(opensil_dir)/xSIM -I$(opensil_dir)/xPRF
+CPPFLAGS_common += -I$(opensil_dir)/Include -I$(opensil_dir)/xUSL -I$(opensil_dir)/xUSL/Include -I$(opensil_dir)/xUSL/FCH -I$(opensil_dir)/xUSL/FCH/Common -I$(opensil_dir)/xSIM -I$(opensil_dir)/xPRF
romstage-y += opensil_console.c
romstage-y += romstage.c
--
To view, visit https://review.coreboot.org/c/coreboot/+/79576?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: Ic7edfdac43c2d71ee3dcbd9d8f59c9799595e7f8
Gerrit-Change-Number: 79576
Gerrit-PatchSet: 10
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged