[PATCH v2 4/4] wifi: ath12k: Refactor data path cmem init

[Jeff Johnson]

On 4/11/2024 3:07 AM, Karthikeyan Periyasamy wrote:
> 
> 
> On 4/11/2024 3:15 PM, Kalle Valo wrote:
>> Karthikeyan Periyasamy <quic_periyasa at 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 at quicinc.com>
>>> Acked-by: Jeff Johnson <quic_jjohnson at 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?