Attention is currently required from: Jakub Czapiga, Kapil Porwal, Tarun Tuli.
Hello Jakub Czapiga, Kapil Porwal, Tarun Tuli,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/76001?usp=email
to look at the new patch set (#2).
Change subject: mb/google/rex: Keep CNVi PCI device enabled for Ovis
......................................................................
mb/google/rex: Keep CNVi PCI device enabled for Ovis
The CNVi PCI device is required for the system to boot properly.
By ensuring that this device is enabled, we can prevent the below
error message from appearing and ensure that the system boots successfully.
BUG=b:274421383
TEST=Able to build and boot google/ovis without any error.
w/o this patch:
[ERROR] CNVi WiFi is enabled without CNVi being enabled
[ERROR] CNVi BT is enabled without CNVi being enabled
Change-Id: I4dbae14f0cfccf96a33437a0e2fdefb508209354
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/mainboard/google/rex/variants/ovis/overridetree.cb
1 file changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/76001/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/76001?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4dbae14f0cfccf96a33437a0e2fdefb508209354
Gerrit-Change-Number: 76001
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak.
Hello Lance Zhao, Tim Wawrzynczak, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/76000?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: acpi: Swap XSDT and RSDT in acpi_add_table()
......................................................................
acpi: Swap XSDT and RSDT in acpi_add_table()
If ACPI is above 4G it's not possible to have a valid RSDT pointer in
RSDP, therefore swap RSDT and XSDT. Both are always generated on x86.
On other architecures RSDT is often skipped, e.g. aarch64. On top of
that the OS looks at XSDT first. So unconditionally using XSDT and not
RSDT is fine.
This also deal with the ACPI pointer being above 4G. This currently
never happens with x86 platforms.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: I6588676186faa896b6076f871d7f8f633db21e70
---
M src/acpi/acpi.c
1 file changed, 25 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/76000/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/76000?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6588676186faa896b6076f871d7f8f633db21e70
Gerrit-Change-Number: 76000
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76001?usp=email )
Change subject: mb/google/rex: Keep CNVi PCI device enabled for Ovis
......................................................................
mb/google/rex: Keep CNVi PCI device enabled for Ovis
The CNVi PCI device is required for the system to boot properly.
By ensuring that this device is enabled, we can prevent the below error
message from appearing and ensure that the system boots successfully.
w/o this patch:
[ERROR] CNVi WiFi is enabled without CNVi being enabled
[ERROR] CNVi BT is enabled without CNVi being enabled
Change-Id: I4dbae14f0cfccf96a33437a0e2fdefb508209354
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/mainboard/google/rex/variants/ovis/overridetree.cb
1 file changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/76001/1
diff --git a/src/mainboard/google/rex/variants/ovis/overridetree.cb b/src/mainboard/google/rex/variants/ovis/overridetree.cb
index 1af0a0e..a6d5c04 100644
--- a/src/mainboard/google/rex/variants/ovis/overridetree.cb
+++ b/src/mainboard/google/rex/variants/ovis/overridetree.cb
@@ -185,6 +185,14 @@
end
end
end
+ device ref cnvi_wifi on
+ chip drivers/wifi/generic
+ register "wake" = "GPE0_PME_B0"
+ register "add_acpi_dma_property" = "true"
+ register "enable_cnvi_ddr_rfim" = "true"
+ device generic 0 on end
+ end
+ end
device ref i2c4 on
chip drivers/i2c/tpm
register "hid" = ""GOOG0005""
--
To view, visit https://review.coreboot.org/c/coreboot/+/76001?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4dbae14f0cfccf96a33437a0e2fdefb508209354
Gerrit-Change-Number: 76001
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Felix Held, Felix Singer, Jan Samek, Martin L Roth, Nicholas Chin.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75893?usp=email )
Change subject: doc/Makefile: Fix build dir setting
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> The reason for the original was to supply a default value for BUILDDIR at the coreboot makefile level, instead of the SPHINX level, without editing Makefile.sphinx.
Sorry, missed that part (was too focused on the last of Gerrit's mails I read).
I guess I understand the intention. But wonder why would we need the "default
at the coreboot makefile level"? Especially, if it's the same value `_build`.
Was there some later patch supposed to change any of the defaults?
--
To view, visit https://review.coreboot.org/c/coreboot/+/75893?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibc44134cf1996592597252aeb9dcf7ffb3378ee3
Gerrit-Change-Number: 75893
Gerrit-PatchSet: 4
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
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: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 22 Jun 2023 11:54:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Felix Singer, Jan Samek, Martin L Roth, Nicholas Chin.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75893?usp=email )
Change subject: doc/Makefile: Fix build dir setting
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> Marking as unresolved until a consensus about the best way forward is reached.
I guess my question was: If the other commit added SPHINXDIR and this
renames it to BUILDDIR (which was already possible to override originally),
and we have now effectively `BUILDDIR="$(BUILDDIR)"`, what difference
beside boilerplate does it make to reverting?
Martin, do you remember if the `mkdir -p` was added to fix something
or just for completeness?
--
To view, visit https://review.coreboot.org/c/coreboot/+/75893?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibc44134cf1996592597252aeb9dcf7ffb3378ee3
Gerrit-Change-Number: 75893
Gerrit-PatchSet: 4
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
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: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 22 Jun 2023 11:48:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76000?usp=email )
Change subject: acpi: Swap XSDT and RSDT in acpi_add_table()
......................................................................
acpi: Swap XSDT and RSDT in acpi_add_table()
If ACPI is above 4G it's not possible to have a valid RSDT pointer in
RSDP, therefore swap RSDT and XSDT. Both are always generated on x86.
On other architecures RSDT is often skipped, e.g. aarch64. On top of
that the OS looks at XSDT first. So unconditionally using XSDT and not
RSDT is fine.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: I6588676186faa896b6076f871d7f8f633db21e70
---
M src/acpi/acpi.c
1 file changed, 25 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/76000/1
diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c
index aa1cd1c..5e03bf4 100644
--- a/src/acpi/acpi.c
+++ b/src/acpi/acpi.c
@@ -57,18 +57,15 @@
acpi_rsdt_t *rsdt;
acpi_xsdt_t *xsdt = NULL;
- /* The RSDT is mandatory... */
+ /* RSDT may not be valid if ACPI is > 4GiB */
rsdt = (acpi_rsdt_t *)(uintptr_t)rsdp->rsdt_address;
-
- /* ...while the XSDT is not. */
- if (rsdp->xsdt_address)
- xsdt = (acpi_xsdt_t *)((uintptr_t)rsdp->xsdt_address);
+ xsdt = (acpi_xsdt_t *)((uintptr_t)rsdp->xsdt_address);
/* This should always be MAX_ACPI_TABLES. */
- entries_num = ARRAY_SIZE(rsdt->entry);
+ entries_num = ARRAY_SIZE(xsdt->entry);
for (i = 0; i < entries_num; i++) {
- if (rsdt->entry[i] == 0)
+ if (xsdt->entry[i] == 0)
break;
}
@@ -78,36 +75,36 @@
return;
}
- /* Add table to the RSDT. */
- rsdt->entry[i] = (uintptr_t)table;
+ /* Add table to the XSDT. */
+ xsdt->entry[i] = (u64)(uintptr_t)table;
/* Fix RSDT length or the kernel will assume invalid entries. */
- rsdt->header.length = sizeof(acpi_header_t) + (sizeof(u32) * (i + 1));
+ xsdt->header.length = sizeof(acpi_header_t) + (sizeof(u64) * (i + 1));
/* Re-calculate checksum. */
- rsdt->header.checksum = 0; /* Hope this won't get optimized away */
- rsdt->header.checksum = acpi_checksum((u8 *)rsdt, rsdt->header.length);
+ xsdt->header.checksum = 0; /* Hope this won't get optimized away */
+ xsdt->header.checksum = acpi_checksum((u8 *)xsdt, xsdt->header.length);
/*
- * And now the same thing for the XSDT. We use the same index as for
+ * And now the same thing for the RSDT. We use the same index as for
* now we want the XSDT and RSDT to always be in sync in coreboot.
*/
- if (xsdt) {
- /* Add table to the XSDT. */
- xsdt->entry[i] = (u64)(uintptr_t)table;
+ if (rsdt) {
+ /* Add table to the RSDT. */
+ rsdt->entry[i] = (u32)(uintptr_t)table;
- /* Fix XSDT length. */
- xsdt->header.length = sizeof(acpi_header_t) +
- (sizeof(u64) * (i + 1));
+ /* Fix RSDT length. */
+ rsdt->header.length = sizeof(acpi_header_t) +
+ (sizeof(u32) * (i + 1));
/* Re-calculate checksum. */
- xsdt->header.checksum = 0;
- xsdt->header.checksum = acpi_checksum((u8 *)xsdt,
- xsdt->header.length);
+ rsdt->header.checksum = 0;
+ rsdt->header.checksum = acpi_checksum((u8 *)rsdt,
+ rsdt->header.length);
}
printk(BIOS_DEBUG, "ACPI: added table %d/%d, length now %d\n",
- i + 1, entries_num, rsdt->header.length);
+ i + 1, entries_num, xsdt->header.length);
}
static int acpi_create_mcfg_mmconfig(acpi_mcfg_mmconfig_t *mmconfig, u32 base,
@@ -1967,9 +1964,11 @@
coreboot_rsdp = (uintptr_t)rsdp;
current += sizeof(acpi_rsdp_t);
current = acpi_align_current(current);
- rsdt = (acpi_rsdt_t *)current;
- current += sizeof(acpi_rsdt_t);
- current = acpi_align_current(current);
+ if (current < 4ULL * GiB) {
+ rsdt = (acpi_rsdt_t *)current;
+ current += sizeof(acpi_rsdt_t);
+ current = acpi_align_current(current);
+ }
xsdt = (acpi_xsdt_t *)current;
current += sizeof(acpi_xsdt_t);
current = acpi_align_current(current);
--
To view, visit https://review.coreboot.org/c/coreboot/+/76000?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6588676186faa896b6076f871d7f8f633db21e70
Gerrit-Change-Number: 76000
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange
Attention is currently required from: Arthur Heymans.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75925?usp=email )
Change subject: mb/qemu/aarch64: Add PCI support
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/75925?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iad5d87731066a4009d2c4930a01bc15543d9447a
Gerrit-Change-Number: 75925
Gerrit-PatchSet: 9
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
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: Thu, 22 Jun 2023 11:43:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner, Martin L Roth, Patrick Georgi, Paul Menzel.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75879?usp=email )
Change subject: Docs/contrib/coding_style: Document the preference for if() vs #if
......................................................................
Patch Set 1:
(1 comment)
File Documentation/contributing/coding_style.md:
https://review.coreboot.org/c/coreboot/+/75879/comment/1a5a8d6c_dcdfeecc :
PS1, Line 539: ifdef
> Should we go further and say that #ifdef is just forbidden entirely? With the way CONFIG() works in […]
I would say no `#ifdef` in `.c` files. IIRC, `#ifdef __SIMPLE_DEVICE__`
was never meant to be used there either.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75879?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I995e3765ce996d9a83efb34be6f905846357f981
Gerrit-Change-Number: 75879
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Thu, 22 Jun 2023 11:26:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75876?usp=email )
Change subject: device/resource_allocator_v4: Remove "ERROR: " from log message
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75876/comment/6d92e93a_57a28a30 :
PS3, Line 9: It is no longer necessary to explicitly add "ERROR: "
> It is referring to https://review.coreboot. […]
Which was made conditional not much later CB:63144.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75876?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3ff2081d38f94556481efa02f242795bbfc77517
Gerrit-Change-Number: 75876
Gerrit-PatchSet: 3
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.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: Thu, 22 Jun 2023 10:44:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76048?usp=email )
Change subject: device/resource_allocator_v4: Restor back indentation
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Please keep an eye on the conflicting changes. I don't
think there is anything left to do.
--
To view, visit https://review.coreboot.org/c/coreboot/+/76048?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I377b84bc7c8be32a697cdb6af1823b43c4451a81
Gerrit-Change-Number: 76048
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Thu, 22 Jun 2023 10:42:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment