| Message ID | 20201028142433.18501-1-kitakar@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mwifiex: disable ps_mode by default for stability | expand |
On Wed, Oct 28, 2020 at 11:24:30PM +0900, Tsuchiya Yuto wrote: > Hello all, > > On Microsoft Surface devices (PCIe-88W8897), we are observing stability > issues when ps_mode (IEEE power_save) is enabled, then eventually causes > firmware crash. Especially on 5GHz APs, the connection is completely > unstable and almost unusable. > > I think the most desirable change is to fix the ps_mode itself. But is > seems to be hard work [1], I'm afraid we have to go this way. > > Therefore, the first patch of this series disables the ps_mode by default > instead of enabling it on driver init. I'm not sure if explicitly > disabling it is really required or not. I don't have access to the details > of this chip. Let me know if it's enough to just remove the code that > enables ps_mode. > > The Second patch adds a new module parameter named "allow_ps_mode". Since > other wifi drivers just disable power_save by default by module parameter > like this, I also added this. > > The third patch adds a message when ps_mode will be changed. Useful when > diagnosing connection issues. > [1] https://bugzilla.kernel.org/show_bug.cgi?id=109681 Can you attach this to the actual patch as BugLink: tag?
On Wed, 2020-10-28 at 17:21 +0200, Andy Shevchenko wrote: > On Wed, Oct 28, 2020 at 11:24:30PM +0900, Tsuchiya Yuto wrote: > > Hello all, > > > > On Microsoft Surface devices (PCIe-88W8897), we are observing stability > > issues when ps_mode (IEEE power_save) is enabled, then eventually causes > > firmware crash. Especially on 5GHz APs, the connection is completely > > unstable and almost unusable. > > > > I think the most desirable change is to fix the ps_mode itself. But is > > seems to be hard work [1], I'm afraid we have to go this way. > > > > Therefore, the first patch of this series disables the ps_mode by default > > instead of enabling it on driver init. I'm not sure if explicitly > > disabling it is really required or not. I don't have access to the details > > of this chip. Let me know if it's enough to just remove the code that > > enables ps_mode. > > > > The Second patch adds a new module parameter named "allow_ps_mode". Since > > other wifi drivers just disable power_save by default by module parameter > > like this, I also added this. > > > > The third patch adds a message when ps_mode will be changed. Useful when > > diagnosing connection issues. > > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=109681 > > Can you attach this to the actual patch as BugLink: tag? > Thanks! Indeed I should have added this... I wrote it in the replies. If I send the v2 version of this series, I'll add it to them.
Hello! Please CC me in future for mwifiex discussion :-) On Wednesday 28 October 2020 23:24:30 Tsuchiya Yuto wrote: > Hello all, > > On Microsoft Surface devices (PCIe-88W8897), we are observing stability > issues when ps_mode (IEEE power_save) is enabled, then eventually causes > firmware crash. Especially on 5GHz APs, the connection is completely > unstable and almost unusable. > > I think the most desirable change is to fix the ps_mode itself. But is > seems to be hard work [1], I'm afraid we have to go this way. > > Therefore, the first patch of this series disables the ps_mode by default > instead of enabling it on driver init. I'm not sure if explicitly > disabling it is really required or not. I don't have access to the details > of this chip. Let me know if it's enough to just remove the code that > enables ps_mode. > > The Second patch adds a new module parameter named "allow_ps_mode". Since > other wifi drivers just disable power_save by default by module parameter > like this, I also added this. > > The third patch adds a message when ps_mode will be changed. Useful when > diagnosing connection issues. There are more issues with power save API and implementation in mwifiex. See my email for more details: https://lore.kernel.org/linux-wireless/20200609111544.v7u5ort3yk4s7coy@pali/T/#u These patches would just break power save API and reporting status to userspace even more due to WIPHY_FLAG_PS_ON_BY_DEFAULT and CONFIG_CFG80211_DEFAULT_PS options. I would suggest to first fix issues mentioned in my email and then start providing a way how to blacklist or whitelist power save feature depending on firmware or card/chip version. From my experience I know that e.g. 88W8997 cards have lot of bugs in their firmware and I'm not aware that bugs are going to be fixed... So we really need workarounds, like disabling power save mode to have cards usable. > Thanks, > Tsuchiya Yuto > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=109681 > > Tsuchiya Yuto (3): > mwifiex: disable ps_mode explicitly by default instead > mwifiex: add allow_ps_mode module parameter > mwifiex: print message when changing ps_mode > > .../net/wireless/marvell/mwifiex/cfg80211.c | 23 +++++++++++++++++++ > .../net/wireless/marvell/mwifiex/sta_cmd.c | 11 ++++++--- > 2 files changed, 31 insertions(+), 3 deletions(-) > > -- > 2.29.1 >