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@xxxxxxxxxxxxx> > > > Sent: Monday, December 18, 2023 10:37 PM > > > To: linux-wireless@xxxxxxxxxxxxxxx > > > Cc: Jes Sorensen <Jes.Sorensen@xxxxxxxxx>; Kalle Valo <kvalo@xxxxxxxxxx>; Ping-Ke Shih > > > <pkshih@xxxxxxxxxxx>; Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>; Sebastian Andrzej Siewior > > > <bigeasy@xxxxxxxxxxxxx> > > > 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? 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.