[PATCH v1 0/4] Add qcom,product-variant properties in Qualcomm

[PATCH v1 0/4] Add qcom,product-variant properties in Qualcomm

[Cheng Jiang]

Add a new property in qualcom bluetooth dts to identify the product
information, so the driver can load the proper firmware.

Several Qualcomm projects will use the same Bluetooth chip, each
focusing on different features. For instance, consumer projects
prioritize the A2DP SRC feature, while IoT projects focus on the A2DP
SINK feature. Due to the patch size, it is not feasible to include all
features in a single firmware.

Therefore, the 'product-variant' devicetree property is used to provide
product information for the Bluetooth driver to load the appropriate
firmware.

The driver will parse 'product-variant' to load firmware from different
directories. If it's not defined in dts, the default firmware will be
loaded, which is compatible with the existing implementaion.


Cheng Jiang (4):
  dt-bindings: bluetooth: add 'qcom,product-variant'
  dt-bindings: bluetooth: Add qca6698 compatible string
  arm64: dts: qcom: sa8775p-ride: update BT nodes
  Bluetooth: hci_qca: add qcom,product-variant properties

 .../net/bluetooth/qualcomm-bluetooth.yaml     |   8 +
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi    |   3 +-
 drivers/bluetooth/btqca.c                     | 142 ++++++++++++++----
 drivers/bluetooth/btqca.h                     |  11 +-
 drivers/bluetooth/hci_qca.c                   |  73 +++++----
 5 files changed, 174 insertions(+), 63 deletions(-)


base-commit: 6fb2fa9805c501d9ade047fc511961f3273cdcb5
-- 
2.25.1

[PATCH v2 1/4] dt-bindings: bluetooth: add 'qcom,product-variant'

[Cheng Jiang]

Several Qualcomm projects will use the same Bluetooth chip, each
focusing on different features. For instance, consumer projects
prioritize the A2DP SRC feature, while IoT projects focus on the A2DP
SINK feature, which may have more optimizations for coexistence when
acting as a SINK. Due to the patch size, it is not feasible to include
all features in a single firmware.

Therefore, the 'product-variant' devicetree property is used to provide
product information for the Bluetooth driver to load the appropriate
firmware.

If this property is not defined, the default firmware will be loaded,
ensuring there are no backward compatibility issues with older
devicetrees.

The product-variant defines like this:
  0 - 15 (16 bits) are product line specific definitions
  16 - 23 (8 bits) are for the product line.
  24 - 31 (8 bits) are reserved for future use, 0 currently

|---------------------------------------------------------------------|
|                       32 Bits                                       |
|---------------------------------------------------------------------|
|  31 - 24 (bits)   |    23 - 16 (bits)   | 15 - 0 (16 bits)          |
|---------------------------------------------------------------------|
|   Reserved        |    0: default       | 0: default                |
|                   |    1: CE            |                           |
|                   |    2: IoT           |                           |
|                   |    3: Auto          |                           |
|                   |    4: Reserved      |                           |
|---------------------------------------------------------------------|

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 .../bindings/net/bluetooth/qualcomm-bluetooth.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
index 7bb68311c609..9019fe7bcdc6 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
@@ -110,6 +110,12 @@ properties:
     description:
       boot firmware is incorrectly passing the address in big-endian order
 
+  qcom,product-variant:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      specify the product information for driver to load the appropriate firmware
+
+
 required:
   - compatible
 
-- 
2.25.1

[PATCH v2 2/4] dt-bindings: bluetooth: Add qca6698 compatible string

[Cheng Jiang]

Add QCA6698 qcom,qca6698-bt compatible strings.

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 .../devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml   | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
index 9019fe7bcdc6..527f947289af 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
@@ -18,6 +18,7 @@ properties:
     enum:
       - qcom,qca2066-bt
       - qcom,qca6174-bt
+      - qcom,qca6698-bt
       - qcom,qca9377-bt
       - qcom,wcn3988-bt
       - qcom,wcn3990-bt
@@ -175,6 +176,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,qca6698-bt
               - qcom,wcn6855-bt
     then:
       required:
-- 
2.25.1

[PATCH v2 3/4] arm64: dts: qcom: sa8775p-ride: update BT nodes

[Cheng Jiang]

Add product-variant property to specify the IoT product line.
Update the chip soc type, SA8775P-ride platform uses the QCA6698
chip, which is compatible with the WCN6855. It's necessary to use this
new SoC type to distinguish it from projects using WCN chips.

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
index 3fc62e123689..da52f425c676 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -856,7 +856,8 @@ &uart17 {
 	status = "okay";
 
 	bluetooth {
-		compatible = "qcom,wcn6855-bt";
+		compatible = "qcom,qca6698-bt";
+		qcom,product-variant = <0x20000>;
 
 		vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
 		vddaon-supply = <&vreg_pmu_aon_0p59>;
-- 
2.25.1

[PATCH v2 4/4] Bluetooth: hci_qca: add qcom,product-variant properties

[Cheng Jiang]

Since different products use the same SoC chip, features cannot
be included in a single patch. Use the qcom,product-variant to
load the appropriate firmware.

The qcom,product-variant provides product line information, which
the driver uses to load firmware from different directories.

If it's not defined in dts, the default firmware will be loaded.

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 drivers/bluetooth/btqca.c   | 142 +++++++++++++++++++++++++++++-------
 drivers/bluetooth/btqca.h   |  11 ++-
 drivers/bluetooth/hci_qca.c |  73 +++++++++---------
 3 files changed, 164 insertions(+), 62 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index dfbbac92242a..0845e5a60412 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -700,8 +700,79 @@ static int qca_check_bdaddr(struct hci_dev *hdev, const struct qca_fw_config *co
 	return 0;
 }
 
-static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
-		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
+
+const char *qca_get_soc_name(enum qca_btsoc_type soc_type)
+{
+	const char *soc_name = "";
+
+	switch (soc_type) {
+	case QCA_QCA2066:
+		soc_name = "QCA2066";
+		break;
+
+	case QCA_QCA6698:
+		soc_name = "QCA6698";
+		break;
+
+	case QCA_WCN3988:
+	case QCA_WCN3990:
+	case QCA_WCN3991:
+	case QCA_WCN3998:
+		soc_name = "WCN399x";
+		break;
+
+	case QCA_WCN6750:
+		soc_name = "WCN6750";
+		break;
+
+	case QCA_WCN6855:
+		soc_name = "WCN6855";
+		break;
+
+	case QCA_WCN7850:
+		soc_name = "WCN7850";
+		break;
+
+	default:
+		soc_name = "ROME/QCA6390";
+	}
+
+	return soc_name;
+}
+EXPORT_SYMBOL_GPL(qca_get_soc_name);
+
+static void qca_get_firmware_path(enum qca_btsoc_type soc_type, char *fw_path,
+		size_t max_size, enum qca_product_type product_type)
+{
+	const char *fw_dir = NULL;
+
+	switch (product_type) {
+	case QCA_MCC:
+		fw_dir = "qca";
+		break;
+	case QCA_CE:
+		fw_dir = "qca/ce";
+		break;
+	case QCA_IOT:
+		fw_dir = "qca/iot";
+		break;
+	case QCA_AUTO:
+		fw_dir = "qca/auto";
+		break;
+	default:
+		fw_dir = "qca";
+		break;
+	}
+
+	if (product_type == QCA_IOT)
+		snprintf(fw_path, max_size, "%s/%s", fw_dir, qca_get_soc_name(soc_type));
+	else
+		snprintf(fw_path, max_size, "%s", fw_dir);
+}
+
+static void qca_generate_hsp_nvm_name(enum qca_btsoc_type soc_type, char *fwname,
+		size_t max_size, const char *fw_path, struct qca_btsoc_version ver, u8 rom_ver,
+		u16 bid)
 {
 	const char *variant;
 
@@ -712,33 +783,36 @@ static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
 		variant = "";
 
 	if (bid == 0x0)
-		snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
+		snprintf(fwname, max_size, "%s/hpnv%02x%s.bin", fw_path, rom_ver, variant);
 	else
-		snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
+		snprintf(fwname, max_size, "%s/hpnv%02x%s.%x", fw_path, rom_ver, variant, bid);
 }
 
-static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
+static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg, const char *fw_path,
 					    const char *stem, u8 rom_ver, u16 bid)
 {
 	if (bid == 0x0)
-		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
+		snprintf(cfg->fwname, sizeof(cfg->fwname),
+			 "%s/%snv%02x.bin", fw_path, stem, rom_ver);
 	else if (bid & 0xff00)
 		snprintf(cfg->fwname, sizeof(cfg->fwname),
-			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
+			 "%s/%snv%02x.b%x", fw_path, stem, rom_ver, bid);
 	else
 		snprintf(cfg->fwname, sizeof(cfg->fwname),
-			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
+			 "%s/%snv%02x.b%02x", fw_path, stem, rom_ver, bid);
 }
 
 int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 		   enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
-		   const char *firmware_name)
+		   const char *firmware_name, uint32_t product_variant)
 {
 	struct qca_fw_config config = {};
 	int err;
 	u8 rom_ver = 0;
 	u32 soc_ver;
 	u16 boardid = 0;
+	enum qca_product_type product_type;
+	char fw_path[64] = {0};
 
 	bt_dev_dbg(hdev, "QCA setup on UART");
 
@@ -759,6 +833,10 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	if (soc_type == QCA_WCN6750)
 		qca_send_patch_config_cmd(hdev);
 
+	/* Get the f/w path based on product variant */
+	product_type = (product_variant >> 16) & 0xff;
+	qca_get_firmware_path(soc_type, fw_path, sizeof(fw_path), product_type);
+
 	/* Download rampatch file */
 	config.type = TLV_TYPE_PATCH;
 	switch (soc_type) {
@@ -766,19 +844,23 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	case QCA_WCN3991:
 	case QCA_WCN3998:
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/crbtfw%02x.tlv", rom_ver);
+			 "%s/crbtfw%02x.tlv", fw_path, rom_ver);
 		break;
 	case QCA_WCN3988:
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/apbtfw%02x.tlv", rom_ver);
+			 "%s/apbtfw%02x.tlv", fw_path, rom_ver);
 		break;
 	case QCA_QCA2066:
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hpbtfw%02x.tlv", rom_ver);
+			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
 		break;
 	case QCA_QCA6390:
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/htbtfw%02x.tlv", rom_ver);
+			 "%s/htbtfw%02x.tlv", fw_path, rom_ver);
+		break;
+	case QCA_QCA6698:
+		snprintf(config.fwname, sizeof(config.fwname),
+			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
 		break;
 	case QCA_WCN6750:
 		/* Choose mbn file by default.If mbn file is not found
@@ -786,19 +868,19 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 		 */
 		config.type = ELF_TYPE_PATCH;
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/msbtfw%02x.mbn", rom_ver);
+			 "%s/msbtfw%02x.mbn", fw_path, rom_ver);
 		break;
 	case QCA_WCN6855:
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hpbtfw%02x.tlv", rom_ver);
+			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
 		break;
 	case QCA_WCN7850:
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hmtbtfw%02x.tlv", rom_ver);
+			 "%s/hmtbtfw%02x.tlv", fw_path, rom_ver);
 		break;
 	default:
 		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/rampatch_%08x.bin", soc_ver);
+			 "%s/rampatch_%08x.bin", fw_path, soc_ver);
 	}
 
 	err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
@@ -810,7 +892,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	/* Give the controller some time to get ready to receive the NVM */
 	msleep(10);
 
-	if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850)
+	if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850 ||
+		soc_type == QCA_QCA6698)
 		qca_read_fw_board_id(hdev, &boardid);
 
 	/* Download NVM configuration */
@@ -825,39 +908,40 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 		case QCA_WCN3998:
 			if (le32_to_cpu(ver.soc_id) == QCA_WCN3991_SOC_ID) {
 				snprintf(config.fwname, sizeof(config.fwname),
-					 "qca/crnv%02xu.bin", rom_ver);
+					 "%s/crnv%02xu.bin", fw_path, rom_ver);
 			} else {
 				snprintf(config.fwname, sizeof(config.fwname),
-					 "qca/crnv%02x.bin", rom_ver);
+					 "%s/crnv%02x.bin", fw_path, rom_ver);
 			}
 			break;
 		case QCA_WCN3988:
 			snprintf(config.fwname, sizeof(config.fwname),
-				 "qca/apnv%02x.bin", rom_ver);
+				 "%s/apnv%02x.bin", fw_path, rom_ver);
 			break;
 		case QCA_QCA2066:
-			qca_generate_hsp_nvm_name(config.fwname,
-				sizeof(config.fwname), ver, rom_ver, boardid);
+		case QCA_QCA6698:
+			qca_generate_hsp_nvm_name(soc_type, config.fwname,
+				sizeof(config.fwname), fw_path, ver, rom_ver, boardid);
 			break;
 		case QCA_QCA6390:
 			snprintf(config.fwname, sizeof(config.fwname),
-				 "qca/htnv%02x.bin", rom_ver);
+				 "%s/htnv%02x.bin", fw_path, rom_ver);
 			break;
 		case QCA_WCN6750:
 			snprintf(config.fwname, sizeof(config.fwname),
-				 "qca/msnv%02x.bin", rom_ver);
+				 "%s/msnv%02x.bin", fw_path, rom_ver);
 			break;
 		case QCA_WCN6855:
 			snprintf(config.fwname, sizeof(config.fwname),
-				 "qca/hpnv%02x.bin", rom_ver);
+				 "%s/hpnv%02x.bin", fw_path, rom_ver);
 			break;
 		case QCA_WCN7850:
-			qca_get_nvm_name_generic(&config, "hmt", rom_ver, boardid);
+			qca_get_nvm_name_generic(&config, "hmt", fw_path, rom_ver, boardid);
 			break;
 
 		default:
 			snprintf(config.fwname, sizeof(config.fwname),
-				 "qca/nvm_%08x.bin", soc_ver);
+				 "%s/nvm_%08x.bin", fw_path, soc_ver);
 		}
 	}
 
@@ -871,6 +955,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	case QCA_WCN3991:
 	case QCA_QCA2066:
 	case QCA_QCA6390:
+	case QCA_QCA6698:
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
@@ -909,6 +994,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
+	case QCA_QCA6698:
 		/* get fw build info */
 		err = qca_read_fw_build_info(hdev);
 		if (err < 0)
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index bb5207d7a8c7..baa3f979d017 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -151,21 +151,30 @@ enum qca_btsoc_type {
 	QCA_WCN3991,
 	QCA_QCA2066,
 	QCA_QCA6390,
+	QCA_QCA6698,
 	QCA_WCN6750,
 	QCA_WCN6855,
 	QCA_WCN7850,
 };
 
+enum qca_product_type {
+	QCA_MCC = 0,
+	QCA_CE,
+	QCA_IOT,
+	QCA_AUTO,
+};
+
 #if IS_ENABLED(CONFIG_BT_QCA)
 
 int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
 int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 		   enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
-		   const char *firmware_name);
+		   const char *firmware_name, uint32_t product_variant);
 int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
 			 enum qca_btsoc_type);
 int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
 int qca_send_pre_shutdown_cmd(struct hci_dev *hdev);
+const char *qca_get_soc_name(enum qca_btsoc_type soc_type);
 #else
 
 static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 37129e6cb0eb..69fec890eb8c 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -227,6 +227,7 @@ struct qca_serdev {
 	struct qca_power *bt_power;
 	u32 init_speed;
 	u32 oper_speed;
+	u32 product_variant;
 	bool bdaddr_property_broken;
 	const char *firmware_name;
 };
@@ -1361,6 +1362,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
+	case QCA_QCA6698:
 		usleep_range(1000, 10000);
 		break;
 
@@ -1447,6 +1449,7 @@ static int qca_check_speeds(struct hci_uart *hu)
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
+	case QCA_QCA6698:
 		if (!qca_get_speed(hu, QCA_INIT_SPEED) &&
 		    !qca_get_speed(hu, QCA_OPER_SPEED))
 			return -EINVAL;
@@ -1489,6 +1492,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
 		case QCA_WCN6750:
 		case QCA_WCN6855:
 		case QCA_WCN7850:
+		case QCA_QCA6698:
 			hci_uart_set_flow_control(hu, true);
 			break;
 
@@ -1523,6 +1527,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
 		case QCA_WCN6750:
 		case QCA_WCN6855:
 		case QCA_WCN7850:
+		case QCA_QCA6698:
 			hci_uart_set_flow_control(hu, false);
 			break;
 
@@ -1803,6 +1808,7 @@ static int qca_power_on(struct hci_dev *hdev)
 	case QCA_WCN6855:
 	case QCA_WCN7850:
 	case QCA_QCA6390:
+	case QCA_QCA6698:
 		ret = qca_regulator_init(hu);
 		break;
 
@@ -1858,7 +1864,6 @@ static int qca_setup(struct hci_uart *hu)
 	int ret;
 	struct qca_btsoc_version ver;
 	struct qca_serdev *qcadev;
-	const char *soc_name;
 
 	ret = qca_check_speeds(hu);
 	if (ret)
@@ -1873,34 +1878,7 @@ static int qca_setup(struct hci_uart *hu)
 	 */
 	set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 
-	switch (soc_type) {
-	case QCA_QCA2066:
-		soc_name = "qca2066";
-		break;
-
-	case QCA_WCN3988:
-	case QCA_WCN3990:
-	case QCA_WCN3991:
-	case QCA_WCN3998:
-		soc_name = "wcn399x";
-		break;
-
-	case QCA_WCN6750:
-		soc_name = "wcn6750";
-		break;
-
-	case QCA_WCN6855:
-		soc_name = "wcn6855";
-		break;
-
-	case QCA_WCN7850:
-		soc_name = "wcn7850";
-		break;
-
-	default:
-		soc_name = "ROME/QCA6390";
-	}
-	bt_dev_info(hdev, "setting up %s", soc_name);
+	bt_dev_info(hdev, "setting up %s", qca_get_soc_name(soc_type));
 
 	qca->memdump_state = QCA_MEMDUMP_IDLE;
 
@@ -1919,6 +1897,7 @@ static int qca_setup(struct hci_uart *hu)
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
+	case QCA_QCA6698:
 		qcadev = serdev_device_get_drvdata(hu->serdev);
 		if (qcadev->bdaddr_property_broken)
 			set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
@@ -1952,6 +1931,7 @@ static int qca_setup(struct hci_uart *hu)
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
+	case QCA_QCA6698:
 		break;
 
 	default:
@@ -1963,7 +1943,7 @@ static int qca_setup(struct hci_uart *hu)
 
 	/* Setup patch / NVM configurations */
 	ret = qca_uart_setup(hdev, qca_baudrate, soc_type, ver,
-			firmware_name);
+			firmware_name, qcadev->product_variant);
 	if (!ret) {
 		clear_bit(QCA_IBS_DISABLED, &qca->flags);
 		qca_debugfs_init(hdev);
@@ -2089,6 +2069,20 @@ static const struct qca_device_data qca_soc_data_qca6390 __maybe_unused = {
 	.num_vregs = 0,
 };
 
+static const struct qca_device_data qca_soc_data_qca6698 __maybe_unused = {
+	.soc_type = QCA_QCA6698,
+	.vregs = (struct qca_vreg []) {
+		{ "vddio", 5000 },
+		{ "vddbtcxmx", 126000 },
+		{ "vddrfacmn", 12500 },
+		{ "vddrfa0p8", 102000 },
+		{ "vddrfa1p7", 302000 },
+		{ "vddrfa1p2", 257000 },
+	},
+	.num_vregs = 6,
+	.capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
+};
+
 static const struct qca_device_data qca_soc_data_wcn6750 __maybe_unused = {
 	.soc_type = QCA_WCN6750,
 	.vregs = (struct qca_vreg []) {
@@ -2165,7 +2159,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
 		pwrseq_power_off(power->pwrseq);
 		set_bit(QCA_BT_OFF, &qca->flags);
 		return;
-        }
+	}
 
 	switch (soc_type) {
 	case QCA_WCN3988:
@@ -2179,6 +2173,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
 
 	case QCA_WCN6750:
 	case QCA_WCN6855:
+	case QCA_QCA6698:
 		gpiod_set_value_cansleep(qcadev->bt_en, 0);
 		msleep(100);
 		qca_regulator_disable(qcadev);
@@ -2313,6 +2308,12 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 					 &qcadev->firmware_name);
 	device_property_read_u32(&serdev->dev, "max-speed",
 				 &qcadev->oper_speed);
+	device_property_read_u32(&serdev->dev, "qcom,product-variant",
+				 &qcadev->product_variant);
+
+	if (qcadev->product_variant != 0)
+		BT_INFO("QC Product Variant: 0x%08x", qcadev->product_variant);
+
 	if (!qcadev->oper_speed)
 		BT_DBG("UART will pick default operating speed");
 
@@ -2333,6 +2334,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 	case QCA_WCN6855:
 	case QCA_WCN7850:
 	case QCA_QCA6390:
+	case QCA_QCA6698:
 		qcadev->bt_power = devm_kzalloc(&serdev->dev,
 						sizeof(struct qca_power),
 						GFP_KERNEL);
@@ -2346,6 +2348,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 	switch (qcadev->btsoc_type) {
 	case QCA_WCN6855:
 	case QCA_WCN7850:
+	case QCA_QCA6698:
 		if (!device_property_present(&serdev->dev, "enable-gpios")) {
 			/*
 			 * Backward compatibility with old DT sources. If the
@@ -2380,7 +2383,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 					       GPIOD_OUT_LOW);
 		if (IS_ERR(qcadev->bt_en) &&
 		    (data->soc_type == QCA_WCN6750 ||
-		     data->soc_type == QCA_WCN6855)) {
+		     data->soc_type == QCA_WCN6855 ||
+		     data->soc_type == QCA_QCA6698)) {
 			dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
 			return PTR_ERR(qcadev->bt_en);
 		}
@@ -2393,7 +2397,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 		if (IS_ERR(qcadev->sw_ctrl) &&
 		    (data->soc_type == QCA_WCN6750 ||
 		     data->soc_type == QCA_WCN6855 ||
-		     data->soc_type == QCA_WCN7850)) {
+		     data->soc_type == QCA_WCN7850 ||
+		     data->soc_type == QCA_QCA6698)) {
 			dev_err(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
 			return PTR_ERR(qcadev->sw_ctrl);
 		}
@@ -2475,6 +2480,7 @@ static void qca_serdev_remove(struct serdev_device *serdev)
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
+	case QCA_QCA6698:
 		if (power->vregs_on)
 			qca_power_shutdown(&qcadev->serdev_hu);
 		break;
@@ -2669,6 +2675,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
 	{ .compatible = "qcom,qca2066-bt", .data = &qca_soc_data_qca2066},
 	{ .compatible = "qcom,qca6174-bt" },
 	{ .compatible = "qcom,qca6390-bt", .data = &qca_soc_data_qca6390},
+	{ .compatible = "qcom,qca6698-bt", .data = &qca_soc_data_qca6698},
 	{ .compatible = "qcom,qca9377-bt" },
 	{ .compatible = "qcom,wcn3988-bt", .data = &qca_soc_data_wcn3988},
 	{ .compatible = "qcom,wcn3990-bt", .data = &qca_soc_data_wcn3990},
-- 
2.25.1

Re: [PATCH v2 1/4] dt-bindings: bluetooth: add 'qcom,product-variant'

[Dmitry Baryshkov]

On Wed, Nov 20, 2024 at 05:54:25PM +0800, Cheng Jiang wrote:
> Several Qualcomm projects will use the same Bluetooth chip, each
> focusing on different features. For instance, consumer projects
> prioritize the A2DP SRC feature, while IoT projects focus on the A2DP
> SINK feature, which may have more optimizations for coexistence when
> acting as a SINK. Due to the patch size, it is not feasible to include
> all features in a single firmware.
> 
> Therefore, the 'product-variant' devicetree property is used to provide
> product information for the Bluetooth driver to load the appropriate
> firmware.
> 
> If this property is not defined, the default firmware will be loaded,
> ensuring there are no backward compatibility issues with older
> devicetrees.
> 
> The product-variant defines like this:
>   0 - 15 (16 bits) are product line specific definitions
>   16 - 23 (8 bits) are for the product line.
>   24 - 31 (8 bits) are reserved for future use, 0 currently

Please use text strings instead of encoding this information into random
integers and then using just 3 bits out of 32.

> 
> |---------------------------------------------------------------------|
> |                       32 Bits                                       |
> |---------------------------------------------------------------------|
> |  31 - 24 (bits)   |    23 - 16 (bits)   | 15 - 0 (16 bits)          |
> |---------------------------------------------------------------------|
> |   Reserved        |    0: default       | 0: default                |
> |                   |    1: CE            |                           |
> |                   |    2: IoT           |                           |
> |                   |    3: Auto          |                           |
> |                   |    4: Reserved      |                           |
> |---------------------------------------------------------------------|
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  .../bindings/net/bluetooth/qualcomm-bluetooth.yaml          | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> index 7bb68311c609..9019fe7bcdc6 100644
> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> @@ -110,6 +110,12 @@ properties:
>      description:
>        boot firmware is incorrectly passing the address in big-endian order
>  
> +  qcom,product-variant:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      specify the product information for driver to load the appropriate firmware

DT describes hardware. Is this a hardware property?

> +
> +
>  required:
>    - compatible
>  
> -- 
> 2.25.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/4] dt-bindings: bluetooth: Add qca6698 compatible string

[Dmitry Baryshkov]

On Wed, Nov 20, 2024 at 05:54:26PM +0800, Cheng Jiang wrote:
> Add QCA6698 qcom,qca6698-bt compatible strings.

Why? Is it the same chip as WCN6855 or a different chip? Is it
completely compatible?

> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  .../devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml   | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> index 9019fe7bcdc6..527f947289af 100644
> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> @@ -18,6 +18,7 @@ properties:
>      enum:
>        - qcom,qca2066-bt
>        - qcom,qca6174-bt
> +      - qcom,qca6698-bt
>        - qcom,qca9377-bt
>        - qcom,wcn3988-bt
>        - qcom,wcn3990-bt
> @@ -175,6 +176,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - qcom,qca6698-bt
>                - qcom,wcn6855-bt
>      then:
>        required:
> -- 
> 2.25.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/4] arm64: dts: qcom: sa8775p-ride: update BT nodes

[Dmitry Baryshkov]

On Wed, Nov 20, 2024 at 05:54:27PM +0800, Cheng Jiang wrote:
> Add product-variant property to specify the IoT product line.
> Update the chip soc type, SA8775P-ride platform uses the QCA6698
> chip, which is compatible with the WCN6855. It's necessary to use this
> new SoC type to distinguish it from projects using WCN chips.

Why?

> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> index 3fc62e123689..da52f425c676 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> @@ -856,7 +856,8 @@ &uart17 {
>  	status = "okay";
>  
>  	bluetooth {
> -		compatible = "qcom,wcn6855-bt";
> +		compatible = "qcom,qca6698-bt";
> +		qcom,product-variant = <0x20000>;
>  
>  		vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
>  		vddaon-supply = <&vreg_pmu_aon_0p59>;
> -- 
> 2.25.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/4] Bluetooth: hci_qca: add qcom,product-variant properties

[Dmitry Baryshkov]

On Wed, Nov 20, 2024 at 05:54:28PM +0800, Cheng Jiang wrote:
> Since different products use the same SoC chip, features cannot
> be included in a single patch. Use the qcom,product-variant to
> load the appropriate firmware.

I can not understand this: what kind of features are so different that
you can not include them into a single firmware image? Please enable all
users with all possible features instead of tying them to a product
segment. If I'm missing something, please provide additional
information.

> 
> The qcom,product-variant provides product line information, which
> the driver uses to load firmware from different directories.
> 
> If it's not defined in dts, the default firmware will be loaded.
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  drivers/bluetooth/btqca.c   | 142 +++++++++++++++++++++++++++++-------
>  drivers/bluetooth/btqca.h   |  11 ++-
>  drivers/bluetooth/hci_qca.c |  73 +++++++++---------
>  3 files changed, 164 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac92242a..0845e5a60412 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -700,8 +700,79 @@ static int qca_check_bdaddr(struct hci_dev *hdev, const struct qca_fw_config *co
>  	return 0;
>  }
>  
> -static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
> -		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
> +
> +const char *qca_get_soc_name(enum qca_btsoc_type soc_type)
> +{
> +	const char *soc_name = "";
> +
> +	switch (soc_type) {
> +	case QCA_QCA2066:
> +		soc_name = "QCA2066";
> +		break;
> +
> +	case QCA_QCA6698:
> +		soc_name = "QCA6698";
> +		break;
> +
> +	case QCA_WCN3988:
> +	case QCA_WCN3990:
> +	case QCA_WCN3991:
> +	case QCA_WCN3998:
> +		soc_name = "WCN399x";
> +		break;
> +
> +	case QCA_WCN6750:
> +		soc_name = "WCN6750";
> +		break;
> +
> +	case QCA_WCN6855:
> +		soc_name = "WCN6855";
> +		break;
> +
> +	case QCA_WCN7850:
> +		soc_name = "WCN7850";
> +		break;
> +
> +	default:
> +		soc_name = "ROME/QCA6390";
> +	}
> +
> +	return soc_name;
> +}
> +EXPORT_SYMBOL_GPL(qca_get_soc_name);
> +
> +static void qca_get_firmware_path(enum qca_btsoc_type soc_type, char *fw_path,
> +		size_t max_size, enum qca_product_type product_type)
> +{
> +	const char *fw_dir = NULL;
> +
> +	switch (product_type) {
> +	case QCA_MCC:
> +		fw_dir = "qca";
> +		break;
> +	case QCA_CE:
> +		fw_dir = "qca/ce";
> +		break;
> +	case QCA_IOT:
> +		fw_dir = "qca/iot";
> +		break;
> +	case QCA_AUTO:
> +		fw_dir = "qca/auto";
> +		break;
> +	default:
> +		fw_dir = "qca";
> +		break;
> +	}
> +
> +	if (product_type == QCA_IOT)
> +		snprintf(fw_path, max_size, "%s/%s", fw_dir, qca_get_soc_name(soc_type));

Why do you need even more nesting for IoT products? "qca/iot/ROME/QCA6390"
also looks strange, but perfectly possible with your patch

> +	else
> +		snprintf(fw_path, max_size, "%s", fw_dir);

Without the IoT platform you can just include a static string.

> +}
> +
> +static void qca_generate_hsp_nvm_name(enum qca_btsoc_type soc_type, char *fwname,
> +		size_t max_size, const char *fw_path, struct qca_btsoc_version ver, u8 rom_ver,
> +		u16 bid)
>  {
>  	const char *variant;
>  
> @@ -712,33 +783,36 @@ static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
>  		variant = "";
>  
>  	if (bid == 0x0)
> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
> +		snprintf(fwname, max_size, "%s/hpnv%02x%s.bin", fw_path, rom_ver, variant);
>  	else
> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
> +		snprintf(fwname, max_size, "%s/hpnv%02x%s.%x", fw_path, rom_ver, variant, bid);
>  }
>  
> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
> +static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg, const char *fw_path,
>  					    const char *stem, u8 rom_ver, u16 bid)
>  {
>  	if (bid == 0x0)
> -		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
> +		snprintf(cfg->fwname, sizeof(cfg->fwname),
> +			 "%s/%snv%02x.bin", fw_path, stem, rom_ver);
>  	else if (bid & 0xff00)
>  		snprintf(cfg->fwname, sizeof(cfg->fwname),
> -			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
> +			 "%s/%snv%02x.b%x", fw_path, stem, rom_ver, bid);
>  	else
>  		snprintf(cfg->fwname, sizeof(cfg->fwname),
> -			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
> +			 "%s/%snv%02x.b%02x", fw_path, stem, rom_ver, bid);
>  }
>  
>  int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  		   enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
> -		   const char *firmware_name)
> +		   const char *firmware_name, uint32_t product_variant)
>  {
>  	struct qca_fw_config config = {};
>  	int err;
>  	u8 rom_ver = 0;
>  	u32 soc_ver;
>  	u16 boardid = 0;
> +	enum qca_product_type product_type;
> +	char fw_path[64] = {0};

No need to init it with lame data.

>  
>  	bt_dev_dbg(hdev, "QCA setup on UART");
>  
> @@ -759,6 +833,10 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  	if (soc_type == QCA_WCN6750)
>  		qca_send_patch_config_cmd(hdev);
>  
> +	/* Get the f/w path based on product variant */
> +	product_type = (product_variant >> 16) & 0xff;
> +	qca_get_firmware_path(soc_type, fw_path, sizeof(fw_path), product_type);
> +
>  	/* Download rampatch file */
>  	config.type = TLV_TYPE_PATCH;
>  	switch (soc_type) {
> @@ -766,19 +844,23 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  	case QCA_WCN3991:
>  	case QCA_WCN3998:
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/crbtfw%02x.tlv", rom_ver);
> +			 "%s/crbtfw%02x.tlv", fw_path, rom_ver);
>  		break;
>  	case QCA_WCN3988:
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/apbtfw%02x.tlv", rom_ver);
> +			 "%s/apbtfw%02x.tlv", fw_path, rom_ver);
>  		break;
>  	case QCA_QCA2066:
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hpbtfw%02x.tlv", rom_ver);
> +			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
>  		break;
>  	case QCA_QCA6390:
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/htbtfw%02x.tlv", rom_ver);
> +			 "%s/htbtfw%02x.tlv", fw_path, rom_ver);
> +		break;
> +	case QCA_QCA6698:
> +		snprintf(config.fwname, sizeof(config.fwname),
> +			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
>  		break;
>  	case QCA_WCN6750:
>  		/* Choose mbn file by default.If mbn file is not found
> @@ -786,19 +868,19 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  		 */
>  		config.type = ELF_TYPE_PATCH;
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/msbtfw%02x.mbn", rom_ver);
> +			 "%s/msbtfw%02x.mbn", fw_path, rom_ver);
>  		break;
>  	case QCA_WCN6855:
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hpbtfw%02x.tlv", rom_ver);
> +			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
>  		break;
>  	case QCA_WCN7850:
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hmtbtfw%02x.tlv", rom_ver);
> +			 "%s/hmtbtfw%02x.tlv", fw_path, rom_ver);
>  		break;
>  	default:
>  		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/rampatch_%08x.bin", soc_ver);
> +			 "%s/rampatch_%08x.bin", fw_path, soc_ver);
>  	}
>  
>  	err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
> @@ -810,7 +892,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  	/* Give the controller some time to get ready to receive the NVM */
>  	msleep(10);
>  
> -	if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850)
> +	if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850 ||
> +		soc_type == QCA_QCA6698)
>  		qca_read_fw_board_id(hdev, &boardid);
>  
>  	/* Download NVM configuration */
> @@ -825,39 +908,40 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  		case QCA_WCN3998:
>  			if (le32_to_cpu(ver.soc_id) == QCA_WCN3991_SOC_ID) {
>  				snprintf(config.fwname, sizeof(config.fwname),
> -					 "qca/crnv%02xu.bin", rom_ver);
> +					 "%s/crnv%02xu.bin", fw_path, rom_ver);
>  			} else {
>  				snprintf(config.fwname, sizeof(config.fwname),
> -					 "qca/crnv%02x.bin", rom_ver);
> +					 "%s/crnv%02x.bin", fw_path, rom_ver);
>  			}
>  			break;
>  		case QCA_WCN3988:
>  			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/apnv%02x.bin", rom_ver);
> +				 "%s/apnv%02x.bin", fw_path, rom_ver);
>  			break;
>  		case QCA_QCA2066:
> -			qca_generate_hsp_nvm_name(config.fwname,
> -				sizeof(config.fwname), ver, rom_ver, boardid);
> +		case QCA_QCA6698:
> +			qca_generate_hsp_nvm_name(soc_type, config.fwname,
> +				sizeof(config.fwname), fw_path, ver, rom_ver, boardid);
>  			break;
>  		case QCA_QCA6390:
>  			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/htnv%02x.bin", rom_ver);
> +				 "%s/htnv%02x.bin", fw_path, rom_ver);
>  			break;
>  		case QCA_WCN6750:
>  			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/msnv%02x.bin", rom_ver);
> +				 "%s/msnv%02x.bin", fw_path, rom_ver);
>  			break;
>  		case QCA_WCN6855:
>  			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/hpnv%02x.bin", rom_ver);
> +				 "%s/hpnv%02x.bin", fw_path, rom_ver);
>  			break;
>  		case QCA_WCN7850:
> -			qca_get_nvm_name_generic(&config, "hmt", rom_ver, boardid);
> +			qca_get_nvm_name_generic(&config, "hmt", fw_path, rom_ver, boardid);
>  			break;
>  
>  		default:
>  			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/nvm_%08x.bin", soc_ver);
> +				 "%s/nvm_%08x.bin", fw_path, soc_ver);
>  		}
>  	}
>  
> @@ -871,6 +955,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  	case QCA_WCN3991:
>  	case QCA_QCA2066:
>  	case QCA_QCA6390:
> +	case QCA_QCA6698:

This wasn't mentioned in the commit message. Please separate unrelated
changes into separate patches.

>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> @@ -909,6 +994,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> +	case QCA_QCA6698:
>  		/* get fw build info */
>  		err = qca_read_fw_build_info(hdev);
>  		if (err < 0)
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index bb5207d7a8c7..baa3f979d017 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -151,21 +151,30 @@ enum qca_btsoc_type {
>  	QCA_WCN3991,
>  	QCA_QCA2066,
>  	QCA_QCA6390,
> +	QCA_QCA6698,
>  	QCA_WCN6750,
>  	QCA_WCN6855,
>  	QCA_WCN7850,
>  };
>  
> +enum qca_product_type {
> +	QCA_MCC = 0,
> +	QCA_CE,
> +	QCA_IOT,
> +	QCA_AUTO,

What is MCC? CE?

> +};
> +
>  #if IS_ENABLED(CONFIG_BT_QCA)
>  
>  int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>  int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  		   enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
> -		   const char *firmware_name);
> +		   const char *firmware_name, uint32_t product_variant);
>  int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
>  			 enum qca_btsoc_type);
>  int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>  int qca_send_pre_shutdown_cmd(struct hci_dev *hdev);
> +const char *qca_get_soc_name(enum qca_btsoc_type soc_type);
>  #else
>  
>  static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 37129e6cb0eb..69fec890eb8c 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -227,6 +227,7 @@ struct qca_serdev {
>  	struct qca_power *bt_power;
>  	u32 init_speed;
>  	u32 oper_speed;
> +	u32 product_variant;
>  	bool bdaddr_property_broken;
>  	const char *firmware_name;
>  };
> @@ -1361,6 +1362,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> +	case QCA_QCA6698:
>  		usleep_range(1000, 10000);
>  		break;
>  
> @@ -1447,6 +1449,7 @@ static int qca_check_speeds(struct hci_uart *hu)
>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> +	case QCA_QCA6698:
>  		if (!qca_get_speed(hu, QCA_INIT_SPEED) &&
>  		    !qca_get_speed(hu, QCA_OPER_SPEED))
>  			return -EINVAL;
> @@ -1489,6 +1492,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
>  		case QCA_WCN6750:
>  		case QCA_WCN6855:
>  		case QCA_WCN7850:
> +		case QCA_QCA6698:
>  			hci_uart_set_flow_control(hu, true);
>  			break;
>  
> @@ -1523,6 +1527,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
>  		case QCA_WCN6750:
>  		case QCA_WCN6855:
>  		case QCA_WCN7850:
> +		case QCA_QCA6698:
>  			hci_uart_set_flow_control(hu, false);
>  			break;
>  
> @@ -1803,6 +1808,7 @@ static int qca_power_on(struct hci_dev *hdev)
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
>  	case QCA_QCA6390:
> +	case QCA_QCA6698:
>  		ret = qca_regulator_init(hu);
>  		break;
>  
> @@ -1858,7 +1864,6 @@ static int qca_setup(struct hci_uart *hu)
>  	int ret;
>  	struct qca_btsoc_version ver;
>  	struct qca_serdev *qcadev;
> -	const char *soc_name;
>  
>  	ret = qca_check_speeds(hu);
>  	if (ret)
> @@ -1873,34 +1878,7 @@ static int qca_setup(struct hci_uart *hu)
>  	 */
>  	set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>  
> -	switch (soc_type) {
> -	case QCA_QCA2066:
> -		soc_name = "qca2066";
> -		break;
> -
> -	case QCA_WCN3988:
> -	case QCA_WCN3990:
> -	case QCA_WCN3991:
> -	case QCA_WCN3998:
> -		soc_name = "wcn399x";
> -		break;
> -
> -	case QCA_WCN6750:
> -		soc_name = "wcn6750";
> -		break;
> -
> -	case QCA_WCN6855:
> -		soc_name = "wcn6855";
> -		break;
> -
> -	case QCA_WCN7850:
> -		soc_name = "wcn7850";
> -		break;
> -
> -	default:
> -		soc_name = "ROME/QCA6390";
> -	}
> -	bt_dev_info(hdev, "setting up %s", soc_name);
> +	bt_dev_info(hdev, "setting up %s", qca_get_soc_name(soc_type));
>  
>  	qca->memdump_state = QCA_MEMDUMP_IDLE;
>  
> @@ -1919,6 +1897,7 @@ static int qca_setup(struct hci_uart *hu)
>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> +	case QCA_QCA6698:
>  		qcadev = serdev_device_get_drvdata(hu->serdev);
>  		if (qcadev->bdaddr_property_broken)
>  			set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
> @@ -1952,6 +1931,7 @@ static int qca_setup(struct hci_uart *hu)
>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> +	case QCA_QCA6698:
>  		break;
>  
>  	default:
> @@ -1963,7 +1943,7 @@ static int qca_setup(struct hci_uart *hu)
>  
>  	/* Setup patch / NVM configurations */
>  	ret = qca_uart_setup(hdev, qca_baudrate, soc_type, ver,
> -			firmware_name);
> +			firmware_name, qcadev->product_variant);
>  	if (!ret) {
>  		clear_bit(QCA_IBS_DISABLED, &qca->flags);
>  		qca_debugfs_init(hdev);
> @@ -2089,6 +2069,20 @@ static const struct qca_device_data qca_soc_data_qca6390 __maybe_unused = {
>  	.num_vregs = 0,
>  };
>  
> +static const struct qca_device_data qca_soc_data_qca6698 __maybe_unused = {
> +	.soc_type = QCA_QCA6698,
> +	.vregs = (struct qca_vreg []) {
> +		{ "vddio", 5000 },
> +		{ "vddbtcxmx", 126000 },
> +		{ "vddrfacmn", 12500 },
> +		{ "vddrfa0p8", 102000 },
> +		{ "vddrfa1p7", 302000 },
> +		{ "vddrfa1p2", 257000 },

No need to describe regulators, use PMU and powerseq.

> +	},
> +	.num_vregs = 6,
> +	.capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> +};

Why can't you use the qca_soc_data_wcn6855?

> +
>  static const struct qca_device_data qca_soc_data_wcn6750 __maybe_unused = {
>  	.soc_type = QCA_WCN6750,
>  	.vregs = (struct qca_vreg []) {
> @@ -2165,7 +2159,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
>  		pwrseq_power_off(power->pwrseq);
>  		set_bit(QCA_BT_OFF, &qca->flags);
>  		return;
> -        }
> +	}

Completely unrelated, cleanups go to a separate patch.

>  
>  	switch (soc_type) {
>  	case QCA_WCN3988:
> @@ -2179,6 +2173,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
>  
>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
> +	case QCA_QCA6698:
>  		gpiod_set_value_cansleep(qcadev->bt_en, 0);
>  		msleep(100);
>  		qca_regulator_disable(qcadev);
> @@ -2313,6 +2308,12 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  					 &qcadev->firmware_name);
>  	device_property_read_u32(&serdev->dev, "max-speed",
>  				 &qcadev->oper_speed);
> +	device_property_read_u32(&serdev->dev, "qcom,product-variant",
> +				 &qcadev->product_variant);
> +
> +	if (qcadev->product_variant != 0)
> +		BT_INFO("QC Product Variant: 0x%08x", qcadev->product_variant);

Don't spam users with useless hex numbers. Printing the sensible string
should be fine though.

> +
>  	if (!qcadev->oper_speed)
>  		BT_DBG("UART will pick default operating speed");
>  
> @@ -2333,6 +2334,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
>  	case QCA_QCA6390:
> +	case QCA_QCA6698:
>  		qcadev->bt_power = devm_kzalloc(&serdev->dev,
>  						sizeof(struct qca_power),
>  						GFP_KERNEL);
> @@ -2346,6 +2348,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  	switch (qcadev->btsoc_type) {
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> +	case QCA_QCA6698:
>  		if (!device_property_present(&serdev->dev, "enable-gpios")) {
>  			/*
>  			 * Backward compatibility with old DT sources. If the
> @@ -2380,7 +2383,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  					       GPIOD_OUT_LOW);
>  		if (IS_ERR(qcadev->bt_en) &&
>  		    (data->soc_type == QCA_WCN6750 ||
> -		     data->soc_type == QCA_WCN6855)) {
> +		     data->soc_type == QCA_WCN6855 ||
> +		     data->soc_type == QCA_QCA6698)) {
>  			dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
>  			return PTR_ERR(qcadev->bt_en);
>  		}
> @@ -2393,7 +2397,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  		if (IS_ERR(qcadev->sw_ctrl) &&
>  		    (data->soc_type == QCA_WCN6750 ||
>  		     data->soc_type == QCA_WCN6855 ||
> -		     data->soc_type == QCA_WCN7850)) {
> +		     data->soc_type == QCA_WCN7850 ||
> +		     data->soc_type == QCA_QCA6698)) {
>  			dev_err(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
>  			return PTR_ERR(qcadev->sw_ctrl);
>  		}
> @@ -2475,6 +2480,7 @@ static void qca_serdev_remove(struct serdev_device *serdev)
>  	case QCA_WCN6750:
>  	case QCA_WCN6855:
>  	case QCA_WCN7850:
> +	case QCA_QCA6698:
>  		if (power->vregs_on)
>  			qca_power_shutdown(&qcadev->serdev_hu);
>  		break;
> @@ -2669,6 +2675,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
>  	{ .compatible = "qcom,qca2066-bt", .data = &qca_soc_data_qca2066},
>  	{ .compatible = "qcom,qca6174-bt" },
>  	{ .compatible = "qcom,qca6390-bt", .data = &qca_soc_data_qca6390},
> +	{ .compatible = "qcom,qca6698-bt", .data = &qca_soc_data_qca6698},
>  	{ .compatible = "qcom,qca9377-bt" },
>  	{ .compatible = "qcom,wcn3988-bt", .data = &qca_soc_data_wcn3988},
>  	{ .compatible = "qcom,wcn3990-bt", .data = &qca_soc_data_wcn3990},
> -- 
> 2.25.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/4] Bluetooth: hci_qca: add qcom,product-variant properties

[neil.armstrong]

On 20/11/2024 10:54, Cheng Jiang wrote:
> Since different products use the same SoC chip, features cannot
> be included in a single patch. Use the qcom,product-variant to
> load the appropriate firmware.
> 
> The qcom,product-variant provides product line information, which
> the driver uses to load firmware from different directories.
> 
> If it's not defined in dts, the default firmware will be loaded.
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>   drivers/bluetooth/btqca.c   | 142 +++++++++++++++++++++++++++++-------
>   drivers/bluetooth/btqca.h   |  11 ++-
>   drivers/bluetooth/hci_qca.c |  73 +++++++++---------
>   3 files changed, 164 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac92242a..0845e5a60412 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -700,8 +700,79 @@ static int qca_check_bdaddr(struct hci_dev *hdev, const struct qca_fw_config *co
>   	return 0;
>   }
>   
> -static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
> -		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
> +
> +const char *qca_get_soc_name(enum qca_btsoc_type soc_type)
> +{
> +	const char *soc_name = "";
> +
> +	switch (soc_type) {
> +	case QCA_QCA2066:
> +		soc_name = "QCA2066";
> +		break;
> +
> +	case QCA_QCA6698:
> +		soc_name = "QCA6698";
> +		break;
> +
> +	case QCA_WCN3988:
> +	case QCA_WCN3990:
> +	case QCA_WCN3991:
> +	case QCA_WCN3998:
> +		soc_name = "WCN399x";
> +		break;
> +
> +	case QCA_WCN6750:
> +		soc_name = "WCN6750";
> +		break;
> +
> +	case QCA_WCN6855:
> +		soc_name = "WCN6855";
> +		break;
> +
> +	case QCA_WCN7850:
> +		soc_name = "WCN7850";
> +		break;
> +
> +	default:
> +		soc_name = "ROME/QCA6390";
> +	}
> +
> +	return soc_name;
> +}
> +EXPORT_SYMBOL_GPL(qca_get_soc_name);
> +
> +static void qca_get_firmware_path(enum qca_btsoc_type soc_type, char *fw_path,
> +		size_t max_size, enum qca_product_type product_type)
> +{
> +	const char *fw_dir = NULL;
> +
> +	switch (product_type) {
> +	case QCA_MCC:
> +		fw_dir = "qca";
> +		break;
> +	case QCA_CE:
> +		fw_dir = "qca/ce";
> +		break;
> +	case QCA_IOT:
> +		fw_dir = "qca/iot";
> +		break;
> +	case QCA_AUTO:
> +		fw_dir = "qca/auto";
> +		break;
> +	default:
> +		fw_dir = "qca";
> +		break;
> +	}
> +
> +	if (product_type == QCA_IOT)
> +		snprintf(fw_path, max_size, "%s/%s", fw_dir, qca_get_soc_name(soc_type));
> +	else
> +		snprintf(fw_path, max_size, "%s", fw_dir);
> +}
> +
> +static void qca_generate_hsp_nvm_name(enum qca_btsoc_type soc_type, char *fwname,
> +		size_t max_size, const char *fw_path, struct qca_btsoc_version ver, u8 rom_ver,
> +		u16 bid)
>   {
>   	const char *variant;
>   
> @@ -712,33 +783,36 @@ static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
>   		variant = "";
>   
>   	if (bid == 0x0)
> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
> +		snprintf(fwname, max_size, "%s/hpnv%02x%s.bin", fw_path, rom_ver, variant);
>   	else
> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
> +		snprintf(fwname, max_size, "%s/hpnv%02x%s.%x", fw_path, rom_ver, variant, bid);
>   }
>   
> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
> +static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg, const char *fw_path,
>   					    const char *stem, u8 rom_ver, u16 bid)
>   {
>   	if (bid == 0x0)
> -		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
> +		snprintf(cfg->fwname, sizeof(cfg->fwname),
> +			 "%s/%snv%02x.bin", fw_path, stem, rom_ver);
>   	else if (bid & 0xff00)
>   		snprintf(cfg->fwname, sizeof(cfg->fwname),
> -			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
> +			 "%s/%snv%02x.b%x", fw_path, stem, rom_ver, bid);
>   	else
>   		snprintf(cfg->fwname, sizeof(cfg->fwname),
> -			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
> +			 "%s/%snv%02x.b%02x", fw_path, stem, rom_ver, bid);
>   }
>   
>   int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   		   enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
> -		   const char *firmware_name)
> +		   const char *firmware_name, uint32_t product_variant)
>   {
>   	struct qca_fw_config config = {};
>   	int err;
>   	u8 rom_ver = 0;
>   	u32 soc_ver;
>   	u16 boardid = 0;
> +	enum qca_product_type product_type;
> +	char fw_path[64] = {0};
>   
>   	bt_dev_dbg(hdev, "QCA setup on UART");
>   
> @@ -759,6 +833,10 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   	if (soc_type == QCA_WCN6750)
>   		qca_send_patch_config_cmd(hdev);
>   
> +	/* Get the f/w path based on product variant */
> +	product_type = (product_variant >> 16) & 0xff;
> +	qca_get_firmware_path(soc_type, fw_path, sizeof(fw_path), product_type);
> +
>   	/* Download rampatch file */
>   	config.type = TLV_TYPE_PATCH;
>   	switch (soc_type) {
> @@ -766,19 +844,23 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   	case QCA_WCN3991:
>   	case QCA_WCN3998:
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/crbtfw%02x.tlv", rom_ver);
> +			 "%s/crbtfw%02x.tlv", fw_path, rom_ver);

Changing firmware path will break existing platforms, please don't, or add fallbacks.

>   		break;
>   	case QCA_WCN3988:
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/apbtfw%02x.tlv", rom_ver);
> +			 "%s/apbtfw%02x.tlv", fw_path, rom_ver);
>   		break;
>   	case QCA_QCA2066:
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hpbtfw%02x.tlv", rom_ver);
> +			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
>   		break;
>   	case QCA_QCA6390:
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/htbtfw%02x.tlv", rom_ver);
> +			 "%s/htbtfw%02x.tlv", fw_path, rom_ver);
> +		break;
> +	case QCA_QCA6698:
> +		snprintf(config.fwname, sizeof(config.fwname),
> +			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
>   		break;
>   	case QCA_WCN6750:
>   		/* Choose mbn file by default.If mbn file is not found
> @@ -786,19 +868,19 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   		 */
>   		config.type = ELF_TYPE_PATCH;
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/msbtfw%02x.mbn", rom_ver);
> +			 "%s/msbtfw%02x.mbn", fw_path, rom_ver);
>   		break;
>   	case QCA_WCN6855:
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hpbtfw%02x.tlv", rom_ver);
> +			 "%s/hpbtfw%02x.tlv", fw_path, rom_ver);
>   		break;
>   	case QCA_WCN7850:
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/hmtbtfw%02x.tlv", rom_ver);
> +			 "%s/hmtbtfw%02x.tlv", fw_path, rom_ver);
>   		break;
>   	default:
>   		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/rampatch_%08x.bin", soc_ver);
> +			 "%s/rampatch_%08x.bin", fw_path, soc_ver);
>   	}
>   
>   	err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
> @@ -810,7 +892,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   	/* Give the controller some time to get ready to receive the NVM */
>   	msleep(10);
>   
> -	if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850)
> +	if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850 ||
> +		soc_type == QCA_QCA6698)
>   		qca_read_fw_board_id(hdev, &boardid);
>   
>   	/* Download NVM configuration */
> @@ -825,39 +908,40 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   		case QCA_WCN3998:
>   			if (le32_to_cpu(ver.soc_id) == QCA_WCN3991_SOC_ID) {
>   				snprintf(config.fwname, sizeof(config.fwname),
> -					 "qca/crnv%02xu.bin", rom_ver);
> +					 "%s/crnv%02xu.bin", fw_path, rom_ver);
>   			} else {
>   				snprintf(config.fwname, sizeof(config.fwname),
> -					 "qca/crnv%02x.bin", rom_ver);
> +					 "%s/crnv%02x.bin", fw_path, rom_ver);
>   			}
>   			break;
>   		case QCA_WCN3988:
>   			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/apnv%02x.bin", rom_ver);
> +				 "%s/apnv%02x.bin", fw_path, rom_ver);
>   			break;
>   		case QCA_QCA2066:
> -			qca_generate_hsp_nvm_name(config.fwname,
> -				sizeof(config.fwname), ver, rom_ver, boardid);
> +		case QCA_QCA6698:
> +			qca_generate_hsp_nvm_name(soc_type, config.fwname,
> +				sizeof(config.fwname), fw_path, ver, rom_ver, boardid);
>   			break;
>   		case QCA_QCA6390:
>   			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/htnv%02x.bin", rom_ver);
> +				 "%s/htnv%02x.bin", fw_path, rom_ver);
>   			break;
>   		case QCA_WCN6750:
>   			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/msnv%02x.bin", rom_ver);
> +				 "%s/msnv%02x.bin", fw_path, rom_ver);
>   			break;
>   		case QCA_WCN6855:
>   			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/hpnv%02x.bin", rom_ver);
> +				 "%s/hpnv%02x.bin", fw_path, rom_ver);
>   			break;
>   		case QCA_WCN7850:
> -			qca_get_nvm_name_generic(&config, "hmt", rom_ver, boardid);
> +			qca_get_nvm_name_generic(&config, "hmt", fw_path, rom_ver, boardid);
>   			break;
>   
>   		default:
>   			snprintf(config.fwname, sizeof(config.fwname),
> -				 "qca/nvm_%08x.bin", soc_ver);
> +				 "%s/nvm_%08x.bin", fw_path, soc_ver);
>   		}
>   	}
>   
> @@ -871,6 +955,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   	case QCA_WCN3991:
>   	case QCA_QCA2066:
>   	case QCA_QCA6390:
> +	case QCA_QCA6698:
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> @@ -909,6 +994,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> +	case QCA_QCA6698:
>   		/* get fw build info */
>   		err = qca_read_fw_build_info(hdev);
>   		if (err < 0)
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index bb5207d7a8c7..baa3f979d017 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -151,21 +151,30 @@ enum qca_btsoc_type {
>   	QCA_WCN3991,
>   	QCA_QCA2066,
>   	QCA_QCA6390,
> +	QCA_QCA6698,
>   	QCA_WCN6750,
>   	QCA_WCN6855,
>   	QCA_WCN7850,
>   };
>   
> +enum qca_product_type {
> +	QCA_MCC = 0,
> +	QCA_CE,
> +	QCA_IOT,
> +	QCA_AUTO,
> +};
> +
>   #if IS_ENABLED(CONFIG_BT_QCA)
>   
>   int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>   int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   		   enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
> -		   const char *firmware_name);
> +		   const char *firmware_name, uint32_t product_variant);
>   int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
>   			 enum qca_btsoc_type);
>   int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>   int qca_send_pre_shutdown_cmd(struct hci_dev *hdev);
> +const char *qca_get_soc_name(enum qca_btsoc_type soc_type);
>   #else
>   
>   static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 37129e6cb0eb..69fec890eb8c 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -227,6 +227,7 @@ struct qca_serdev {
>   	struct qca_power *bt_power;
>   	u32 init_speed;
>   	u32 oper_speed;
> +	u32 product_variant;
>   	bool bdaddr_property_broken;
>   	const char *firmware_name;
>   };
> @@ -1361,6 +1362,7 @@ static int qca_set_baudrate(struct hci_dev *hdev, uint8_t baudrate)
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> +	case QCA_QCA6698:
>   		usleep_range(1000, 10000);
>   		break;
>   
> @@ -1447,6 +1449,7 @@ static int qca_check_speeds(struct hci_uart *hu)
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> +	case QCA_QCA6698:
>   		if (!qca_get_speed(hu, QCA_INIT_SPEED) &&
>   		    !qca_get_speed(hu, QCA_OPER_SPEED))
>   			return -EINVAL;
> @@ -1489,6 +1492,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
>   		case QCA_WCN6750:
>   		case QCA_WCN6855:
>   		case QCA_WCN7850:
> +		case QCA_QCA6698:
>   			hci_uart_set_flow_control(hu, true);
>   			break;
>   
> @@ -1523,6 +1527,7 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)
>   		case QCA_WCN6750:
>   		case QCA_WCN6855:
>   		case QCA_WCN7850:
> +		case QCA_QCA6698:
>   			hci_uart_set_flow_control(hu, false);
>   			break;
>   
> @@ -1803,6 +1808,7 @@ static int qca_power_on(struct hci_dev *hdev)
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
>   	case QCA_QCA6390:
> +	case QCA_QCA6698:
>   		ret = qca_regulator_init(hu);
>   		break;
>   
> @@ -1858,7 +1864,6 @@ static int qca_setup(struct hci_uart *hu)
>   	int ret;
>   	struct qca_btsoc_version ver;
>   	struct qca_serdev *qcadev;
> -	const char *soc_name;
>   
>   	ret = qca_check_speeds(hu);
>   	if (ret)
> @@ -1873,34 +1878,7 @@ static int qca_setup(struct hci_uart *hu)
>   	 */
>   	set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>   
> -	switch (soc_type) {
> -	case QCA_QCA2066:
> -		soc_name = "qca2066";
> -		break;
> -
> -	case QCA_WCN3988:
> -	case QCA_WCN3990:
> -	case QCA_WCN3991:
> -	case QCA_WCN3998:
> -		soc_name = "wcn399x";
> -		break;
> -
> -	case QCA_WCN6750:
> -		soc_name = "wcn6750";
> -		break;
> -
> -	case QCA_WCN6855:
> -		soc_name = "wcn6855";
> -		break;
> -
> -	case QCA_WCN7850:
> -		soc_name = "wcn7850";
> -		break;
> -
> -	default:
> -		soc_name = "ROME/QCA6390";
> -	}
> -	bt_dev_info(hdev, "setting up %s", soc_name);
> +	bt_dev_info(hdev, "setting up %s", qca_get_soc_name(soc_type));

Move this into a new function in a separate patch

>   
>   	qca->memdump_state = QCA_MEMDUMP_IDLE;
>   
> @@ -1919,6 +1897,7 @@ static int qca_setup(struct hci_uart *hu)
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> +	case QCA_QCA6698:
>   		qcadev = serdev_device_get_drvdata(hu->serdev);
>   		if (qcadev->bdaddr_property_broken)
>   			set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
> @@ -1952,6 +1931,7 @@ static int qca_setup(struct hci_uart *hu)
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> +	case QCA_QCA6698:
>   		break;
>   
>   	default:
> @@ -1963,7 +1943,7 @@ static int qca_setup(struct hci_uart *hu)
>   
>   	/* Setup patch / NVM configurations */
>   	ret = qca_uart_setup(hdev, qca_baudrate, soc_type, ver,
> -			firmware_name);
> +			firmware_name, qcadev->product_variant);

Add product variant support separately from adding QCA6698

>   	if (!ret) {
>   		clear_bit(QCA_IBS_DISABLED, &qca->flags);
>   		qca_debugfs_init(hdev);
> @@ -2089,6 +2069,20 @@ static const struct qca_device_data qca_soc_data_qca6390 __maybe_unused = {
>   	.num_vregs = 0,
>   };
>   
> +static const struct qca_device_data qca_soc_data_qca6698 __maybe_unused = {
> +	.soc_type = QCA_QCA6698,
> +	.vregs = (struct qca_vreg []) {
> +		{ "vddio", 5000 },
> +		{ "vddbtcxmx", 126000 },
> +		{ "vddrfacmn", 12500 },
> +		{ "vddrfa0p8", 102000 },
> +		{ "vddrfa1p7", 302000 },
> +		{ "vddrfa1p2", 257000 },
> +	},
> +	.num_vregs = 6,
> +	.capabilities = QCA_CAP_WIDEBAND_SPEECH | QCA_CAP_VALID_LE_STATES,
> +};
> +
>   static const struct qca_device_data qca_soc_data_wcn6750 __maybe_unused = {
>   	.soc_type = QCA_WCN6750,
>   	.vregs = (struct qca_vreg []) {
> @@ -2165,7 +2159,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
>   		pwrseq_power_off(power->pwrseq);
>   		set_bit(QCA_BT_OFF, &qca->flags);
>   		return;
> -        }
> +	}

This is cleanup

>   
>   	switch (soc_type) {
>   	case QCA_WCN3988:
> @@ -2179,6 +2173,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
>   
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
> +	case QCA_QCA6698:
>   		gpiod_set_value_cansleep(qcadev->bt_en, 0);
>   		msleep(100);
>   		qca_regulator_disable(qcadev);
> @@ -2313,6 +2308,12 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>   					 &qcadev->firmware_name);
>   	device_property_read_u32(&serdev->dev, "max-speed",
>   				 &qcadev->oper_speed);
> +	device_property_read_u32(&serdev->dev, "qcom,product-variant",
> +				 &qcadev->product_variant);
> +
> +	if (qcadev->product_variant != 0)
> +		BT_INFO("QC Product Variant: 0x%08x", qcadev->product_variant);
> +
>   	if (!qcadev->oper_speed)
>   		BT_DBG("UART will pick default operating speed");
>   
> @@ -2333,6 +2334,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
>   	case QCA_QCA6390:
> +	case QCA_QCA6698:
>   		qcadev->bt_power = devm_kzalloc(&serdev->dev,
>   						sizeof(struct qca_power),
>   						GFP_KERNEL);
> @@ -2346,6 +2348,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>   	switch (qcadev->btsoc_type) {
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> +	case QCA_QCA6698:
>   		if (!device_property_present(&serdev->dev, "enable-gpios")) {
>   			/*
>   			 * Backward compatibility with old DT sources. If the
> @@ -2380,7 +2383,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>   					       GPIOD_OUT_LOW);
>   		if (IS_ERR(qcadev->bt_en) &&
>   		    (data->soc_type == QCA_WCN6750 ||
> -		     data->soc_type == QCA_WCN6855)) {
> +		     data->soc_type == QCA_WCN6855 ||
> +		     data->soc_type == QCA_QCA6698)) {
>   			dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n");
>   			return PTR_ERR(qcadev->bt_en);
>   		}
> @@ -2393,7 +2397,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>   		if (IS_ERR(qcadev->sw_ctrl) &&
>   		    (data->soc_type == QCA_WCN6750 ||
>   		     data->soc_type == QCA_WCN6855 ||
> -		     data->soc_type == QCA_WCN7850)) {
> +		     data->soc_type == QCA_WCN7850 ||
> +		     data->soc_type == QCA_QCA6698)) {
>   			dev_err(&serdev->dev, "failed to acquire SW_CTRL gpio\n");
>   			return PTR_ERR(qcadev->sw_ctrl);
>   		}
> @@ -2475,6 +2480,7 @@ static void qca_serdev_remove(struct serdev_device *serdev)
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> +	case QCA_QCA6698:
>   		if (power->vregs_on)
>   			qca_power_shutdown(&qcadev->serdev_hu);
>   		break;
> @@ -2669,6 +2675,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
>   	{ .compatible = "qcom,qca2066-bt", .data = &qca_soc_data_qca2066},
>   	{ .compatible = "qcom,qca6174-bt" },
>   	{ .compatible = "qcom,qca6390-bt", .data = &qca_soc_data_qca6390},
> +	{ .compatible = "qcom,qca6698-bt", .data = &qca_soc_data_qca6698},
>   	{ .compatible = "qcom,qca9377-bt" },
>   	{ .compatible = "qcom,wcn3988-bt", .data = &qca_soc_data_wcn3988},
>   	{ .compatible = "qcom,wcn3990-bt", .data = &qca_soc_data_wcn3990},

This patch is too long anf has multiple different changes merged together,
please split into multiple small pieces that can be reviewed easier,

Thanks,
Neil