[PATCHv5] wifi: ath11k: skip status ring entry processing

Commit Message

Tamizh Chelvam Raja April 29, 2024, 7:36 a.m. UTC

From: Venkateswara Naralasetty <[e-mail]>

If STATUS_BUFFER_DONE is not set for a monitor status ring entry,
we don't process the status ring until STATUS_BUFFER_DONE set
for that status ring entry.

During LMAC reset it may happen that hardware will not write
STATUS_BUFFER_DONE tlv in status buffer, in that case we end up
waiting for STATUS_BUFFER_DONE leading to backpressure on monitor
status ring.

To fix the issue, when HP(Head Pointer) + 1 entry is peeked and if DMA
is not done and if HP + 2 entry's DMA done is set,
replenish HP + 1 entry and start processing in next interrupt.
If HP + 2 entry's DMA done is not set, poll onto HP + 1 entry DMA
done to be set.

Also, during monitor attach HP points to the end of the ring and
TP(Tail Pointer) points to the start of the ring.
Using ath11k_hal_srng_src_peek() may result in processing invalid buffer
for the very first interrupt. Since, HW starts writing buffer from TP.

To avoid this issue call ath11k_hal_srng_src_next_peek() instead of
calling ath11k_hal_srng_src_peek().

Tested-on: IPQ5018 hw1.0 AHB WLAN.HK.2.6.0.1-00861-QCAHKSWPL_SILICONZ-1

Signed-off-by: Venkateswara Naralasetty <[e-mail]>
Co-developed-by: Tamizh Chelvam Raja <[e-mail]>
Signed-off-by: Tamizh Chelvam Raja <[e-mail]>
---
v5:
  * Updated copyright info in dp_rx.c and hal.c
v4:
  * Removed kernel reported compilation warning from Signed off place
v3:
  * Rebased on top of ToT
v2:
  * Fixed compilation warning Reported-by: kernel test robot <[e-mail]>

 drivers/net/wireless/ath/ath11k/dp_rx.c | 90 ++++++++++++++++++++++---
 drivers/net/wireless/ath/ath11k/hal.c   | 16 ++++-
 drivers/net/wireless/ath/ath11k/hal.h   |  2 +
 3 files changed, 96 insertions(+), 12 deletions(-)


base-commit: bf99bc7423e18aa3475ef00a7a6fb773c31ce6df

Comments

Jeff Johnson April 29, 2024, 6:12 p.m. UTC | #1

On 4/29/2024 12:36 AM, Tamizh Chelvam Raja wrote:
> From: Venkateswara Naralasetty <[e-mail]>
> 
> If STATUS_BUFFER_DONE is not set for a monitor status ring entry,
> we don't process the status ring until STATUS_BUFFER_DONE set
> for that status ring entry.
> 
> During LMAC reset it may happen that hardware will not write
> STATUS_BUFFER_DONE tlv in status buffer, in that case we end up
> waiting for STATUS_BUFFER_DONE leading to backpressure on monitor
> status ring.
> 
> To fix the issue, when HP(Head Pointer) + 1 entry is peeked and if DMA
> is not done and if HP + 2 entry's DMA done is set,
> replenish HP + 1 entry and start processing in next interrupt.
> If HP + 2 entry's DMA done is not set, poll onto HP + 1 entry DMA
> done to be set.
> 
> Also, during monitor attach HP points to the end of the ring and
> TP(Tail Pointer) points to the start of the ring.
> Using ath11k_hal_srng_src_peek() may result in processing invalid buffer
> for the very first interrupt. Since, HW starts writing buffer from TP.
> 
> To avoid this issue call ath11k_hal_srng_src_next_peek() instead of
> calling ath11k_hal_srng_src_peek().
> 
> Tested-on: IPQ5018 hw1.0 AHB WLAN.HK.2.6.0.1-00861-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Venkateswara Naralasetty <[e-mail]>
> Co-developed-by: Tamizh Chelvam Raja <[e-mail]>
> Signed-off-by: Tamizh Chelvam Raja <[e-mail]>

Acked-by: Jeff Johnson <[e-mail]>

however note...

> +
> +				/* If done status is missing:
> +				 * 1. As per MAC team's suggestion,
> +				 *    when HP + 1 entry is peeked and if DMA
> +				 *    is not done and if HP + 2 entry's DMA done
> +				 *    is set. skip HP + 1 entry and
> +				 *    start processing in next interrupt.
> +				 * 2. If HP + 2 entry's DMA done is not set,
> +				 *    poll onto HP + 1 entry DMA done to be set.
> +				 *    Check status for same buffer for next time
> +				 *    dp_rx_mon_status_srng_process
> +				 */
> +
> +				reap_status = ath11k_dp_rx_mon_handle_status_buf_done(ab, srng,
> +										      rx_ring);

ath11k-check reports:

drivers/net/wireless/ath/ath11k/dp_rx.c:3116: line length of 95 exceeds 90 columns
drivers/net/wireless/ath/ath11k/dp_rx.c:3117: line length of 95 exceeds 90 columns

Kalle, in this case we may want to make an exception since I don't think there
is a clean way to fix this other than refactoring.

FWIW I'd like to see this function refactored to avoid the excessive
indentation, but that should be a separate exercise.

/jeff

Kalle Valo April 30, 2024, 1:48 p.m. UTC | #2

Jeff Johnson <[e-mail]> writes:

> On 4/29/2024 12:36 AM, Tamizh Chelvam Raja wrote:
>
>> From: Venkateswara Naralasetty <[e-mail]>
>> 
>> If STATUS_BUFFER_DONE is not set for a monitor status ring entry,
>> we don't process the status ring until STATUS_BUFFER_DONE set
>> for that status ring entry.
>> 
>> During LMAC reset it may happen that hardware will not write
>> STATUS_BUFFER_DONE tlv in status buffer, in that case we end up
>> waiting for STATUS_BUFFER_DONE leading to backpressure on monitor
>> status ring.
>> 
>> To fix the issue, when HP(Head Pointer) + 1 entry is peeked and if DMA
>> is not done and if HP + 2 entry's DMA done is set,
>> replenish HP + 1 entry and start processing in next interrupt.
>> If HP + 2 entry's DMA done is not set, poll onto HP + 1 entry DMA
>> done to be set.
>> 
>> Also, during monitor attach HP points to the end of the ring and
>> TP(Tail Pointer) points to the start of the ring.
>> Using ath11k_hal_srng_src_peek() may result in processing invalid buffer
>> for the very first interrupt. Since, HW starts writing buffer from TP.
>> 
>> To avoid this issue call ath11k_hal_srng_src_next_peek() instead of
>> calling ath11k_hal_srng_src_peek().
>> 
>> Tested-on: IPQ5018 hw1.0 AHB WLAN.HK.2.6.0.1-00861-QCAHKSWPL_SILICONZ-1
>> 
>> Signed-off-by: Venkateswara Naralasetty <[e-mail]>
>> Co-developed-by: Tamizh Chelvam Raja <[e-mail]>
>> Signed-off-by: Tamizh Chelvam Raja <[e-mail]>
>
> Acked-by: Jeff Johnson <[e-mail]>
>
> however note...
>
>> +
>> +				/* If done status is missing:
>> +				 * 1. As per MAC team's suggestion,
>> +				 *    when HP + 1 entry is peeked and if DMA
>> +				 *    is not done and if HP + 2 entry's DMA done
>> +				 *    is set. skip HP + 1 entry and
>> +				 *    start processing in next interrupt.
>> +				 * 2. If HP + 2 entry's DMA done is not set,
>> +				 *    poll onto HP + 1 entry DMA done to be set.
>> +				 *    Check status for same buffer for next time
>> +				 *    dp_rx_mon_status_srng_process
>> +				 */
>> +
>> + reap_status = ath11k_dp_rx_mon_handle_status_buf_done(ab, srng,
>> + rx_ring);
>
> ath11k-check reports:
>
> drivers/net/wireless/ath/ath11k/dp_rx.c:3116: line length of 95 exceeds 90 columns
> drivers/net/wireless/ath/ath11k/dp_rx.c:3117: line length of 95 exceeds 90 columns

Tamizh, please ALWAYS run ath11k-check. We are wasting time for trivial
stuff like this.

> Kalle, in this case we may want to make an exception since I don't think there
> is a clean way to fix this other than refactoring.

The new function name looked quite long so I shortened it to
ath11k_dp_rx_mon_buf_done() and the warning is now gone. Does that look
reasonable name?

Also I removed one unrelated change and removed unnecessary else. Please
check my changes:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6e88d559268779107715008c51e006f7a5f62045

But now I noticed that the warning message needs some work:

ath11k_warn(ab, "mon status DONE not set %lx, buf_id %d\n",
	    FIELD_GET(HAL_TLV_HDR_TAG, tlv->tl),
	    buf_id);

Please use understandable warning and error messages in plain english.
Any suggestions? I can edit the commit in the pending branch.

> FWIW I'd like to see this function refactored to avoid the excessive
> indentation, but that should be a separate exercise.

Indeed, the indentation in this function is getting close to the limit.

Tamizh Chelvam Raja April 30, 2024, 2:44 p.m. UTC | #3

On 4/30/2024 7:18 PM, Kalle Valo wrote:
> Jeff Johnson <[e-mail]> writes:
> 
>> On 4/29/2024 12:36 AM, Tamizh Chelvam Raja wrote:
>>
>>> From: Venkateswara Naralasetty <[e-mail]>
>>>
>>> If STATUS_BUFFER_DONE is not set for a monitor status ring entry,
>>> we don't process the status ring until STATUS_BUFFER_DONE set
>>> for that status ring entry.
>>>
>>> During LMAC reset it may happen that hardware will not write
>>> STATUS_BUFFER_DONE tlv in status buffer, in that case we end up
>>> waiting for STATUS_BUFFER_DONE leading to backpressure on monitor
>>> status ring.
>>>
>>> To fix the issue, when HP(Head Pointer) + 1 entry is peeked and if DMA
>>> is not done and if HP + 2 entry's DMA done is set,
>>> replenish HP + 1 entry and start processing in next interrupt.
>>> If HP + 2 entry's DMA done is not set, poll onto HP + 1 entry DMA
>>> done to be set.
>>>
>>> Also, during monitor attach HP points to the end of the ring and
>>> TP(Tail Pointer) points to the start of the ring.
>>> Using ath11k_hal_srng_src_peek() may result in processing invalid buffer
>>> for the very first interrupt. Since, HW starts writing buffer from TP.
>>>
>>> To avoid this issue call ath11k_hal_srng_src_next_peek() instead of
>>> calling ath11k_hal_srng_src_peek().
>>>
>>> Tested-on: IPQ5018 hw1.0 AHB WLAN.HK.2.6.0.1-00861-QCAHKSWPL_SILICONZ-1
>>>
>>> Signed-off-by: Venkateswara Naralasetty <[e-mail]>
>>> Co-developed-by: Tamizh Chelvam Raja <[e-mail]>
>>> Signed-off-by: Tamizh Chelvam Raja <[e-mail]>
>>
>> Acked-by: Jeff Johnson <[e-mail]>
>>
>> however note...
>>
>>> +
>>> +				/* If done status is missing:
>>> +				 * 1. As per MAC team's suggestion,
>>> +				 *    when HP + 1 entry is peeked and if DMA
>>> +				 *    is not done and if HP + 2 entry's DMA done
>>> +				 *    is set. skip HP + 1 entry and
>>> +				 *    start processing in next interrupt.
>>> +				 * 2. If HP + 2 entry's DMA done is not set,
>>> +				 *    poll onto HP + 1 entry DMA done to be set.
>>> +				 *    Check status for same buffer for next time
>>> +				 *    dp_rx_mon_status_srng_process
>>> +				 */
>>> +
>>> + reap_status = ath11k_dp_rx_mon_handle_status_buf_done(ab, srng,
>>> + rx_ring);
>>
>> ath11k-check reports:
>>
>> drivers/net/wireless/ath/ath11k/dp_rx.c:3116: line length of 95 exceeds 90 columns
>> drivers/net/wireless/ath/ath11k/dp_rx.c:3117: line length of 95 exceeds 90 columns
> 
> Tamizh, please ALWAYS run ath11k-check. We are wasting time for trivial
> stuff like this.
Sure Kalle.
> 
>> Kalle, in this case we may want to make an exception since I don't think there
>> is a clean way to fix this other than refactoring.
> 
> The new function name looked quite long so I shortened it to
> ath11k_dp_rx_mon_buf_done() and the warning is now gone. Does that look
> reasonable name?
> 
> Also I removed one unrelated change and removed unnecessary else. Please
> check my changes:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6e88d559268779107715008c51e006f7a5f62045
Change looks good!
> 
> But now I noticed that the warning message needs some work:
> 
> ath11k_warn(ab, "mon status DONE not set %lx, buf_id %d\n",
> 	    FIELD_GET(HAL_TLV_HDR_TAG, tlv->tl),
> 	    buf_id);
> 

> Please use understandable warning and error messages in plain english.
> Any suggestions? I can edit the commit in the pending branch.
>
"hal rx status buffer done is not set in tlv %lx buf_id %d" -> is this fine?
 
>> FWIW I'd like to see this function refactored to avoid the excessive
>> indentation, but that should be a separate exercise.
> 
> Indeed, the indentation in this function is getting close to the limit.
> 


Tamizh

Jeff Johnson April 30, 2024, 3:14 p.m. UTC | #4

On 4/30/2024 6:48 AM, Kalle Valo wrote:
> Jeff Johnson <[e-mail]> writes:
> 
>> On 4/29/2024 12:36 AM, Tamizh Chelvam Raja wrote:
>>
>>> From: Venkateswara Naralasetty <[e-mail]>
>>>
>>> If STATUS_BUFFER_DONE is not set for a monitor status ring entry,
>>> we don't process the status ring until STATUS_BUFFER_DONE set
>>> for that status ring entry.
>>>
>>> During LMAC reset it may happen that hardware will not write
>>> STATUS_BUFFER_DONE tlv in status buffer, in that case we end up
>>> waiting for STATUS_BUFFER_DONE leading to backpressure on monitor
>>> status ring.
>>>
>>> To fix the issue, when HP(Head Pointer) + 1 entry is peeked and if DMA
>>> is not done and if HP + 2 entry's DMA done is set,
>>> replenish HP + 1 entry and start processing in next interrupt.
>>> If HP + 2 entry's DMA done is not set, poll onto HP + 1 entry DMA
>>> done to be set.
>>>
>>> Also, during monitor attach HP points to the end of the ring and
>>> TP(Tail Pointer) points to the start of the ring.
>>> Using ath11k_hal_srng_src_peek() may result in processing invalid buffer
>>> for the very first interrupt. Since, HW starts writing buffer from TP.
>>>
>>> To avoid this issue call ath11k_hal_srng_src_next_peek() instead of
>>> calling ath11k_hal_srng_src_peek().
>>>
>>> Tested-on: IPQ5018 hw1.0 AHB WLAN.HK.2.6.0.1-00861-QCAHKSWPL_SILICONZ-1
>>>
>>> Signed-off-by: Venkateswara Naralasetty <[e-mail]>
>>> Co-developed-by: Tamizh Chelvam Raja <[e-mail]>
>>> Signed-off-by: Tamizh Chelvam Raja <[e-mail]>
>>
>> Acked-by: Jeff Johnson <[e-mail]>
>>
>> however note...
>>
>>> +
>>> +				/* If done status is missing:
>>> +				 * 1. As per MAC team's suggestion,
>>> +				 *    when HP + 1 entry is peeked and if DMA
>>> +				 *    is not done and if HP + 2 entry's DMA done
>>> +				 *    is set. skip HP + 1 entry and
>>> +				 *    start processing in next interrupt.
>>> +				 * 2. If HP + 2 entry's DMA done is not set,
>>> +				 *    poll onto HP + 1 entry DMA done to be set.
>>> +				 *    Check status for same buffer for next time
>>> +				 *    dp_rx_mon_status_srng_process
>>> +				 */
>>> +
>>> + reap_status = ath11k_dp_rx_mon_handle_status_buf_done(ab, srng,
>>> + rx_ring);
>>
>> ath11k-check reports:
>>
>> drivers/net/wireless/ath/ath11k/dp_rx.c:3116: line length of 95 exceeds 90 columns
>> drivers/net/wireless/ath/ath11k/dp_rx.c:3117: line length of 95 exceeds 90 columns
> 
> Tamizh, please ALWAYS run ath11k-check. We are wasting time for trivial
> stuff like this.
> 
>> Kalle, in this case we may want to make an exception since I don't think there
>> is a clean way to fix this other than refactoring.
> 
> The new function name looked quite long so I shortened it to
> ath11k_dp_rx_mon_buf_done() and the warning is now gone. Does that look
> reasonable name?
> 
> Also I removed one unrelated change and removed unnecessary else. Please
> check my changes:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6e88d559268779107715008c51e006f7a5f62045

So looking at the 'pending' change I have the observation that
ath11k_dp_rx_mon_buf_done() only returns one of two values:
DP_MON_STATUS_NO_DMA
DP_MON_STATUS_REPLINISH

And the return value handling has explicit handling for those values, without
any logic for other values:
+				if (reap_status == DP_MON_STATUS_NO_DMA)
+					continue;
+
+				if (reap_status == DP_MON_STATUS_REPLINISH) {

if we only expect these two values to ever be returned, then we could remove
the testing for DP_MON_STATUS_REPLINISH since, it it isn't NO_DMA then it must
be REPLINISH

Also after this patch is merged, can we have a spelling correcting patch:
s/REPLINISH/REPLENISH/

+					ath11k_warn(ab, "mon status DONE not set %lx, buf_id %d\n",
+						    FIELD_GET(HAL_TLV_HDR_TAG, tlv->tl),
+						    buf_id);

I don't think we should log anything here. we already warn before calling the
new function. if we get here it means the next buffer had DONE set so we can
replenish the current buffer

Kalle Valo May 3, 2024, 1:11 p.m. UTC | #5

Jeff Johnson <[e-mail]> writes:

>> Also I removed one unrelated change and removed unnecessary else. Please
>> check my changes:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6e88d559268779107715008c51e006f7a5f62045
>
> So looking at the 'pending' change I have the observation that
> ath11k_dp_rx_mon_buf_done() only returns one of two values:
> DP_MON_STATUS_NO_DMA
> DP_MON_STATUS_REPLINISH
>
> And the return value handling has explicit handling for those values, without
> any logic for other values:
> +				if (reap_status == DP_MON_STATUS_NO_DMA)
> +					continue;
> +
> +				if (reap_status == DP_MON_STATUS_REPLINISH) {
>
> if we only expect these two values to ever be returned, then we could remove
> the testing for DP_MON_STATUS_REPLINISH since, it it isn't NO_DMA then it must
> be REPLINISH

Nice, I simplified this error handling now. Less indentation now which
is much better.

> + ath11k_warn(ab, "mon status DONE not set %lx, buf_id %d\n",
> + FIELD_GET(HAL_TLV_HDR_TAG, tlv->tl),
> +						    buf_id);
>
> I don't think we should log anything here. we already warn before calling the
> new function. if we get here it means the next buffer had DONE set so we can
> replenish the current buffer

Yeah, I removed the warning altogether. Please check my changes:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=49c28a9720959fb5daf702fe1732a716f3cff15c

Jeff Johnson May 3, 2024, 2:55 p.m. UTC | #6

On 5/3/2024 6:11 AM, Kalle Valo wrote:
> Jeff Johnson <[e-mail]> writes:
> 
>>> Also I removed one unrelated change and removed unnecessary else. Please
>>> check my changes:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6e88d559268779107715008c51e006f7a5f62045
>>
>> So looking at the 'pending' change I have the observation that
>> ath11k_dp_rx_mon_buf_done() only returns one of two values:
>> DP_MON_STATUS_NO_DMA
>> DP_MON_STATUS_REPLINISH
>>
>> And the return value handling has explicit handling for those values, without
>> any logic for other values:
>> +				if (reap_status == DP_MON_STATUS_NO_DMA)
>> +					continue;
>> +
>> +				if (reap_status == DP_MON_STATUS_REPLINISH) {
>>
>> if we only expect these two values to ever be returned, then we could remove
>> the testing for DP_MON_STATUS_REPLINISH since, it it isn't NO_DMA then it must
>> be REPLINISH
> 
> Nice, I simplified this error handling now. Less indentation now which
> is much better.
> 
>> + ath11k_warn(ab, "mon status DONE not set %lx, buf_id %d\n",
>> + FIELD_GET(HAL_TLV_HDR_TAG, tlv->tl),
>> +						    buf_id);
>>
>> I don't think we should log anything here. we already warn before calling the
>> new function. if we get here it means the next buffer had DONE set so we can
>> replenish the current buffer
> 
> Yeah, I removed the warning altogether. Please check my changes:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=49c28a9720959fb5daf702fe1732a716f3cff15c
> 
LGTM, thanks!

Kalle Valo May 7, 2024, 10 a.m. UTC | #7

Tamizh Chelvam Raja <[e-mail]> wrote:

> If STATUS_BUFFER_DONE is not set for a monitor status ring entry,
> we don't process the status ring until STATUS_BUFFER_DONE set
> for that status ring entry.
> 
> During LMAC reset it may happen that hardware will not write
> STATUS_BUFFER_DONE tlv in status buffer, in that case we end up
> waiting for STATUS_BUFFER_DONE leading to backpressure on monitor
> status ring.
> 
> To fix the issue, when HP (Head Pointer) + 1 entry is peeked and if DMA is not
> done and if HP + 2 entry's DMA done is set, replenish HP + 1 entry and start
> processing in next interrupt. If HP + 2 entry's DMA done is not set, poll onto
> HP + 1 entry DMA done to be set.
> 
> Also, during monitor attach HP points to the end of the ring and TP (Tail
> Pointer) points to the start of the ring.  Using ath11k_hal_srng_src_peek() may
> result in processing invalid buffer for the very first interrupt. Since, HW
> starts writing buffer from TP.
> 
> To avoid this issue call ath11k_hal_srng_src_next_peek() instead of
> calling ath11k_hal_srng_src_peek().
> 
> Tested-on: IPQ5018 hw1.0 AHB WLAN.HK.2.6.0.1-00861-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Venkateswara Naralasetty <[e-mail]>
> Co-developed-by: Tamizh Chelvam Raja <[e-mail]>
> Signed-off-by: Tamizh Chelvam Raja <[e-mail]>
> Signed-off-by: Kalle Valo <[e-mail]>

Patch applied to ath-next branch of ath.git, thanks.

4c2b796be3a1 wifi: ath11k: skip status ring entry processing

Patch

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index afd481f5858f..e9919a2ef4fb 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: BSD-3-Clause-Clear
 /*
  * Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #include <linux/ieee80211.h>
@@ -2990,11 +2990,52 @@  ath11k_dp_rx_mon_update_status_buf_state(struct ath11k_mon_data *pmon,
 	}
 }
 
+static enum dp_mon_status_buf_state
+ath11k_dp_rx_mon_handle_status_buf_done(struct ath11k_base *ab, struct hal_srng *srng,
+					struct dp_rxdma_ring *rx_ring)
+{
+	struct ath11k_skb_rxcb *rxcb;
+	struct hal_tlv_hdr *tlv;
+	struct sk_buff *skb;
+	void *status_desc;
+	dma_addr_t paddr;
+	u32 cookie;
+	int buf_id;
+	u8 rbm;
+
+	status_desc = ath11k_hal_srng_src_next_peek(ab, srng);
+	if (!status_desc)
+		return DP_MON_STATUS_NO_DMA;
+
+	ath11k_hal_rx_buf_addr_info_get(status_desc, &paddr, &cookie, &rbm);
+
+	buf_id = FIELD_GET(DP_RXDMA_BUF_COOKIE_BUF_ID, cookie);
+
+	spin_lock_bh(&rx_ring->idr_lock);
+	skb = idr_find(&rx_ring->bufs_idr, buf_id);
+	spin_unlock_bh(&rx_ring->idr_lock);
+
+	if (!skb)
+		return DP_MON_STATUS_NO_DMA;
+
+	rxcb = ATH11K_SKB_RXCB(skb);
+	dma_sync_single_for_cpu(ab->dev, rxcb->paddr,
+				skb->len + skb_tailroom(skb),
+				DMA_FROM_DEVICE);
+
+	tlv = (struct hal_tlv_hdr *)skb->data;
+	if (FIELD_GET(HAL_TLV_HDR_TAG, tlv->tl) != HAL_RX_STATUS_BUFFER_DONE)
+		return DP_MON_STATUS_NO_DMA;
+
+	return DP_MON_STATUS_REPLINISH;
+}
+
 static int ath11k_dp_rx_reap_mon_status_ring(struct ath11k_base *ab, int mac_id,
 					     int *budget, struct sk_buff_head *skb_list)
 {
 	struct ath11k *ar;
 	const struct ath11k_hw_hal_params *hal_params;
+	enum dp_mon_status_buf_state reap_status;
 	struct ath11k_pdev_dp *dp;
 	struct dp_rxdma_ring *rx_ring;
 	struct ath11k_mon_data *pmon;
@@ -3022,8 +3063,7 @@  static int ath11k_dp_rx_reap_mon_status_ring(struct ath11k_base *ab, int mac_id,
 	ath11k_hal_srng_access_begin(ab, srng);
 	while (*budget) {
 		*budget -= 1;
-		rx_mon_status_desc =
-			ath11k_hal_srng_src_peek(ab, srng);
+		rx_mon_status_desc = ath11k_hal_srng_src_peek(ab, srng);
 		if (!rx_mon_status_desc) {
 			pmon->buf_state = DP_MON_STATUS_REPLINISH;
 			break;
@@ -3057,15 +3097,43 @@  static int ath11k_dp_rx_reap_mon_status_ring(struct ath11k_base *ab, int mac_id,
 				ath11k_warn(ab, "mon status DONE not set %lx, buf_id %d\n",
 					    FIELD_GET(HAL_TLV_HDR_TAG,
 						      tlv->tl), buf_id);
-				/* If done status is missing, hold onto status
-				 * ring until status is done for this status
-				 * ring buffer.
-				 * Keep HP in mon_status_ring unchanged,
-				 * and break from here.
-				 * Check status for same buffer for next time
+				/* RxDMA status done bit might not be set even
+				 * though tp is moved by HW.
 				 */
-				pmon->buf_state = DP_MON_STATUS_NO_DMA;
-				break;
+
+				/* If done status is missing:
+				 * 1. As per MAC team's suggestion,
+				 *    when HP + 1 entry is peeked and if DMA
+				 *    is not done and if HP + 2 entry's DMA done
+				 *    is set. skip HP + 1 entry and
+				 *    start processing in next interrupt.
+				 * 2. If HP + 2 entry's DMA done is not set,
+				 *    poll onto HP + 1 entry DMA done to be set.
+				 *    Check status for same buffer for next time
+				 *    dp_rx_mon_status_srng_process
+				 */
+
+				reap_status = ath11k_dp_rx_mon_handle_status_buf_done(ab, srng,
+										      rx_ring);
+				if (reap_status == DP_MON_STATUS_NO_DMA) {
+					continue;
+				} else if (reap_status == DP_MON_STATUS_REPLINISH) {
+					ath11k_warn(ab, "mon status DONE not set %lx, buf_id %d\n",
+						    FIELD_GET(HAL_TLV_HDR_TAG, tlv->tl),
+						    buf_id);
+
+					spin_lock_bh(&rx_ring->idr_lock);
+					idr_remove(&rx_ring->bufs_idr, buf_id);
+					spin_unlock_bh(&rx_ring->idr_lock);
+
+					dma_unmap_single(ab->dev, rxcb->paddr,
+							 skb->len + skb_tailroom(skb),
+							 DMA_FROM_DEVICE);
+
+					dev_kfree_skb_any(skb);
+					pmon->buf_state = DP_MON_STATUS_REPLINISH;
+					goto move_next;
+				}
 			}
 
 			spin_lock_bh(&rx_ring->idr_lock);
diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c
index f3d04568c221..f02599bd1c36 100644
--- a/drivers/net/wireless/ath/ath11k/hal.c
+++ b/drivers/net/wireless/ath/ath11k/hal.c
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: BSD-3-Clause-Clear
 /*
  * Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 #include <linux/dma-mapping.h>
 #include "hal_tx.h"
@@ -796,6 +796,20 @@  u32 *ath11k_hal_srng_src_get_next_reaped(struct ath11k_base *ab,
 	return desc;
 }
 
+u32 *ath11k_hal_srng_src_next_peek(struct ath11k_base *ab, struct hal_srng *srng)
+{
+	u32 next_hp;
+
+	lockdep_assert_held(&srng->lock);
+
+	next_hp = (srng->u.src_ring.hp + srng->entry_size) % srng->ring_size;
+
+	if (next_hp != srng->u.src_ring.cached_tp)
+		return srng->ring_base_vaddr + next_hp;
+
+	return NULL;
+}
+
 u32 *ath11k_hal_srng_src_peek(struct ath11k_base *ab, struct hal_srng *srng)
 {
 	lockdep_assert_held(&srng->lock);
diff --git a/drivers/net/wireless/ath/ath11k/hal.h b/drivers/net/wireless/ath/ath11k/hal.h
index e453c137385e..dc8bbe073017 100644
--- a/drivers/net/wireless/ath/ath11k/hal.h
+++ b/drivers/net/wireless/ath/ath11k/hal.h
@@ -947,6 +947,8 @@  u32 *ath11k_hal_srng_dst_peek(struct ath11k_base *ab, struct hal_srng *srng);
 int ath11k_hal_srng_dst_num_free(struct ath11k_base *ab, struct hal_srng *srng,
 				 bool sync_hw_ptr);
 u32 *ath11k_hal_srng_src_peek(struct ath11k_base *ab, struct hal_srng *srng);
+u32 *ath11k_hal_srng_src_next_peek(struct ath11k_base *ab,
+				   struct hal_srng *srng);
 u32 *ath11k_hal_srng_src_get_next_reaped(struct ath11k_base *ab,
 					 struct hal_srng *srng);
 u32 *ath11k_hal_srng_src_reap_next(struct ath11k_base *ab,