Ramon Fried [off-list ref] writes:

From: Eyal Ilsar <redacted>

Introduced infrastructure for supporting FTM WLAN in user space exposing
the netlink channel from the kernel WLAN driver.

What's FTM WLAN? Don't assume that people know all the acronyms.

This included:

1) Registered wcn36xx driver to testmode callback from netlink
2) Added testmode callback implementation to handle incoming FTM commands
3) Added FTM command packet structure
4) Added handling for GET_BUILD_RELEASE_NUMBER (msgid=0x32A2)
5) Added generic handling for all PTT_MSG packets

I don't remember the english grammar terminology, but in the commit log
use the form "register", "add" instead of "registered", "added".

+struct wcn36xx_hal_process_ptt_msg_rsp_msg {
+	struct wcn36xx_hal_msg_header header;
+
+	/* FTM Command response status */
+	u32   ptt_msg_resp_status;

Unnecessary whitespace after u32.

+static int wcn36xx_smd_process_ptt_msg_rsp(void *buf, size_t len,
+			void **p_ptt_rsp_msg)
+{
+	struct wcn36xx_hal_process_ptt_msg_rsp_msg *rsp;
+	int ret = 0;
+
+	ret = wcn36xx_smd_rsp_status_check(buf, len);
+	if (ret)
+		return ret;
+	rsp = (struct wcn36xx_hal_process_ptt_msg_rsp_msg *)buf;
+	wcn36xx_dbg(WCN36XX_DBG_HAL, "process ptt msg responded with length %d\n",
+			rsp->header.len);
+	wcn36xx_dbg_dump(WCN36XX_DBG_HAL_DUMP, "HAL_PTT_MSG_RSP:", rsp->ptt_msg,
+			rsp->header.len - sizeof(rsp->ptt_msg_resp_status));
+	if (rsp->header.len > 0) {
+		*p_ptt_rsp_msg = kmalloc(rsp->header.len, GFP_ATOMIC);
+		memcpy(*p_ptt_rsp_msg, rsp->ptt_msg, rsp->header.len);
+	}
+	return ret;
+}

Adding few empty lines to the function would make it more readable.

Unrelated change.

+int wcn36xx_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+		  void *data, int len)
+{
+	struct wcn36xx *wcn = hw->priv;
+	struct nlattr *tb[WCN36XX_TM_ATTR_MAX + 1];
+	int ret = 0;
+	unsigned short attr;
+
+	wcn36xx_dbg_dump(WCN36XX_DBG_TESTMODE_DUMP, "Data:", data, len);
+	ret = nla_parse(tb, WCN36XX_TM_ATTR_MAX, data, len,
+			wcn36xx_tm_policy, NULL);
+	if (ret)
+		return ret;
+
+	if (!tb[WCN36XX_TM_ATTR_CMD])
+		return -EINVAL;
+
+	attr = nla_get_u16(tb[WCN36XX_TM_ATTR_CMD]);
+
+	switch (attr) {
+	case WCN36XX_TM_CMD_START:
+	case WCN36XX_TM_CMD_STOP:
+		// N/A to this driver as it does not need to switch state
+		break;

[...]

+enum wcn36xx_tm_cmd {
+	/* For backwards compatibility
+	 */
+	WCN36XX_TM_CMD_START = 1,
+
+	/* For backwards compatibility
+	 */
+	WCN36XX_TM_CMD_STOP = 2,

This looks odd. If wcn36xx does not need START and STOP commands why add
those in the first place?

-- 
Kalle Valo