[PATCH 1/6 v2] mac80211: add sta_notify_ps callback

[PATCH 1/6 v2] mac80211: add sta_notify_ps callback

[Christian Lamparter]

This patch is necessary in order to provide a proper Access point support for p54.

Unfortunately 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_ps callback, 
so that we can update the filter state.

Signed-off-by: Christian Lamparter <chunkeey@web.de>
---
BTW, the extra patch for mac80211_hwsim is no longer needed...
This is all for wireless-testing.
---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 6a1d4ea..a6baf37 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -779,6 +779,19 @@ enum sta_notify_cmd {
 };
 
 /**
+ * enum sta_notify_ps_cmd - sta power save notify command
+ *
+ * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
+ * notify the driver if a station made a power state transition.
+ *
+ * @STA_WILL_SLEEP: a station is now sleeping
+ * @STA_WOKE_UP: a sleeping station woke up
+ */
+enum sta_notify_ps_cmd {
+	STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
+};
+
+/**
  * enum ieee80211_tkip_key_type - get tkip key
  *
  * Used by drivers which need to get a tkip key for skb. Some drivers need a
@@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
  * @sta_notify: Notifies low level driver about addition or removal
  *	of associated station or AP.
  *
+ * @sta_ps_notify: Notifies low level driver about the power state transition
+ *	of a associated station. Must be atomic.
+ *
  * @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
  *	bursting) for a hardware TX queue.
  *
@@ -1314,6 +1330,8 @@ struct ieee80211_ops {
 	int (*set_frag_threshold)(struct ieee80211_hw *hw, u32 value);
 	void (*sta_notify)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			enum sta_notify_cmd, struct ieee80211_sta *sta);
+	void (*sta_notify_ps)(struct ieee80211_hw *hw,
+			enum sta_notify_ps_cmd, struct ieee80211_sta *sta);
 	int (*conf_tx)(struct ieee80211_hw *hw, u16 queue,
 		       const struct ieee80211_tx_queue_params *params);
 	int (*get_tx_stats)(struct ieee80211_hw *hw,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 5a1a60f..3bf056e 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_ps)
+		local->ops->sta_notify_ps(local_to_hw(local), STA_WILL_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_ps)
+		local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
+					  &sta->sta);
 
 	if (!skb_queue_empty(&sta->ps_tx_buf))
 		sta_info_clear_tim_bit(sta);

Re: [PATCH 1/6 v2] mac80211: add sta_notify_ps callback

[Johannes Berg]

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

On Sat, 2008-11-29 at 22:32 +0100, Christian Lamparter wrote:
> This patch is necessary in order to provide a proper Access point support for p54.
> 
> Unfortunately 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_ps callback, 
> so that we can update the filter state.
> 
> Signed-off-by: Christian Lamparter <chunkeey@web.de>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
> BTW, the extra patch for mac80211_hwsim is no longer needed...
> This is all for wireless-testing.
> ---
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 6a1d4ea..a6baf37 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -779,6 +779,19 @@ enum sta_notify_cmd {
>  };
>  
>  /**
> + * enum sta_notify_ps_cmd - sta power save notify command
> + *
> + * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
> + * notify the driver if a station made a power state transition.
> + *
> + * @STA_WILL_SLEEP: a station is now sleeping
> + * @STA_WOKE_UP: a sleeping station woke up
> + */
> +enum sta_notify_ps_cmd {
> +	STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
> +};
> +
> +/**
>   * enum ieee80211_tkip_key_type - get tkip key
>   *
>   * Used by drivers which need to get a tkip key for skb. Some drivers need a
> @@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
>   * @sta_notify: Notifies low level driver about addition or removal
>   *	of associated station or AP.
>   *
> + * @sta_ps_notify: Notifies low level driver about the power state transition
> + *	of a associated station. Must be atomic.
> + *
>   * @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
>   *	bursting) for a hardware TX queue.
>   *
> @@ -1314,6 +1330,8 @@ struct ieee80211_ops {
>  	int (*set_frag_threshold)(struct ieee80211_hw *hw, u32 value);
>  	void (*sta_notify)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>  			enum sta_notify_cmd, struct ieee80211_sta *sta);
> +	void (*sta_notify_ps)(struct ieee80211_hw *hw,
> +			enum sta_notify_ps_cmd, struct ieee80211_sta *sta);
>  	int (*conf_tx)(struct ieee80211_hw *hw, u16 queue,
>  		       const struct ieee80211_tx_queue_params *params);
>  	int (*get_tx_stats)(struct ieee80211_hw *hw,
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 5a1a60f..3bf056e 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_ps)
> +		local->ops->sta_notify_ps(local_to_hw(local), STA_WILL_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_ps)
> +		local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
> +					  &sta->sta);
>  
>  	if (!skb_queue_empty(&sta->ps_tx_buf))
>  		sta_info_clear_tim_bit(sta);
> 

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

[PATCH 1/6 v2.1] mac80211: add sta_notify_ps callback

[Christian Lamparter]

This patch is necessary in order to provide a proper Access point support for p54.
 
Unfortunately 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_ps callback, 
so that we can update the filter state.
 
Signed-off-by: Christian Lamparter <chunkeey@web.de>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
---
the extra patch for mac80211_hwsim is no longer needed...
This whole series is all for wireless-testing.

erm... now with a updated documentation!!1one
---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 6a1d4ea..a6baf37 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -779,6 +779,19 @@ enum sta_notify_cmd {
 };
 
 /**
+ * enum sta_notify_ps_cmd - sta power save notify command
+ *
+ * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
+ * notify the driver if a station made a power state transition.
+ *
+ * @STA_NOTIFY_SLEEP: a station is now sleeping
+ * @STA_NOTIFY_AWAKE: a sleeping station woke up
+ */
+enum sta_notify_ps_cmd {
+	STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
+};
+
+/**
  * enum ieee80211_tkip_key_type - get tkip key
  *
  * Used by drivers which need to get a tkip key for skb. Some drivers need a
@@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
  * @sta_notify: Notifies low level driver about addition or removal
  *	of associated station or AP.
  *
+ * @sta_ps_notify: Notifies low level driver about the power state transition
+ *	of a associated station. Must be atomic.
+ *
  * @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
  *	bursting) for a hardware TX queue.
  *
@@ -1314,6 +1330,8 @@ struct ieee80211_ops {
 	int (*set_frag_threshold)(struct ieee80211_hw *hw, u32 value);
 	void (*sta_notify)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			enum sta_notify_cmd, struct ieee80211_sta *sta);
+	void (*sta_notify_ps)(struct ieee80211_hw *hw,
+			enum sta_notify_ps_cmd, struct ieee80211_sta *sta);
 	int (*conf_tx)(struct ieee80211_hw *hw, u16 queue,
 		       const struct ieee80211_tx_queue_params *params);
 	int (*get_tx_stats)(struct ieee80211_hw *hw,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 5a1a60f..3bf056e 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_ps)
+		local->ops->sta_notify_ps(local_to_hw(local), STA_WILL_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_ps)
+		local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
+					  &sta->sta);
 
 	if (!skb_queue_empty(&sta->ps_tx_buf))
 		sta_info_clear_tim_bit(sta);

Re: [PATCH 1/6 v2.1] mac80211: add sta_notify_ps callback

[Larry Finger]

Christian Lamparter wrote:
> This patch is necessary in order to provide a proper Access point support for p54.
>  
> Unfortunately 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_ps callback, 
> so that we can update the filter state.
>  
> Signed-off-by: Christian Lamparter <chunkeey@web.de>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> the extra patch for mac80211_hwsim is no longer needed...
> This whole series is all for wireless-testing.
> 
> erm... now with a updated documentation!!1one
> ---
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 6a1d4ea..a6baf37 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -779,6 +779,19 @@ enum sta_notify_cmd {
>  };
>  
>  /**
> + * enum sta_notify_ps_cmd - sta power save notify command
> + *
> + * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
> + * notify the driver if a station made a power state transition.
> + *
> + * @STA_NOTIFY_SLEEP: a station is now sleeping
> + * @STA_NOTIFY_AWAKE: a sleeping station woke up
> + */
> +enum sta_notify_ps_cmd {
> +	STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
> +};
> +
> +/**
>   * enum ieee80211_tkip_key_type - get tkip key
>   *
>   * Used by drivers which need to get a tkip key for skb. Some drivers need a
> @@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
>   * @sta_notify: Notifies low level driver about addition or removal
>   *	of associated station or AP.
>   *
> + * @sta_ps_notify: Notifies low level driver about the power state transition
> + *	of a associated station. Must be atomic.
> + *
>   * @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
>   *	bursting) for a hardware TX queue.
>   *
> @@ -1314,6 +1330,8 @@ struct ieee80211_ops {
>  	int (*set_frag_threshold)(struct ieee80211_hw *hw, u32 value);
>  	void (*sta_notify)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>  			enum sta_notify_cmd, struct ieee80211_sta *sta);
> +	void (*sta_notify_ps)(struct ieee80211_hw *hw,
> +			enum sta_notify_ps_cmd, struct ieee80211_sta *sta);
>  	int (*conf_tx)(struct ieee80211_hw *hw, u16 queue,
>  		       const struct ieee80211_tx_queue_params *params);
>  	int (*get_tx_stats)(struct ieee80211_hw *hw,
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 5a1a60f..3bf056e 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_ps)
> +		local->ops->sta_notify_ps(local_to_hw(local), STA_WILL_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_ps)
> +		local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
> +					  &sta->sta);

With this patch, STA_WILL_SLEEP and STA_WOKE_UP are undefined. Is there a
prerequisite that I missed?

Larry

[PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Christian Lamparter]

This patch is necessary in order to provide a proper Access point support for p54.
Unfortunately 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_ps callback, 
so that we can update the filter state.
  
Signed-off-by: Christian Lamparter <chunkeey@web.de>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
---
 the extra patch for mac80211_hwsim is no longer needed...
This whole series is all for wireless-testing.
 
erm... now with a updated documentation!!1one
---
> >  
> >  	clear_sta_flags(sta, WLAN_STA_PS | WLAN_STA_PSPOLL);
> > +	if (local->ops->sta_notify_ps)
> > +		local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
> > +					  &sta->sta);
> 
> With this patch, STA_WILL_SLEEP and STA_WOKE_UP are undefined. Is there a
> prerequisite that I missed?
> Larry
no it's just fsck fallout. one file got synced in time... the other didn't.
SLUB_DEBUG/SLAB_DEBUG don't work at all with p54usb.
---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 6a1d4ea..4b972fc 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -779,6 +779,19 @@ enum sta_notify_cmd {
 };
 
 /**
+ * enum sta_notify_ps_cmd - sta power save notify command
+ *
+ * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
+ * notify the driver if a station made a power state transition.
+ *
+ * @STA_NOTIFY_SLEEP: a station is now sleeping
+ * @STA_NOTIFY_AWAKE: a sleeping station woke up
+ */
+enum sta_notify_ps_cmd {
+	STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
+};
+
+/**
  * enum ieee80211_tkip_key_type - get tkip key
  *
  * Used by drivers which need to get a tkip key for skb. Some drivers need a
@@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
  * @sta_notify: Notifies low level driver about addition or removal
  *	of associated station or AP.
  *
+ * @sta_ps_notify: Notifies low level driver about the power state transition
+ *	of a associated station. Must be atomic.
+ *
  * @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
  *	bursting) for a hardware TX queue.
  *
@@ -1314,6 +1330,8 @@ struct ieee80211_ops {
 	int (*set_frag_threshold)(struct ieee80211_hw *hw, u32 value);
 	void (*sta_notify)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			enum sta_notify_cmd, struct ieee80211_sta *sta);
+	void (*sta_notify_ps)(struct ieee80211_hw *hw,
+			enum sta_notify_ps_cmd, struct ieee80211_sta *sta);
 	int (*conf_tx)(struct ieee80211_hw *hw, u16 queue,
 		       const struct ieee80211_tx_queue_params *params);
 	int (*get_tx_stats)(struct ieee80211_hw *hw,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 5a1a60f..d2e8cc3 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_ps)
+		local->ops->sta_notify_ps(local_to_hw(local), 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_ps)
+		local->ops->sta_notify_ps(local_to_hw(local), STA_NOTIFY_AWAKE,
+					  &sta->sta);
 
 	if (!skb_queue_empty(&sta->ps_tx_buf))
 		sta_info_clear_tim_bit(sta);

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Luis R. Rodriguez]

On Sat, Nov 29, 2008 at 03:48:41PM -0800, Christian Lamparter wrote:
> This patch is necessary in order to provide a proper Access point support for p54.
> Unfortunately 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_ps callback,
> so that we can update the filter state.
> 
> Signed-off-by: Christian Lamparter <chunkeey@web.de>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>
> ---
>  the extra patch for mac80211_hwsim is no longer needed...
> This whole series is all for wireless-testing.
> 
> erm... now with a updated documentation!!1one
> ---
> > >
> > >     clear_sta_flags(sta, WLAN_STA_PS | WLAN_STA_PSPOLL);
> > > +   if (local->ops->sta_notify_ps)
> > > +           local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
> > > +                                     &sta->sta);
> >
> > With this patch, STA_WILL_SLEEP and STA_WOKE_UP are undefined. Is there a
> > prerequisite that I missed?
> > Larry
> no it's just fsck fallout. one file got synced in time... the other didn't.
> SLUB_DEBUG/SLAB_DEBUG don't work at all with p54usb.
> ---
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 6a1d4ea..4b972fc 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -779,6 +779,19 @@ enum sta_notify_cmd {
>  };
> 
>  /**
> + * enum sta_notify_ps_cmd - sta power save notify command
> + *
> + * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
> + * notify the driver if a station made a power state transition.
> + *
> + * @STA_NOTIFY_SLEEP: a station is now sleeping
> + * @STA_NOTIFY_AWAKE: a sleeping station woke up
> + */
> +enum sta_notify_ps_cmd {
> +       STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
> +};
> +
> +/**
>   * enum ieee80211_tkip_key_type - get tkip key
>   *
>   * Used by drivers which need to get a tkip key for skb. Some drivers need a
> @@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
>   * @sta_notify: Notifies low level driver about addition or removal
>   *     of associated station or AP.
>   *
> + * @sta_ps_notify: Notifies low level driver about the power state transition
> + *     of a associated station. Must be atomic.
> + *

Can we add documentation that this is only required when the AP cannot
figure this out by itself? This can help people working on AP support to
not think they need to implement this for all drivers.

A patch can go on top of this I suppose.

  Luis

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Christian Lamparter]

On Monday 01 December 2008 20:17:23 Luis R. Rodriguez wrote:
> On Sat, Nov 29, 2008 at 03:48:41PM -0800, Christian Lamparter wrote:
> > This patch is necessary in order to provide a proper Access point support for p54.
> > Unfortunately 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_ps callback,
> > so that we can update the filter state.
> > 
> > Signed-off-by: Christian Lamparter <chunkeey@web.de>
> > Acked-by: Johannes Berg <johannes@sipsolutions.net>
> > ---
> > +/**
> >   * enum ieee80211_tkip_key_type - get tkip key
> >   *
> >   * Used by drivers which need to get a tkip key for skb. Some drivers need a
> > @@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
> >   * @sta_notify: Notifies low level driver about addition or removal
> >   *     of associated station or AP.
> >   *
> > + * @sta_ps_notify: Notifies low level driver about the power state transition
> > + *     of a associated station. Must be atomic.
> > + *
> 
> Can we add documentation that this is only required when the AP cannot
> figure this out by itself? This can help people working on AP support to
> not think they need to implement this for all drivers.

Do you think "This callback is optional. Only use it, if your hardware/
firmware really implements a station filter table" is enough (and correct spelled btw?)
Or do you volunteer for adding one or two DOC entries about APs, BC & MC Buffering,
TIM and station filters mechanism, caveats, and available callbacks or solutions?

Regards,
	Chr

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Luis R. Rodriguez]

On Tue, Dec 02, 2008 at 07:55:53AM -0800, Christian Lamparter wrote:
> On Monday 01 December 2008 20:17:23 Luis R. Rodriguez wrote:
> > On Sat, Nov 29, 2008 at 03:48:41PM -0800, Christian Lamparter wrote:
> > > This patch is necessary in order to provide a proper Access point support for p54.
> > > Unfortunately 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_ps callback,
> > > so that we can update the filter state.
> > >
> > > Signed-off-by: Christian Lamparter <chunkeey@web.de>
> > > Acked-by: Johannes Berg <johannes@sipsolutions.net>
> > > ---
> > > +/**
> > >   * enum ieee80211_tkip_key_type - get tkip key
> > >   *
> > >   * Used by drivers which need to get a tkip key for skb. Some drivers need a
> > > @@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
> > >   * @sta_notify: Notifies low level driver about addition or removal
> > >   *     of associated station or AP.
> > >   *
> > > + * @sta_ps_notify: Notifies low level driver about the power state transition
> > > + *     of a associated station. Must be atomic.
> > > + *
> >
> > Can we add documentation that this is only required when the AP cannot
> > figure this out by itself? This can help people working on AP support to
> > not think they need to implement this for all drivers.
> 
> Do you think "This callback is optional. Only use it, if your hardware/
> firmware really implements a station filter table" 

You would know better, I don't know why this callback was needed in
terms of hardware, but from reading the reason why it came up it seems
it was specific to p54. I just wanted that to be pointed out so new
developers reading the kdoc for it would know it maybe optional.

So atheros hardware supports a BSSID mask and we use this to know which
STAs to send ACKs to. Is this what the p54 "station filter table" is
used for? If not what is meant by a "station filter table"?

> is enough (and correct spelled btw?)
> Or do you volunteer for adding one or two DOC entries about APs, BC & MC Buffering,
> TIM and station filters mechanism, caveats, and available callbacks or solutions?

Well I did add this but to the wiki (with Ivan's help on the TIM stuff
and image):

http://wireless.kernel.org/en/developers/Documentation/ieee80211/power-savings

And unfortunately it is not very hardware design specifics, and has
nothing about caveats, or filter mechanisms. But feel free to take as
you like/refer to it/edit as you see fit.

  Luis

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Johannes Berg]

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

On Tue, 2008-12-02 at 16:55 +0100, Christian Lamparter wrote:

> Do you think "This callback is optional. Only use it, if your hardware/
> firmware really implements a station filter table" is enough (and correct spelled btw?)
> Or do you volunteer for adding one or two DOC entries about APs, BC & MC Buffering,
> TIM and station filters mechanism, caveats, and available callbacks or solutions?

Incidentally, I was entirely wrong about the sta_notify callback being
allowed to sleep, it's not, it's under a spinlock, and, more
importantly, RCU read lock for station removal (and this cannot be
changed). Must have been wishful thinking...

johannes

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

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Christian Lamparter]

On Monday 08 December 2008 11:16:09 Johannes Berg wrote:
> On Tue, 2008-12-02 at 16:55 +0100, Christian Lamparter wrote:
> 
> > Do you think "This callback is optional. Only use it, if your hardware/
> > firmware really implements a station filter table" is enough (and correct spelled btw?)
> > Or do you volunteer for adding one or two DOC entries about APs, BC & MC Buffering,
> > TIM and station filters mechanism, caveats, and available callbacks or solutions?
> 
> Incidentally, I was entirely wrong about the sta_notify callback being
> allowed to sleep, it's not, it's under a spinlock, and, more
> importantly, RCU read lock for station removal (and this cannot be
> changed). Must have been wishful thinking...
> 
So?
can I move the notification enums STA_NOTIFY_SLEEP/STA_NOTIFY_AWAKE
to the sta_notify callback again, or is this wishful thinking as well?
After all, sta_notify wasn't intended to be used to tell the driver about
power state transitions but it does fit the bill...


Regards,
	Chr

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Johannes Berg]

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

On Mon, 2008-12-08 at 15:20 +0100, Christian Lamparter wrote:
> On Monday 08 December 2008 11:16:09 Johannes Berg wrote:
> > On Tue, 2008-12-02 at 16:55 +0100, Christian Lamparter wrote:
> > 
> > > Do you think "This callback is optional. Only use it, if your hardware/
> > > firmware really implements a station filter table" is enough (and correct spelled btw?)
> > > Or do you volunteer for adding one or two DOC entries about APs, BC & MC Buffering,
> > > TIM and station filters mechanism, caveats, and available callbacks or solutions?
> > 
> > Incidentally, I was entirely wrong about the sta_notify callback being
> > allowed to sleep, it's not, it's under a spinlock, and, more
> > importantly, RCU read lock for station removal (and this cannot be
> > changed). Must have been wishful thinking...
> > 
> So?
> can I move the notification enums STA_NOTIFY_SLEEP/STA_NOTIFY_AWAKE
> to the sta_notify callback again, or is this wishful thinking as well?
> After all, sta_notify wasn't intended to be used to tell the driver about
> power state transitions but it does fit the bill...

Don't bother, I just wanted to point out I had been wrong.

johannes

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

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Christian Lamparter]

On Monday 01 December 2008 20:17:23 Luis R. Rodriguez wrote:
> On Sat, Nov 29, 2008 at 03:48:41PM -0800, Christian Lamparter wrote:
> > This patch is necessary in order to provide a proper Access point support for p54.
> > Unfortunately 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_ps callback,
> > so that we can update the filter state.
> > 
> > Signed-off-by: Christian Lamparter <chunkeey@web.de>
> > Acked-by: Johannes Berg <johannes@sipsolutions.net>
> > ---
> >  the extra patch for mac80211_hwsim is no longer needed...
> > This whole series is all for wireless-testing.
> > 
> > erm... now with a updated documentation!!1one
> > ---
> > > >
> > > >     clear_sta_flags(sta, WLAN_STA_PS | WLAN_STA_PSPOLL);
> > > > +   if (local->ops->sta_notify_ps)
> > > > +           local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
> > > > +                                     &sta->sta);
> > >
> > > With this patch, STA_WILL_SLEEP and STA_WOKE_UP are undefined. Is there a
> > > prerequisite that I missed?
> > > Larry
> > no it's just fsck fallout. one file got synced in time... the other didn't.
> > SLUB_DEBUG/SLAB_DEBUG don't work at all with p54usb.
> > ---
> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > index 6a1d4ea..4b972fc 100644
> > --- a/include/net/mac80211.h
> > +++ b/include/net/mac80211.h
> > @@ -779,6 +779,19 @@ enum sta_notify_cmd {
> >  };
> > 
> >  /**
> > + * enum sta_notify_ps_cmd - sta power save notify command
> > + *
> > + * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
> > + * notify the driver if a station made a power state transition.
> > + *
> > + * @STA_NOTIFY_SLEEP: a station is now sleeping
> > + * @STA_NOTIFY_AWAKE: a sleeping station woke up
> > + */
> > +enum sta_notify_ps_cmd {
> > +       STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
> > +};
> > +
> > +/**
> >   * enum ieee80211_tkip_key_type - get tkip key
> >   *
> >   * Used by drivers which need to get a tkip key for skb. Some drivers need a
> > @@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
> >   * @sta_notify: Notifies low level driver about addition or removal
> >   *     of associated station or AP.
> >   *
> > + * @sta_ps_notify: Notifies low level driver about the power state transition
> > + *     of a associated station. Must be atomic.
> > + *
> 
> Can we add documentation that this is only required when the AP cannot
> figure this out by itself? This can help people working on AP support to
> not think they need to implement this for all drivers.
> 
Hmm, yeah... should I edit the documentation for sta_notify as well?

Regards,
	Chr

Re: [PATCH 1/6 v2.2] mac80211: add sta_notify_ps callback

[Luis R. Rodriguez]

On Mon, Dec 1, 2008 at 2:00 PM, Christian Lamparter <chunkeey@web.de> wrote:
> On Monday 01 December 2008 20:17:23 Luis R. Rodriguez wrote:
>> On Sat, Nov 29, 2008 at 03:48:41PM -0800, Christian Lamparter wrote:
>> > This patch is necessary in order to provide a proper Access point support for p54.
>> > Unfortunately 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_ps callback,
>> > so that we can update the filter state.
>> >
>> > Signed-off-by: Christian Lamparter <chunkeey@web.de>
>> > Acked-by: Johannes Berg <johannes@sipsolutions.net>
>> > ---
>> >  the extra patch for mac80211_hwsim is no longer needed...
>> > This whole series is all for wireless-testing.
>> >
>> > erm... now with a updated documentation!!1one
>> > ---
>> > > >
>> > > >     clear_sta_flags(sta, WLAN_STA_PS | WLAN_STA_PSPOLL);
>> > > > +   if (local->ops->sta_notify_ps)
>> > > > +           local->ops->sta_notify_ps(local_to_hw(local), STA_WOKE_UP,
>> > > > +                                     &sta->sta);
>> > >
>> > > With this patch, STA_WILL_SLEEP and STA_WOKE_UP are undefined. Is there a
>> > > prerequisite that I missed?
>> > > Larry
>> > no it's just fsck fallout. one file got synced in time... the other didn't.
>> > SLUB_DEBUG/SLAB_DEBUG don't work at all with p54usb.
>> > ---
>> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> > index 6a1d4ea..4b972fc 100644
>> > --- a/include/net/mac80211.h
>> > +++ b/include/net/mac80211.h
>> > @@ -779,6 +779,19 @@ enum sta_notify_cmd {
>> >  };
>> >
>> >  /**
>> > + * enum sta_notify_ps_cmd - sta power save notify command
>> > + *
>> > + * Used with the sta_notify_ps() callback in &struct ieee80211_ops to
>> > + * notify the driver if a station made a power state transition.
>> > + *
>> > + * @STA_NOTIFY_SLEEP: a station is now sleeping
>> > + * @STA_NOTIFY_AWAKE: a sleeping station woke up
>> > + */
>> > +enum sta_notify_ps_cmd {
>> > +       STA_NOTIFY_SLEEP, STA_NOTIFY_AWAKE,
>> > +};
>> > +
>> > +/**
>> >   * enum ieee80211_tkip_key_type - get tkip key
>> >   *
>> >   * Used by drivers which need to get a tkip key for skb. Some drivers need a
>> > @@ -1248,6 +1261,9 @@ enum ieee80211_ampdu_mlme_action {
>> >   * @sta_notify: Notifies low level driver about addition or removal
>> >   *     of associated station or AP.
>> >   *
>> > + * @sta_ps_notify: Notifies low level driver about the power state transition
>> > + *     of a associated station. Must be atomic.
>> > + *
>>
>> Can we add documentation that this is only required when the AP cannot
>> figure this out by itself? This can help people working on AP support to
>> not think they need to implement this for all drivers.
>>
> Hmm, yeah... should I edit the documentation for sta_notify as well?

Would be good if it helps.

  Luis