Hi Chris,
On Tue, May 28, 2019 at 11:03 PM Chris Chiu <chiu@xxxxxxxxxxxx> wrote:
> + /*
> + * Single virtual interface permitted since the driver supports STATION
> + * mode only.
I think you can be a bit more explicit by saying e.g.:
Only one virtual interface permitted because only STA mode is
supported and no iface_combinations are provided.
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 039e5ca9d2e4..2d612c2df5b2 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4345,7 +4345,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
>
> h2c.ramask.arg = 0x80;
> - h2c.b_macid_cfg.data1 = 0;
> + h2c.b_macid_cfg.data1 = priv->ratr_index;
I think ratr_index can be moved to be a function parameter of the
update_rate_mask function. It looks like all callsites already know
which value they want to set. Then you don't have to store it in the
priv structure.
> @@ -5471,6 +5509,10 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>
> switch (vif->type) {
> case NL80211_IFTYPE_STATION:
> + if (!priv->vif)
> + priv->vif = vif;
> + else
> + return -EOPNOTSUPP;
> rtl8xxxu_stop_tx_beacon(priv);
rtl8xxxu_remove_interface should also set priv->vif back to NULL.
> @@ -6183,6 +6259,8 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
> mutex_destroy(&priv->usb_buf_mutex);
> mutex_destroy(&priv->h2c_mutex);
>
> + cancel_delayed_work_sync(&priv->ra_watchdog);
Given that the work was started in rtl8xxxu_start, I think it should
be cancelled in rtl8xxxu_stop() instead.
Daniel
