Received-SPF: pass (google.com: domain of linux-wireless+bounces-1935-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Kalle Valo <kvalo@kernel.org>
To: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Cc: <ath12k@lists.infradead.org>,  <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 4/4] wifi: ath12k: Refactor MAC un/register helper function
References: <20231206034920.1037449-1-quic_periyasa@quicinc.com>
	<20231206034920.1037449-5-quic_periyasa@quicinc.com>
	<87bk9uej0b.fsf@kernel.org>
	<269ea05b-d665-eceb-d7a1-d0ac20d341a7@quicinc.com>
Date: Mon, 15 Jan 2024 17:27:55 +0200
In-Reply-To: <269ea05b-d665-eceb-d7a1-d0ac20d341a7@quicinc.com> (Karthikeyan
	Periyasamy's message of "Tue, 9 Jan 2024 19:11:38 +0530")
Message-ID: <87v87u7h1g.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

Karthikeyan Periyasamy <quic_periyasa@quicinc.com> writes:

>> 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.

But is there a concrete reason to have the functions in core.c? In your
following patches I couldn't see why to move these functions to core.c,
everything seems to be suitable for mac.c.

I experimented this in the pending branch and moved the functions to
mac.c (tag ath-pending-202401151424):

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bee89ac2b5755754849deaf7054828e982cc6658

I also fixed your other patchsets to match that and to me it makes more
sense to have everything in mac.c. I prefer to make core.c as simple as possible.

As you can see I also made changes to the patch titles to make them more
understandable:

wifi: ath12k: refactor ath12k_mac_register() and ath12k_mac_unregister()

Thoughts?

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

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