All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Fri, 29 Apr 2022 12:52:49 +0300	[thread overview]
Message-ID: <87h76cdxn2.fsf@kernel.org> (raw)
In-Reply-To: <6e607455-e42a-a591-f58b-b3b2c83ea2cc@quicinc.com> (Manikanta Pubbisetty's message of "Wed, 27 Apr 2022 18:56:33 +0530")

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

WARNING: multiple messages have this Message-ID (diff)
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
Date: Fri, 29 Apr 2022 12:52:49 +0300	[thread overview]
Message-ID: <87h76cdxn2.fsf@kernel.org> (raw)
In-Reply-To: <6e607455-e42a-a591-f58b-b3b2c83ea2cc@quicinc.com> (Manikanta Pubbisetty's message of "Wed, 27 Apr 2022 18:56:33 +0530")

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

  reply	other threads:[~2022-04-29 10:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-27 11:18 [PATCH v6 0/9] add support for WCN6750 Manikanta Pubbisetty
2022-04-27 11:18 ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 1/9] dt: bindings: net: add bindings of WCN6750 for ath11k Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 2/9] ath11k: Add HW params for WCN6750 Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 3/9] ath11k: Add bus " Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 4/9] ath11k: Add register access logic " Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 12:25   ` Kalle Valo
2022-04-27 12:25     ` Kalle Valo
2022-04-27 12:35     ` Manikanta Pubbisetty
2022-04-27 12:35       ` Manikanta Pubbisetty
2022-04-27 13:26     ` Manikanta Pubbisetty
2022-04-27 13:26       ` Manikanta Pubbisetty
2022-04-29  9:52       ` Kalle Valo [this message]
2022-04-29  9:52         ` Kalle Valo
2022-04-29 10:17         ` Manikanta Pubbisetty
2022-04-29 10:17           ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 5/9] ath11k: Fetch device information via QMI " Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 6/9] ath11k: Add QMI changes " Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 7/9] ath11k: HAL changes to support WCN6750 Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 8/9] ath11k: Datapath " Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty
2022-04-27 11:18 ` [PATCH v6 9/9] ath11k: Add support for WCN6750 device Manikanta Pubbisetty
2022-04-27 11:18   ` Manikanta Pubbisetty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h76cdxn2.fsf@kernel.org \
    --to=kvalo@kernel.org \
    --cc=ath11k@lists.infradead.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=quic_mpubbise@quicinc.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.