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?
> > > 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?
