Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Kalle Valo <kvalo@kernel.org>
To:     Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
Cc:     <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <robh@kernel.org>, <mka@chromium.org>
Subject: Re: [PATCH v6 4/9] ath11k: Add register access logic for WCN6750
References: <20220427111848.9257-1-quic_mpubbise@quicinc.com>
        <20220427111848.9257-5-quic_mpubbise@quicinc.com>
        <87pml2g1c7.fsf@kernel.org>
        <6e607455-e42a-a591-f58b-b3b2c83ea2cc@quicinc.com>
Date:   Fri, 29 Apr 2022 12:52:49 +0300
In-Reply-To: <6e607455-e42a-a591-f58b-b3b2c83ea2cc@quicinc.com> (Manikanta
        Pubbisetty's message of "Wed, 27 Apr 2022 18:56:33 +0530")
Message-ID: <87h76cdxn2.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes:

>>> @@ -704,11 +718,26 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
>>>   {
>>>   	struct ath11k_base *ab;
>>>   	struct ath11k_pci *ab_pci;
>>> +	const struct ath11k_bus_params *bus_params;
>>>   	u32 soc_hw_version_major, soc_hw_version_minor, addr;
>>>   	int ret;
>>>   +	switch (pci_dev->device) {
>>> +	case QCA6390_DEVICE_ID:
>>> +	case WCN6855_DEVICE_ID:
>>> +		bus_params = &ath11k_pci_bus_params_qca6390;
>>> +		break;
>>> +	case QCN9074_DEVICE_ID:
>>> +		bus_params = &ath11k_pci_bus_params_qcn9074;
>>> +		break;
>>
>> Now you are making bus_params device specific, that's not really the
>> point of bus params. They are supposed to be _bus_ specific parameters.
>>
>> Can't you use hw_params like I mentioned in the review?
>>
>
> Even without this patch, as of today, bus_params is already device
> specific with QCN9074 changing the static_window_map in bus_params to
> true in ath11k_pci_probe().

Yeah, that's a mistake which slipped in review.

> And if we have to move these device specific bus_params to hw_parmas,
> then bus_params can be pretty much removed completely with the changes
> that WCN6750 bring in. Any thoughts on this? I can make the changes
> that can get along with WCN6750 series.

My original idea with bus_params was to there are bus specific
parameters and that way we could simplify hw_params. Clearly that's not
working and I agree with you, bus_params should be removed. So it would
be good if you can do that in the next patchset.

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

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