diff mbox series

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

Message ID 2eb79c8c-cf2c-4696-b958-e8d961628e17@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Ping-Ke Shih
Headers show
Series wifi: rtlwifi: Add new rtl8192du driver | expand

Commit Message

Bitterblue Smith March 20, 2024, 7:43 p.m. UTC
These contain the new module's entry point.

Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
v3:
 - Add USB ID 2001:330c found by Zenm Chen.

v2:
 - Patch is new in v2, split from patch 3/3 in v1.
---
 .../wireless/realtek/rtlwifi/rtl8192du/sw.c   | 312 ++++++++++++++++++
 .../wireless/realtek/rtlwifi/rtl8192du/sw.h   |  12 +
 2 files changed, 324 insertions(+)
 create mode 100644 drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c
 create mode 100644 drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h

Comments

Ping-Ke Shih March 22, 2024, 6:04 a.m. UTC | #1
On Wed, 2024-03-20 at 21:43 +0200, Bitterblue Smith wrote:
> 
> These contain the new module's entry point.
> 
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
> v3:
>  - Add USB ID 2001:330c found by Zenm Chen.
> 
> v2:
>  - Patch is new in v2, split from patch 3/3 in v1.
> ---
>  .../wireless/realtek/rtlwifi/rtl8192du/sw.c   | 312 ++++++++++++++++++
>  .../wireless/realtek/rtlwifi/rtl8192du/sw.h   |  12 +
>  2 files changed, 324 insertions(+)
>  create mode 100644 drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c
>  create mode 100644 drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c
> new file mode 100644
> index 000000000000..6d7f40e7add5
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2009-2012  Realtek Corporation.*/
> +
> +#include "../wifi.h"
> +#include "../core.h"
> +#include "../usb.h"
> +#include "../base.h"
> +#include "../rtl8192d/reg.h"
> +#include "../rtl8192d/def.h"
> +#include "../rtl8192d/fw_common.h"
> +#include "../rtl8192d/hw_common.h"
> +#include "../rtl8192d/phy_common.h"
> +#include "../rtl8192d/trx_common.h"
> +#include "phy.h"
> +#include "dm.h"
> +#include "fw.h"
> +#include "hw.h"
> +#include "sw.h"
> +#include "trx.h"
> +#include "led.h"
> +
> +#include <linux/module.h>
> +
> +static int rtl92du_init_sw_vars(struct ieee80211_hw *hw)
> +{
> +       const char *fw_name = "rtlwifi/rtl8192dufw.bin";
> +       struct rtl_priv *rtlpriv = rtl_priv(hw);
> +       int err;
> +
> +       rtlpriv->dm.dm_initialgain_enable = true;
> +       rtlpriv->dm.dm_flag = 0;
> +       rtlpriv->dm.disable_framebursting = false;
> +       rtlpriv->dm.thermalvalue = 0;
> +       rtlpriv->dm.useramask = true;
> +
> +       /* dual mac */
> +       if (rtlpriv->rtlhal.current_bandtype == BAND_ON_5G)
> +               rtlpriv->phy.current_channel = 36;
> +       else
> +               rtlpriv->phy.current_channel = 1;
> +
> +       if (rtlpriv->rtlhal.macphymode != SINGLEMAC_SINGLEPHY)
> +               rtlpriv->rtlhal.disable_amsdu_8k = true;
> +
> +       /* for LPS & IPS */
> +       rtlpriv->psc.inactiveps = rtlpriv->cfg->mod_params->inactiveps;
> +       rtlpriv->psc.swctrl_lps = rtlpriv->cfg->mod_params->swctrl_lps;
> +       rtlpriv->psc.fwctrl_lps = rtlpriv->cfg->mod_params->fwctrl_lps;
> +       if (!rtlpriv->psc.inactiveps)
> +               pr_info("Inactive Power Save off (module option)\n");
> +
> +       /* for early mode */
> +       rtlpriv->rtlhal.earlymode_enable = false;
> +
> +       /* for firmware buf */
> +       rtlpriv->rtlhal.pfirmware = kmalloc(0x8000, GFP_KERNEL);
> +       if (!rtlpriv->rtlhal.pfirmware) {
> +               pr_err("Can't alloc buffer for fw\n");

WARNING: Possible unnecessary 'out of memory' message
#75: FILE: drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c:58:
+	if (!rtlpriv->rtlhal.pfirmware) {
+		pr_err("Can't alloc buffer for fw\n");

> +               return 1;

returning standard error number will be better, but others have been
there....

> +       }
> +
> +       rtlpriv->max_fw_size = 0x8000;
> +       pr_info("Driver for Realtek RTL8192DU WLAN interface\n");
> +       pr_info("Loading firmware file %s\n", fw_name);
> +
> +       /* request fw */
> +       err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
> +                                     rtlpriv->io.dev, GFP_KERNEL, hw,
> +                                     rtl_fw_cb);
> +       if (err) {
> +               pr_err("Failed to request firmware!\n");
> +               kfree(rtlpriv->rtlhal.pfirmware);
> +               rtlpriv->rtlhal.pfirmware = NULL;
> +               return 1;
> +       }
> +
> +       return 0;
> +}
> +
> +static void rtl92du_deinit_sw_vars(struct ieee80211_hw *hw)
> +{
> +       struct rtl_priv *rtlpriv = rtl_priv(hw);
> +
> +       kfree(rtlpriv->rtlhal.pfirmware);
> +       rtlpriv->rtlhal.pfirmware = NULL;
> +}
> +
> +static struct rtl_hal_ops rtl8192du_hal_ops = {

static const (also below tables)

[...]

> +
> +MODULE_AUTHOR("lizhaoming      <chaoming_li@realsil.com.cn>");
> +MODULE_AUTHOR("Realtek WlanFAE <wlanfae@realtek.com>");
> +MODULE_AUTHOR("Larry Finger    <Larry.Finger@lwfinger.net>");

Author should be you. 

> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Realtek 8192DU 802.11an Dual Mac USB wireless");
> +MODULE_FIRMWARE("rtlwifi/rtl8192dufw.bin");

Normally, we put MODULE_xxx at bottom of files.

> +
> +module_param_named(swenc, rtl92du_mod_params.sw_crypto, bool, 0444);
> +module_param_named(debug_level, rtl92du_mod_params.debug_level, int, 0644);
> +module_param_named(ips, rtl92du_mod_params.inactiveps, bool, 0444);
> +module_param_named(swlps, rtl92du_mod_params.swctrl_lps, bool, 0444);
> +module_param_named(debug_mask, rtl92du_mod_params.debug_mask, ullong, 0644);
> +MODULE_PARM_DESC(swenc, "Set to 1 for software crypto (default 0)\n");
> +MODULE_PARM_DESC(ips, "Set to 0 to not use link power save (default 0)\n");
> +MODULE_PARM_DESC(swlps, "Set to 1 to use SW control power save (default 0)\n");
> +MODULE_PARM_DESC(debug_level, "Set debug level (0-5) (default 0)");
> +MODULE_PARM_DESC(debug_mask, "Set debug mask (default 0)");
> +
> +/* Add global mutex to solve the problem that
> + * dual mac register operation on the same time
> + */

I see the reason now, it seems work.

> +DEFINE_MUTEX(globalmutex_power);
> +DEFINE_MUTEX(globalmutex_for_fwdownload);
> +DEFINE_MUTEX(globalmutex_for_power_and_efuse);
> +DEFINE_MUTEX(globalmutex_for_mac0_2g_mac1_5g);

The consumers of globalmutex_for_mac0_2g_mac1_5g are complex. Why do they
check mutex_is_locked()? Race conditions between two instances?


> +
> +#define USB_VENDOR_ID_REALTEK          0x0bda
> +
> +static const struct usb_device_id rtl8192d_usb_ids[] = {
> +       {RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8193, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8194, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8111, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x0193, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8171, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0xe194, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x2019, 0xab2c, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x2019, 0xab2d, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x2019, 0x4903, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x2019, 0x4904, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x07b8, 0x8193, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x20f4, 0x664b, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x04dd, 0x954f, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x04dd, 0x96a6, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x050d, 0x110a, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x050d, 0x1105, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x050d, 0x120a, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x1668, 0x8102, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x0930, 0x0a0a, rtl92du_hal_cfg)},
> +       {RTL_USB_DEVICE(0x2001, 0x330c, rtl92du_hal_cfg)},
> +       {}
> +};
> +
> +MODULE_DEVICE_TABLE(usb, rtl8192d_usb_ids);
> +
> +static int rtl8192du_probe(struct usb_interface *intf,
> +                          const struct usb_device_id *id)
> +{
> +       return rtl_usb_probe(intf, id, &rtl92du_hal_cfg);
> +}
> +
> +static struct usb_driver rtl8192du_driver = {
> +       .name = "rtl8192du",
> +       .probe = rtl8192du_probe,
> +       .disconnect = rtl_usb_disconnect,
> +       .id_table = rtl8192d_usb_ids,
> +       .disable_hub_initiated_lpm = 1,
> +};
> +
> +module_usb_driver(rtl8192du_driver);
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h
> new file mode 100644
> index 000000000000..364d9a471dc0
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2009-2012  Realtek Corporation.*/
> +
> +#ifndef __RTL92DE_SW_H__
> +#define __RTL92DE_SW_H__

8192DU

> +
> +extern struct mutex globalmutex_power;
> +extern struct mutex globalmutex_for_fwdownload;
> +extern struct mutex globalmutex_for_power_and_efuse;
> +extern struct mutex globalmutex_for_mac0_2g_mac1_5g;
> +
> +#endif
> --
> 2.43.2
Bitterblue Smith March 27, 2024, 2:07 p.m. UTC | #2
On 22/03/2024 08:04, Ping-Ke Shih wrote:
> On Wed, 2024-03-20 at 21:43 +0200, Bitterblue Smith wrote:

[...]

>> +DEFINE_MUTEX(globalmutex_power);
>> +DEFINE_MUTEX(globalmutex_for_fwdownload);
>> +DEFINE_MUTEX(globalmutex_for_power_and_efuse);
>> +DEFINE_MUTEX(globalmutex_for_mac0_2g_mac1_5g);
> 
> The consumers of globalmutex_for_mac0_2g_mac1_5g are complex. Why do they
> check mutex_is_locked()? Race conditions between two instances?
> 

I couldn't think of a sufficiently short name, like
"lock_mac0_2g_mac1_5g", so I used mutex_is_locked(). That's probably
a bad idea. It should be like this:

	/* Let the first starting mac load RF parameters and do LCK */
	if (rtlhal->macphymode == DUALMAC_DUALPHY &&
	    ((rtlhal->interfaceindex == 0 && rtlhal->bandset == BAND_ON_2_4G) ||
	     (rtlhal->interfaceindex == 1 && rtlhal->bandset == BAND_ON_5G))) {
		mutex_lock(&globalmutex_for_mac0_2g_mac1_5g);
		lock_mac0_2g_mac1_5g = true;
	}

	....

	if (lock_mac0_2g_mac1_5g)
		mutex_unlock(&globalmutex_for_mac0_2g_mac1_5g);
Kalle Valo March 27, 2024, 6:48 p.m. UTC | #3
Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:

> On 22/03/2024 08:04, Ping-Ke Shih wrote:
>> On Wed, 2024-03-20 at 21:43 +0200, Bitterblue Smith wrote:
>
> [...]
>
>>> +DEFINE_MUTEX(globalmutex_power);
>>> +DEFINE_MUTEX(globalmutex_for_fwdownload);
>>> +DEFINE_MUTEX(globalmutex_for_power_and_efuse);
>>> +DEFINE_MUTEX(globalmutex_for_mac0_2g_mac1_5g);
>> 
>> The consumers of globalmutex_for_mac0_2g_mac1_5g are complex. Why do they
>> check mutex_is_locked()? Race conditions between two instances?
>> 
>
> I couldn't think of a sufficiently short name, like
> "lock_mac0_2g_mac1_5g", so I used mutex_is_locked(). That's probably
> a bad idea. It should be like this:
>
> 	/* Let the first starting mac load RF parameters and do LCK */
> 	if (rtlhal->macphymode == DUALMAC_DUALPHY &&
> 	    ((rtlhal->interfaceindex == 0 && rtlhal->bandset == BAND_ON_2_4G) ||
> 	     (rtlhal->interfaceindex == 1 && rtlhal->bandset == BAND_ON_5G))) {
> 		mutex_lock(&globalmutex_for_mac0_2g_mac1_5g);
> 		lock_mac0_2g_mac1_5g = true;
> 	}

Few quick comments, I haven't reviewed the actual patchset yet:

The mutexes look too finegrained and makes me suspicious about the
locking design.

Having global variables like globalmutex_power is a big no no. They would
not work if there are two devices on the same host, right?

Conditional locking is usually something to avoid.

I'm starting to wonder if extending rtlwifi is actually the right
approach. We have modern drivers like rtl8xxxu, rtw88 etc. with better
design.
Bitterblue Smith March 27, 2024, 10:53 p.m. UTC | #4
On 27/03/2024 20:48, Kalle Valo wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:
> 
>> On 22/03/2024 08:04, Ping-Ke Shih wrote:
>>> On Wed, 2024-03-20 at 21:43 +0200, Bitterblue Smith wrote:
>>
>> [...]
>>
>>>> +DEFINE_MUTEX(globalmutex_power);
>>>> +DEFINE_MUTEX(globalmutex_for_fwdownload);
>>>> +DEFINE_MUTEX(globalmutex_for_power_and_efuse);
>>>> +DEFINE_MUTEX(globalmutex_for_mac0_2g_mac1_5g);
>>>
>>> The consumers of globalmutex_for_mac0_2g_mac1_5g are complex. Why do they
>>> check mutex_is_locked()? Race conditions between two instances?
>>>
>>
>> I couldn't think of a sufficiently short name, like
>> "lock_mac0_2g_mac1_5g", so I used mutex_is_locked(). That's probably
>> a bad idea. It should be like this:
>>
>> 	/* Let the first starting mac load RF parameters and do LCK */
>> 	if (rtlhal->macphymode == DUALMAC_DUALPHY &&
>> 	    ((rtlhal->interfaceindex == 0 && rtlhal->bandset == BAND_ON_2_4G) ||
>> 	     (rtlhal->interfaceindex == 1 && rtlhal->bandset == BAND_ON_5G))) {
>> 		mutex_lock(&globalmutex_for_mac0_2g_mac1_5g);
>> 		lock_mac0_2g_mac1_5g = true;
>> 	}
> 
> Few quick comments, I haven't reviewed the actual patchset yet:
> 
> The mutexes look too finegrained and makes me suspicious about the
> locking design.
> > Having global variables like globalmutex_power is a big no no. They would
> not work if there are two devices on the same host, right?
> 
> Conditional locking is usually something to avoid.
> 

The design is not mine, I can't fix that. I don't even have
the type of device which needs all these mutexes. There are
two types of RTL8192DU:

* Single MAC single PHY: this is the one I have. I can still
buy it from Aliexpress. It's a lot like the other Realtek
Wifi 4 USB devices.

* Dual MAC dual PHY: this I can't find to buy anymore. This
appears in the system as two Wifi devices, each working on
a different band. It has two USB interfaces. Two instances
of the driver access the same device. This is what the
mutexes are for.

I said earlier that I think two devices can work at the same
time, even with the global mutexes, but now I remembered there
are two more global variables: curveindex_5g[] and
curveindex_2g[] in phy.c. One driver instance fills one array
during LC calibration, but the other driver instance reads
from the array when switching the channel. If I'm reading this
right. So two devices plugged in at the same time probably
won't work correctly.

How can you avoid this when the hardware is the way it is?
My one idea is to add a global map to hold the mutexes and
arrays, using the USB port number, etc as the key.

> I'm starting to wonder if extending rtlwifi is actually the right
> approach. We have modern drivers like rtl8xxxu, rtw88 etc. with better
> design.
> 

I think extending rtlwifi is the right approach when it already
has most of the code needed. I really don't want to reinvent this
particular wheel. I don't want to add the dual MAC stuff to
rtl8xxxu or rtw88. I don't think rtw88 is the right driver for
this old chip, anyway.
Ping-Ke Shih March 28, 2024, 1:46 a.m. UTC | #5
On Thu, 2024-03-28 at 00:53 +0200, Bitterblue Smith wrote:
> 
> On 27/03/2024 20:48, Kalle Valo wrote:
> > Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:
> > 
> > > On 22/03/2024 08:04, Ping-Ke Shih wrote:
> > > > On Wed, 2024-03-20 at 21:43 +0200, Bitterblue Smith wrote:
> > > 
> > > [...]
> > > 
> > > > > +DEFINE_MUTEX(globalmutex_power);
> > > > > +DEFINE_MUTEX(globalmutex_for_fwdownload);
> > > > > +DEFINE_MUTEX(globalmutex_for_power_and_efuse);
> > > > > +DEFINE_MUTEX(globalmutex_for_mac0_2g_mac1_5g);
> > > > 
> > > > The consumers of globalmutex_for_mac0_2g_mac1_5g are complex. Why do they
> > > > check mutex_is_locked()? Race conditions between two instances?
> > > > 
> > > 
> > > I couldn't think of a sufficiently short name, like
> > > "lock_mac0_2g_mac1_5g", so I used mutex_is_locked(). That's probably
> > > a bad idea. It should be like this:
> > > 
> > >      /* Let the first starting mac load RF parameters and do LCK */
> > >      if (rtlhal->macphymode == DUALMAC_DUALPHY &&
> > >          ((rtlhal->interfaceindex == 0 && rtlhal->bandset == BAND_ON_2_4G) ||
> > >           (rtlhal->interfaceindex == 1 && rtlhal->bandset == BAND_ON_5G))) {
> > >              mutex_lock(&globalmutex_for_mac0_2g_mac1_5g);
> > >              lock_mac0_2g_mac1_5g = true;
> > >      }

globalmutex_for_mac0_2g_mac1_5g is only used in rtl92du_hw_init(), and
globalmutex_for_power_and_efuse does very similar thing. Can we combine them
into one? Since both are only used in rtl92du_hw_init(), it would not be a
problem to enlarge their critical section.  


> 
> * Dual MAC dual PHY: this I can't find to buy anymore. This
> appears in the system as two Wifi devices, each working on
> a different band. It has two USB interfaces. Two instances
> of the driver access the same device. This is what the
> mutexes are for.

I traced the code, and found rules of two MAC/PHY are:
1. read efuse to decide single or two MAC/PHY
2.1. if single MAC/PHY, register_hw as 2T2R (done)
2.2. if dual MAC/PHY (to step 3)
3. read interface index (USB: from bInterfaceNumber; PCI: from PCI onfigure space)
4. register interface index 0 as 1T1R on 5 GHz band only.
   register interface index 1 as 1T1R on 2 GHz band only.

This is the case two instances (netdev) access single one hardware device,
so seemingly it is hard to avoid global locks to prevent racing between them.
An alternative thought is to support only single MAC/PHY, but not sure if
driver can override setting of efuse that programmed the card as two MAC/PHY.

> 
> I said earlier that I think two devices can work at the same
> time, even with the global mutexes, but now I remembered there
> are two more global variables: curveindex_5g[] and
> curveindex_2g[] in phy.c. One driver instance fills one array
> during LC calibration, but the other driver instance reads
> from the array when switching the channel. If I'm reading this
> right. So two devices plugged in at the same time probably
> won't work correctly.

That should be a problem. 

> 
> How can you avoid this when the hardware is the way it is?
> My one idea is to add a global map to hold the mutexes and
> arrays, using the USB port number, etc as the key.

Seemingly we need something like per device data. 

> 
> > I'm starting to wonder if extending rtlwifi is actually the right
> > approach. We have modern drivers like rtl8xxxu, rtw88 etc. with better
> > design.
> > 
> 
> I think extending rtlwifi is the right approach when it already
> has most of the code needed. I really don't want to reinvent this
> particular wheel. 

Right, but honestly rtlwifi did a lot of tricky things we can't understand
the whole picture...


> I don't want to add the dual MAC stuff to rtl8xxxu or rtw88. 

I don't want that neither.
Ping-Ke Shih March 28, 2024, 1:49 a.m. UTC | #6
On Wed, 2024-03-27 at 16:07 +0200, Bitterblue Smith wrote:
> 
> I couldn't think of a sufficiently short name, like
> "lock_mac0_2g_mac1_5g", so I used mutex_is_locked(). That's probably
> a bad idea. It should be like this:
> 
>         /* Let the first starting mac load RF parameters and do LCK */
>         if (rtlhal->macphymode == DUALMAC_DUALPHY &&
>             ((rtlhal->interfaceindex == 0 && rtlhal->bandset == BAND_ON_2_4G) ||
>              (rtlhal->interfaceindex == 1 && rtlhal->bandset == BAND_ON_5G))) {

After tracing the code, I feel here can only check rtlhal->macphymode. 


>                 mutex_lock(&globalmutex_for_mac0_2g_mac1_5g);
>                 lock_mac0_2g_mac1_5g = true;
>         }
> 
>         ....
> 
>         if (lock_mac0_2g_mac1_5g)
>                 mutex_unlock(&globalmutex_for_mac0_2g_mac1_5g);
>
Bitterblue Smith March 28, 2024, 1:31 p.m. UTC | #7
On 28/03/2024 03:46, Ping-Ke Shih wrote:
> On Thu, 2024-03-28 at 00:53 +0200, Bitterblue Smith wrote:
>>
>> On 27/03/2024 20:48, Kalle Valo wrote:
>>> Bitterblue Smith <rtl8821cerfe2@gmail.com> writes:
>>>
>>>> On 22/03/2024 08:04, Ping-Ke Shih wrote:
>>>>> On Wed, 2024-03-20 at 21:43 +0200, Bitterblue Smith wrote:
>>>>
>>>> [...]
>>>>
>>>>>> +DEFINE_MUTEX(globalmutex_power);
>>>>>> +DEFINE_MUTEX(globalmutex_for_fwdownload);
>>>>>> +DEFINE_MUTEX(globalmutex_for_power_and_efuse);
>>>>>> +DEFINE_MUTEX(globalmutex_for_mac0_2g_mac1_5g);
>>>>>
>>>>> The consumers of globalmutex_for_mac0_2g_mac1_5g are complex. Why do they
>>>>> check mutex_is_locked()? Race conditions between two instances?
>>>>>
>>>>
>>>> I couldn't think of a sufficiently short name, like
>>>> "lock_mac0_2g_mac1_5g", so I used mutex_is_locked(). That's probably
>>>> a bad idea. It should be like this:
>>>>
>>>>      /* Let the first starting mac load RF parameters and do LCK */
>>>>      if (rtlhal->macphymode == DUALMAC_DUALPHY &&
>>>>          ((rtlhal->interfaceindex == 0 && rtlhal->bandset == BAND_ON_2_4G) ||
>>>>           (rtlhal->interfaceindex == 1 && rtlhal->bandset == BAND_ON_5G))) {
>>>>              mutex_lock(&globalmutex_for_mac0_2g_mac1_5g);
>>>>              lock_mac0_2g_mac1_5g = true;
>>>>      }
> 
> globalmutex_for_mac0_2g_mac1_5g is only used in rtl92du_hw_init(), and
> globalmutex_for_power_and_efuse does very similar thing. Can we combine them
> into one? Since both are only used in rtl92du_hw_init(), it would not be a
> problem to enlarge their critical section.  
> 
> 
>>
>> * Dual MAC dual PHY: this I can't find to buy anymore. This
>> appears in the system as two Wifi devices, each working on
>> a different band. It has two USB interfaces. Two instances
>> of the driver access the same device. This is what the
>> mutexes are for.
> 
> I traced the code, and found rules of two MAC/PHY are:
> 1. read efuse to decide single or two MAC/PHY
> 2.1. if single MAC/PHY, register_hw as 2T2R (done)
> 2.2. if dual MAC/PHY (to step 3)
> 3. read interface index (USB: from bInterfaceNumber; PCI: from PCI onfigure space)
> 4. register interface index 0 as 1T1R on 5 GHz band only.
>    register interface index 1 as 1T1R on 2 GHz band only.
> 
> This is the case two instances (netdev) access single one hardware device,
> so seemingly it is hard to avoid global locks to prevent racing between them.
> An alternative thought is to support only single MAC/PHY, but not sure if
> driver can override setting of efuse that programmed the card as two MAC/PHY.
> 
>>
>> I said earlier that I think two devices can work at the same
>> time, even with the global mutexes, but now I remembered there
>> are two more global variables: curveindex_5g[] and
>> curveindex_2g[] in phy.c. One driver instance fills one array
>> during LC calibration, but the other driver instance reads
>> from the array when switching the channel. If I'm reading this
>> right. So two devices plugged in at the same time probably
>> won't work correctly.
> 
> That should be a problem. 
> 
>>
>> How can you avoid this when the hardware is the way it is?
>> My one idea is to add a global map to hold the mutexes and
>> arrays, using the USB port number, etc as the key.
> 
> Seemingly we need something like per device data. 
> 

I got another idea: if we have a guarantee that the two USB
interfaces are probed one at a time, then we can move the global
things into struct rtl_priv. The first probe call will allocate
the arrays and initialise the mutexes. The second probe call
will obtain those from the first struct rtl_priv:

int rtl_usb_probe(struct usb_interface *intf, ...) {
	udev = interface_to_usbdev(intf);
	struct ieee80211_hw *first_hw = usb_get_intfdata(udev->actconfig->interface[0]);
	struct rtl_priv *first_rtlpriv = rtl_priv(first_hw);

Something like that.

I am having regrets...
Ping-Ke Shih March 29, 2024, 12:34 a.m. UTC | #8
> I got another idea: if we have a guarantee that the two USB
> interfaces are probed one at a time, then we can move the global
> things into struct rtl_priv. The first probe call will allocate
> the arrays and initialise the mutexes. The second probe call
> will obtain those from the first struct rtl_priv:
> 
> int rtl_usb_probe(struct usb_interface *intf, ...) {
>         udev = interface_to_usbdev(intf);
>         struct ieee80211_hw *first_hw = usb_get_intfdata(udev->actconfig->interface[0]);
>         struct rtl_priv *first_rtlpriv = rtl_priv(first_hw);
> 

Could it have racing when obtaining mutexes from second probe? 
Should we need a lock to ensure the sequence? 

When driver is going to down, how can mutexes get free safely?

It seems like we still need another global lock to ensure that.
Bitterblue Smith March 31, 2024, 6:48 p.m. UTC | #9
On 29/03/2024 02:34, Ping-Ke Shih wrote:
> 
>> I got another idea: if we have a guarantee that the two USB
>> interfaces are probed one at a time, then we can move the global
>> things into struct rtl_priv. The first probe call will allocate
>> the arrays and initialise the mutexes. The second probe call
>> will obtain those from the first struct rtl_priv:
>>
>> int rtl_usb_probe(struct usb_interface *intf, ...) {
>>         udev = interface_to_usbdev(intf);
>>         struct ieee80211_hw *first_hw = usb_get_intfdata(udev->actconfig->interface[0]);
>>         struct rtl_priv *first_rtlpriv = rtl_priv(first_hw);
>>
> 
> Could it have racing when obtaining mutexes from second probe? 
> Should we need a lock to ensure the sequence? 
> 
> When driver is going to down, how can mutexes get free safely?
> 
> It seems like we still need another global lock to ensure that. 
> 

I asked linux-usb and they said the two interfaces are probed
and disconnected one at a time, so it should be fine without
another mutex:

https://lore.kernel.org/linux-usb/2024032907-smokeless-imperial-f3f9@gregkh/
Ping-Ke Shih April 1, 2024, 1:21 a.m. UTC | #10
> 
> I asked linux-usb and they said the two interfaces are probed
> and disconnected one at a time, so it should be fine without
> another mutex:
> 
> https://lore.kernel.org/linux-usb/2024032907-smokeless-imperial-f3f9@gregkh/

Thanks for the clarification. Then, will you dynamically allocate mutex
by first interface with ref_cnt=1, and second interface obtains mutex from
first interface and increases ref_cnt=2?

When USB disconnection, decrease ref_cnt and if ref_cnt==0 free the mutex 
no matter which one disconnect first. 

My thinking above is the same as yours?
Bitterblue Smith April 7, 2024, 12:03 p.m. UTC | #11
On 01/04/2024 04:21, Ping-Ke Shih wrote:
>>
>> I asked linux-usb and they said the two interfaces are probed
>> and disconnected one at a time, so it should be fine without
>> another mutex:
>>
>> https://lore.kernel.org/linux-usb/2024032907-smokeless-imperial-f3f9@gregkh/
> 
> Thanks for the clarification. Then, will you dynamically allocate mutex
> by first interface with ref_cnt=1, and second interface obtains mutex from
> first interface and increases ref_cnt=2?
> 
> When USB disconnection, decrease ref_cnt and if ref_cnt==0 free the mutex 
> no matter which one disconnect first. 
> 
> My thinking above is the same as yours?
> 

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


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;

	udev = interface_to_usbdev(intf);

	return usb_ifnum_to_if(udev, other_interfaceindex);
}

static int rtl92du_init_shared_data(struct ieee80211_hw *hw)
{
	struct usb_interface *other_intf = rtl92du_get_other_intf(hw);
	struct rtl_priv *rtlpriv = rtl_priv(hw);
	struct rtl_priv *other_rtlpriv = NULL;
	struct ieee80211_hw *other_hw = NULL;

	if (other_intf)
		other_hw = usb_get_intfdata(other_intf);

	if (other_hw) {
		/* The other interface was already probed. */
		other_rtlpriv = rtl_priv(other_hw);
		rtlpriv->curveindex_2g = other_rtlpriv->curveindex_2g;
		rtlpriv->curveindex_5g = other_rtlpriv->curveindex_5g;
		rtlpriv->mutex_for_power_on_off = other_rtlpriv->mutex_for_power_on_off;
		rtlpriv->mutex_for_hw_init = other_rtlpriv->mutex_for_hw_init;

		if (!rtlpriv->curveindex_2g || !rtlpriv->curveindex_5g ||
		    !rtlpriv->mutex_for_power_on_off || !rtlpriv->mutex_for_hw_init)
			return 1;

		return 0;
	}

	/* The other interface doesn't exist or was not probed yet. */
	rtlpriv->curveindex_2g =
		kzalloc(TARGET_CHNL_NUM_2G * sizeof(*rtlpriv->curveindex_2g),
			GFP_KERNEL);
	rtlpriv->curveindex_5g =
		kzalloc(TARGET_CHNL_NUM_5G * sizeof(*rtlpriv->curveindex_5g),
			GFP_KERNEL);
	rtlpriv->mutex_for_power_on_off =
		kzalloc(sizeof(*rtlpriv->mutex_for_power_on_off), GFP_KERNEL);
	rtlpriv->mutex_for_hw_init =
		kzalloc(sizeof(*rtlpriv->mutex_for_hw_init), GFP_KERNEL);

	if (!rtlpriv->curveindex_2g || !rtlpriv->curveindex_5g ||
	    !rtlpriv->mutex_for_power_on_off || !rtlpriv->mutex_for_hw_init) {
		kfree(rtlpriv->curveindex_2g);
		kfree(rtlpriv->curveindex_5g);
		kfree(rtlpriv->mutex_for_power_on_off);
		kfree(rtlpriv->mutex_for_hw_init);
		rtlpriv->curveindex_2g = NULL;
		rtlpriv->curveindex_5g = NULL;
		rtlpriv->mutex_for_power_on_off = NULL;
		rtlpriv->mutex_for_hw_init = NULL;
		return 1;
	}

	mutex_init(rtlpriv->mutex_for_power_on_off);
	mutex_init(rtlpriv->mutex_for_hw_init);

	return 0;
}

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. */
		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);
	}
}

static int rtl92du_init_sw_vars(struct ieee80211_hw *hw)
{
	const char *fw_name = "rtlwifi/rtl8192dufw.bin";
	struct rtl_priv *rtlpriv = rtl_priv(hw);
	int err;

	if (rtl92du_init_shared_data(hw))
		return 1;

	[...]

	return 0;
}

static void rtl92du_deinit_sw_vars(struct ieee80211_hw *hw)
{
	struct rtl_priv *rtlpriv = rtl_priv(hw);

	kfree(rtlpriv->rtlhal.pfirmware);
	rtlpriv->rtlhal.pfirmware = NULL;

	rtl92du_deinit_shared_data(hw);
}
Ping-Ke Shih April 8, 2024, 2:45 a.m. UTC | #12
> 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);
>         }
> }
> 

[...]
Bitterblue Smith April 8, 2024, 9:01 a.m. UTC | #13
On 08/04/2024 05:45, Ping-Ke Shih wrote:
> 
>> 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?
> 

Maybe. Sure, the second disconnect can free them.

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

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

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);
Ping-Ke Shih April 9, 2024, 12:27 a.m. UTC | #14
Bitterblue Smith <rtl8821cerfe2@gmail.com> 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.
Bitterblue Smith April 9, 2024, 11:16 a.m. UTC | #15
On 09/04/2024 03:27, Ping-Ke Shih wrote:
> 
> 
> Bitterblue Smith <rtl8821cerfe2@gmail.com> 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().
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c
new file mode 100644
index 000000000000..6d7f40e7add5
--- /dev/null
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.c
@@ -0,0 +1,312 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2009-2012  Realtek Corporation.*/
+
+#include "../wifi.h"
+#include "../core.h"
+#include "../usb.h"
+#include "../base.h"
+#include "../rtl8192d/reg.h"
+#include "../rtl8192d/def.h"
+#include "../rtl8192d/fw_common.h"
+#include "../rtl8192d/hw_common.h"
+#include "../rtl8192d/phy_common.h"
+#include "../rtl8192d/trx_common.h"
+#include "phy.h"
+#include "dm.h"
+#include "fw.h"
+#include "hw.h"
+#include "sw.h"
+#include "trx.h"
+#include "led.h"
+
+#include <linux/module.h>
+
+static int rtl92du_init_sw_vars(struct ieee80211_hw *hw)
+{
+	const char *fw_name = "rtlwifi/rtl8192dufw.bin";
+	struct rtl_priv *rtlpriv = rtl_priv(hw);
+	int err;
+
+	rtlpriv->dm.dm_initialgain_enable = true;
+	rtlpriv->dm.dm_flag = 0;
+	rtlpriv->dm.disable_framebursting = false;
+	rtlpriv->dm.thermalvalue = 0;
+	rtlpriv->dm.useramask = true;
+
+	/* dual mac */
+	if (rtlpriv->rtlhal.current_bandtype == BAND_ON_5G)
+		rtlpriv->phy.current_channel = 36;
+	else
+		rtlpriv->phy.current_channel = 1;
+
+	if (rtlpriv->rtlhal.macphymode != SINGLEMAC_SINGLEPHY)
+		rtlpriv->rtlhal.disable_amsdu_8k = true;
+
+	/* for LPS & IPS */
+	rtlpriv->psc.inactiveps = rtlpriv->cfg->mod_params->inactiveps;
+	rtlpriv->psc.swctrl_lps = rtlpriv->cfg->mod_params->swctrl_lps;
+	rtlpriv->psc.fwctrl_lps = rtlpriv->cfg->mod_params->fwctrl_lps;
+	if (!rtlpriv->psc.inactiveps)
+		pr_info("Inactive Power Save off (module option)\n");
+
+	/* for early mode */
+	rtlpriv->rtlhal.earlymode_enable = false;
+
+	/* for firmware buf */
+	rtlpriv->rtlhal.pfirmware = kmalloc(0x8000, GFP_KERNEL);
+	if (!rtlpriv->rtlhal.pfirmware) {
+		pr_err("Can't alloc buffer for fw\n");
+		return 1;
+	}
+
+	rtlpriv->max_fw_size = 0x8000;
+	pr_info("Driver for Realtek RTL8192DU WLAN interface\n");
+	pr_info("Loading firmware file %s\n", fw_name);
+
+	/* request fw */
+	err = request_firmware_nowait(THIS_MODULE, 1, fw_name,
+				      rtlpriv->io.dev, GFP_KERNEL, hw,
+				      rtl_fw_cb);
+	if (err) {
+		pr_err("Failed to request firmware!\n");
+		kfree(rtlpriv->rtlhal.pfirmware);
+		rtlpriv->rtlhal.pfirmware = NULL;
+		return 1;
+	}
+
+	return 0;
+}
+
+static void rtl92du_deinit_sw_vars(struct ieee80211_hw *hw)
+{
+	struct rtl_priv *rtlpriv = rtl_priv(hw);
+
+	kfree(rtlpriv->rtlhal.pfirmware);
+	rtlpriv->rtlhal.pfirmware = NULL;
+}
+
+static struct rtl_hal_ops rtl8192du_hal_ops = {
+	.init_sw_vars = rtl92du_init_sw_vars,
+	.deinit_sw_vars = rtl92du_deinit_sw_vars,
+	.read_chip_version = rtl92du_read_chip_version,
+	.read_eeprom_info = rtl92de_read_eeprom_info,
+	.hw_init = rtl92du_hw_init,
+	.hw_disable = rtl92du_card_disable,
+	.enable_interrupt = rtl92de_enable_interrupt,
+	.disable_interrupt = rtl92de_disable_interrupt,
+	.set_network_type = rtl92de_set_network_type,
+	.set_chk_bssid = rtl92de_set_check_bssid,
+	.set_qos = rtl92de_set_qos,
+	.set_bcn_reg = rtl92de_set_beacon_related_registers,
+	.set_bcn_intv = rtl92de_set_beacon_interval,
+	.update_interrupt_mask = rtl92de_update_interrupt_mask,
+	.get_hw_reg = rtl92du_get_hw_reg,
+	.set_hw_reg = rtl92du_set_hw_reg,
+	.update_rate_tbl = rtl92de_update_hal_rate_tbl,
+	.fill_tx_desc = rtl92du_tx_fill_desc,
+	.query_rx_desc = rtl92de_rx_query_desc,
+	.set_channel_access = rtl92de_update_channel_access_setting,
+	.radio_onoff_checking = rtl92de_gpio_radio_on_off_checking,
+	.set_bw_mode = rtl92d_phy_set_bw_mode,
+	.switch_channel = rtl92d_phy_sw_chnl,
+	.dm_watchdog = rtl92du_dm_watchdog,
+	.scan_operation_backup = rtl_phy_scan_operation_backup,
+	.set_rf_power_state = rtl92d_phy_set_rf_power_state,
+	.led_control = rtl92de_led_control,
+	.set_desc = rtl92de_set_desc,
+	.get_desc = rtl92de_get_desc,
+	.enable_hw_sec = rtl92de_enable_hw_security_config,
+	.set_key = rtl92de_set_key,
+	.get_bbreg = rtl92d_phy_query_bb_reg,
+	.set_bbreg = rtl92d_phy_set_bb_reg,
+	.get_rfreg = rtl92d_phy_query_rf_reg,
+	.set_rfreg = rtl92d_phy_set_rf_reg,
+	.linked_set_reg = rtl92d_linked_set_reg,
+	.fill_h2c_cmd = rtl92d_fill_h2c_cmd,
+	.get_btc_status = rtl_btc_status_false,
+	.phy_iq_calibrate = rtl92d_phy_iq_calibrate,
+	.phy_lc_calibrate = rtl92d_phy_lc_calibrate,
+};
+
+static struct rtl_mod_params rtl92du_mod_params = {
+	.sw_crypto = false,
+	.inactiveps = false,
+	.swctrl_lps = false,
+	.debug_level = 0,
+	.debug_mask = 0,
+};
+
+static struct rtl_hal_usbint_cfg rtl92du_interface_cfg = {
+	/* rx */
+	.rx_urb_num = 8,
+	.rx_max_size = 15360,
+	.usb_rx_hdl = NULL,
+	.usb_rx_segregate_hdl = NULL,
+	/* tx */
+	.usb_tx_cleanup = rtl92du_tx_cleanup,
+	.usb_tx_post_hdl = rtl92du_tx_post_hdl,
+	.usb_tx_aggregate_hdl = rtl92du_tx_aggregate_hdl,
+	.usb_endpoint_mapping = rtl92du_endpoint_mapping,
+	.usb_mq_to_hwq = rtl92du_mq_to_hwq,
+};
+
+static struct rtl_hal_cfg rtl92du_hal_cfg = {
+	.name = "rtl8192du",
+	.ops = &rtl8192du_hal_ops,
+	.mod_params = &rtl92du_mod_params,
+	.usb_interface_cfg = &rtl92du_interface_cfg,
+
+	.maps[SYS_ISO_CTRL] = REG_SYS_ISO_CTRL,
+	.maps[SYS_FUNC_EN] = REG_SYS_FUNC_EN,
+	.maps[SYS_CLK] = REG_SYS_CLKR,
+	.maps[MAC_RCR_AM] = RCR_AM,
+	.maps[MAC_RCR_AB] = RCR_AB,
+	.maps[MAC_RCR_ACRC32] = RCR_ACRC32,
+	.maps[MAC_RCR_ACF] = RCR_ACF,
+	.maps[MAC_RCR_AAP] = RCR_AAP,
+
+	.maps[EFUSE_TEST] = REG_EFUSE_TEST,
+	.maps[EFUSE_ACCESS] = REG_EFUSE_ACCESS,
+	.maps[EFUSE_CTRL] = REG_EFUSE_CTRL,
+	.maps[EFUSE_CLK] = 0,	/* just for 92se */
+	.maps[EFUSE_CLK_CTRL] = REG_EFUSE_CTRL,
+	.maps[EFUSE_PWC_EV12V] = PWC_EV12V,
+	.maps[EFUSE_FEN_ELDR] = FEN_ELDR,
+	.maps[EFUSE_LOADER_CLK_EN] = 0,
+	.maps[EFUSE_ANA8M] = 0,	/* just for 92se */
+	.maps[EFUSE_HWSET_MAX_SIZE] = HWSET_MAX_SIZE,
+	.maps[EFUSE_MAX_SECTION_MAP] = EFUSE_MAX_SECTION,
+	.maps[EFUSE_REAL_CONTENT_SIZE] = EFUSE_REAL_CONTENT_LEN,
+
+	.maps[RWCAM] = REG_CAMCMD,
+	.maps[WCAMI] = REG_CAMWRITE,
+	.maps[RCAMO] = REG_CAMREAD,
+	.maps[CAMDBG] = REG_CAMDBG,
+	.maps[SECR] = REG_SECCFG,
+	.maps[SEC_CAM_NONE] = CAM_NONE,
+	.maps[SEC_CAM_WEP40] = CAM_WEP40,
+	.maps[SEC_CAM_TKIP] = CAM_TKIP,
+	.maps[SEC_CAM_AES] = CAM_AES,
+	.maps[SEC_CAM_WEP104] = CAM_WEP104,
+
+	.maps[RTL_IMR_BCNDMAINT6] = IMR_BCNDMAINT6,
+	.maps[RTL_IMR_BCNDMAINT5] = IMR_BCNDMAINT5,
+	.maps[RTL_IMR_BCNDMAINT4] = IMR_BCNDMAINT4,
+	.maps[RTL_IMR_BCNDMAINT3] = IMR_BCNDMAINT3,
+	.maps[RTL_IMR_BCNDMAINT2] = IMR_BCNDMAINT2,
+	.maps[RTL_IMR_BCNDMAINT1] = IMR_BCNDMAINT1,
+	.maps[RTL_IMR_BCNDOK8] = IMR_BCNDOK8,
+	.maps[RTL_IMR_BCNDOK7] = IMR_BCNDOK7,
+	.maps[RTL_IMR_BCNDOK6] = IMR_BCNDOK6,
+	.maps[RTL_IMR_BCNDOK5] = IMR_BCNDOK5,
+	.maps[RTL_IMR_BCNDOK4] = IMR_BCNDOK4,
+	.maps[RTL_IMR_BCNDOK3] = IMR_BCNDOK3,
+	.maps[RTL_IMR_BCNDOK2] = IMR_BCNDOK2,
+	.maps[RTL_IMR_BCNDOK1] = IMR_BCNDOK1,
+	.maps[RTL_IMR_TIMEOUT2] = IMR_TIMEOUT2,
+	.maps[RTL_IMR_TIMEOUT1] = IMR_TIMEOUT1,
+
+	.maps[RTL_IMR_TXFOVW] = IMR_TXFOVW,
+	.maps[RTL_IMR_PSTIMEOUT] = IMR_PSTIMEOUT,
+	.maps[RTL_IMR_BCNINT] = IMR_BCNINT,
+	.maps[RTL_IMR_RXFOVW] = IMR_RXFOVW,
+	.maps[RTL_IMR_RDU] = IMR_RDU,
+	.maps[RTL_IMR_ATIMEND] = IMR_ATIMEND,
+	.maps[RTL_IMR_BDOK] = IMR_BDOK,
+	.maps[RTL_IMR_MGNTDOK] = IMR_MGNTDOK,
+	.maps[RTL_IMR_TBDER] = IMR_TBDER,
+	.maps[RTL_IMR_HIGHDOK] = IMR_HIGHDOK,
+	.maps[RTL_IMR_TBDOK] = IMR_TBDOK,
+	.maps[RTL_IMR_BKDOK] = IMR_BKDOK,
+	.maps[RTL_IMR_BEDOK] = IMR_BEDOK,
+	.maps[RTL_IMR_VIDOK] = IMR_VIDOK,
+	.maps[RTL_IMR_VODOK] = IMR_VODOK,
+	.maps[RTL_IMR_ROK] = IMR_ROK,
+	.maps[RTL_IBSS_INT_MASKS] = (IMR_BCNINT | IMR_TBDOK | IMR_TBDER),
+
+	.maps[RTL_RC_CCK_RATE1M] = DESC_RATE1M,
+	.maps[RTL_RC_CCK_RATE2M] = DESC_RATE2M,
+	.maps[RTL_RC_CCK_RATE5_5M] = DESC_RATE5_5M,
+	.maps[RTL_RC_CCK_RATE11M] = DESC_RATE11M,
+	.maps[RTL_RC_OFDM_RATE6M] = DESC_RATE6M,
+	.maps[RTL_RC_OFDM_RATE9M] = DESC_RATE9M,
+	.maps[RTL_RC_OFDM_RATE12M] = DESC_RATE12M,
+	.maps[RTL_RC_OFDM_RATE18M] = DESC_RATE18M,
+	.maps[RTL_RC_OFDM_RATE24M] = DESC_RATE24M,
+	.maps[RTL_RC_OFDM_RATE36M] = DESC_RATE36M,
+	.maps[RTL_RC_OFDM_RATE48M] = DESC_RATE48M,
+	.maps[RTL_RC_OFDM_RATE54M] = DESC_RATE54M,
+
+	.maps[RTL_RC_HT_RATEMCS7] = DESC_RATEMCS7,
+	.maps[RTL_RC_HT_RATEMCS15] = DESC_RATEMCS15,
+};
+
+MODULE_AUTHOR("lizhaoming	<chaoming_li@realsil.com.cn>");
+MODULE_AUTHOR("Realtek WlanFAE	<wlanfae@realtek.com>");
+MODULE_AUTHOR("Larry Finger	<Larry.Finger@lwfinger.net>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Realtek 8192DU 802.11an Dual Mac USB wireless");
+MODULE_FIRMWARE("rtlwifi/rtl8192dufw.bin");
+
+module_param_named(swenc, rtl92du_mod_params.sw_crypto, bool, 0444);
+module_param_named(debug_level, rtl92du_mod_params.debug_level, int, 0644);
+module_param_named(ips, rtl92du_mod_params.inactiveps, bool, 0444);
+module_param_named(swlps, rtl92du_mod_params.swctrl_lps, bool, 0444);
+module_param_named(debug_mask, rtl92du_mod_params.debug_mask, ullong, 0644);
+MODULE_PARM_DESC(swenc, "Set to 1 for software crypto (default 0)\n");
+MODULE_PARM_DESC(ips, "Set to 0 to not use link power save (default 0)\n");
+MODULE_PARM_DESC(swlps, "Set to 1 to use SW control power save (default 0)\n");
+MODULE_PARM_DESC(debug_level, "Set debug level (0-5) (default 0)");
+MODULE_PARM_DESC(debug_mask, "Set debug mask (default 0)");
+
+/* Add global mutex to solve the problem that
+ * dual mac register operation on the same time
+ */
+DEFINE_MUTEX(globalmutex_power);
+DEFINE_MUTEX(globalmutex_for_fwdownload);
+DEFINE_MUTEX(globalmutex_for_power_and_efuse);
+DEFINE_MUTEX(globalmutex_for_mac0_2g_mac1_5g);
+
+#define USB_VENDOR_ID_REALTEK		0x0bda
+
+static const struct usb_device_id rtl8192d_usb_ids[] = {
+	{RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8193, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8194, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8111, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x0193, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0x8171, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(USB_VENDOR_ID_REALTEK, 0xe194, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x2019, 0xab2c, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x2019, 0xab2d, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x2019, 0x4903, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x2019, 0x4904, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x07b8, 0x8193, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x20f4, 0x664b, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x04dd, 0x954f, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x04dd, 0x96a6, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x050d, 0x110a, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x050d, 0x1105, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x050d, 0x120a, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x1668, 0x8102, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x0930, 0x0a0a, rtl92du_hal_cfg)},
+	{RTL_USB_DEVICE(0x2001, 0x330c, rtl92du_hal_cfg)},
+	{}
+};
+
+MODULE_DEVICE_TABLE(usb, rtl8192d_usb_ids);
+
+static int rtl8192du_probe(struct usb_interface *intf,
+			   const struct usb_device_id *id)
+{
+	return rtl_usb_probe(intf, id, &rtl92du_hal_cfg);
+}
+
+static struct usb_driver rtl8192du_driver = {
+	.name = "rtl8192du",
+	.probe = rtl8192du_probe,
+	.disconnect = rtl_usb_disconnect,
+	.id_table = rtl8192d_usb_ids,
+	.disable_hub_initiated_lpm = 1,
+};
+
+module_usb_driver(rtl8192du_driver);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h
new file mode 100644
index 000000000000..364d9a471dc0
--- /dev/null
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/sw.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2009-2012  Realtek Corporation.*/
+
+#ifndef __RTL92DE_SW_H__
+#define __RTL92DE_SW_H__
+
+extern struct mutex globalmutex_power;
+extern struct mutex globalmutex_for_fwdownload;
+extern struct mutex globalmutex_for_power_and_efuse;
+extern struct mutex globalmutex_for_mac0_2g_mac1_5g;
+
+#endif