Change in coreboot[master]: sar: Fix semantics of `get_wifi_sar_cbfs_filename()` - coreboot-gerrit

17 Mar 2021

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/51485 )
Change subject: sar: Fix semantics of `get_wifi_sar_cbfs_filename()`
......................................................................
sar: Fix semantics of `get_wifi_sar_cbfs_filename()`
Currently, if `get_wifi_sar_cbfs_filename()` returns NULL, then
`get_wifi_sar_limits()` assumes that the default filename is used for
CBFS SAR file. This prevents a board from supporting different models
using the same firmware -- some which require SAR support and some
which don't.
This change updates the logic in `get_wifi_sar_limits()` to return
early if filename is not provided by the mainboard. In order to
maintain the same logic as before, current mainboards are updated to
return WIFI_SAR_CBFS_DEFAULT_FILENAME instead of NULL in default
case.
Change-Id: I68b5bdd213767a3cd81fe41ace66540acd68e26a
Signed-off-by: Furquan Shaikh furquan@google.com
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51485
Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org
Tested-by: build bot (Jenkins) no-reply@coreboot.org
---
M src/include/sar.h
M src/mainboard/google/dedede/variants/drawcia/variant.c
M src/mainboard/google/dedede/variants/galtic/variant.c
M src/mainboard/google/dedede/variants/madoo/variant.c
M src/mainboard/google/dedede/variants/magolor/variant.c
M src/mainboard/google/hatch/variants/dratini/variant.c
M src/mainboard/google/hatch/variants/kindred/variant.c
M src/mainboard/google/octopus/variants/bloog/variant.c
M src/mainboard/google/octopus/variants/bobba/variant.c
M src/mainboard/google/octopus/variants/casta/variant.c
M src/mainboard/google/octopus/variants/fleex/variant.c
M src/mainboard/google/octopus/variants/foob/variant.c
M src/mainboard/google/octopus/variants/meep/variant.c
M src/mainboard/google/octopus/variants/phaser/mainboard.c
M src/mainboard/google/poppy/variants/rammus/mainboard.c
M src/mainboard/google/volteer/variants/lindar/variant.c
M src/mainboard/google/zork/variants/vilboz/variant.c
M src/vendorcode/google/chromeos/sar.c
18 files changed, 49 insertions(+), 82 deletions(-)
Approvals:
  build bot (Jenkins): Verified
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/include/sar.h b/src/include/sar.h
index c4a6cd8..78a1b09 100644
--- a/src/include/sar.h
+++ b/src/include/sar.h
@@ -41,6 +41,8 @@
  */
 int get_wifi_sar_limits(struct wifi_sar_limits *sar_limits);
+#define WIFI_SAR_CBFS_DEFAULT_FILENAME	"wifi_sar_defaults.hex"
+
 const char *get_wifi_sar_cbfs_filename(void);
#endif /* _SAR_H_ */
diff --git a/src/mainboard/google/dedede/variants/drawcia/variant.c b/src/mainboard/google/dedede/variants/drawcia/variant.c
index 0c22904..9dc0028 100644
--- a/src/mainboard/google/dedede/variants/drawcia/variant.c
+++ b/src/mainboard/google/dedede/variants/drawcia/variant.c
@@ -6,12 +6,10 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
-
    if (fw_config_probe(FW_CONFIG(TABLETMODE, TABLETMODE_ENABLED)))
-		filename = "wifi_sar-drawcia.hex";
+		return "wifi_sar-drawcia.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
const char *mainboard_vbt_filename(void)
diff --git a/src/mainboard/google/dedede/variants/galtic/variant.c b/src/mainboard/google/dedede/variants/galtic/variant.c
index e7b1bb8..92aa176 100644
--- a/src/mainboard/google/dedede/variants/galtic/variant.c
+++ b/src/mainboard/google/dedede/variants/galtic/variant.c
@@ -5,9 +5,7 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
-
    if (fw_config_probe(FW_CONFIG(TABLETMODE, TABLETMODE_ENABLED)))
-		filename = "wifi_sar-galtic.hex";
-	return filename;
+		return "wifi_sar-galtic.hex";
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
diff --git a/src/mainboard/google/dedede/variants/madoo/variant.c b/src/mainboard/google/dedede/variants/madoo/variant.c
index b8c3654..94e4d90 100644
--- a/src/mainboard/google/dedede/variants/madoo/variant.c
+++ b/src/mainboard/google/dedede/variants/madoo/variant.c
@@ -5,10 +5,8 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
-
    if (fw_config_probe(FW_CONFIG(TABLETMODE, TABLETMODE_ENABLED)))
-		filename = "wifi_sar-madoo.hex";
+		return "wifi_sar-madoo.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
diff --git a/src/mainboard/google/dedede/variants/magolor/variant.c b/src/mainboard/google/dedede/variants/magolor/variant.c
index e3b9599..68ce6c2 100644
--- a/src/mainboard/google/dedede/variants/magolor/variant.c
+++ b/src/mainboard/google/dedede/variants/magolor/variant.c
@@ -5,10 +5,8 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
-
    if (fw_config_probe(FW_CONFIG(TABLETMODE, TABLETMODE_ENABLED)))
-		filename = "wifi_sar-magolor.hex";
+		return "wifi_sar-magolor.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
diff --git a/src/mainboard/google/hatch/variants/dratini/variant.c b/src/mainboard/google/hatch/variants/dratini/variant.c
index 1a4cd40..72f6231 100644
--- a/src/mainboard/google/hatch/variants/dratini/variant.c
+++ b/src/mainboard/google/hatch/variants/dratini/variant.c
@@ -7,7 +7,6 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id = google_chromeec_get_board_sku();
switch (sku_id) {
@@ -15,8 +14,7 @@
    case SKU_22_DRAGONAIR:
    case SKU_23_DRAGONAIR:
    case SKU_24_DRAGONAIR:
-		filename = "wifi_sar-dragonair.hex";
-		break;
+		return "wifi_sar-dragonair.hex";
    }
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
diff --git a/src/mainboard/google/hatch/variants/kindred/variant.c b/src/mainboard/google/hatch/variants/kindred/variant.c
index 9430f67..3510dbb 100644
--- a/src/mainboard/google/hatch/variants/kindred/variant.c
+++ b/src/mainboard/google/hatch/variants/kindred/variant.c
@@ -39,12 +39,11 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 1 || sku_id == 2 || sku_id == 3 || sku_id == 4)
-		filename = "wifi_sar-kled.hex";
-	return filename;
+		return "wifi_sar-kled.hex";
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
const char *mainboard_vbt_filename(void)
diff --git a/src/mainboard/google/octopus/variants/bloog/variant.c b/src/mainboard/google/octopus/variants/bloog/variant.c
index 5682ac3..28f2ce8 100644
--- a/src/mainboard/google/octopus/variants/bloog/variant.c
+++ b/src/mainboard/google/octopus/variants/bloog/variant.c
@@ -28,7 +28,6 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id = google_chromeec_get_board_sku();
switch (sku_id) {
@@ -36,18 +35,16 @@
    case SKU_34_BLOOG:
    case SKU_35_BLOOG:
    case SKU_36_BLOOG:
-		filename = "wifi_sar-bloog.hex";
-		break;
+		return "wifi_sar-bloog.hex";
    case SKU_49_BLOOGUARD:
    case SKU_50_BLOOGUARD:
    case SKU_51_BLOOGUARD:
    case SKU_52_BLOOGUARD:
    case SKU_53_BIPSHIP:
    case SKU_54_BIPSHIP:
-		filename = "wifi_sar-blooguard.hex";
-		break;
+		return "wifi_sar-blooguard.hex";
    }
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
const char *mainboard_vbt_filename(void)
diff --git a/src/mainboard/google/octopus/variants/bobba/variant.c b/src/mainboard/google/octopus/variants/bobba/variant.c
index 2d09270..5765eb7 100644
--- a/src/mainboard/google/octopus/variants/bobba/variant.c
+++ b/src/mainboard/google/octopus/variants/bobba/variant.c
@@ -18,13 +18,12 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id >= 33 && sku_id <= 44)
-		filename = "wifi_sar-droid.hex";
+		return "wifi_sar-droid.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
void variant_smi_sleep(u8 slp_typ)
diff --git a/src/mainboard/google/octopus/variants/casta/variant.c b/src/mainboard/google/octopus/variants/casta/variant.c
index a5b948c..b9165a7 100644
--- a/src/mainboard/google/octopus/variants/casta/variant.c
+++ b/src/mainboard/google/octopus/variants/casta/variant.c
@@ -8,13 +8,12 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id = google_chromeec_get_board_sku();
if (sku_id == 2)
-		filename = "wifi_sar-bluebird.hex";
+		return "wifi_sar-bluebird.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
bool variant_ext_usb_status(unsigned int port_type, unsigned int port_id)
diff --git a/src/mainboard/google/octopus/variants/fleex/variant.c b/src/mainboard/google/octopus/variants/fleex/variant.c
index 6333b07..63908e2 100644
--- a/src/mainboard/google/octopus/variants/fleex/variant.c
+++ b/src/mainboard/google/octopus/variants/fleex/variant.c
@@ -32,12 +32,10 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
-
    if (!is_lte_sku())
-		filename = "wifi_sar-fleex.hex";
+		return "wifi_sar-fleex.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
void variant_update_devtree(struct device *dev)
diff --git a/src/mainboard/google/octopus/variants/foob/variant.c b/src/mainboard/google/octopus/variants/foob/variant.c
index 8cc4a3d..aeddc3b 100644
--- a/src/mainboard/google/octopus/variants/foob/variant.c
+++ b/src/mainboard/google/octopus/variants/foob/variant.c
@@ -25,11 +25,10 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id = SKU_UNKNOWN;
sku_id = google_chromeec_get_board_sku();
    if (sku_id == 9)
-		filename = "wifi_sar-foob360.hex";
-	return filename;
+		return "wifi_sar-foob360.hex";
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
diff --git a/src/mainboard/google/octopus/variants/meep/variant.c b/src/mainboard/google/octopus/variants/meep/variant.c
index df4dbd6..4e5c4be 100644
--- a/src/mainboard/google/octopus/variants/meep/variant.c
+++ b/src/mainboard/google/octopus/variants/meep/variant.c
@@ -8,7 +8,6 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id = google_chromeec_get_board_sku();
switch (sku_id) {
@@ -16,16 +15,14 @@
    case SKU_2_MEEP:
    case SKU_3_MEEP:
    case SKU_4_MEEP:
-		filename = "wifi_sar-meep.hex";
-		break;
+		return "wifi_sar-meep.hex";
    case SKU_49_VORTININJA:
    case SKU_50_VORTININJA:
    case SKU_51_VORTININJA:
    case SKU_52_VORTININJA:
-		filename = "wifi_sar-vortininja.hex";
-		break;
+		return "wifi_sar-vortininja.hex";
    }
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
const char *mainboard_vbt_filename(void)
diff --git a/src/mainboard/google/octopus/variants/phaser/mainboard.c b/src/mainboard/google/octopus/variants/phaser/mainboard.c
index 4997ce4..b5b5353 100644
--- a/src/mainboard/google/octopus/variants/phaser/mainboard.c
+++ b/src/mainboard/google/octopus/variants/phaser/mainboard.c
@@ -6,14 +6,10 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id;
-	if (google_chromeec_cbi_get_sku_id(&sku_id))
-		return NULL;
+	if (google_chromeec_cbi_get_sku_id(&sku_id) || (sku_id != 5))
+		return WIFI_SAR_CBFS_DEFAULT_FILENAME;
-	if (sku_id == 5)
-		filename = "wifi_sar-laser.hex";
-
-	return filename;
+	return "wifi_sar-laser.hex";
 }
diff --git a/src/mainboard/google/poppy/variants/rammus/mainboard.c b/src/mainboard/google/poppy/variants/rammus/mainboard.c
index 8c09b2a..9892dda 100644
--- a/src/mainboard/google/poppy/variants/rammus/mainboard.c
+++ b/src/mainboard/google/poppy/variants/rammus/mainboard.c
@@ -33,14 +33,13 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    uint32_t sku_id;
sku_id = variant_board_sku();
    if (sku_id & 0x200)
-		filename = "wifi_sar-shyvana.hex";
+		return "wifi_sar-shyvana.hex";
    else
-		filename = "wifi_sar-leona.hex";
+		return "wifi_sar-leona.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
diff --git a/src/mainboard/google/volteer/variants/lindar/variant.c b/src/mainboard/google/volteer/variants/lindar/variant.c
index b8d7165..2d10dc6 100644
--- a/src/mainboard/google/volteer/variants/lindar/variant.c
+++ b/src/mainboard/google/volteer/variants/lindar/variant.c
@@ -5,12 +5,10 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
-
    if (fw_config_probe(FW_CONFIG(WIFI_SAR_ID, WIFI_SAR_ID_0)))
-		filename = "wifi_sar_0.hex";
+		return "wifi_sar_0.hex";
    else if (fw_config_probe(FW_CONFIG(WIFI_SAR_ID, WIFI_SAR_ID_1)))
-		filename = "wifi_sar_1.hex";
+		return "wifi_sar_1.hex";
-	return filename;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
diff --git a/src/mainboard/google/zork/variants/vilboz/variant.c b/src/mainboard/google/zork/variants/vilboz/variant.c
index e75d8fc..57563cc 100644
--- a/src/mainboard/google/zork/variants/vilboz/variant.c
+++ b/src/mainboard/google/zork/variants/vilboz/variant.c
@@ -60,29 +60,23 @@
const char *get_wifi_sar_cbfs_filename(void)
 {
-	const char *filename = NULL;
    int sar_config;
sar_config = variant_gets_sar_config();
switch (sar_config) {
    case 1:
-		filename = "wifi_sar-vilboz-0.hex";
-		break;
+		return "wifi_sar-vilboz-0.hex";
    case 3:
    	/*
    	TODO: Set default first. It will be replaced after the
    	new table is generated.
    	*/
-		filename = "wifi_sar_defaults.hex";
-		break;
+		return WIFI_SAR_CBFS_DEFAULT_FILENAME;
    case 5:
-		filename = "wifi_sar-vilboz-1.hex";
-		break;
    case 7:
-		filename = "wifi_sar-vilboz-1.hex";
-		break;
+		return "wifi_sar-vilboz-1.hex";
+	default:
+		return WIFI_SAR_CBFS_DEFAULT_FILENAME;
    }
-
-	return filename;
 }
diff --git a/src/vendorcode/google/chromeos/sar.c b/src/vendorcode/google/chromeos/sar.c
index 83c1c0c..a38ffe2 100644
--- a/src/vendorcode/google/chromeos/sar.c
+++ b/src/vendorcode/google/chromeos/sar.c
@@ -9,8 +9,6 @@
 #include <string.h>
 #include <types.h>
-#define WIFI_SAR_CBFS_FILENAME	"wifi_sar_defaults.hex"
-
 /*
  * Retrieve WiFi SAR limits data from CBFS and decode it
  * WiFi SAR data is expected in the format: [<WRDD><EWRD>][WGDS]
@@ -37,6 +35,12 @@
    char *sar_str;
    int ret = -1;
+	filename = get_wifi_sar_cbfs_filename();
+	if (filename == NULL) {
+		printk(BIOS_DEBUG, "Filename missing for CBFS SAR file!\n");
+		return ret;
+	}
+
    /*
     * If GEO_SAR_ENABLE is not selected, SAR file does not contain
     * delta table settings.
@@ -64,10 +68,6 @@
printk(BIOS_DEBUG, "Checking CBFS for default SAR values\n");
-	filename = get_wifi_sar_cbfs_filename();
-	if (filename == NULL)
-		filename = WIFI_SAR_CBFS_FILENAME;
-
    if (cbfs_load(filename, sar_str, sar_str_len) != sar_str_len) {
    	printk(BIOS_ERR, "%s has bad len in CBFS\n", filename);
    	goto done;
@@ -88,5 +88,5 @@
 __weak
 const char *get_wifi_sar_cbfs_filename(void)
 {
-	return WIFI_SAR_CBFS_FILENAME;
+	return WIFI_SAR_CBFS_DEFAULT_FILENAME;
 }
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/51485
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I68b5bdd213767a3cd81fe41ace66540acd68e26a
Gerrit-Change-Number: 51485
Gerrit-PatchSet: 7
Gerrit-Owner: Furquan Shaikh furquan@google.com
Gerrit-Reviewer: Duncan Laurie dlaurie@chromium.org
Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com
Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org
Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org
Gerrit-MessageType: merged



    