[PATCH 1/8] iwlwifi: do not set tx power when channel is changing

[PATCH 1/8] iwlwifi: do not set tx power when channel is changing

[Stanislaw Gruszka]

Mac80211 can request for tx power and channel change in one ->config
call. If that happens, *_send_tx_power functions will try to setup tx
power for old channel, what can be not correct because we already change
the band. I.e  error  "Failed to get channel info for channel 140 [0]",
can be printed frequently when operating in software scanning mode.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-3945.c     |    2 +-
 drivers/net/wireless/iwlwifi/iwl-4965.c     |    2 +-
 drivers/net/wireless/iwlwifi/iwl-agn-rxon.c |    5 ++---
 drivers/net/wireless/iwlwifi/iwl-core.c     |   13 ++++++++++---
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
index 1d9dcd7..294221b 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
@@ -1890,7 +1890,7 @@ int iwl3945_commit_rxon(struct iwl_priv *priv, struct iwl_rxon_context *ctx)
 
 	/* If we issue a new RXON command which required a tune then we must
 	 * send a new TXPOWER command or we won't be able to Tx any frames */
-	rc = priv->cfg->ops->lib->send_tx_power(priv);
+	rc = iwl_set_tx_power(priv, priv->tx_power_next, true);
 	if (rc) {
 		IWL_ERR(priv, "Error setting Tx power (%d).\n", rc);
 		return rc;
diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 313e92e..7c14eb3 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -1571,7 +1571,7 @@ static int iwl4965_commit_rxon(struct iwl_priv *priv, struct iwl_rxon_context *c
 
 	/* If we issue a new RXON command which required a tune then we must
 	 * send a new TXPOWER command or we won't be able to Tx any frames */
-	ret = iwl_set_tx_power(priv, priv->tx_power_user_lmt, true);
+	ret = iwl_set_tx_power(priv, priv->tx_power_next, true);
 	if (ret) {
 		IWL_ERR(priv, "Error sending TX power (%d)\n", ret);
 		return ret;
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c b/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c
index 2a4ff83..6c2adc5 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rxon.c
@@ -316,10 +316,9 @@ int iwlagn_commit_rxon(struct iwl_priv *priv, struct iwl_rxon_context *ctx)
 	 * If we issue a new RXON command which required a tune then we must
 	 * send a new TXPOWER command or we won't be able to Tx any frames.
 	 *
-	 * FIXME: which RXON requires a tune? Can we optimise this out in
-	 *        some cases?
+	 * It's expected we set power here if channel is changing.
 	 */
-	ret = iwl_set_tx_power(priv, priv->tx_power_user_lmt, true);
+	ret = iwl_set_tx_power(priv, priv->tx_power_next, true);
 	if (ret) {
 		IWL_ERR(priv, "Error sending TX power (%d)\n", ret);
 		return ret;
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index a46ad60..92724cb 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -1162,6 +1162,8 @@ int iwl_set_tx_power(struct iwl_priv *priv, s8 tx_power, bool force)
 {
 	int ret;
 	s8 prev_tx_power;
+	bool defer;
+	struct iwl_rxon_context *ctx = &priv->contexts[IWL_RXON_CTX_BSS];
 
 	lockdep_assert_held(&priv->mutex);
 
@@ -1189,10 +1191,15 @@ int iwl_set_tx_power(struct iwl_priv *priv, s8 tx_power, bool force)
 	if (!iwl_is_ready_rf(priv))
 		return -EIO;
 
-	/* scan complete use tx_power_next, need to be updated */
+	/* scan complete and commit_rxon use tx_power_next value,
+	 * it always need to be updated for newest request */
 	priv->tx_power_next = tx_power;
-	if (test_bit(STATUS_SCANNING, &priv->status) && !force) {
-		IWL_DEBUG_INFO(priv, "Deferring tx power set while scanning\n");
+
+	/* do not set tx power when scanning or channel changing */
+	defer = test_bit(STATUS_SCANNING, &priv->status) ||
+		memcmp(&ctx->active, &ctx->staging, sizeof(ctx->staging));
+	if (defer && !force) {
+		IWL_DEBUG_INFO(priv, "Deferring tx power set\n");
 		return 0;
 	}
 
-- 
1.7.1

[PATCH 2/8] iwl3945: set STATUS_READY before commit_rxon

[Stanislaw Gruszka]

Similar change as we already do for agn, need to avoid
"Error setting Tx power (-5)" message when loading module.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 9c986f2..55837d6 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2535,13 +2535,14 @@ static void iwl3945_alive_start(struct iwl_priv *priv)
 	/* Configure Bluetooth device coexistence support */
 	priv->cfg->ops->hcmd->send_bt_config(priv);
 
+	set_bit(STATUS_READY, &priv->status);
+
 	/* Configure the adapter for unassociated operation */
 	iwl3945_commit_rxon(priv, ctx);
 
 	iwl3945_reg_txpower_periodic(priv);
 
 	IWL_DEBUG_INFO(priv, "ALIVE processing complete.\n");
-	set_bit(STATUS_READY, &priv->status);
 	wake_up_interruptible(&priv->wait_command_queue);
 
 	return;
-- 
1.7.1

Re: [PATCH 2/8] iwl3945: set STATUS_READY before commit_rxon

[wwguy]

Stanislaw,

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> Similar change as we already do for agn, need to avoid
> "Error setting Tx power (-5)" message when loading module.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 

good catch, miss that

Wey

[PATCH 3/8] iwlwifi: remove unneeded __packed

[Stanislaw Gruszka]

struct iwl_queue is not part of firmware interface, so __packed is not
needed. Remove it since is may affect performance.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-dev.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index 6fa1383..b5f21e0 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -138,7 +138,7 @@ struct iwl_queue {
 				* space more than this */
 	int high_mark;         /* high watermark, stop queue if free
 				* space less than this */
-} __packed;
+};
 
 /* One for each TFD */
 struct iwl_tx_info {
-- 
1.7.1

Re: [PATCH 3/8] iwlwifi: remove unneeded __packed

[wwguy]

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> struct iwl_queue is not part of firmware interface, so __packed is not
> needed. Remove it since is may affect performance.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-dev.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
Wey

[PATCH 4/8] iwlwifi: introduce iwl_advanced_bt_coexist()

[Stanislaw Gruszka]

We use priv->cfg->bt_params && priv->cfg->bt_params->advanced_bt_coexist
conditional in few places, merge it into one function.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-core.h    |    6 ++++++
 drivers/net/wireless/iwlwifi/iwl-debugfs.c |    2 +-
 drivers/net/wireless/iwlwifi/iwl-power.c   |    6 ++----
 drivers/net/wireless/iwlwifi/iwl-scan.c    |    3 +--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index bbc5aa7..705711a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -741,6 +741,12 @@ static inline const struct ieee80211_supported_band *iwl_get_hw_mode(
 	return priv->hw->wiphy->bands[band];
 }
 
+static inline bool iwl_advanced_bt_coexist(struct iwl_priv *priv)
+{
+	return priv->cfg->bt_params &&
+	       priv->cfg->bt_params->advanced_bt_coexist;
+}
+
 extern bool bt_coex_active;
 extern bool bt_siso_mode;
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-debugfs.c b/drivers/net/wireless/iwlwifi/iwl-debugfs.c
index 418c8ac..bde16ac 100644
--- a/drivers/net/wireless/iwlwifi/iwl-debugfs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-debugfs.c
@@ -1771,7 +1771,7 @@ int iwl_dbgfs_register(struct iwl_priv *priv, const char *name)
 	DEBUGFS_ADD_FILE(rxon_flags, dir_debug, S_IWUSR);
 	DEBUGFS_ADD_FILE(rxon_filter_flags, dir_debug, S_IWUSR);
 	DEBUGFS_ADD_FILE(wd_timeout, dir_debug, S_IWUSR);
-	if (priv->cfg->bt_params && priv->cfg->bt_params->advanced_bt_coexist)
+	if (iwl_advanced_bt_coexist(priv))
 		DEBUGFS_ADD_FILE(bt_traffic, dir_debug, S_IRUSR);
 	if (priv->cfg->base_params->sensitivity_calib_by_driver)
 		DEBUGFS_ADD_BOOL(disable_sensitivity, dir_rf,
diff --git a/drivers/net/wireless/iwlwifi/iwl-power.c b/drivers/net/wireless/iwlwifi/iwl-power.c
index 1eec18d..25f7d47 100644
--- a/drivers/net/wireless/iwlwifi/iwl-power.c
+++ b/drivers/net/wireless/iwlwifi/iwl-power.c
@@ -226,8 +226,7 @@ static void iwl_static_sleep_cmd(struct iwl_priv *priv,
 	else
 		cmd->flags &= ~IWL_POWER_SHADOW_REG_ENA;
 
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->advanced_bt_coexist) {
+	if (iwl_advanced_bt_coexist(priv)) {
 		if (!priv->cfg->bt_params->bt_sco_disable)
 			cmd->flags |= IWL_POWER_BT_SCO_ENA;
 		else
@@ -313,8 +312,7 @@ static void iwl_power_fill_sleep_cmd(struct iwl_priv *priv,
 	else
 		cmd->flags &= ~IWL_POWER_SHADOW_REG_ENA;
 
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->advanced_bt_coexist) {
+	if (iwl_advanced_bt_coexist(priv)) {
 		if (!priv->cfg->bt_params->bt_sco_disable)
 			cmd->flags |= IWL_POWER_BT_SCO_ENA;
 		else
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 12d9363..08f1bea 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -257,8 +257,7 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
 	queue_work(priv->workqueue, &priv->scan_completed);
 
 	if (priv->iw_mode != NL80211_IFTYPE_ADHOC &&
-	    priv->cfg->bt_params &&
-	    priv->cfg->bt_params->advanced_bt_coexist &&
+	    iwl_advanced_bt_coexist(priv) &&
 	    priv->bt_status != scan_notif->bt_status) {
 		if (scan_notif->bt_status) {
 			/* BT on */
-- 
1.7.1

Re: [PATCH 4/8] iwlwifi: introduce iwl_advanced_bt_coexist()

[wwguy]

Hi Stanislaw,

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> We use priv->cfg->bt_params && priv->cfg->bt_params->advanced_bt_coexist
> conditional in few places, merge it into one function.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-core.h    |    6 ++++++
>  drivers/net/wireless/iwlwifi/iwl-debugfs.c |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-power.c   |    6 ++----
>  drivers/net/wireless/iwlwifi/iwl-scan.c    |    3 +--
>  4 files changed, 10 insertions(+), 7 deletions(-)

Wey

 

[PATCH 5/8] iwlwifi: remove unneeded disable_hw_scan check

[Stanislaw Gruszka]

We never set STATUS_SCANNING in softwre scanning mode, disable_hw_scan
check is unneeded. Correct debug message while at it.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-legacy.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-legacy.c b/drivers/net/wireless/iwlwifi/iwl-legacy.c
index 927fe37..e1ace3c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-legacy.c
+++ b/drivers/net/wireless/iwlwifi/iwl-legacy.c
@@ -85,10 +85,9 @@ int iwl_legacy_mac_config(struct ieee80211_hw *hw, u32 changed)
 	IWL_DEBUG_MAC80211(priv, "enter to channel %d changed 0x%X\n",
 					channel->hw_value, changed);
 
-	if (unlikely(!priv->cfg->mod_params->disable_hw_scan &&
-			test_bit(STATUS_SCANNING, &priv->status))) {
+	if (unlikely(test_bit(STATUS_SCANNING, &priv->status))) {
 		scan_active = 1;
-		IWL_DEBUG_MAC80211(priv, "leave - scanning\n");
+		IWL_DEBUG_MAC80211(priv, "scan active\n");
 	}
 
 	if (changed & (IEEE80211_CONF_CHANGE_SMPS |
-- 
1.7.1

Re: [PATCH 5/8] iwlwifi: remove unneeded disable_hw_scan check

[wwguy]

Stanislaw,

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> We never set STATUS_SCANNING in softwre scanning mode, disable_hw_scan
> check is unneeded. Correct debug message while at it.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---

[PATCH 6/8] iwl3945: do not use agn specific IWL_RATE_COUNT

[Stanislaw Gruszka]

Only use IWL_RATE_COUNT_3945 in 3945 code.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-3945.c     |    5 ++---
 drivers/net/wireless/iwlwifi/iwl-agn-rs.h   |    1 +
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
index 294221b..58213e7 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
@@ -762,8 +762,7 @@ void iwl3945_hw_build_tx_cmd_rate(struct iwl_priv *priv,
 
 	/* We need to figure out how to get the sta->supp_rates while
 	 * in this running context */
-	rate_mask = IWL_RATES_MASK;
-
+	rate_mask = IWL_RATES_MASK_3945;
 
 	/* Set retry limit on DATA packets and Probe Responses*/
 	if (ieee80211_is_probe_resp(fc))
@@ -1650,7 +1649,7 @@ static int iwl3945_hw_reg_comp_txpower_temp(struct iwl_priv *priv)
 							      ref_temp);
 
 		/* set tx power value for all rates, OFDM and CCK */
-		for (rate_index = 0; rate_index < IWL_RATE_COUNT;
+		for (rate_index = 0; rate_index < IWL_RATE_COUNT_3945;
 		     rate_index++) {
 			int power_idx =
 			    ch_info->power_info[rate_index].base_power_index;
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rs.h b/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
index 75e50d3..184828c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
@@ -213,6 +213,7 @@ enum {
 	 IWL_CCK_BASIC_RATES_MASK)
 
 #define IWL_RATES_MASK ((1 << IWL_RATE_COUNT) - 1)
+#define IWL_RATES_MASK_3945 ((1 << IWL_RATE_COUNT_3945) - 1)
 
 #define IWL_INVALID_VALUE    -1
 
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 55837d6..2945acd 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2517,7 +2517,7 @@ static void iwl3945_alive_start(struct iwl_priv *priv)
 
 	ieee80211_wake_queues(priv->hw);
 
-	priv->active_rate = IWL_RATES_MASK;
+	priv->active_rate = IWL_RATES_MASK_3945;
 
 	iwl_power_update_mode(priv, true);
 
-- 
1.7.1

Re: [PATCH 6/8] iwl3945: do not use agn specific IWL_RATE_COUNT

[wwguy]

Stanislaw,

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> Only use IWL_RATE_COUNT_3945 in 3945 code.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945.c     |    5 ++---
>  drivers/net/wireless/iwlwifi/iwl-agn-rs.h   |    1 +
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
> index 294221b..58213e7 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
> @@ -762,8 +762,7 @@ void iwl3945_hw_build_tx_cmd_rate(struct iwl_priv *priv,
>  
>  	/* We need to figure out how to get the sta->supp_rates while
>  	 * in this running context */
> -	rate_mask = IWL_RATES_MASK;
> -
> +	rate_mask = IWL_RATES_MASK_3945;
>  
>  	/* Set retry limit on DATA packets and Probe Responses*/
>  	if (ieee80211_is_probe_resp(fc))
> @@ -1650,7 +1649,7 @@ static int iwl3945_hw_reg_comp_txpower_temp(struct iwl_priv *priv)
>  							      ref_temp);
>  
>  		/* set tx power value for all rates, OFDM and CCK */
> -		for (rate_index = 0; rate_index < IWL_RATE_COUNT;
> +		for (rate_index = 0; rate_index < IWL_RATE_COUNT_3945;
>  		     rate_index++) {
>  			int power_idx =
>  			    ch_info->power_info[rate_index].base_power_index;
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rs.h b/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> index 75e50d3..184828c 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> @@ -213,6 +213,7 @@ enum {
>  	 IWL_CCK_BASIC_RATES_MASK)
>  
>  #define IWL_RATES_MASK ((1 << IWL_RATE_COUNT) - 1)
> +#define IWL_RATES_MASK_3945 ((1 << IWL_RATE_COUNT_3945) - 1)
>  

Why define 3945 in agn header file?
I understand iwl-agn-rs.h is include in iwl-dev.h, which is not the
right thing to do. but specify 3945 in agn header file make it even more
confuse.


Wey

Re: [PATCH 6/8] iwl3945: do not use agn specific IWL_RATE_COUNT

[Stanislaw Gruszka]

On Fri, Jan 28, 2011 at 08:20:48AM -0800, wwguy wrote:
> > --- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> > +++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> > @@ -213,6 +213,7 @@ enum {
> >  	 IWL_CCK_BASIC_RATES_MASK)
> >  
> >  #define IWL_RATES_MASK ((1 << IWL_RATE_COUNT) - 1)
> > +#define IWL_RATES_MASK_3945 ((1 << IWL_RATE_COUNT_3945) - 1)
> >  
> 
> Why define 3945 in agn header file?
> I understand iwl-agn-rs.h is include in iwl-dev.h, which is not the
> right thing to do. but specify 3945 in agn header file make it even more
> confuse.

$ grep 3945 drivers/net/wireless/iwlwifi/iwl-agn-rs.h
struct iwl3945_rate_info {
	IWL_RATE_COUNT_3945 = IWL_RATE_COUNT - 1,
 * iwl3945_rate_scale_init - Initialize the rate scale table based on
 * assoc info
extern void iwl3945_rate_scale_init(struct ieee80211_hw *hw, s32
sta_id);
extern void iwl3945_rs_rate_init(struct iwl_priv *priv,
extern int iwl3945_rate_control_register(void);
extern void iwl3945_rate_control_unregister(void);

This file contains 3945 data, I agree this is confused
and need cleanup :)

Stanislaw

Re: [PATCH 6/8] iwl3945: do not use agn specific IWL_RATE_COUNT

[wwguy]

Stanislaw,

On Fri, 2011-01-28 at 08:33 -0800, Stanislaw Gruszka wrote:
> On Fri, Jan 28, 2011 at 08:20:48AM -0800, wwguy wrote:
> > > --- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> > > @@ -213,6 +213,7 @@ enum {
> > >  	 IWL_CCK_BASIC_RATES_MASK)
> > >  
> > >  #define IWL_RATES_MASK ((1 << IWL_RATE_COUNT) - 1)
> > > +#define IWL_RATES_MASK_3945 ((1 << IWL_RATE_COUNT_3945) - 1)
> > >  
> > 
> > Why define 3945 in agn header file?
> > I understand iwl-agn-rs.h is include in iwl-dev.h, which is not the
> > right thing to do. but specify 3945 in agn header file make it even more
> > confuse.
> 
> $ grep 3945 drivers/net/wireless/iwlwifi/iwl-agn-rs.h
> struct iwl3945_rate_info {
> 	IWL_RATE_COUNT_3945 = IWL_RATE_COUNT - 1,
>  * iwl3945_rate_scale_init - Initialize the rate scale table based on
>  * assoc info
> extern void iwl3945_rate_scale_init(struct ieee80211_hw *hw, s32
> sta_id);
> extern void iwl3945_rs_rate_init(struct iwl_priv *priv,
> extern int iwl3945_rate_control_register(void);
> extern void iwl3945_rate_control_unregister(void);
> 
> This file contains 3945 data, I agree this is confused
> and need cleanup :)
> 
we are in the process of split 3945/4965 driver from _agn_

if it is ok for you, could you wait for we done the work to see if
anything need to be clean up 


Thanks
Wey

Re: [PATCH 6/8] iwl3945: do not use agn specific IWL_RATE_COUNT

[wwguy]

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> Only use IWL_RATE_COUNT_3945 in 3945 code.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945.c     |    5 ++---
>  drivers/net/wireless/iwlwifi/iwl-agn-rs.h   |    1 +
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
oops, sure

Wey

[PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[Stanislaw Gruszka]

This option practically do not change object files size, and only add
complication for users wanted to play with experimental firmware.
Someone who want to use or stop to use experimental firmware can
simply add or remove proper ucode files in /lib/firmware

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/Kconfig   |    6 ------
 drivers/net/wireless/iwlwifi/iwl-agn.c |    2 --
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
index e1e3b1c..4a5c51f 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -43,12 +43,6 @@ config IWLWIFI_DEBUGFS
 	  is a low-impact option that allows getting insight into the
 	  driver's state at runtime.
 
-config IWLWIFI_DEBUG_EXPERIMENTAL_UCODE
-        bool "Experimental uCode support"
-        depends on IWLWIFI && IWLWIFI_DEBUG
-        ---help---
-	  Enable use of experimental ucode for testing and debugging.
-
 config IWLWIFI_DEVICE_TRACING
 	bool "iwlwifi device access tracing"
 	depends on IWLWIFI
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index eb16647..23bf006 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -1610,11 +1610,9 @@ static int __must_check iwl_request_firmware(struct iwl_priv *priv, bool first)
 	char tag[8];
 
 	if (first) {
-#ifdef CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE
 		priv->fw_index = UCODE_EXPERIMENTAL_INDEX;
 		strcpy(tag, UCODE_EXPERIMENTAL_TAG);
 	} else if (priv->fw_index == UCODE_EXPERIMENTAL_INDEX) {
-#endif
 		priv->fw_index = priv->cfg->ucode_api_max;
 		sprintf(tag, "%d", priv->fw_index);
 	} else {
-- 
1.7.1

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[wwguy]

Stanislaw,

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> This option practically do not change object files size, and only add
> complication for users wanted to play with experimental firmware.
> Someone who want to use or stop to use experimental firmware can
> simply add or remove proper ucode files in /lib/firmware
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/Kconfig   |    6 ------
>  drivers/net/wireless/iwlwifi/iwl-agn.c |    2 --
>  2 files changed, 0 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
> index e1e3b1c..4a5c51f 100644
> --- a/drivers/net/wireless/iwlwifi/Kconfig
> +++ b/drivers/net/wireless/iwlwifi/Kconfig
> @@ -43,12 +43,6 @@ config IWLWIFI_DEBUGFS
>  	  is a low-impact option that allows getting insight into the
>  	  driver's state at runtime.
>  
> -config IWLWIFI_DEBUG_EXPERIMENTAL_UCODE
> -        bool "Experimental uCode support"
> -        depends on IWLWIFI && IWLWIFI_DEBUG
> -        ---help---
> -	  Enable use of experimental ucode for testing and debugging.
> -
>  config IWLWIFI_DEVICE_TRACING
>  	bool "iwlwifi device access tracing"
>  	depends on IWLWIFI
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index eb16647..23bf006 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -1610,11 +1610,9 @@ static int __must_check iwl_request_firmware(struct iwl_priv *priv, bool first)
>  	char tag[8];
>  
>  	if (first) {
> -#ifdef CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE
>  		priv->fw_index = UCODE_EXPERIMENTAL_INDEX;
>  		strcpy(tag, UCODE_EXPERIMENTAL_TAG);
>  	} else if (priv->fw_index == UCODE_EXPERIMENTAL_INDEX) {
> -#endif
>  		priv->fw_index = priv->cfg->ucode_api_max;
>  		sprintf(tag, "%d", priv->fw_index);
>  	} else {

The whole reason has it in Kconfig file is to have user not remove the
experimental uCode from /lib/firmware and not use it.

Also put it in Kconfig file, so it is easy for user to see they are
using experimental option. Agree?

Wey

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[Stanislaw Gruszka]

On Fri, Jan 28, 2011 at 08:23:37AM -0800, wwguy wrote:
> > -#ifdef CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE
> >  		priv->fw_index = UCODE_EXPERIMENTAL_INDEX;
> >  		strcpy(tag, UCODE_EXPERIMENTAL_TAG);
> >  	} else if (priv->fw_index == UCODE_EXPERIMENTAL_INDEX) {
> > -#endif
> >  		priv->fw_index = priv->cfg->ucode_api_max;
> >  		sprintf(tag, "%d", priv->fw_index);
> >  	} else {
> 
> The whole reason has it in Kconfig file is to have user not remove the
> experimental uCode from /lib/firmware and not use it.

What is easiest and less time consuming:

this:

$ mv /lib/firmware/iwlwifi-5000-exp.ucode /lib/firmware/iwlwifi-5000-exp.ucode.not_use

or this:

$ make menuconfig
navigate to find option and edit it 
$ make modules
$ make modules_install

?

> Also put it in Kconfig file, so it is easy for user to see they are
> using experimental option. Agree?

Not really. I believe someone who download a tarball, unpack it and copy
files to /lib/firmware is pretty much aware what he/she is doing.

Stanislaw

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[wwguy]

Hi Stanislaw,

On Fri, 2011-01-28 at 08:41 -0800, Stanislaw Gruszka wrote:
> On Fri, Jan 28, 2011 at 08:23:37AM -0800, wwguy wrote:
> > > -#ifdef CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE
> > >  		priv->fw_index = UCODE_EXPERIMENTAL_INDEX;
> > >  		strcpy(tag, UCODE_EXPERIMENTAL_TAG);
> > >  	} else if (priv->fw_index == UCODE_EXPERIMENTAL_INDEX) {
> > > -#endif
> > >  		priv->fw_index = priv->cfg->ucode_api_max;
> > >  		sprintf(tag, "%d", priv->fw_index);
> > >  	} else {
> > 
> > The whole reason has it in Kconfig file is to have user not remove the
> > experimental uCode from /lib/firmware and not use it.
> 
> What is easiest and less time consuming:
> 
> this:
> 
> $ mv /lib/firmware/iwlwifi-5000-exp.ucode /lib/firmware/iwlwifi-5000-exp.ucode.not_use
> 
> or this:
> 
> $ make menuconfig
> navigate to find option and edit it 
> $ make modules
> $ make modules_install
> 
> ?
> 
> > Also put it in Kconfig file, so it is easy for user to see they are
> > using experimental option. Agree?
> 
> Not really. I believe someone who download a tarball, unpack it and copy
> files to /lib/firmware is pretty much aware what he/she is doing.
> 
Agree whoever download the tarball know what he/she is doing, but I am
not fully agree the driver always use experimental uCode if it is in the
firmware directory

Wey

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[Stanislaw Gruszka]

On Fri, Jan 28, 2011 at 03:20:21PM -0800, wwguy wrote:
> > What is easiest and less time consuming:
> > 
> > this:
> > 
> > $ mv /lib/firmware/iwlwifi-5000-exp.ucode /lib/firmware/iwlwifi-5000-exp.ucode.not_use
> > 
> > or this:
> > 
> > $ make menuconfig
> > navigate to find option and edit it 
> > $ make modules
> > $ make modules_install
> > 
> > ?
> > 
> > > Also put it in Kconfig file, so it is easy for user to see they are
> > > using experimental option. Agree?
> > 
> > Not really. I believe someone who download a tarball, unpack it and copy
> > files to /lib/firmware is pretty much aware what he/she is doing.
> > 
> Agree whoever download the tarball know what he/she is doing, but I am
> not fully agree the driver always use experimental uCode if it is in the
> firmware directory

I do not understand what for you need that, but ok, I can live with it.

John, please skip this patch.

Stanislaw

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[Rafał Miłecki]

2011/1/31 Stanislaw Gruszka <sgruszka@redhat.com>:
> On Fri, Jan 28, 2011 at 03:20:21PM -0800, wwguy wrote:
>> > What is easiest and less time consuming:
>> >
>> > this:
>> >
>> > $ mv /lib/firmware/iwlwifi-5000-exp.ucode /lib/firmware/iwlwifi-5000-exp.ucode.not_use
>> >
>> > or this:
>> >
>> > $ make menuconfig
>> > navigate to find option and edit it
>> > $ make modules
>> > $ make modules_install
>> >
>> > ?
>> >
>> > > Also put it in Kconfig file, so it is easy for user to see they are
>> > > using experimental option. Agree?
>> >
>> > Not really. I believe someone who download a tarball, unpack it and copy
>> > files to /lib/firmware is pretty much aware what he/she is doing.
>> >
>> Agree whoever download the tarball know what he/she is doing, but I am
>> not fully agree the driver always use experimental uCode if it is in the
>> firmware directory
>
> I do not understand what for you need that, but ok, I can live with it.
>
> John, please skip this patch.

Maybe make it module parameter? Like use_experimental_fw, default to 0?

-- 
Rafał

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[Stanislaw Gruszka]

On Mon, Jan 31, 2011 at 11:23:33AM +0100, Rafał Miłecki wrote:
> >> > Not really. I believe someone who download a tarball, unpack it and copy
> >> > files to /lib/firmware is pretty much aware what he/she is doing.
> >> >
> >> Agree whoever download the tarball know what he/she is doing, but I am
> >> not fully agree the driver always use experimental uCode if it is in the
> >> firmware directory
> >
> > I do not understand what for you need that, but ok, I can live with it.
> >
> > John, please skip this patch.
> 
> Maybe make it module parameter? Like use_experimental_fw, default to 0?

For me that even worse. Now I can provide users with kernel binaries with
enabled experimental fw support. Telling them to install firmware is
enough, to get test results.

With module option:

sgruszka: download exp firmware and load module with use_experimental_fw=1
user: ok, but how to do this 
sgruszka: echo "options iwlagn use_experiental_fw=1" >> /etc/modprobe.d/iwlwifi.conf
sgruszka: and restart the system
user: hmm, I have error
user: bash: /etc/modprobe.d/iwlwifi.conf: Permission denied 
sgruszka: login as root
user: ok I did, but new firmware did not help me
sgruszka: show me dmesg please
user: dmesg
sgruszka: old firmware is used
sgruszka: gash, I made a typo, it should be
sgruszka: echo "options iwlagn use_experimental_fw=1" >> /etc/modprobe.d/iwlwifi.conf
sgruszka: grrrrrrr

Stanislaw

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[wwguy]

On Mon, 2011-01-31 at 03:29 -0800, Stanislaw Gruszka wrote:
> On Mon, Jan 31, 2011 at 11:23:33AM +0100, Rafał Miłecki wrote:
> > >> > Not really. I believe someone who download a tarball, unpack it and copy
> > >> > files to /lib/firmware is pretty much aware what he/she is doing.
> > >> >
> > >> Agree whoever download the tarball know what he/she is doing, but I am
> > >> not fully agree the driver always use experimental uCode if it is in the
> > >> firmware directory
> > >
> > > I do not understand what for you need that, but ok, I can live with it.
> > >
> > > John, please skip this patch.
> > 
> > Maybe make it module parameter? Like use_experimental_fw, default to 0?
> 
> For me that even worse. Now I can provide users with kernel binaries with
> enabled experimental fw support. Telling them to install firmware is
> enough, to get test results.
> 
> With module option:
> 
> sgruszka: download exp firmware and load module with use_experimental_fw=1
> user: ok, but how to do this 
> sgruszka: echo "options iwlagn use_experiental_fw=1" >> /etc/modprobe.d/iwlwifi.conf
> sgruszka: and restart the system
> user: hmm, I have error
> user: bash: /etc/modprobe.d/iwlwifi.conf: Permission denied 
> sgruszka: login as root
> user: ok I did, but new firmware did not help me
> sgruszka: show me dmesg please
> user: dmesg
> sgruszka: old firmware is used
> sgruszka: gash, I made a typo, it should be
> sgruszka: echo "options iwlagn use_experimental_fw=1" >> /etc/modprobe.d/iwlwifi.conf
> sgruszka: grrrrrrr
> 
there is a "ucode_alternative" module parameter already there which
doing the similar thing

I agree there will be mistake, but whoever want to use the experimental
uCode should know what they are doing, otherwise, regular uCode
works.right?

Wey

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[Stanislaw Gruszka]

On Mon, Jan 31, 2011 at 07:01:41AM -0800, wwguy wrote:
> > With module option:
> > 
> > sgruszka: download exp firmware and load module with use_experimental_fw=1
> > user: ok, but how to do this 
> > sgruszka: echo "options iwlagn use_experiental_fw=1" >> /etc/modprobe.d/iwlwifi.conf
> > sgruszka: and restart the system
> > user: hmm, I have error
> > user: bash: /etc/modprobe.d/iwlwifi.conf: Permission denied 
> > sgruszka: login as root
> > user: ok I did, but new firmware did not help me
> > sgruszka: show me dmesg please
> > user: dmesg
> > sgruszka: old firmware is used
> > sgruszka: gash, I made a typo, it should be
> > sgruszka: echo "options iwlagn use_experimental_fw=1" >> /etc/modprobe.d/iwlwifi.conf
> > sgruszka: grrrrrrr
> > 
> there is a "ucode_alternative" module parameter already there which
> doing the similar thing
> 
> I agree there will be mistake, but whoever want to use the experimental
> uCode should know what they are doing, otherwise, regular uCode
> works.right?

If you mean that only experienced linux users should be able to use
experimental firmware, I do not share that opinion. I dislike making
things not necessary complex and supporting driver harder, but I'll
not argue about that.

Stanislaw

Re: [PATCH 7/8] iwlagn: remove CONFIG_IWLWIFI_DEBUG_EXPERIMENTAL_UCODE

[wwguy]

Stanislaw,

On Mon, 2011-01-31 at 02:23 -0800, Rafał Miłecki wrote:
> 2011/1/31 Stanislaw Gruszka <sgruszka@redhat.com>:
> > On Fri, Jan 28, 2011 at 03:20:21PM -0800, wwguy wrote:
> >> > What is easiest and less time consuming:
> >> >
> >> > this:
> >> >
> >> > $ mv /lib/firmware/iwlwifi-5000-exp.ucode /lib/firmware/iwlwifi-5000-exp.ucode.not_use
> >> >
> >> > or this:
> >> >
> >> > $ make menuconfig
> >> > navigate to find option and edit it
> >> > $ make modules
> >> > $ make modules_install
> >> >
> >> > ?
> >> >
> >> > > Also put it in Kconfig file, so it is easy for user to see they are
> >> > > using experimental option. Agree?
> >> >
> >> > Not really. I believe someone who download a tarball, unpack it and copy
> >> > files to /lib/firmware is pretty much aware what he/she is doing.
> >> >
> >> Agree whoever download the tarball know what he/she is doing, but I am
> >> not fully agree the driver always use experimental uCode if it is in the
> >> firmware directory
> >
> > I do not understand what for you need that, but ok, I can live with it.
> >
> > John, please skip this patch.
> 
> Maybe make it module parameter? Like use_experimental_fw, default to 0?
> 
I think module parameter is better idea, if you agree, and I will ack

Thanks
Wey

[PATCH 8/8] iwlwifi: introduce iwl_bt_statistics

[Stanislaw Gruszka]

We use priv->cfg->bt_params && priv->cfg->bt_params->bt_statistics
conditional in few places, merge it into one function.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn-calib.c   |    9 +++------
 drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c |   12 ++++--------
 drivers/net/wireless/iwlwifi/iwl-agn-rx.c      |   15 +++++----------
 drivers/net/wireless/iwlwifi/iwl-agn.c         |    3 +--
 drivers/net/wireless/iwlwifi/iwl-core.h        |    5 +++++
 drivers/net/wireless/iwlwifi/iwl-debugfs.c     |    2 +-
 6 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-calib.c b/drivers/net/wireless/iwlwifi/iwl-agn-calib.c
index d16bb5e..9006293 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-calib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-calib.c
@@ -631,8 +631,7 @@ void iwl_sensitivity_calibration(struct iwl_priv *priv, void *resp)
 	}
 
 	spin_lock_irqsave(&priv->lock, flags);
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		rx_info = &(((struct iwl_bt_notif_statistics *)resp)->
 			      rx.general.common);
 		ofdm = &(((struct iwl_bt_notif_statistics *)resp)->rx.ofdm);
@@ -897,8 +896,7 @@ void iwl_chain_noise_calibration(struct iwl_priv *priv, void *stat_resp)
 	}
 
 	spin_lock_irqsave(&priv->lock, flags);
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		rx_info = &(((struct iwl_bt_notif_statistics *)stat_resp)->
 			      rx.general.common);
 	} else {
@@ -913,8 +911,7 @@ void iwl_chain_noise_calibration(struct iwl_priv *priv, void *stat_resp)
 
 	rxon_band24 = !!(ctx->staging.flags & RXON_FLG_BAND_24G_MSK);
 	rxon_chnum = le16_to_cpu(ctx->staging.channel);
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		stat_band24 = !!(((struct iwl_bt_notif_statistics *)
 				 stat_resp)->flag &
 				 STATISTICS_REPLY_FLG_BAND_24G_MSK);
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c b/drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c
index a6dbd89..b500aaa 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c
@@ -39,8 +39,7 @@ static int iwl_statistics_flag(struct iwl_priv *priv, char *buf, int bufsz)
 	int p = 0;
 	u32 flag;
 
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics)
+	if (iwl_bt_statistics(priv))
 		flag = le32_to_cpu(priv->_agn.statistics_bt.flag);
 	else
 		flag = le32_to_cpu(priv->_agn.statistics.flag);
@@ -89,8 +88,7 @@ ssize_t iwl_ucode_rx_stats_read(struct file *file, char __user *user_buf,
 	 * the last statistics notification from uCode
 	 * might not reflect the current uCode activity
 	 */
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		ofdm = &priv->_agn.statistics_bt.rx.ofdm;
 		cck = &priv->_agn.statistics_bt.rx.cck;
 		general = &priv->_agn.statistics_bt.rx.general.common;
@@ -536,8 +534,7 @@ ssize_t iwl_ucode_tx_stats_read(struct file *file,
 	  * the last statistics notification from uCode
 	  * might not reflect the current uCode activity
 	  */
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		tx = &priv->_agn.statistics_bt.tx;
 		accum_tx = &priv->_agn.accum_statistics_bt.tx;
 		delta_tx = &priv->_agn.delta_statistics_bt.tx;
@@ -737,8 +734,7 @@ ssize_t iwl_ucode_general_stats_read(struct file *file, char __user *user_buf,
 	  * the last statistics notification from uCode
 	  * might not reflect the current uCode activity
 	  */
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		general = &priv->_agn.statistics_bt.general.common;
 		dbg = &priv->_agn.statistics_bt.general.common.dbg;
 		div = &priv->_agn.statistics_bt.general.common.div;
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
index bbd40b7..b192ca8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
@@ -73,8 +73,7 @@ static void iwl_rx_calc_noise(struct iwl_priv *priv)
 	int bcn_silence_a, bcn_silence_b, bcn_silence_c;
 	int last_rx_noise;
 
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics)
+	if (iwl_bt_statistics(priv))
 		rx_info = &(priv->_agn.statistics_bt.rx.general.common);
 	else
 		rx_info = &(priv->_agn.statistics.rx.general);
@@ -125,8 +124,7 @@ static void iwl_accumulative_statistics(struct iwl_priv *priv,
 	struct statistics_general_common *general, *accum_general;
 	struct statistics_tx *tx, *accum_tx;
 
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		prev_stats = (__le32 *)&priv->_agn.statistics_bt;
 		accum_stats = (u32 *)&priv->_agn.accum_statistics_bt;
 		size = sizeof(struct iwl_bt_notif_statistics);
@@ -207,8 +205,7 @@ bool iwl_good_plcp_health(struct iwl_priv *priv,
 		struct statistics_rx_phy *ofdm;
 		struct statistics_rx_ht_phy *ofdm_ht;
 
-		if (priv->cfg->bt_params &&
-		    priv->cfg->bt_params->bt_statistics) {
+		if (iwl_bt_statistics(priv)) {
 			ofdm = &pkt->u.stats_bt.rx.ofdm;
 			ofdm_ht = &pkt->u.stats_bt.rx.ofdm_ht;
 			combined_plcp_delta =
@@ -265,8 +262,7 @@ void iwl_rx_statistics(struct iwl_priv *priv,
 	int change;
 	struct iwl_rx_packet *pkt = rxb_addr(rxb);
 
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics) {
+	if (iwl_bt_statistics(priv)) {
 		IWL_DEBUG_RX(priv,
 			     "Statistics notification received (%d vs %d).\n",
 			     (int)sizeof(struct iwl_bt_notif_statistics),
@@ -304,8 +300,7 @@ void iwl_rx_statistics(struct iwl_priv *priv,
 
 	iwl_recover_from_statistics(priv, pkt);
 
-	if (priv->cfg->bt_params &&
-	    priv->cfg->bt_params->bt_statistics)
+	if (iwl_bt_statistics(priv))
 		memcpy(&priv->_agn.statistics_bt, &pkt->u.stats_bt,
 			sizeof(priv->_agn.statistics_bt));
 	else
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 23bf006..9b5c3f9 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -3075,8 +3075,7 @@ static void iwl_bg_run_time_calib_work(struct work_struct *work)
 	}
 
 	if (priv->start_calib) {
-		if (priv->cfg->bt_params &&
-		    priv->cfg->bt_params->bt_statistics) {
+		if (iwl_bt_statistics(priv)) {
 			iwl_chain_noise_calibration(priv,
 					(void *)&priv->_agn.statistics_bt);
 			iwl_sensitivity_calibration(priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index 705711a..c83fcc6 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -747,6 +747,11 @@ static inline bool iwl_advanced_bt_coexist(struct iwl_priv *priv)
 	       priv->cfg->bt_params->advanced_bt_coexist;
 }
 
+static inline bool iwl_bt_statistics(struct iwl_priv *priv)
+{
+	return priv->cfg->bt_params && priv->cfg->bt_params->bt_statistics;
+}
+
 extern bool bt_coex_active;
 extern bool bt_siso_mode;
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-debugfs.c b/drivers/net/wireless/iwlwifi/iwl-debugfs.c
index bde16ac..bdcb742 100644
--- a/drivers/net/wireless/iwlwifi/iwl-debugfs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-debugfs.c
@@ -1765,7 +1765,7 @@ int iwl_dbgfs_register(struct iwl_priv *priv, const char *name)
 		DEBUGFS_ADD_FILE(chain_noise, dir_debug, S_IRUSR);
 	if (priv->cfg->base_params->ucode_tracing)
 		DEBUGFS_ADD_FILE(ucode_tracing, dir_debug, S_IWUSR | S_IRUSR);
-	if (priv->cfg->bt_params && priv->cfg->bt_params->bt_statistics)
+	if (iwl_bt_statistics(priv))
 		DEBUGFS_ADD_FILE(ucode_bt_stats, dir_debug, S_IRUSR);
 	DEBUGFS_ADD_FILE(reply_tx_error, dir_debug, S_IRUSR);
 	DEBUGFS_ADD_FILE(rxon_flags, dir_debug, S_IWUSR);
-- 
1.7.1

Re: [PATCH 8/8] iwlwifi: introduce iwl_bt_statistics

[wwguy]

Stanislaw,

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> We use priv->cfg->bt_params && priv->cfg->bt_params->bt_statistics
> conditional in few places, merge it into one function.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---

Wey

Re: [PATCH 1/8] iwlwifi: do not set tx power when channel is changing

[wwguy]

Hi Stanislaw,

On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> Mac80211 can request for tx power and channel change in one ->config
> call. If that happens, *_send_tx_power functions will try to setup tx
> power for old channel, what can be not correct because we already change
> the band. I.e  error  "Failed to get channel info for channel 140 [0]",
> can be printed frequently when operating in software scanning mode.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945.c     |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-4965.c     |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-agn-rxon.c |    5 ++---
>  drivers/net/wireless/iwlwifi/iwl-core.c     |   13 ++++++++++---
>  4 files changed, 14 insertions(+), 8 deletions(-)
> 

Do you have a bugzilla bug being addressed by this patch?

Wey

Re: [PATCH 1/8] iwlwifi: do not set tx power when channel is changing

[Stanislaw Gruszka]

Hi

On Fri, Jan 28, 2011 at 08:03:15AM -0800, wwguy wrote:
> On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> > Mac80211 can request for tx power and channel change in one ->config
> > call. If that happens, *_send_tx_power functions will try to setup tx
> > power for old channel, what can be not correct because we already change
> > the band. I.e  error  "Failed to get channel info for channel 140 [0]",
> > can be printed frequently when operating in software scanning mode.
> > 
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> >  drivers/net/wireless/iwlwifi/iwl-3945.c     |    2 +-
> >  drivers/net/wireless/iwlwifi/iwl-4965.c     |    2 +-
> >  drivers/net/wireless/iwlwifi/iwl-agn-rxon.c |    5 ++---
> >  drivers/net/wireless/iwlwifi/iwl-core.c     |   13 ++++++++++---
> >  4 files changed, 14 insertions(+), 8 deletions(-)
> > 
> 
> Do you have a bugzilla bug being addressed by this patch?

No, I found this issue by myself testing, I don't know if
patch address any bugzilla report.

Stanislaw

Re: [PATCH 1/8] iwlwifi: do not set tx power when channel is changing

[wwguy]

On Fri, 2011-01-28 at 08:27 -0800, Stanislaw Gruszka wrote:
> Hi
> 
> On Fri, Jan 28, 2011 at 08:03:15AM -0800, wwguy wrote:
> > On Fri, 2011-01-28 at 07:47 -0800, Stanislaw Gruszka wrote:
> > > Mac80211 can request for tx power and channel change in one ->config
> > > call. If that happens, *_send_tx_power functions will try to setup tx
> > > power for old channel, what can be not correct because we already change
> > > the band. I.e  error  "Failed to get channel info for channel 140 [0]",
> > > can be printed frequently when operating in software scanning mode.
> > > 
> > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > > ---
> > 

Wey