Manikanta Pubbisetty <quic_mpubbise@xxxxxxxxxxx> writes:
> On 4/27/2022 11:18 AM, Kalle Valo wrote:
>> Manikanta Pubbisetty <quic_mpubbise@xxxxxxxxxxx> writes:
>>
>>> On 4/27/2022 11:02 AM, Kalle Valo wrote:
>>>> Manikanta Pubbisetty <quic_mpubbise@xxxxxxxxxxx> writes:
>>>>
>>>>> HAL RX descriptor for WCN6750 is same as QCN9074, so use
>>>>> the DP APIs of QCN9074 for WCN6750. There is one change
>>>>> wrt to REO configuration though, REO configuration for
>>>>> WCN6750 is same as WCN6855; Therefore, use reo_setup()
>>>>> of WCN6855 for WCN6750.
>>>>>
>>>>> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00573-QCAMSLSWPLZ-1
>>>>> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>>>>> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
>>>>> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00192-QCAHKSWPL_SILICONZ-1
>>>>>
>>>>> Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@xxxxxxxxxxx>
>>>>> ---
>>>>> drivers/net/wireless/ath/ath11k/core.c | 1 +
>>>>> drivers/net/wireless/ath/ath11k/hw.c | 31 +++++++++++++++++++++++++++++++
>>>>> 2 files changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
>>>>> index 347131a..f65347c 100644
>>>>> --- a/drivers/net/wireless/ath/ath11k/core.c
>>>>> +++ b/drivers/net/wireless/ath/ath11k/core.c
>>>>> @@ -458,6 +458,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
>>>>> .num_vdevs = 16 + 1,
>>>>> .num_peers = 512,
>>>>> .supports_suspend = false,
>>>>> + .hal_desc_sz = sizeof(struct hal_rx_desc_qcn9074),
>>>>
>>>> You mention nothing about this change in the commit log, I'll add that.
>>>>
>>>
>>> I did mention that "HAL RX descriptor for WCN6750 is same as QCN9074",
>>> you were looking at something specific?
>>
>> If I'm understanding correctly, by accident hal_desc_sz was not
>> initialised for qcn9074. If that's the case, it would be good to mention
>> that in the commit log.
>>
>
> If I understand your comment correctly, since HAL RX descriptor is
> related to data path, I thought of adding it as part of the data path
> patch.
Ah, now I understand better. I thought you were setting hal_desc_sz for
qcn9074, but now I checked it and you were actually setting it for
wcn6750. It would be better to set hal_desc_sz in patch "ath11k: Add HW
params for WCN6750".
Actually I would prefer that _all_ existing hw_params fields for wcn6750
would be initialised in patch "ath11k: Add HW params for WCN6750", even
if just set to null or invalid values. That way it's easier to track
that we are not missing any fields and makes it easier to review other
patches. But this is getting more to nitpicking so definitely not a hard
requirement.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
