| Message ID | 20241127105709.4014302-1-treapking@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: mwifiex: decrease timeout waiting for host sleep from 10s to 5s | expand |
Hi, On Wed, Nov 27, 2024 at 2:58 AM Pin-yen Lin <treapking@chromium.org> wrote: > > In commit 52250cbee7f6 ("mwifiex: use timeout variant for > wait_event_interruptible") it was noted that sometimes we seemed > to miss the signal that our host sleep settings took effect. A > 10 second timeout was added to the code to make sure we didn't > hang forever waiting. It appears that this problem still exists > and we hit the timeout sometimes for Chromebooks in the field. > > Recently on ChromeOS we've started setting the DPM watchdog > to trip if full system suspend takes over 10 seconds. Given > the timeout in the original patch, obviously we're hitting > the DPM watchdog before mwifiex gets a chance to timeout. > > While we could increase the DPM watchdog in ChromeOS to avoid > this problem, it's probably better to simply decrease the > timeout. Any time we're waiting several seconds for the > firmware to respond it's likely that the firmware won't ever > respond. With that in mind, decrease the timeout in mwifiex > from 10 seconds to 5 seconds. > > Suggested-by: Doug Anderson <dianders@chromium.org> > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > --- > > drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I believe Brian Norris is still anointed as the personally nominally in charge of mwifiex upstream (get_maintainer labels him as "odd" fixer, which seems awfully judgemental), so he should be CCed on fixes. Added him. > diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c > index e06a0622973e..f79589cafe57 100644 > --- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c > +++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c > @@ -545,7 +545,7 @@ int mwifiex_enable_hs(struct mwifiex_adapter *adapter) > > if (wait_event_interruptible_timeout(adapter->hs_activate_wait_q, > adapter->hs_activate_wait_q_woken, > - (10 * HZ)) <= 0) { > + (5 * HZ)) <= 0) { Given that I suggested this fix, it should be no surprise: Reviewed-by: Douglas Anderson <dianders@chromium.org> Upon sleeping on the idea, the only slight concern I have here is whether we'll trigger this timeout if we try to suspend while WiFi scanning is in progress. I don't have any actual evidence supporting this concern, but I remember many years ago when I used to deal with the WiFi drivers more often there were cases where suspend could be delayed if it happened while a scan was in progress. Maybe/hopefully that's fixed now, but I figured I'd at least bring it up in case it tickled anything in someone's mind... If somehow that turns out to be a problem, hopefully we'd be able to find a way to cancel the scan or break scans up into smaller chunks because even delaying suspend for 5 seconds seems like it would be a big problem. -Doug
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c index e06a0622973e..f79589cafe57 100644 --- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c +++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c @@ -545,7 +545,7 @@ int mwifiex_enable_hs(struct mwifiex_adapter *adapter) if (wait_event_interruptible_timeout(adapter->hs_activate_wait_q, adapter->hs_activate_wait_q_woken, - (10 * HZ)) <= 0) { + (5 * HZ)) <= 0) { mwifiex_dbg(adapter, ERROR, "hs_activate_wait_q terminated\n"); return false;
In commit 52250cbee7f6 ("mwifiex: use timeout variant for wait_event_interruptible") it was noted that sometimes we seemed to miss the signal that our host sleep settings took effect. A 10 second timeout was added to the code to make sure we didn't hang forever waiting. It appears that this problem still exists and we hit the timeout sometimes for Chromebooks in the field. Recently on ChromeOS we've started setting the DPM watchdog to trip if full system suspend takes over 10 seconds. Given the timeout in the original patch, obviously we're hitting the DPM watchdog before mwifiex gets a chance to timeout. While we could increase the DPM watchdog in ChromeOS to avoid this problem, it's probably better to simply decrease the timeout. Any time we're waiting several seconds for the firmware to respond it's likely that the firmware won't ever respond. With that in mind, decrease the timeout in mwifiex from 10 seconds to 5 seconds. Suggested-by: Doug Anderson <dianders@chromium.org> Signed-off-by: Pin-yen Lin <treapking@chromium.org> --- drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)