Hi,
On Wed, Nov 27, 2024 at 2:58 AM Pin-yen Lin <treapking@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> Signed-off-by: Pin-yen Lin <treapking@xxxxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxx>
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
