[PATCH v9 4/5] wifi: ath12k: Fill pdev id for fw test cmd

[Aaradhana Sahu]

On 1/15/2025 12:10 PM, Aditya Kumar Singh wrote:
> On 1/15/25 11:11, Aaradhana Sahu wrote:
>>
>>
>> On 1/15/2025 10:56 AM, Aditya Kumar Singh wrote:
>>> On 1/15/25 09:55, Aaradhana Sahu wrote:
>>>> Currently pdev id is not set properly. That can cause a crash
>>>> if pdev id is not equal to the pdev id when firmware test
>>>> command is run during AP bring up or ping.
>>>>
>>>> Set pdev id in function ath12k_tm_cmd_wmi to resolve this
>>>> issue.
>>>>
>>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>>>>
>>>> Co-developed-by: Rajat Soni <quic_rajson at quicinc.com>
>>>> Signed-off-by: Rajat Soni <quic_rajson at quicinc.com>
>>>> Signed-off-by: Aaradhana Sahu <quic_aarasahu at quicinc.com>
>>>> ---
>>>>    drivers/net/wireless/ath/ath12k/testmode.c | 11 +++++++++--
>>>>    1 file changed, 9 insertions(+), 2 deletions(-)
>>>>
>>>
>>> Previous patch "[PATCH v9 3/5] wifi: ath12k: add factory test mode support" only added testmode.c file isn't it? So can't we squash this patch in that?
>>>
>>> Let's not introduce a bug in patch X and then in same series fix it at patch Y.
>>>
>>
>> This patch does not address any issues related to Factory Test Mode (FTM).
>> Instead, it focuses only on the ath11k-fwtest command, which is used for
>> certification testing and is distinct from FTM.
>>
>> Initially, this patch was submitted independently as '[PATCH v2] wifi: ath12k: Fill pdev id for fw test cmd'
>> during the internal review.
>> However, Kalle suggested incorporating this patch with the FTM patches.
> 
> That is good. But that comment does not mean it needs to be a *separate* patch? It should be in this series that's what Kalle wanted to ensure.
> 
>>
>> As this patch addresses ath11k-fwtest command issue, that's why this is not merge with [PATCH v9 3/5].
>>
> 
> That is fine, but you are adding the file in 3/5 so why not have this part as well in same patch only? And let me ask this way, if you don't apply 3/5, will your issue exist? Probably No? Since ath12k/testmode.c file itself is not there before 3/5. So if you know something is incorrect, why introduce it in the first place?
> 
> 
During the internal review of this patch, the leads explicitly stated that it should be separate.
Based on their feedback, I made the necessary changes. Since this was previously discussed, I would prefer to keep this patch separate.