This patch is necessary in order to provide a proper Access point support for p54.
Unfortunatly for us, there is no documented way to disable the interfering
power save buffering mechanism in firmware completely.
Therefore we give in and notify the driver through our new sta_notify callback,
so that we can update the filter state.
Signed-off-by: Christian Lamparter <[email protected]>
---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 6a1d4ea..7bd8edc 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -768,14 +768,18 @@ struct ieee80211_sta {
/**
* enum sta_notify_cmd - sta notify command
*
- * Used with the sta_notify() callback in &struct ieee80211_ops, this
- * indicates addition and removal of a station to station table.
+ * Used with the sta_notify() callback in &struct ieee80211_ops.
+ * this command indicates addition and removal of a station to
+ * station table, or if a station made a power state transition.
*
* @STA_NOTIFY_ADD: a station was added to the station table
* @STA_NOTIFY_REMOVE: a station being removed from the station table
+ * @STA_NOTIFY_SLEEP: a station is now sleeping
+ * @STA_NOTIFY_AWAKE: a sleeping station woke up
*/
enum sta_notify_cmd {
- STA_NOTIFY_ADD, STA_NOTIFY_REMOVE
+ STA_NOTIFY_ADD, STA_NOTIFY_REMOVE,
+ STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
};
/**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 5a1a60f..2d311a1 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -654,10 +654,14 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
static void ap_sta_ps_start(struct sta_info *sta)
{
struct ieee80211_sub_if_data *sdata = sta->sdata;
+ struct ieee80211_local *local = sdata->local;
DECLARE_MAC_BUF(mac);
atomic_inc(&sdata->bss->num_sta_ps);
set_and_clear_sta_flags(sta, WLAN_STA_PS, WLAN_STA_PSPOLL);
+ if (local->ops->sta_notify)
+ local->ops->sta_notify(local_to_hw(local), &sdata->vif,
+ STA_NOTIFY_SLEEP, &sta->sta);
#ifdef CONFIG_MAC80211_VERBOSE_PS_DEBUG
printk(KERN_DEBUG "%s: STA %s aid %d enters power save mode\n",
sdata->dev->name, print_mac(mac, sta->sta.addr), sta->sta.aid);
@@ -675,6 +679,9 @@ static int ap_sta_ps_end(struct sta_info *sta)
atomic_dec(&sdata->bss->num_sta_ps);
clear_sta_flags(sta, WLAN_STA_PS | WLAN_STA_PSPOLL);
+ if (local->ops->sta_notify)
+ local->ops->sta_notify(local_to_hw(local), &sdata->vif,
+ STA_NOTIFY_AWAKE, &sta->sta);
if (!skb_queue_empty(&sta->ps_tx_buf))
sta_info_clear_tim_bit(sta);
On Saturday 29 November 2008 10:41:49 Johannes Berg wrote:
> On Fri, 2008-11-28 at 23:45 +0100, Christian Lamparter wrote:
>
> > /**
> > * enum sta_notify_cmd - sta notify command
> > *
> > - * Used with the sta_notify() callback in &struct ieee80211_ops, this
> > - * indicates addition and removal of a station to station table.
> > + * Used with the sta_notify() callback in &struct ieee80211_ops.
> > + * this command indicates addition and removal of a station to
> > + * station table, or if a station made a power state transition.
> > *
> > * @STA_NOTIFY_ADD: a station was added to the station table
> > * @STA_NOTIFY_REMOVE: a station being removed from the station table
> > + * @STA_NOTIFY_SLEEP: a station is now sleeping
> > + * @STA_NOTIFY_AWAKE: a sleeping station woke up
> > */
> > enum sta_notify_cmd {
> > - STA_NOTIFY_ADD, STA_NOTIFY_REMOVE
> > + STA_NOTIFY_ADD, STA_NOTIFY_REMOVE,
> > + STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
> > };
>
> You know, I'm sorry, but I've come to realise that adding it to
> sta_notify isn't a good idea after all. The one thing I forgot is that
> this new callback has to be atomic because it's called from the rx path,
> and sta_notify doesn't have to be.
>
> Unless we want to defer RX packet processing to a workqueue rather than
> a tasklet?
>
Hmm, but if we put the RX packet processing into a workqueue, we have to
put the tx status report processing in one too (see comment about rx/tx race
in main.c line 400)?
Or do you mean I should only put ap_sta_ps_end (& ap_sta_ps_start)
resending stuff into a workqueue? (Yeah, this makes a lot more sense,
and might actually work!, well let's prepare another patch)
Regards,
Chr
On Fri, 2008-11-28 at 23:45 +0100, Christian Lamparter wrote:
> /**
> * enum sta_notify_cmd - sta notify command
> *
> - * Used with the sta_notify() callback in &struct ieee80211_ops, this
> - * indicates addition and removal of a station to station table.
> + * Used with the sta_notify() callback in &struct ieee80211_ops.
> + * this command indicates addition and removal of a station to
> + * station table, or if a station made a power state transition.
> *
> * @STA_NOTIFY_ADD: a station was added to the station table
> * @STA_NOTIFY_REMOVE: a station being removed from the station table
> + * @STA_NOTIFY_SLEEP: a station is now sleeping
> + * @STA_NOTIFY_AWAKE: a sleeping station woke up
> */
> enum sta_notify_cmd {
> - STA_NOTIFY_ADD, STA_NOTIFY_REMOVE
> + STA_NOTIFY_ADD, STA_NOTIFY_REMOVE,
> + STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
> };
You know, I'm sorry, but I've come to realise that adding it to
sta_notify isn't a good idea after all. The one thing I forgot is that
this new callback has to be atomic because it's called from the rx path,
and sta_notify doesn't have to be.
Unless we want to defer RX packet processing to a workqueue rather than
a tasklet?
johannes
On Sat, 2008-11-29 at 13:15 +0100, Christian Lamparter wrote:
> > Unless we want to defer RX packet processing to a workqueue rather than
> > a tasklet?
> Hmm, but if we put the RX packet processing into a workqueue, we have to
> put the tx status report processing in one too (see comment about rx/tx race
> in main.c line 400)?
Yeah, probably. I don't know all the details all the time, so I write
such comments ;)
> Or do you mean I should only put ap_sta_ps_end (& ap_sta_ps_start)
> resending stuff into a workqueue? (Yeah, this makes a lot more sense,
> and might actually work!, well let's prepare another patch)
No idea!
The only thing I was thinking is that it's not a good plan to have a
callback that has different locking requirements depending on a
parameter.
johannes
On Saturday 29 November 2008 13:21:33 Johannes Berg wrote:
> On Sat, 2008-11-29 at 13:15 +0100, Christian Lamparter wrote:
>
> > > Unless we want to defer RX packet processing to a workqueue rather than
> > > a tasklet?
>
> > Hmm, but if we put the RX packet processing into a workqueue, we have to
> > put the tx status report processing in one too (see comment about rx/tx race
> > in main.c line 400)?
>
> Yeah, probably. I don't know all the details all the time, so I write
> such comments ;)
>
> > Or do you mean I should only put ap_sta_ps_end (& ap_sta_ps_start)
> > resending stuff into a workqueue? (Yeah, this makes a lot more sense,
> > and might actually work!, well let's prepare another patch)
>
> No idea!
>
> The only thing I was thinking is that it's not a good plan to have a
> callback that has different locking requirements depending on a
> parameter.
yeah... go back to the extra sta_notify_ps callback approach?
I tried to do this with workqueues, but It turned out that queue_work
doesn't work since we disabled the irqs in the rx path.
( http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-02/3350.html
has a story about the bitter consequences of queue_work & irq_disabled)
Regards,
Chr
On Sat, 2008-11-29 at 18:06 +0100, Christian Lamparter wrote:
> > The only thing I was thinking is that it's not a good plan to have a
> > callback that has different locking requirements depending on a
> > parameter.
>
> yeah... go back to the extra sta_notify_ps callback approach?
Fine with me, yeah. It's not very complicated so we can always change it
again later.
> I tried to do this with workqueues, but It turned out that queue_work
> doesn't work since we disabled the irqs in the rx path.
> ( http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-02/3350.html
> has a story about the bitter consequences of queue_work & irq_disabled)
Interesting.
johannes