[PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[Gabor Juhos]

The REDUCE_SCALED_POWER_BY_THREE_CHAIN symbol is
defined in different eeprom files, and the value
varies between the different files.

In eeprom_def.c and in ar9003_eeprom.c the value
of the symbol is 9, however the comments in these
files indicates the value should be 10*log10(3)*2
which is 9.54242509439325. Replace the the value
to 10 in these files.

Also add comments to eeprom_9287.c.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c |    2 +-
 drivers/net/wireless/ath/ath9k/eeprom_9287.c   |    4 ++--
 drivers/net/wireless/ath/ath9k/eeprom_def.c    |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 672b6c9..25a4a02 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -31,7 +31,7 @@
 #define CTL_11G_EXT (CTL_11G | EXT_ADDITIVE)
 #define CTL_11B_EXT (CTL_11B | EXT_ADDITIVE)
 #define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
-#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   9  /* 10*log10(3)*2 */
+#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
 #define PWRINCR_3_TO_1_CHAIN      9             /* 10*log(3)*2 */
 #define PWRINCR_3_TO_2_CHAIN      3             /* floor(10*log(3/2)*2) */
 #define PWRINCR_2_TO_1_CHAIN      6             /* 10*log(2)*2 */
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_9287.c b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
index f272236..604eab8 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
@@ -564,8 +564,8 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
 	(((cfgCtl & ~CTL_MODE_M) | (pCtlMode[ctlMode] & CTL_MODE_M)) == \
 	 ((pEepData->ctlIndex[i] & CTL_MODE_M) | SD_NO_CTL))
 
-#define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6
-#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10
+#define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
+#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
 
 	u16 twiceMaxEdgePower;
 	int i;
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_def.c b/drivers/net/wireless/ath/ath9k/eeprom_def.c
index 619b95d..0068627 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_def.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_def.c
@@ -992,7 +992,7 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
 						  u16 powerLimit)
 {
 #define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
-#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   9 /* 10*log10(3)*2 */
+#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
 
 	struct ar5416_eeprom_def *pEepData = &ah->eeprom.def;
 	u16 twiceMaxEdgePower;
-- 
1.7.2.1

[PATCH 2/3] ath9k: introduce ath9k_hw_get_scaled_power helper

[Gabor Juhos]

The computation of the scaled power value in
various eeprom files uses identical code. Move
that code into a helper function and use that
instead of code duplication.

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c |   28 +------------------
 drivers/net/wireless/ath/ath9k/eeprom.c        |   33 ++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/eeprom.h        |    5 +++
 drivers/net/wireless/ath/ath9k/eeprom_9287.c   |   30 +--------------------
 drivers/net/wireless/ath/ath9k/eeprom_def.c    |   23 +---------------
 5 files changed, 44 insertions(+), 75 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 25a4a02..d254571 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -30,8 +30,6 @@
 #define CTL_11A_EXT (CTL_11A | EXT_ADDITIVE)
 #define CTL_11G_EXT (CTL_11G | EXT_ADDITIVE)
 #define CTL_11B_EXT (CTL_11B | EXT_ADDITIVE)
-#define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
-#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
 #define PWRINCR_3_TO_1_CHAIN      9             /* 10*log(3)*2 */
 #define PWRINCR_3_TO_2_CHAIN      3             /* floor(10*log(3/2)*2) */
 #define PWRINCR_2_TO_1_CHAIN      6             /* 10*log(2)*2 */
@@ -4789,30 +4787,8 @@ static void ar9003_hw_set_power_per_rate_table(struct ath_hw *ah,
 	bool is2ghz = IS_CHAN_2GHZ(chan);
 
 	ath9k_hw_get_channel_centers(ah, chan, &centers);
-	scaledPower = powerLimit - antenna_reduction;
-
-	/*
-	 * Reduce scaled Power by number of chains active to get
-	 * to per chain tx power level
-	 */
-	switch (ar5416_get_ntxchains(ah->txchainmask)) {
-	case 1:
-		break;
-	case 2:
-		if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN)
-			scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
-		else
-			scaledPower = 0;
-		break;
-	case 3:
-		if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN)
-			scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
-		else
-			scaledPower = 0;
-		break;
-	}
-
-	scaledPower = max((u16)0, scaledPower);
+	scaledPower = ath9k_hw_get_scaled_power(ah, powerLimit,
+						antenna_reduction);
 
 	/*
 	 * Get target powers from EEPROM - our baseline for TX Power
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index c435232..61bad99 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -290,6 +290,39 @@ u16 ath9k_hw_get_max_edge_power(u16 freq, struct cal_ctl_edges *pRdEdgesPower,
 	return twiceMaxEdgePower;
 }
 
+u16 ath9k_hw_get_scaled_power(struct ath_hw *ah, u16 power_limit,
+			      u8 antenna_reduction)
+{
+	u16 scaled_power;
+
+	scaled_power = power_limit - antenna_reduction;
+
+	/*
+	 * Reduce scaled Power by number of chains active
+	 * to get the per chain tx power level.
+	 */
+	switch (ar5416_get_ntxchains(ah->txchainmask)) {
+	case 1:
+		break;
+	case 2:
+		if (scaled_power > REDUCE_SCALED_POWER_BY_TWO_CHAIN)
+			scaled_power -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
+		else
+			scaled_power = 0;
+		break;
+	case 3:
+		if (scaled_power > REDUCE_SCALED_POWER_BY_THREE_CHAIN)
+			scaled_power -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
+		else
+			scaled_power = 0;
+		break;
+	}
+
+	scaled_power = max((u16)0, scaled_power);
+
+	return scaled_power;
+}
+
 void ath9k_hw_update_regulatory_maxpower(struct ath_hw *ah)
 {
 	struct ath_common *common = ath9k_hw_common(ah);
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h b/drivers/net/wireless/ath/ath9k/eeprom.h
index 5ff7ab9..8d779b4 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.h
+++ b/drivers/net/wireless/ath/ath9k/eeprom.h
@@ -82,6 +82,9 @@
 #define INCREASE_MAXPOW_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
 #define INCREASE_MAXPOW_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
 
+#define REDUCE_SCALED_POWER_BY_TWO_CHAIN	6  /* 10*log10(2)*2 */
+#define REDUCE_SCALED_POWER_BY_THREE_CHAIN	10 /* 10*log10(3)*2 */
+
 /*
  * For AR9285 and later chipsets, the following bits are not being programmed
  * in EEPROM and so need to be enabled always.
@@ -686,6 +689,8 @@ void ath9k_hw_get_target_powers(struct ath_hw *ah,
 				u16 numRates, bool isHt40Target);
 u16 ath9k_hw_get_max_edge_power(u16 freq, struct cal_ctl_edges *pRdEdgesPower,
 				bool is2GHz, int num_band_edges);
+u16 ath9k_hw_get_scaled_power(struct ath_hw *ah, u16 power_limit,
+			      u8 antenna_reduction);
 void ath9k_hw_update_regulatory_maxpower(struct ath_hw *ah);
 int ath9k_hw_eeprom_init(struct ath_hw *ah);
 
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_9287.c b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
index 604eab8..5ab0e6e 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
@@ -564,9 +564,6 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
 	(((cfgCtl & ~CTL_MODE_M) | (pCtlMode[ctlMode] & CTL_MODE_M)) == \
 	 ((pEepData->ctlIndex[i] & CTL_MODE_M) | SD_NO_CTL))
 
-#define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
-#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
-
 	u16 twiceMaxEdgePower;
 	int i;
 	struct cal_ctl_data_ar9287 *rep;
@@ -591,29 +588,8 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
 	tx_chainmask = ah->txchainmask;
 
 	ath9k_hw_get_channel_centers(ah, chan, &centers);
-	scaledPower = powerLimit - antenna_reduction;
-
-	/*
-	 * Reduce scaled Power by number of chains active
-	 * to get the per chain tx power level.
-	 */
-	switch (ar5416_get_ntxchains(tx_chainmask)) {
-	case 1:
-		break;
-	case 2:
-		if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN)
-			scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
-		else
-			scaledPower = 0;
-		break;
-	case 3:
-		if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN)
-			scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
-		else
-			scaledPower = 0;
-		break;
-	}
-	scaledPower = max((u16)0, scaledPower);
+	scaledPower = ath9k_hw_get_scaled_power(ah, powerLimit,
+						antenna_reduction);
 
 	/*
 	 * Get TX power from EEPROM.
@@ -786,8 +762,6 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
 
 #undef CMP_CTL
 #undef CMP_NO_CTL
-#undef REDUCE_SCALED_POWER_BY_TWO_CHAIN
-#undef REDUCE_SCALED_POWER_BY_THREE_CHAIN
 }
 
 static void ath9k_hw_ar9287_set_txpower(struct ath_hw *ah,
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_def.c b/drivers/net/wireless/ath/ath9k/eeprom_def.c
index 0068627..3a9c9ed 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_def.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_def.c
@@ -991,9 +991,6 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
 						  u16 antenna_reduction,
 						  u16 powerLimit)
 {
-#define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
-#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
-
 	struct ar5416_eeprom_def *pEepData = &ah->eeprom.def;
 	u16 twiceMaxEdgePower;
 	int i;
@@ -1027,24 +1024,8 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
 
 	ath9k_hw_get_channel_centers(ah, chan, &centers);
 
-	scaledPower = powerLimit - antenna_reduction;
-
-	switch (ar5416_get_ntxchains(tx_chainmask)) {
-	case 1:
-		break;
-	case 2:
-		if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN)
-			scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
-		else
-			scaledPower = 0;
-		break;
-	case 3:
-		if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN)
-			scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
-		else
-			scaledPower = 0;
-		break;
-	}
+	scaledPower = ath9k_hw_get_scaled_power(ah, powerLimit,
+						antenna_reduction);
 
 	if (IS_CHAN_2GHZ(chan)) {
 		numCtlModes = ARRAY_SIZE(ctlModesFor11g) -
-- 
1.7.2.1

[PATCH 3/3] ath9k: simplify ath9k_hw_get_scaled_power function

[Gabor Juhos]

Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/eeprom.c |   21 ++++++++-------------
 1 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index 61bad99..6a64dec 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -293,9 +293,7 @@ u16 ath9k_hw_get_max_edge_power(u16 freq, struct cal_ctl_edges *pRdEdgesPower,
 u16 ath9k_hw_get_scaled_power(struct ath_hw *ah, u16 power_limit,
 			      u8 antenna_reduction)
 {
-	u16 scaled_power;
-
-	scaled_power = power_limit - antenna_reduction;
+	u16 reduction = antenna_reduction;
 
 	/*
 	 * Reduce scaled Power by number of chains active
@@ -305,22 +303,19 @@ u16 ath9k_hw_get_scaled_power(struct ath_hw *ah, u16 power_limit,
 	case 1:
 		break;
 	case 2:
-		if (scaled_power > REDUCE_SCALED_POWER_BY_TWO_CHAIN)
-			scaled_power -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
-		else
-			scaled_power = 0;
+		reduction += REDUCE_SCALED_POWER_BY_TWO_CHAIN;
 		break;
 	case 3:
-		if (scaled_power > REDUCE_SCALED_POWER_BY_THREE_CHAIN)
-			scaled_power -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
-		else
-			scaled_power = 0;
+		reduction += REDUCE_SCALED_POWER_BY_THREE_CHAIN;
 		break;
 	}
 
-	scaled_power = max((u16)0, scaled_power);
+	if (power_limit > reduction)
+		power_limit -= reduction;
+	else
+		power_limit = 0;
 
-	return scaled_power;
+	return power_limit;
 }
 
 void ath9k_hw_update_regulatory_maxpower(struct ath_hw *ah)
-- 
1.7.2.1

Re: [PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[Daniel Halperin]

(Failed to reply-all originally, sorry:)

Erm...

On Sat, Apr 14, 2012 at 1:01 PM, Gabor Juhos <juhosg@openwrt.org> wrote:
>
> In eeprom_def.c and in ar9003_eeprom.c the value
> of the symbol is 9, however the comments in these
> files indicates the value should be 10*log10(3)*2
> which is 9.54242509439325. Replace the the value
> to 10 in these files.

So the truncated constant is off by a quarter-dB for 9 or for 10. Do
the chips care one way or the other? (For comparison, Intel uses 4.5
dB, i.e. 9 in this notation, to mean "divide-by-3". That's what ath9k
has now).

Are you making it consistent with some other file that's not mentioned
here? If not, why?

>  #define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
> -#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   9  /* 10*log10(3)*2 */
> +#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
>  #define PWRINCR_3_TO_1_CHAIN      9             /* 10*log(3)*2 */

Why only change one of two 9s to a 10? Why not also PWRINCR_3_TO_1_CHAIN?

Dan

Re: [PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[Felix Fietkau]

On 2012-04-14 10:01 PM, Gabor Juhos wrote:
> The REDUCE_SCALED_POWER_BY_THREE_CHAIN symbol is
> defined in different eeprom files, and the value
> varies between the different files.
> 
> In eeprom_def.c and in ar9003_eeprom.c the value
> of the symbol is 9, however the comments in these
> files indicates the value should be 10*log10(3)*2
> which is 9.54242509439325. Replace the the value
> to 10 in these files.
> 
> Also add comments to eeprom_9287.c.
> 
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
I think we should keep the value 9.
If I understand the logic behind the power increase through chain
combining properly, this value only describes the worst-case (wrt.
regulatory compliance) upper limit of tx power, whereas in practice the
measured combined power output will be much lower than that due to
signal/phase differences.
Regulatory compliance is already properly tested on all devices with the
truncated value 9, so I don't think that we need to be even more
conservative and round up here.

- Felix

Re: [PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[Gabor Juhos]

Hi Felix,

Thank you for the comments.

> On 2012-04-14 10:01 PM, Gabor Juhos wrote:
>> The REDUCE_SCALED_POWER_BY_THREE_CHAIN symbol is
>> defined in different eeprom files, and the value
>> varies between the different files.
>>
>> In eeprom_def.c and in ar9003_eeprom.c the value
>> of the symbol is 9, however the comments in these
>> files indicates the value should be 10*log10(3)*2
>> which is 9.54242509439325. Replace the the value
>> to 10 in these files.
>>
>> Also add comments to eeprom_9287.c.
>>
>> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> I think we should keep the value 9.
> If I understand the logic behind the power increase through chain
> combining properly, this value only describes the worst-case (wrt.
> regulatory compliance) upper limit of tx power, whereas in practice the
> measured combined power output will be much lower than that due to
> signal/phase differences.
> Regulatory compliance is already properly tested on all devices with the
> truncated value 9, so I don't think that we need to be even more
> conservative and round up here.

Ok, I will rework the patch.

Apart from the inconsistency between the REDUCE_SCALED_POWER_BY_THREE_CHAIN
values, there is another thing which confuses me.

In the 'ath9k_hw_set_*_power_cal_table' functions the driver uses the
REDUCE_SCALED_POWER_BY_THREE_CHAIN constant to reduce the power values.
In the 'ath9k_hw_update_regulatory_maxpower' function it uses the
INCREASE_MAXPOW_BY_THREE_CHAIN constant to compensate the reduction. However the
two constants are different. I might be wrong, but in my opinion the driver
should use the same constant in both functions.

-Gabor

Re: [PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[Gabor Juhos]

Hi Dan,

Thank you for the comments.

> (Failed to reply-all originally, sorry:)
> 
> Erm...
> 
> On Sat, Apr 14, 2012 at 1:01 PM, Gabor Juhos <juhosg@openwrt.org> wrote:
>>
>> In eeprom_def.c and in ar9003_eeprom.c the value
>> of the symbol is 9, however the comments in these
>> files indicates the value should be 10*log10(3)*2
>> which is 9.54242509439325. Replace the the value
>> to 10 in these files.
> 
> So the truncated constant is off by a quarter-dB for 9 or for 10. Do
> the chips care one way or the other? (For comparison, Intel uses 4.5
> dB, i.e. 9 in this notation, to mean "divide-by-3". That's what ath9k
> has now).

In my understanding the value is not related to the chip. It is used for
per-chain tx power reduction, in order to ensure regulatory compliance. Using 10
instead of 9 means that the overall tx power will be lower.

> Are you making it consistent with some other file that's not mentioned
> here? If not, why?

I'm tyring to make it consistent between ar9003_eeprom.c, eeprom_def.c and
eeprom_9287.c. The first two files uses 9, whereas eeprom_9287 uses 10.

Considering Felix's comment, I will update the patch and will use 9 instead of
10. Although that will replace the value in eeprom_9287.c, but that should not
cause problems, because the AR9287 does not supports 3x3.

>>  #define REDUCE_SCALED_POWER_BY_TWO_CHAIN     6  /* 10*log10(2)*2 */
>> -#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   9  /* 10*log10(3)*2 */
>> +#define REDUCE_SCALED_POWER_BY_THREE_CHAIN   10 /* 10*log10(3)*2 */
>>  #define PWRINCR_3_TO_1_CHAIN      9             /* 10*log(3)*2 */
> 
> Why only change one of two 9s to a 10? Why not also PWRINCR_3_TO_1_CHAIN?

The PWRINCR_* symbols are not used anywhere and should be removed.

-Gabor

Re: [PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[Felix Fietkau]

On 2012-04-15 7:21 PM, Gabor Juhos wrote:
> Hi Felix,
> 
> Thank you for the comments.
> 
>> On 2012-04-14 10:01 PM, Gabor Juhos wrote:
>>> The REDUCE_SCALED_POWER_BY_THREE_CHAIN symbol is
>>> defined in different eeprom files, and the value
>>> varies between the different files.
>>>
>>> In eeprom_def.c and in ar9003_eeprom.c the value
>>> of the symbol is 9, however the comments in these
>>> files indicates the value should be 10*log10(3)*2
>>> which is 9.54242509439325. Replace the the value
>>> to 10 in these files.
>>>
>>> Also add comments to eeprom_9287.c.
>>>
>>> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
>> I think we should keep the value 9.
>> If I understand the logic behind the power increase through chain
>> combining properly, this value only describes the worst-case (wrt.
>> regulatory compliance) upper limit of tx power, whereas in practice the
>> measured combined power output will be much lower than that due to
>> signal/phase differences.
>> Regulatory compliance is already properly tested on all devices with the
>> truncated value 9, so I don't think that we need to be even more
>> conservative and round up here.
> 
> Ok, I will rework the patch.
> 
> Apart from the inconsistency between the REDUCE_SCALED_POWER_BY_THREE_CHAIN
> values, there is another thing which confuses me.
> 
> In the 'ath9k_hw_set_*_power_cal_table' functions the driver uses the
> REDUCE_SCALED_POWER_BY_THREE_CHAIN constant to reduce the power values.
> In the 'ath9k_hw_update_regulatory_maxpower' function it uses the
> INCREASE_MAXPOW_BY_THREE_CHAIN constant to compensate the reduction. However the
> two constants are different. I might be wrong, but in my opinion the driver
> should use the same constant in both functions.
I just checked with Atheros sources, and it seems I was wrong about
sticking with the value 9.
It appears that Atheros changed it to 10 at some point, and ath9k
contains a mix of the old and the new version. Consistently using the
value 10 for 3 chains for both increase and reduction seems to be the
right thing to do.

- Felix

Re: [PATCH 1/3] ath9k: use consistent value for REDUCE_SCALED_POWER_BY_THREE_CHAIN

[Gabor Juhos]

2012.04.15. 19:26 keltezéssel, Felix Fietkau írta:
> On 2012-04-15 7:21 PM, Gabor Juhos wrote:
>> Hi Felix,
>>
>> Thank you for the comments.
>>
>>> On 2012-04-14 10:01 PM, Gabor Juhos wrote:
>>>> The REDUCE_SCALED_POWER_BY_THREE_CHAIN symbol is
>>>> defined in different eeprom files, and the value
>>>> varies between the different files.
>>>>
>>>> In eeprom_def.c and in ar9003_eeprom.c the value
>>>> of the symbol is 9, however the comments in these
>>>> files indicates the value should be 10*log10(3)*2
>>>> which is 9.54242509439325. Replace the the value
>>>> to 10 in these files.
>>>>
>>>> Also add comments to eeprom_9287.c.
>>>>
>>>> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
>>> I think we should keep the value 9.
>>> If I understand the logic behind the power increase through chain
>>> combining properly, this value only describes the worst-case (wrt.
>>> regulatory compliance) upper limit of tx power, whereas in practice the
>>> measured combined power output will be much lower than that due to
>>> signal/phase differences.
>>> Regulatory compliance is already properly tested on all devices with the
>>> truncated value 9, so I don't think that we need to be even more
>>> conservative and round up here.
>>
>> Ok, I will rework the patch.
>>
>> Apart from the inconsistency between the REDUCE_SCALED_POWER_BY_THREE_CHAIN
>> values, there is another thing which confuses me.
>>
>> In the 'ath9k_hw_set_*_power_cal_table' functions the driver uses the
>> REDUCE_SCALED_POWER_BY_THREE_CHAIN constant to reduce the power values.
>> In the 'ath9k_hw_update_regulatory_maxpower' function it uses the
>> INCREASE_MAXPOW_BY_THREE_CHAIN constant to compensate the reduction. However the
>> two constants are different. I might be wrong, but in my opinion the driver
>> should use the same constant in both functions.
> I just checked with Atheros sources, and it seems I was wrong about
> sticking with the value 9.
> It appears that Atheros changed it to 10 at some point, and ath9k
> contains a mix of the old and the new version. Consistently using the
> value 10 for 3 chains for both increase and reduction seems to be the
> right thing to do.

Thank you for the information. I will add it to the commit message.

-Gabor