2012-04-14 20:24:58

by Gabor Juhos

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

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 <[email protected]>
---
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



2012-04-15 10:31:47

by Felix Fietkau

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

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 <[email protected]>
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

2012-04-14 20:25:04

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 3/3] ath9k: simplify ath9k_hw_get_scaled_power function

Signed-off-by: Gabor Juhos <[email protected]>
---
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


2012-04-15 17:50:05

by Gabor Juhos

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

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 <[email protected]>
>>> 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

2012-04-14 20:24:58

by Gabor Juhos

[permalink] [raw]
Subject: [PATCH 2/3] ath9k: introduce ath9k_hw_get_scaled_power helper

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 <[email protected]>
---
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


2012-04-15 17:26:57

by Felix Fietkau

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

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 <[email protected]>
>> 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

2012-04-15 17:21:42

by Gabor Juhos

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

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 <[email protected]>
> 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

2012-04-14 22:11:36

by Daniel Halperin

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

(Failed to reply-all originally, sorry:)

Erm...

On Sat, Apr 14, 2012 at 1:01 PM, Gabor Juhos?<[email protected]>?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

2012-04-15 17:21:57

by Gabor Juhos

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

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 <[email protected]> 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