Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86502?usp=email )
Change subject: mb/starlabs/{lite_adl,byte_adl}: Disable CNVi Audio Offload
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/86502?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: Idcdbd7cc83eda50ece74ce823bef60b16b49600c
Gerrit-Change-Number: 86502
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 18 Feb 2025 20:35:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Attention is currently required from: Ana Carolina Cabral.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/86084?usp=email
to look at the new patch set (#16).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: mb/amd/birman_plus/ec: Rectify ECRAM register bits
......................................................................
mb/amd/birman_plus/ec: Rectify ECRAM register bits
Rectify wrong EC module RAM register bits
based on PI source code 1.0.0.1b
Change-Id: I1a13d99a55a4aa02a5cb0e67ffa4ed555f91a471
Signed-off-by: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
---
M src/mainboard/amd/birman_plus/ec.c
1 file changed, 56 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/86084/16
--
To view, visit https://review.coreboot.org/c/coreboot/+/86084?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1a13d99a55a4aa02a5cb0e67ffa4ed555f91a471
Gerrit-Change-Number: 86084
Gerrit-PatchSet: 16
Gerrit-Owner: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Attention is currently required from: Sean Rhodes.
Matt DeVillier has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86400?usp=email )
Change subject: drivers/usb/acpi: Account for GPIO polarity
......................................................................
Patch Set 8:
(1 comment)
File src/drivers/usb/acpi/chip.h:
https://review.coreboot.org/c/coreboot/+/86400/comment/f6389647_8b4723cc?us… :
PS7, Line 95: void acpi_device_intel_bt_common(const struct acpi_gpio *reset_gpio,
: const struct acpi_gpio *enable_gpio);
> https://review.coreboot.org/c/coreboot/+/86400/7/src/drivers/usb/acpi/intel…. […]
can you be more specific? the function params are changed from int to struct acpi_gpio, and the functions to which the reset/enable params are passed are changed to ones which take the structs as params.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86400?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: Ib481d49d536b702fef149af882209501c61de6da
Gerrit-Change-Number: 86400
Gerrit-PatchSet: 8
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 18 Feb 2025 18:59:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Attention is currently required from: Boris Mittelberg, Caveh Jalali, Dinesh Gehlot, Eric Lai, Ivy Jian, Jayvik Desai, John Su, Kapil Porwal, Nick Vaccaro, Subrata Banik, Tony Huang, Wisley Chen.
Paul Menzel has posted comments on this change by John Su. ( https://review.coreboot.org/c/coreboot/+/86493?usp=email )
Change subject: mb/google/brya/var/agah: Separate the AGAH DPTF OEM variant
......................................................................
Patch Set 3:
(1 comment)
File src/ec/google/chromeec/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/86493/comment/4f54b2c4_c05ff07d?us… :
PS2, Line 383: #ifdef DPTF_ENABLE_OEM_VARIABLES_PROJECT_AGAH
: Local0 = ToInteger(EOVD) & EC_OEM_VARIABLE_DATA_MASK
: \_SB.DPTF.ODUP(0, Local0)
: Local0 = \_SB.DPTF.ODGT(0)
: \_SB.DPTF.ODVP()
: Notify (\_SB.DPTF, INT3400_ODVP_CHANGED)
: #endif
> Because the AGAH EC code is based on monitoring adapter current to choose the corresponding DPTF OEM […]
Is that person in the Cc list? It’d be great if you amended the commit message and added more details.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86493?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: I2929eaa65a518b06f32e33cc31ae4a01bcfb77e8
Gerrit-Change-Number: 86493
Gerrit-PatchSet: 3
Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tony Huang <tony-huang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: John Su <john_su(a)compal.corp-partner.google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Tony Huang <tony-huang(a)quanta.corp-partner.google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com>
Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Tue, 18 Feb 2025 18:48:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: John Su <john_su(a)compal.corp-partner.google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Sean Rhodes.
Jérémy Compostella has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86400?usp=email )
Change subject: drivers/usb/acpi: Account for GPIO polarity
......................................................................
Patch Set 8:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/86400/comment/c78e4f92_dc60deda?us… :
PS7, Line 9: Whilst the GPIO's used for Intel Bluetooth should always be
Line length for commit message is usually 72.
File src/drivers/usb/acpi/chip.h:
https://review.coreboot.org/c/coreboot/+/86400/comment/50f5609d_1b13a6d5?us… :
PS7, Line 95: void acpi_device_intel_bt_common(const struct acpi_gpio *reset_gpio,
: const struct acpi_gpio *enable_gpio);
> I'm not seeing any instances which were not properly updated in the patch, is there something I'm no […]
https://review.coreboot.org/c/coreboot/+/86400/7/src/drivers/usb/acpi/intel…
--
To view, visit https://review.coreboot.org/c/coreboot/+/86400?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: Ib481d49d536b702fef149af882209501c61de6da
Gerrit-Change-Number: 86400
Gerrit-PatchSet: 8
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 18 Feb 2025 18:48:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Attention is currently required from: Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro.
Paul Menzel has posted comments on this change by Dinesh Gehlot. ( https://review.coreboot.org/c/coreboot/+/86422?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/google/brya: Do not select HAVE_ACPI_RESUME
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/86422/comment/43ee0c64_33eed786?us… :
PS1, Line 9: Brya mainboard does not reliably support S3 entry/exit.
> Its a configuration, system will not suspend to S3 but s0ix, enabling quicker resume. […]
Please update the commit message then. It says, ACPI S3 could work.
Is it a hardware or software (firmware) limitation.
https://review.coreboot.org/c/coreboot/+/86422/comment/4f375b84_8dfe4660?us… :
PS1, Line 10: Also trigger a fail-safe board
: reset if the system resumes from S3.
> These changes are designed to work in conjunction and are best submitted as a single commit. […]
Sorry for not being clear in my comment. How can Chromebook with ChromeOS go into ACPI S3 if it’s not advertised by firmware?
--
To view, visit https://review.coreboot.org/c/coreboot/+/86422?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: Ic0dce9c7779333ca079001e3763e843a4aad9a81
Gerrit-Change-Number: 86422
Gerrit-PatchSet: 2
Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Tue, 18 Feb 2025 18:46:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Dinesh Gehlot <digehlot(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Intel coreboot Reviewers, Sean Rhodes.
Paul Menzel has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86450?usp=email )
Change subject: soc/intel/cnvi: Deref BTRK as it might not exist
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/86450/comment/eb21fa30_fc24edaa?us… :
PS5, Line 10: as coreboot doesn't enforce its creation
> I don't think so - not really coreboot's style?
No idea, what would be best.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86450?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: Ibb0dace635c6a014ce65ae3d1c96a92ff991ce5b
Gerrit-Change-Number: 86450
Gerrit-PatchSet: 6
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Comment-Date: Tue, 18 Feb 2025 18:43:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Karthik Ramasubramanian, Ronak Kanabar, Subrata Banik.
Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86452?usp=email )
Change subject: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init
......................................................................
Patch Set 5:
(1 comment)
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/86452/comment/83cdd585_d6a2dcd0?us… :
PS5, Line 394: if (!arch_upd->NvsBufferPtr && CONFIG(PLATFORM_HAS_EARLY_LOW_BATTERY_INDICATOR) &&
Do you mind moving this code in a static function. It would keep the main code simpler and more readable.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86452?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: Ia135b238d1e16722c2ca8d3b461e83b4ce513adf
Gerrit-Change-Number: 86452
Gerrit-PatchSet: 5
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 18 Feb 2025 18:37:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No