Received-SPF: pass (google.com: domain of linux-wireless+bounces-5221-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <85455dcb-9242-4f50-b3e2-17499633189a@quicinc.com>
Date: Mon, 25 Mar 2024 21:57:12 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 05/12] wifi: ath12k: scan statemachine changes for
 single wiphy
Content-Language: en-US
To: Jeff Johnson <quic_jjohnson@quicinc.com>, <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>, Sriram R <quic_srirrama@quicinc.com>
References: <20240320190943.3850106-1-quic_ramess@quicinc.com>
 <20240320190943.3850106-6-quic_ramess@quicinc.com>
 <85e7f591-ed52-4554-bd92-09086eaebcdb@quicinc.com>
 <e5fe8dc4-c44b-4734-906f-a9dcbb724bb1@quicinc.com>
 <b03ab374-4d46-48ca-826c-3d2aa39034b4@quicinc.com>
From: Rameshkumar Sundaram <quic_ramess@quicinc.com>
In-Reply-To: <b03ab374-4d46-48ca-826c-3d2aa39034b4@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 3/25/2024 9:03 PM, Jeff Johnson wrote:
> On 3/25/2024 8:24 AM, Rameshkumar Sundaram wrote:
>> On 3/22/2024 1:24 AM, Jeff Johnson wrote:
>>> On 3/20/2024 12:09 PM, Rameshkumar Sundaram wrote:
>>>> From: Sriram R <quic_srirrama@quicinc.com>
> ...
>>>>    	ret = ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
>>>>    	if (ret) {
>>>> -		ath12k_warn(ab, "failed to delete WMI vdev %d: %d\n",
>>>> +		ath12k_warn(ab, "failed to delete WMI scan vdev %d: %d\n",
>>>
>>> this change seems strange. isn't ath12k_mac_vdev_delete() called from both the
>>> scan logic and from the normal ath12k_mac_op_remove_interface(), so it might
>>> not be a scan vdev that is being deleted?
>>>
>> No, in Single wiphy, the vdev logic creation for an arvif is such that
>> at any given point of time only one vdev is created for an arvif (either
>> by ath12k_mac_op_add_intf/assign_chanctx/hw_scan).
>> Vdev created by mac_op_scan can either be re-used or deleted &
>> re-created (on a different ar) by mac_op_assign_chanctx() if required.
>> Also once mac_op_assign_chanctx has started the vdev of an arvif,
>> mac_op_hw_scan can only use that vdev. So mac_op_remove just simply
>> deletes the one that is currently created.
> 
> then if this function is only ever used to delete a scan vdev, then shouldn't
> the name be changed to reflect that? the current generic name doesn't reflect
> that specificity.
> 
Ah Sorry, i misunderstood your point earlier, as i mentioned vdev is 
created for an arvif either by 
ath12k_mac_op_add_intf/assign_chanctx/hw_scan).
So this vdev_delete can never say if its a scan vdev. For some reason it 
was put as scan vdev delete in print message :( .
We should remove this change.

Earlier I thought you were asking why vdev delete is being called from 
both places and was trying explaining the logic behind. Totally missed 
the print message, Really sorry about that.