On Thu, Jan 31, 2013 at 04:20:48PM +0100, Johannes Berg wrote: > On Tue, 2013-01-29 at 17:47 -0600, Seth Forshee wrote: > > > +static inline bool ieee80211_is_ps_disabled(struct ieee80211_conf *conf) > > > +static inline bool ieee80211_is_ps_enabled(struct ieee80211_conf *conf) > > Huh, is that worth the confusion? It seems !enabled should be the same > as disabled, but it's not quite the same, which might be confusing. In this patch there's no distinction, but after adding the off-channel powersave state there is -- disabled == !enabled && !offchannel. 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? > > +/** > > + * ieee80211_set_ps_state - set device powersave state > > + * > > + * Sets the powersave state in the supplied device configuration to the > > + * specified state. > > + * > > + * @conf: device configuration > > + * @state: new powersave state. Must be one of the IEEE80211_CONF_PS_* > > + * flags from enum ieee80211_conf_flags. > > + */ > > +static inline void ieee80211_set_ps_state(struct ieee80211_conf *conf, > > + u32 state) > > +{ > > + conf->flags = (conf->flags & ~IEEE80211_CONF_PS_MASK) | > > + (state & IEEE80211_CONF_PS_MASK); > > +} > > I don't think the driver should do this, so the inline shouldn't be > here? That's true. Would moving it to ieee80211_i.h be appropriate, or is there somewhere better? Seth -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html