On 09/04/2024 03:27, Ping-Ke Shih wrote:
>
>
> 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.
>
>
It should. rtl_usb_disconnect() has usb_set_intfdata(intf, NULL);
at the end. Also usb_unbind_interface() in drivers/usb/core/driver.c
does the same after calling rtl_usb_disconnect().
