On Thu, 2023-12-21 at 11:54 +0100, Martin Kaistra wrote: > > Am 21.12.23 um 09:25 schrieb Ping-Ke Shih: > > On Wed, 2023-12-20 at 17:34 +0100, Martin Kaistra wrote: > > > > > > 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? > > 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. > Would you like to add a comment like above description to help people to understand your thinking? Anyway, this patch looks good to me. Please add my reviewed-by by v2.
