Received-SPF: pass (google.com: domain of linux-wireless+bounces-1142-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <2c444230-bf30-427e-a498-877ed6d3e7cd@linutronix.de>
Date: Thu, 21 Dec 2023 11:54:29 +0100
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on
 port 0 transparent
Content-Language: de-DE
To: Ping-Ke Shih <pkshih@realtek.com>,
 "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc: "Jes.Sorensen@gmail.com" <Jes.Sorensen@gmail.com>,
 "kvalo@kernel.org" <kvalo@kernel.org>,
 "rtl8821cerfe2@gmail.com" <rtl8821cerfe2@gmail.com>,
 "bigeasy@linutronix.de" <bigeasy@linutronix.de>
References: <20231218143645.433356-1-martin.kaistra@linutronix.de>
 <20231218143645.433356-20-martin.kaistra@linutronix.de>
 <56eed6a3e237435f9d21082ca12eeaec@realtek.com>
 <797e4962-2ff3-4ae5-a1a7-d4d964fb768d@linutronix.de>
 <97e91ccfa2d8118b15166a9f2f25a56f84b460c8.camel@realtek.com>
From: Martin Kaistra <martin.kaistra@linutronix.de>
In-Reply-To: <97e91ccfa2d8118b15166a9f2f25a56f84b460c8.camel@realtek.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Am 21.12.23 um 09:25 schrieb Ping-Ke Shih:
> On Wed, 2023-12-20 at 17:34 +0100, Martin Kaistra wrote:
>> External mail.
>>
>>
>>
>> Am 20.12.23 um 07:28 schrieb Ping-Ke Shih:
>>>
>>>> -----Original Message-----
>>>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>>>> Sent: Monday, December 18, 2023 10:37 PM
>>>> To: linux-wireless@vger.kernel.org
>>>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
>>>> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
>>>> <bigeasy@linutronix.de>
>>>> Subject: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
>>>>
>>>> +
>>>> +       vif = priv->vifs[0];
>>>> +       priv->vifs[0] = priv->vifs[1];
>>>> +       priv->vifs[1] = vif;
>>>> +       rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
>>>> +       rtlvif->port_num = 1;
>>>
>>> nit: Would it be better to swap port_num as well? Currently, port_num of vifs[0]
>>> will be set to 0 by caller, but not sure if further people could misuse this
>>> function.
>>
>> the main reason, I did not include setting port_num for priv->vifs[0], is that
>> priv->vifs[0] is a NULL pointer in the current way this function is called from
>> rtl8xxxu_add_interface().
>>
>> do you think it makes sense to add
>>
>> if (priv->vifs[0])
>>          rtlvif = (struct rtl8xxxu_vif *)priv->vifs[0]->drv_priv;
>>          rtlvif->port_num = 0;
>>
>> just for completeness sake, even though this code path will currently never get
>> executed?
>>
> 
> I missed that point. I just did focus on "switch", but actually this is
> "move" from port 0 to 1, right?

Yes, currently, the function is only used to move the STA mode interface from 0 
to 1 in order to make room for AP on 0.

I will leave this patch as is for v2. When the function is used in the future 
for a different scenario, the possibility of vifs[0] or vifs[1] being NULL needs 
to be thought through anyway and if necessary the setting of port_num = 0 can be 
added then as well.

> 
> As I know, two cases only work on port 0. One is AP mode that you are
> doing, and the other is PS in STA mode that isn't implemented by rtl8xxxxu.
> For AP mode, current implement moving port 0 to 1 is fine.
> 
> In the future, PS in STA mode could need to move port 1 to 0, because
> we may disconnect port 0 first and we want STA on port 1 entering PS.
> 
> But, I think we can defer this until we really need it.
> 
>