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