Received-SPF: pass (google.com: domain of linux-wireless+bounces-5209-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <b03ab374-4d46-48ca-826c-3d2aa39034b4@quicinc.com>
Date: Mon, 25 Mar 2024 08:33:49 -0700
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: Rameshkumar Sundaram <quic_ramess@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>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <e5fe8dc4-c44b-4734-906f-a9dcbb724bb1@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

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.

/jeff