LinuxLists.cc - Asus eeepc 1008HA suspend issue and mac80211 suspend corner case

2009-12-23 16:28:18

by Luis R. Rodriguez

[permalink] [raw]

Subject: Re: mac80211 suspend corner case (was: Asus eeepc 1008HA suspend issue and mac80211 suspend corner) case

On Wed, Dec 23, 2009 at 04:08:01AM -0800, Johannes Berg wrote:
> On Tue, 2009-12-22 at 14:30 -0500, Luis R. Rodriguez wrote:
>
> > Johannes, any preferences how to handle this? The patch below avoids
> > the Interrupt being turned off but its not enough given that we still
> > could be associated according to userspace. If the hardware is
> > unresponsive maybe it is best to just let the IRQ go disabled, not
> > sure, but its likely not what happens in all cases.
> >
> > Trimming out the irrelevant parts below.
>
> > http://bombadil.infradead.org/~mcgrof/logs/2.6.31-with-2.6.32-wireless
> > /irq-disabled.txt
> > >
> > > This can be fixed by something like the following:
> > >
> > > diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> > > index e6c08da..63d42fa 100644
> > > --- a/net/mac80211/util.c
> > > +++ b/net/mac80211/util.c
> > > @@ -1031,7 +1031,14 @@ int ieee80211_reconfig(struct ieee80211_local
> > *local)
> > >
> > > /* restart hardware */
> > > if (local->open_count) {
> > > + /*
> > > + * Upon resume hardware can sometimes be goofy due to
> > > + * various platform issues, so restarting the device may
> > > + * at times not work immediately. Propagate the error.
> > > + */
> > > res = drv_start(local);
> > > + if (res)
> > > + return res;
> > >
> > > ieee80211_led_radio(local, true);
> > > }
> > >
> > > But this isn't enough. And since we cannot exactly talk to hardware
> > > we can't try to send a deassoc as harware would be unresponsive. I
> > > also don't see us handling such cases before either on cfg80211 or
> > > mac80211, so curious what we should do.
>
> Well it seems to me that if the driver determines that the hardware is
> unreachable or not responding, it would unregister it from mac80211,
> which would clean up all user-visible state, obviously.

The drivers would not know this until it fails on the first call
from mac80211 which would be start().

> The patch above seems ok to me, but basically papers over the problem.
> If the start there fails, the driver will have to unregister the hw
> since any subsequent start will fail as well.

How about just having mac80211 do that for drivers where the start()
fails and we are resuming? I can give that a shot.

> The way I see it, it's like a USB unplug while suspended/hibernated ...
> the driver notices that and remo

I see, in this case though I'd prefer if mac80211 could do more of
the work instead of requiring each driver to treat start() failures
by unregistering themselves, specially since we don't inform drivers
they are coming back from resume on start().

Luis

2009-12-23 19:01:59

On Wed, 2009-12-23 at 11:08 -0800, Luis R. Rodriguez wrote:

> So you want drivers to handle start() failures (even if its not for
> resume) with an unregistration to mac80211?

Not necessarily. If it's a temporary error, that's fine, of course, but
if the hardware is suddenly inaccessible for whatever reason then yes,
of course, unregistering is the only good choice no matter when it
happens.

johannes

Attachments:

signature.asc (801.00 B)
This is a digitally signed message part

2009-12-23 19:19:57

by Luis R. Rodriguez

[permalink] [raw]

Subject: Re: mac80211 suspend corner case (was: Asus eeepc 1008HA suspend issue and mac80211 suspend corner) case

On Wed, Dec 23, 2009 at 11:11 AM, Johannes Berg
<[email protected]> wrote:
> On Wed, 2009-12-23 at 11:08 -0800, Luis R. Rodriguez wrote:
>
>> So you want drivers to handle start() failures (even if its not for
>> resume) with an unregistration to mac80211?
>
> Not necessarily. If it's a temporary error, that's fine, of course, but
> if the hardware is suddenly inaccessible for whatever reason then yes,
> of course, unregistering is the only good choice no matter when it
> happens.

Reason for me suggesting for mac80211 to deal with this is drivers
won't know if their failed start() call will have been from resume and
we likely won't add a bool to start() callback since we already
debated this a while back.

I don't really care about hardware failures in other cases right now,
and do think resume is reasonable enough to check for and handle on
mac8021 alone. Since stop() would have stopped everything in the
driver and is documented as such I don't see why it would be
complicated to unregister it instead from mac80211.

Luis

2009-12-24 12:43:53

by Johannes Berg

[permalink] [raw]

Subject: Re: mac80211 suspend corner case (was: Asus eeepc 1008HA suspend issue and mac80211 suspend corner) case

On Wed, 2009-12-23 at 11:19 -0800, Luis R. Rodriguez wrote:

> Reason for me suggesting for mac80211 to deal with this is drivers
> won't know if their failed start() call will have been from resume and
> we likely won't add a bool to start() callback since we already
> debated this a while back.
>
> I don't really care about hardware failures in other cases right now,

Which is the problem.

If your hardware suddenly fails, _regardless_ of when, you need to
unregister from mac80211 since otherwise you'll get loads of callbacks
to failed hardware.

Resume really isn't as special as you'd like to believe.

johannes

Attachments:

signature.asc (801.00 B)
This is a digitally signed message part