RE: [PATCH v3 11/12] wifi: rtlwifi: Add rtl8192du/sw.{c,h}

[Ping-Ke Shih <pkshih@realtek.com>]

> I did not consider using a ref_cnt variable. I made the first
> probe allocate the things and the first disconnect frees them:

The allocation seems fine, but I feel we should free shared data by second
disconnect because the other intf can still use them, no?

> 
> 
> static struct usb_interface *rtl92du_get_other_intf(struct ieee80211_hw *hw)
> {
>         struct usb_interface *intf;
>         struct usb_device *udev;
>         u8 other_interfaceindex;
> 
>         /* See SET_IEEE80211_DEV(hw, &intf->dev); in usb.c */
>         intf = container_of_const(wiphy_dev(hw->wiphy), struct usb_interface, dev);
> 
>         other_interfaceindex = 1 - intf->altsetting[0].desc.bInterfaceNumber;

The value of bInterfaceNumber for two instances are 0 and 1, right? Then
'1 - x' to get each other -- that looks a little tricky ;-) 

> 
>         udev = interface_to_usbdev(intf);
> 
>         return usb_ifnum_to_if(udev, other_interfaceindex);
> }
> 

[...]

> 
> static void rtl92du_deinit_shared_data(struct ieee80211_hw *hw)
> {
>         struct usb_interface *other_intf = rtl92du_get_other_intf(hw);
>         struct rtl_priv *rtlpriv = rtl_priv(hw);
> 
>         if (!other_intf || usb_get_intfdata(other_intf)) {
>                 /* The other interface doesn't exist or was not disconnected yet. */

For the USB adaptor with single one interface, you don't have other_intf.
Then, just free them.

If the USB adaptor has two interfaces, it has both other_intf and
usb_get_intfdata(other_intf), so you want to free them. But, I wonder if both 
interfaces can enter this branch?

Also as I mentioned above, how can you ensure other_intf isn't still using the
shared data?


>                 kfree(rtlpriv->curveindex_2g);
>                 kfree(rtlpriv->curveindex_5g);
>                 if (rtlpriv->mutex_for_power_on_off)
>                         mutex_destroy(rtlpriv->mutex_for_power_on_off);
>                 if (rtlpriv->mutex_for_hw_init)
>                         mutex_destroy(rtlpriv->mutex_for_hw_init);
>                 kfree(rtlpriv->mutex_for_power_on_off);
>                 kfree(rtlpriv->mutex_for_hw_init);
>         }
> }
> 

[...]