All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics
@ 2011-02-08  8:31 Stanislaw Gruszka
  2011-02-08  8:31 ` [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health Stanislaw Gruszka
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-08  8:31 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

No functional change, make recover from statistics code
easies to read.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-rx.c |   37 ++++++++++----------------------
 1 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
index 87a6fd8..bc89393 100644
--- a/drivers/net/wireless/iwlwifi/iwl-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
@@ -234,33 +234,20 @@ EXPORT_SYMBOL(iwl_rx_spectrum_measure_notif);
 void iwl_recover_from_statistics(struct iwl_priv *priv,
 				struct iwl_rx_packet *pkt)
 {
-	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
+	if (test_bit(STATUS_EXIT_PENDING, &priv->status) ||
+	    !iwl_is_any_associated(priv))
 		return;
-	if (iwl_is_any_associated(priv)) {
-		if (priv->cfg->ops->lib->check_ack_health) {
-			if (!priv->cfg->ops->lib->check_ack_health(
-			    priv, pkt)) {
-				/*
-				 * low ack count detected
-				 * restart Firmware
-				 */
-				IWL_ERR(priv, "low ack count detected, "
-					"restart firmware\n");
-				if (!iwl_force_reset(priv, IWL_FW_RESET, false))
-					return;
-			}
-		}
-		if (priv->cfg->ops->lib->check_plcp_health) {
-			if (!priv->cfg->ops->lib->check_plcp_health(
-			    priv, pkt)) {
-				/*
-				 * high plcp error detected
-				 * reset Radio
-				 */
-				iwl_force_reset(priv, IWL_RF_RESET, false);
-			}
-		}
+
+	if (priv->cfg->ops->lib->check_ack_health &&
+	    !priv->cfg->ops->lib->check_ack_health(priv, pkt)) {
+		IWL_ERR(priv, "low ack count detected, restart firmware\n");
+		if (!iwl_force_reset(priv, IWL_FW_RESET, false))
+			return;
 	}
+
+	if (priv->cfg->ops->lib->check_plcp_health &&
+	    !priv->cfg->ops->lib->check_plcp_health(priv, pkt))
+		iwl_force_reset(priv, IWL_RF_RESET, false);
 }
 EXPORT_SYMBOL(iwl_recover_from_statistics);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health
  2011-02-08  8:31 [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics Stanislaw Gruszka
@ 2011-02-08  8:31 ` Stanislaw Gruszka
  2011-02-08 15:24   ` wwguy
  2011-02-08  8:31 ` [PATCH 3/5] iwlwifi: cleanup iwl_good_ack_health Stanislaw Gruszka
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-08  8:31 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

Make plcp health code human readable.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-3945.c     |   83 +++++++-----------
 drivers/net/wireless/iwlwifi/iwl-agn-rx.c   |  122 +++++++++++----------------
 drivers/net/wireless/iwlwifi/iwl-commands.h |    1 +
 3 files changed, 80 insertions(+), 126 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
index 58213e7..f371d42 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
@@ -411,61 +411,40 @@ static void iwl3945_accumulative_statistics(struct iwl_priv *priv,
 static bool iwl3945_good_plcp_health(struct iwl_priv *priv,
 				struct iwl_rx_packet *pkt)
 {
-	bool rc = true;
-	struct iwl3945_notif_statistics current_stat;
-	int combined_plcp_delta;
-	unsigned int plcp_msec;
-	unsigned long plcp_received_jiffies;
-
-	if (priv->cfg->base_params->plcp_delta_threshold ==
-	    IWL_MAX_PLCP_ERR_THRESHOLD_DISABLE) {
+	__le32 cur_plcp_err, old_plcp_err;
+	unsigned int msecs;
+	unsigned long stamp;
+	int delta;
+	int threshold = priv->cfg->base_params->plcp_delta_threshold;
+
+	if (threshold == IWL_MAX_PLCP_ERR_THRESHOLD_DISABLE) {
 		IWL_DEBUG_RADIO(priv, "plcp_err check disabled\n");
-		return rc;
+		return true;
 	}
-	memcpy(&current_stat, pkt->u.raw, sizeof(struct
-			iwl3945_notif_statistics));
-	/*
-	 * check for plcp_err and trigger radio reset if it exceeds
-	 * the plcp error threshold plcp_delta.
-	 */
-	plcp_received_jiffies = jiffies;
-	plcp_msec = jiffies_to_msecs((long) plcp_received_jiffies -
-					(long) priv->plcp_jiffies);
-	priv->plcp_jiffies = plcp_received_jiffies;
-	/*
-	 * check to make sure plcp_msec is not 0 to prevent division
-	 * by zero.
-	 */
-	if (plcp_msec) {
-		combined_plcp_delta =
-			(le32_to_cpu(current_stat.rx.ofdm.plcp_err) -
-			le32_to_cpu(priv->_3945.statistics.rx.ofdm.plcp_err));
-
-		if ((combined_plcp_delta > 0) &&
-			((combined_plcp_delta * 100) / plcp_msec) >
-			priv->cfg->base_params->plcp_delta_threshold) {
-			/*
-			 * if plcp_err exceed the threshold, the following
-			 * data is printed in csv format:
-			 *    Text: plcp_err exceeded %d,
-			 *    Received ofdm.plcp_err,
-			 *    Current ofdm.plcp_err,
-			 *    combined_plcp_delta,
-			 *    plcp_msec
-			 */
-			IWL_DEBUG_RADIO(priv, "plcp_err exceeded %u, "
-				"%u, %d, %u mSecs\n",
-				priv->cfg->base_params->plcp_delta_threshold,
-				le32_to_cpu(current_stat.rx.ofdm.plcp_err),
-				combined_plcp_delta, plcp_msec);
-			/*
-			 * Reset the RF radio due to the high plcp
-			 * error rate
-			 */
-			rc = false;
-		}
+
+	stamp = jiffies;
+	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
+	priv->plcp_jiffies = stamp;
+
+	if (msecs == 0)
+		return true;
+
+	old_plcp_err = priv->_3945.statistics.rx.ofdm.plcp_err;
+	cur_plcp_err = pkt->u.stats_39.rx.ofdm.plcp_err;
+
+	delta = le32_to_cpu(cur_plcp_err) - le32_to_cpu(old_plcp_err);
+
+	/* Can be negative if firmware reset statistics */
+	if (delta <= 0)
+		return true;
+
+	if ((delta * 100 / msecs) > threshold) {
+		IWL_DEBUG_RADIO(priv, "plcp health threshold %u delta %d msecs %u\n",
+				threshold, delta, msecs);
+		return false;
 	}
-	return rc;
+
+	return true;
 }
 
 void iwl3945_hw_rx_statistics(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
index b192ca8..5539fcb 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
@@ -172,88 +172,62 @@ static void iwl_accumulative_statistics(struct iwl_priv *priv,
 /**
  * iwl_good_plcp_health - checks for plcp error.
  *
- * When the plcp error is exceeding the thresholds, reset the radio
- * to improve the throughput.
+ * When numbers of the plcp errors is exceeding the thresholds,
+ * reset the radio to improve the throughput.
  */
-bool iwl_good_plcp_health(struct iwl_priv *priv,
-				struct iwl_rx_packet *pkt)
+bool iwl_good_plcp_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
 {
-	bool rc = true;
-	int combined_plcp_delta;
-	unsigned int plcp_msec;
-	unsigned long plcp_received_jiffies;
+	unsigned int msecs;
+	unsigned long stamp;
+	int delta;
+	int threshold = priv->cfg->base_params->plcp_delta_threshold;
 
-	if (priv->cfg->base_params->plcp_delta_threshold ==
-	    IWL_MAX_PLCP_ERR_THRESHOLD_DISABLE) {
+	if (threshold == IWL_MAX_PLCP_ERR_THRESHOLD_DISABLE) {
 		IWL_DEBUG_RADIO(priv, "plcp_err check disabled\n");
-		return rc;
+		return true;
 	}
 
-	/*
-	 * check for plcp_err and trigger radio reset if it exceeds
-	 * the plcp error threshold plcp_delta.
-	 */
-	plcp_received_jiffies = jiffies;
-	plcp_msec = jiffies_to_msecs((long) plcp_received_jiffies -
-					(long) priv->plcp_jiffies);
-	priv->plcp_jiffies = plcp_received_jiffies;
-	/*
-	 * check to make sure plcp_msec is not 0 to prevent division
-	 * by zero.
-	 */
-	if (plcp_msec) {
-		struct statistics_rx_phy *ofdm;
-		struct statistics_rx_ht_phy *ofdm_ht;
-
-		if (iwl_bt_statistics(priv)) {
-			ofdm = &pkt->u.stats_bt.rx.ofdm;
-			ofdm_ht = &pkt->u.stats_bt.rx.ofdm_ht;
-			combined_plcp_delta =
-			   (le32_to_cpu(ofdm->plcp_err) -
-			   le32_to_cpu(priv->_agn.statistics_bt.
-				       rx.ofdm.plcp_err)) +
-			   (le32_to_cpu(ofdm_ht->plcp_err) -
-			   le32_to_cpu(priv->_agn.statistics_bt.
-				       rx.ofdm_ht.plcp_err));
-		} else {
-			ofdm = &pkt->u.stats.rx.ofdm;
-			ofdm_ht = &pkt->u.stats.rx.ofdm_ht;
-			combined_plcp_delta =
-			    (le32_to_cpu(ofdm->plcp_err) -
-			    le32_to_cpu(priv->_agn.statistics.
-					rx.ofdm.plcp_err)) +
-			    (le32_to_cpu(ofdm_ht->plcp_err) -
-			    le32_to_cpu(priv->_agn.statistics.
-					rx.ofdm_ht.plcp_err));
-		}
+	stamp = jiffies;
+	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
+	priv->plcp_jiffies = stamp;
 
-		if ((combined_plcp_delta > 0) &&
-		    ((combined_plcp_delta * 100) / plcp_msec) >
-			priv->cfg->base_params->plcp_delta_threshold) {
-			/*
-			 * if plcp_err exceed the threshold,
-			 * the following data is printed in csv format:
-			 *    Text: plcp_err exceeded %d,
-			 *    Received ofdm.plcp_err,
-			 *    Current ofdm.plcp_err,
-			 *    Received ofdm_ht.plcp_err,
-			 *    Current ofdm_ht.plcp_err,
-			 *    combined_plcp_delta,
-			 *    plcp_msec
-			 */
-			IWL_DEBUG_RADIO(priv, "plcp_err exceeded %u, "
-				"%u, %u, %u, %u, %d, %u mSecs\n",
-				priv->cfg->base_params->plcp_delta_threshold,
-				le32_to_cpu(ofdm->plcp_err),
-				le32_to_cpu(ofdm->plcp_err),
-				le32_to_cpu(ofdm_ht->plcp_err),
-				le32_to_cpu(ofdm_ht->plcp_err),
-				combined_plcp_delta, plcp_msec);
-
-			rc = false;
-		}
+	if (msecs == 0)
+		return true;
+
+	if (iwl_bt_statistics(priv)) {
+		struct statistics_rx_bt *cur, *old;
+
+		cur = &pkt->u.stats_bt.rx;
+		old = &priv->_agn.statistics_bt.rx;
+
+		delta = le32_to_cpu(cur->ofdm.plcp_err) -
+			le32_to_cpu(old->ofdm.plcp_err) +
+			le32_to_cpu(cur->ofdm_ht.plcp_err) -
+			le32_to_cpu(old->ofdm_ht.plcp_err);
+	} else {
+		struct statistics_rx *cur, *old;
+
+		cur = &pkt->u.stats.rx;
+		old = &priv->_agn.statistics.rx;
+
+		delta = le32_to_cpu(cur->ofdm.plcp_err) -
+			le32_to_cpu(old->ofdm.plcp_err) +
+			le32_to_cpu(cur->ofdm_ht.plcp_err) -
+			le32_to_cpu(old->ofdm_ht.plcp_err);
 	}
-	return rc;
+
+	/* Can be negative if firmware reset statistics */
+	if (delta <= 0)
+		return true;
+
+	if ((delta * 100 / msecs) > threshold) {
+		IWL_DEBUG_RADIO(priv, "plcp health threshold %u delta %d msecs %u\n",
+				threshold, delta, msecs);
+
+		return false;
+	}
+
+	return true;
 }
 
 void iwl_rx_statistics(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-commands.h b/drivers/net/wireless/iwlwifi/iwl-commands.h
index 0a1d4ae..7d6192d 100644
--- a/drivers/net/wireless/iwlwifi/iwl-commands.h
+++ b/drivers/net/wireless/iwlwifi/iwl-commands.h
@@ -4348,6 +4348,7 @@ struct iwl_rx_packet {
 		struct iwl3945_rx_frame rx_frame;
 		struct iwl3945_tx_resp tx_resp;
 		struct iwl3945_beacon_notif beacon_status;
+		struct iwl3945_notif_statistics stats_39;
 
 		struct iwl_alive_resp alive_frame;
 		struct iwl_spectrum_notification spectrum_notif;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] iwlwifi: cleanup iwl_good_ack_health
  2011-02-08  8:31 [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics Stanislaw Gruszka
  2011-02-08  8:31 ` [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health Stanislaw Gruszka
@ 2011-02-08  8:31 ` Stanislaw Gruszka
  2011-02-08 15:32   ` wwguy
  2011-02-08  8:31 ` [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices Stanislaw Gruszka
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-08  8:31 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

Make ack health code easies to read. Compared to previous
code, we do not print debug messages when expected_ack_cnt_delta == 0
and also do check against negative deltas.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn.c |   69 ++++++++++++++++----------------
 1 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index a5daf64..8c4ca39 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -1407,34 +1407,37 @@ static void iwl_irq_tasklet(struct iwl_priv *priv)
 /**
  * iwl_good_ack_health - checks for ACK count ratios, BA timeout retries.
  *
- * When the ACK count ratio is 0 and aggregated BA timeout retries exceeding
+ * When the ACK count ratio is low and aggregated BA timeout retries exceeding
  * the BA_TIMEOUT_MAX, reload firmware and bring system back to normal
  * operation state.
  */
-bool iwl_good_ack_health(struct iwl_priv *priv,
-				struct iwl_rx_packet *pkt)
+bool iwl_good_ack_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
 {
-	bool rc = true;
-	int actual_ack_cnt_delta, expected_ack_cnt_delta;
-	int ba_timeout_delta;
-
-	actual_ack_cnt_delta =
-		le32_to_cpu(pkt->u.stats.tx.actual_ack_cnt) -
-		le32_to_cpu(priv->_agn.statistics.tx.actual_ack_cnt);
-	expected_ack_cnt_delta =
-		le32_to_cpu(pkt->u.stats.tx.expected_ack_cnt) -
-		le32_to_cpu(priv->_agn.statistics.tx.expected_ack_cnt);
-	ba_timeout_delta =
-		le32_to_cpu(pkt->u.stats.tx.agg.ba_timeout) -
-		le32_to_cpu(priv->_agn.statistics.tx.agg.ba_timeout);
-	if ((priv->_agn.agg_tids_count > 0) &&
-	    (expected_ack_cnt_delta > 0) &&
-	    (((actual_ack_cnt_delta * 100) / expected_ack_cnt_delta)
-		< ACK_CNT_RATIO) &&
-	    (ba_timeout_delta > BA_TIMEOUT_CNT)) {
-		IWL_DEBUG_RADIO(priv, "actual_ack_cnt delta = %d,"
-				" expected_ack_cnt = %d\n",
-				actual_ack_cnt_delta, expected_ack_cnt_delta);
+	int actual_delta, expected_delta, ba_timeout_delta;
+	struct statistics_tx *cur, *old;
+
+	if (priv->_agn.agg_tids_count)
+		return true;
+
+	cur = &pkt->u.stats.tx;
+	old = &priv->_agn.statistics.tx;
+
+	actual_delta = le32_to_cpu(cur->actual_ack_cnt) -
+		       le32_to_cpu(old->actual_ack_cnt);
+	expected_delta = le32_to_cpu(cur->expected_ack_cnt) -
+			 le32_to_cpu(old->expected_ack_cnt);
+
+	/* Values should not be negative, but we do not trust the firmware */
+	if (actual_delta <= 0 || expected_delta <= 0)
+		return true;
+
+	ba_timeout_delta = le32_to_cpu(cur->agg.ba_timeout) -
+			   le32_to_cpu(old->agg.ba_timeout);
+
+	if ((actual_delta * 100 / expected_delta) < ACK_CNT_RATIO &&
+	    ba_timeout_delta > BA_TIMEOUT_CNT) {
+		IWL_DEBUG_RADIO(priv, "deltas: actual %d expected %d ba_timeout %d\n",
+				actual_delta, expected_delta, ba_timeout_delta);
 
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 		/*
@@ -1442,20 +1445,18 @@ bool iwl_good_ack_health(struct iwl_priv *priv,
 		 * statistics aren't available. If DEBUGFS is set but
 		 * DEBUG is not, these will just compile out.
 		 */
-		IWL_DEBUG_RADIO(priv, "rx_detected_cnt delta = %d\n",
+		IWL_DEBUG_RADIO(priv, "rx_detected_cnt delta %d\n",
 				priv->_agn.delta_statistics.tx.rx_detected_cnt);
 		IWL_DEBUG_RADIO(priv,
-				"ack_or_ba_timeout_collision delta = %d\n",
-				priv->_agn.delta_statistics.tx.
-				ack_or_ba_timeout_collision);
+				"ack_or_ba_timeout_collision delta %d\n",
+				priv->_agn.delta_statistics.tx.ack_or_ba_timeout_collision);
 #endif
-		IWL_DEBUG_RADIO(priv, "agg ba_timeout delta = %d\n",
-				ba_timeout_delta);
-		if (!actual_ack_cnt_delta &&
-		    (ba_timeout_delta >= BA_TIMEOUT_MAX))
-			rc = false;
+
+		if (ba_timeout_delta >= BA_TIMEOUT_MAX)
+			return false;
 	}
-	return rc;
+
+	return true;
 }
 
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices
  2011-02-08  8:31 [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics Stanislaw Gruszka
  2011-02-08  8:31 ` [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health Stanislaw Gruszka
  2011-02-08  8:31 ` [PATCH 3/5] iwlwifi: cleanup iwl_good_ack_health Stanislaw Gruszka
@ 2011-02-08  8:31 ` Stanislaw Gruszka
  2011-02-08 15:36   ` wwguy
  2011-02-08  8:31 ` [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics Stanislaw Gruszka
  2011-02-08 15:16 ` [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics wwguy
  4 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-08  8:31 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

Combo devices have TX statistics on different place, because
struct statistics_rx_bt and struct statistics_rx have different
size. User proper values on combo devices instead of random data.

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

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 8c4ca39..6dd5001 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -1419,8 +1419,13 @@ bool iwl_good_ack_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
 	if (priv->_agn.agg_tids_count)
 		return true;
 
-	cur = &pkt->u.stats.tx;
-	old = &priv->_agn.statistics.tx;
+	if (iwl_bt_statistics(priv)) {
+		cur = &pkt->u.stats_bt.tx;
+		old = &priv->_agn.statistics_bt.tx;
+	} else {
+		cur = &pkt->u.stats.tx;
+		old = &priv->_agn.statistics.tx;
+	}
 
 	actual_delta = le32_to_cpu(cur->actual_ack_cnt) -
 		       le32_to_cpu(old->actual_ack_cnt);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics
  2011-02-08  8:31 [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics Stanislaw Gruszka
                   ` (2 preceding siblings ...)
  2011-02-08  8:31 ` [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices Stanislaw Gruszka
@ 2011-02-08  8:31 ` Stanislaw Gruszka
  2011-02-08 15:45   ` wwguy
  2011-02-08 15:16 ` [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics wwguy
  4 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-08  8:31 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

Usually H/W generate statistics notify once per about 100ms, but
sometimes we can receive notify in shorter time, even 2 ms.

This can be problem for plcp health and ack health checking.

I.e. with 2 plcp errors happens randomly in 2 ms duration, we
exceed plcp delta threshold equal to 100 (2*100/2).

Also checking ack's in short time, can results not necessary false
positive and firmware reset, for example when channel is noised and
we do not receive ACKs frames or when remote device does not send
ACKs at the moment.

Patch change code, to do statistic check and possible recovery only
if 99ms elapsed from last check. Forced delay should assure we have
good statistic data to estimate hardware state.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-3945.c     |   24 ++++++++------
 drivers/net/wireless/iwlwifi/iwl-agn-rx.c   |   46 +++++++++++++++++----------
 drivers/net/wireless/iwlwifi/iwl-agn.c      |    2 +
 drivers/net/wireless/iwlwifi/iwl-agn.h      |    2 +-
 drivers/net/wireless/iwlwifi/iwl-core.h     |    4 +-
 drivers/net/wireless/iwlwifi/iwl-dev.h      |    4 +-
 drivers/net/wireless/iwlwifi/iwl-rx.c       |    4 +-
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    1 +
 8 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
index f371d42..9646cbc 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
@@ -409,11 +409,9 @@ static void iwl3945_accumulative_statistics(struct iwl_priv *priv,
  * to improve the throughput.
  */
 static bool iwl3945_good_plcp_health(struct iwl_priv *priv,
-				struct iwl_rx_packet *pkt)
+				struct iwl_rx_packet *pkt, unsigned int msecs)
 {
 	__le32 cur_plcp_err, old_plcp_err;
-	unsigned int msecs;
-	unsigned long stamp;
 	int delta;
 	int threshold = priv->cfg->base_params->plcp_delta_threshold;
 
@@ -422,12 +420,7 @@ static bool iwl3945_good_plcp_health(struct iwl_priv *priv,
 		return true;
 	}
 
-	stamp = jiffies;
-	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
-	priv->plcp_jiffies = stamp;
-
-	if (msecs == 0)
-		return true;
+	BUG_ON(msecs == 0);
 
 	old_plcp_err = priv->_3945.statistics.rx.ofdm.plcp_err;
 	cur_plcp_err = pkt->u.stats_39.rx.ofdm.plcp_err;
@@ -450,6 +443,8 @@ static bool iwl3945_good_plcp_health(struct iwl_priv *priv,
 void iwl3945_hw_rx_statistics(struct iwl_priv *priv,
 		struct iwl_rx_mem_buffer *rxb)
 {
+	unsigned long stamp;
+	unsigned int msecs;
 	struct iwl_rx_packet *pkt = rxb_addr(rxb);
 
 	IWL_DEBUG_RX(priv, "Statistics notification received (%d vs %d).\n",
@@ -458,8 +453,17 @@ void iwl3945_hw_rx_statistics(struct iwl_priv *priv,
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 	iwl3945_accumulative_statistics(priv, (__le32 *)&pkt->u.raw);
 #endif
-	iwl_recover_from_statistics(priv, pkt);
 
+	stamp = jiffies;
+	msecs = jiffies_to_msecs(stamp - priv->rx_statistics_jiffies);
+
+	/* Do not check/recover when do not have enough statistics data */
+	if (msecs < 99)
+		return;
+
+	priv->rx_statistics_jiffies = stamp;
+
+	iwl_recover_from_statistics(priv, pkt, msecs);
 	memcpy(&priv->_3945.statistics, pkt->u.raw, sizeof(priv->_3945.statistics));
 }
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
index 5539fcb..245d909 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
@@ -175,10 +175,9 @@ static void iwl_accumulative_statistics(struct iwl_priv *priv,
  * When numbers of the plcp errors is exceeding the thresholds,
  * reset the radio to improve the throughput.
  */
-bool iwl_good_plcp_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
+bool iwl_good_plcp_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt,
+			  unsigned int msecs)
 {
-	unsigned int msecs;
-	unsigned long stamp;
 	int delta;
 	int threshold = priv->cfg->base_params->plcp_delta_threshold;
 
@@ -187,12 +186,7 @@ bool iwl_good_plcp_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
 		return true;
 	}
 
-	stamp = jiffies;
-	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
-	priv->plcp_jiffies = stamp;
-
-	if (msecs == 0)
-		return true;
+	BUG_ON(msecs == 0);
 
 	if (iwl_bt_statistics(priv)) {
 		struct statistics_rx_bt *cur, *old;
@@ -230,6 +224,31 @@ bool iwl_good_plcp_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
 	return true;
 }
 
+static void iwlagn_check_health(struct iwl_priv *priv,
+				struct iwl_rx_packet *pkt)
+{
+	unsigned int msecs;
+	unsigned long stamp;
+
+	stamp = jiffies;
+	msecs = jiffies_to_msecs(stamp - priv->rx_statistics_jiffies);
+
+	/* Do not check/recover when do not have enough statistics data */
+	if (msecs < 99)
+		return;
+
+	priv->rx_statistics_jiffies = stamp;
+
+	iwl_recover_from_statistics(priv, pkt, msecs);
+
+	if (iwl_bt_statistics(priv))
+		memcpy(&priv->_agn.statistics_bt, &pkt->u.stats_bt,
+		       sizeof(priv->_agn.statistics_bt));
+	else
+		memcpy(&priv->_agn.statistics, &pkt->u.stats,
+		       sizeof(priv->_agn.statistics));
+}
+
 void iwl_rx_statistics(struct iwl_priv *priv,
 			      struct iwl_rx_mem_buffer *rxb)
 {
@@ -272,14 +291,7 @@ void iwl_rx_statistics(struct iwl_priv *priv,
 
 	}
 
-	iwl_recover_from_statistics(priv, pkt);
-
-	if (iwl_bt_statistics(priv))
-		memcpy(&priv->_agn.statistics_bt, &pkt->u.stats_bt,
-			sizeof(priv->_agn.statistics_bt));
-	else
-		memcpy(&priv->_agn.statistics, &pkt->u.stats,
-			sizeof(priv->_agn.statistics));
+	iwlagn_check_health(priv, pkt);
 
 	set_bit(STATUS_STATISTICS, &priv->status);
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 6dd5001..eb639dd 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -3964,6 +3964,8 @@ static int iwl_init_drv(struct iwl_priv *priv)
 	priv->force_reset[IWL_FW_RESET].reset_duration =
 		IWL_DELAY_NEXT_FORCE_FW_RELOAD;
 
+	priv->rx_statistics_jiffies = jiffies;
+
 	/* Choose which receivers/antennas to use */
 	if (priv->cfg->ops->hcmd->set_rxon_chain)
 		priv->cfg->ops->hcmd->set_rxon_chain(priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.h b/drivers/net/wireless/iwlwifi/iwl-agn.h
index d00e1ea..490c6d9 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.h
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.h
@@ -249,7 +249,7 @@ u8 iwl_toggle_tx_ant(struct iwl_priv *priv, u8 ant_idx, u8 valid);
 void iwl_rx_missed_beacon_notif(struct iwl_priv *priv,
 				struct iwl_rx_mem_buffer *rxb);
 bool iwl_good_plcp_health(struct iwl_priv *priv,
-			  struct iwl_rx_packet *pkt);
+			  struct iwl_rx_packet *pkt, unsigned int msecs);
 void iwl_rx_statistics(struct iwl_priv *priv,
 		       struct iwl_rx_mem_buffer *rxb);
 void iwl_reply_statistics(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index e0ec170..6e14a7b 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -212,7 +212,7 @@ struct iwl_lib_ops {
 	struct iwl_temp_ops temp_ops;
 	/* check for plcp health */
 	bool (*check_plcp_health)(struct iwl_priv *priv,
-					struct iwl_rx_packet *pkt);
+				  struct iwl_rx_packet *pkt, unsigned int msec);
 	/* check for ack health */
 	bool (*check_ack_health)(struct iwl_priv *priv,
 					struct iwl_rx_packet *pkt);
@@ -518,7 +518,7 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb);
 void iwl_rx_spectrum_measure_notif(struct iwl_priv *priv,
 					  struct iwl_rx_mem_buffer *rxb);
 void iwl_recover_from_statistics(struct iwl_priv *priv,
-				struct iwl_rx_packet *pkt);
+				struct iwl_rx_packet *pkt, unsigned int msecs);
 void iwl_chswitch_done(struct iwl_priv *priv, bool is_success);
 void iwl_rx_csa(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb);
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index ecfbef4..17b3024 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -1256,8 +1256,8 @@ struct iwl_priv {
 	/* track IBSS manager (last beacon) status */
 	u32 ibss_manager;
 
-	/* storing the jiffies when the plcp error rate is received */
-	unsigned long plcp_jiffies;
+	/* jiffies when last recovery from statistics was performed */
+	unsigned long rx_statistics_jiffies;
 
 	/* force reset */
 	struct iwl_force_reset force_reset[IWL_MAX_FORCE_RESET];
diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
index bc89393..b9c2591 100644
--- a/drivers/net/wireless/iwlwifi/iwl-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
@@ -232,7 +232,7 @@ void iwl_rx_spectrum_measure_notif(struct iwl_priv *priv,
 EXPORT_SYMBOL(iwl_rx_spectrum_measure_notif);
 
 void iwl_recover_from_statistics(struct iwl_priv *priv,
-				struct iwl_rx_packet *pkt)
+				struct iwl_rx_packet *pkt, unsigned int msecs)
 {
 	if (test_bit(STATUS_EXIT_PENDING, &priv->status) ||
 	    !iwl_is_any_associated(priv))
@@ -246,7 +246,7 @@ void iwl_recover_from_statistics(struct iwl_priv *priv,
 	}
 
 	if (priv->cfg->ops->lib->check_plcp_health &&
-	    !priv->cfg->ops->lib->check_plcp_health(priv, pkt))
+	    !priv->cfg->ops->lib->check_plcp_health(priv, pkt, msecs))
 		iwl_force_reset(priv, IWL_RF_RESET, false);
 }
 EXPORT_SYMBOL(iwl_recover_from_statistics);
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 76fae81..f13cc11 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -3863,6 +3863,7 @@ static int iwl3945_init_drv(struct iwl_priv *priv)
 	priv->force_reset[IWL_FW_RESET].reset_duration =
 		IWL_DELAY_NEXT_FORCE_FW_RELOAD;
 
+	priv->rx_statistics_jiffies = jiffies;
 
 	priv->tx_power_user_lmt = IWL_DEFAULT_TX_POWER;
 	priv->tx_power_next = IWL_DEFAULT_TX_POWER;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics
  2011-02-08  8:31 [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics Stanislaw Gruszka
                   ` (3 preceding siblings ...)
  2011-02-08  8:31 ` [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics Stanislaw Gruszka
@ 2011-02-08 15:16 ` wwguy
  4 siblings, 0 replies; 16+ messages in thread
From: wwguy @ 2011-02-08 15:16 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> No functional change, make recover from statistics code
> easies to read.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-rx.c |   37 ++++++++++----------------------
>  1 files changed, 12 insertions(+), 25 deletions(-)
> 
Wey


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health
  2011-02-08  8:31 ` [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health Stanislaw Gruszka
@ 2011-02-08 15:24   ` wwguy
  2011-02-09  6:24     ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: wwguy @ 2011-02-08 15:24 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

Hi Stanislaw,

On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> Make plcp health code human readable.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945.c     |   83 +++++++-----------
>  drivers/net/wireless/iwlwifi/iwl-agn-rx.c   |  122 +++++++++++----------------
>  drivers/net/wireless/iwlwifi/iwl-commands.h |    1 +
>  3 files changed, 80 insertions(+), 126 deletions(-)
> 

Thanks for the clean up, now is much easier to read.

Now we are very close to release the split driver, here you change both
agn and 3945, not sure what is the easier approach will be? push the
patch now, or wait for the driver split finish, then push the patch
after that.

Wey



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] iwlwifi: cleanup iwl_good_ack_health
  2011-02-08  8:31 ` [PATCH 3/5] iwlwifi: cleanup iwl_good_ack_health Stanislaw Gruszka
@ 2011-02-08 15:32   ` wwguy
  0 siblings, 0 replies; 16+ messages in thread
From: wwguy @ 2011-02-08 15:32 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> Make ack health code easies to read. Compared to previous
> code, we do not print debug messages when expected_ack_cnt_delta == 0
> and also do check against negative deltas.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-agn.c |   69 ++++++++++++++++----------------
>  1 files changed, 35 insertions(+), 34 deletions(-)
> 

Thank you for cleaning up the code

Wey


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices
  2011-02-08  8:31 ` [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices Stanislaw Gruszka
@ 2011-02-08 15:36   ` wwguy
  2011-02-09  6:26     ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: wwguy @ 2011-02-08 15:36 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

Hi Stanislaw,

On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> Combo devices have TX statistics on different place, because
> struct statistics_rx_bt and struct statistics_rx have different
> size. User proper values on combo devices instead of random data.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-agn.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index 8c4ca39..6dd5001 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -1419,8 +1419,13 @@ bool iwl_good_ack_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
>  	if (priv->_agn.agg_tids_count)
>  		return true;
>  
> -	cur = &pkt->u.stats.tx;
> -	old = &priv->_agn.statistics.tx;
> +	if (iwl_bt_statistics(priv)) {
> +		cur = &pkt->u.stats_bt.tx;
> +		old = &priv->_agn.statistics_bt.tx;
> +	} else {
> +		cur = &pkt->u.stats.tx;
> +		old = &priv->_agn.statistics.tx;
> +	}
>  
>  	actual_delta = le32_to_cpu(cur->actual_ack_cnt) -
>  		       le32_to_cpu(old->actual_ack_cnt);

Should this combine with 3/5?

Thanks
Wey


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics
  2011-02-08  8:31 ` [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics Stanislaw Gruszka
@ 2011-02-08 15:45   ` wwguy
  2011-02-09  6:28     ` Stanislaw Gruszka
  0 siblings, 1 reply; 16+ messages in thread
From: wwguy @ 2011-02-08 15:45 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

Hi Stanislaw,

On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> Usually H/W generate statistics notify once per about 100ms, but
> sometimes we can receive notify in shorter time, even 2 ms.
> 
> This can be problem for plcp health and ack health checking.
> 
> I.e. with 2 plcp errors happens randomly in 2 ms duration, we
> exceed plcp delta threshold equal to 100 (2*100/2).
> 
> Also checking ack's in short time, can results not necessary false
> positive and firmware reset, for example when channel is noised and
> we do not receive ACKs frames or when remote device does not send
> ACKs at the moment.
> 
> Patch change code, to do statistic check and possible recovery only
> if 99ms elapsed from last check. Forced delay should assure we have
> good statistic data to estimate hardware state.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945.c     |   24 ++++++++------
>  drivers/net/wireless/iwlwifi/iwl-agn-rx.c   |   46 +++++++++++++++++----------
>  drivers/net/wireless/iwlwifi/iwl-agn.c      |    2 +
>  drivers/net/wireless/iwlwifi/iwl-agn.h      |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-core.h     |    4 +-
>  drivers/net/wireless/iwlwifi/iwl-dev.h      |    4 +-
>  drivers/net/wireless/iwlwifi/iwl-rx.c       |    4 +-
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    1 +
>  8 files changed, 53 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
> index f371d42..9646cbc 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
> @@ -409,11 +409,9 @@ static void iwl3945_accumulative_statistics(struct iwl_priv *priv,
>   * to improve the throughput.
>   */
>  static bool iwl3945_good_plcp_health(struct iwl_priv *priv,
> -				struct iwl_rx_packet *pkt)
> +				struct iwl_rx_packet *pkt, unsigned int msecs)
>  {
>  	__le32 cur_plcp_err, old_plcp_err;
> -	unsigned int msecs;
> -	unsigned long stamp;
>  	int delta;
>  	int threshold = priv->cfg->base_params->plcp_delta_threshold;
>  
> @@ -422,12 +420,7 @@ static bool iwl3945_good_plcp_health(struct iwl_priv *priv,
>  		return true;
>  	}
>  
> -	stamp = jiffies;
> -	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
> -	priv->plcp_jiffies = stamp;
> -
> -	if (msecs == 0)
> -		return true;
> +	BUG_ON(msecs == 0);

why not just return? I understand it should not be "0", but really need
BUG_ON?
 
>  
> -	stamp = jiffies;
> -	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
> -	priv->plcp_jiffies = stamp;
> -
> -	if (msecs == 0)
> -		return true;
> +	BUG_ON(msecs == 0);
Same above


Similar question, we are very close to finish driver split, when is the
best time for this patch?

Thanks
Wey


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health
  2011-02-08 15:24   ` wwguy
@ 2011-02-09  6:24     ` Stanislaw Gruszka
  2011-02-09 17:03       ` wwguy
  0 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-09  6:24 UTC (permalink / raw)
  To: wwguy; +Cc: Intel Linux Wireless, linux-wireless

Hi Wey

On Tue, Feb 08, 2011 at 07:24:05AM -0800, wwguy wrote:
> On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> > Make plcp health code human readable.
> > 
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> >  drivers/net/wireless/iwlwifi/iwl-3945.c     |   83 +++++++-----------
> >  drivers/net/wireless/iwlwifi/iwl-agn-rx.c   |  122 +++++++++++----------------
> >  drivers/net/wireless/iwlwifi/iwl-commands.h |    1 +
> >  3 files changed, 80 insertions(+), 126 deletions(-)
> > 
> 
> Thanks for the clean up, now is much easier to read.
> 
> Now we are very close to release the split driver, here you change both
> agn and 3945, not sure what is the easier approach will be? push the
> patch now, or wait for the driver split finish, then push the patch
> after that.

I will repost my patches on top of your changes,
hope they'll go to 2.6.39 ?

Stanislaw

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices
  2011-02-08 15:36   ` wwguy
@ 2011-02-09  6:26     ` Stanislaw Gruszka
  2011-02-09 17:04       ` wwguy
  0 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-09  6:26 UTC (permalink / raw)
  To: wwguy; +Cc: Intel Linux Wireless, linux-wireless

On Tue, Feb 08, 2011 at 07:36:10AM -0800, wwguy wrote:
> Hi Stanislaw,
> 
> On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> > Combo devices have TX statistics on different place, because
> > struct statistics_rx_bt and struct statistics_rx have different
> > size. User proper values on combo devices instead of random data.
> > 
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> >  drivers/net/wireless/iwlwifi/iwl-agn.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> > index 8c4ca39..6dd5001 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> > @@ -1419,8 +1419,13 @@ bool iwl_good_ack_health(struct iwl_priv *priv, struct iwl_rx_packet *pkt)
> >  	if (priv->_agn.agg_tids_count)
> >  		return true;
> >  
> > -	cur = &pkt->u.stats.tx;
> > -	old = &priv->_agn.statistics.tx;
> > +	if (iwl_bt_statistics(priv)) {
> > +		cur = &pkt->u.stats_bt.tx;
> > +		old = &priv->_agn.statistics_bt.tx;
> > +	} else {
> > +		cur = &pkt->u.stats.tx;
> > +		old = &priv->_agn.statistics.tx;
> > +	}
> >  
> >  	actual_delta = le32_to_cpu(cur->actual_ack_cnt) -
> >  		       le32_to_cpu(old->actual_ack_cnt);
> 
> Should this combine with 3/5?

This bug was there before my 3'rd patch, it was just hard
to see it. I would like to have fix separate for documenting 
purposes.

Stanislaw

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics
  2011-02-08 15:45   ` wwguy
@ 2011-02-09  6:28     ` Stanislaw Gruszka
  2011-02-09 17:05       ` wwguy
  0 siblings, 1 reply; 16+ messages in thread
From: Stanislaw Gruszka @ 2011-02-09  6:28 UTC (permalink / raw)
  To: wwguy; +Cc: Intel Linux Wireless, linux-wireless

On Tue, Feb 08, 2011 at 07:45:53AM -0800, wwguy wrote:
> > -	priv->plcp_jiffies = stamp;
> > -
> > -	if (msecs == 0)
> > -		return true;
> > +	BUG_ON(msecs == 0);
> 
> why not just return? I understand it should not be "0", but really need
> BUG_ON?
>  
> >  
> > -	stamp = jiffies;
> > -	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
> > -	priv->plcp_jiffies = stamp;
> > -
> > -	if (msecs == 0)
> > -		return true;
> > +	BUG_ON(msecs == 0);
> Same above
> 
> 
> Similar question, we are very close to finish driver split, when is the
> best time for this patch?

The same aswer :-) I will respin, when your patches will land in John tree.

Stanislaw

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health
  2011-02-09  6:24     ` Stanislaw Gruszka
@ 2011-02-09 17:03       ` wwguy
  0 siblings, 0 replies; 16+ messages in thread
From: wwguy @ 2011-02-09 17:03 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

On Tue, 2011-02-08 at 22:24 -0800, Stanislaw Gruszka wrote:
> Hi Wey
> 
> On Tue, Feb 08, 2011 at 07:24:05AM -0800, wwguy wrote:
> > On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> > > Make plcp health code human readable.
> > > 
> > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > > ---
> > >  drivers/net/wireless/iwlwifi/iwl-3945.c     |   83 +++++++-----------
> > >  drivers/net/wireless/iwlwifi/iwl-agn-rx.c   |  122 +++++++++++----------------
> > >  drivers/net/wireless/iwlwifi/iwl-commands.h |    1 +
> > >  3 files changed, 80 insertions(+), 126 deletions(-)
> > > 
> > 
> > Thanks for the clean up, now is much easier to read.
> > 
> > Now we are very close to release the split driver, here you change both
> > agn and 3945, not sure what is the easier approach will be? push the
> > patch now, or wait for the driver split finish, then push the patch
> > after that.
> 
> I will repost my patches on top of your changes,
> hope they'll go to 2.6.39 ?
> 
yes, we are targeting 2.6.39, but if there is any delay, I will let you
know. Don't want to hold off your patch if we delay.

We should know by the end of this week

Wey



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices
  2011-02-09  6:26     ` Stanislaw Gruszka
@ 2011-02-09 17:04       ` wwguy
  0 siblings, 0 replies; 16+ messages in thread
From: wwguy @ 2011-02-09 17:04 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

On Tue, 2011-02-08 at 22:26 -0800, Stanislaw Gruszka wrote:
> On Tue, Feb 08, 2011 at 07:36:10AM -0800, wwguy wrote:
> > Hi Stanislaw,
> > 
> > On Tue, 2011-02-08 at 00:31 -0800, Stanislaw Gruszka wrote:
> > > Combo devices have TX statistics on different place, because
> > > struct statistics_rx_bt and struct statistics_rx have different
> > > size. User proper values on combo devices instead of random data.
> > > 
> > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>


It make sense, thanks
Wey



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics
  2011-02-09  6:28     ` Stanislaw Gruszka
@ 2011-02-09 17:05       ` wwguy
  0 siblings, 0 replies; 16+ messages in thread
From: wwguy @ 2011-02-09 17:05 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless

On Tue, 2011-02-08 at 22:28 -0800, Stanislaw Gruszka wrote:
> On Tue, Feb 08, 2011 at 07:45:53AM -0800, wwguy wrote:
> > > -	priv->plcp_jiffies = stamp;
> > > -
> > > -	if (msecs == 0)
> > > -		return true;
> > > +	BUG_ON(msecs == 0);
> > 
> > why not just return? I understand it should not be "0", but really need
> > BUG_ON?
> >  
> > >  
> > > -	stamp = jiffies;
> > > -	msecs = jiffies_to_msecs(stamp - priv->plcp_jiffies);
> > > -	priv->plcp_jiffies = stamp;
> > > -
> > > -	if (msecs == 0)
> > > -		return true;
> > > +	BUG_ON(msecs == 0);
> > Same above
> > 
> > 
> > Similar question, we are very close to finish driver split, when is the
> > best time for this patch?
> 
> The same aswer :-) I will respin, when your patches will land in John tree.
> 
Thank you

Wey



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2011-02-09 17:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-08  8:31 [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics Stanislaw Gruszka
2011-02-08  8:31 ` [PATCH 2/5] iwlwifi: cleanup {iwl,iwl3945}_good_plcp_health Stanislaw Gruszka
2011-02-08 15:24   ` wwguy
2011-02-09  6:24     ` Stanislaw Gruszka
2011-02-09 17:03       ` wwguy
2011-02-08  8:31 ` [PATCH 3/5] iwlwifi: cleanup iwl_good_ack_health Stanislaw Gruszka
2011-02-08 15:32   ` wwguy
2011-02-08  8:31 ` [PATCH 4/5] iwlwifi: fix ack health for WiFi/BT combo devices Stanislaw Gruszka
2011-02-08 15:36   ` wwguy
2011-02-09  6:26     ` Stanislaw Gruszka
2011-02-09 17:04       ` wwguy
2011-02-08  8:31 ` [PATCH 5/5] iwlwifi: avoid too frequent recover from statistics Stanislaw Gruszka
2011-02-08 15:45   ` wwguy
2011-02-09  6:28     ` Stanislaw Gruszka
2011-02-09 17:05       ` wwguy
2011-02-08 15:16 ` [PATCH 1/5] iwlwifi: cleanup iwl_recover_from_statistics wwguy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.