Attention is currently required from: Dinesh Gehlot, Jakub "Kuba" Czapiga, Julius Werner, Jérémy Compostella, Karthik Ramasubramanian.
Hello Dinesh Gehlot, Jakub "Kuba" Czapiga, Jérémy Compostella, Karthik Ramasubramanian, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/86284?usp=email
to look at the new patch set (#12).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: lib: Add low battery UX locale message
......................................................................
lib: Add low battery UX locale message
This commit adds a new UX locale message to display a warning when the
battery is critically low.
The message informs the user about the low battery and indicates that
the system is shutting down.
This change ensures that users are notified before the system
unexpectedly shuts down due to low battery.
BUG=b:339673254
TEST=Built and booted google/brox.
Change-Id: I75c7a0d4d439901098c7f17a1dc90355307116ac
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/include/ux_locales.h
M src/lib/ux_locales.c
M tests/lib/ux_locales-test.c
3 files changed, 16 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/86284/12
--
To view, visit https://review.coreboot.org/c/coreboot/+/86284?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: I75c7a0d4d439901098c7f17a1dc90355307116ac
Gerrit-Change-Number: 86284
Gerrit-PatchSet: 12
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Andrey Petrov, Fred Reitberger, Intel coreboot Reviewers, Jason Glenesk, Matt DeVillier, Patrick Rudolph, Ronak Kanabar.
Felix Held has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86299?usp=email )
Change subject: soc/amd/common/block/graphics: Use vbt_get()
......................................................................
Patch Set 2:
(1 comment)
File src/soc/amd/common/block/include/amdblocks/vbt.h:
https://review.coreboot.org/c/coreboot/+/86299/comment/0ce327fc_61759a77?us… :
PS2, Line 20: void *vbt_get(void);
i'd add an empty line between the function prototype and the #endif
--
To view, visit https://review.coreboot.org/c/coreboot/+/86299?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: I92d76fc4df88fbce792b9d7c912c6799617704a0
Gerrit-Change-Number: 86299
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.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: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 12:29:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86298?usp=email )
Change subject: soc/amd: Document VBIOS handling
......................................................................
soc/amd: Document VBIOS handling
The code flow isn't that obvious in the beginning. You pass an address
of the VBIOS to FSP, but don't load any VBIOS until BS_DEV_RESOURCES
phase.
Add comments to document what is done and when. This will help to
improve the code in the next step.
Change-Id: I643bc9088306d99cc0fbb79648809e16b068fb33
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/86298
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de>
---
M src/soc/amd/cezanne/fsp_s_params.c
M src/soc/amd/common/block/graphics/graphics.c
M src/soc/amd/glinda/fsp_s_params.c
M src/soc/amd/mendocino/fsp_s_params.c
M src/soc/amd/phoenix/fsp_s_params.c
M src/soc/amd/picasso/fsp_s_params.c
6 files changed, 38 insertions(+), 3 deletions(-)
Approvals:
build bot (Jenkins): Verified
Felix Held: Looks good to me, approved
diff --git a/src/soc/amd/cezanne/fsp_s_params.c b/src/soc/amd/cezanne/fsp_s_params.c
index 60f3942..b9770f3 100644
--- a/src/soc/amd/cezanne/fsp_s_params.c
+++ b/src/soc/amd/cezanne/fsp_s_params.c
@@ -8,6 +8,11 @@
static void fsp_assign_vbios_upds(FSP_S_CONFIG *scfg)
{
+ /*
+ * The VBIOS contains the ATOMBIOS tables that will be modified as
+ * part of FSP GOP init. We can delay loading of the VBIOS until
+ * before FSP notify AFTER_PCI_ENUM.
+ */
scfg->vbios_buffer = CONFIG(RUN_FSP_GOP) ? PCI_VGA_RAM_IMAGE_START : 0;
}
diff --git a/src/soc/amd/common/block/graphics/graphics.c b/src/soc/amd/common/block/graphics/graphics.c
index ea7308a..77420ae 100644
--- a/src/soc/amd/common/block/graphics/graphics.c
+++ b/src/soc/amd/common/block/graphics/graphics.c
@@ -147,9 +147,13 @@
}
/*
- * Even though AMD does not need VBT we still need to implement the
- * vbt_get() function to not break the build with GOP driver enabled
- * (see fsps_return_value_handler() in fsp2_0/silicon_init.c
+ * On AMD platforms the VBT is called ATOMBIOS and is always part of the
+ * VGA Option ROM. As part of the FSP GOP init the ATOMBIOS tables are
+ * updated in place. Thus the VBIOS must be loaded into RAM before FSP GOP
+ * runs. The address of the VBIOS must be passed to FSP-S using UPDs, but
+ * loading of the VBIOS can be delayed until before FSP AFTER_PCI_ENUM
+ * notify is called. FSP expects a pointer to the PCI option rom instead
+ * a pointer to the ATOMBIOS table directly.
*/
void *vbt_get(void)
{
@@ -165,6 +169,7 @@
if (!CONFIG(RUN_FSP_GOP))
return;
+ /* Load the VBIOS before FSP AFTER_PCI_ENUM notify is called. */
timestamp_add_now(TS_OPROM_INITIALIZE);
if (CONFIG(USE_SELECTIVE_GOP_INIT) && vbios_cache_is_valid() &&
!display_init_required()) {
@@ -172,6 +177,11 @@
timestamp_add_now(TS_OPROM_COPY_END);
return;
}
+
+ /*
+ * VBIOS cache was not used, so load it from CBFS and let FSP GOP
+ * initialize the ATOMBIOS tables.
+ */
rom = pci_rom_probe(dev);
if (rom == NULL) {
printk(BIOS_ERR, "%s: Unable to find ROM for %s\n",
diff --git a/src/soc/amd/glinda/fsp_s_params.c b/src/soc/amd/glinda/fsp_s_params.c
index 597f7a9..e037957 100644
--- a/src/soc/amd/glinda/fsp_s_params.c
+++ b/src/soc/amd/glinda/fsp_s_params.c
@@ -10,6 +10,11 @@
static void fsp_assign_vbios_upds(FSP_S_CONFIG *scfg)
{
+ /*
+ * The VBIOS contains the ATOMBIOS tables that will be modified as
+ * part of FSP GOP init. We can delay loading of the VBIOS until
+ * before FSP notify AFTER_PCI_ENUM.
+ */
scfg->vbios_buffer = CONFIG(RUN_FSP_GOP) ? PCI_VGA_RAM_IMAGE_START : 0;
}
diff --git a/src/soc/amd/mendocino/fsp_s_params.c b/src/soc/amd/mendocino/fsp_s_params.c
index 50923e9..5c37334 100644
--- a/src/soc/amd/mendocino/fsp_s_params.c
+++ b/src/soc/amd/mendocino/fsp_s_params.c
@@ -20,6 +20,11 @@
printk(BIOS_SPEW, "%s: using VBIOS cache; skipping GOP driver.\n", __func__);
return;
}
+ /*
+ * The VBIOS contains the ATOMBIOS tables that will be modified as
+ * part of FSP GOP init. We can delay loading of the VBIOS until
+ * before FSP notify AFTER_PCI_ENUM.
+ */
printk(BIOS_SPEW, "%s: not using VBIOS cache; running GOP driver.\n", __func__);
scfg->vbios_buffer = CONFIG(RUN_FSP_GOP) ? PCI_VGA_RAM_IMAGE_START : 0;
}
diff --git a/src/soc/amd/phoenix/fsp_s_params.c b/src/soc/amd/phoenix/fsp_s_params.c
index 46c8e76..883cde0 100644
--- a/src/soc/amd/phoenix/fsp_s_params.c
+++ b/src/soc/amd/phoenix/fsp_s_params.c
@@ -10,6 +10,11 @@
static void fsp_assign_vbios_upds(FSP_S_CONFIG *scfg)
{
+ /*
+ * The VBIOS contains the ATOMBIOS tables that will be modified as
+ * part of FSP GOP init. We can delay loading of the VBIOS until
+ * before FSP notify AFTER_PCI_ENUM.
+ */
scfg->vbios_buffer = CONFIG(RUN_FSP_GOP) ? PCI_VGA_RAM_IMAGE_START : 0;
}
diff --git a/src/soc/amd/picasso/fsp_s_params.c b/src/soc/amd/picasso/fsp_s_params.c
index 9f491d6..d4cfa27 100644
--- a/src/soc/amd/picasso/fsp_s_params.c
+++ b/src/soc/amd/picasso/fsp_s_params.c
@@ -185,6 +185,11 @@
static void fsp_assign_vbios_upds(FSP_S_CONFIG *scfg)
{
+ /*
+ * The VBIOS contains the ATOMBIOS tables that will be modified as
+ * part of FSP GOP init. We can delay loading of the VBIOS until
+ * before FSP notify AFTER_PCI_ENUM.
+ */
scfg->vbios_buffer_addr = CONFIG(RUN_FSP_GOP) ? PCI_VGA_RAM_IMAGE_START : 0;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/86298?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I643bc9088306d99cc0fbb79648809e16b068fb33
Gerrit-Change-Number: 86298
Gerrit-PatchSet: 3
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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>
Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier, Patrick Rudolph.
Felix Held has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/86298?usp=email )
Change subject: soc/amd: Document VBIOS handling
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86298?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: I643bc9088306d99cc0fbb79648809e16b068fb33
Gerrit-Change-Number: 86298
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 12:28:05 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Dinesh Gehlot, Jakub "Kuba" Czapiga, Julius Werner, Jérémy Compostella, Karthik Ramasubramanian.
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86284?usp=email )
Change subject: lib: Add low battery UX locale message
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
> Please expand ux_locales-test.c here so that we have at least two messages in the test data there.
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/86284?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: I75c7a0d4d439901098c7f17a1dc90355307116ac
Gerrit-Change-Number: 86284
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 11:49:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Dinesh Gehlot, Jérémy Compostella, Karthik Ramasubramanian, Subrata Banik.
Hello Dinesh Gehlot, Jérémy Compostella, Karthik Ramasubramanian, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/86284?usp=email
to look at the new patch set (#11).
The following approvals got outdated and were removed:
Code-Review+1 by Jérémy Compostella, Code-Review+2 by Dinesh Gehlot, Code-Review+2 by Karthik Ramasubramanian, Verified+1 by build bot (Jenkins)
Change subject: lib: Add low battery UX locale message
......................................................................
lib: Add low battery UX locale message
This commit adds a new UX locale message to display a warning when the
battery is critically low.
The message informs the user about the low battery and indicates that
the system is shutting down.
This change ensures that users are notified before the system
unexpectedly shuts down due to low battery.
BUG=b:339673254
TEST=Built and booted google/brox.
Change-Id: I75c7a0d4d439901098c7f17a1dc90355307116ac
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/include/ux_locales.h
M src/lib/ux_locales.c
M tests/lib/ux_locales-test.c
3 files changed, 16 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/86284/11
--
To view, visit https://review.coreboot.org/c/coreboot/+/86284?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: I75c7a0d4d439901098c7f17a1dc90355307116ac
Gerrit-Change-Number: 86284
Gerrit-PatchSet: 11
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Angel Pons, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Fabian Meyer, Intel coreboot Reviewers, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Nico Huber, Patrick Rudolph, Srinidhi N Kaushik, Tim Chu.
Shuo Liu has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/80360?usp=email )
Change subject: soc/intel/xeon-sp/spr: Hook up public FSP bin and headers
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> Looks like you need to update the 3rdparty/fsp submodule, I see there's stuff in https://github. […]
Yes, it comes - https://github.com/intel/FSP/pull/115/
--
To view, visit https://review.coreboot.org/c/coreboot/+/80360?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: I778d3535c273dff653330518653bdefcb45e66f4
Gerrit-Change-Number: 80360
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.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 <tanleansheng(a)outlook.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <kaushiksrinidhin(a)gmail.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Fabian Meyer <fabian(a)meyfa.net>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Fabian Meyer <fabian(a)meyfa.net>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <tanleansheng(a)outlook.com>
Gerrit-Attention: Srinidhi N Kaushik <kaushiksrinidhin(a)gmail.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 11:43:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Ana Carolina Cabral.
Angel Pons has posted comments on this change by Ana Carolina Cabral. ( https://review.coreboot.org/c/coreboot/+/85374?usp=email )
Change subject: mb/amd/birman_plus: Factor out get_dd1_type
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85374/comment/5645ccbb_41209e25?us… :
PS5, Line 7: dd1
typo: ddi1
--
To view, visit https://review.coreboot.org/c/coreboot/+/85374?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: I6277b10a01f2913e7308e456a34fa28470adcb8e
Gerrit-Change-Number: 85374
Gerrit-PatchSet: 5
Gerrit-Owner: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 11:28:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Ana Carolina Cabral.
Angel Pons has posted comments on this change by Ana Carolina Cabral. ( https://review.coreboot.org/c/coreboot/+/84776?usp=email )
Change subject: drivers/amd/nova: Add Nova Card common driver
......................................................................
Patch Set 10: Code-Review+1
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84776/comment/8fa21438_9b21b30e?us… :
PS10, Line 7: drivers/amd/nova: Add Nova Card common driver
Out of curiosity, what is a Nova/NOVA card?
File src/drivers/amd/nova/nova_card.c:
https://review.coreboot.org/c/coreboot/+/84776/comment/4ce2b263_bfc99629?us… :
PS10, Line 28: switch (connector_type) {
> `switch and case should be at the same indent`
Please fix (remove one tab across the entire switch body)
--
To view, visit https://review.coreboot.org/c/coreboot/+/84776?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: I5e9ded2090d6a5865e3330408f490e59fbf480f4
Gerrit-Change-Number: 84776
Gerrit-PatchSet: 10
Gerrit-Owner: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-CC: Anand Vaikar <a.vaikar2021(a)gmail.com>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 11:27:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, John Su, Kapil Porwal, Nick Vaccaro.
Subrata Banik has posted comments on this change by John Su. ( https://review.coreboot.org/c/coreboot/+/86362?usp=email )
Change subject: mb/trulo/var/uldrenite: Remove GPP_E13 from being used as RAM ID3
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/brya/variants/uldrenite/gpio.c:
https://review.coreboot.org/c/coreboot/+/86362/comment/eefd5db8_11c27c50?us… :
PS1, Line 187: /* E13 : NC ==> GPP_E13_STRAP */
if possible, i would request to mark GPP_E13 NC in this CL and then add the correct configuration in next CL.
In that way, this CL would appear independent as in, removing the GPP_E13 completely and then next CL has added GPP_E13 for mem channel control
--
To view, visit https://review.coreboot.org/c/coreboot/+/86362?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: I418f84255959452d5a63612ab703ec11d81f2e33
Gerrit-Change-Number: 86362
Gerrit-PatchSet: 1
Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com>
Gerrit-Reviewer: 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: 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: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com>
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: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Tue, 11 Feb 2025 10:33:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No