[PATCH v6 1/3] wifi: ath12k: report station mode transmit rate

[Jeff Johnson]

On 7/31/2024 4:36 AM, Lingbo Kong wrote:
...
> +static void ath12k_dp_tx_update_txcompl(struct ath12k *ar, struct hal_tx_status *ts)
> +{
...
> +
> +	if (ts->pkt_type == HAL_TX_RATE_STATS_PKT_TYPE_11A ||
> +	    ts->pkt_type == HAL_TX_RATE_STATS_PKT_TYPE_11B) {

I think this if else if else if else if would this be clearer as:

	switch (ts->pkt_type) {
	case HAL_TX_RATE_STATS_PKT_TYPE_11A:
	case HAL_TX_RATE_STATS_PKT_TYPE_11B:
		...
		break;
	case HAL_TX_RATE_STATS_PKT_TYPE_11N:
		...
		break;

...

> @@ -610,10 +723,23 @@ static void ath12k_dp_tx_status_parse(struct ath12k_base *ab,
>  
>  	ts->ppdu_id = le32_get_bits(desc->info1,
>  				    HAL_WBM_COMPL_TX_INFO1_TQM_STATUS_NUMBER);
> -	if (le32_to_cpu(desc->rate_stats.info0) & HAL_TX_RATE_STATS_INFO0_VALID)
> -		ts->rate_stats = le32_to_cpu(desc->rate_stats.info0);
> -	else
> -		ts->rate_stats = 0;
> +
> +	ts->peer_id = le32_get_bits(desc->info3, HAL_WBM_COMPL_TX_INFO3_PEER_ID);
> +
> +	if (le32_to_cpu(desc->rate_stats.info0) & HAL_TX_RATE_STATS_INFO0_VALID) {

since you are extracting multiple fields from info0, suggest you have a local
u32 info0 = le32_to_cpu(desc->rate_stats.info0) and then use u32_get_bits()
everywhere. note this will have no impact on LE systems but would potentially
be better for BE systems.

> +		ts->pkt_type = le32_get_bits(desc->rate_stats.info0,
> +					     HAL_TX_RATE_STATS_INFO0_PKT_TYPE);
> +		ts->mcs = le32_get_bits(desc->rate_stats.info0,
> +					HAL_TX_RATE_STATS_INFO0_MCS);
> +		ts->sgi = le32_get_bits(desc->rate_stats.info0,
> +					HAL_TX_RATE_STATS_INFO0_SGI);
> +		ts->bw = le32_get_bits(desc->rate_stats.info0,
> +				       HAL_TX_RATE_STATS_INFO0_BW);
> +		ts->tones = le32_get_bits(desc->rate_stats.info0,
> +					  HAL_TX_RATE_STATS_INFO0_TONES_IN_RU);
> +		ts->ofdma = le32_get_bits(desc->rate_stats.info0,
> +					  HAL_TX_RATE_STATS_INFO0_OFDMA_TX);
> +	}
>  }

/jeff