Received-SPF: pass (google.com: domain of linux-wireless+bounces-7150-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <143e04a0-3bda-4718-8647-1ed31e1f5774@quicinc.com>
Date: Fri, 3 May 2024 07:55:34 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCHv5] wifi: ath11k: skip status ring entry processing
Content-Language: en-US
To: Kalle Valo <kvalo@kernel.org>
CC: Tamizh Chelvam Raja <quic_tamizhr@quicinc.com>,
        <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
        "Venkateswara
 Naralasetty" <quic_vnaralas@quicinc.com>
References: <20240429073624.736147-1-quic_tamizhr@quicinc.com>
 <35f114c4-1ff7-4a4b-aadf-ed147f19e170@quicinc.com>
 <87cyq7ota5.fsf@kernel.org>
 <4f9abe89-a004-45e8-b369-5c2b39bff440@quicinc.com>
 <871q6jnipu.fsf@kernel.org>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <871q6jnipu.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 5/3/2024 6:11 AM, Kalle Valo wrote:
> Jeff Johnson <quic_jjohnson@quicinc.com> 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!