[PATCH] ath10k: request firmware flush in ath10k_flush.

[PATCH] ath10k: request firmware flush in ath10k_flush.

[greearb]

From: Ben Greear <greearb@candelatech.com>

CT firmware now supports flushing all tids for all
peers for all vdevs.  This appears to help the ath10k_flush
logic work faster and not cause timeouts.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

This requires the previously-sent patch that enables the
ATH10K_FW_FEATURE_WMI_10X_CT feature bit.  This patch provide
a nice improvement in stability and flush performance in my
testing scenarios.

 drivers/net/wireless/ath/ath10k/mac.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index af24f6c..6014856 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3830,6 +3830,7 @@ static void ath10k_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	struct ath10k *ar = hw->priv;
 	bool skip;
 	int ret;
+	u8 peer_addr[ETH_ALEN] = {0};
 
 	/* mac80211 doesn't care if we really xmit queued frames or not
 	 * we'll collect those frames either way if we stop/delete vdevs */
@@ -3841,6 +3842,12 @@ static void ath10k_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (ar->state == ATH10K_STATE_WEDGED)
 		goto skip;
 
+	/* If we are CT firmware, ask it to flush all tids on all peers on
+	 * all vdevs.  Normal firmware will just crash if you do this.
+	 */
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
+		ath10k_wmi_peer_flush(ar, 0xFFFFFFFF, peer_addr, 0xFFFFFFFF);
+
 	ret = wait_event_timeout(ar->htt.empty_tx_wq, ({
 			bool empty;
 
-- 
1.7.11.7

Re: [PATCH] ath10k: request firmware flush in ath10k_flush.

[Michal Kazior]

On 19 September 2014 20:28,  <greearb@candelatech.com> wrote:
[...]
> +       /* If we are CT firmware, ask it to flush all tids on all peers on
> +        * all vdevs.  Normal firmware will just crash if you do this.
> +        */
> +       if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
> +               ath10k_wmi_peer_flush(ar, 0xFFFFFFFF, peer_addr, 0xFFFFFFFF);

I recall you've explained this some time ago, but can you refresh my
memory, please? Is this any different from iterating over all peers
and flushing each? Or does your firmware do so extra magic that is
impossible to do with normal firmware commands?


Michał

Re: [PATCH] ath10k: request firmware flush in ath10k_flush.

[Ben Greear]

On 09/23/2014 02:16 AM, Michal Kazior wrote:
> On 19 September 2014 20:28,  <greearb@candelatech.com> wrote:
> [...]
>> +       /* If we are CT firmware, ask it to flush all tids on all peers on
>> +        * all vdevs.  Normal firmware will just crash if you do this.
>> +        */
>> +       if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
>> +               ath10k_wmi_peer_flush(ar, 0xFFFFFFFF, peer_addr, 0xFFFFFFFF);
>
> I recall you've explained this some time ago, but can you refresh my
> memory, please? Is this any different from iterating over all peers
> and flushing each? Or does your firmware do so extra magic that is
> impossible to do with normal firmware commands?

My firmware does that iteration internally.

You could probably do that in the driver, but it would be a lot
of messages (for all vdevs, all peers, all tids)...
I was not sure if there were limits to the number
of commands you should attempt during the flush...

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] ath10k: request firmware flush in ath10k_flush.

[Michal Kazior]

On 23 September 2014 16:19, Ben Greear <greearb@candelatech.com> wrote:
> On 09/23/2014 02:16 AM, Michal Kazior wrote:
>> On 19 September 2014 20:28,  <greearb@candelatech.com> wrote:
>> [...]
>>>
>>> +       /* If we are CT firmware, ask it to flush all tids on all peers
>>> on
>>> +        * all vdevs.  Normal firmware will just crash if you do this.
>>> +        */
>>> +       if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
>>> +               ath10k_wmi_peer_flush(ar, 0xFFFFFFFF, peer_addr,
>>> 0xFFFFFFFF);
>>
>> I recall you've explained this some time ago, but can you refresh my
>> memory, please? Is this any different from iterating over all peers
>> and flushing each? Or does your firmware do so extra magic that is
>> impossible to do with normal firmware commands?
>
> My firmware does that iteration internally.
>
> You could probably do that in the driver, but it would be a lot
> of messages (for all vdevs, all peers, all tids)...
> I was not sure if there were limits to the number
> of commands you should attempt during the flush...

Thanks. I think ath10k should do this instead of having CT-specific
flush eventually.

I recall I actually did per-peer flushing in RFC patches a long time
ago but I didn't pursue getting them merged. I think Denton uses these
patches in his setup. They don't flush vdev self-peers though (do you
do that internally?).


Michał

Re: [PATCH] ath10k: request firmware flush in ath10k_flush.

[Ben Greear]

On 09/23/2014 11:50 PM, Michal Kazior wrote:
> On 23 September 2014 16:19, Ben Greear <greearb@candelatech.com> wrote:
>> On 09/23/2014 02:16 AM, Michal Kazior wrote:
>>> On 19 September 2014 20:28,  <greearb@candelatech.com> wrote:
>>> [...]
>>>>
>>>> +       /* If we are CT firmware, ask it to flush all tids on all peers
>>>> on
>>>> +        * all vdevs.  Normal firmware will just crash if you do this.
>>>> +        */
>>>> +       if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
>>>> +               ath10k_wmi_peer_flush(ar, 0xFFFFFFFF, peer_addr,
>>>> 0xFFFFFFFF);
>>>
>>> I recall you've explained this some time ago, but can you refresh my
>>> memory, please? Is this any different from iterating over all peers
>>> and flushing each? Or does your firmware do so extra magic that is
>>> impossible to do with normal firmware commands?
>>
>> My firmware does that iteration internally.
>>
>> You could probably do that in the driver, but it would be a lot
>> of messages (for all vdevs, all peers, all tids)...
>> I was not sure if there were limits to the number
>> of commands you should attempt during the flush...
> 
> Thanks. I think ath10k should do this instead of having CT-specific
> flush eventually.

Yes, though I would still like the optimization enabled if my
firmware is running...

> I recall I actually did per-peer flushing in RFC patches a long time
> ago but I didn't pursue getting them merged. I think Denton uses these
> patches in his setup. They don't flush vdev self-peers though (do you
> do that internally?).

I think I do.  Let me check, and send you some details privately.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] ath10k: request firmware flush in ath10k_flush.

[Kalle Valo]

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

> On 23 September 2014 16:19, Ben Greear <greearb@candelatech.com> wrote:
>> On 09/23/2014 02:16 AM, Michal Kazior wrote:
>>> On 19 September 2014 20:28,  <greearb@candelatech.com> wrote:
>>> [...]
>>>>
>>>> +       /* If we are CT firmware, ask it to flush all tids on all peers
>>>> on
>>>> +        * all vdevs.  Normal firmware will just crash if you do this.
>>>> +        */
>>>> +       if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
>>>> +               ath10k_wmi_peer_flush(ar, 0xFFFFFFFF, peer_addr,
>>>> 0xFFFFFFFF);
>>>
>>> I recall you've explained this some time ago, but can you refresh my
>>> memory, please? Is this any different from iterating over all peers
>>> and flushing each? Or does your firmware do so extra magic that is
>>> impossible to do with normal firmware commands?
>>
>> My firmware does that iteration internally.
>>
>> You could probably do that in the driver, but it would be a lot
>> of messages (for all vdevs, all peers, all tids)...
>> I was not sure if there were limits to the number
>> of commands you should attempt during the flush...
>
> Thanks. I think ath10k should do this instead of having CT-specific
> flush eventually.

I agree. We should not be forking functionality unless absolutely
necessary.

-- 
Kalle Valo

Re: [PATCH] ath10k: request firmware flush in ath10k_flush.

[Ben Greear]

On 09/29/2014 03:53 AM, Kalle Valo wrote:

>> Thanks. I think ath10k should do this instead of having CT-specific
>> flush eventually.
> 
> I agree. We should not be forking functionality unless absolutely
> necessary.

Ok, this patch is easy to carry in my own tree.

But, for things upstream firmware just cannot support,
like tx-rate-status and rx-software-crypt, how do you
want it flagged in the driver?  Can I get the CT firmware
flag in for that, or does it have to be some other way?

If you just will not accept any such patch, let me know so
I can quite bothering you about it.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com