| Message ID | 1399278818-19152-1-git-send-email-adam.lee@canonical.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 <adam.lee@canonical.com> Acked-by: Larry Finger <Larry.Finger@lwfinger.net> (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 { > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 05 May 2014 09:56:16 -0500 Larry Finger <Larry.Finger@lwfinger.net> 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 <adam.lee@canonical.com> > > Acked-by: Larry Finger <Larry.Finger@lwfinger.net> (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. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 05, 2014 at 03:40:37PM -0700, Stephen Hemminger wrote: > On Mon, 05 May 2014 09:56:16 -0500 > Larry Finger <Larry.Finger@lwfinger.net> 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 <adam.lee@canonical.com> > > > > Acked-by: Larry Finger <Larry.Finger@lwfinger.net> (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.
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 <Larry.Finger@lwfinger.net> 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 <adam.lee@canonical.com> > > > > > > Acked-by: Larry Finger <Larry.Finger@lwfinger.net> (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'?
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.
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.
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 {
This makes MSI support a module parameter, for debugging and workaround convenience. Signed-off-by: Adam Lee <adam.lee@canonical.com> --- drivers/net/wireless/rtlwifi/wifi.h | 4 ++++ 1 file changed, 4 insertions(+)