Daniel Maslowski has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83311?usp=email )
Change subject: Revert "Makefile.mk: Use Walloc-size GCC option"
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/83311?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: I0bdc909c0e53b5353743dca521c963bbec792f7e
Gerrit-Change-Number: 83311
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Maslowski <info(a)orangecms.org>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Uwe Poeche <uwe.poeche(a)siemens.com>
Gerrit-Comment-Date: Wed, 03 Jul 2024 06:40:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Nicholas Chin.
PugzAreCute has posted comments on this change by PugzAreCute. ( https://review.coreboot.org/c/coreboot/+/83267?usp=email )
Change subject: mb/gigabyte/ga-h61m-series: Initial GA-H61M-S2P-R3 bringup
......................................................................
Patch Set 6:
(2 comments)
This change is ready for review.
Commit Message:
https://review.coreboot.org/c/coreboot/+/83267/comment/06ad1b5d_72affdbb?us… :
PS5, Line 17: current version
> It probably would be good to list the actual version number so that this statement is valid if SeaBI […]
Done
https://review.coreboot.org/c/coreboot/+/83267/comment/8b443097_963680bd?us… :
PS5, Line 18: current version
> Same here. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83267?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: I106c195c890823f07227739c6b30133b996f6510
Gerrit-Change-Number: 83267
Gerrit-PatchSet: 6
Gerrit-Owner: PugzAreCute <me(a)pugzarecute.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 03 Jul 2024 06:30:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Jérémy Compostella, YH Lin, dinesh.sharma(a)intel.com.
Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/83200?usp=email )
Change subject: drivers/wifi: Support Bluetooth Regulator Domain Settings
......................................................................
Patch Set 10:
(1 comment)
File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/83200/comment/1920f315_091c7679?us… :
PS9, Line 493: bsar
> The other functions do receive a potentially NULL pointer. This is part of the design and they should return without complaining if they receive a NULL pointer.
>
> This logic does does not apply to the function I am adding.
Does the code static analyzer catch this issue due to not having a NULL check?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83200?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: Iebe95815c944d045f4cf686abcd1874a8a45e209
Gerrit-Change-Number: 83200
Gerrit-PatchSet: 10
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: dinesh.sharma(a)intel.com
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: dinesh.sharma(a)intel.com
Gerrit-Comment-Date: Wed, 03 Jul 2024 06:16:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: YH Lin <yueherngl(a)google.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83258?usp=email )
Change subject: soc/intel/common: Skip ME version log for Lite SKU
......................................................................
soc/intel/common: Skip ME version log for Lite SKU
This change skips the ME firmware version logging in
print_me_fw_version() if the ME firmware SKU is detected as Lite SKU.
The reasoning is that the RO (BP1) and RW (BP2) versions are already
logged by the cse_print_boot_partition_info() function for Lite SKUs,
making the additional log redundant.
The check for the Lite SKU has been moved to print_me_fw_version(),
where the decision to print the version is made, instead of in
get_me_fw_version(), where the version information is retrieved.
TEST=Able to build and boot google/rex.
w/o this patch:
[DEBUG] ME: Version: Unavailable
w/ this patch:
Unable to see such debug msg.
Change-Id: Ic3843109326153d5060c2c4c25936aaa6b4cddda
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83258
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Dinesh Gehlot <digehlot(a)google.com>
---
M src/soc/intel/common/block/cse/cse.c
1 file changed, 7 insertions(+), 7 deletions(-)
Approvals:
Dinesh Gehlot: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c
index 51241b2..ed07f69 100644
--- a/src/soc/intel/common/block/cse/cse.c
+++ b/src/soc/intel/common/block/cse/cse.c
@@ -880,13 +880,6 @@
return CB_ERR;
/*
- * Ignore if ME Firmware SKU type is Lite since
- * print_boot_partition_info() logs RO(BP1) and RW(BP2) versions.
- */
- if (cse_is_hfs3_fw_sku_lite())
- return CB_ERR;
-
- /*
* Prerequisites:
* 1) HFSTS1 Current Working State is Normal
* 2) HFSTS1 Current Operation Mode is Normal
@@ -917,6 +910,13 @@
if (!CONFIG(CONSOLE_SERIAL))
return;
+ /*
+ * Skip if ME firmware is Lite SKU, as RO/RW versions are
+ * already logged by `cse_print_boot_partition_info()`
+ */
+ if (cse_is_hfs3_fw_sku_lite())
+ return;
+
if (get_me_fw_version(&resp) == CB_SUCCESS) {
printk(BIOS_DEBUG, "ME: Version: %d.%d.%d.%d\n", resp.code.major,
resp.code.minor, resp.code.hotfix, resp.code.build);
--
To view, visit https://review.coreboot.org/c/coreboot/+/83258?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: Ic3843109326153d5060c2c4c25936aaa6b4cddda
Gerrit-Change-Number: 83258
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83257?usp=email )
Change subject: soc/intel/cmn/cse: Make ME firmware version query function static
......................................................................
soc/intel/cmn/cse: Make ME firmware version query function static
This change modifies the get_me_fw_version() function to be statically
scoped within src/soc/intel/common/block/cse/cse.c, as it is only used
by the print_me_fw_version() function in the same file.
The function declaration is also removed from intelblocks/cse.h.
The order of the function definitions in cse.c was also changed to be
more logical, with the now static helper function get_me_fw_version()
defined first, before it is used.
TEST=Able to build google/rex.
Change-Id: Idd3a6431cfa824227361c7ed4f0d5300f1d04846
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83257
Reviewed-by: Dinesh Gehlot <digehlot(a)google.com>
Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/intel/common/block/cse/cse.c
M src/soc/intel/common/block/include/intelblocks/cse.h
2 files changed, 18 insertions(+), 25 deletions(-)
Approvals:
build bot (Jenkins): Verified
Dinesh Gehlot: Looks good to me, approved
Nick Vaccaro: Looks good to me, approved
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c
index b278b9f..51241b2 100644
--- a/src/soc/intel/common/block/cse/cse.c
+++ b/src/soc/intel/common/block/cse/cse.c
@@ -861,26 +861,8 @@
return resp.status;
}
-void print_me_fw_version(void *unused)
-{
- if (CONFIG(SOC_INTEL_CSE_LITE_SYNC_BY_PAYLOAD))
- return;
-
- struct me_fw_ver_resp resp = {0};
-
- /* Ignore if UART debugging is disabled */
- if (!CONFIG(CONSOLE_SERIAL))
- return;
-
- if (get_me_fw_version(&resp) == CB_SUCCESS) {
- printk(BIOS_DEBUG, "ME: Version: %d.%d.%d.%d\n", resp.code.major,
- resp.code.minor, resp.code.hotfix, resp.code.build);
- return;
- }
- printk(BIOS_DEBUG, "ME: Version: Unavailable\n");
-}
-
-enum cb_err get_me_fw_version(struct me_fw_ver_resp *resp)
+/* Queries and gets ME firmware version */
+static enum cb_err get_me_fw_version(struct me_fw_ver_resp *resp)
{
const struct mkhi_hdr fw_ver_msg = {
.group_id = MKHI_GROUP_ID_GEN,
@@ -927,6 +909,22 @@
return CB_SUCCESS;
}
+void print_me_fw_version(void *unused)
+{
+ struct me_fw_ver_resp resp = {0};
+
+ /* Ignore if UART debugging is disabled */
+ if (!CONFIG(CONSOLE_SERIAL))
+ return;
+
+ if (get_me_fw_version(&resp) == CB_SUCCESS) {
+ printk(BIOS_DEBUG, "ME: Version: %d.%d.%d.%d\n", resp.code.major,
+ resp.code.minor, resp.code.hotfix, resp.code.build);
+ return;
+ }
+ printk(BIOS_DEBUG, "ME: Version: Unavailable\n");
+}
+
void cse_trigger_vboot_recovery(enum csme_failure_reason reason)
{
printk(BIOS_DEBUG, "cse: CSE status registers: HFSTS1: 0x%x, HFSTS2: 0x%x "
diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h
index 3db1324..552eb7b 100644
--- a/src/soc/intel/common/block/include/intelblocks/cse.h
+++ b/src/soc/intel/common/block/include/intelblocks/cse.h
@@ -444,11 +444,6 @@
void print_me_fw_version(void *unused);
/*
- * Queries and gets ME firmware version
- */
-enum cb_err get_me_fw_version(struct me_fw_ver_resp *resp);
-
-/*
* Checks current working operation state is normal or not.
* Returns true if CSE's current working state is normal, otherwise false.
*/
--
To view, visit https://review.coreboot.org/c/coreboot/+/83257?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: Idd3a6431cfa824227361c7ed4f0d5300f1d04846
Gerrit-Change-Number: 83257
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>