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
