Loic Poulain <loic.poulain@xxxxxxxxxx> 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@xxxxxxxxxx>
[...]
> +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
