On Thu, Feb 20, 2014 at 10:42:48AM +0100, Johannes Berg wrote:
> On Thu, 2014-02-20 at 10:40 +0100, Stanislaw Gruszka wrote:
>
> > > This area is a bit confusing, but I don't think WLAN_STA_PS_STA will be
> > > clear until after WLAN_STA_PS_DRIVER is?
> >
> > Hmm, actually looks like we call ps_deliver_wakeup() with WLAN_STA_PS_STA
> > flag currently cleared.
> >
> > tatic void sta_unblock(struct work_struct *wk)
> > {
> > if (!test_sta_flag(sta, WLAN_STA_PS_STA)) {
> > local_bh_disable();
> > ieee80211_sta_ps_deliver_wakeup(sta);
> > local_bh_enable();
> >
> > so on TX we should rather check WLAN_STA_PS_DRIVER flag .
>
> But that flag may never be set if the driver doesn't support/use it
> (which means it's racy, but anyway), so we need to check both.
>
> Now I'm confused though. It seems the intent here was to clear the
> WLAN_STA_PS_STA flag in something like sta_ps_end() in the RX path, but
> we don't do that. I'm not sure how it works now, but I know from testing
> that it does ;-)
LOL. There is another call to ieee80211_sta_ps_deliver_wakeup() from
sta_ps_end(), I missed this somehow. Anyway, we need to check both
flags.
> Do you think the v2 patch is fine?
Yes, I think it's ok.
Stanislaw
--
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
