Re: [PATCH 1/2 V2] mac80211: send action frame when toggling SM PS mode

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2884 bytes --]

On Thu, 2008-10-02 at 14:03 +0300, Tomas Winkler wrote:

> >> +static void ieee80211_send_sm_ps(struct ieee80211_sub_if_data *sdata, u8 mode)
> >
> > Can we use an enum for 'mode'?
> 
> We use spec numbers here why to invent more numbers.

Well we can also just put the spec numbers into an enum and use that
enum here. enum doesn't mean that we don't fix the numbers :)

> >> +     /* Implemented for STA only */
> >> +     if (sdata->vif.type != NL80211_IFTYPE_STATION)
> >> +             return;
> >
> > What about mesh, ibss, wds, ...? I see this doesn't make much sense for
> > an AP, but...?
> 
> This is explicitly defined in spec for STA mode.

Ok. Maybe change the comment then so that it's more obvious.

> > It seems that for the second use case you're citing you've now done
> > something that might lead to bouncing back and forth because there is no
> > central instance deciding on the powersave mode. You seem to be trying
> > to avoid this by introducing the two new variables sm_ps_psp_mode and
> > sm_ps_cam_mode, but this seems very strange. Is the driver supposed to
> > modify them at runtime?
> 
> No it should be set on registration only
> 
> What sort of policy does the driver impose on
> > them? Why is this policy driver-specific?
> 
> Depends on radio ability rather then policy. The only policy that is
> set here is that we coupling SM PS and PS
> 
> Why doesn't the driver just
> > inform mac80211 of the antenna status and mac80211 then decides based on
> > the antenna status and the requested powersave mode which SM PS mode
> > should be activated, and then notifies the driver of that?
> 
> Correct that's the solution, just keep in mind that driver change rx
> chain configuration upon SM PS request as well.

Yeah but you were saying it's about detecting whether MIMO is effective
or not. So I think what makes more sense is to have the driver _only_
push that information to mac80211, and then wait for mac80211 to make a
decision about the SM PS mode, and not reconfigure its chains
immediately. Therefore, it wouldn't change chain configuration when it
detects MIMO doesn't work, it would only do that when mac80211 requests
it based on the information.

> > I'd much rather see you implement this in a different way:
> >  * a HW flag that determines whether the driver can wake up with or
> >   without an RTS frame (?)
> 
> Not enough there are 3 states you may request in SM_PS

Yes, but this is just a hardware capability which indirectly influences
the PS mode, no? So not having the wakeup-without-rts capability may
mean that SM_PS_XYZ cannot be used or whatever.

> >   - possibly interface modes (maybe no need for MIMO when in mesh etc.)
> 
> Out of scope, HT is not enabled at all in these modes.

currently.

I'll look at more details after lunch.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]