[PATCH v5 0/4] Expand firmware-name property to load specific

[PATCH v5 0/4] Expand firmware-name property to load specific

[Cheng Jiang]

Expand the firmware-name property to specify the names of NVM and
rampatch firmware to load.

This update will support loading specific firmware (nvm and rampatch)
for certain chips, like the QCA6698 Bluetooth chip, which shares the
same IP core as the WCN6855 but has different RF components and RAM
sizes, requiring new firmware files.

Different connectivity boards may be attached to the same platform. For
example, QCA6698-based boards can support either a two-antenna or
three-antenna solution, both of which work on the sa8775p-ride platform.
Due to differences in connectivity boards and variations in RF
performance from different foundries, different NVM configurations are
used based on the board ID.

So In firmware-name, if the NVM file has an extension, the NVM file will
be used. Otherwise, the system will first try the .bNN (board ID) file,
and if that fails, it will fall back to the .bin file.

Possible configurations:
firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21.bin";

---
v5:
  1. Update the dt-bindings decription.
  2. Extract the have-suffix check code to a helper function.
  3. Merge three generate nvm name functions to a signle function.

v4:
  1. Split nvm and rampatch changes to 2 commits.
  2. Fix the dt_binding_check error.
  3. Change the qca_get_alt_nvm_path return type from int to bool.
  4. Fix the nvm name suffix check error when path has '.'.
  5. Optimize the nvm file name generation function.

v3:
  1. Expand firmware-name property to specify the nvm and rampatch to
  load.
  2. Change the driver to support two items in firmware-name and choose
  the NVM file according to board id if there is no extension in NVM
  file.
  3. Update the dts file to idendify the firmware for QCA6698.
---

Cheng Jiang (4):
  dt-bindings: net: bluetooth: qca: Expand firmware-name property
  Bluetooth: qca: Update firmware-name to support board specific nvm
  Bluetooth: qca: Expand firmware-name to load specific rampatch
  arm64: dts: qcom: sa8775p-ride: Add firmware-name in BT node

 .../net/bluetooth/qualcomm-bluetooth.yaml     |   5 +-
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi    |   1 +
 drivers/bluetooth/btqca.c                     | 194 ++++++++++++------
 drivers/bluetooth/btqca.h                     |   5 +-
 drivers/bluetooth/hci_qca.c                   |  22 +-
 5 files changed, 152 insertions(+), 75 deletions(-)


base-commit: 3e42dc9229c5950e84b1ed705f94ed75ed208228
-- 
2.25.1

[PATCH v5 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Cheng Jiang]

Expand the firmware-name property to specify the names of NVM and
rampatch firmware to load. This update will support loading specific
firmware (nvm and rampatch) for certain chips, like the QCA6698
Bluetooth chip, which shares the same IP core as the WCN6855 but has
different RF components and RAM sizes, requiring new firmware files.

We might use different connectivity boards on the same platform. For
example, QCA6698-based boards can support either a two-antenna or
three-antenna solution, both of which work on the sa8775p-ride platform.
Due to differences in connectivity boards and variations in RF
performance from different foundries, different NVM configurations are
used based on the board ID.

So In firmware-name, if the NVM file has an extension, the NVM file will
be used. Otherwise, the system will first try the .bNN (board ID) file,
and if that fails, it will fall back to the .bin file.

Possible configurations:
firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21.bin";

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

diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
index 7bb68311c..a6bc0b18b 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
@@ -101,7 +101,10 @@ properties:
   max-speed: true
 
   firmware-name:
-    description: specify the name of nvm firmware to load
+    minItems: 1
+    items:
+      - description: specify the name of nvm firmware to load
+      - description: specify the name of rampatch firmware to load
 
   local-bd-address: true
 
-- 
2.25.1

[PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Cheng Jiang]

Different connectivity boards may be attached to the same platform. For
example, QCA6698-based boards can support either a two-antenna or
three-antenna solution, both of which work on the sa8775p-ride platform.
Due to differences in connectivity boards and variations in RF
performance from different foundries, different NVM configurations are
used based on the board ID.

Therefore, in the firmware-name property, if the NVM file has an
extension, the NVM file will be used. Otherwise, the system will first
try the .bNN (board ID) file, and if that fails, it will fall back to
the .bin file.

Possible configurations:
firmware-name = "QCA6698/hpnv21";
firmware-name = "QCA6698/hpnv21.bin";

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 drivers/bluetooth/btqca.c | 112 ++++++++++++++++++++++++++++----------
 1 file changed, 84 insertions(+), 28 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index dfbbac922..4842f4335 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -272,6 +272,38 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
 }
 EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
 
+static bool qca_filename_has_extension(const char *filename)
+{
+	const char *suffix;
+
+	suffix = strrchr(filename, '.');
+	if (suffix && suffix != filename && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL)
+		return true;
+	else
+		return false;
+}
+
+static bool qca_get_alt_nvm_file(char *filename, size_t max_size)
+{
+	char fwname[64];
+	const char *suffix;
+
+	/* nvm file name has an extension, replace with .bin */
+	if (qca_filename_has_extension(filename)) {
+		suffix = strrchr(filename, '.');
+		strscpy(fwname, filename, suffix - filename + 1);
+		snprintf(fwname + (suffix - filename),
+		       sizeof(fwname) - (suffix - filename), ".bin");
+		/* If nvm file is already the default one, return false to skip the retry. */
+		if (strcmp(fwname, filename) == 0)
+			return false;
+
+		snprintf(filename, max_size, "%s", fwname);
+		return true;
+	}
+	return false;
+}
+
 static int qca_tlv_check_data(struct hci_dev *hdev,
 			       struct qca_fw_config *config,
 			       u8 *fw_data, size_t fw_size,
@@ -564,6 +596,19 @@ static int qca_download_firmware(struct hci_dev *hdev,
 					   config->fwname, ret);
 				return ret;
 			}
+		}
+		/* For nvm, if desired nvm file is not present and it's not the
+		 * default nvm file(ends with .bin), try to load the default nvm.
+		 */
+		else if (config->type == TLV_TYPE_NVM &&
+			 qca_get_alt_nvm_file(config->fwname, sizeof(config->fwname))) {
+			bt_dev_info(hdev, "QCA Downloading %s", config->fwname);
+			ret = request_firmware(&fw, config->fwname, &hdev->dev);
+			if (ret) {
+				bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
+					   config->fwname, ret);
+				return ret;
+			}
 		} else {
 			bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
 				   config->fwname, ret);
@@ -700,34 +745,38 @@ 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,
+static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
+		const char *stem, enum qca_btsoc_type soc_type,
 		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
 {
 	const char *variant;
+	const char *prefix;
 
-	/* hsp gf chip */
-	if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
-		variant = "g";
-	else
-		variant = "";
+	/* Set the defalut value to variant and prefixt */
+	variant = "";
+	prefix = "b";
 
-	if (bid == 0x0)
-		snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
-	else
-		snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
-}
+	if (soc_type == QCA_QCA2066)
+		prefix = "";
 
-static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
-					    const char *stem, u8 rom_ver, u16 bid)
-{
-	if (bid == 0x0)
-		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
-	else if (bid & 0xff00)
-		snprintf(cfg->fwname, sizeof(cfg->fwname),
-			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
-	else
-		snprintf(cfg->fwname, sizeof(cfg->fwname),
-			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
+	if (soc_type == QCA_WCN6855 || soc_type == QCA_QCA2066) {
+		/* hsp gf chip */
+		if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
+			variant = "g";
+	}
+
+	if (rom_ver != 0) {
+		if (bid == 0x0 || bid == 0xffff)
+			snprintf(fwname, max_size, "qca/%s%02x%s.bin", stem, rom_ver, variant);
+		else
+			snprintf(fwname, max_size, "qca/%s%02x%s.%s%02x", stem, rom_ver,
+						variant, prefix, bid);
+	} else {
+		if (bid == 0x0 || bid == 0xffff)
+			snprintf(fwname, max_size, "qca/%s%s.bin", stem, variant);
+		else
+			snprintf(fwname, max_size, "qca/%s%s.%s%02x", stem, variant, prefix, bid);
+	}
 }
 
 int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
@@ -816,8 +865,14 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	/* Download NVM configuration */
 	config.type = TLV_TYPE_NVM;
 	if (firmware_name) {
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/%s", firmware_name);
+		/* The firmware name has an extension, use it directly */
+		if (qca_filename_has_extension(firmware_name)) {
+			snprintf(config.fwname, sizeof(config.fwname), "qca/%s", firmware_name);
+		} else {
+			qca_read_fw_board_id(hdev, &boardid);
+			qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
+				 firmware_name, soc_type, ver, 0, boardid);
+		}
 	} else {
 		switch (soc_type) {
 		case QCA_WCN3990:
@@ -836,8 +891,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 				 "qca/apnv%02x.bin", rom_ver);
 			break;
 		case QCA_QCA2066:
-			qca_generate_hsp_nvm_name(config.fwname,
-				sizeof(config.fwname), ver, rom_ver, boardid);
+			qca_get_nvm_name_by_board(config.fwname,
+				sizeof(config.fwname), "hpnv", soc_type, ver,
+				rom_ver, boardid);
 			break;
 		case QCA_QCA6390:
 			snprintf(config.fwname, sizeof(config.fwname),
@@ -852,9 +908,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 				 "qca/hpnv%02x.bin", rom_ver);
 			break;
 		case QCA_WCN7850:
-			qca_get_nvm_name_generic(&config, "hmt", rom_ver, boardid);
+			qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
+				 "hmtnv", soc_type, ver, rom_ver, boardid);
 			break;
-
 		default:
 			snprintf(config.fwname, sizeof(config.fwname),
 				 "qca/nvm_%08x.bin", soc_ver);
-- 
2.25.1

[PATCH v5 3/4] Bluetooth: qca: Expand firmware-name to load specific rampatch

[Cheng Jiang]

The firmware-name property has been expanded to specify the names of NVM
and rampatch firmware for certain chips, such as the QCA6698 Bluetooth
chip. Although it shares the same IP core as the WCN6855, the QCA6698
has different RF components and RAM sizes, necessitating new firmware
files. This change allows for the configuration of NVM and rampatch in
DT.

Possible configurations:
firmware-name = QCA6698/hpnv21.bin, QCA6698/hpbtfw21.tlv;
firmware-name = QCA6698/hpnv21, QCA6698/hpbtfw21.tlv;

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 drivers/bluetooth/btqca.c   | 82 +++++++++++++++++++------------------
 drivers/bluetooth/btqca.h   |  5 ++-
 drivers/bluetooth/hci_qca.c | 22 +++++++---
 3 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 4842f4335..709e872bc 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -781,7 +781,7 @@ static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
 
 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, const char *rampatch_name)
 {
 	struct qca_fw_config config = {};
 	int err;
@@ -810,44 +810,48 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 
 	/* Download rampatch file */
 	config.type = TLV_TYPE_PATCH;
-	switch (soc_type) {
-	case QCA_WCN3990:
-	case QCA_WCN3991:
-	case QCA_WCN3998:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/crbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_WCN3988:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/apbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_QCA2066:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hpbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_QCA6390:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/htbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_WCN6750:
-		/* Choose mbn file by default.If mbn file is not found
-		 * then choose tlv file
-		 */
-		config.type = ELF_TYPE_PATCH;
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/msbtfw%02x.mbn", rom_ver);
-		break;
-	case QCA_WCN6855:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hpbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_WCN7850:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hmtbtfw%02x.tlv", rom_ver);
-		break;
-	default:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/rampatch_%08x.bin", soc_ver);
+	if (rampatch_name) {
+		snprintf(config.fwname, sizeof(config.fwname), "qca/%s", rampatch_name);
+	} else {
+		switch (soc_type) {
+		case QCA_WCN3990:
+		case QCA_WCN3991:
+		case QCA_WCN3998:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/crbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_WCN3988:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/apbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_QCA2066:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/hpbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_QCA6390:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/htbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_WCN6750:
+			/* Choose mbn file by default.If mbn file is not found
+			 * then choose tlv file
+			 */
+			config.type = ELF_TYPE_PATCH;
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/msbtfw%02x.mbn", rom_ver);
+			break;
+		case QCA_WCN6855:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/hpbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_WCN7850:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/hmtbtfw%02x.tlv", rom_ver);
+			break;
+		default:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/rampatch_%08x.bin", soc_ver);
+		}
 	}
 
 	err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index bb5207d7a..9d28c8800 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -161,7 +161,7 @@ enum qca_btsoc_type {
 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, const char *rampatch_name);
 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);
@@ -176,7 +176,8 @@ static inline int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdad
 static inline 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,
+				 const char *rampatch_name)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 37129e6cb..5d75087cc 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -228,7 +228,7 @@ struct qca_serdev {
 	u32 init_speed;
 	u32 oper_speed;
 	bool bdaddr_property_broken;
-	const char *firmware_name;
+	const char *firmware_name[2];
 };
 
 static int qca_regulator_enable(struct qca_serdev *qcadev);
@@ -258,7 +258,18 @@ static const char *qca_get_firmware_name(struct hci_uart *hu)
 	if (hu->serdev) {
 		struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
 
-		return qsd->firmware_name;
+		return qsd->firmware_name[0];
+	} else {
+		return NULL;
+	}
+}
+
+static const char *qca_get_rampatch_name(struct hci_uart *hu)
+{
+	if (hu->serdev) {
+		struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
+
+		return qsd->firmware_name[1];
 	} else {
 		return NULL;
 	}
@@ -1855,6 +1866,7 @@ static int qca_setup(struct hci_uart *hu)
 	unsigned int retries = 0;
 	enum qca_btsoc_type soc_type = qca_soc_type(hu);
 	const char *firmware_name = qca_get_firmware_name(hu);
+	const char *rampatch_name = qca_get_rampatch_name(hu);
 	int ret;
 	struct qca_btsoc_version ver;
 	struct qca_serdev *qcadev;
@@ -1963,7 +1975,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, rampatch_name);
 	if (!ret) {
 		clear_bit(QCA_IBS_DISABLED, &qca->flags);
 		qca_debugfs_init(hdev);
@@ -2309,8 +2321,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 	qcadev->serdev_hu.serdev = serdev;
 	data = device_get_match_data(&serdev->dev);
 	serdev_device_set_drvdata(serdev, qcadev);
-	device_property_read_string(&serdev->dev, "firmware-name",
-					 &qcadev->firmware_name);
+	device_property_read_string_array(&serdev->dev, "firmware-name",
+					 qcadev->firmware_name, ARRAY_SIZE(qcadev->firmware_name));
 	device_property_read_u32(&serdev->dev, "max-speed",
 				 &qcadev->oper_speed);
 	if (!qcadev->oper_speed)
-- 
2.25.1

[PATCH v5 4/4] arm64: dts: qcom: sa8775p-ride: Add firmware-name in BT node

[Cheng Jiang]

The sa8775p-ride platform uses the QCA6698 Bluetooth chip. While the
QCA6698 shares the same IP core as the WCN6855, it has different RF
components and RAM sizes, requiring new firmware files. Use the
firmware-name property to specify the NVM and rampatch firmware to load.

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

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
index 3fc62e123..e7fe53d95 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -857,6 +857,7 @@ &uart17 {
 
 	bluetooth {
 		compatible = "qcom,wcn6855-bt";
+		firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
 
 		vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
 		vddaon-supply = <&vreg_pmu_aon_0p59>;
-- 
2.25.1

Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Konrad Dybcio]

On 12.12.2024 4:02 PM, Cheng Jiang wrote:
> Different connectivity boards may be attached to the same platform. For
> example, QCA6698-based boards can support either a two-antenna or
> three-antenna solution, both of which work on the sa8775p-ride platform.
> Due to differences in connectivity boards and variations in RF
> performance from different foundries, different NVM configurations are
> used based on the board ID.
> 
> Therefore, in the firmware-name property, if the NVM file has an
> extension, the NVM file will be used. Otherwise, the system will first
> try the .bNN (board ID) file, and if that fails, it will fall back to
> the .bin file.
> 
> Possible configurations:
> firmware-name = "QCA6698/hpnv21";
> firmware-name = "QCA6698/hpnv21.bin";

I think we should agree on one and then do some magic to look up
the other variants.

> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  drivers/bluetooth/btqca.c | 112 ++++++++++++++++++++++++++++----------
>  1 file changed, 84 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac922..4842f4335 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -272,6 +272,38 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
>  }
>  EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
>  
> +static bool qca_filename_has_extension(const char *filename)
> +{
> +	const char *suffix;
> +
> +	suffix = strrchr(filename, '.');
> +	if (suffix && suffix != filename && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL)
> +		return true;
> +	else
> +		return false;

How about:

const char *suffix = strrchr(filename, '.');

/* File extensions require a dot, but not as the last character */
if (!suffix || *(suffix + 1) == NULL)
	return false;

/* Avoid matching directories with names that look like files with extensions */
return !(suffix, '/');


> +		}
> +		/* For nvm, if desired nvm file is not present and it's not the
> +		 * default nvm file(ends with .bin), try to load the default nvm.

nvm appears 4 times in two lines, how about:

/*
 * If the board-specific file is missing, try loading the default
 * one, unless that was attempted already
 */

But, even more importantly:

a) do we want to load the "incorrect" file?
b) why would we want to specify the .bin file if it's the default anyway?

> +		 */
> +		else if (config->type == TLV_TYPE_NVM &&
> +			 qca_get_alt_nvm_file(config->fwname, sizeof(config->fwname))) {
> +			bt_dev_info(hdev, "QCA Downloading %s", config->fwname);
> +			ret = request_firmware(&fw, config->fwname, &hdev->dev);
> +			if (ret) {
> +				bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
> +					   config->fwname, ret);
> +				return ret;
> +			}
>  		} else {
>  			bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
>  				   config->fwname, ret);
> @@ -700,34 +745,38 @@ 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,
> +static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
> +		const char *stem, enum qca_btsoc_type soc_type,
>  		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
>  {
>  	const char *variant;
> +	const char *prefix;
>  
> -	/* hsp gf chip */
> -	if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
> -		variant = "g";
> -	else
> -		variant = "";
> +	/* Set the defalut value to variant and prefixt */

typos: default, prefix

> +	variant = "";
> +	prefix = "b";
>  
> -	if (bid == 0x0)
> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
> -	else
> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
> -}
> +	if (soc_type == QCA_QCA2066)
> +		prefix = "";
>  
> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
> -					    const char *stem, u8 rom_ver, u16 bid)
> -{
> -	if (bid == 0x0)
> -		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
> -	else if (bid & 0xff00)
> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
> -			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
> -	else
> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
> -			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
> +	if (soc_type == QCA_WCN6855 || soc_type == QCA_QCA2066) {
> +		/* hsp gf chip */

This is a good opportunity to explain what that means

Konrad

Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Cheng Jiang (IOE)]

Hi Konrad,

On 12/13/2024 8:17 AM, Konrad Dybcio wrote:
> On 12.12.2024 4:02 PM, Cheng Jiang wrote:
>> Different connectivity boards may be attached to the same platform. For
>> example, QCA6698-based boards can support either a two-antenna or
>> three-antenna solution, both of which work on the sa8775p-ride platform.
>> Due to differences in connectivity boards and variations in RF
>> performance from different foundries, different NVM configurations are
>> used based on the board ID.
>>
>> Therefore, in the firmware-name property, if the NVM file has an
>> extension, the NVM file will be used. Otherwise, the system will first
>> try the .bNN (board ID) file, and if that fails, it will fall back to
>> the .bin file.
>>
>> Possible configurations:
>> firmware-name = "QCA6698/hpnv21";
>> firmware-name = "QCA6698/hpnv21.bin";
> 
> I think we should agree on one and then do some magic to look up
> the other variants.
> 
These two possible configurations in DT to specific the NVM file. 

In the existing driver implementation, the firmware-name will specify a 
exactly nvm file. Driver will loaded it directory. But on some platform, 
we may attach different board, then need load the board-specific nvm. 
 
We assume if the nvm file has an extension, user should know the exactly 
connectivity board to attach to the platform, then the exactly nvm file 
will be loaded. This keeps back compatible.
 
If user think different connectivity boards will be attached to the 
platform, the nvm should depends on the board id. Then just leave the 
extension empty. Driver will append the board id info as extension. 

>>
>> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
>> ---
>>  drivers/bluetooth/btqca.c | 112 ++++++++++++++++++++++++++++----------
>>  1 file changed, 84 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
>> index dfbbac922..4842f4335 100644
>> --- a/drivers/bluetooth/btqca.c
>> +++ b/drivers/bluetooth/btqca.c
>> @@ -272,6 +272,38 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
>>  }
>>  EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
>>  
>> +static bool qca_filename_has_extension(const char *filename)
>> +{
>> +	const char *suffix;
>> +
>> +	suffix = strrchr(filename, '.');
>> +	if (suffix && suffix != filename && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL)
>> +		return true;
>> +	else
>> +		return false;
> 
> How about:
> 
> const char *suffix = strrchr(filename, '.');
> 
> /* File extensions require a dot, but not as the last character */
> if (!suffix || *(suffix + 1) == NULL)
> 	return false;
> 
> /* Avoid matching directories with names that look like files with extensions */
> return !(suffix, '/');
> 
> 
Ack. Thanks!
>> +		}
>> +		/* For nvm, if desired nvm file is not present and it's not the
>> +		 * default nvm file(ends with .bin), try to load the default nvm.
> 
> nvm appears 4 times in two lines, how about:
> 
> /*
>  * If the board-specific file is missing, try loading the default
>  * one, unless that was attempted already
>  */
> 
> But, even more importantly:
> 
> a) do we want to load the "incorrect" file?
Normally, there is a default NVM file ending with .bin, which is suitable 
for most boards. However, some boards have different configurations that 
require a specific NVM. If a board-specific NVM is not found, a default 
NVM is preferred over not loading any NVM.

> b) why would we want to specify the .bin file if it's the default anyway?
The default NVM directory is the root of qca. The 'firmware-name' property 
can specify an NVM file in another directory. This can be either a default 
NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'.

> 
>> +		 */
>> +		else if (config->type == TLV_TYPE_NVM &&
>> +			 qca_get_alt_nvm_file(config->fwname, sizeof(config->fwname))) {
>> +			bt_dev_info(hdev, "QCA Downloading %s", config->fwname);
>> +			ret = request_firmware(&fw, config->fwname, &hdev->dev);
>> +			if (ret) {
>> +				bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
>> +					   config->fwname, ret);
>> +				return ret;
>> +			}
>>  		} else {
>>  			bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
>>  				   config->fwname, ret);
>> @@ -700,34 +745,38 @@ 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,
>> +static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
>> +		const char *stem, enum qca_btsoc_type soc_type,
>>  		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
>>  {
>>  	const char *variant;
>> +	const char *prefix;
>>  
>> -	/* hsp gf chip */
>> -	if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
>> -		variant = "g";
>> -	else
>> -		variant = "";
>> +	/* Set the defalut value to variant and prefixt */
> 
> typos: default, prefix
> 
Ack.
>> +	variant = "";
>> +	prefix = "b";
>>  
>> -	if (bid == 0x0)
>> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
>> -	else
>> -		snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
>> -}
>> +	if (soc_type == QCA_QCA2066)
>> +		prefix = "";
>>  
>> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
>> -					    const char *stem, u8 rom_ver, u16 bid)
>> -{
>> -	if (bid == 0x0)
>> -		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
>> -	else if (bid & 0xff00)
>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>> -			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
>> -	else
>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>> -			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
>> +	if (soc_type == QCA_WCN6855 || soc_type == QCA_QCA2066) {
>> +		/* hsp gf chip */
> 
> This is a good opportunity to explain what that means
> 
Ack. This means the chip is produced by GlobalFoundries.

> Konrad

Re: [PATCH v5 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Krzysztof Kozlowski]

On Thu, Dec 12, 2024 at 11:02:29PM +0800, Cheng Jiang wrote:
> Expand the firmware-name property to specify the names of NVM and
> rampatch firmware to load. This update will support loading specific
> firmware (nvm and rampatch) for certain chips, like the QCA6698
> Bluetooth chip, which shares the same IP core as the WCN6855 but has
> different RF components and RAM sizes, requiring new firmware files.
> 
> We might use different connectivity boards on the same platform. For
> example, QCA6698-based boards can support either a two-antenna or
> three-antenna solution, both of which work on the sa8775p-ride platform.
> Due to differences in connectivity boards and variations in RF
> performance from different foundries, different NVM configurations are
> used based on the board ID.
> 
> So In firmware-name, if the NVM file has an extension, the NVM file will
> be used. Otherwise, the system will first try the .bNN (board ID) file,
> and if that fails, it will fall back to the .bin file.
> 
> Possible configurations:
> firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21.bin";
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  .../bindings/net/bluetooth/qualcomm-bluetooth.yaml           | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v5 4/4] arm64: dts: qcom: sa8775p-ride: Add firmware-name in BT node

[Krzysztof Kozlowski]

On Thu, Dec 12, 2024 at 11:02:32PM +0800, Cheng Jiang wrote:
> The sa8775p-ride platform uses the QCA6698 Bluetooth chip. While the
> QCA6698 shares the same IP core as the WCN6855, it has different RF
> components and RAM sizes, requiring new firmware files. Use the
> firmware-name property to specify the NVM and rampatch firmware to load.
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 1 +
>  1 file changed, 1 insertion(+)

Just to recap: this patch must not be applied to Bluetooth tree, but it
should go via Qualcomm soc.

Best regards,
Krzysztof

Re: [PATCH v5 4/4] arm64: dts: qcom: sa8775p-ride: Add firmware-name in BT node

[Cheng Jiang (IOE)]

Hi Krzysztof,

On 12/13/2024 4:51 PM, Krzysztof Kozlowski wrote:
> On Thu, Dec 12, 2024 at 11:02:32PM +0800, Cheng Jiang wrote:
>> The sa8775p-ride platform uses the QCA6698 Bluetooth chip. While the
>> QCA6698 shares the same IP core as the WCN6855, it has different RF
>> components and RAM sizes, requiring new firmware files. Use the
>> firmware-name property to specify the NVM and rampatch firmware to load.
>>
>> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
> 
> Just to recap: this patch must not be applied to Bluetooth tree, but it
> should go via Qualcomm soc.
> 
Thanks, I will submit a separated patch of this changes. 

> Best regards,
> Krzysztof
> 

Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Konrad Dybcio]

On 13.12.2024 8:05 AM, Cheng Jiang (IOE) wrote:

[...]

>> /*
>>  * If the board-specific file is missing, try loading the default
>>  * one, unless that was attempted already
>>  */
>>
>> But, even more importantly:
>>
>> a) do we want to load the "incorrect" file?
> Normally, there is a default NVM file ending with .bin, which is suitable 
> for most boards. However, some boards have different configurations that 
> require a specific NVM. If a board-specific NVM is not found, a default 
> NVM is preferred over not loading any NVM.

So, if one is specified, but not found, this should either be a loud error,
or a very loud warning & fallback. Otherwise, the device may provide subpar
user experience without the user getting a chance to know the reason.

I think failing is better here, as that sends a clearer message, and would
only happen if the DT has a specific path (meaning the user put some
intentions behind that choice)

>> b) why would we want to specify the .bin file if it's the default anyway?
> The default NVM directory is the root of qca. The 'firmware-name' property 
> can specify an NVM file in another directory. This can be either a default 
> NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'.

Do we expect QCA6698/hpnv21.bin and QCAabcd/hpnv21.bin to be compatible?

[...]

>>> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
>>> -					    const char *stem, u8 rom_ver, u16 bid)
>>> -{
>>> -	if (bid == 0x0)
>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
>>> -	else if (bid & 0xff00)
>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>>> -			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
>>> -	else
>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>>> -			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
>>> +	if (soc_type == QCA_WCN6855 || soc_type == QCA_QCA2066) {
>>> +		/* hsp gf chip */
>>
>> This is a good opportunity to explain what that means
>>
> Ack. This means the chip is produced by GlobalFoundries.

Please update the comment there

Konrad

Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Cheng Jiang (IOE)]

Hi Konrad,

On 12/20/2024 9:46 PM, Konrad Dybcio wrote:
> On 13.12.2024 8:05 AM, Cheng Jiang (IOE) wrote:
> 
> [...]
> 
>>> /*
>>>  * If the board-specific file is missing, try loading the default
>>>  * one, unless that was attempted already
>>>  */
>>>
>>> But, even more importantly:
>>>
>>> a) do we want to load the "incorrect" file?
>> Normally, there is a default NVM file ending with .bin, which is suitable 
>> for most boards. However, some boards have different configurations that 
>> require a specific NVM. If a board-specific NVM is not found, a default 
>> NVM is preferred over not loading any NVM.
> 
> So, if one is specified, but not found, this should either be a loud error,
> or a very loud warning & fallback. Otherwise, the device may provide subpar
> user experience without the user getting a chance to know the reason.
> 
> I think failing is better here, as that sends a clearer message, and would
> only happen if the DT has a specific path (meaning the user put some
> intentions behind that choice)
> 
In the existing BT driver implementation, even if the rampatch/nvm are not found,
BT still works with ROM code only. No fails, just a warning in the dmesg. For this
new approach, we use the similar logic. 

The fallback to load a default nvm file is due to each board has a unique board
id, it's not necessary to upstream all the board-specific nvm since most of them 
may be the same, the default nvm file is suitable for them. But we can't set the 
default nvm file name in the DT, since the platform can attach different 
connectivity boards. This is a reasonable way to approach this. 

>>> b) why would we want to specify the .bin file if it's the default anyway?
>> The default NVM directory is the root of qca. The 'firmware-name' property 
>> can specify an NVM file in another directory. This can be either a default 
>> NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'.
> 
> Do we expect QCA6698/hpnv21.bin and QCAabcd/hpnv21.bin to be compatible?
> 
No. It may be different. 
> [...]
> 
>>>> -static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
>>>> -					    const char *stem, u8 rom_ver, u16 bid)
>>>> -{
>>>> -	if (bid == 0x0)
>>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/%snv%02x.bin", stem, rom_ver);
>>>> -	else if (bid & 0xff00)
>>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>>>> -			 "qca/%snv%02x.b%x", stem, rom_ver, bid);
>>>> -	else
>>>> -		snprintf(cfg->fwname, sizeof(cfg->fwname),
>>>> -			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
>>>> +	if (soc_type == QCA_WCN6855 || soc_type == QCA_QCA2066) {
>>>> +		/* hsp gf chip */
>>>
>>> This is a good opportunity to explain what that means
>>>
>> Ack. This means the chip is produced by GlobalFoundries.
> 
> Please update the comment there
> 
ACK. 
> Konrad

Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Konrad Dybcio]

On 23.12.2024 3:47 AM, Cheng Jiang (IOE) wrote:
> Hi Konrad,
> 
> On 12/20/2024 9:46 PM, Konrad Dybcio wrote:
>> On 13.12.2024 8:05 AM, Cheng Jiang (IOE) wrote:
>>
>> [...]
>>
>>>> /*
>>>>  * If the board-specific file is missing, try loading the default
>>>>  * one, unless that was attempted already
>>>>  */
>>>>
>>>> But, even more importantly:
>>>>
>>>> a) do we want to load the "incorrect" file?
>>> Normally, there is a default NVM file ending with .bin, which is suitable 
>>> for most boards. However, some boards have different configurations that 
>>> require a specific NVM. If a board-specific NVM is not found, a default 
>>> NVM is preferred over not loading any NVM.
>>
>> So, if one is specified, but not found, this should either be a loud error,
>> or a very loud warning & fallback. Otherwise, the device may provide subpar
>> user experience without the user getting a chance to know the reason.
>>
>> I think failing is better here, as that sends a clearer message, and would
>> only happen if the DT has a specific path (meaning the user put some
>> intentions behind that choice)
>>
> In the existing BT driver implementation, even if the rampatch/nvm are not found,
> BT still works with ROM code only. No fails, just a warning in the dmesg. For this
> new approach, we use the similar logic. 
> 
> The fallback to load a default nvm file is due to each board has a unique board
> id, it's not necessary to upstream all the board-specific nvm since most of them 
> may be the same, the default nvm file is suitable for them. But we can't set the 
> default nvm file name in the DT, since the platform can attach different 
> connectivity boards. This is a reasonable way to approach this. 

Okay, let's do it this way then.

>>>> b) why would we want to specify the .bin file if it's the default anyway?
>>> The default NVM directory is the root of qca. The 'firmware-name' property 
>>> can specify an NVM file in another directory. This can be either a default 
>>> NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'.
>>
>> Do we expect QCA6698/hpnv21.bin and QCAabcd/hpnv21.bin to be compatible?
>>
> No. It may be different. 

That's a bit disappointing considering the filename implies it's suitable
for a family of chips.. But I guess there's nothing we can change here.

Konrad

Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Cheng Jiang (IOE)]

On 12/23/2024 7:46 PM, Konrad Dybcio wrote:
> On 23.12.2024 3:47 AM, Cheng Jiang (IOE) wrote:
>> Hi Konrad,
>>
>> On 12/20/2024 9:46 PM, Konrad Dybcio wrote:
>>> On 13.12.2024 8:05 AM, Cheng Jiang (IOE) wrote:
>>>
>>> [...]
>>>
>>>>> /*
>>>>>  * If the board-specific file is missing, try loading the default
>>>>>  * one, unless that was attempted already
>>>>>  */
>>>>>
>>>>> But, even more importantly:
>>>>>
>>>>> a) do we want to load the "incorrect" file?
>>>> Normally, there is a default NVM file ending with .bin, which is suitable 
>>>> for most boards. However, some boards have different configurations that 
>>>> require a specific NVM. If a board-specific NVM is not found, a default 
>>>> NVM is preferred over not loading any NVM.
>>>
>>> So, if one is specified, but not found, this should either be a loud error,
>>> or a very loud warning & fallback. Otherwise, the device may provide subpar
>>> user experience without the user getting a chance to know the reason.
>>>
>>> I think failing is better here, as that sends a clearer message, and would
>>> only happen if the DT has a specific path (meaning the user put some
>>> intentions behind that choice)
>>>
>> In the existing BT driver implementation, even if the rampatch/nvm are not found,
>> BT still works with ROM code only. No fails, just a warning in the dmesg. For this
>> new approach, we use the similar logic. 
>>
>> The fallback to load a default nvm file is due to each board has a unique board
>> id, it's not necessary to upstream all the board-specific nvm since most of them 
>> may be the same, the default nvm file is suitable for them. But we can't set the 
>> default nvm file name in the DT, since the platform can attach different 
>> connectivity boards. This is a reasonable way to approach this. 
> 
> Okay, let's do it this way then.
> 
Ack.
>>>>> b) why would we want to specify the .bin file if it's the default anyway?
>>>> The default NVM directory is the root of qca. The 'firmware-name' property 
>>>> can specify an NVM file in another directory. This can be either a default 
>>>> NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'.
>>>
>>> Do we expect QCA6698/hpnv21.bin and QCAabcd/hpnv21.bin to be compatible?
>>>
>> No. It may be different. 
> 
> That's a bit disappointing considering the filename implies it's suitable
> for a family of chips.. But I guess there's nothing we can change here.
> 
They are different generations, so the nvm file may be not compatible. Yes,
there is nothing we can change here. 
 
> Konrad