[PATCH v2 0/5] ath5k: Various tx power fixes

[PATCH v2 0/5] ath5k: Various tx power fixes

[Nick Kossifidis]

This series fixes a few bugs on tx power processing code. Some more fixes
on EEPROM stuff are still pending.

Thanks to Thomas Huehn and Felix Fietkau for the feedback.

Nick Kossifidis (5):
  ath5k: Use correct value for min_pwr and cur_pwr
  ath5k: Fix range scaling when setting rate power table
  ath5k: Preserve tx power level requested from above on phy_init
  ath5k: Put power_level where it belongs and rename it
  ath5k: Return correct offset when reading frequencies

 drivers/net/wireless/ath/ath5k/ath5k.h        |    2 +-
 drivers/net/wireless/ath/ath5k/base.c         |    5 ++-
 drivers/net/wireless/ath/ath5k/eeprom.c       |    4 +-
 drivers/net/wireless/ath/ath5k/mac80211-ops.c |    4 +-
 drivers/net/wireless/ath/ath5k/phy.c          |   43 ++++++++++++++++++------
 5 files changed, 40 insertions(+), 18 deletions(-)

-- 
1.7.3.4

[PATCH v2 1/5] ath5k: Use correct value for min_pwr and cur_pwr

[Nick Kossifidis]

Make sure we don't store the table offsets for min and cur power levels,
store the 0.25dB values instead. This way we don't clamp the tx power level
to max (because now cur_pwr holds the 0.25dB value, not the table offset) after
re-using cur_pwr on reset.

Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>
---
 drivers/net/wireless/ath/ath5k/phy.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 8b71a2d..aa1a77d 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -3562,6 +3562,20 @@ ath5k_setup_rate_powertable(struct ath5k_hw *ah, u16 max_pwr,
 		for (i = 8; i <= 15; i++)
 			rates[i] -= ah->ah_txpower.txp_cck_ofdm_gainf_delta;
 
+	/* Save min/max and current tx power for this channel
+	 * in 0.25dB units.
+	 *
+	 * Note: We use rates[0] for current tx power because
+	 * it covers most of the rates, in most cases. It's our
+	 * tx power limit and what the user expects to see. */
+	ah->ah_txpower.txp_min_pwr = 2 * rates[7];
+	ah->ah_txpower.txp_cur_pwr = 2 * rates[0];
+
+	/* Set max txpower for correct OFDM operation on all rates
+	 * -that is the txpower for 54Mbit-, it's used for the PAPD
+	 * gain probe and it's in 0.5dB units */
+	ah->ah_txpower.txp_ofdm = rates[7];
+
 	/* Now that we have all rates setup use table offset to
 	 * match the power range set by user with the power indices
 	 * on PCDAC/PDADC table */
@@ -3571,11 +3585,6 @@ ath5k_setup_rate_powertable(struct ath5k_hw *ah, u16 max_pwr,
 		if (rates[i] > 63)
 			rates[i] = 63;
 	}
-
-	/* Min/max in 0.25dB units */
-	ah->ah_txpower.txp_min_pwr = 2 * rates[7];
-	ah->ah_txpower.txp_cur_pwr = 2 * rates[0];
-	ah->ah_txpower.txp_ofdm = rates[7];
 }
 
 
-- 
1.7.3.4

[PATCH v2 2/5] ath5k: Fix range scaling when setting rate power table

[Nick Kossifidis]

rates[i] is unsigned but txp_offset can be negative for newer parts
with PDADC table. We cover the case when rates[i] + txp_offset > 63
but we must also cover the case when its < 0 or else rates[i] will overflow.

Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>
---
 drivers/net/wireless/ath/ath5k/phy.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index aa1a77d..84a9aaf 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -3516,6 +3516,7 @@ ath5k_setup_rate_powertable(struct ath5k_hw *ah, u16 max_pwr,
 {
 	unsigned int i;
 	u16 *rates;
+	s16 rate_idx_scaled = 0;
 
 	/* max_pwr is power level we got from driver/user in 0.5dB
 	 * units, switch to 0.25dB units so we can compare */
@@ -3580,10 +3581,13 @@ ath5k_setup_rate_powertable(struct ath5k_hw *ah, u16 max_pwr,
 	 * match the power range set by user with the power indices
 	 * on PCDAC/PDADC table */
 	for (i = 0; i < 16; i++) {
-		rates[i] += ah->ah_txpower.txp_offset;
+		rate_idx_scaled = rates[i] + ah->ah_txpower.txp_offset;
 		/* Don't get out of bounds */
-		if (rates[i] > 63)
-			rates[i] = 63;
+		if (rate_idx_scaled > 63)
+			rate_idx_scaled = 63;
+		if (rate_idx_scaled < 0)
+			rate_idx_scaled = 0;
+		rates[i] = rate_idx_scaled;
 	}
 }
 
-- 
1.7.3.4

[PATCH v2 3/5] ath5k: Preserve tx power level requested from above on phy_init

[Nick Kossifidis]

By using cur_pwr on phy_init we re-use the power level previously set by the
driver, not the one we got from above.

Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>
---
 drivers/net/wireless/ath/ath5k/phy.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 84a9aaf..27ca993 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -3802,8 +3802,8 @@ ath5k_hw_phy_init(struct ath5k_hw *ah, struct ieee80211_channel *channel,
 	 * RF buffer settings on 5211/5212+ so that we
 	 * properly set curve indices.
 	 */
-	ret = ath5k_hw_txpower(ah, channel, ah->ah_txpower.txp_cur_pwr ?
-			ah->ah_txpower.txp_cur_pwr / 2 : AR5K_TUNE_MAX_TXPOWER);
+	ret = ath5k_hw_txpower(ah, channel, ah->power_level ?
+			ah->power_level * 2 : AR5K_TUNE_MAX_TXPOWER);
 	if (ret)
 		return ret;
 
-- 
1.7.3.4

[PATCH v2 4/5] ath5k: Put power_level where it belongs and rename it

[Nick Kossifidis]

Put power_level to ah_txpower struct with the rest tx power infos and
also rename it to txp_requested to make more sense.

v2 make sure we don't memset it to zero on reset

Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>
---
 drivers/net/wireless/ath/ath5k/ath5k.h        |    2 +-
 drivers/net/wireless/ath/ath5k/base.c         |    5 +++--
 drivers/net/wireless/ath/ath5k/mac80211-ops.c |    4 ++--
 drivers/net/wireless/ath/ath5k/phy.c          |   14 +++++++++++---
 4 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index 64a453a..3150def 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1331,7 +1331,6 @@ struct ath5k_hw {
 	unsigned int		nexttbtt;	/* next beacon time in TU */
 	struct ath5k_txq	*cabq;		/* content after beacon */
 
-	int			power_level;	/* Requested tx power in dBm */
 	bool			assoc;		/* associate state */
 	bool			enable_beacon;	/* true if beacons are on */
 
@@ -1425,6 +1424,7 @@ struct ath5k_hw {
 		/* Value in dB units */
 		s16		txp_cck_ofdm_pwr_delta;
 		bool		txp_setup;
+		int		txp_requested;	/* Requested tx power in dBm */
 	} ah_txpower;
 
 	struct ath5k_nfcal_hist ah_nfcal_hist;
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 8c4c040..9d48f9a 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -723,7 +723,7 @@ ath5k_txbuf_setup(struct ath5k_hw *ah, struct ath5k_buf *bf,
 	ret = ah->ah_setup_tx_desc(ah, ds, pktlen,
 		ieee80211_get_hdrlen_from_skb(skb), padsize,
 		get_hw_packet_type(skb),
-		(ah->power_level * 2),
+		(ah->ah_txpower.txp_requested * 2),
 		hw_rate,
 		info->control.rates[0].count, keyidx, ah->ah_tx_ant, flags,
 		cts_rate, duration);
@@ -1778,7 +1778,8 @@ ath5k_beacon_setup(struct ath5k_hw *ah, struct ath5k_buf *bf)
 	ds->ds_data = bf->skbaddr;
 	ret = ah->ah_setup_tx_desc(ah, ds, skb->len,
 			ieee80211_get_hdrlen_from_skb(skb), padsize,
-			AR5K_PKT_TYPE_BEACON, (ah->power_level * 2),
+			AR5K_PKT_TYPE_BEACON,
+			(ah->ah_txpower.txp_requested * 2),
 			ieee80211_get_tx_rate(ah->hw, info)->hw_value,
 			1, AR5K_TXKEYIX_INVALID,
 			antenna, flags, 0, 0);
diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
index 260e7dc..92ee3a0 100644
--- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
+++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
@@ -207,8 +207,8 @@ ath5k_config(struct ieee80211_hw *hw, u32 changed)
 	}
 
 	if ((changed & IEEE80211_CONF_CHANGE_POWER) &&
-	(ah->power_level != conf->power_level)) {
-		ah->power_level = conf->power_level;
+	(ah->ah_txpower.txp_requested != conf->power_level)) {
+		ah->ah_txpower.txp_requested = conf->power_level;
 
 		/* Half dB steps */
 		ath5k_hw_set_txpower_limit(ah, (conf->power_level * 2));
diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 27ca993..01c90ed 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -3652,10 +3652,17 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel,
 	if (!ah->ah_txpower.txp_setup ||
 	    (channel->hw_value != curr_channel->hw_value) ||
 	    (channel->center_freq != curr_channel->center_freq)) {
-		/* Reset TX power values */
+		/* Reset TX power values but preserve requested
+		 * tx power from above */
+		int requested_txpower = ah->ah_txpower.txp_requested;
+
 		memset(&ah->ah_txpower, 0, sizeof(ah->ah_txpower));
+
+		/* Restore TPC setting and requested tx power */
 		ah->ah_txpower.txp_tpc = AR5K_TUNE_TPC_TXPOWER;
 
+		ah->ah_txpower.txp_requested = requested_txpower;
+
 		/* Calculate the powertable */
 		ret = ath5k_setup_channel_powertable(ah, channel,
 							ee_mode, type);
@@ -3802,8 +3809,9 @@ ath5k_hw_phy_init(struct ath5k_hw *ah, struct ieee80211_channel *channel,
 	 * RF buffer settings on 5211/5212+ so that we
 	 * properly set curve indices.
 	 */
-	ret = ath5k_hw_txpower(ah, channel, ah->power_level ?
-			ah->power_level * 2 : AR5K_TUNE_MAX_TXPOWER);
+	ret = ath5k_hw_txpower(ah, channel, ah->ah_txpower.txp_requested ?
+					ah->ah_txpower.txp_requested * 2 :
+					AR5K_TUNE_MAX_TXPOWER);
 	if (ret)
 		return ret;
 
-- 
1.7.3.4

[PATCH v2 5/5] ath5k: Return correct offset when reading frequencies

[Nick Kossifidis]

If we have a zeroed frequency on the calibration piers it means that we
shouldn't use that pier, not stop reading the EEPROM and break out from
the loop. By doing that we return the wrong offset and the whole dataset
gets corrupted.

Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>
Tested-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de>
---
 drivers/net/wireless/ath/ath5k/eeprom.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c
index 4026c90..10a4396 100644
--- a/drivers/net/wireless/ath/ath5k/eeprom.c
+++ b/drivers/net/wireless/ath/ath5k/eeprom.c
@@ -522,7 +522,7 @@ ath5k_eeprom_read_freq_list(struct ath5k_hw *ah, int *offset, int max,
 
 		freq1 = val & 0xff;
 		if (!freq1)
-			break;
+			continue;
 
 		pc[i++].freq = ath5k_eeprom_bin2freq(ee,
 				freq1, mode);
@@ -530,7 +530,7 @@ ath5k_eeprom_read_freq_list(struct ath5k_hw *ah, int *offset, int max,
 
 		freq2 = (val >> 8) & 0xff;
 		if (!freq2)
-			break;
+			continue;
 
 		pc[i++].freq = ath5k_eeprom_bin2freq(ee,
 				freq2, mode);
-- 
1.7.3.4

Re: [ath5k-devel] [PATCH v2 5/5] ath5k: Return correct offset when reading frequencies

[Thomas Huehn]

Hi Nick, hi Felix,

This patch does break the operation of all DCMA82 (AR5413), as this
chips have only 8 valid piers, but with the "continue" all 10 got read.
So the former version was correct as it break after 8 piers.
The patch was initiated by my observation and after revisiting what I
did, blame Thomas is correct here... The power variation is properly
working and I miss measured based on an uncleaned patch mess in my env.

Please revert this patch to get proper pier readings back in ath5k.


Greetings Thomas


Nick Kossifidis schrieb:

> If we have a zeroed frequency on the calibration piers it means that we
> shouldn't use that pier, not stop reading the EEPROM and break out from
> the loop. By doing that we return the wrong offset and the whole dataset
> gets corrupted.
> 
> Signed-off-by: Nick Kossifidis <mickflemm@gmail.com>
> Tested-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de>
> ---
>  drivers/net/wireless/ath/ath5k/eeprom.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c
> index 4026c90..10a4396 100644
> --- a/drivers/net/wireless/ath/ath5k/eeprom.c
> +++ b/drivers/net/wireless/ath/ath5k/eeprom.c
> @@ -522,7 +522,7 @@ ath5k_eeprom_read_freq_list(struct ath5k_hw *ah, int *offset, int max,
>  
>  		freq1 = val & 0xff;
>  		if (!freq1)
> -			break;
> +			continue;
>  
>  		pc[i++].freq = ath5k_eeprom_bin2freq(ee,
>  				freq1, mode);
> @@ -530,7 +530,7 @@ ath5k_eeprom_read_freq_list(struct ath5k_hw *ah, int *offset, int max,
>  
>  		freq2 = (val >> 8) & 0xff;
>  		if (!freq2)
> -			break;
> +			continue;
>  
>  		pc[i++].freq = ath5k_eeprom_bin2freq(ee,
>  				freq2, mode);

Re: [ath5k-devel] [PATCH v2 5/5] ath5k: Return correct offset when reading frequencies

[Nick Kossifidis]

2012/8/8 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>:
> Hi Nick, hi Felix,
>
> This patch does break the operation of all DCMA82 (AR5413), as this
> chips have only 8 valid piers, but with the "continue" all 10 got read.
> So the former version was correct as it break after 8 piers.
> The patch was initiated by my observation and after revisiting what I
> did, blame Thomas is correct here... The power variation is properly
> working and I miss measured based on an uncleaned patch mess in my env.
>
> Please revert this patch to get proper pier readings back in ath5k.
>
>
> Greetings Thomas
>

ACK so documentation is once again misleading because it says that we
always have 10 frequencies to read and if some are zero we just ignore
the matching data. It doesn't say that if we get a zeroed frequency
the rest are missing from the EEPROM and we should move on to the next
section. I'll keep that in mind ;-)

John please ignore this one and sorry for the mess !


-- 
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick