Re: [PATCH] wifi: mwifiex: decrease timeout waiting for host sleep from 10s to 5s

From: Doug Anderson
Date: Wed Dec 04 2024 - 13:09:51 EST


Hi,

On Tue, Dec 3, 2024 at 6:04 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
>
> 10 seconds is likely that *something* is wrong (or at least suboptimal),
> but IMO, it's not quite at unreasonable levels. But yes, my point was
> mainly that it's squishy, and I personally wouldn't want to be the one
> running with the lowest CONFIG_DPM_WATCHDOG_TIMEOUT out there, given the
> known behavior of multiple drivers and the timeout-means-panic behavior.
>
> > Maybe the ChromeOS should change to 15 seconds for the DPM Watchdog
> > timer and that's a better solution and leave the WiFi driver how it
> > is?
>
> That seems reasonable.

FWIW, I created a public feature request for this:

https://issuetracker.google.com/382269699

...we'll see if we can get anyone to bite on it. ...and then see if
upstream folks like the idea too.


> To be clear, I'm OK with this patch, if we can get a little more
> confidence in it (like the timing data and HW info). I *think* 5 vs 10
> isn't a big deal here, but I don't have much other than my guess at the
> moment.
>
> > Another thought: I wonder if it's possible to be dynamic and somehow
> > set the timeout as "max(10, dpm_watchdog_timeout / 2)". Not that I've
>
> You probably meant min()?

Yeah, I always screw that up. Sigh.


> > > Can you try testing (and gather timing numbers) when
> > > suspending soon after initiating scans? It's hard to judge what the
> > > lower limit of this timeout should really be without any numbers, just
> > > like it's hard to judge whether your 10 second watchdog is reasonable.
> >
> > Pin-yen: is this something you could gather?
> >
> >
> > > Also, for the record, since we might have to field regression reports
> > > for other systems: what hardware (chip variant, FW version) are you
> > > seeing problems on?
> >
> > Pin-yen: I'm assuming you'll provide this.
>
> I'll leave it up to y'all (Doug and Pin-Yen) whether you want to provide
> the above to provide a little more confidence, or if you want to
> reconsider your use of CONFIG_DPM_WATCHDOG_TIMEOUT.

Possibly the answer could be both?