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>
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83233?usp=email )
Change subject: soc/intel/cmn/cse: Conditionally disable ME status reporting
......................................................................
soc/intel/cmn/cse: Conditionally disable ME status reporting
This patch disables the ME status reporting functionality
(dump_me_status, print_me_fw_version) in the CSE driver when
SOC_INTEL_CSE_LITE_SYNC_BY_PAYLOAD is defined.
This is likely intended for platforms or configurations where the
CSE communication is only limited to payload.
BUG=b:305898363
TEST=Able to build google/rex.
Change-Id: I5e360408a7847968117df475ff244d79ceafa23f
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83233
Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com>
Reviewed-by: Dinesh Gehlot <digehlot(a)google.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Eric Lai <ericllai(a)google.com>
---
M src/soc/intel/common/block/cse/cse.c
M src/soc/intel/common/block/cse/cse_spec.c
2 files changed, 6 insertions(+), 0 deletions(-)
Approvals:
Eric Lai: Looks good to me, but someone else must approve
Nick Vaccaro: Looks good to me, approved
build bot (Jenkins): Verified
Dinesh Gehlot: 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 32ca399..b278b9f 100644
--- a/src/soc/intel/common/block/cse/cse.c
+++ b/src/soc/intel/common/block/cse/cse.c
@@ -863,6 +863,9 @@
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 */
diff --git a/src/soc/intel/common/block/cse/cse_spec.c b/src/soc/intel/common/block/cse/cse_spec.c
index 74155cd..af64255 100644
--- a/src/soc/intel/common/block/cse/cse_spec.c
+++ b/src/soc/intel/common/block/cse/cse_spec.c
@@ -28,6 +28,9 @@
static void dump_me_status(void *unused)
{
+ if (CONFIG(SOC_INTEL_CSE_LITE_SYNC_BY_PAYLOAD))
+ return;
+
union me_hfsts1 hfsts1;
union me_hfsts2 hfsts2;
union me_hfsts3 hfsts3;
--
To view, visit https://review.coreboot.org/c/coreboot/+/83233?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: I5e360408a7847968117df475ff244d79ceafa23f
Gerrit-Change-Number: 83233
Gerrit-PatchSet: 4
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/+/83231?usp=email )
Change subject: drivers/intel/ish: Skip ISH version call if CSE sync is done by payload
......................................................................
drivers/intel/ish: Skip ISH version call if CSE sync is done by payload
This patch skips the ISH firmware version print when CSE sync is done
by payload. The payload is responsible to dump the ISH version as
ISH version resides into the CSE boot partition table.
BUG=b:305898363
TEST=Able to build google/rex.
Change-Id: I1895a4d3c44838a9cc6380912f09aa4f0e6687bd
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83231
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com>
Reviewed-by: Dinesh Gehlot <digehlot(a)google.com>
Reviewed-by: Eric Lai <ericllai(a)google.com>
---
M src/drivers/intel/ish/ish.c
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Eric Lai: Looks good to me, but someone else must approve
Dinesh Gehlot: Looks good to me, approved
Nick Vaccaro: Looks good to me, approved
diff --git a/src/drivers/intel/ish/ish.c b/src/drivers/intel/ish/ish.c
index 7d332e1..1ab6a0a 100644
--- a/src/drivers/intel/ish/ish.c
+++ b/src/drivers/intel/ish/ish.c
@@ -50,6 +50,9 @@
static void intel_ish_get_version(void)
{
+ if (CONFIG(SOC_INTEL_CSE_LITE_SYNC_BY_PAYLOAD))
+ return;
+
struct cse_specific_info *info = cbmem_find(CBMEM_ID_CSE_INFO);
if (info == NULL)
return;
--
To view, visit https://review.coreboot.org/c/coreboot/+/83231?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: I1895a4d3c44838a9cc6380912f09aa4f0e6687bd
Gerrit-Change-Number: 83231
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/+/83230?usp=email )
Change subject: soc/intel/cmn/cse: Skip CSE version call if sync is done by payload
......................................................................
soc/intel/cmn/cse: Skip CSE version call if sync is done by payload
This patch skips the CSE firmware version print when CSE sync is done
by payload. The payload is responsible to dump the CSE version.
BUG=b:305898363
TEST=Able to build google/rex.
Change-Id: I1a9e5583c79ebd81291a4b3ae24529b4582502cb
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83230
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Dinesh Gehlot <digehlot(a)google.com>
Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com>
Reviewed-by: Eric Lai <ericllai(a)google.com>
---
M src/soc/intel/common/block/cse/cse.c
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
Eric Lai: Looks good to me, but someone else must approve
Dinesh Gehlot: Looks good to me, approved
build bot (Jenkins): Verified
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 d78b8a0..32ca399 100644
--- a/src/soc/intel/common/block/cse/cse.c
+++ b/src/soc/intel/common/block/cse/cse.c
@@ -1418,6 +1418,9 @@
static void intel_cse_get_rw_version(void)
{
+ if (CONFIG(SOC_INTEL_CSE_LITE_SYNC_BY_PAYLOAD))
+ return;
+
struct cse_specific_info *info = cbmem_find(CBMEM_ID_CSE_INFO);
if (info == NULL)
return;
--
To view, visit https://review.coreboot.org/c/coreboot/+/83230?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: I1a9e5583c79ebd81291a4b3ae24529b4582502cb
Gerrit-Change-Number: 83230
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>