[ath9k-devel] [PATCH 3/3] ath10k: enable STA UAPSD support

[Kalle Valo <kvalo@qca.qualcomm.com>]

Janusz Dziedzic <janusz.dziedzic@tieto.com> writes:

> Enable UAPSD support for STA mode.
>
> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>

Same here, please explain briefly what you have tested.

> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -2296,6 +2296,68 @@ static int ath10k_sta_state(struct ieee80211_hw *hw,
>  	return ret;
>  }
>  
> +#define SET_UAPSD(enable, out, flags) do {	\
> +	if ((enable))				\
> +		(out) |= (flags);		\
> +	else					\
> +		(out) &= ~(flags);		\
> +} while (0)

Why the macro? To workaround a long line warning from checkpatch?

If that's a problem we could increate line length limit, for example to
85 or 90. In some cases the 80 char limit is a bit too excessive. Would
that help?

[...]

> +	if (ret) {
> +		ath10k_warn("could not set uapsd params (%d)\n", ret);

"could not set uapsd params: %d\n"

> +		goto exit;
> +	}
> +
> +	if (arvif->u.sta.uapsd)
> +		value = WMI_STA_PS_RX_WAKE_POLICY_POLL_UAPSD;
> +	else
> +		value = WMI_STA_PS_RX_WAKE_POLICY_WAKE;
> +
> +	ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
> +					  WMI_STA_PS_PARAM_RX_WAKE_POLICY,
> +					  value);
> +	if (ret)
> +		ath10k_warn("could not set rx wake param (%d)\n", ret);
> +exit:
> +	return ret;
> +}

Empty line before the exit label.

> @@ -2335,13 +2399,20 @@ static int ath10k_conf_tx(struct ieee80211_hw *hw,
>  	 */
>  	p->txop = params->txop * 32;
>  
> -	/* FIXME: can we pass the params->uapsd to the FW? */
> +

This now has two empty lines, one is enough.

> -	if (ret)
> +	if (ret) {
>  		ath10k_warn("could not set wmm params (%d)\n", ret);
> +		goto exit;
> +	}

The same comment as with the other warning message above.

> +	ret = ath10k_conf_tx_uapsd(ar, vif, ac, params->uapsd);
> +	if (ret)
> +		ath10k_warn("could not set sta uapsd (%d)\n", ret);

Ditto.

-- 
Kalle Valo