2022-05-28 19:44:49

by Christian Marangi

Subject: [PATCH 1/2] ath11k: fix missing srng_access_end in CE

When a CE completed send next operation is done, the srng access end is
never called. Correctly end the srng access to make sure we have the
correct values in the srng struct.

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1

Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Signed-off-by: Christian 'Ansuel' Marangi <[email protected]>
---
drivers/net/wireless/ath/ath11k/ce.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/ce.c b/drivers/net/wireless/ath/ath11k/ce.c
index c14c51f38709..665205d2322e 100644
--- a/drivers/net/wireless/ath/ath11k/ce.c
+++ b/drivers/net/wireless/ath/ath11k/ce.c
@@ -490,6 +490,8 @@ static struct sk_buff *ath11k_ce_completed_send_next(struct ath11k_ce_pipe *pipe
pipe->src_ring->sw_index = sw_index;

err_unlock:
+ ath11k_hal_srng_access_end(ab, srng);
+
spin_unlock_bh(&srng->lock);

spin_unlock_bh(&ab->ce.ce_lock);
--
2.36.1

2022-06-01 21:41:03

by Jeff Johnson

Subject: Re: [PATCH 1/2] ath11k: fix missing srng_access_end in CE

On 5/28/2022 7:25 AM, Christian 'Ansuel' Marangi wrote:
> When a CE completed send next operation is done, the srng access end is
> never called. Correctly end the srng access to make sure we have the
> correct values in the srng struct.
>
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01208-QCAHKSWPL_SILICONZ-1
>
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
> Signed-off-by: Christian 'Ansuel' Marangi <[email protected]>
> ---
> drivers/net/wireless/ath/ath11k/ce.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath11k/ce.c b/drivers/net/wireless/ath/ath11k/ce.c
> index c14c51f38709..665205d2322e 100644
> --- a/drivers/net/wireless/ath/ath11k/ce.c
> +++ b/drivers/net/wireless/ath/ath11k/ce.c
> @@ -490,6 +490,8 @@ static struct sk_buff *ath11k_ce_completed_send_next(struct ath11k_ce_pipe *pipe
> pipe->src_ring->sw_index = sw_index;
>
> err_unlock:
> + ath11k_hal_srng_access_end(ab, srng);
> +
> spin_unlock_bh(&srng->lock);
>
> spin_unlock_bh(&ab->ce.ce_lock);

NAK -- Not calling ath11k_hal_srng_access_end() is intentional.

CE_SRC ring is a special ring, we perform tx_request and tx_completion
on the same ring. The completion processing frees SKBs associated to the
ce_desc, this is what ath11k_ce_completed_send_next() does, it does not
update the head pointer to hw because the ce_desc at hp does not have
any new data, it is just available for new messages. ath11k_ce_send()
retrieves a processed (processed by ath11k_ce_completed_send_next)
ce_desc and sets it up for the new message. Then it updates the hw about
the newly available ce_desc by calling ath11k_hal_srng_access_end().

If you want to do anything here, adding a comment based upon the above
would be good.

/jeff