[PATCH] ath10k: set the mactime of ieee80211_rx_status

[PATCH] ath10k: set the mactime of ieee80211_rx_status

[Chun-Yeow Yeoh]

Retrieve the mactime of ieee80211_rx_status based on received
data frame. The value is obtained from the htt_rx_indication_ppdu
structure and only available in 32-bit.

Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htt.h    |    1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c |    1 +
 drivers/net/wireless/ath/ath10k/txrx.c   |    6 ++++++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index b93ae35..bf6b415 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1181,6 +1181,7 @@ struct htt_rx_info {
 		u32 info1;
 		u32 info2;
 	} rate;
+	u32 tsf;
 	bool fcs_err;
 	bool amsdu_more;
 	bool mic_err;
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 820c8ba..4ec2b5d 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -989,6 +989,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			info.rate.info0 = rx->ppdu.info0;
 			info.rate.info1 = __le32_to_cpu(rx->ppdu.info1);
 			info.rate.info2 = __le32_to_cpu(rx->ppdu.info2);
+			info.tsf = __le32_to_cpu(rx->ppdu.tsf);
 
 			hdr = ath10k_htt_rx_skb_get_hdr(msdu_head);
 
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index b11e478..4713bdb 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -258,6 +258,12 @@ void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info)
 	status->band = ch->band;
 	status->freq = ch->center_freq;
 
+	if (info->rate.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
+		/* TSF available only in 32-bit */
+		status->mactime = info->tsf & 0xffffffff;
+		status->flag |= RX_FLAG_MACTIME_END;
+	}
+
 	ath10k_dbg(ATH10K_DBG_DATA,
 		   "rx skb %p len %u %s%s%s%s%s %srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i\n",
 		   info->skb,
-- 
1.7.9.5

Re: [PATCH] ath10k: set the mactime of ieee80211_rx_status

[Kalle Valo]

Chun-Yeow Yeoh <yeohchunyeow@gmail.com> writes:

> Retrieve the mactime of ieee80211_rx_status based on received
> data frame. The value is obtained from the htt_rx_indication_ppdu
> structure and only available in 32-bit.
>
> Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com>

Why? Where do you need tsf exactly? And what bug are you actually fixing
here?

> --- a/drivers/net/wireless/ath/ath10k/txrx.c
> +++ b/drivers/net/wireless/ath/ath10k/txrx.c
> @@ -258,6 +258,12 @@ void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info)
>  	status->band = ch->band;
>  	status->freq = ch->center_freq;
>  
> +	if (info->rate.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
> +		/* TSF available only in 32-bit */
> +		status->mactime = info->tsf & 0xffffffff;
> +		status->flag |= RX_FLAG_MACTIME_END;
> +	}

Do we get some regressions because of proving only a 32 bit TSF? Which
one is better, provide a 32-bit TSF or not at all?

-- 
Kalle Valo

Re: [PATCH] ath10k: set the mactime of ieee80211_rx_status

[Yeoh Chun-Yeow]

> Why? Where do you need tsf exactly? And what bug are you actually fixing
> here?

There are two type of configuration modes that require local TSF, IBSS
and mesh for operation and also monitor mode.

For IBSS, it is use for merging of BSSID (mac address) with same SSID
name, but currently this is taking care by the following "ugly" patch.
http://lists.infradead.org/pipermail/ath10k/2014-February/001159.html

Mesh mode needs this for more accurate synchronization purpose.

Besides, the monitor mode requires this to add extra piece of
information in radiotap header for local TSF.

> Do we get some regressions because of proving only a 32 bit TSF? Which
> one is better, provide a 32-bit TSF or not at all?
32-bit is not good. It could cause problem when inter-operate with
other non-ath10k drivers with 64-bit local TSF. The better is that we
can get the 64-bit local TSF. providing high TSF and low TSF as found
in "struct wmi_comb_phyerr_rx_hd". Is this possible with current FW?

---
Chun-Yeow

Re: [PATCH] ath10k: set the mactime of ieee80211_rx_status

[Kalle Valo]

Yeoh Chun-Yeow <yeohchunyeow@gmail.com> writes:

>> Why? Where do you need tsf exactly? And what bug are you actually fixing
>> here?
>
> There are two type of configuration modes that require local TSF, IBSS
> and mesh for operation and also monitor mode.
>
> For IBSS, it is use for merging of BSSID (mac address) with same SSID
> name, but currently this is taking care by the following "ugly" patch.
> http://lists.infradead.org/pipermail/ath10k/2014-February/001159.html
>
> Mesh mode needs this for more accurate synchronization purpose.
>
> Besides, the monitor mode requires this to add extra piece of
> information in radiotap header for local TSF.
>
>> Do we get some regressions because of proving only a 32 bit TSF? Which
>> one is better, provide a 32-bit TSF or not at all?
>
> 32-bit is not good. It could cause problem when inter-operate with
> other non-ath10k drivers with 64-bit local TSF.

Yeah, I understand that. But my question is will we create regressions
if I apply this patch which provides only 32-bit TSF? I guess not as we
haven't provided TSF at all before, but I would like to be sure.

> The better is that we can get the 64-bit local TSF. providing high TSF
> and low TSF as found in "struct wmi_comb_phyerr_rx_hd". Is this
> possible with current FW?

I'm not familiar with the firmware so I sent a question to the firmware
team about this.

-- 
Kalle Valo

Re: [PATCH] ath10k: set the mactime of ieee80211_rx_status

[Yeoh Chun-Yeow]

On Thu, Feb 27, 2014 at 7:38 PM, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Yeoh Chun-Yeow <yeohchunyeow@gmail.com> writes:
>
>>> Why? Where do you need tsf exactly? And what bug are you actually fixing
>>> here?
>>
>> There are two type of configuration modes that require local TSF, IBSS
>> and mesh for operation and also monitor mode.
>>
>> For IBSS, it is use for merging of BSSID (mac address) with same SSID
>> name, but currently this is taking care by the following "ugly" patch.
>> http://lists.infradead.org/pipermail/ath10k/2014-February/001159.html
>>
>> Mesh mode needs this for more accurate synchronization purpose.
>>
>> Besides, the monitor mode requires this to add extra piece of
>> information in radiotap header for local TSF.
>>
>>> Do we get some regressions because of proving only a 32 bit TSF? Which
>>> one is better, provide a 32-bit TSF or not at all?
>>
>> 32-bit is not good. It could cause problem when inter-operate with
>> other non-ath10k drivers with 64-bit local TSF.
>
> Yeah, I understand that. But my question is will we create regressions
> if I apply this patch which provides only 32-bit TSF? I guess not as we
> haven't provided TSF at all before, but I would like to be sure.
>
>> The better is that we can get the 64-bit local TSF. providing high TSF
>> and low TSF as found in "struct wmi_comb_phyerr_rx_hd". Is this
>> possible with current FW?
>
> I'm not familiar with the firmware so I sent a question to the firmware
> team about this.

Great, appreciate if the FW team can take a look on this. Hope to hear
from you again on this.

----
Chun-Yeow

Re: [PATCH] ath10k: set the mactime of ieee80211_rx_status

[Kalle Valo]

Chun-Yeow Yeoh <yeohchunyeow@gmail.com> writes:

> Retrieve the mactime of ieee80211_rx_status based on received
> data frame. The value is obtained from the htt_rx_indication_ppdu
> structure and only available in 32-bit.
>
> Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow@gmail.com>

Thanks, applied.

If we learn better way to do this with full TSF, let's do that in a
followup patch.

-- 
Kalle Valo