[PATCH 2/4] wcn36xx: Add TX ack support

[Kalle Valo]

Loic Poulain <loic.poulain at linaro.org> writes:

> The controller is capable of reporting TX indication which can be used
> to report TX ack. It was only partially implemented.
>
> Signed-off-by: Loic Poulain <loic.poulain at linaro.org>

[...]

> +static void wcn36xx_dxe_tx_timer(struct timer_list *t)
> +{
> +	struct wcn36xx *wcn = from_timer(wcn, t, tx_ack_timer);
> +	struct ieee80211_tx_info *info;
> +	unsigned long flags;
> +	struct sk_buff *skb;
> +
> +	/* TX Timeout */
> +	wcn36xx_dbg(WCN36XX_DBG_DXE, "TX timeout\n");
> +
> +	spin_lock_irqsave(&wcn->dxe_lock, flags);
> +	skb = wcn->tx_ack_skb;
> +	wcn->tx_ack_skb = NULL;
> +	spin_unlock_irqrestore(&wcn->dxe_lock, flags);
> +
> +	if (!skb)
> +		return;
> +
> +	info = IEEE80211_SKB_CB(skb);
> +	info->flags &= ~IEEE80211_TX_STAT_ACK;
> +	info->flags &= ~IEEE80211_TX_STAT_NOACK_TRANSMITTED;
> +
> +	ieee80211_tx_status_irqsafe(wcn->hw, skb);
> +	ieee80211_wake_queues(wcn->hw);
> +}

What's this timer thing? The commit log mentions nothing about that. To
me this looks like a some kind of TX timeout detection and has nothing
to do ACK handling, but of course I might have misunderstood.

Should it be in a separate patch to follow one logical change per patch
rule?

> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -45,8 +45,8 @@ static struct wcn36xx_cfg_val wcn36xx_cfg_vals[] = {
>  	WCN36XX_CFG_VAL(MAX_MEDIUM_TIME, 6000),
>  	WCN36XX_CFG_VAL(MAX_MPDUS_IN_AMPDU, 64),
>  	WCN36XX_CFG_VAL(RTS_THRESHOLD, 2347),
> -	WCN36XX_CFG_VAL(SHORT_RETRY_LIMIT, 6),
> -	WCN36XX_CFG_VAL(LONG_RETRY_LIMIT, 6),
> +	WCN36XX_CFG_VAL(SHORT_RETRY_LIMIT, 15),
> +	WCN36XX_CFG_VAL(LONG_RETRY_LIMIT, 15),

Also there's no mention of these changes in the commit log. Should these
in a third patch as they are more like a separate change?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches