Re: [PATCH v2 3/5] wifi: ath11k: use plain accesses for monitor descriptor

[Baochen Qiang <quic_bqiang@quicinc.com>]

On 6/4/2025 10:34 PM, Johan Hovold wrote:
> The read memory barrier added by commit ab52e3e44fe9 ("wifi: ath11k: fix
> rx completion meta data corruption") is enough to guarantee ordering
> also for plain descriptor accesses so drop the unnecessary READ_ONCE().
> 
> Tested-on: WCN6855 hw2.1 WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.41
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/net/wireless/ath/ath11k/dp_rx.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
> index d8dab182a9af..218ab41c0f3c 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
> @@ -2637,7 +2637,7 @@ int ath11k_dp_process_rx(struct ath11k_base *ab, int ring_id,
>  	struct ath11k *ar;
>  	struct hal_reo_dest_ring *desc;
>  	enum hal_reo_dest_ring_push_reason push_reason;
> -	u32 cookie, info0, rx_msdu_info0, rx_mpdu_info0;
> +	u32 cookie;
>  	int i;
>  
>  	for (i = 0; i < MAX_RADIOS; i++)
> @@ -2654,7 +2654,7 @@ int ath11k_dp_process_rx(struct ath11k_base *ab, int ring_id,
>  	      (struct hal_reo_dest_ring *)ath11k_hal_srng_dst_get_next_entry(ab,
>  									     srng))) {
>  		cookie = FIELD_GET(BUFFER_ADDR_INFO1_SW_COOKIE,
> -				   READ_ONCE(desc->buf_addr_info.info1));
> +				   desc->buf_addr_info.info1);
>  		buf_id = FIELD_GET(DP_RXDMA_BUF_COOKIE_BUF_ID,
>  				   cookie);
>  		mac_id = FIELD_GET(DP_RXDMA_BUF_COOKIE_PDEV_ID, cookie);
> @@ -2683,9 +2683,8 @@ int ath11k_dp_process_rx(struct ath11k_base *ab, int ring_id,
>  
>  		num_buffs_reaped[mac_id]++;
>  
> -		info0 = READ_ONCE(desc->info0);
>  		push_reason = FIELD_GET(HAL_REO_DEST_RING_INFO0_PUSH_REASON,
> -					info0);
> +					desc->info0);
>  		if (unlikely(push_reason !=
>  			     HAL_REO_DEST_RING_PUSH_REASON_ROUTING_INSTRUCTION)) {
>  			dev_kfree_skb_any(msdu);
> @@ -2693,21 +2692,18 @@ int ath11k_dp_process_rx(struct ath11k_base *ab, int ring_id,
>  			continue;
>  		}
>  
> -		rx_msdu_info0 = READ_ONCE(desc->rx_msdu_info.info0);
> -		rx_mpdu_info0 = READ_ONCE(desc->rx_mpdu_info.info0);
> -
> -		rxcb->is_first_msdu = !!(rx_msdu_info0 &
> +		rxcb->is_first_msdu = !!(desc->rx_msdu_info.info0 &
>  					 RX_MSDU_DESC_INFO0_FIRST_MSDU_IN_MPDU);
> -		rxcb->is_last_msdu = !!(rx_msdu_info0 &
> +		rxcb->is_last_msdu = !!(desc->rx_msdu_info.info0 &
>  					RX_MSDU_DESC_INFO0_LAST_MSDU_IN_MPDU);
> -		rxcb->is_continuation = !!(rx_msdu_info0 &
> +		rxcb->is_continuation = !!(desc->rx_msdu_info.info0 &
>  					   RX_MSDU_DESC_INFO0_MSDU_CONTINUATION);
>  		rxcb->peer_id = FIELD_GET(RX_MPDU_DESC_META_DATA_PEER_ID,
> -					  READ_ONCE(desc->rx_mpdu_info.meta_data));
> +					  desc->rx_mpdu_info.meta_data);
>  		rxcb->seq_no = FIELD_GET(RX_MPDU_DESC_INFO0_SEQ_NUM,
> -					 rx_mpdu_info0);
> +					 desc->rx_mpdu_info.info0);
>  		rxcb->tid = FIELD_GET(HAL_REO_DEST_RING_INFO0_RX_QUEUE_NUM,
> -				      info0);
> +				      desc->info0);
>  
>  		rxcb->mac_id = mac_id;
>  		__skb_queue_tail(&msdu_list[mac_id], msdu);

Reviewed-by: Baochen Qiang <quic_bqiang@quicinc.com>