Elyes Haouas has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83296?usp=email )
Change subject: Revert "util/crossgcc: Update ACPICA from 20230628 to 20240321"
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS3:
> We have the incorrect version on our site as well. So does debian, in their sources, which really surprised me. I would have thought that they'd generate a new tarball based on their own sources, but they just used the one intel released.
Arch Linux also have this issue:
installed version ` acpica 20240321-1`, but when you run `acpibin -v` it shows "20230628"
--
To view, visit https://review.coreboot.org/c/coreboot/+/83296?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: I3ce0c5798f14162eaa063a9a64e16e6dbbb9e468
Gerrit-Change-Number: 83296
Gerrit-PatchSet: 5
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Tue, 09 Jul 2024 10:13:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>
Attention is currently required from: Arthur Heymans, Felix Held, Martin L Roth, Maximilian Brune.
Angel Pons has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83073?usp=email )
Change subject: util/kconfig: Add SOT support to kconfig parser
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/83073?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: I91b5b13a6bbfcdea442d5ef884cf9da11c0cd422
Gerrit-Change-Number: 83073
Gerrit-PatchSet: 2
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: 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-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 09 Jul 2024 10:03:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Benjamin Doron, Elyes Haouas, Martin L Roth, Maximilian Brune, Patrick Rudolph, Sean Rhodes.
Angel Pons has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/77882?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: include/commonlib/sot.h: Add SOT structs
......................................................................
Patch Set 14:
(8 comments)
File Documentation/drivers/sot.md:
https://review.coreboot.org/c/coreboot/+/77882/comment/2da7152c_6e612efe?us… :
PS14, Line 120: Ccontains
```suggestion
Contains an expression `struct sot_expression` that needs to be evaluated in order to
```
https://review.coreboot.org/c/coreboot/+/77882/comment/661216f8_a493fdde?us… :
PS14, Line 122: `struct sot_expr`
What is a `sot_expr`? It's not an expression but part of one. Would it make sense to call it `sot_atom` instead?
https://review.coreboot.org/c/coreboot/+/77882/comment/189cb628_de34c1c6?us… :
PS14, Line 140: EFI_VARIABLE_RUNTIME_ACCESS attribute. It is out of scope of this specification how non runtime
: variables are protected after the payload has hand over control.
nit:
```suggestion
EFI_VARIABLE_RUNTIME_ACCESS attribute. It is out of scope of this specification how non
runtime variables are protected after the payload has hand over control.
```
File src/commonlib/include/commonlib/sot.h:
https://review.coreboot.org/c/coreboot/+/77882/comment/2479344b_0a8ebfa5?us… :
PS14, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
Would it be possible to license this under BSD?
https://review.coreboot.org/c/coreboot/+/77882/comment/9084a0c4_57339f3a?us… :
PS14, Line 51: #define MAX_SOT_SIZE 5000
Units?
https://review.coreboot.org/c/coreboot/+/77882/comment/3bac48ba_6ccff7ef?us… :
PS14, Line 87: SOT_OPTFALG_HEX = 1 << 2, // SOT_TAG_OPTION_NUM_* options are displayed in hex.
```suggestion
SOT_OPTFLAG_HEX = 1 << 2, // SOT_TAG_OPTION_NUM_* options are displayed in hex.
```
https://review.coreboot.org/c/coreboot/+/77882/comment/35b64bc5_be4fd1ea?us… :
PS14, Line 90: typedef uint64_t sot_string; // offset into strings array
`sot_stridx` or `sot_strindex`?
https://review.coreboot.org/c/coreboot/+/77882/comment/acc9d177_74000174?us… :
PS14, Line 103: SOT_TAG_DEFAULT_VISIBLE, SOT_TAG_PROMPT_VISIBLE
I don't see these values anywhere, is this a tag with flags?
--
To view, visit https://review.coreboot.org/c/coreboot/+/77882?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: I44f006318a15556f97da286dc032c58ccb7993e4
Gerrit-Change-Number: 77882
Gerrit-PatchSet: 14
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Tue, 09 Jul 2024 10:02:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Angel Pons has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/83325?usp=email )
Change subject: soc/intel/xeon_sp: Share save_dimm_info among Xeon-SP SoCs
......................................................................
Patch Set 3:
(5 comments)
File src/soc/intel/xeon_sp/cpx/romstage.c:
https://review.coreboot.org/c/coreboot/+/83325/comment/cfcb05bf_3b26dede?us… :
PS3, Line 126: MEMORY_TYPE_DDR4
Note to self: it's equivalent `MEMORY_TYPE_DDR4 = 0x1a`
File src/soc/intel/xeon_sp/gnr/romstage.c:
https://review.coreboot.org/c/coreboot/+/83325/comment/b7afc921_8dfdf134?us… :
PS3, Line 90: uint8_t get_dram_type(void)
: {
: const struct SystemMemoryMapHob *hob = get_system_memory_map();
: if (hob && hob->DramType == SPD_MEMORY_TYPE_DDR5_SDRAM)
The code that has been factored out (made shared) uses `struct SystemMemoryMapHob` already. So, instead of getting it here, why not do:
```suggestion
uint8_t get_dram_type(const struct SystemMemoryMapHob *hob)
{
if (hob->DramType == SPD_MEMORY_TYPE_DDR5_SDRAM)
```
I intentionally removed the NULL check: `save_dimm_info()` already checks that the pointer is non-NULL.
File src/soc/intel/xeon_sp/romstage.c:
https://review.coreboot.org/c/coreboot/+/83325/comment/09fbdc56_b1680f28?us… :
PS3, Line 43: #if CONFIG(SOC_INTEL_SKYLAKE_SP)
Hmmm, instead of this, why not place `save_dimm_info()` in a separate `dimm_info.c` file, and compile it only for platforms that use it?
https://review.coreboot.org/c/coreboot/+/83325/comment/56e8081e_c50c18d7?us… :
PS3, Line 69:
: for (int skt = 0; skt < CONFIG_MAX_SOCKET; skt++) {
: for (int ch = 0; ch < MAX_CH; ch++) {
: for (int dimm = 0; dimm < get_max_dimm_count(); dimm++) {
nit: if `get_max_dimm_count()` always returns the same value, I would add a helper constant:
```suggestion
const int max_dimm_count = get_max_dimm_count();
int dimm_num = 0;
for (int skt = 0; skt < CONFIG_MAX_SOCKET; skt++) {
for (int ch = 0; ch < MAX_CH; ch++) {
for (int dimm = 0; dimm < max_dimm_count; dimm++) {
```
File src/soc/intel/xeon_sp/spr/romstage.c:
https://review.coreboot.org/c/coreboot/+/83325/comment/7a821cc4_e41bf4a9?us… :
PS3, Line 132: uint32_t get_max_capacity_mib(void)
Move this next to the other functions?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83325?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: I2f5b7a4ffabed033d54d4724b3c41246503166fe
Gerrit-Change-Number: 83325
Gerrit-PatchSet: 3
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: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.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: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Jincheng Li <jincheng.li(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: Tue, 09 Jul 2024 09:56:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Angel Pons, Elyes Haouas, Jason Glenesk, Martin L Roth.
Nico Huber has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83384?usp=email )
Change subject: releases/coreboot-24.08: Remove ACPICA line
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83384?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: Id238f77c6a0b4052ae3d835caf98aaf26a7e570f
Gerrit-Change-Number: 83384
Gerrit-PatchSet: 3
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Tue, 09 Jul 2024 09:48:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Nico Huber has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83296?usp=email )
Change subject: Revert "util/crossgcc: Update ACPICA from 20230628 to 20240321"
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
Hmmm, is something wrong with the CI then? Why wasn't it caught,
why do people get different test results. Did anyone figure that
out?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83296?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: I3ce0c5798f14162eaa063a9a64e16e6dbbb9e468
Gerrit-Change-Number: 83296
Gerrit-PatchSet: 5
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Tue, 09 Jul 2024 09:48:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Felix Singer, Nicholas Chin.
Nico Huber has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/83381?usp=email )
Change subject: Documentation/Makefile: Fix test target
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83381?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: Id514950b4486ed8644d078af222c96ed711fc8f9
Gerrit-Change-Number: 83381
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 09:30:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Singer, Nicholas Chin.
Ronald Claveau has posted comments on this change by Ronald Claveau. ( https://review.coreboot.org/c/coreboot/+/83104?usp=email )
Change subject: mainboard/dell: Add new mainboard XPS 8300 (Sandy Bridge)
......................................................................
Patch Set 16:
(2 comments)
File src/mainboard/dell/xps_8300/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/83104/comment/66ccbf0f_032cf181?us… :
PS15, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
> Should be CC-PDDC as per commit cf4722d317ea (src/mb: Update unlicensable files with the CC-PDDC SPD […]
Nice catch, fixed now
File src/mainboard/dell/xps_8300/board.fmd:
PS15:
> Just stumbled upon this. […]
Ok with SeaBIOS, I 'll try again with EDK2 later and adjust if needed.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83104?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: I7d394794fec580bc7aed3f6396ceb47d4a6fd059
Gerrit-Change-Number: 83104
Gerrit-PatchSet: 16
Gerrit-Owner: Ronald Claveau <sousmangoosta(a)aliel.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Tue, 09 Jul 2024 09:20:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>