2014-05-05 08:33:45

by Adam Lee

[permalink] [raw]
Subject: [PATCH 1/3] rtlwifi: make MSI support a module parameter

This makes MSI support a module parameter, for debugging and workaround
convenience.

Signed-off-by: Adam Lee <[email protected]>
---
drivers/net/wireless/rtlwifi/wifi.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
index 6965afd..eef93d1 100644
--- a/drivers/net/wireless/rtlwifi/wifi.h
+++ b/drivers/net/wireless/rtlwifi/wifi.h
@@ -2030,6 +2030,10 @@ struct rtl_mod_params {

/* default: 1 = using linked fw power save */
bool fwctrl_lps;
+
+ /* default: 0 = not using MSI interrupts mode */
+ /* submodules should set their own defalut value */
+ bool msi_support;
};

struct rtl_hal_usbint_cfg {
--
2.0.0.rc0



2014-05-06 18:15:11

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 1/3] rtlwifi: make MSI support a module parameter

On Tue, May 06, 2014 at 10:19:15AM +0800, Adam Lee wrote:
> On Mon, May 05, 2014 at 03:40:37PM -0700, Stephen Hemminger wrote:
> > On Mon, 05 May 2014 09:56:16 -0500
> > Larry Finger <[email protected]> wrote:
> >
> > > On 05/05/2014 03:33 AM, Adam Lee wrote:
> > > > This makes MSI support a module parameter, for debugging and workaround
> > > > convenience.
> > > >
> > > > Signed-off-by: Adam Lee <[email protected]>
> > >
> > > Acked-by: Larry Finger <[email protected]> (for all 3 patches)
> > >
> > > I would have made the default for the MSI option to be true, but that is a
> > > matter of preference, and only experience would show which default leads to the
> > > fewer failures. On my laptop, both rtl8188ee and rtl8723be work either way. I am
> > > now working on a driver for the RTL8192EE that also can use MSI - that has only
> > > been tested with the option on.
> > >
> > > Larry
> >
> > Standard practice is to assume MSI is available, and let the quirks
> > in the PCI subsystem reject the request to enable MSI.
> >
> > Also other drivers have a 'disable_msi' module parameter why not follow
> > their example.
> >
>
> Because some submodule's MSI causes an regression, and other submodules
> of rtlwifi are not fully tested under MSI, we need to disable it by
> default(regression has higher priority) and have an 'enable_msi' module
> parameter for some certain users.

Couldn't it be called 'disable_msi' and default to 'on'?

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2014-05-05 14:56:19

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 1/3] rtlwifi: make MSI support a module parameter

On 05/05/2014 03:33 AM, Adam Lee wrote:
> This makes MSI support a module parameter, for debugging and workaround
> convenience.
>
> Signed-off-by: Adam Lee <[email protected]>

Acked-by: Larry Finger <[email protected]> (for all 3 patches)

I would have made the default for the MSI option to be true, but that is a
matter of preference, and only experience would show which default leads to the
fewer failures. On my laptop, both rtl8188ee and rtl8723be work either way. I am
now working on a driver for the RTL8192EE that also can use MSI - that has only
been tested with the option on.

Larry

> ---
> drivers/net/wireless/rtlwifi/wifi.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
> index 6965afd..eef93d1 100644
> --- a/drivers/net/wireless/rtlwifi/wifi.h
> +++ b/drivers/net/wireless/rtlwifi/wifi.h
> @@ -2030,6 +2030,10 @@ struct rtl_mod_params {
>
> /* default: 1 = using linked fw power save */
> bool fwctrl_lps;
> +
> + /* default: 0 = not using MSI interrupts mode */
> + /* submodules should set their own defalut value */
> + bool msi_support;
> };
>
> struct rtl_hal_usbint_cfg {
>


2014-05-05 22:40:41

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH 1/3] rtlwifi: make MSI support a module parameter

On Mon, 05 May 2014 09:56:16 -0500
Larry Finger <[email protected]> wrote:

> On 05/05/2014 03:33 AM, Adam Lee wrote:
> > This makes MSI support a module parameter, for debugging and workaround
> > convenience.
> >
> > Signed-off-by: Adam Lee <[email protected]>
>
> Acked-by: Larry Finger <[email protected]> (for all 3 patches)
>
> I would have made the default for the MSI option to be true, but that is a
> matter of preference, and only experience would show which default leads to the
> fewer failures. On my laptop, both rtl8188ee and rtl8723be work either way. I am
> now working on a driver for the RTL8192EE that also can use MSI - that has only
> been tested with the option on.
>
> Larry

Standard practice is to assume MSI is available, and let the quirks
in the PCI subsystem reject the request to enable MSI.

Also other drivers have a 'disable_msi' module parameter why not follow
their example.


2014-05-05 08:33:56

by Adam Lee

[permalink] [raw]
Subject: [PATCH 2/3] rtlwifi: rtl8188ee: add msi module parameter

The msi module parameter offers an option to enable or disable MSI
interrupts mode. For now, some users report RTL8188EE works only with
MSI on their certain platforms, some others report it works only without
MSI, this parameter will help.

Signed-off-by: Adam Lee <[email protected]>
---
drivers/net/wireless/rtlwifi/rtl8188ee/sw.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/rtlwifi/rtl8188ee/sw.c b/drivers/net/wireless/rtlwifi/rtl8188ee/sw.c
index 347af1e..79792d4 100644
--- a/drivers/net/wireless/rtlwifi/rtl8188ee/sw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8188ee/sw.c
@@ -93,6 +93,7 @@ int rtl88e_init_sw_vars(struct ieee80211_hw *hw)
u8 tid;

rtl8188ee_bt_reg_init(hw);
+ rtlpci->msi_support = rtlpriv->cfg->mod_params->msi_support;

rtlpriv->dm.dm_initialgain_enable = 1;
rtlpriv->dm.dm_flag = 0;
@@ -266,6 +267,7 @@ static struct rtl_mod_params rtl88ee_mod_params = {
.inactiveps = true,
.swctrl_lps = false,
.fwctrl_lps = true,
+ .msi_support = false,
.debug = DBG_EMERG,
};

@@ -382,10 +384,12 @@ module_param_named(debug, rtl88ee_mod_params.debug, int, 0444);
module_param_named(ips, rtl88ee_mod_params.inactiveps, bool, 0444);
module_param_named(swlps, rtl88ee_mod_params.swctrl_lps, bool, 0444);
module_param_named(fwlps, rtl88ee_mod_params.fwctrl_lps, bool, 0444);
+module_param_named(msi, rtl88ee_mod_params.msi_support, bool, 0444);
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 1)\n");
MODULE_PARM_DESC(swlps, "Set to 1 to use SW control power save (default 0)\n");
MODULE_PARM_DESC(fwlps, "Set to 1 to use FW control power save (default 1)\n");
+MODULE_PARM_DESC(msi, "Set to 1 to use MSI interrupts mode (default 0)\n");
MODULE_PARM_DESC(debug, "Set debug level (0-5) (default 0)");

static SIMPLE_DEV_PM_OPS(rtlwifi_pm_ops, rtl_pci_suspend, rtl_pci_resume);
--
2.0.0.rc0


2014-05-07 02:31:11

by Adam Lee

[permalink] [raw]
Subject: Re: [PATCH 1/3] rtlwifi: make MSI support a module parameter

On Tue, May 06, 2014 at 02:08:37PM -0400, John W. Linville wrote:
> > > Standard practice is to assume MSI is available, and let the quirks
> > > in the PCI subsystem reject the request to enable MSI.
> > >
> > > Also other drivers have a 'disable_msi' module parameter why not follow
> > > their example.
> > >
> >
> > Because some submodule's MSI causes an regression, and other submodules
> > of rtlwifi are not fully tested under MSI, we need to disable it by
> > default(regression has higher priority) and have an 'enable_msi' module
> > parameter for some certain users.
>
> Couldn't it be called 'disable_msi' and default to 'on'?

It could be, will send an v2 patchset, thanks.

--
Adam Lee

2014-05-06 02:19:32

by Adam Lee

[permalink] [raw]
Subject: Re: [PATCH 1/3] rtlwifi: make MSI support a module parameter

On Mon, May 05, 2014 at 03:40:37PM -0700, Stephen Hemminger wrote:
> On Mon, 05 May 2014 09:56:16 -0500
> Larry Finger <[email protected]> wrote:
>
> > On 05/05/2014 03:33 AM, Adam Lee wrote:
> > > This makes MSI support a module parameter, for debugging and workaround
> > > convenience.
> > >
> > > Signed-off-by: Adam Lee <[email protected]>
> >
> > Acked-by: Larry Finger <[email protected]> (for all 3 patches)
> >
> > I would have made the default for the MSI option to be true, but that is a
> > matter of preference, and only experience would show which default leads to the
> > fewer failures. On my laptop, both rtl8188ee and rtl8723be work either way. I am
> > now working on a driver for the RTL8192EE that also can use MSI - that has only
> > been tested with the option on.
> >
> > Larry
>
> Standard practice is to assume MSI is available, and let the quirks
> in the PCI subsystem reject the request to enable MSI.
>
> Also other drivers have a 'disable_msi' module parameter why not follow
> their example.
>

Because some submodule's MSI causes an regression, and other submodules
of rtlwifi are not fully tested under MSI, we need to disable it by
default(regression has higher priority) and have an 'enable_msi' module
parameter for some certain users.

--
Adam Lee

2014-05-07 02:52:35

by Adam Lee

[permalink] [raw]
Subject: Re: [PATCH 1/3] rtlwifi: make MSI support a module parameter

On Wed, May 07, 2014 at 10:31:05AM +0800, Adam Lee wrote:
> On Tue, May 06, 2014 at 02:08:37PM -0400, John W. Linville wrote:
> > > > Standard practice is to assume MSI is available, and let the quirks
> > > > in the PCI subsystem reject the request to enable MSI.
> > > >
> > > > Also other drivers have a 'disable_msi' module parameter why not follow
> > > > their example.
> > > >
> > >
> > > Because some submodule's MSI causes an regression, and other submodules
> > > of rtlwifi are not fully tested under MSI, we need to disable it by
> > > default(regression has higher priority) and have an 'enable_msi' module
> > > parameter for some certain users.
> >
> > Couldn't it be called 'disable_msi' and default to 'on'?
>
> It could be, will send an v2 patchset, thanks.

Sorry, just grep linux/driver, there is no convention of 'disable_msi'
parameter, only 5 drivers uses that, but 15 drivers are using 'msi'...

Please still apply v1, setting 'disable_msi' to 'on' and asking users to
set it to 'off' is a little weird. John, please ask me to send v2 if you
think the 'disable_msi' parameter is better.

Thanks.

--
Adam Lee

2014-05-05 08:34:14

by Adam Lee

[permalink] [raw]
Subject: [PATCH 3/3] rtlwifi: rtl8723be: add msi module parameter

The msi module parameter offers an option to enable or disable MSI
interrupts mode, for debugging and workaround(in case) convenience.

Signed-off-by: Adam Lee <[email protected]>
---
drivers/net/wireless/rtlwifi/rtl8723be/sw.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8723be/sw.c b/drivers/net/wireless/rtlwifi/rtl8723be/sw.c
index a072136..ff12bf4 100644
--- a/drivers/net/wireless/rtlwifi/rtl8723be/sw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8723be/sw.c
@@ -92,7 +92,7 @@ int rtl8723be_init_sw_vars(struct ieee80211_hw *hw)
struct rtl_mac *mac = rtl_mac(rtl_priv(hw));

rtl8723be_bt_reg_init(hw);
- rtlpci->msi_support = false;
+ rtlpci->msi_support = rtlpriv->cfg->mod_params->msi_support;
rtlpriv->btcoexist.btc_ops = rtl_btc_get_ops_pointer();

rtlpriv->dm.dm_initialgain_enable = 1;
@@ -253,6 +253,7 @@ static struct rtl_mod_params rtl8723be_mod_params = {
.inactiveps = true,
.swctrl_lps = false,
.fwctrl_lps = true,
+ .msi_support = false,
.debug = DBG_EMERG,
};

@@ -365,9 +366,11 @@ module_param_named(debug, rtl8723be_mod_params.debug, int, 0444);
module_param_named(ips, rtl8723be_mod_params.inactiveps, bool, 0444);
module_param_named(swlps, rtl8723be_mod_params.swctrl_lps, bool, 0444);
module_param_named(fwlps, rtl8723be_mod_params.fwctrl_lps, bool, 0444);
+module_param_named(msi, rtl8723be_mod_params.msi_support, bool, 0444);
MODULE_PARM_DESC(swenc, "using hardware crypto (default 0 [hardware])\n");
MODULE_PARM_DESC(ips, "using no link power save (default 1 is open)\n");
MODULE_PARM_DESC(fwlps, "using linked fw control power save (default 1 is open)\n");
+MODULE_PARM_DESC(msi, "Set to 1 to use MSI interrupts mode (default 0)\n");
MODULE_PARM_DESC(debug, "Set debug level (0-5) (default 0)");

static SIMPLE_DEV_PM_OPS(rtlwifi_pm_ops, rtl_pci_suspend, rtl_pci_resume);
--
2.0.0.rc0