Re: [PATCH v6 4/9] ath11k: Add register access logic for WCN6750

[Kalle Valo <kvalo@kernel.org>]

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