diff mbox series

[01/10] qtnfmac: use MAJOR.MINOR format for firmware protocol

Message ID 20200127104634.7248-2-sergey.matyukevich.os@quantenna.com (mailing list archive)
State Accepted
Commit a3ebb0335c545e05fb7693e246b91946276b83fe
Delegated to: Kalle Valo
Headers show
Series qtnfmac: update firmware protocol | expand

Commit Message

Sergey Matyukevich Jan. 27, 2020, 10:46 a.m. UTC
From: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>

Use MAJOR.MINOR format for QLink firmware protocol. MAJOR part is
incremented when backward compatibility is broken. Normally this
part should not be incremented unless there is a good reason for
that. MINOR part is incremented each time when new features are
added to qlink.h, e.g. new TLVs, events, commands. These changes
should not break backward compatibility. For instance, older
firmware versions may not be able to parse new flags or send new
types of events, but this does not impact normal system operations.

As part of initialization sequence, driver requests protocol version
from firmware and refuses to start in case there is a mismatch in MAJOR
part of the version.

Signed-off-by: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 59 ++++++++++++++---------
 drivers/net/wireless/quantenna/qtnfmac/core.c     | 16 +++---
 drivers/net/wireless/quantenna/qtnfmac/core.h     |  2 +-
 drivers/net/wireless/quantenna/qtnfmac/qlink.h    | 42 ++++++++++++++--
 4 files changed, 86 insertions(+), 33 deletions(-)

Comments

Kalle Valo Feb. 12, 2020, 4:17 p.m. UTC | #1
Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> wrote:

> From: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>
> 
> Use MAJOR.MINOR format for QLink firmware protocol. MAJOR part is
> incremented when backward compatibility is broken. Normally this
> part should not be incremented unless there is a good reason for
> that. MINOR part is incremented each time when new features are
> added to qlink.h, e.g. new TLVs, events, commands. These changes
> should not break backward compatibility. For instance, older
> firmware versions may not be able to parse new flags or send new
> types of events, but this does not impact normal system operations.
> 
> As part of initialization sequence, driver requests protocol version
> from firmware and refuses to start in case there is a mismatch in MAJOR
> part of the version.
> 
> Signed-off-by: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>

10 patches applied to wireless-drivers-next.git, thanks.

a3ebb0335c54 qtnfmac: use MAJOR.MINOR format for firmware protocol
310cd5dd5070 qtnfmac: pass hardware capabilities in TLV element
e70cf22bc7b2 qtnfmac: merge PHY_PARAMS_GET into MAC_INFO
bc5db734c1c4 qtnfmac: drop QTN_TLV_ID_NUM_IFACE_COMB TLV type
601ce21ff88d qtnfmac: implement extendable channel survey dump
0d18a9c0a387 qtnfmac: pass max scan SSIDs limit on per-radio basis
8b0b5f1ba9e0 qtnfmac: cleanup alignment in firmware communication protocol
5edadc5a3f11 qtnfmac: update channel switch command to support 6GHz band
501c3be1ec3c qtnfmac: drop unnecessary TLVs from scan command
946d077a4256 qtnfmac: fix potential Spectre vulnerabilities
diff mbox series

Patch

diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index d0d7ec8794c4..4f52e2fd7f38 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -900,7 +900,6 @@  qtnf_cmd_resp_proc_hw_info(struct qtnf_bus *bus,
 	hwinfo->num_mac = resp->num_mac;
 	hwinfo->mac_bitmap = resp->mac_bitmap;
 	hwinfo->fw_ver = le32_to_cpu(resp->fw_ver);
-	hwinfo->ql_proto_ver = le16_to_cpu(resp->ql_proto_ver);
 	hwinfo->total_tx_chain = resp->total_tx_chain;
 	hwinfo->total_rx_chain = resp->total_rx_chain;
 	hwinfo->hw_capab = le32_to_cpu(resp->hw_capab);
@@ -954,25 +953,29 @@  qtnf_cmd_resp_proc_hw_info(struct qtnf_bus *bus,
 		tlv = (struct qlink_tlv_hdr *)(tlv->val + tlv_value_len);
 	}
 
-	pr_info("fw_version=%d, MACs map %#x, chains Tx=%u Rx=%u, capab=0x%x\n",
-		hwinfo->fw_ver, hwinfo->mac_bitmap,
-		hwinfo->total_tx_chain, hwinfo->total_rx_chain,
-		hwinfo->hw_capab);
-
-	pr_info("\nBuild name:            %s"  \
-		"\nBuild revision:        %s"  \
-		"\nBuild type:            %s"  \
-		"\nBuild label:           %s"  \
-		"\nBuild timestamp:       %lu" \
-		"\nPlatform ID:           %lu" \
-		"\nHardware ID:           %s"  \
-		"\nCalibration version:   %s"  \
-		"\nU-Boot version:        %s"  \
-		"\nHardware version:      0x%08x\n",
+	pr_info("\nBuild name:            %s\n"
+		"Build revision:        %s\n"
+		"Build type:            %s\n"
+		"Build label:           %s\n"
+		"Build timestamp:       %lu\n"
+		"Platform ID:           %lu\n"
+		"Hardware ID:           %s\n"
+		"Calibration version:   %s\n"
+		"U-Boot version:        %s\n"
+		"Hardware version:      0x%08x\n"
+		"Qlink ver:             %u.%u\n"
+		"MACs map:              %#x\n"
+		"Chains Rx-Tx:          %ux%u\n"
+		"FW version:            0x%x\n",
 		bld_name, bld_rev, bld_type, bld_label,
 		(unsigned long)bld_tmstamp,
 		(unsigned long)plat_id,
-		hw_id, calibration_ver, uboot_ver, hw_ver);
+		hw_id, calibration_ver, uboot_ver, hw_ver,
+		QLINK_VER_MAJOR(bus->hw_info.ql_proto_ver),
+		QLINK_VER_MINOR(bus->hw_info.ql_proto_ver),
+		hwinfo->mac_bitmap,
+		hwinfo->total_rx_chain, hwinfo->total_tx_chain,
+		hwinfo->fw_ver);
 
 	strlcpy(hwinfo->fw_version, bld_label, sizeof(hwinfo->fw_version));
 	hwinfo->hw_version = hw_ver;
@@ -1866,23 +1869,35 @@  int qtnf_cmd_send_update_phy_params(struct qtnf_wmac *mac, u32 changed)
 
 int qtnf_cmd_send_init_fw(struct qtnf_bus *bus)
 {
+	struct sk_buff *resp_skb = NULL;
+	struct qlink_resp_init_fw *resp;
+	struct qlink_cmd_init_fw *cmd;
 	struct sk_buff *cmd_skb;
-	int ret = 0;
+	size_t info_len = 0;
+	int ret;
 
 	cmd_skb = qtnf_cmd_alloc_new_cmdskb(QLINK_MACID_RSVD, QLINK_VIFID_RSVD,
 					    QLINK_CMD_FW_INIT,
-					    sizeof(struct qlink_cmd));
+					    sizeof(*cmd));
 	if (!cmd_skb)
 		return -ENOMEM;
 
+	cmd = (struct qlink_cmd_init_fw *)cmd_skb->data;
+	cmd->qlink_proto_ver = cpu_to_le32(QLINK_PROTO_VER);
+
 	qtnf_bus_lock(bus);
-	ret = qtnf_cmd_send(bus, cmd_skb);
+	ret = qtnf_cmd_send_with_reply(bus, cmd_skb, &resp_skb,
+				       sizeof(*resp), &info_len);
+	qtnf_bus_unlock(bus);
+
 	if (ret)
 		goto out;
 
-out:
-	qtnf_bus_unlock(bus);
+	resp = (struct qlink_resp_init_fw *)resp_skb->data;
+	bus->hw_info.ql_proto_ver = le32_to_cpu(resp->qlink_proto_ver);
 
+out:
+	consume_skb(resp_skb);
 	return ret;
 }
 
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index 4320180f8c07..a21b09afd103 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -756,6 +756,15 @@  int qtnf_core_attach(struct qtnf_bus *bus)
 		goto error;
 	}
 
+	if (QLINK_VER_MAJOR(bus->hw_info.ql_proto_ver) !=
+	    QLINK_PROTO_VER_MAJOR) {
+		pr_err("qlink driver vs FW version mismatch: %u vs %u\n",
+		       QLINK_PROTO_VER_MAJOR,
+		       QLINK_VER_MAJOR(bus->hw_info.ql_proto_ver));
+		ret = -EPROTONOSUPPORT;
+		goto error;
+	}
+
 	bus->fw_state = QTNF_FW_STATE_ACTIVE;
 	ret = qtnf_cmd_get_hw_info(bus);
 	if (ret) {
@@ -763,13 +772,6 @@  int qtnf_core_attach(struct qtnf_bus *bus)
 		goto error;
 	}
 
-	if (bus->hw_info.ql_proto_ver != QLINK_PROTO_VER) {
-		pr_err("qlink version mismatch %u != %u\n",
-		       QLINK_PROTO_VER, bus->hw_info.ql_proto_ver);
-		ret = -EPROTONOSUPPORT;
-		goto error;
-	}
-
 	if ((bus->hw_info.hw_capab & QLINK_HW_CAPAB_HW_BRIDGE) &&
 	    bus->bus_ops->data_tx_use_meta_set)
 		bus->bus_ops->data_tx_use_meta_set(bus, true);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index d715e1cd0006..78fee516cea4 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -117,7 +117,7 @@  struct qtnf_wmac {
 };
 
 struct qtnf_hw_info {
-	u16 ql_proto_ver;
+	u32 ql_proto_ver;
 	u8 num_mac;
 	u8 mac_bitmap;
 	u32 fw_ver;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index b2edb03819d1..3722e707232c 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -6,7 +6,18 @@ 
 
 #include <linux/ieee80211.h>
 
-#define QLINK_PROTO_VER		16
+#define QLINK_PROTO_VER_MAJOR_M		0xFFFF
+#define QLINK_PROTO_VER_MAJOR_S		16
+#define QLINK_PROTO_VER_MINOR_M		0xFFFF
+#define QLINK_VER_MINOR(_ver)	((_ver) & QLINK_PROTO_VER_MINOR_M)
+#define QLINK_VER_MAJOR(_ver)	\
+	(((_ver) >> QLINK_PROTO_VER_MAJOR_S) & QLINK_PROTO_VER_MAJOR_M)
+#define QLINK_VER(_maj, _min)	(((_maj) << QLINK_PROTO_VER_MAJOR_S) | (_min))
+
+#define QLINK_PROTO_VER_MAJOR		18
+#define QLINK_PROTO_VER_MINOR		0
+#define QLINK_PROTO_VER		\
+	QLINK_VER(QLINK_PROTO_VER_MAJOR, QLINK_PROTO_VER_MINOR)
 
 #define QLINK_MACID_RSVD		0xFF
 #define QLINK_VIFID_RSVD		0xFF
@@ -327,6 +338,23 @@  struct qlink_cmd {
 } __packed;
 
 /**
+ * struct qlink_cmd_init_fw - data for QLINK_CMD_FW_INIT
+ *
+ * Initialize firmware based on specified host configuration. This is the first
+ * command sent to wifi card and it's fixed part should never be changed, any
+ * additions must be done by appending TLVs.
+ * If wifi card can not operate with a specified parameters it will return
+ * error.
+ *
+ * @qlink_proto_ver: QLINK protocol version used by host driver.
+ */
+struct qlink_cmd_init_fw {
+	struct qlink_cmd chdr;
+	__le32 qlink_proto_ver;
+	u8 var_info[0];
+} __packed;
+
+/**
  * struct qlink_cmd_manage_intf - interface management command
  *
  * Data for interface management commands QLINK_CMD_ADD_INTF, QLINK_CMD_DEL_INTF
@@ -896,6 +924,16 @@  struct qlink_resp {
 } __packed;
 
 /**
+ * struct qlink_resp_init_fw - response for QLINK_CMD_FW_INIT
+ *
+ * @qlink_proto_ver: QLINK protocol version used by wifi card firmware.
+ */
+struct qlink_resp_init_fw {
+	struct qlink_resp rhdr;
+	__le32 qlink_proto_ver;
+} __packed;
+
+/**
  * enum qlink_dfs_regions - regulatory DFS regions
  *
  * Corresponds to &enum nl80211_dfs_regions.
@@ -953,7 +991,6 @@  struct qlink_resp_get_mac_info {
  *
  * @fw_ver: wireless hardware firmware version.
  * @hw_capab: Bitmap of capabilities supported by firmware.
- * @ql_proto_ver: Version of QLINK protocol used by firmware.
  * @num_mac: Number of separate physical radio devices provided by hardware.
  * @mac_bitmap: Bitmap of MAC IDs that are active and can be used in firmware.
  * @total_tx_chains: total number of transmit chains used by device.
@@ -967,7 +1004,6 @@  struct qlink_resp_get_hw_info {
 	__le32 bld_tmstamp;
 	__le32 plat_id;
 	__le32 hw_ver;
-	__le16 ql_proto_ver;
 	u8 num_mac;
 	u8 mac_bitmap;
 	u8 total_tx_chain;