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;
Message-ID: <ab4e4298-a2d6-d66b-f90f-4282dd3f1454@quicinc.com>
Date:   Fri, 29 Apr 2022 15:47:12 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH v6 4/9] ath11k: Add register access logic for WCN6750
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>
CC:     <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <robh@kernel.org>, <mka@chromium.org>
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>
 <87h76cdxn2.fsf@kernel.org>
From:   Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
In-Reply-To: <87h76cdxn2.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/29/2022 3:22 PM, Kalle Valo wrote:
> 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.
> 

Sure Kalle, pls ignore V6 for now, I'll send out V7 with the bus_params 
completely removed.

Thanks,
Manikanta