Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>
> On 08/04/2024 05:45, Ping-Ke Shih wrote:
> >>
> >>
> >> 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 ;-)
> >
>
> The vendor driver assumes bInterfaceNumber can only be 0 or 1.
> I can make it more explicit:
>
> if (intf->altsetting[0].desc.bInterfaceNumber == 0)
> other_interfaceindex = 1;
> else
> other_interfaceindex = 0;
>
That looks easier to understand.
> >
> >>
> >> 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?
> >
>
> They can't both enter this branch because after the first
> disconnect usb_get_intfdata() will return NULL.
>
> > Also as I mentioned above, how can you ensure other_intf isn't still using the
> > shared data?
> >
>
> I can make the second disconnect free the shared data by
> checking if usb_get_intfdata() returns NULL:
>
> if (!other_intf || !usb_get_intfdata(other_intf)) {
> /* The other interface doesn't exist or was already disconnected. */
> kfree(rtlpriv->curveindex_2g);
Will usb_get_intfdata(other_intf) return NULL if the intf disconnected?
If yes, that looks good to me.
