From: Jeff Johnson <quic_jjohnson@quicinc.com>
To: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>,
Kalle Valo <kvalo@kernel.org>
Cc: <ath12k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] wifi: ath12k: Refactor data path cmem init
Date: Thu, 11 Apr 2024 08:20:38 -0700 [thread overview]
Message-ID: <823fe8a6-1e6c-49e9-a225-2012e77093ac@quicinc.com> (raw)
In-Reply-To: <6ac588c2-6206-c12e-e741-cbc507365bc3@quicinc.com>
On 4/11/2024 3:07 AM, Karthikeyan Periyasamy wrote:
>
>
> On 4/11/2024 3:15 PM, Kalle Valo wrote:
>> Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:
>>
>>> Move the data path Tx and Rx descriptor primary page table CMEM
>>> configuration into a helper function. This will make the code more
>>> scalable for configuring partner device in support of multi-device MLO.
>>>
>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1
>>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>>
>>> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>>> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
>>
>> [...]
>>
>>> +static void ath12k_dp_cmem_init(struct ath12k_base *ab,
>>> + struct ath12k_dp *dp,
>>> + enum ath12k_dp_desc_type type)
>>> +{
>>> + u32 cmem_base;
>>> + int i, start, end;
>>> +
>>> + cmem_base = ab->qmi.dev_mem[ATH12K_QMI_DEVMEM_CMEM_INDEX].start;
>>> +
>>> + switch (type) {
>>> + case ATH12K_DP_TX_DESC:
>>> + start = ATH12K_TX_SPT_PAGE_OFFSET;
>>> + end = start + ATH12K_NUM_TX_SPT_PAGES;
>>> + break;
>>> + case ATH12K_DP_RX_DESC:
>>> + start = ATH12K_RX_SPT_PAGE_OFFSET;
>>> + end = start + ATH12K_NUM_RX_SPT_PAGES;
>>> + break;
>>> + default:
>>> + ath12k_err(ab, "invalid descriptor type %d in cmem init\n", type);
>>> + return;
>>> + }
>>> +
>>> + /* Write to PPT in CMEM */
>>> + for (i = start; i < end; i++)
>>> + ath12k_hif_write32(ab, cmem_base + ATH12K_PPT_ADDR_OFFSET(i),
>>> + dp->spt_info[i].paddr >> ATH12K_SPT_4K_ALIGN_OFFSET);
>>> +}
>>
>> Here's a good example why I don't like functions returning void. How do
>> we handle the errors in this case?
>>
>
> sure, will handle the error case in the caller.
>
this is a static function with one caller. the only error is the default case
which will never be hit. adding logic to return an error and then check it in
the caller seems like overkill. why not just WARN() in the default case since
this would be a logic error with newly added code?
next prev parent reply other threads:[~2024-04-11 15:20 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 15:14 [PATCH v2 0/4] wifi: ath12k: Refactor hardware cookie conversion Karthikeyan Periyasamy
2024-04-09 15:14 ` [PATCH v2 1/4] wifi: ath12k: avoid redundant code in Rx cookie conversion init Karthikeyan Periyasamy
2024-04-09 15:14 ` [PATCH v2 2/4] wifi: ath12k: Refactor the hardware " Karthikeyan Periyasamy
2024-04-09 15:14 ` [PATCH v2 3/4] wifi: ath12k: displace the Tx and Rx descriptor in cookie conversion table Karthikeyan Periyasamy
2024-04-09 15:14 ` [PATCH v2 4/4] wifi: ath12k: Refactor data path cmem init Karthikeyan Periyasamy
2024-04-11 9:45 ` Kalle Valo
2024-04-11 10:07 ` Karthikeyan Periyasamy
2024-04-11 15:20 ` Jeff Johnson [this message]
2024-04-22 12:04 ` Kalle Valo
2024-04-22 16:09 ` Jeff Johnson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=823fe8a6-1e6c-49e9-a225-2012e77093ac@quicinc.com \
--to=quic_jjohnson@quicinc.com \
--cc=ath12k@lists.infradead.org \
--cc=kvalo@kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=quic_periyasa@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).