On Thu, Oct 28, 2021 at 10:38 AM Jonas Dreßler <verdre@xxxxxxx> 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