[PATCH v5 0/2] ath11k: add handler for board-2.bin without variant and read SMBIOS

[PATCH v5 0/2] ath11k: add handler for board-2.bin without variant and read SMBIOS

[Wen Gong]

v5:
   1. rebased to ath.git ath-202112161538
   2. fix warning "failed to fetch board data for bus=pci,qmi-chip-id=0,qmi-board-id=255,variant=DE_1901 from ath11k/QCA6390/hw2.0/board-2.bin"

v4:
   1. add helper function ath11k_core_create_fallback_board_name()
   2. rebased to ath.git ath-202112071521

v3:
   1. add patch "ath11k: add fallback board name without variant while searching board-2.bin"
   2. reabsed to latest ath.git master ath-202111221436

v2: rebased to latest ath.git master ath-202111170737

When it does not have matched board name with variant in board-2.bin,
ath11k will fallback to search without variant.

Also read the variant from SMBIOS.

Wen Gong (2):
  ath11k: add fallback board name without variant while searching
    board-2.bin
  ath11k: add read variant from SMBIOS for download board data

 drivers/net/wireless/ath/ath11k/core.c | 109 +++++++++++++++++++++++--
 drivers/net/wireless/ath/ath11k/core.h |  11 +++
 drivers/net/wireless/ath/ath11k/qmi.c  |   4 +
 3 files changed, 118 insertions(+), 6 deletions(-)


base-commit: 12018ad8603a598d773e2cae12542a1981c94886
-- 
2.31.1

[PATCH v5 1/2] ath11k: add fallback board name without variant while searching board-2.bin

[Wen Gong]

Sometimes it has a variant value which read from DT or SMBIOS by
ath11k, and meanwhile it does not have the matched board name in
board-2.bin, then it will failed at boot up phase.

Add fallback board name which removed variant value and search again
in board-2.bin when failed with variant and try to load the board
data again to increase boot up success rate.

dmesg log after this patch:
[169547.248472] ath11k_pci 0000:05:00.0: boot using board name 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262,variant=test'
[169547.248565] ath11k_pci 0000:05:00.0: boot firmware request ath11k/WCN6855/hw2.0/board-2.bin size 180324
[169547.248568] ath11k_pci 0000:05:00.0: board name
[169547.248570] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
[169547.248571] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
[169547.248572] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
[169547.248574] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
[169547.248575] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
[169547.248576] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 32 2c 71 6d 69 2d 62 6f  chip-id=2,qmi-bo
[169547.248577] ath11k_pci 0000:05:00.0: 00000060: 61 72 64 2d 69 64 3d 32 36 32                    ard-id=262
[169547.248578] ath11k_pci 0000:05:00.0: board name
[169547.248579] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
[169547.248581] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
[169547.248582] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
[169547.248583] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
[169547.248584] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
[169547.248585] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 32 2c 71 6d 69 2d 62 6f  chip-id=2,qmi-bo
[169547.248587] ath11k_pci 0000:05:00.0: 00000060: 61 72 64 2d 69 64 3d 32 36 36                    ard-id=266
[169547.248588] ath11k_pci 0000:05:00.0: board name
[169547.248589] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
[169547.248590] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
[169547.248591] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
[169547.248592] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
[169547.248594] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
[169547.248595] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 31 38 2c 71 6d 69 2d 62  chip-id=18,qmi-b
[169547.248596] ath11k_pci 0000:05:00.0: 00000060: 6f 61 72 64 2d 69 64 3d 32 36 36                 oard-id=266
[169547.248597] ath11k_pci 0000:05:00.0: failed to fetch board data for bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262,variant=test from ath11k/WCN6855/hw2.0/board-2.bin
[169547.248476] ath11k_pci 0000:05:00.0: boot using board name 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262'
[169547.248634] ath11k_pci 0000:05:00.0: boot firmware request ath11k/WCN6855/hw2.0/board-2.bin size 180324
[169547.248636] ath11k_pci 0000:05:00.0: board name
[169547.248637] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
[169547.248638] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
[169547.248639] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
[169547.248641] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
[169547.248642] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
[169547.248643] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 32 2c 71 6d 69 2d 62 6f  chip-id=2,qmi-bo
[169547.248645] ath11k_pci 0000:05:00.0: 00000060: 61 72 64 2d 69 64 3d 32 36 32                    ard-id=262
[169547.248646] ath11k_pci 0000:05:00.0: boot found match for name 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262'
[169547.248647] ath11k_pci 0000:05:00.0: boot found board data for 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262'
[169547.248649] ath11k_pci 0000:05:00.0: using board api 2

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/core.c | 35 +++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 606e867c36ec..641c133563ab 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -483,13 +483,13 @@ int ath11k_core_check_dt(struct ath11k_base *ab)
 	return 0;
 }
 
-static int ath11k_core_create_board_name(struct ath11k_base *ab, char *name,
-					 size_t name_len)
+static int __ath11k_core_create_board_name(struct ath11k_base *ab, char *name,
+					   size_t name_len, bool with_variant)
 {
 	/* strlen(',variant=') + strlen(ab->qmi.target.bdf_ext) */
 	char variant[9 + ATH11K_QMI_BDF_EXT_STR_LENGTH] = { 0 };
 
-	if (ab->qmi.target.bdf_ext[0] != '\0')
+	if (with_variant && ab->qmi.target.bdf_ext[0] != '\0')
 		scnprintf(variant, sizeof(variant), ",variant=%s",
 			  ab->qmi.target.bdf_ext);
 
@@ -519,6 +519,18 @@ static int ath11k_core_create_board_name(struct ath11k_base *ab, char *name,
 	return 0;
 }
 
+static int ath11k_core_create_board_name(struct ath11k_base *ab, char *name,
+					 size_t name_len)
+{
+	return __ath11k_core_create_board_name(ab, name, name_len, true);
+}
+
+static int ath11k_core_create_fallback_board_name(struct ath11k_base *ab, char *name,
+						  size_t name_len)
+{
+	return __ath11k_core_create_board_name(ab, name, name_len, false);
+}
+
 const struct firmware *ath11k_core_firmware_request(struct ath11k_base *ab,
 						    const char *file)
 {
@@ -722,7 +734,7 @@ static int ath11k_core_fetch_board_data_api_n(struct ath11k_base *ab,
 
 out:
 	if (!bd->data || !bd->len) {
-		ath11k_err(ab,
+		ath11k_dbg(ab, ATH11K_DBG_BOOT,
 			   "failed to fetch board data for %s from %s\n",
 			   boardname, filepath);
 		ret = -ENODATA;
@@ -752,10 +764,10 @@ static int ath11k_core_fetch_board_data_api_1(struct ath11k_base *ab,
 #define BOARD_NAME_SIZE 200
 int ath11k_core_fetch_bdf(struct ath11k_base *ab, struct ath11k_board_data *bd)
 {
-	char boardname[BOARD_NAME_SIZE];
+	char boardname[BOARD_NAME_SIZE], fallback_boardname[BOARD_NAME_SIZE];
 	int ret;
 
-	ret = ath11k_core_create_board_name(ab, boardname, BOARD_NAME_SIZE);
+	ret = ath11k_core_create_board_name(ab, boardname, sizeof(boardname));
 	if (ret) {
 		ath11k_err(ab, "failed to create board name: %d", ret);
 		return ret;
@@ -766,6 +778,17 @@ int ath11k_core_fetch_bdf(struct ath11k_base *ab, struct ath11k_board_data *bd)
 	if (!ret)
 		goto success;
 
+	ret = ath11k_core_create_fallback_board_name(ab, fallback_boardname,
+						     sizeof(fallback_boardname));
+	if (ret) {
+		ath11k_err(ab, "failed to create fallback board name: %d", ret);
+		return ret;
+	}
+
+	ret = ath11k_core_fetch_board_data_api_n(ab, bd, fallback_boardname);
+	if (!ret)
+		goto success;
+
 	ab->bd_api = 1;
 	ret = ath11k_core_fetch_board_data_api_1(ab, bd);
 	if (ret) {
-- 
2.31.1

Re: [PATCH v5 1/2] ath11k: add fallback board name without variant while searching board-2.bin

[Kalle Valo]

Wen Gong <quic_wgong@quicinc.com> writes:

> Sometimes it has a variant value which read from DT or SMBIOS by
> ath11k, and meanwhile it does not have the matched board name in
> board-2.bin, then it will failed at boot up phase.
>
> Add fallback board name which removed variant value and search again
> in board-2.bin when failed with variant and try to load the board
> data again to increase boot up success rate.
>
> dmesg log after this patch:
> [169547.248472] ath11k_pci 0000:05:00.0: boot using board name 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262,variant=test'
> [169547.248565] ath11k_pci 0000:05:00.0: boot firmware request ath11k/WCN6855/hw2.0/board-2.bin size 180324
> [169547.248568] ath11k_pci 0000:05:00.0: board name
> [169547.248570] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
> [169547.248571] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
> [169547.248572] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
> [169547.248574] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
> [169547.248575] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
> [169547.248576] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 32 2c 71 6d 69 2d 62 6f  chip-id=2,qmi-bo
> [169547.248577] ath11k_pci 0000:05:00.0: 00000060: 61 72 64 2d 69 64 3d 32 36 32                    ard-id=262
> [169547.248578] ath11k_pci 0000:05:00.0: board name
> [169547.248579] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
> [169547.248581] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
> [169547.248582] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
> [169547.248583] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
> [169547.248584] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
> [169547.248585] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 32 2c 71 6d 69 2d 62 6f  chip-id=2,qmi-bo
> [169547.248587] ath11k_pci 0000:05:00.0: 00000060: 61 72 64 2d 69 64 3d 32 36 36                    ard-id=266
> [169547.248588] ath11k_pci 0000:05:00.0: board name
> [169547.248589] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
> [169547.248590] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
> [169547.248591] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
> [169547.248592] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
> [169547.248594] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
> [169547.248595] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 31 38 2c 71 6d 69 2d 62  chip-id=18,qmi-b
> [169547.248596] ath11k_pci 0000:05:00.0: 00000060: 6f 61 72 64 2d 69 64 3d 32 36 36                 oard-id=266
> [169547.248597] ath11k_pci 0000:05:00.0: failed to fetch board data for bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262,variant=test from ath11k/WCN6855/hw2.0/board-2.bin
> [169547.248476] ath11k_pci 0000:05:00.0: boot using board name 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262'
> [169547.248634] ath11k_pci 0000:05:00.0: boot firmware request ath11k/WCN6855/hw2.0/board-2.bin size 180324
> [169547.248636] ath11k_pci 0000:05:00.0: board name
> [169547.248637] ath11k_pci 0000:05:00.0: 00000000: 62 75 73 3d 70 63 69 2c 76 65 6e 64 6f 72 3d 31  bus=pci,vendor=1
> [169547.248638] ath11k_pci 0000:05:00.0: 00000010: 37 63 62 2c 64 65 76 69 63 65 3d 31 31 30 33 2c  7cb,device=1103,
> [169547.248639] ath11k_pci 0000:05:00.0: 00000020: 73 75 62 73 79 73 74 65 6d 2d 76 65 6e 64 6f 72  subsystem-vendor
> [169547.248641] ath11k_pci 0000:05:00.0: 00000030: 3d 31 37 63 62 2c 73 75 62 73 79 73 74 65 6d 2d  =17cb,subsystem-
> [169547.248642] ath11k_pci 0000:05:00.0: 00000040: 64 65 76 69 63 65 3d 33 33 37 34 2c 71 6d 69 2d  device=3374,qmi-
> [169547.248643] ath11k_pci 0000:05:00.0: 00000050: 63 68 69 70 2d 69 64 3d 32 2c 71 6d 69 2d 62 6f  chip-id=2,qmi-bo
> [169547.248645] ath11k_pci 0000:05:00.0: 00000060: 61 72 64 2d 69 64 3d 32 36 32                    ard-id=262
> [169547.248646] ath11k_pci 0000:05:00.0: boot found match for name 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262'
> [169547.248647] ath11k_pci 0000:05:00.0: boot found board data for 'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=2,qmi-board-id=262'
> [169547.248649] ath11k_pci 0000:05:00.0: using board api 2
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>

[...]

> @@ -722,7 +734,7 @@ static int ath11k_core_fetch_board_data_api_n(struct ath11k_base *ab,
>  
>  out:
>  	if (!bd->data || !bd->len) {
> -		ath11k_err(ab,
> +		ath11k_dbg(ab, ATH11K_DBG_BOOT,
>  			   "failed to fetch board data for %s from %s\n",
>  			   boardname, filepath);
>  		ret = -ENODATA;

This error message was useful as it made easy to find out what board
data file we are missing. An example:

https://bugzilla.kernel.org/show_bug.cgi?id=215586#c0

So I'm worried it will become more difficult if the message is changed
to a debug message. But I don't know what's a good solution either, we
shouldn't print an error message while we are searching for the board
file and print it only after we could not find any. One idea I have was
to print every boardname we tried to find or just print the first
boardname, but don't know really what's the best.

But to summarise, we need to think carefully how to handle the missing
board file case. It's very common for users that a board file is missing
in board-2.bin so the error message should be informative enough.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v5 1/2] ath11k: add fallback board name without variant while searching board-2.bin

[Wen Gong]

On 3/10/2022 4:00 PM, Kalle Valo wrote:
> Wen Gong <quic_wgong@quicinc.com> writes:
>
>
...
>> @@ -722,7 +734,7 @@ static int ath11k_core_fetch_board_data_api_n(struct ath11k_base *ab,
>>   
>>   out:
>>   	if (!bd->data || !bd->len) {
>> -		ath11k_err(ab,
>> +		ath11k_dbg(ab, ATH11K_DBG_BOOT,
>>   			   "failed to fetch board data for %s from %s\n",
>>   			   boardname, filepath);
>>   		ret = -ENODATA;
> This error message was useful as it made easy to find out what board
> data file we are missing. An example:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215586#c0
>
> So I'm worried it will become more difficult if the message is changed
> to a debug message. But I don't know what's a good solution either, we
> shouldn't print an error message while we are searching for the board
> file and print it only after we could not find any. One idea I have was
> to print every boardname we tried to find or just print the first
> boardname, but don't know really what's the best.
>
> But to summarise, we need to think carefully how to handle the missing
> board file case. It's very common for users that a board file is missing
> in board-2.bin so the error message should be informative enough.

Yes,

I think we can change to print the error message after we could not find 
any, I will change it in next version patch.

[PATCH v5 2/2] ath11k: add read variant from SMBIOS for download board data

[Wen Gong]

This is to read variant from SMBIOS such as read from DT, the variant
string will be used to one part of string which used to search board
data from board-2.bin.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/core.c | 74 ++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath11k/core.h | 11 ++++
 drivers/net/wireless/ath/ath11k/qmi.c  |  4 ++
 3 files changed, 89 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 641c133563ab..30ae68ac7515 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -8,6 +8,9 @@
 #include <linux/remoteproc.h>
 #include <linux/firmware.h>
 #include <linux/of.h>
+#include <linux/dmi.h>
+#include <linux/ctype.h>
+
 #include "core.h"
 #include "dp_tx.h"
 #include "dp_rx.h"
@@ -460,6 +463,77 @@ int ath11k_core_resume(struct ath11k_base *ab)
 }
 EXPORT_SYMBOL(ath11k_core_resume);
 
+static void ath11k_core_check_bdfext(const struct dmi_header *hdr, void *data)
+{
+	struct ath11k_base *ab = data;
+	const char *bdf_ext;
+	const char *magic = ATH11K_SMBIOS_BDF_EXT_MAGIC;
+	u8 bdf_enabled;
+	int i;
+	size_t len;
+
+	if (ab->qmi.target.bdf_ext[0] != '\0')
+		return;
+
+	if (hdr->type != ATH11K_SMBIOS_BDF_EXT_TYPE)
+		return;
+
+	if (hdr->length != ATH11K_SMBIOS_BDF_EXT_LENGTH) {
+		ath11k_dbg(ab, ATH11K_DBG_BOOT,
+			   "wrong smbios bdf ext type length (%d).\n",
+			   hdr->length);
+		return;
+	}
+
+	bdf_enabled = *((u8 *)hdr + ATH11K_SMBIOS_BDF_EXT_OFFSET);
+	if (!bdf_enabled) {
+		ath11k_dbg(ab, ATH11K_DBG_BOOT, "bdf variant name not found.\n");
+		return;
+	}
+
+	/* Only one string exists (per spec) */
+	bdf_ext = (char *)hdr + hdr->length;
+
+	if (memcmp(bdf_ext, magic, strlen(magic)) != 0) {
+		ath11k_dbg(ab, ATH11K_DBG_BOOT,
+			   "bdf variant magic does not match.\n");
+		return;
+	}
+
+	len = strlen(bdf_ext);
+	for (i = 0; i < len; i++) {
+		if (!isascii(bdf_ext[i]) || !isprint(bdf_ext[i])) {
+			ath11k_dbg(ab, ATH11K_DBG_BOOT,
+				   "bdf variant name contains non ascii chars.\n");
+			return;
+		}
+	}
+
+	/* Copy extension name without magic prefix */
+	if (strscpy(ab->qmi.target.bdf_ext, bdf_ext + strlen(magic),
+		    sizeof(ab->qmi.target.bdf_ext)) < 0) {
+		ath11k_dbg(ab, ATH11K_DBG_BOOT,
+			   "bdf variant string is longer than the buffer can accommodate (variant: %s)\n",
+			    bdf_ext);
+		return;
+	}
+
+	ath11k_dbg(ab, ATH11K_DBG_BOOT,
+		   "found and validated bdf variant smbios_type 0x%x bdf %s\n",
+		   ATH11K_SMBIOS_BDF_EXT_TYPE, bdf_ext);
+}
+
+int ath11k_core_check_smbios(struct ath11k_base *ab)
+{
+	ab->qmi.target.bdf_ext[0] = '\0';
+	dmi_walk(ath11k_core_check_bdfext, ab);
+
+	if (ab->qmi.target.bdf_ext[0] == '\0')
+		return -ENODATA;
+
+	return 0;
+}
+
 int ath11k_core_check_dt(struct ath11k_base *ab)
 {
 	size_t max_len = sizeof(ab->qmi.target.bdf_ext);
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index bab3878a41a7..c0b8f5ab09c9 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -971,7 +971,18 @@ int ath11k_core_fetch_bdf(struct ath11k_base *ath11k,
 			  struct ath11k_board_data *bd);
 void ath11k_core_free_bdf(struct ath11k_base *ab, struct ath11k_board_data *bd);
 int ath11k_core_check_dt(struct ath11k_base *ath11k);
+/* SMBIOS type containing Board Data File Name Extension */
+#define ATH11K_SMBIOS_BDF_EXT_TYPE 0xF8
 
+/* SMBIOS type structure length (excluding strings-set) */
+#define ATH11K_SMBIOS_BDF_EXT_LENGTH 0x9
+
+/* Offset pointing to Board Data File Name Extension */
+#define ATH11K_SMBIOS_BDF_EXT_OFFSET 0x8
+
+/* The magic used by QCA spec */
+#define ATH11K_SMBIOS_BDF_EXT_MAGIC "BDF_"
+int ath11k_core_check_smbios(struct ath11k_base *ab);
 void ath11k_core_halt(struct ath11k *ar);
 int ath11k_core_resume(struct ath11k_base *ab);
 int ath11k_core_suspend(struct ath11k_base *ab);
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 3b9ba0e03a66..f84a83af145d 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2037,6 +2037,10 @@ static int ath11k_qmi_request_target_cap(struct ath11k_base *ab)
 		    ab->qmi.target.fw_build_timestamp,
 		    ab->qmi.target.fw_build_id);
 
+	r = ath11k_core_check_smbios(ab);
+	if (r)
+		ath11k_dbg(ab, ATH11K_DBG_QMI, "SMBIOS bdf variant name not set.\n");
+
 	r = ath11k_core_check_dt(ab);
 	if (r)
 		ath11k_dbg(ab, ATH11K_DBG_QMI, "DT bdf variant name not set.\n");
-- 
2.31.1

Re: [PATCH v5 2/2] ath11k: add read variant from SMBIOS for download board data

[Kalle Valo]

Wen Gong <quic_wgong@quicinc.com> writes:

> This is to read variant from SMBIOS such as read from DT, the variant
> string will be used to one part of string which used to search board
> data from board-2.bin.
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>

[...]

> +static void ath11k_core_check_bdfext(const struct dmi_header *hdr, void *data)
> +{
> +	struct ath11k_base *ab = data;
> +	const char *bdf_ext;
> +	const char *magic = ATH11K_SMBIOS_BDF_EXT_MAGIC;
> +	u8 bdf_enabled;
> +	int i;
> +	size_t len;
> +
> +	if (ab->qmi.target.bdf_ext[0] != '\0')
> +		return;
> +
> +	if (hdr->type != ATH11K_SMBIOS_BDF_EXT_TYPE)
> +		return;
> +
> +	if (hdr->length != ATH11K_SMBIOS_BDF_EXT_LENGTH) {
> +		ath11k_dbg(ab, ATH11K_DBG_BOOT,
> +			   "wrong smbios bdf ext type length (%d).\n",
> +			   hdr->length);
> +		return;
> +	}
> +
> +	bdf_enabled = *((u8 *)hdr + ATH11K_SMBIOS_BDF_EXT_OFFSET);
> +	if (!bdf_enabled) {
> +		ath11k_dbg(ab, ATH11K_DBG_BOOT, "bdf variant name not found.\n");
> +		return;
> +	}
> +
> +	/* Only one string exists (per spec) */
> +	bdf_ext = (char *)hdr + hdr->length;

A proper struct is preferred over pointer arithmetic. For example
something like this:

struct ath11k_smbios_bdf {
        struct dmi_header hdr;
        u32 padding;
        u8 bdf_enabled;
        u8 bdf_ext[ATH11K_SMBIOS_BDF_EXT_MAX_LEN];
}

I'm not sure if I got the offsets right, but I hope you get the idea
anyway.

> +
> +	if (memcmp(bdf_ext, magic, strlen(magic)) != 0) {
> +		ath11k_dbg(ab, ATH11K_DBG_BOOT,
> +			   "bdf variant magic does not match.\n");
> +		return;
> +	}
> +
> +	len = strlen(bdf_ext);

What if bdf_ext is not null terminated? Wouldn't strnlen() with
ATH11K_SMBIOS_BDF_EXT_MAX_LEN would be safer?

> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -971,7 +971,18 @@ int ath11k_core_fetch_bdf(struct ath11k_base *ath11k,
>  			  struct ath11k_board_data *bd);
>  void ath11k_core_free_bdf(struct ath11k_base *ab, struct ath11k_board_data *bd);
>  int ath11k_core_check_dt(struct ath11k_base *ath11k);
> +/* SMBIOS type containing Board Data File Name Extension */
> +#define ATH11K_SMBIOS_BDF_EXT_TYPE 0xF8
>  
> +/* SMBIOS type structure length (excluding strings-set) */
> +#define ATH11K_SMBIOS_BDF_EXT_LENGTH 0x9
> +
> +/* Offset pointing to Board Data File Name Extension */
> +#define ATH11K_SMBIOS_BDF_EXT_OFFSET 0x8
> +
> +/* The magic used by QCA spec */
> +#define ATH11K_SMBIOS_BDF_EXT_MAGIC "BDF_"
> +int ath11k_core_check_smbios(struct ath11k_base *ab);
>  void ath11k_core_halt(struct ath11k *ar);
>  int ath11k_core_resume(struct ath11k_base *ab);
>  int ath11k_core_suspend(struct ath11k_base *ab);

Please don't mix defines and function declarations, so move defines up
in the file.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v5 2/2] ath11k: add read variant from SMBIOS for download board data

[Wen Gong]

On 3/10/2022 4:12 PM, Kalle Valo wrote:
> Wen Gong <quic_wgong@quicinc.com> writes:
>
>> This is to read variant from SMBIOS such as read from DT, the variant
>> string will be used to one part of string which used to search board
>> data from board-2.bin.
>>
>> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>>
>> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
> [...]
>
>> +static void ath11k_core_check_bdfext(const struct dmi_header *hdr, void *data)
>> +{
>> +	struct ath11k_base *ab = data;
>> +	const char *bdf_ext;
>> +	const char *magic = ATH11K_SMBIOS_BDF_EXT_MAGIC;
>> +	u8 bdf_enabled;
>> +	int i;
>> +	size_t len;
>> +
>> +	if (ab->qmi.target.bdf_ext[0] != '\0')
>> +		return;
>> +
>> +	if (hdr->type != ATH11K_SMBIOS_BDF_EXT_TYPE)
>> +		return;
>> +
>> +	if (hdr->length != ATH11K_SMBIOS_BDF_EXT_LENGTH) {
>> +		ath11k_dbg(ab, ATH11K_DBG_BOOT,
>> +			   "wrong smbios bdf ext type length (%d).\n",
>> +			   hdr->length);
>> +		return;
>> +	}
>> +
>> +	bdf_enabled = *((u8 *)hdr + ATH11K_SMBIOS_BDF_EXT_OFFSET);
>> +	if (!bdf_enabled) {
>> +		ath11k_dbg(ab, ATH11K_DBG_BOOT, "bdf variant name not found.\n");
>> +		return;
>> +	}
>> +
>> +	/* Only one string exists (per spec) */
>> +	bdf_ext = (char *)hdr + hdr->length;
> A proper struct is preferred over pointer arithmetic. For example
> something like this:
>
> struct ath11k_smbios_bdf {
>          struct dmi_header hdr;
>          u32 padding;
>          u8 bdf_enabled;
>          u8 bdf_ext[ATH11K_SMBIOS_BDF_EXT_MAX_LEN];
> }
>
> I'm not sure if I got the offsets right, but I hope you get the idea
> anyway.
Will change it.
>> +
>> +	if (memcmp(bdf_ext, magic, strlen(magic)) != 0) {
>> +		ath11k_dbg(ab, ATH11K_DBG_BOOT,
>> +			   "bdf variant magic does not match.\n");
>> +		return;
>> +	}
>> +
>> +	len = strlen(bdf_ext);
> What if bdf_ext is not null terminated? Wouldn't strnlen() with
> ATH11K_SMBIOS_BDF_EXT_MAX_LEN would be safer?
Yes, will change it.
>
>> --- a/drivers/net/wireless/ath/ath11k/core.h
>> +++ b/drivers/net/wireless/ath/ath11k/core.h
>> @@ -971,7 +971,18 @@ int ath11k_core_fetch_bdf(struct ath11k_base *ath11k,
>>   			  struct ath11k_board_data *bd);
>>   void ath11k_core_free_bdf(struct ath11k_base *ab, struct ath11k_board_data *bd);
>>   int ath11k_core_check_dt(struct ath11k_base *ath11k);
>> +/* SMBIOS type containing Board Data File Name Extension */
>> +#define ATH11K_SMBIOS_BDF_EXT_TYPE 0xF8
>>   
>> +/* SMBIOS type structure length (excluding strings-set) */
>> +#define ATH11K_SMBIOS_BDF_EXT_LENGTH 0x9
>> +
>> +/* Offset pointing to Board Data File Name Extension */
>> +#define ATH11K_SMBIOS_BDF_EXT_OFFSET 0x8
>> +
>> +/* The magic used by QCA spec */
>> +#define ATH11K_SMBIOS_BDF_EXT_MAGIC "BDF_"
>> +int ath11k_core_check_smbios(struct ath11k_base *ab);
>>   void ath11k_core_halt(struct ath11k *ar);
>>   int ath11k_core_resume(struct ath11k_base *ab);
>>   int ath11k_core_suspend(struct ath11k_base *ab);
> Please don't mix defines and function declarations, so move defines up
> in the file.
Yes, will change it.

Re: [PATCH v5 0/2] ath11k: add handler for board-2.bin without variant and read SMBIOS

[Wen Gong]

Hi Kalle,

Do you have any comment for this patch set?

On 12/20/2021 2:48 PM, Wen Gong wrote:
> v5:
>     1. rebased to ath.git ath-202112161538
>     2. fix warning "failed to fetch board data for bus=pci,qmi-chip-id=0,qmi-board-id=255,variant=DE_1901 from ath11k/QCA6390/hw2.0/board-2.bin"
>
> v4:
>     1. add helper function ath11k_core_create_fallback_board_name()
>     2. rebased to ath.git ath-202112071521
>
> v3:
>     1. add patch "ath11k: add fallback board name without variant while searching board-2.bin"
>     2. reabsed to latest ath.git master ath-202111221436
>
> v2: rebased to latest ath.git master ath-202111170737
>
> When it does not have matched board name with variant in board-2.bin,
> ath11k will fallback to search without variant.
>
> Also read the variant from SMBIOS.
>
> Wen Gong (2):
>    ath11k: add fallback board name without variant while searching
>      board-2.bin
>    ath11k: add read variant from SMBIOS for download board data
>
>   drivers/net/wireless/ath/ath11k/core.c | 109 +++++++++++++++++++++++--
>   drivers/net/wireless/ath/ath11k/core.h |  11 +++
>   drivers/net/wireless/ath/ath11k/qmi.c  |   4 +
>   3 files changed, 118 insertions(+), 6 deletions(-)
>
>
> base-commit: 12018ad8603a598d773e2cae12542a1981c94886