[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-wireless
Subject:    Re: [PATCH 5/7] mac80211: Expand powersave configuration flag to be two bits
From:       Johannes Berg <johannes () sipsolutions ! net>
Date:       2013-01-31 17:50:34
Message-ID: 1359654634.8415.101.camel () jlt4 ! sipsolutions ! net
[Download RAW message or body]

On Thu, 2013-01-31 at 11:18 -0600, Seth Forshee wrote:

> > > Actually one of the last bugs I fixed before sending these was a place
> > > where I had used disabled instead of !enabled, and the frames ended up
> > > with PM set when it shouldn't have been.
> > > 
> > > I agree though that the distinction is confusing. Maybe some better
> > > state names are needed. Perhaps awake, offchannel, and doze?
> > 
> > I think what you really want is to distinguish between "HW can go to
> > powersave" and "PM bit should be set"? That's pretty much what your
> > CONF_PS_ENABLED and CONF_PS_OFFCHANNEL means, respectively, but maybe
> 
> Correct, with the understanding that "HW can go to powersave" also
> implies "PM bit should be set."
> 
> Another approach would be to keep the CONF_PS flag the same and add a
> CONF_PM flag or similar. I didn't go with this approach because CONF_PS
> && !CONF_PM really doesn't make any sense, which really doesn't help
> with reducing confusion. The advantage is that it separates setting PM
> from PS for those driver that don't support PS but need to configure the
> hardware to set PM for off-channel.

Good point, that'd work too. PS && !PM would never be used, I guess?
It'd also have the advantage of not having to touch all the drivers?

It doesn't really matter all that much to me though, I just think what
you have right now is (too) confusing.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic