Received-SPF: pass (google.com: domain of linux-wireless+bounces-1632-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <269ea05b-d665-eceb-d7a1-d0ac20d341a7@quicinc.com>
Date: Tue, 9 Jan 2024 19:11:38 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH 4/4] wifi: ath12k: Refactor MAC un/register helper
 function
Content-Language: en-US
To: Kalle Valo <kvalo@kernel.org>
CC: <ath12k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
References: <20231206034920.1037449-1-quic_periyasa@quicinc.com>
 <20231206034920.1037449-5-quic_periyasa@quicinc.com>
 <87bk9uej0b.fsf@kernel.org>
From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
In-Reply-To: <87bk9uej0b.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/9/2024 6:55 PM, Kalle Valo wrote:
> Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:
>
>> Currently, the mac80211 hw registration procedure is tightly coupled with
>> the handling of link/radio (ar). Define a new helper function to separate
>> the link/radio handling from the mac80211 hw registration procedure for
>> improved code readability. Also, it can be easy to scale these
>> functionality to support single/multi link operation in the future.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/core.c |  61 +++++++++++-
>>   drivers/net/wireless/ath/ath12k/mac.c  | 132 ++++++++++---------------
>>   drivers/net/wireless/ath/ath12k/mac.h  |   4 +-
>>   3 files changed, 109 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
>> index e10c5f2cd8eb..d1ac00c59b8c 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.c
>> +++ b/drivers/net/wireless/ath/ath12k/core.c
>> @@ -498,11 +498,62 @@ static void ath12k_core_soc_destroy(struct ath12k_base *ab)
>>   	ath12k_qmi_deinit_service(ab);
>>   }
>>   
>> +static int ath12k_core_mac_register(struct ath12k_base *ab)
>> +{
>> +	struct ath12k *ar;
>> +	struct ath12k_pdev *pdev;
>> +	int i;
>> +	int ret;
>> +
>> +	if (test_bit(ATH12K_FLAG_REGISTERED, &ab->dev_flags))
>> +		return 0;
>> +
>> +	/* Initialize channel counters frequency value in hertz */
>> +	ab->cc_freq_hz = 320000;
>> +	ab->free_vdev_map = (1LL << (ab->num_radios * TARGET_NUM_VDEVS)) - 1;
>> +
>> +	for (i = 0; i < ab->num_radios; i++) {
>> +		pdev = &ab->pdevs[i];
>> +		ar = pdev->ar;
>> +
>> +		ret = ath12k_mac_hw_register(ar);
>> +		if (ret)
>> +			goto err_cleanup;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_cleanup:
>> +	for (i = i - 1; i >= 0; i--) {
>> +		pdev = &ab->pdevs[i];
>> +		ar = pdev->ar;
>> +		ath12k_mac_hw_unregister(ar);
>> +	}
>> +
>> +	return ret;
>> +}
> Is there a reason why you moved these two functions from mac.c to
> core.c? This is mac level code anyway, right?


This is not fully mac level code, some parts of SoC/chip level code 
bindup in the mac level.
So i segregated the SoC level code like ab related param initialization 
handling from the mac level procedure.

Now, mac/radio should take care only radio level handling procedure.


In future for MLO, SoC level can be extend easily.


Thanks,

Karthikeyan P.