On 14/03/2024 03:20, Ping-Ke Shih wrote:
> On Thu, 2024-03-14 at 00:47 +0200, Bitterblue Smith wrote:
>>
>> On 13/03/2024 05:46, Ping-Ke Shih wrote:
>>> On Wed, 2024-03-13 at 00:20 +0200, Bitterblue Smith wrote:
>>>>
>>>> @@ -966,12 +980,17 @@ static void rtl92de_update_hal_rate_mask(struct ieee80211_hw *hw,
>>>> break;
>>>> }
>>>>
>>>> - value[0] = (ratr_bitmap & 0x0fffffff) | (ratr_index << 28);
>>>> - value[1] = macid | (shortgi ? 0x20 : 0x00) | 0x80;
>>>> + *(u32 *)&rate_mask = (ratr_bitmap & 0x0fffffff) |
>>>> + (ratr_index << 28);
>>>
>>> 'u32' is weird to me. Shouldn't it be __le32?
>>> But I prefer a struct of rate_mask.
>>>
>>
>> I don't like this either, but it was easy to copy from rtl8192cu.
>>
>> Something like this?
>>
>> #define RAID_MASK GENMASK(31, 28)
>> #define RATE_MASK_MASK GENMASK(27, 0)
>> #define SHORT_GI_MASK BIT(5)
>> #define MACID_MASK GENMASK(4, 0)
>>
>> struct rtl92d_rate_mask {
>> __le32 rate_mask_and_raid;
>> u8 macid_and_short_gi;
>> } __packed;
>
> Yes, something like that, but struct size should be 5.
>
>>>> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.c
>>>> b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.c
>>>> index 487628ac491b..1e39940a3ba7 100644
>>>> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.c
>>>> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.c
>>>> @@ -81,11 +81,13 @@ u32 rtl92d_phy_query_rf_reg(struct ieee80211_hw *hw, enum radio_path
>>>> rfpath,
>>>> rtl_dbg(rtlpriv, COMP_RF, DBG_TRACE,
>>>> "regaddr(%#x), rfpath(%#x), bitmask(%#x)\n",
>>>> regaddr, rfpath, bitmask);
>>>> - spin_lock(&rtlpriv->locks.rf_lock);
>>>> + if (rtlpriv->rtlhal.interface == INTF_PCI)
>>>> + spin_lock(&rtlpriv->locks.rf_lock);
>>>
>>> Does it mean USB never read/write RF registers simultaneously? How can you
>>> ensure that?
>>>
>>
>> I don't know. It seems to work fine. The out-of-tree driver
>> doesn't have locks here:
>> https://github.com/lwfinger/rtl8192du/blob/2c5450dd3783e1085f09a8c7a632318c7d0f1d39/hal/rtl8192d_phycfg.c#L637
>>
>> rtl8xxxu and rtl8192cu don't have locks either.
>
> Not sure if race condition is existing to read/write RF registers, but
> no idea to dig the problem. Maybe, current code has no problem though.
> At least, your newly changes don't affect original PCI behavior, right?
>
Yes, the PCI driver should behave like before.
>
>>>> diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c
>>>> b/drivers/net/wireless/realtek/rtlwifi/usb.c
>>>> index 6e8c87a2fae4..2ea72d9e3957 100644
>>>> --- a/drivers/net/wireless/realtek/rtlwifi/usb.c
>>>> +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
>>>> @@ -979,6 +979,9 @@ int rtl_usb_probe(struct usb_interface *intf,
>>>> usb_priv->dev.intf = intf;
>>>> usb_priv->dev.udev = udev;
>>>> usb_set_intfdata(intf, hw);
>>>> + /* For dual MAC RTL8192DU, which has two interfaces. */
>>>> + rtlpriv->rtlhal.interfaceindex =
>>>> + intf->altsetting[0].desc.bInterfaceNumber;
>>>
>>> So, you will see two USB adapters when you plug 8192DU?
>>>
>>
>> When you plug the dual MAC version, lsusb will show one device,
>> with two interfaces. rtl_usb_probe() is called twice. This is
>> copied from linux-hardware.org:
>>
>> Mine is the single MAC version:
>>
>
> Does it mean you only tested single MAC version? But, your code will support
> two MAC version, right?
>
>
Theoretically all the code for dual MAC is there. But I only
tested the single MAC version, and Stefan also has the single
MAC version.
