[RFC 0/3] ath10k: firmware-related updates

[RFC 0/3] ath10k: firmware-related updates

[Michal Kazior]

Hi,

This patchset contains 2 firmware-related upgrades
and 1 fix (that I though is a good pick to include
here, since it's related with HTT tx).

This keeps backward compatbility with old
firmware.


Pozdrawiam / Best regards,
Michal Kazior.


Michal Kazior (3):
  ath10k: add support for firmware newer than 636
  ath10k: clean up HTT tx tid handling
  ath10k: add support for HTT 3.0

 drivers/net/wireless/ath/ath10k/htt.c    |   16 +++----
 drivers/net/wireless/ath/ath10k/htt.h    |   12 ++---
 drivers/net/wireless/ath/ath10k/htt_tx.c |   70 ++++++++++++++++++++----------
 drivers/net/wireless/ath/ath10k/hw.h     |    3 ++
 drivers/net/wireless/ath/ath10k/mac.c    |   17 ++++++--
 drivers/net/wireless/ath/ath10k/wmi.c    |   29 +++++++++----
 drivers/net/wireless/ath/ath10k/wmi.h    |   16 +++++--
 7 files changed, 111 insertions(+), 52 deletions(-)

-- 
1.7.9.5

[RFC 1/3] ath10k: add support for firmware newer than 636

[Michal Kazior]

The mgmt_rx event structure has been expanded.
Since the structure header is expanded the payload
(i.e. mgmt frame) is shifted by a few bytes. This
needs to be taken into account in order to support
both old and new firmware.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi.c |   29 +++++++++++++++++++++--------
 drivers/net/wireless/ath/ath10k/wmi.h |   16 +++++++++++++---
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 55f90c7..fc33713 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -315,7 +315,9 @@ static inline u8 get_rate_idx(u32 rate, enum ieee80211_band band)
 
 static int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
 {
-	struct wmi_mgmt_rx_event *event = (struct wmi_mgmt_rx_event *)skb->data;
+	struct wmi_mgmt_rx_event_abi1 *ev_abi1;
+	struct wmi_mgmt_rx_event_abi2 *ev_abi2;
+	struct wmi_mgmt_rx_hdr_abi1 *ev_hdr;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_hdr *hdr;
 	u32 rx_status;
@@ -325,13 +327,24 @@ static int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
 	u32 rate;
 	u32 buf_len;
 	u16 fc;
+	int pull_len;
+
+	if (ar->fw_version_build > 636) {
+		ev_abi2 = (struct wmi_mgmt_rx_event_abi2 *)skb->data;
+		ev_hdr = &ev_abi2->hdr.abi1;
+		pull_len = sizeof(*ev_abi2);
+	} else {
+		ev_abi1 = (struct wmi_mgmt_rx_event_abi1 *)skb->data;
+		ev_hdr = &ev_abi1->hdr;
+		pull_len = sizeof(*ev_abi1);
+	}
 
-	channel   = __le32_to_cpu(event->hdr.channel);
-	buf_len   = __le32_to_cpu(event->hdr.buf_len);
-	rx_status = __le32_to_cpu(event->hdr.status);
-	snr       = __le32_to_cpu(event->hdr.snr);
-	phy_mode  = __le32_to_cpu(event->hdr.phy_mode);
-	rate	  = __le32_to_cpu(event->hdr.rate);
+	channel   = __le32_to_cpu(ev_hdr->channel);
+	buf_len   = __le32_to_cpu(ev_hdr->buf_len);
+	rx_status = __le32_to_cpu(ev_hdr->status);
+	snr       = __le32_to_cpu(ev_hdr->snr);
+	phy_mode  = __le32_to_cpu(ev_hdr->phy_mode);
+	rate	  = __le32_to_cpu(ev_hdr->rate);
 
 	memset(status, 0, sizeof(*status));
 
@@ -358,7 +371,7 @@ static int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
 	status->signal = snr + ATH10K_DEFAULT_NOISE_FLOOR;
 	status->rate_idx = get_rate_idx(rate, status->band);
 
-	skb_pull(skb, sizeof(event->hdr));
+	skb_pull(skb, pull_len);
 
 	hdr = (struct ieee80211_hdr *)skb->data;
 	fc = le16_to_cpu(hdr->frame_control);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 2c5a4f8..0dd0e10 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -1268,7 +1268,7 @@ struct wmi_scan_event {
  * good idea to pass all the fields in the RX status
  * descriptor up to the host.
  */
-struct wmi_mgmt_rx_hdr {
+struct wmi_mgmt_rx_hdr_abi1 {
 	__le32 channel;
 	__le32 snr;
 	__le32 rate;
@@ -1277,8 +1277,18 @@ struct wmi_mgmt_rx_hdr {
 	__le32 status; /* %WMI_RX_STATUS_ */
 } __packed;
 
-struct wmi_mgmt_rx_event {
-	struct wmi_mgmt_rx_hdr hdr;
+struct wmi_mgmt_rx_hdr_abi2 {
+	struct wmi_mgmt_rx_hdr_abi1 abi1;
+	__le32 rssi_ctl[4];
+} __packed;
+
+struct wmi_mgmt_rx_event_abi1 {
+	struct wmi_mgmt_rx_hdr_abi1 hdr;
+	u8 buf[0];
+} __packed;
+
+struct wmi_mgmt_rx_event_abi2 {
+	struct wmi_mgmt_rx_hdr_abi2 hdr;
 	u8 buf[0];
 } __packed;
 
-- 
1.7.9.5

Re: [RFC 1/3] ath10k: add support for firmware newer than 636

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> The mgmt_rx event structure has been expanded.
> Since the structure header is expanded the payload
> (i.e. mgmt frame) is shifted by a few bytes. This
> needs to be taken into account in order to support
> both old and new firmware.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> @@ -325,13 +327,24 @@ static int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
>  	u32 rate;
>  	u32 buf_len;
>  	u16 fc;
> +	int pull_len;
> +
> +	if (ar->fw_version_build > 636) {
> +		ev_abi2 = (struct wmi_mgmt_rx_event_abi2 *)skb->data;
> +		ev_hdr = &ev_abi2->hdr.abi1;
> +		pull_len = sizeof(*ev_abi2);
> +	} else {
> +		ev_abi1 = (struct wmi_mgmt_rx_event_abi1 *)skb->data;
> +		ev_hdr = &ev_abi1->hdr;
> +		pull_len = sizeof(*ev_abi1);
> +	}

I would prefer to have ar->fw_features (or similar) bitmap for handling
this. That way we can group all firmware version tests into one place
and not sprinkle them around. I suspect there will be more tests like
this.

[...]

> --- a/drivers/net/wireless/ath/ath10k/wmi.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> @@ -1268,7 +1268,7 @@ struct wmi_scan_event {
>   * good idea to pass all the fields in the RX status
>   * descriptor up to the host.
>   */
> -struct wmi_mgmt_rx_hdr {
> +struct wmi_mgmt_rx_hdr_abi1 {
>  	__le32 channel;
>  	__le32 snr;
>  	__le32 rate;
> @@ -1277,8 +1277,18 @@ struct wmi_mgmt_rx_hdr {
>  	__le32 status; /* %WMI_RX_STATUS_ */
>  } __packed;
>  
> -struct wmi_mgmt_rx_event {
> -	struct wmi_mgmt_rx_hdr hdr;
> +struct wmi_mgmt_rx_hdr_abi2 {
> +	struct wmi_mgmt_rx_hdr_abi1 abi1;
> +	__le32 rssi_ctl[4];
> +} __packed;
> +
> +struct wmi_mgmt_rx_event_abi1 {
> +	struct wmi_mgmt_rx_hdr_abi1 hdr;
> +	u8 buf[0];
> +} __packed;
> +
> +struct wmi_mgmt_rx_event_abi2 {
> +	struct wmi_mgmt_rx_hdr_abi2 hdr;
>  	u8 buf[0];
>  } __packed;

I would prefer to use v1 and v2 instead on abi1 and abi2.

-- 
Kalle Valo

[RFC 2/3] ath10k: clean up HTT tx tid handling

[Michal Kazior]

The tids weren't mapped really properly. This
doesn't fix any known bug but seems the right
thing to do.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt.h |    6 ++++--
 drivers/net/wireless/ath/ath10k/mac.c |    6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 318be46..de45d02 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -106,8 +106,10 @@ enum htt_data_tx_desc_flags1 {
 };
 
 enum htt_data_tx_ext_tid {
-	HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 16,
-	HTT_DATA_TX_EXT_TID_MGMT                = 17,
+	HTT_DATA_TX_EXT_TID_NON_QOS_UNICAST     = 16,
+	HTT_DATA_TX_EXT_TID_UNUSED              = 17,
+	HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 18,
+	HTT_DATA_TX_EXT_TID_MGMT                = 19,
 	HTT_DATA_TX_EXT_TID_INVALID             = 31
 };
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index cf2ba4d..fb1f24f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1726,12 +1726,14 @@ static void ath10k_tx(struct ieee80211_hw *hw,
 
 	/* we must calculate tid before we apply qos workaround
 	 * as we'd lose the qos control field */
-	tid = HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST;
 	if (ieee80211_is_data_qos(hdr->frame_control) &&
 	    is_unicast_ether_addr(ieee80211_get_DA(hdr))) {
 		u8 *qc = ieee80211_get_qos_ctl(hdr);
 		tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
-	}
+	} else if (is_unicast_ether_addr(ieee80211_get_DA(hdr)))
+		tid = HTT_DATA_TX_EXT_TID_NON_QOS_UNICAST;
+	else
+		tid = HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST;
 
 	/* it makes no sense to process injected frames like that */
 	if (info->control.vif &&
-- 
1.7.9.5

[RFC 3/3] ath10k: add support for HTT 3.0

[Michal Kazior]

New firmware comes with new HTT protocol version.
In 3.0 the separate mgmt tx command has been
removed. All traffic is to be pushed through data
tx (tx_frm) command with a twist - FW seems to not
be able (yet?) to access tx fragment table so for
manamgement frames frame pointer is passed
directly.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt.c    |   16 +++----
 drivers/net/wireless/ath/ath10k/htt.h    |    6 +--
 drivers/net/wireless/ath/ath10k/htt_tx.c |   70 ++++++++++++++++++++----------
 drivers/net/wireless/ath/ath10k/hw.h     |    3 ++
 drivers/net/wireless/ath/ath10k/mac.c    |   11 ++++-
 5 files changed, 69 insertions(+), 37 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 39342c5..44facc1 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -104,21 +104,15 @@ err_htc_attach:
 
 static int ath10k_htt_verify_version(struct ath10k_htt *htt)
 {
-	ath10k_dbg(ATH10K_DBG_HTT,
-		   "htt target version %d.%d; host version %d.%d\n",
-		    htt->target_version_major,
-		    htt->target_version_minor,
-		    HTT_CURRENT_VERSION_MAJOR,
-		    HTT_CURRENT_VERSION_MINOR);
-
-	if (htt->target_version_major != HTT_CURRENT_VERSION_MAJOR) {
+	ath10k_dbg(ATH10K_DBG_HTT, "htt target version %d.%d\n",
+		   htt->target_version_major, htt->target_version_minor);
+
+	if (htt->target_version_major != 2 &&
+	    htt->target_version_major != 3) {
 		ath10k_err("htt major versions are incompatible!\n");
 		return -ENOTSUPP;
 	}
 
-	if (htt->target_version_minor != HTT_CURRENT_VERSION_MINOR)
-		ath10k_warn("htt minor version differ but still compatible\n");
-
 	return 0;
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index de45d02..2488623 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -23,9 +23,6 @@
 #include "htc.h"
 #include "rx_desc.h"
 
-#define HTT_CURRENT_VERSION_MAJOR	2
-#define HTT_CURRENT_VERSION_MINOR	1
-
 enum htt_dbg_stats_type {
 	HTT_DBG_STATS_WAL_PDEV_TXRX = 1 << 0,
 	HTT_DBG_STATS_RX_REORDER    = 1 << 1,
@@ -45,6 +42,9 @@ enum htt_h2t_msg_type { /* host-to-target */
 	HTT_H2T_MSG_TYPE_SYNC               = 4,
 	HTT_H2T_MSG_TYPE_AGGR_CFG           = 5,
 	HTT_H2T_MSG_TYPE_FRAG_DESC_BANK_CFG = 6,
+
+	/* This command is used for sending management frames in HTT < 3.0.
+	 * HTT >= 3.0 uses TX_FRM for everything. */
 	HTT_H2T_MSG_TYPE_MGMT_TX            = 7,
 
 	HTT_H2T_NUM_MSGS /* keep this last */
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 656c254..dce128a 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -401,10 +401,15 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 		goto err;
 	}
 
-	txfrag = dev_alloc_skb(frag_len);
-	if (!txfrag) {
-		res = -ENOMEM;
-		goto err;
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (!(htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))) {
+		txfrag = dev_alloc_skb(frag_len);
+		if (!txfrag) {
+			res = -ENOMEM;
+			goto err;
+		}
 	}
 
 	if (!IS_ALIGNED((unsigned long)txdesc->data, 4)) {
@@ -427,23 +432,30 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 	if (res)
 		goto err;
 
-	/* tx fragment list must be terminated with zero-entry */
-	skb_put(txfrag, frag_len);
-	tx_frags = (struct htt_data_tx_desc_frag *)txfrag->data;
-	tx_frags[0].paddr = __cpu_to_le32(ATH10K_SKB_CB(msdu)->paddr);
-	tx_frags[0].len   = __cpu_to_le32(msdu->len);
-	tx_frags[1].paddr = __cpu_to_le32(0);
-	tx_frags[1].len   = __cpu_to_le32(0);
-
-	res = ath10k_skb_map(dev, txfrag);
-	if (res)
-		goto err;
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (!(htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))) {
+		/* tx fragment list must be terminated with zero-entry */
+		skb_put(txfrag, frag_len);
+		tx_frags = (struct htt_data_tx_desc_frag *)txfrag->data;
+		tx_frags[0].paddr = __cpu_to_le32(ATH10K_SKB_CB(msdu)->paddr);
+		tx_frags[0].len   = __cpu_to_le32(msdu->len);
+		tx_frags[1].paddr = __cpu_to_le32(0);
+		tx_frags[1].len   = __cpu_to_le32(0);
+
+		res = ath10k_skb_map(dev, txfrag);
+		if (res)
+			goto err;
+
+		ath10k_dbg(ATH10K_DBG_HTT, "txfrag 0x%llx\n",
+			   (unsigned long long) ATH10K_SKB_CB(txfrag)->paddr);
+		ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "txfrag: ",
+				txfrag->data, frag_len);
+	}
 
-	ath10k_dbg(ATH10K_DBG_HTT, "txfrag 0x%llx msdu 0x%llx\n",
-		   (unsigned long long) ATH10K_SKB_CB(txfrag)->paddr,
+	ath10k_dbg(ATH10K_DBG_HTT, "msdu 0x%llx\n",
 		   (unsigned long long) ATH10K_SKB_CB(msdu)->paddr);
-	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "txfrag: ",
-			txfrag->data, frag_len);
 	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "msdu: ",
 			msdu->data, msdu->len);
 
@@ -459,8 +471,16 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 	if (!ieee80211_has_protected(hdr->frame_control))
 		flags0 |= HTT_DATA_TX_DESC_FLAGS0_NO_ENCRYPT;
 	flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT;
-	flags0 |= SM(ATH10K_HW_TXRX_NATIVE_WIFI,
-		     HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
+
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))
+		flags0 |= SM(ATH10K_HW_TXRX_MGMT,
+			     HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
+	else
+		flags0 |= SM(ATH10K_HW_TXRX_NATIVE_WIFI,
+			     HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
 
 	flags1  = 0;
 	flags1 |= SM((u16)vdev_id, HTT_DATA_TX_DESC_FLAGS1_VDEV_ID);
@@ -468,7 +488,13 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 	flags1 |= HTT_DATA_TX_DESC_FLAGS1_CKSUM_L3_OFFLOAD;
 	flags1 |= HTT_DATA_TX_DESC_FLAGS1_CKSUM_L4_OFFLOAD;
 
-	frags_paddr = ATH10K_SKB_CB(txfrag)->paddr;
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))
+		frags_paddr = ATH10K_SKB_CB(msdu)->paddr;
+	else
+		frags_paddr = ATH10K_SKB_CB(txfrag)->paddr;
 
 	cmd->hdr.msg_type        = HTT_H2T_MSG_TYPE_TX_FRM;
 	cmd->data_tx.flags0      = flags0;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 44ed5af..9c5d6fc 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -53,6 +53,9 @@ enum ath10k_hw_txrx_mode {
 	ATH10K_HW_TXRX_RAW = 0,
 	ATH10K_HW_TXRX_NATIVE_WIFI = 1,
 	ATH10K_HW_TXRX_ETHERNET = 2,
+
+	/* Valid for HTT >= 3.0. Used for management frames in TX_FRM. */
+	ATH10K_HW_TXRX_MGMT = 3,
 };
 
 enum ath10k_mcast2ucast_mode {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index fb1f24f..aca423c 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1480,6 +1480,12 @@ static void ath10k_tx_htt(struct ath10k *ar, struct sk_buff *skb)
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int ret;
 
+	if (ar->htt.target_version_major >= 3) {
+		/* Since HTT 3.0 there is no separate mgmt tx command */
+		ret = ath10k_htt_tx(&ar->htt, skb);
+		goto exit;
+	}
+
 	if (ieee80211_is_mgmt(hdr->frame_control))
 		ret = ath10k_htt_mgmt_tx(&ar->htt, skb);
 	else if (ieee80211_is_nullfunc(hdr->frame_control))
@@ -1491,6 +1497,7 @@ static void ath10k_tx_htt(struct ath10k *ar, struct sk_buff *skb)
 	else
 		ret = ath10k_htt_tx(&ar->htt, skb);
 
+exit:
 	if (ret) {
 		ath10k_warn("tx failed (%d). dropping packet.\n", ret);
 		ieee80211_free_txskb(ar->hw, skb);
@@ -1726,7 +1733,9 @@ static void ath10k_tx(struct ieee80211_hw *hw,
 
 	/* we must calculate tid before we apply qos workaround
 	 * as we'd lose the qos control field */
-	if (ieee80211_is_data_qos(hdr->frame_control) &&
+	if (ieee80211_is_mgmt(hdr->frame_control))
+		tid = HTT_DATA_TX_EXT_TID_MGMT;
+	else if (ieee80211_is_data_qos(hdr->frame_control) &&
 	    is_unicast_ether_addr(ieee80211_get_DA(hdr))) {
 		u8 *qc = ieee80211_get_qos_ctl(hdr);
 		tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
-- 
1.7.9.5

Re: [RFC 3/3] ath10k: add support for HTT 3.0

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> New firmware comes with new HTT protocol version.
> In 3.0 the separate mgmt tx command has been
> removed. All traffic is to be pushed through data
> tx (tx_frm) command with a twist - FW seems to not
> be able (yet?) to access tx fragment table so for
> manamgement frames frame pointer is passed
> directly.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

>  static int ath10k_htt_verify_version(struct ath10k_htt *htt)
>  {
> -	ath10k_dbg(ATH10K_DBG_HTT,
> -		   "htt target version %d.%d; host version %d.%d\n",
> -		    htt->target_version_major,
> -		    htt->target_version_minor,
> -		    HTT_CURRENT_VERSION_MAJOR,
> -		    HTT_CURRENT_VERSION_MINOR);
> -
> -	if (htt->target_version_major != HTT_CURRENT_VERSION_MAJOR) {
> +	ath10k_dbg(ATH10K_DBG_HTT, "htt target version %d.%d\n",
> +		   htt->target_version_major, htt->target_version_minor);

This debug print is good to have, but with the new htt version it would
be good to print it always using the info level. For example, can we add
it to the same line with "firmware %s booted" string?

(Please take into account that the info messages still need to be
compact, by default they should not be longer than five lines or so.)

> +	if (htt->target_version_major != 2 &&
> +	    htt->target_version_major != 3) {
>  		ath10k_err("htt major versions are incompatible!\n");
>  		return -ENOTSUPP;
>  	}

Print the htt version in the error message as well?

> @@ -401,10 +401,15 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
>  		goto err;
>  	}
>  
> -	txfrag = dev_alloc_skb(frag_len);
> -	if (!txfrag) {
> -		res = -ENOMEM;
> -		goto err;
> +	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
> +	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
> +	 * fragment list host driver specifies directly frame pointer. */
> +	if (!(htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))) {

I think using "< 3" is more readable:

if (htt->target_version_major < 3 &&
   !ieee80211_is_mgmt(hdr->frame_control)) {
        ...
}

And is the single line too long? Didn't count it, though.

> @@ -427,23 +432,30 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
>  	if (res)
>  		goto err;
>  
> -	/* tx fragment list must be terminated with zero-entry */
> -	skb_put(txfrag, frag_len);
> -	tx_frags = (struct htt_data_tx_desc_frag *)txfrag->data;
> -	tx_frags[0].paddr = __cpu_to_le32(ATH10K_SKB_CB(msdu)->paddr);
> -	tx_frags[0].len   = __cpu_to_le32(msdu->len);
> -	tx_frags[1].paddr = __cpu_to_le32(0);
> -	tx_frags[1].len   = __cpu_to_le32(0);
> -
> -	res = ath10k_skb_map(dev, txfrag);
> -	if (res)
> -		goto err;
> +	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
> +	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
> +	 * fragment list host driver specifies directly frame pointer. */
> +	if (!(htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))) {

Ditto.

-- 
Kalle Valo

Re: [RFC 3/3] ath10k: add support for HTT 3.0

[Michal Kazior]

On 8 August 2013 11:05, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> New firmware comes with new HTT protocol version.
>> In 3.0 the separate mgmt tx command has been
>> removed. All traffic is to be pushed through data
>> tx (tx_frm) command with a twist - FW seems to not
>> be able (yet?) to access tx fragment table so for
>> manamgement frames frame pointer is passed
>> directly.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> [...]
>
>>  static int ath10k_htt_verify_version(struct ath10k_htt *htt)
>>  {
>> -     ath10k_dbg(ATH10K_DBG_HTT,
>> -                "htt target version %d.%d; host version %d.%d\n",
>> -                 htt->target_version_major,
>> -                 htt->target_version_minor,
>> -                 HTT_CURRENT_VERSION_MAJOR,
>> -                 HTT_CURRENT_VERSION_MINOR);
>> -
>> -     if (htt->target_version_major != HTT_CURRENT_VERSION_MAJOR) {
>> +     ath10k_dbg(ATH10K_DBG_HTT, "htt target version %d.%d\n",
>> +                htt->target_version_major, htt->target_version_minor);
>
> This debug print is good to have, but with the new htt version it would
> be good to print it always using the info level. For example, can we add
> it to the same line with "firmware %s booted" string?

HTT target version is not known when firmware boots up. It's not known
until everything other (HTC, WMI) is set up. We then send a version
request command and we get a response.

We need to print it in a separate line.


> (Please take into account that the info messages still need to be
> compact, by default they should not be longer than five lines or so.)
>
>> +     if (htt->target_version_major != 2 &&
>> +         htt->target_version_major != 3) {
>>               ath10k_err("htt major versions are incompatible!\n");
>>               return -ENOTSUPP;
>>       }
>
> Print the htt version in the error message as well?

Target version is printed in ath10k_dbg() now. If we change that to
ath10k_info() I don't see any reason to print the version again on
error. We may want to print the list of supported HTT major version
numbers though?


>> @@ -401,10 +401,15 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
>>               goto err;
>>       }
>>
>> -     txfrag = dev_alloc_skb(frag_len);
>> -     if (!txfrag) {
>> -             res = -ENOMEM;
>> -             goto err;
>> +     /* Since HTT 3.0 there is no separate mgmt tx command. However in case
>> +      * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
>> +      * fragment list host driver specifies directly frame pointer. */
>> +     if (!(htt->target_version_major >= 3 && ieee80211_is_mgmt(hdr->frame_control))) {
>
> I think using "< 3" is more readable:
>
> if (htt->target_version_major < 3 &&
>    !ieee80211_is_mgmt(hdr->frame_control)) {
>         ...
> }

I don't have a problem with changing that.


> And is the single line too long? Didn't count it, though.

Ah, I didn't check that. Sorry. Good catch.



Pozdrawiam / Best regards,
Michał Kazior.

Re: [RFC 3/3] ath10k: add support for HTT 3.0

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 8 August 2013 11:05, Kalle Valo <kvalo@qca.qualcomm.com> wrote:

>> This debug print is good to have, but with the new htt version it would
>> be good to print it always using the info level. For example, can we add
>> it to the same line with "firmware %s booted" string?
>
> HTT target version is not known when firmware boots up. It's not known
> until everything other (HTC, WMI) is set up. We then send a version
> request command and we get a response.

Oh, missed that.

> We need to print it in a separate line.

Or could we print the "firmware booted" message later?

>> (Please take into account that the info messages still need to be
>> compact, by default they should not be longer than five lines or so.)
>>
>>> +     if (htt->target_version_major != 2 &&
>>> +         htt->target_version_major != 3) {
>>>               ath10k_err("htt major versions are incompatible!\n");
>>>               return -ENOTSUPP;
>>>       }
>>
>> Print the htt version in the error message as well?
>
> Target version is printed in ath10k_dbg() now. If we change that to
> ath10k_info() I don't see any reason to print the version again on
> error.

Frequently users just copy the error message, that's why it's better to
have the firmware's htt version in the error message as well.

> We may want to print the list of supported HTT major version numbers
> though?

That's good to have as well, at least in the debug messages.

-- 
Kalle Valo

Re: [RFC 3/3] ath10k: add support for HTT 3.0

[Michal Kazior]

On 8 August 2013 11:22, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> On 8 August 2013 11:05, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>
>>> This debug print is good to have, but with the new htt version it would
>>> be good to print it always using the info level. For example, can we add
>>> it to the same line with "firmware %s booted" string?
>>
>> HTT target version is not known when firmware boots up. It's not known
>> until everything other (HTC, WMI) is set up. We then send a version
>> request command and we get a response.
>
> Oh, missed that.
>
>> We need to print it in a separate line.
>
> Or could we print the "firmware booted" message later?

I'm worried it may be error-prone in case of firmware loading failure
in-between (i.e. firmware is booted, but WMI init fails). We'd need to
print the firmware version in the error path then.


>
>>> (Please take into account that the info messages still need to be
>>> compact, by default they should not be longer than five lines or so.)
>>>
>>>> +     if (htt->target_version_major != 2 &&
>>>> +         htt->target_version_major != 3) {
>>>>               ath10k_err("htt major versions are incompatible!\n");
>>>>               return -ENOTSUPP;
>>>>       }
>>>
>>> Print the htt version in the error message as well?
>>
>> Target version is printed in ath10k_dbg() now. If we change that to
>> ath10k_info() I don't see any reason to print the version again on
>> error.
>
> Frequently users just copy the error message, that's why it's better to
> have the firmware's htt version in the error message as well.

Good point.


Pozdrawiam / Best regards,
Michał Kazior.

Re: [RFC 3/3] ath10k: add support for HTT 3.0

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 8 August 2013 11:22, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> On 8 August 2013 11:05, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>
>>>> This debug print is good to have, but with the new htt version it would
>>>> be good to print it always using the info level. For example, can we add
>>>> it to the same line with "firmware %s booted" string?
>>>
>>> HTT target version is not known when firmware boots up. It's not known
>>> until everything other (HTC, WMI) is set up. We then send a version
>>> request command and we get a response.
>>
>> Oh, missed that.
>>
>>> We need to print it in a separate line.
>>
>> Or could we print the "firmware booted" message later?
>
> I'm worried it may be error-prone in case of firmware loading failure
> in-between (i.e. firmware is booted, but WMI init fails). We'd need to
> print the firmware version in the error path then.

True, let's just print in a separate line. We can worry about compacting
it later.

-- 
Kalle Valo

Re: [RFC 0/3] ath10k: firmware-related updates

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> This patchset contains 2 firmware-related upgrades
> and 1 fix (that I though is a good pick to include
> here, since it's related with HTT tx).
>
> This keeps backward compatbility with old
> firmware.

Thanks, looks very good! I just had more or less cosmetic comments.

-- 
Kalle Valo

[PATCH 0/3] ath10k: firmware-related updates

[Michal Kazior]

Hi,

This patchset contains 2 firmware-related upgrades
and 1 fix (that I though is a good pick to include
here, since it's related with HTT tx).

This keeps backward compatbility with old
firmware.

This includes fixes for issues pointed out during
RFC review.


Pozdrawiam / Best regards,
Michal Kazior.


Michal Kazior (3):
  ath10k: clean up HTT tx tid handling
  ath10k: add support for firmware newer than 636
  ath10k: add support for HTT 3.0

 drivers/net/wireless/ath/ath10k/core.h   |   10 ++++
 drivers/net/wireless/ath/ath10k/htt.c    |   19 +++-----
 drivers/net/wireless/ath/ath10k/htt.h    |   12 +++--
 drivers/net/wireless/ath/ath10k/htt_tx.c |   74 +++++++++++++++++++++---------
 drivers/net/wireless/ath/ath10k/hw.h     |    3 ++
 drivers/net/wireless/ath/ath10k/mac.c    |   18 ++++++--
 drivers/net/wireless/ath/ath10k/wmi.c    |   32 +++++++++----
 drivers/net/wireless/ath/ath10k/wmi.h    |   16 +++++--
 8 files changed, 131 insertions(+), 53 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] ath10k: clean up HTT tx tid handling

[Michal Kazior]

The tids weren't mapped really properly. This
doesn't fix any known bug but seems the right
thing to do.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
patch:
 * use brackets {}

 drivers/net/wireless/ath/ath10k/htt.h |    6 ++++--
 drivers/net/wireless/ath/ath10k/mac.c |    5 ++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 318be46..de45d02 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -106,8 +106,10 @@ enum htt_data_tx_desc_flags1 {
 };
 
 enum htt_data_tx_ext_tid {
-	HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 16,
-	HTT_DATA_TX_EXT_TID_MGMT                = 17,
+	HTT_DATA_TX_EXT_TID_NON_QOS_UNICAST     = 16,
+	HTT_DATA_TX_EXT_TID_UNUSED              = 17,
+	HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 18,
+	HTT_DATA_TX_EXT_TID_MGMT                = 19,
 	HTT_DATA_TX_EXT_TID_INVALID             = 31
 };
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index cf2ba4d..cc6ad7a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1726,11 +1726,14 @@ static void ath10k_tx(struct ieee80211_hw *hw,
 
 	/* we must calculate tid before we apply qos workaround
 	 * as we'd lose the qos control field */
-	tid = HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST;
 	if (ieee80211_is_data_qos(hdr->frame_control) &&
 	    is_unicast_ether_addr(ieee80211_get_DA(hdr))) {
 		u8 *qc = ieee80211_get_qos_ctl(hdr);
 		tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
+	} else if (is_unicast_ether_addr(ieee80211_get_DA(hdr))) {
+		tid = HTT_DATA_TX_EXT_TID_NON_QOS_UNICAST;
+	} else {
+		tid = HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST;
 	}
 
 	/* it makes no sense to process injected frames like that */
-- 
1.7.9.5

Re: [PATCH 1/3] ath10k: clean up HTT tx tid handling

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> The tids weren't mapped really properly. This
> doesn't fix any known bug but seems the right
> thing to do.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

>  enum htt_data_tx_ext_tid {
> -	HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 16,
> -	HTT_DATA_TX_EXT_TID_MGMT                = 17,
> +	HTT_DATA_TX_EXT_TID_NON_QOS_UNICAST     = 16,
> +	HTT_DATA_TX_EXT_TID_UNUSED              = 17,
> +	HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 18,
> +	HTT_DATA_TX_EXT_TID_MGMT                = 19,
>  	HTT_DATA_TX_EXT_TID_INVALID             = 31
>  };

This doesn't seem to match what firmware uses. We need to investigate
more about this so I drop the patch for now.

-- 
Kalle Valo

[PATCH 2/3] ath10k: add support for firmware newer than 636

[Michal Kazior]

The mgmt_rx event structure has been expanded.
Since the structure header is expanded the payload
(i.e. mgmt frame) is shifted by a few bytes. This
needs to be taken into account in order to support
both old and new firmware.

This introduces a fw_features to keep track of any
FW-related ABI/behaviour changes.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
patch:
 * add/use fw_features instead of direct build
   version probing (Kalle)
 * use _v1/_v2 instead of _abi1/_abi2 (Kalle)

 drivers/net/wireless/ath/ath10k/core.h |   10 ++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c  |   32 ++++++++++++++++++++++++--------
 drivers/net/wireless/ath/ath10k/wmi.h  |   16 +++++++++++++---
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index e4bba56..ab05c4c 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -270,6 +270,14 @@ enum ath10k_state {
 	ATH10K_STATE_WEDGED,
 };
 
+enum ath10k_fw_features {
+	/* wmi_mgmt_rx_hdr contains extra RSSI information */
+	ATH10K_FW_FEATURE_EXT_WMI_MGMT_RX = 0,
+
+	/* keep last */
+	ATH10K_FW_FEATURE_COUNT,
+};
+
 struct ath10k {
 	struct ath_common ath_common;
 	struct ieee80211_hw *hw;
@@ -288,6 +296,8 @@ struct ath10k {
 	u32 vht_cap_info;
 	u32 num_rf_chains;
 
+	DECLARE_BITMAP(fw_features, ATH10K_FW_FEATURE_COUNT);
+
 	struct targetdef *targetdef;
 	struct hostdef *hostdef;
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 55f90c7..d606fef 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -315,7 +315,9 @@ static inline u8 get_rate_idx(u32 rate, enum ieee80211_band band)
 
 static int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
 {
-	struct wmi_mgmt_rx_event *event = (struct wmi_mgmt_rx_event *)skb->data;
+	struct wmi_mgmt_rx_event_v1 *ev_v1;
+	struct wmi_mgmt_rx_event_v2 *ev_v2;
+	struct wmi_mgmt_rx_hdr_v1 *ev_hdr;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_hdr *hdr;
 	u32 rx_status;
@@ -325,13 +327,24 @@ static int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
 	u32 rate;
 	u32 buf_len;
 	u16 fc;
+	int pull_len;
+
+	if (test_bit(ATH10K_FW_FEATURE_EXT_WMI_MGMT_RX, ar->fw_features)) {
+		ev_v2 = (struct wmi_mgmt_rx_event_v2 *)skb->data;
+		ev_hdr = &ev_v2->hdr.v1;
+		pull_len = sizeof(*ev_v2);
+	} else {
+		ev_v1 = (struct wmi_mgmt_rx_event_v1 *)skb->data;
+		ev_hdr = &ev_v1->hdr;
+		pull_len = sizeof(*ev_v1);
+	}
 
-	channel   = __le32_to_cpu(event->hdr.channel);
-	buf_len   = __le32_to_cpu(event->hdr.buf_len);
-	rx_status = __le32_to_cpu(event->hdr.status);
-	snr       = __le32_to_cpu(event->hdr.snr);
-	phy_mode  = __le32_to_cpu(event->hdr.phy_mode);
-	rate	  = __le32_to_cpu(event->hdr.rate);
+	channel   = __le32_to_cpu(ev_hdr->channel);
+	buf_len   = __le32_to_cpu(ev_hdr->buf_len);
+	rx_status = __le32_to_cpu(ev_hdr->status);
+	snr       = __le32_to_cpu(ev_hdr->snr);
+	phy_mode  = __le32_to_cpu(ev_hdr->phy_mode);
+	rate	  = __le32_to_cpu(ev_hdr->rate);
 
 	memset(status, 0, sizeof(*status));
 
@@ -358,7 +371,7 @@ static int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
 	status->signal = snr + ATH10K_DEFAULT_NOISE_FLOOR;
 	status->rate_idx = get_rate_idx(rate, status->band);
 
-	skb_pull(skb, sizeof(event->hdr));
+	skb_pull(skb, pull_len);
 
 	hdr = (struct ieee80211_hdr *)skb->data;
 	fc = le16_to_cpu(hdr->frame_control);
@@ -943,6 +956,9 @@ static void ath10k_wmi_service_ready_event_rx(struct ath10k *ar,
 	ar->phy_capability = __le32_to_cpu(ev->phy_capability);
 	ar->num_rf_chains = __le32_to_cpu(ev->num_rf_chains);
 
+	if (ar->fw_version_build > 636)
+		set_bit(ATH10K_FW_FEATURE_EXT_WMI_MGMT_RX, ar->fw_features);
+
 	if (ar->num_rf_chains > WMI_MAX_SPATIAL_STREAM) {
 		ath10k_warn("hardware advertises support for more spatial streams than it should (%d > %d)\n",
 			    ar->num_rf_chains, WMI_MAX_SPATIAL_STREAM);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 2c5a4f8..08860c4 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -1268,7 +1268,7 @@ struct wmi_scan_event {
  * good idea to pass all the fields in the RX status
  * descriptor up to the host.
  */
-struct wmi_mgmt_rx_hdr {
+struct wmi_mgmt_rx_hdr_v1 {
 	__le32 channel;
 	__le32 snr;
 	__le32 rate;
@@ -1277,8 +1277,18 @@ struct wmi_mgmt_rx_hdr {
 	__le32 status; /* %WMI_RX_STATUS_ */
 } __packed;
 
-struct wmi_mgmt_rx_event {
-	struct wmi_mgmt_rx_hdr hdr;
+struct wmi_mgmt_rx_hdr_v2 {
+	struct wmi_mgmt_rx_hdr_v1 v1;
+	__le32 rssi_ctl[4];
+} __packed;
+
+struct wmi_mgmt_rx_event_v1 {
+	struct wmi_mgmt_rx_hdr_v1 hdr;
+	u8 buf[0];
+} __packed;
+
+struct wmi_mgmt_rx_event_v2 {
+	struct wmi_mgmt_rx_hdr_v2 hdr;
 	u8 buf[0];
 } __packed;
 
-- 
1.7.9.5

[PATCH 3/3] ath10k: add support for HTT 3.0

[Michal Kazior]

New firmware comes with new HTT protocol version.
In 3.0 the separate mgmt tx command has been
removed. All traffic is to be pushed through data
tx (tx_frm) command with a twist - FW seems to not
be able (yet?) to access tx fragment table so for
manamgement frames frame pointer is passed
directly.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
patch:
 * use ath10k_info for htt version prompt (Kalle)
 * more verbose error message on htt failure (Kalle)
 * conditions tweaked and broken on 80 column (Kalle)
 * use brackets {}

 drivers/net/wireless/ath/ath10k/htt.c    |   19 +++-----
 drivers/net/wireless/ath/ath10k/htt.h    |    6 +--
 drivers/net/wireless/ath/ath10k/htt_tx.c |   74 +++++++++++++++++++++---------
 drivers/net/wireless/ath/ath10k/hw.h     |    3 ++
 drivers/net/wireless/ath/ath10k/mac.c    |   13 +++++-
 5 files changed, 76 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 39342c5..5f7eeeb 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -104,21 +104,16 @@ err_htc_attach:
 
 static int ath10k_htt_verify_version(struct ath10k_htt *htt)
 {
-	ath10k_dbg(ATH10K_DBG_HTT,
-		   "htt target version %d.%d; host version %d.%d\n",
-		    htt->target_version_major,
-		    htt->target_version_minor,
-		    HTT_CURRENT_VERSION_MAJOR,
-		    HTT_CURRENT_VERSION_MINOR);
-
-	if (htt->target_version_major != HTT_CURRENT_VERSION_MAJOR) {
-		ath10k_err("htt major versions are incompatible!\n");
+	ath10k_info("htt target version %d.%d\n",
+		    htt->target_version_major, htt->target_version_minor);
+
+	if (htt->target_version_major != 2 &&
+	    htt->target_version_major != 3) {
+		ath10k_err("unsupported htt major version %d. supported versions are 2 and 3\n",
+			   htt->target_version_major);
 		return -ENOTSUPP;
 	}
 
-	if (htt->target_version_minor != HTT_CURRENT_VERSION_MINOR)
-		ath10k_warn("htt minor version differ but still compatible\n");
-
 	return 0;
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index de45d02..2488623 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -23,9 +23,6 @@
 #include "htc.h"
 #include "rx_desc.h"
 
-#define HTT_CURRENT_VERSION_MAJOR	2
-#define HTT_CURRENT_VERSION_MINOR	1
-
 enum htt_dbg_stats_type {
 	HTT_DBG_STATS_WAL_PDEV_TXRX = 1 << 0,
 	HTT_DBG_STATS_RX_REORDER    = 1 << 1,
@@ -45,6 +42,9 @@ enum htt_h2t_msg_type { /* host-to-target */
 	HTT_H2T_MSG_TYPE_SYNC               = 4,
 	HTT_H2T_MSG_TYPE_AGGR_CFG           = 5,
 	HTT_H2T_MSG_TYPE_FRAG_DESC_BANK_CFG = 6,
+
+	/* This command is used for sending management frames in HTT < 3.0.
+	 * HTT >= 3.0 uses TX_FRM for everything. */
 	HTT_H2T_MSG_TYPE_MGMT_TX            = 7,
 
 	HTT_H2T_NUM_MSGS /* keep this last */
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 656c254..d4fb387 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -401,10 +401,16 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 		goto err;
 	}
 
-	txfrag = dev_alloc_skb(frag_len);
-	if (!txfrag) {
-		res = -ENOMEM;
-		goto err;
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (htt->target_version_major < 3 ||
+	    !ieee80211_is_mgmt(hdr->frame_control)) {
+		txfrag = dev_alloc_skb(frag_len);
+		if (!txfrag) {
+			res = -ENOMEM;
+			goto err;
+		}
 	}
 
 	if (!IS_ALIGNED((unsigned long)txdesc->data, 4)) {
@@ -427,23 +433,31 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 	if (res)
 		goto err;
 
-	/* tx fragment list must be terminated with zero-entry */
-	skb_put(txfrag, frag_len);
-	tx_frags = (struct htt_data_tx_desc_frag *)txfrag->data;
-	tx_frags[0].paddr = __cpu_to_le32(ATH10K_SKB_CB(msdu)->paddr);
-	tx_frags[0].len   = __cpu_to_le32(msdu->len);
-	tx_frags[1].paddr = __cpu_to_le32(0);
-	tx_frags[1].len   = __cpu_to_le32(0);
-
-	res = ath10k_skb_map(dev, txfrag);
-	if (res)
-		goto err;
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (htt->target_version_major < 3 ||
+	    !ieee80211_is_mgmt(hdr->frame_control)) {
+		/* tx fragment list must be terminated with zero-entry */
+		skb_put(txfrag, frag_len);
+		tx_frags = (struct htt_data_tx_desc_frag *)txfrag->data;
+		tx_frags[0].paddr = __cpu_to_le32(ATH10K_SKB_CB(msdu)->paddr);
+		tx_frags[0].len   = __cpu_to_le32(msdu->len);
+		tx_frags[1].paddr = __cpu_to_le32(0);
+		tx_frags[1].len   = __cpu_to_le32(0);
+
+		res = ath10k_skb_map(dev, txfrag);
+		if (res)
+			goto err;
+
+		ath10k_dbg(ATH10K_DBG_HTT, "txfrag 0x%llx\n",
+			   (unsigned long long) ATH10K_SKB_CB(txfrag)->paddr);
+		ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "txfrag: ",
+				txfrag->data, frag_len);
+	}
 
-	ath10k_dbg(ATH10K_DBG_HTT, "txfrag 0x%llx msdu 0x%llx\n",
-		   (unsigned long long) ATH10K_SKB_CB(txfrag)->paddr,
+	ath10k_dbg(ATH10K_DBG_HTT, "msdu 0x%llx\n",
 		   (unsigned long long) ATH10K_SKB_CB(msdu)->paddr);
-	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "txfrag: ",
-			txfrag->data, frag_len);
 	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "msdu: ",
 			msdu->data, msdu->len);
 
@@ -459,8 +473,17 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 	if (!ieee80211_has_protected(hdr->frame_control))
 		flags0 |= HTT_DATA_TX_DESC_FLAGS0_NO_ENCRYPT;
 	flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT;
-	flags0 |= SM(ATH10K_HW_TXRX_NATIVE_WIFI,
-		     HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
+
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (htt->target_version_major >= 3 &&
+	    ieee80211_is_mgmt(hdr->frame_control))
+		flags0 |= SM(ATH10K_HW_TXRX_MGMT,
+			     HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
+	else
+		flags0 |= SM(ATH10K_HW_TXRX_NATIVE_WIFI,
+			     HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
 
 	flags1  = 0;
 	flags1 |= SM((u16)vdev_id, HTT_DATA_TX_DESC_FLAGS1_VDEV_ID);
@@ -468,7 +491,14 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 	flags1 |= HTT_DATA_TX_DESC_FLAGS1_CKSUM_L3_OFFLOAD;
 	flags1 |= HTT_DATA_TX_DESC_FLAGS1_CKSUM_L4_OFFLOAD;
 
-	frags_paddr = ATH10K_SKB_CB(txfrag)->paddr;
+	/* Since HTT 3.0 there is no separate mgmt tx command. However in case
+	 * of mgmt tx using TX_FRM there is not tx fragment list. Instead of tx
+	 * fragment list host driver specifies directly frame pointer. */
+	if (htt->target_version_major >= 3 &&
+	    ieee80211_is_mgmt(hdr->frame_control))
+		frags_paddr = ATH10K_SKB_CB(msdu)->paddr;
+	else
+		frags_paddr = ATH10K_SKB_CB(txfrag)->paddr;
 
 	cmd->hdr.msg_type        = HTT_H2T_MSG_TYPE_TX_FRM;
 	cmd->data_tx.flags0      = flags0;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 44ed5af..9c5d6fc 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -53,6 +53,9 @@ enum ath10k_hw_txrx_mode {
 	ATH10K_HW_TXRX_RAW = 0,
 	ATH10K_HW_TXRX_NATIVE_WIFI = 1,
 	ATH10K_HW_TXRX_ETHERNET = 2,
+
+	/* Valid for HTT >= 3.0. Used for management frames in TX_FRM. */
+	ATH10K_HW_TXRX_MGMT = 3,
 };
 
 enum ath10k_mcast2ucast_mode {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index cc6ad7a..3598b48 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1480,6 +1480,12 @@ static void ath10k_tx_htt(struct ath10k *ar, struct sk_buff *skb)
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int ret;
 
+	if (ar->htt.target_version_major >= 3) {
+		/* Since HTT 3.0 there is no separate mgmt tx command */
+		ret = ath10k_htt_tx(&ar->htt, skb);
+		goto exit;
+	}
+
 	if (ieee80211_is_mgmt(hdr->frame_control))
 		ret = ath10k_htt_mgmt_tx(&ar->htt, skb);
 	else if (ieee80211_is_nullfunc(hdr->frame_control))
@@ -1491,6 +1497,7 @@ static void ath10k_tx_htt(struct ath10k *ar, struct sk_buff *skb)
 	else
 		ret = ath10k_htt_tx(&ar->htt, skb);
 
+exit:
 	if (ret) {
 		ath10k_warn("tx failed (%d). dropping packet.\n", ret);
 		ieee80211_free_txskb(ar->hw, skb);
@@ -1726,8 +1733,10 @@ static void ath10k_tx(struct ieee80211_hw *hw,
 
 	/* we must calculate tid before we apply qos workaround
 	 * as we'd lose the qos control field */
-	if (ieee80211_is_data_qos(hdr->frame_control) &&
-	    is_unicast_ether_addr(ieee80211_get_DA(hdr))) {
+	if (ieee80211_is_mgmt(hdr->frame_control)) {
+		tid = HTT_DATA_TX_EXT_TID_MGMT;
+	} else if (ieee80211_is_data_qos(hdr->frame_control) &&
+		   is_unicast_ether_addr(ieee80211_get_DA(hdr))) {
 		u8 *qc = ieee80211_get_qos_ctl(hdr);
 		tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
 	} else if (is_unicast_ether_addr(ieee80211_get_DA(hdr))) {
-- 
1.7.9.5

Re: [PATCH 3/3] ath10k: add support for HTT 3.0

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> New firmware comes with new HTT protocol version.
> In 3.0 the separate mgmt tx command has been
> removed. All traffic is to be pushed through data
> tx (tx_frm) command with a twist - FW seems to not
> be able (yet?) to access tx fragment table so for
> manamgement frames frame pointer is passed
> directly.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Because I dropped patch 1 this patch had conflicts. I tried to be extra
careful, but please double check my conflict resolution.

-- 
Kalle Valo

Re: [PATCH 3/3] ath10k: add support for HTT 3.0

[Michal Kazior]

On 15 August 2013 15:10, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> New firmware comes with new HTT protocol version.
>> In 3.0 the separate mgmt tx command has been
>> removed. All traffic is to be pushed through data
>> tx (tx_frm) command with a twist - FW seems to not
>> be able (yet?) to access tx fragment table so for
>> manamgement frames frame pointer is passed
>> directly.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> Because I dropped patch 1 this patch had conflicts. I tried to be extra
> careful, but please double check my conflict resolution.

Looks good, thanks!


Pozdrawiam / Best regards,
Michał Kazior.

Re: [PATCH 0/3] ath10k: firmware-related updates

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> Hi,
>
> This patchset contains 2 firmware-related upgrades
> and 1 fix (that I though is a good pick to include
> here, since it's related with HTT tx).
>
> This keeps backward compatbility with old
> firmware.
>
> This includes fixes for issues pointed out during
> RFC review.

[...]

> Michal Kazior (3):
>   ath10k: clean up HTT tx tid handling

I dropped this patch.

>   ath10k: add support for firmware newer than 636
>   ath10k: add support for HTT 3.0

These two are applied.

-- 
Kalle Valo