Received-SPF: pass (google.com: domain of linux-wireless+bounces-6950-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Content-Type: text/plain;
	charset=us-ascii
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 15.0 \(3693.20.0.1.32\))
Subject: Re: [PATCHv3] wifi: ath11k: skip status ring entry processing
From: Meiyong Yu <meiyong.yu@126.com>
In-Reply-To: <A8B8371F-2FF4-45C7-9BEA-BE7FD5CA288F@126.com>
Date: Sun, 28 Apr 2024 21:02:28 +0800
Cc: Tamizh Chelvam Raja <quic_tamizhr@quicinc.com>,
 ath11k@lists.infradead.org,
 linux-wireless@vger.kernel.org,
 Venkateswara Naralasetty <quic_vnaralas@quicinc.com>,
 kernel test robot <lkp@intel.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <22DBEF03-83C8-460D-AF50-E1DE0038C072@126.com>
References: <20240427091248.2013946-1-quic_tamizhr@quicinc.com>
 <c8812227-ae90-4bbe-9fd3-425b61e36838@quicinc.com>
 <A8B8371F-2FF4-45C7-9BEA-BE7FD5CA288F@126.com>
To: Jeff Johnson <quic_jjohnson@quicinc.com>


> On Apr 28, 2024, at 8:54 PM, Meiyong Yu <meiyong.yu@126.com> wrote:
>=20
>=20
>=20
>> On Apr 28, 2024, at 6:46 AM, Jeff Johnson <quic_jjohnson@quicinc.com> =
wrote:
>>=20
>> On 4/27/2024 2:12 AM, Tamizh Chelvam Raja wrote:
>>> From: Venkateswara Naralasetty <quic_vnaralas@quicinc.com>
>>>=20
>>> 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.
>>>=20
>>> 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.
>>>=20
>=20
> Can you known the LMAC reset event, if you can known, you can set all =
the ring entry
> status  to done after reset is done, and the logic of code will be =
more clear.
>=20

If sene of the LMAC reset event is asynchronous, You can do this:
  1) when  LMAC init than set  a value to the new add global variabe =
lmac_life_cycle_id
  2) before add tlv to ring, set lmac_life_cycle_id to tlv
  3) when LMAC reset event is trigger, increase the value of =
lmac_life_cycle_id=20
 4) when get the status of tlv in ring(must delay for same period to =
ensure the real send ring is already send),=20
    check the value  lmac_life_cycle_id  in tlv, if it smaller than the  =
global one set tlv status to DONE=20


>>> 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.
>>>=20
>>> 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.
>>>=20
>>> To avoid this issue call ath11k_hal_srng_src_next_peek() instead of
>>> calling ath11k_hal_srng_src_peek().
>>>=20
>>> Tested-on: IPQ5018 hw1.0 AHB =
WLAN.HK.2.6.0.1-00861-QCAHKSWPL_SILICONZ-1
>>>=20
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: =
https://lore.kernel.org/oe-kbuild-all/202303281719.CvnPkOiK-lkp@intel.com/=

>>=20
>> I believe these are misleading. LKP didn't find the problem you are =
fixing, it
>> found a problem in the implementation of the patch.
>> So I would move these below the "---" so the LKP knows the issue it =
found is
>> fixed, but the git history isn't itself isn't misleading
>>=20
>>> Signed-off-by: Venkateswara Naralasetty <quic_vnaralas@quicinc.com>
>>> Co-developed-by: Tamizh Chelvam Raja <quic_tamizhr@quicinc.com>
>>> Signed-off-by: Tamizh Chelvam Raja <quic_tamizhr@quicinc.com>
>>> ---
>>> v3:
>>> * Rebased on top of ToT
>>> v2:
>>> * Fixed compilation warning Reported-by: kernel test robot =
<lkp@intel.com>
>>>=20
>>> drivers/net/wireless/ath/ath11k/dp_rx.c | 88 =
++++++++++++++++++++++---
>>> drivers/net/wireless/ath/ath11k/hal.c   | 14 ++++
>>> drivers/net/wireless/ath/ath11k/hal.h   |  2 +
>>=20
>> My Qualcomm Innovation Center copyright checker reports:
>> drivers/net/wireless/ath/ath11k/dp_rx.c copyright missing 2024
>> drivers/net/wireless/ath/ath11k/hal.c copyright missing 2024
>>=20
>=20