Fourth revision of this patch.
v1: https://lore.kernel.org/linux-wireless/[email protected]/T/
v2: https://lore.kernel.org/linux-wireless/[email protected]/T/
v3: https://lore.kernel.org/linux-wireless/[email protected]/T/
Changes between v3 and v4:
- Add patch to ensure 0-termination of version string
Jonas Dreßler (3):
mwifiex: Use a define for firmware version string length
mwifiex: Add quirk to disable deep sleep with certain hardware
revision
mwifiex: Ensure the version string from the firmware is 0-terminated
drivers/net/wireless/marvell/mwifiex/fw.h | 4 ++-
drivers/net/wireless/marvell/mwifiex/main.c | 18 ++++++++++++
drivers/net/wireless/marvell/mwifiex/main.h | 3 +-
.../wireless/marvell/mwifiex/sta_cmdresp.c | 28 +++++++++++++++++--
4 files changed, 49 insertions(+), 4 deletions(-)
--
2.33.1
The 88W8897 PCIe+USB card in the hardware revision 20 apparently has a
hardware issue where the card wakes up from deep sleep randomly and very
often, somewhat depending on the card activity, maybe the hardware has a
floating wakeup pin or something. This was found by comparing two MS
Surface Book 2 devices, where one devices wifi card experienced spurious
wakeups, while the other one didn't.
Those continuous wakeups prevent the card from entering host sleep when
the computer suspends. And because the host won't answer to events from
the card anymore while it's suspended, the firmwares internal power
saving state machine seems to get confused and the card can't sleep
anymore at all after that.
Since we can't work around that hardware bug in the firmware, let's
get the hardware revision string from the firmware and match it with
known bad revisions. Then disable auto deep sleep for those revisions,
which makes sure we no longer get those spurious wakeups.
Signed-off-by: Jonas Dreßler <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/main.c | 18 +++++++++++++++++
drivers/net/wireless/marvell/mwifiex/main.h | 1 +
.../wireless/marvell/mwifiex/sta_cmdresp.c | 20 +++++++++++++++++++
3 files changed, 39 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 19b996c6a260..ace7371c4773 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -226,6 +226,23 @@ static int mwifiex_process_rx(struct mwifiex_adapter *adapter)
return 0;
}
+static void maybe_quirk_fw_disable_ds(struct mwifiex_adapter *adapter)
+{
+ struct mwifiex_private *priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_STA);
+ struct mwifiex_ver_ext ver_ext;
+
+ if (test_and_set_bit(MWIFIEX_IS_REQUESTING_FW_VEREXT, &adapter->work_flags))
+ return;
+
+ memset(&ver_ext, 0, sizeof(ver_ext));
+ ver_ext.version_str_sel = 1;
+ if (mwifiex_send_cmd(priv, HostCmd_CMD_VERSION_EXT,
+ HostCmd_ACT_GEN_GET, 0, &ver_ext, false)) {
+ mwifiex_dbg(priv->adapter, MSG,
+ "Checking hardware revision failed.\n");
+ }
+}
+
/*
* The main process.
*
@@ -356,6 +373,7 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
if (adapter->hw_status == MWIFIEX_HW_STATUS_INIT_DONE) {
adapter->hw_status = MWIFIEX_HW_STATUS_READY;
mwifiex_init_fw_complete(adapter);
+ maybe_quirk_fw_disable_ds(adapter);
}
}
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 65609ea2327e..eabd0e0a9f56 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -524,6 +524,7 @@ enum mwifiex_adapter_work_flags {
MWIFIEX_IS_SUSPENDED,
MWIFIEX_IS_HS_CONFIGURED,
MWIFIEX_IS_HS_ENABLING,
+ MWIFIEX_IS_REQUESTING_FW_VEREXT,
};
struct mwifiex_band_config {
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 20b69a37f9e1..6c7b0b9bc4e9 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -708,6 +708,26 @@ static int mwifiex_ret_ver_ext(struct mwifiex_private *priv,
{
struct host_cmd_ds_version_ext *ver_ext = &resp->params.verext;
+ if (test_and_clear_bit(MWIFIEX_IS_REQUESTING_FW_VEREXT, &priv->adapter->work_flags)) {
+ if (strncmp(ver_ext->version_str, "ChipRev:20, BB:9b(10.00), RF:40(21)",
+ MWIFIEX_VERSION_STR_LENGTH) == 0) {
+ struct mwifiex_ds_auto_ds auto_ds = {
+ .auto_ds = DEEP_SLEEP_OFF,
+ };
+
+ mwifiex_dbg(priv->adapter, MSG,
+ "Bad HW revision detected, disabling deep sleep\n");
+
+ if (mwifiex_send_cmd(priv, HostCmd_CMD_802_11_PS_MODE_ENH,
+ DIS_AUTO_PS, BITMAP_AUTO_DS, &auto_ds, false)) {
+ mwifiex_dbg(priv->adapter, MSG,
+ "Disabling deep sleep failed.\n");
+ }
+ }
+
+ return 0;
+ }
+
if (version_ext) {
version_ext->version_str_sel = ver_ext->version_str_sel;
memcpy(version_ext->version_str, ver_ext->version_str,
--
2.33.1
We assume at a few places that priv->version_str is 0-terminated, but
right now we trust the firmware that this is the case with the version
string we get from it.
Let's rather ensure this ourselves and replace the last character with
'\0'.
Signed-off-by: Jonas Dreßler <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 6c7b0b9bc4e9..1a4ae8a42a31 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -734,6 +734,9 @@ static int mwifiex_ret_ver_ext(struct mwifiex_private *priv,
MWIFIEX_VERSION_STR_LENGTH);
memcpy(priv->version_str, ver_ext->version_str,
MWIFIEX_VERSION_STR_LENGTH);
+
+ /* Ensure the version string from the firmware is 0-terminated */
+ priv->version_str[MWIFIEX_VERSION_STR_LENGTH - 1] = '\0';
}
return 0;
}
--
2.33.1
On Wed, Nov 3, 2021 at 10:19 PM Jonas Dreßler <[email protected]> wrote:
>
> Fourth revision of this patch.
> v1: https://lore.kernel.org/linux-wireless/[email protected]/T/
> v2: https://lore.kernel.org/linux-wireless/[email protected]/T/
> v3: https://lore.kernel.org/linux-wireless/[email protected]/T/
Not sure why you ignored my tag...
As we discussed with Bjorn, it's fine to me to leave messages splitted
to two lines.
> Changes between v3 and v4:
> - Add patch to ensure 0-termination of version string
--
With Best Regards,
Andy Shevchenko
On 11/3/21 22:50, Andy Shevchenko wrote:
> On Wed, Nov 3, 2021 at 10:19 PM Jonas Dreßler <[email protected]> wrote:
>>
>> Fourth revision of this patch.
>> v1: https://lore.kernel.org/linux-wireless/[email protected]/T/
>> v2: https://lore.kernel.org/linux-wireless/[email protected]/T/
>> v3: https://lore.kernel.org/linux-wireless/[email protected]/T/
>
> Not sure why you ignored my tag...
> As we discussed with Bjorn, it's fine to me to leave messages splitted
> to two lines.
Whoops, sorry, that wasn't on purpose. I'm still not really used to the whole
email workflow of the kernel...
>
>> Changes between v3 and v4:
>> - Add patch to ensure 0-termination of version string
>
>