Received-SPF: pass (google.com: domain of linux-wireless+bounces-7144-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Kalle Valo <kvalo@kernel.org>
To: Jeff Johnson <quic_jjohnson@quicinc.com>
Cc: Tamizh Chelvam Raja <quic_tamizhr@quicinc.com>,
  <ath11k@lists.infradead.org>,  <linux-wireless@vger.kernel.org>,
  "Venkateswara Naralasetty" <quic_vnaralas@quicinc.com>
Subject: Re: [PATCHv5] wifi: ath11k: skip status ring entry processing
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>
Date: Fri, 03 May 2024 16:11:25 +0300
In-Reply-To: <4f9abe89-a004-45e8-b369-5c2b39bff440@quicinc.com> (Jeff
	Johnson's message of "Tue, 30 Apr 2024 08:14:16 -0700")
Message-ID: <871q6jnipu.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

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

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches