On Thu, Oct 28, 2021 at 10:38 AM Jonas Dreßler [off-list ref] wrote:

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.

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
powersaving state machine seems to get confused and the card can't sleep

power saving

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.

...

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

+       set_bit(MWIFIEX_IS_REQUESTING_FW_VEREXT, &adapter->work_flags);

This does not bring atomicity to this function.
You need test_and_set_bit().

Otherwise the bit may very well be cleared already here. And function
may enter here again.

If this state machine is protected by lock or so, then why not use
__set_bit() to show this clearly?

+       memset(&ver_ext, 0, sizeof(ver_ext));
+       ver_ext.version_str_sel = 1;
+       mwifiex_send_cmd(priv, HostCmd_CMD_VERSION_EXT,
+                        HostCmd_ACT_GEN_GET, 0, &ver_ext, false);
+}

-- 
With Best Regards,
Andy Shevchenko