* [PATCH 0/2] ath5k: fixing broken power gain calibration at 5GHz and txpower handling @ 2012-07-23 16:01 Thomas Huehn 2012-07-23 16:01 ` [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A Thomas Huehn 2012-07-23 16:01 ` [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes Thomas Huehn 0 siblings, 2 replies; 30+ messages in thread From: Thomas Huehn @ 2012-07-23 16:01 UTC (permalink / raw) To: linville Cc: jirislaby, mickflemm, mcgrof, ath5k-devel, linux-wireless, johannes.berg, thomas, nbd This patch series fix a wrong power gain calibration in ath5k for entire 5GHz band and enables consistent usage of tx_power specified by the user. (1) Function ath5k_eeprom_read_target_rate_pwr_info() read 10 lines of rate_pcal_info[i].target_power which worngly produces e.g. from Winstron CM9: [ 38.988000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[0].freq = 4920, rate_pcal_info[0].target_power_6to24 = 36, rate_pcal_info[0].target_power_54 = 24 [ 39.004000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[1].freq = 5040, rate_pcal_info[1].target_power_6to24 = 36, rate_pcal_info[1].target_power_54 = 24 [ 39.020000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[2].freq = 5170, rate_pcal_info[2].target_power_6to24 = 36, rate_pcal_info[2].target_power_54 = 26 [ 39.040000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[3].freq = 5240, rate_pcal_info[3].target_power_6to24 = 38, rate_pcal_info[3].target_power_54 = 26 [ 39.056000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[4].freq = 5320, rate_pcal_info[4].target_power_6to24 = 38, rate_pcal_info[4].target_power_54 = 26 [ 39.076000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[5].freq = 5500, rate_pcal_info[5].target_power_6to24 = 38, rate_pcal_info[5].target_power_54 = 26 [ 39.092000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[6].freq = 5700, rate_pcal_info[6].target_power_6to24 = 36, rate_pcal_info[6].target_power_54 = 26 [ 39.112000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[7].freq = 5825, rate_pcal_info[7].target_power_6to24 = 34, rate_pcal_info[7].target_power_54 = 24 [ 39.128000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[8].freq = 5360, rate_pcal_info[8].target_power_6to24 = 39, rate_pcal_info[8].target_power_54 = 39 [ 39.144000] [ath5k_eeprom_read_target_rate_pwr_info] rate_pcal_info[9].freq = 5720, rate_pcal_info[9].target_power_6to24 = 38, rate_pcal_info[9].target_power_54 = 38 Beside that both last 2 lines hold invalid power gain values the last line is responsible that subsequent interpolation functions apply full power gain to all rates at all frequencies greater rate_pcal_info[max].freq, i.e. > 5720 MHz. This leads to spectral distortion when then global tx_power is higher than the expected rate_pcal_info[xy].target_power_54 and therfore gives rates above 24MBit a very poor performance in therms of throughput. The first patch reduces the eeprom calibration reads to 8. A validation against madwifi showed 8x eeprom reads at 5GHz as well. (2) Furthermore the tx_power specified by the user is not respected in cases where function phy_init() got called: tx_power is always set to ah->ah_txpower.txp_cur_pwr which is never updated with the txpower value specified by the user instead it equales max chan power and is even incremented. So this wrong behavior amplifies the occurrence of the performance problems coming from the wrong calibration in cases the user set a low power. The second patch fixes this by properly introduce and respect user tx_power. Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-07-23 16:01 [PATCH 0/2] ath5k: fixing broken power gain calibration at 5GHz and txpower handling Thomas Huehn @ 2012-07-23 16:01 ` Thomas Huehn 2012-07-25 18:42 ` Nick Kossifidis 2012-08-04 8:14 ` Thomas Huehn 2012-07-23 16:01 ` [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes Thomas Huehn 1 sibling, 2 replies; 30+ messages in thread From: Thomas Huehn @ 2012-07-23 16:01 UTC (permalink / raw) To: linville Cc: jirislaby, mickflemm, mcgrof, ath5k-devel, linux-wireless, johannes.berg, thomas, nbd This patch reduces the per rate target power eeprom reads for AR5K_EEPROM_MODE_11A from 10 to 8, as there are only 8 valid power curve entries on the eeprom. The former 10 reads lead to wrong per rate power limits where rates above 24MBit could be amplified with to high distortion leading to bad performance. This was fix validated against the madwifi codebase and a new AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 is defined. Signed-off-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de> --- madwifi cross validation check. Thx to Felix Fiethkau --- drivers/net/wireless/ath/ath5k/eeprom.c | 2 +- drivers/net/wireless/ath/ath5k/eeprom.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c index 4026c90..b7e0258 100644 --- a/drivers/net/wireless/ath/ath5k/eeprom.c +++ b/drivers/net/wireless/ath/ath5k/eeprom.c @@ -1482,7 +1482,7 @@ ath5k_eeprom_read_target_rate_pwr_info(struct ath5k_hw *ah, unsigned int mode) case AR5K_EEPROM_MODE_11A: offset += AR5K_EEPROM_TARGET_PWR_OFF_11A(ee->ee_version); rate_pcal_info = ee->ee_rate_tpwr_a; - ee->ee_rate_target_pwr_num[mode] = AR5K_EEPROM_N_5GHZ_CHAN; + ee->ee_rate_target_pwr_num[mode] = AR5K_EEPROM_N_5GHZ_RATE_CHAN; break; case AR5K_EEPROM_MODE_11B: offset += AR5K_EEPROM_TARGET_PWR_OFF_11B(ee->ee_version); diff --git a/drivers/net/wireless/ath/ath5k/eeprom.h b/drivers/net/wireless/ath/ath5k/eeprom.h index dc2bcfe..94a9bbe 100644 --- a/drivers/net/wireless/ath/ath5k/eeprom.h +++ b/drivers/net/wireless/ath/ath5k/eeprom.h @@ -182,6 +182,7 @@ #define AR5K_EEPROM_EEP_DELTA 10 #define AR5K_EEPROM_N_MODES 3 #define AR5K_EEPROM_N_5GHZ_CHAN 10 +#define AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 #define AR5K_EEPROM_N_2GHZ_CHAN 3 #define AR5K_EEPROM_N_2GHZ_CHAN_2413 4 #define AR5K_EEPROM_N_2GHZ_CHAN_MAX 4 -- 1.7.11.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-07-23 16:01 ` [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A Thomas Huehn @ 2012-07-25 18:42 ` Nick Kossifidis 2012-07-25 18:55 ` Felix Fietkau 2012-07-25 22:01 ` [ath5k-devel] " Thomas Huehn 2012-08-04 8:14 ` Thomas Huehn 1 sibling, 2 replies; 30+ messages in thread From: Nick Kossifidis @ 2012-07-25 18:42 UTC (permalink / raw) To: Thomas Huehn Cc: linville, jirislaby, mcgrof, ath5k-devel, linux-wireless, johannes.berg, nbd 2012/7/23 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > This patch reduces the per rate target power eeprom reads for > AR5K_EEPROM_MODE_11A from 10 to 8, as there are only 8 valid > power curve entries on the eeprom. The former 10 reads lead to > wrong per rate power limits where rates above 24MBit could be > amplified with to high distortion leading to bad performance. > This was fix validated against the madwifi codebase and a new > AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 is defined. > > Signed-off-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de> > --- > madwifi cross validation check. Thx to Felix Fiethkau > --- Thanks for catching this but the bug is elsewhere: I've tried to document hw code as much as I can, eeprom stuff is still missing and I'm sorry for that but here is what comments say on phy.c @ DOC: Per-rate tx power setting, that should give you an idea of what's happening... 3498 * Rate power table contains indices to PCDAC/PDADC table (0.5dB steps - 3499 * x values) and is indexed as follows: 3500 * rates[0] - rates[7] -> OFDM rates 3501 * rates[8] - rates[14] -> CCK rates 3502 * rates[15] -> XR rates (they all have the same power) So guess why the last 2 "invalid" power gain values have the same power levels for all rates ? They are for XR mode (they've put it there on the eeprom because XR mode also operates at 5GHz) and since XR stuff is stripped off Madwifi (at least the public HAL) that's why it only reads 8 of the piers. Now notice that the last 2 readings on your previous post start from a different frequency and most important that 5360 < 5825 and that makes sense since the idea is to have only 2 points to interpolate between them for the whole XR range. So we have 2 frequency "ranges" to search, the one is the 11a mode from 4920 to 5825 and the other one is for XR mode from 5360 to 5720. When the first range ends the other starts. The bug is here: ath5k_get_rate_pcal_data: 2704 max = ee->ee_rate_target_pwr_num[mode] - 1; [...] 2713 if (target > rpinfo[max].freq) { 2714 idx_l = idx_r = max; 2715 goto done; 2716 } So please instead of tweaking the EEPROM code just change max = ee->ee_rate_target_pwr_num[mode] - 1; to max = ee->ee_rate_target_pwr_num[mode] - 3; in case of AR5K_MODE_11A Each function has a job to do, ath5k_eeprom_read_target_rate_pwr_info should read EEPROM contents, not decide what we'll use on interpolation and what we won't use. Also since EEPROM code uses offsets etc I don't want to change the way we read it because if we miss something then the whole dataset will get "shifted" and we'll get weird stuff :P -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-07-25 18:42 ` Nick Kossifidis @ 2012-07-25 18:55 ` Felix Fietkau 2012-07-25 22:22 ` Nick Kossifidis 2012-07-25 22:01 ` [ath5k-devel] " Thomas Huehn 1 sibling, 1 reply; 30+ messages in thread From: Felix Fietkau @ 2012-07-25 18:55 UTC (permalink / raw) To: Nick Kossifidis Cc: Thomas Huehn, linville, jirislaby, mcgrof, ath5k-devel, linux-wireless, johannes.berg On 2012-07-25 8:42 PM, Nick Kossifidis wrote: > 2012/7/23 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >> This patch reduces the per rate target power eeprom reads for >> AR5K_EEPROM_MODE_11A from 10 to 8, as there are only 8 valid >> power curve entries on the eeprom. The former 10 reads lead to >> wrong per rate power limits where rates above 24MBit could be >> amplified with to high distortion leading to bad performance. >> This was fix validated against the madwifi codebase and a new >> AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 is defined. >> >> Signed-off-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de> >> --- >> madwifi cross validation check. Thx to Felix Fiethkau >> --- > > > Thanks for catching this but the bug is elsewhere: > > I've tried to document hw code as much as I can, eeprom stuff is still > missing and I'm sorry for that but here is what comments say on phy.c > @ DOC: Per-rate tx power setting, that should give you an idea of > what's happening... > > 3498 * Rate power table contains indices to PCDAC/PDADC table (0.5dB steps - > 3499 * x values) and is indexed as follows: > 3500 * rates[0] - rates[7] -> OFDM rates > 3501 * rates[8] - rates[14] -> CCK rates > 3502 * rates[15] -> XR rates (they all have the same power) > > So guess why the last 2 "invalid" power gain values have the same > power levels for all rates ? They are for XR mode (they've put it > there on the eeprom because XR mode also operates at 5GHz) and since > XR stuff is stripped off Madwifi (at least the public HAL) that's why > it only reads 8 of the piers. It's not just stripped in the public HAL. Every driver that I've looked at only reads 8 of the piers in the function that parses the EEPROM. - Felix ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-07-25 18:55 ` Felix Fietkau @ 2012-07-25 22:22 ` Nick Kossifidis 0 siblings, 0 replies; 30+ messages in thread From: Nick Kossifidis @ 2012-07-25 22:22 UTC (permalink / raw) To: Felix Fietkau Cc: Thomas Huehn, linville, jirislaby, mcgrof, ath5k-devel, linux-wireless, johannes.berg 2012/7/25 Felix Fietkau <nbd@openwrt.org>: > On 2012-07-25 8:42 PM, Nick Kossifidis wrote: >> 2012/7/23 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>> This patch reduces the per rate target power eeprom reads for >>> AR5K_EEPROM_MODE_11A from 10 to 8, as there are only 8 valid >>> power curve entries on the eeprom. The former 10 reads lead to >>> wrong per rate power limits where rates above 24MBit could be >>> amplified with to high distortion leading to bad performance. >>> This was fix validated against the madwifi codebase and a new >>> AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 is defined. >>> >>> Signed-off-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de> >>> --- >>> madwifi cross validation check. Thx to Felix Fiethkau >>> --- >> >> >> Thanks for catching this but the bug is elsewhere: >> >> I've tried to document hw code as much as I can, eeprom stuff is still >> missing and I'm sorry for that but here is what comments say on phy.c >> @ DOC: Per-rate tx power setting, that should give you an idea of >> what's happening... >> >> 3498 * Rate power table contains indices to PCDAC/PDADC table (0.5dB steps - >> 3499 * x values) and is indexed as follows: >> 3500 * rates[0] - rates[7] -> OFDM rates >> 3501 * rates[8] - rates[14] -> CCK rates >> 3502 * rates[15] -> XR rates (they all have the same power) >> >> So guess why the last 2 "invalid" power gain values have the same >> power levels for all rates ? They are for XR mode (they've put it >> there on the eeprom because XR mode also operates at 5GHz) and since >> XR stuff is stripped off Madwifi (at least the public HAL) that's why >> it only reads 8 of the piers. > It's not just stripped in the public HAL. Every driver that I've looked > at only reads 8 of the piers in the function that parses the EEPROM. > > - Felix XR mode stuff is being stripped out from anything I got (docs etc) so in some cases I don't understand something I blame XR :P However I double checked this and it seems you are right: a) 11a has 10 cal piers for the channel power table and 8 cal piers for the rate powertable, not only that but the 8 are optional and might not even cover the whole freq range. b) I printed out the offsets and what happens it that the last 2 measurements we got have the same format because they are the 802.11b cal piers and they have the same power levels because 11b is CCK so it makes sense. c) Something fishy is going on with ee->ee_xr_power[mode], is this the max rate power for all channels on XR mode ? Is that possible to have the same power index (and a high one) for all channels ? That's the main reason I suspected these two values have something to do with XR. If that's the case shouldn't we use that instead of rates[0] on ath5k_setup_rate_powertable for XR rates ? Does any of the drivers you 've seen use ee->ee_xr_power[mode] ? Anyway I left the EEPROM code unmaintained for a long time, the related TODOs on wireless.kernel.org wiki are there too. Since I now have documentation on EEPROM I'll start working on it again and clean it up. -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-07-25 18:42 ` Nick Kossifidis 2012-07-25 18:55 ` Felix Fietkau @ 2012-07-25 22:01 ` Thomas Huehn 2012-07-25 22:31 ` Nick Kossifidis 1 sibling, 1 reply; 30+ messages in thread From: Thomas Huehn @ 2012-07-25 22:01 UTC (permalink / raw) To: Nick Kossifidis Cc: jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville, nbd Hi Nick, Nick Kossifidis schrieb: > Thanks for catching this but the bug is elsewhere: > So guess why the last 2 "invalid" power gain values have the same > power levels for all rates ? They are for XR mode (they've put it > there on the eeprom because XR mode also operates at 5GHz) and since > XR stuff is stripped off Madwifi (at least the public HAL) that's why > it only reads 8 of the piers. > > Now notice that the last 2 readings on your previous post start from a > different frequency and most important that 5360 < 5825 and that > makes sense since the idea is to have only 2 points to interpolate > between them for the whole XR range. So we have 2 frequency "ranges" > to search, the one is the 11a mode from 4920 to 5825 and the other > one is for XR mode from 5360 to 5720. When the first range ends the > other starts. > > The bug is here: > > ath5k_get_rate_pcal_data: > 2704 max = ee->ee_rate_target_pwr_num[mode] - 1; > [...] > 2713 if (target > rpinfo[max].freq) { > 2714 idx_l = idx_r = max; > 2715 goto done; > 2716 } > > So please instead of tweaking the EEPROM code just change > max = ee->ee_rate_target_pwr_num[mode] - 1; > to > max = ee->ee_rate_target_pwr_num[mode] - 3; > in case of AR5K_MODE_11A The calibration data for 802.11a in the standard conform mode should only consider 8 reads from the eeprom, thats why I introduced this new define... a "-3" is from my view of readability not as traceable. Any other non standard mode as you mentioned XR as one of, could just ready its necessary lines of the eeprom ones it get supported in ath5k. Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-07-25 22:01 ` [ath5k-devel] " Thomas Huehn @ 2012-07-25 22:31 ` Nick Kossifidis 0 siblings, 0 replies; 30+ messages in thread From: Nick Kossifidis @ 2012-07-25 22:31 UTC (permalink / raw) To: Thomas Huehn Cc: jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville, nbd 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi Nick, > > [...] > > The calibration data for 802.11a in the standard conform mode should > only consider 8 reads from the eeprom, thats why I introduced this new > define... a "-3" is from my view of readability not as traceable. > Any other non standard mode as you mentioned XR as one of, could just > ready its necessary lines of the eeprom ones it get supported in ath5k. > > Greetings Thomas Sorry for the mess Thomas, guess I should start trusting the docs etc I have and leave rev. engineering mode from now on :P Acked-by: Nick Kossifidis <mickflemm@gmail.com> -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-07-23 16:01 ` [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A Thomas Huehn 2012-07-25 18:42 ` Nick Kossifidis @ 2012-08-04 8:14 ` Thomas Huehn 2012-08-04 15:28 ` Nick Kossifidis 1 sibling, 1 reply; 30+ messages in thread From: Thomas Huehn @ 2012-08-04 8:14 UTC (permalink / raw) To: nbd, Nick Kossifidis Cc: jirislaby, linux-wireless, mcgrof, ath5k-devel, Bob Copeland Hi all, After several experiments on cm9 and dcma82 cards, I figured out that this patch does not solve the max_power calibration problem as intended. The partly reduction to 8 with this: #define AR5K_EEPROM_N_5GHZ_CHAN 10 #define AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 ... creates a wrong power curve on the card, as function ath5k_eeprom_read_freq_list() runs its while loop still 10 times, which results in wrong AR5K_EEPROM_READ(o++, val) readings, leading the card to use a very low power level over all. This should also be limited to 8, as I tested it. My suggestion is to just set: #define AR5K_EEPROM_N_5GHZ_CHAN 8 .. without introducing a separate variable, it is not needed. I will send a v2. While I am browsing through /ath5k/eeprom.c there are several other suspicious places where 10 eeprom lines of chips like 5111, 5112, 2413 depending on their EEPROM Version are read. I can not test this, as I have only CM9 and DCMA82 over here, but I guess those 10 line reads are also wrong. Can someone test this ? Greetings Thomas > --- a/drivers/net/wireless/ath/ath5k/eeprom.c > +++ b/drivers/net/wireless/ath/ath5k/eeprom.c > @@ -1482,7 +1482,7 @@ ath5k_eeprom_read_target_rate_pwr_info(struct ath5k_hw *ah, unsigned int mode) > case AR5K_EEPROM_MODE_11A: > offset += AR5K_EEPROM_TARGET_PWR_OFF_11A(ee->ee_version); > rate_pcal_info = ee->ee_rate_tpwr_a; > - ee->ee_rate_target_pwr_num[mode] = AR5K_EEPROM_N_5GHZ_CHAN; > + ee->ee_rate_target_pwr_num[mode] = AR5K_EEPROM_N_5GHZ_RATE_CHAN; > break; > case AR5K_EEPROM_MODE_11B: > offset += AR5K_EEPROM_TARGET_PWR_OFF_11B(ee->ee_version); > diff --git a/drivers/net/wireless/ath/ath5k/eeprom.h b/drivers/net/wireless/ath/ath5k/eeprom.h > index dc2bcfe..94a9bbe 100644 > --- a/drivers/net/wireless/ath/ath5k/eeprom.h > +++ b/drivers/net/wireless/ath/ath5k/eeprom.h > @@ -182,6 +182,7 @@ > #define AR5K_EEPROM_EEP_DELTA 10 > #define AR5K_EEPROM_N_MODES 3 > #define AR5K_EEPROM_N_5GHZ_CHAN 10 > +#define AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 > #define AR5K_EEPROM_N_2GHZ_CHAN 3 > #define AR5K_EEPROM_N_2GHZ_CHAN_2413 4 > #define AR5K_EEPROM_N_2GHZ_CHAN_MAX 4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-08-04 8:14 ` Thomas Huehn @ 2012-08-04 15:28 ` Nick Kossifidis 2012-08-05 11:06 ` Thomas Huehn 0 siblings, 1 reply; 30+ messages in thread From: Nick Kossifidis @ 2012-08-04 15:28 UTC (permalink / raw) To: Thomas Huehn Cc: nbd, jirislaby, linux-wireless, mcgrof, ath5k-devel, Bob Copeland 2012/8/4 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi all, > > After several experiments on cm9 and dcma82 cards, I figured out that > this patch does not solve the max_power calibration problem as intended. > > The partly reduction to 8 with this: > #define AR5K_EEPROM_N_5GHZ_CHAN 10 > #define AR5K_EEPROM_N_5GHZ_RATE_CHAN 8 > > ... creates a wrong power curve on the card, as function > ath5k_eeprom_read_freq_list() runs its while loop still 10 times, which > results in wrong AR5K_EEPROM_READ(o++, val) readings, leading the card > to use a very low power level over all. > This should also be limited to 8, as I tested it. > > My suggestion is to just set: > #define AR5K_EEPROM_N_5GHZ_CHAN 8 > > .. without introducing a separate variable, it is not needed. > I will send a v2. > > While I am browsing through /ath5k/eeprom.c there are several other > suspicious places where 10 eeprom lines of chips like 5111, 5112, 2413 > depending on their EEPROM Version are read. I can not test this, as I > have only CM9 and DCMA82 over here, but I guess those 10 line reads are > also wrong. Can someone test this ? > > > Greetings Thomas > There is nothing suspicious about it, it's what EEPROM docs say: a) For 11a mode there are 10 calibration peers and for b/g there are 3 for chips < 2413 and 4 on later chips. b) We first read the frequencies, if freq != 0 we use the calibration info, else we ignore it. c) We then read the power deltas for each calibration pier d) Then we store that info for use by the channel power table calibration Also what do you mean wrong curve and how did you actually test your card and measured the tx power ? -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-08-04 15:28 ` Nick Kossifidis @ 2012-08-05 11:06 ` Thomas Huehn 2012-08-05 11:56 ` Nick Kossifidis 0 siblings, 1 reply; 30+ messages in thread From: Thomas Huehn @ 2012-08-05 11:06 UTC (permalink / raw) To: Nick Kossifidis Cc: nbd, jirislaby, linux-wireless, mcgrof, ath5k-devel, Bob Copeland Hi Nick, > There is nothing suspicious about it, it's what EEPROM docs say: > > a) For 11a mode there are 10 calibration peers As I tested AR5413, AR5414 and AR5213 there are only 8 eeprom lines for 802.11a that provide valid per rate target power levels. I am curious if this also holds for other chips. > Also what do you mean wrong curve and how did you actually test your > card and measured the tx power ? > My first patch only changed the read iterations in function ath5k_eeprom_read_target_rate_pwr_info() from 10 reads to 8. Having only this, I measured received rssi values while changing the tx_power on a sender. I did not observe any changes in rssi, so I went back to the code to investigate. And the function ath5k_eeprom_init_11a_pcal_freq() (my hardware eeprom version is > 3.3) there is ath5k_eeprom_read_freq_list(..,10,..) called, and those 10 iterations produce 2 wrong frequency peer values. If there are only 8 iterations used, I am able to measure a 15 dB variation in rssi values at the receiver side, while changing the tx_power on the sender. For the chips I tested, ath5k_eeprom_read_freq_list() should only read 8 pears for 802.11a. With 'suspicious' I meant e.g. function ath5k_eeprom_init_11a_pcal_freq(), as it checks the eeprom version and if > 3.3 it calls ath5k_eeprom_init_11a_pcal_freq(..,AR5K_EEPROM_N_5GHZ_CHAN,..), in the else case, it reads 10 hardcoded freq.pears .. I am questioning if this is correct, so maybe someone has the chance to test this. Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-08-05 11:06 ` Thomas Huehn @ 2012-08-05 11:56 ` Nick Kossifidis 2012-08-05 12:37 ` Thomas Huehn 0 siblings, 1 reply; 30+ messages in thread From: Nick Kossifidis @ 2012-08-05 11:56 UTC (permalink / raw) To: Thomas Huehn Cc: nbd, jirislaby, linux-wireless, mcgrof, ath5k-devel, Bob Copeland 2012/8/5 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi Nick, > >> There is nothing suspicious about it, it's what EEPROM docs say: >> >> a) For 11a mode there are 10 calibration peers > > > As I tested AR5413, AR5414 and AR5213 there are only 8 eeprom lines for > 802.11a that provide valid per rate target power levels. I am curious if > this also holds for other chips. > Based on EEPROM docs I have it's 8 per rate target power levels for all chips after 5210. >> Also what do you mean wrong curve and how did you actually test your >> card and measured the tx power ? >> > > > My first patch only changed the read iterations in function > ath5k_eeprom_read_target_rate_pwr_info() from 10 reads to 8. Having only > this, I measured received rssi values while changing the tx_power on a > sender. I did not observe any changes in rssi, so I went back to the > code to investigate. Your patch is correct, that doesn't mean it 'll fix everything ;-) To give you a picture not all vendors follow Atheros's refference design, most of them focus on supporting the max power, not all power levels, so their calibration curves don't provide much. This mostly happens on chips that can have extrenal preamplifiers and components so vendors have the freedom to do whatever they want. This doesn't only happen on ath5k, in my tests with MadWiFi I also didn't see any tx power changes (you can also see that on the TODO description on wireless.kernel.org). So have that in mind when testing for tx power changes. I'm not saying the code is perfect or bugless I'm saying that you can't be sure, maybe a cross-check with MadWiFi would help to understand if it's our fault or vendor's choice. > And the function ath5k_eeprom_init_11a_pcal_freq() (my hardware eeprom > version is > 3.3) there is ath5k_eeprom_read_freq_list(..,10,..) called, > and those 10 iterations produce 2 wrong frequency peer values. If there > are only 8 iterations used, I am able to measure a 15 dB variation in > rssi values at the receiver side, while changing the tx_power on the > sender. For the chips I tested, ath5k_eeprom_read_freq_list() should > only read 8 pears for 802.11a. > > With 'suspicious' I meant e.g. function > ath5k_eeprom_init_11a_pcal_freq(), as it checks the eeprom version and > if > 3.3 it calls > ath5k_eeprom_init_11a_pcal_freq(..,AR5K_EEPROM_N_5GHZ_CHAN,..), in the > else case, it reads 10 hardcoded freq.pears .. I am questioning if this > is correct, so maybe someone has the chance to test this. > According to EEPROM docs there are 10 calibration piers on all chips after 5111 for 11a but some of them are not used and the way to test this is if frequency is zero. Now here is a bug in the EEPROM code that might result the wrong reads you get, check this out: on ath5k_eeprom_read_freq_list we do this if (!freq1) break; instead of this if (!freq1) continue; so maybe the offset at the end of this function is wrong, can you please test if this change fixes your readings ? You can also see that HAL code does the same: #define NUM_11A_EEPROM_CHANNELS 10 #define NUM_11A_EEPROM_CHANNELS_2413 10 while (numPiers < NUM_11A_EEPROM_CHANNELS) { EEREAD(off++); if ((eeval & freqmask) == 0) break; freq[numPiers++] = fbin2freq(ee, eeval & freqmask); if (((eeval >> 8) & freqmask) == 0) break; freq[numPiers++] = fbin2freq(ee, (eeval>>8) & freqmask); } Some more infos on the EEPROM code: Most of this code was written by rev-engineering when working on ath_info tool a few years back when we didn't had any docs, all we had were some EEPROM dumps posted on the internet (the output of the eeprom tool that comes with Atheros's ART program). When we got documentation and HAL's source some parts of this code were fixed but we haven't done a complete review since then. -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-08-05 11:56 ` Nick Kossifidis @ 2012-08-05 12:37 ` Thomas Huehn 2012-08-05 12:59 ` Nick Kossifidis 2012-08-05 18:26 ` Nick Kossifidis 0 siblings, 2 replies; 30+ messages in thread From: Thomas Huehn @ 2012-08-05 12:37 UTC (permalink / raw) To: Nick Kossifidis Cc: nbd, jirislaby, linux-wireless, mcgrof, ath5k-devel, Bob Copeland Hi Nick > According to EEPROM docs there are 10 calibration piers on all chips > after 5111 for 11a but some of them are not used and the way to test > this is if frequency is zero. Now here is a bug in the EEPROM code > that might result the wrong reads you get, check this out: > > on ath5k_eeprom_read_freq_list we do this > > if (!freq1) > break; > > instead of this > > if (!freq1) > continue; That did the trick ! So reading 10 calibration piers and using the fix from above works in terms of tx_power changes on CM9 and DCMA82 are properly used by the hardware. I will send a patch for that. Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-08-05 12:37 ` Thomas Huehn @ 2012-08-05 12:59 ` Nick Kossifidis 2012-08-05 18:26 ` Nick Kossifidis 1 sibling, 0 replies; 30+ messages in thread From: Nick Kossifidis @ 2012-08-05 12:59 UTC (permalink / raw) To: Thomas Huehn Cc: nbd, jirislaby, linux-wireless, mcgrof, ath5k-devel, Bob Copeland, Adrian Chadd 2012/8/5 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi Nick > >> According to EEPROM docs there are 10 calibration piers on all chips >> after 5111 for 11a but some of them are not used and the way to test >> this is if frequency is zero. Now here is a bug in the EEPROM code >> that might result the wrong reads you get, check this out: >> >> on ath5k_eeprom_read_freq_list we do this >> >> if (!freq1) >> break; >> >> instead of this >> >> if (!freq1) >> continue; > > > That did the trick ! > So reading 10 calibration piers and using the fix from above works in > terms of tx_power changes on CM9 and DCMA82 are properly used by the > hardware. > > I will send a patch for that. > > > Greetings Thomas > CCing Adrian in case this bug is also present on newer HAL sources on FreeBSD etc... Thanks for testing/patching Thomas, I 'll try to find some time to work on EEPROM code again, I know it's a mess :P -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A 2012-08-05 12:37 ` Thomas Huehn 2012-08-05 12:59 ` Nick Kossifidis @ 2012-08-05 18:26 ` Nick Kossifidis 1 sibling, 0 replies; 30+ messages in thread From: Nick Kossifidis @ 2012-08-05 18:26 UTC (permalink / raw) To: Thomas Huehn Cc: nbd, jirislaby, linux-wireless, mcgrof, ath5k-devel, Bob Copeland 2012/8/5 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi Nick > >> According to EEPROM docs there are 10 calibration piers on all chips >> after 5111 for 11a but some of them are not used and the way to test >> this is if frequency is zero. Now here is a bug in the EEPROM code >> that might result the wrong reads you get, check this out: >> >> on ath5k_eeprom_read_freq_list we do this >> >> if (!freq1) >> break; >> >> instead of this >> >> if (!freq1) >> continue; > > > That did the trick ! > So reading 10 calibration piers and using the fix from above works in > terms of tx_power changes on CM9 and DCMA82 are properly used by the > hardware. > > I will send a patch for that. > > > Greetings Thomas > I have some time right now so I'll re-send my patches (one of them needs a fix anyway) together with this one to make John's job easier. Send your EEPROM patch on top of them so that he can see it (remember to CC him). -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-23 16:01 [PATCH 0/2] ath5k: fixing broken power gain calibration at 5GHz and txpower handling Thomas Huehn 2012-07-23 16:01 ` [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A Thomas Huehn @ 2012-07-23 16:01 ` Thomas Huehn 2012-07-23 16:42 ` [ath5k-devel] " Bob Copeland 2012-07-25 19:22 ` Nick Kossifidis 1 sibling, 2 replies; 30+ messages in thread From: Thomas Huehn @ 2012-07-23 16:01 UTC (permalink / raw) To: linville Cc: jirislaby, mickflemm, mcgrof, ath5k-devel, linux-wireless, johannes.berg, thomas, nbd In such cases where phy_init() function got called, tx_power is always set to ah->ah_txpower.txp_cur_pwr which is never updated with txpower specified by the user instead it equale max chan power and got potentially incremented by ah_txpower.txp_offset. In any case the card was switching to a txpower level higher that someone specified to use. This patch fix this by introducing ah_txpower.txp_user_pwr which holds the tx_power specified at userland. Function ath5k_hw_txpower is restructured and uses the value at ah_txpower.txp_user_pwr directly. Signed-off-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de> --- restructure of ath5k_hw_txpower as suggested by Felix Fietkau. --- drivers/net/wireless/ath/ath5k/ath5k.h | 1 + drivers/net/wireless/ath/ath5k/base.c | 3 +++ drivers/net/wireless/ath/ath5k/phy.c | 27 ++++++++++++++------------- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h index 64a453a..89d9ac54 100644 --- a/drivers/net/wireless/ath/ath5k/ath5k.h +++ b/drivers/net/wireless/ath/ath5k/ath5k.h @@ -1418,6 +1418,7 @@ struct ath5k_hw { s16 txp_min_pwr; s16 txp_max_pwr; s16 txp_cur_pwr; + s16 txp_user_pwr; /* Values in 0.5dB units */ s16 txp_offset; s16 txp_ofdm; diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c index 8c4c040..e831f69 100644 --- a/drivers/net/wireless/ath/ath5k/base.c +++ b/drivers/net/wireless/ath/ath5k/base.c @@ -2951,6 +2951,9 @@ ath5k_init(struct ieee80211_hw *hw) hw->queues = 1; } + /* init tx_power setting to maximum */ + ah->ah_txpower.txp_user_pwr = AR5K_TUNE_MAX_TXPOWER; + tasklet_init(&ah->rxtq, ath5k_tasklet_rx, (unsigned long)ah); tasklet_init(&ah->txtq, ath5k_tasklet_tx, (unsigned long)ah); tasklet_init(&ah->beacontq, ath5k_tasklet_beacon, (unsigned long)ah); diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c index 8b71a2d..728ff07 100644 --- a/drivers/net/wireless/ath/ath5k/phy.c +++ b/drivers/net/wireless/ath/ath5k/phy.c @@ -3583,14 +3583,12 @@ ath5k_setup_rate_powertable(struct ath5k_hw *ah, u16 max_pwr, * ath5k_hw_txpower() - Set transmission power limit for a given channel * @ah: The &struct ath5k_hw * @channel: The &struct ieee80211_channel - * @txpower: Requested tx power in 0.5dB steps * * Combines all of the above to set the requested tx power limit - * on hw. + * on hw to ah->ah_txpower.txp_user_pwr. */ static int -ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, - u8 txpower) +ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel) { struct ath5k_rate_pcal_info rate_info; struct ieee80211_channel *curr_channel = ah->ah_current_channel; @@ -3598,11 +3596,6 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, u8 type; int ret; - if (txpower > AR5K_TUNE_MAX_TXPOWER) { - ATH5K_ERR(ah, "invalid tx power: %u\n", txpower); - return -EINVAL; - } - ee_mode = ath5k_eeprom_mode_from_channel(channel); if (ee_mode < 0) { ATH5K_ERR(ah, @@ -3667,7 +3660,7 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, ath5k_get_rate_pcal_data(ah, channel, &rate_info); /* Setup rate power table */ - ath5k_setup_rate_powertable(ah, txpower, &rate_info, ee_mode); + ath5k_setup_rate_powertable(ah, ah->ah_txpower.txp_user_pwr, &rate_info, ee_mode); /* Write rate power table on hw */ ath5k_hw_reg_write(ah, AR5K_TXPOWER_OFDM(3, 24) | @@ -3717,8 +3710,16 @@ ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower) { ATH5K_DBG(ah, ATH5K_DEBUG_TXPOWER, "changing txpower to %d\n", txpower); + if (txpower) { + ah->ah_txpower.txp_user_pwr = txpower; + + if (ah->ah_txpower.txp_user_pwr > AR5K_TUNE_MAX_TXPOWER) { + ATH5K_ERR(ah, "invalid tx power: %u\n", ah->ah_txpower.txp_user_pwr); + return -EINVAL; + } + } - return ath5k_hw_txpower(ah, ah->ah_current_channel, txpower); + return ath5k_hw_txpower(ah, ah->ah_current_channel); } @@ -3789,8 +3790,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); + if (ret) return ret; -- 1.7.11.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-23 16:01 ` [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes Thomas Huehn @ 2012-07-23 16:42 ` Bob Copeland 2012-07-23 18:25 ` Thomas Huehn 2012-07-25 19:22 ` Nick Kossifidis 1 sibling, 1 reply; 30+ messages in thread From: Bob Copeland @ 2012-07-23 16:42 UTC (permalink / raw) To: Thomas Huehn Cc: linville, jirislaby, johannes.berg, ath5k-devel, linux-wireless, nbd On Mon, Jul 23, 2012 at 06:01:15PM +0200, Thomas Huehn wrote: > /* Setup rate power table */ > - ath5k_setup_rate_powertable(ah, txpower, &rate_info, ee_mode); > + ath5k_setup_rate_powertable(ah, ah->ah_txpower.txp_user_pwr, &rate_info, ee_mode); txpower used to be .5 dB units -- is it still? > - 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); > + Did this / 2 move elsewhere or was it wrong before? Is txp_cur_pwr still used for anything? -- Bob Copeland %% www.bobcopeland.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-23 16:42 ` [ath5k-devel] " Bob Copeland @ 2012-07-23 18:25 ` Thomas Huehn 2012-07-23 18:29 ` Thomas Huehn 2012-07-23 19:20 ` Bob Copeland 0 siblings, 2 replies; 30+ messages in thread From: Thomas Huehn @ 2012-07-23 18:25 UTC (permalink / raw) To: Bob Copeland Cc: jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville, nbd Hi Bob, Bob Copeland schrieb: > On Mon, Jul 23, 2012 at 06:01:15PM +0200, Thomas Huehn wrote: >> /* Setup rate power table */ >> - ath5k_setup_rate_powertable(ah, txpower, &rate_info, ee_mode); >> + ath5k_setup_rate_powertable(ah, ah->ah_txpower.txp_user_pwr, &rate_info, ee_mode); > > txpower used to be .5 dB units -- is it still? ah->ah_txpower.txp_user_pwr is now in 1dB units, as it is triggered from mac802.11 >> - 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); >> + > > Did this / 2 move elsewhere or was it wrong before? Is txp_cur_pwr still > used for anything? the "/2" is not needed anymore as I also skipped the "*2" in ath5k_hw_set_txpower_limit .. so I avoid this calculations. Felix is goint to use txp_cur_pwr, so for now it is not used ... maybe he can explain where it will be needed. Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-23 18:25 ` Thomas Huehn @ 2012-07-23 18:29 ` Thomas Huehn 2012-07-23 19:20 ` Bob Copeland 1 sibling, 0 replies; 30+ messages in thread From: Thomas Huehn @ 2012-07-23 18:29 UTC (permalink / raw) To: Bob Copeland Cc: jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville, nbd, rodrigue, c_manoha Hi all, After applying this 2 patches I got the following eeprom read on an Wistron CM9: [ 38.880000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[0].freq = 4920, rate[0].target_power_6to24 = 36, rate[0].target_power_36 = 32, rate[0].target_power_48 = 28, rate[0].target_power_54 = 24 [ 38.896000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[1].freq = 5040, rate[1].target_power_6to24 = 36, rate[1].target_power_36 = 32, rate[1].target_power_48 = 28, rate[1].target_power_54 = 24 [ 38.916000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[2].freq = 5170, rate[2].target_power_6to24 = 36, rate[2].target_power_36 = 32, rate[2].target_power_48 = 30, rate[2].target_power_54 = 26 [ 38.936000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[3].freq = 5240, rate[3].target_power_6to24 = 38, rate[3].target_power_36 = 34, rate[3].target_power_48 = 30, rate[3].target_power_54 = 26 [ 38.952000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[4].freq = 5320, rate[4].target_power_6to24 = 38, rate[4].target_power_36 = 34, rate[4].target_power_48 = 30, rate[4].target_power_54 = 26 [ 38.972000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[5].freq = 5500, rate[5].target_power_6to24 = 38, rate[5].target_power_36 = 32, rate[5].target_power_48 = 28, rate[5].target_power_54 = 26 [ 38.992000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[6].freq = 5700, rate[6].target_power_6to24 = 36, rate[6].target_power_36 = 32, rate[6].target_power_48 = 28, rate[6].target_power_54 = 26 [ 39.012000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[7].freq = 5825, rate[7].target_power_6to24 = 34, rate[7].target_power_36 = 30, rate[7].target_power_48 = 26, rate[7].target_power_54 = 24 [ 39.036000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[0].freq = 2412, rate[0].target_power_6to24 = 39, rate[0].target_power_36 = 39, rate[0].target_power_48 = 39, rate[0].target_power_54 = 39 [ 39.056000] ath5k: phy0: [bluse ath5k_eeprom] rate_pcal_info[1].freq = 2484, rate[1].target_power_6to24 = 38, rate[1].target_power_36 = 38, rate[1].target_power_48 = 38, rate[1].target_power_54 = 38 The 5 GHz values look ok for me, but is the 2,4 GHz amp really that linear to support all powers at all rates ? Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-23 18:25 ` Thomas Huehn 2012-07-23 18:29 ` Thomas Huehn @ 2012-07-23 19:20 ` Bob Copeland 1 sibling, 0 replies; 30+ messages in thread From: Bob Copeland @ 2012-07-23 19:20 UTC (permalink / raw) To: Thomas Huehn Cc: jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville, nbd On Mon, Jul 23, 2012 at 08:25:26PM +0200, Thomas Huehn wrote: > > Did this / 2 move elsewhere or was it wrong before? Is txp_cur_pwr still > > used for anything? > > the "/2" is not needed anymore as I also skipped the "*2" in > ath5k_hw_set_txpower_limit .. so I avoid this calculations. > > Felix is goint to use txp_cur_pwr, so for now it is not used ... maybe > he can explain where it will be needed. Ok great, thanks for the clarification. -- Bob Copeland %% www.bobcopeland.com ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-23 16:01 ` [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes Thomas Huehn 2012-07-23 16:42 ` [ath5k-devel] " Bob Copeland @ 2012-07-25 19:22 ` Nick Kossifidis 2012-07-25 23:07 ` Thomas Huehn 1 sibling, 1 reply; 30+ messages in thread From: Nick Kossifidis @ 2012-07-25 19:22 UTC (permalink / raw) To: Thomas Huehn Cc: linville, jirislaby, mcgrof, ath5k-devel, linux-wireless, johannes.berg, nbd 2012/7/23 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > In such cases where phy_init() function got called, tx_power is always > set to ah->ah_txpower.txp_cur_pwr which is never updated with txpower > specified by the user instead it equale max chan power and got > potentially incremented by ah_txpower.txp_offset. Never updated ? It gets updated when setting the rate powertable (the final step of the whole process): ath5k_setup_rate_powertable: 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; > In any case the card was switching to a txpower level higher that > someone specified to use. This patch fix this by introducing > ah_txpower.txp_user_pwr which holds the tx_power specified at userland. > Function ath5k_hw_txpower is restructured and uses the value at > directly. > > Signed-off-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de> > --- > restructure of ath5k_hw_txpower as suggested by Felix Fietkau. > --- > drivers/net/wireless/ath/ath5k/ath5k.h | 1 + > drivers/net/wireless/ath/ath5k/base.c | 3 +++ > drivers/net/wireless/ath/ath5k/phy.c | 27 ++++++++++++++------------- > 3 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h > index 64a453a..89d9ac54 100644 > --- a/drivers/net/wireless/ath/ath5k/ath5k.h > +++ b/drivers/net/wireless/ath/ath5k/ath5k.h > @@ -1418,6 +1418,7 @@ struct ath5k_hw { > s16 txp_min_pwr; > s16 txp_max_pwr; > s16 txp_cur_pwr; > + s16 txp_user_pwr; > /* Values in 0.5dB units */ > s16 txp_offset; > s16 txp_ofdm; We already have 1334 int power_level; /* Requested tx power in dBm */ and s16 txp_cur_pwr; What's the reason to use another one ? If you want a different type/name just change power_pevel. We use power_level on ath5k_config when someone updates txpower: 209 if ((changed & IEEE80211_CONF_CHANGE_POWER) && 210 (ah->power_level != conf->power_level)) { 211 ah->power_level = conf->power_level; 212 213 /* Half dB steps */ 214 ath5k_hw_set_txpower_limit(ah, (conf->power_level * 2)); 215 } And on each descriptor (it's ignored for now by hardware since we haven't enabled TPC): 723 ret = ah->ah_setup_tx_desc(ah, ds, pktlen, 724 ieee80211_get_hdrlen_from_skb(skb), padsize, 725 get_hw_packet_type(skb), 726 (ah->power_level * 2), 727 hw_rate, 728 info->control.rates[0].count, keyidx, ah->ah_tx_ant, flags, 729 cts_rate, duration); We use txp_cur_pwr when setting txpower level on ath5k_setup_rate_powertable 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; as I mentioned above, and in ath5k_hw_phy_init to preserve this setting across resets 3792 ret = ath5k_hw_txpower(ah, channel, ah->ah_txpower.txp_cur_pwr ? 3793 ah->ah_txpower.txp_cur_pwr / 2 : AR5K_TUNE_MAX_TXPOWER); To give you an idea why we already have 2 places to keep tx power setting, base.c/mac80211-ops.c should use power_level thats in 1db steps (and makes sense to someone reading the driver part and doesn't want to understand why we use 0.5 and 0.25db steps on the hw functions), and ah->ah_txpower.txp_cur_pwr on phy.c functions internally. I find this to be clean and simple and I don't see any problem with it, if I did some cleanup that would be maybe to change power_level to ah_tx_power_level and from int to s16 to match the rest but other than that I think it's O.K. the way it is.. > diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c > index 8c4c040..e831f69 100644 > --- a/drivers/net/wireless/ath/ath5k/base.c > +++ b/drivers/net/wireless/ath/ath5k/base.c > @@ -2951,6 +2951,9 @@ ath5k_init(struct ieee80211_hw *hw) > hw->queues = 1; > } > > + /* init tx_power setting to maximum */ > + ah->ah_txpower.txp_user_pwr = AR5K_TUNE_MAX_TXPOWER; > + > tasklet_init(&ah->rxtq, ath5k_tasklet_rx, (unsigned long)ah); > tasklet_init(&ah->txtq, ath5k_tasklet_tx, (unsigned long)ah); > tasklet_init(&ah->beacontq, ath5k_tasklet_beacon, (unsigned long)ah); No need to do that, if txpower is not initialized we use max power anyway on phy_init 3792 ret = ath5k_hw_txpower(ah, channel, ah->ah_txpower.txp_cur_pwr ? 3793 ah->ah_txpower.txp_cur_pwr / 2 : AR5K_TUNE_MAX_TXPOWER); > diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c > index 8b71a2d..728ff07 100644 > --- a/drivers/net/wireless/ath/ath5k/phy.c > +++ b/drivers/net/wireless/ath/ath5k/phy.c > @@ -3583,14 +3583,12 @@ ath5k_setup_rate_powertable(struct ath5k_hw *ah, u16 max_pwr, > * ath5k_hw_txpower() - Set transmission power limit for a given channel > * @ah: The &struct ath5k_hw > * @channel: The &struct ieee80211_channel > - * @txpower: Requested tx power in 0.5dB steps > * > * Combines all of the above to set the requested tx power limit > - * on hw. > + * on hw to ah->ah_txpower.txp_user_pwr. > */ > static int > -ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, > - u8 txpower) > +ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel) > { > struct ath5k_rate_pcal_info rate_info; > struct ieee80211_channel *curr_channel = ah->ah_current_channel; > @@ -3598,11 +3596,6 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, > u8 type; > int ret; > > - if (txpower > AR5K_TUNE_MAX_TXPOWER) { > - ATH5K_ERR(ah, "invalid tx power: %u\n", txpower); > - return -EINVAL; > - } > - > ee_mode = ath5k_eeprom_mode_from_channel(channel); > if (ee_mode < 0) { > ATH5K_ERR(ah, > @@ -3667,7 +3660,7 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, > ath5k_get_rate_pcal_data(ah, channel, &rate_info); > > /* Setup rate power table */ > - ath5k_setup_rate_powertable(ah, txpower, &rate_info, ee_mode); > + ath5k_setup_rate_powertable(ah, ah->ah_txpower.txp_user_pwr, &rate_info, ee_mode); > txpower here comes from above in 0.5db steps as the kerneldoc says, that's what ath5k_setup_rate_powertable expects, by passing ah->ah_txpower.txp_user_pwr directly to ath5k_setup_rate_powertable, it would result getting half the power requested because hw expects this to be an index to the channel powertable (check the implementation of ath5k_setup_rate_powertable) that uses 0.5db steps. > /* Write rate power table on hw */ > ath5k_hw_reg_write(ah, AR5K_TXPOWER_OFDM(3, 24) | > @@ -3717,8 +3710,16 @@ ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower) > { > ATH5K_DBG(ah, ATH5K_DEBUG_TXPOWER, > "changing txpower to %d\n", txpower); > + if (txpower) { > + ah->ah_txpower.txp_user_pwr = txpower; > + > + if (ah->ah_txpower.txp_user_pwr > AR5K_TUNE_MAX_TXPOWER) { > + ATH5K_ERR(ah, "invalid tx power: %u\n", ah->ah_txpower.txp_user_pwr); > + return -EINVAL; > + } > + } > Again this would result getting half the power requested, you should *2 this if you want it to be used directly by ath5k_setup_rate_powertable later. > - return ath5k_hw_txpower(ah, ah->ah_current_channel, txpower); > + return ath5k_hw_txpower(ah, ah->ah_current_channel); > } > > > @@ -3789,8 +3790,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); > + > if (ret) > return ret; > > -- > 1.7.11.1 > To summarize IMHO this patch doesn't fix anything and breaks current txpower setting so from my point of view it's a NACK. -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-25 19:22 ` Nick Kossifidis @ 2012-07-25 23:07 ` Thomas Huehn 2012-07-25 23:23 ` Nick Kossifidis 0 siblings, 1 reply; 30+ messages in thread From: Thomas Huehn @ 2012-07-25 23:07 UTC (permalink / raw) To: Nick Kossifidis Cc: linville, jirislaby, mcgrof, ath5k-devel, linux-wireless, johannes.berg, nbd Hi Nick, Nick Kossifidis schrieb: > 2012/7/23 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >> In such cases where phy_init() function got called, tx_power is always >> set to ah->ah_txpower.txp_cur_pwr which is never updated with txpower >> specified by the user instead it equale max chan power and got >> potentially incremented by ah_txpower.txp_offset. > > Never updated ? > > It gets updated when setting the rate powertable (the final step of > the whole process): > As it took a while that I got my head into the function calls, let me try to explain what is going wrong in the current state of ath5k regarding tx_power. In short: ah->ah_txpower.txp_cur_pwr is probably intended to respect the power level set by the user, BUT it is jut reused and changed in different places that make it difficult to follow and brake the tx_power level specified. -long-version-in-code: - assume you configure your device to use 17 dBm and max power is 19dBm and you are just in the boot process -in the last steps to init you Atheros Wifi, the mac802.11 calls hw_config, as it recognise a change in tx-power from maximum(19dBm) to user_txpower(17dBm) -this leads in case of ath5k to call the ops representative function ath5k_config... which transforms 1 dBm steps in 2 half-dBm steps ... concrete: ath5k_hw_set_txpower_limit(..34 half-dBm..) is called which calls ath5k_hw_txpower(..34 half-dBm..) .. and in here .. just before writing the power to registers.. it calls ath5k_setup_rate_powertable(..34 half-dBm..) - now lets jump into ath5k_setup_rate_powertable(..34 half-dBm..): our 34 half-dBm is now transformed to 68 quarter-dBm stored in max_pwr. max_pwr is re-assigned as to be the minimum of (our user_txpower 68 quarter_dBm and ah->ah_txpower.txp_max_pwr) / 2 ... so it is set back to 34 half_dBm -one step further: rates[0..5] = min(max_pwr (34 half dBm), rate_info->target_power_6to24 (38 half-dBm); -now it gets interesting: for i [1..16] rates[i] = rates[i] + OFFSET (line 3560) { .. and OFFSET is in my case 18 .. so rates[0] = 34+21 = 55 } ah->ah_txpower.txp_cur_pwr = 2 * rates[0] = 2 * 55 half_dBm = 110 half_dBm +BREAK: ah->ah_txpower.txp_cur_pwr is at 110 half_dBm but not yet used. +in ALL cases where ath5k_hw_phy_init() is called ... queue reset, wifi up/down, iwconfig set same txpower .. we reuse this wrong set txpower variable which does not represent the user setting -now our ah->ah_txpower.txp_cur_pwr is just reused in the function ath5k_hw_phy_init(): ret = ath5k_hw_txpower(ah, channel, ah->ah_txpower.txp_cur_pwr (110 half_dBm)? ah->ah_txpower.txp_cur_pwr / 2 (55 half_dBm): AR5K_TUNE_MAX_TXPOWER (63 half_dBm)); .. which leads to the call ath5k_hw_txpower(..AR5K_TUNE_MAX_TXPOWER (63 half-dBm)..) .. to its end ... txpower is changed from 17 dBm to the maximum. So the OFFSET summation is the cause of a wrong ah->ah_txpower.txp_cur_pwr usage. My path fixes exactly that and keeps ah->ah_txpower.txp_cur_pwr unused for Felix :) Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-25 23:07 ` Thomas Huehn @ 2012-07-25 23:23 ` Nick Kossifidis 2012-07-25 23:40 ` [ath5k-devel] " Thomas Huehn 0 siblings, 1 reply; 30+ messages in thread From: Nick Kossifidis @ 2012-07-25 23:23 UTC (permalink / raw) To: Thomas Huehn Cc: linville, jirislaby, mcgrof, ath5k-devel, linux-wireless, johannes.berg, nbd 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi Nick, > > Nick Kossifidis schrieb: > >> 2012/7/23 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>> In such cases where phy_init() function got called, tx_power is always >>> set to ah->ah_txpower.txp_cur_pwr which is never updated with txpower >>> specified by the user instead it equale max chan power and got >>> potentially incremented by ah_txpower.txp_offset. >> >> Never updated ? >> >> It gets updated when setting the rate powertable (the final step of >> the whole process): >> > > > As it took a while that I got my head into the function calls, let me > try to explain what is going wrong in the current state of ath5k > regarding tx_power. > > In short: ah->ah_txpower.txp_cur_pwr is probably intended to respect the > power level set by the user, BUT it is jut reused and changed in > different places that make it difficult to follow and brake the tx_power > level specified. > > -long-version-in-code: > > - assume you configure your device to use 17 dBm and max power is 19dBm > and you are just in the boot process > > -in the last steps to init you Atheros Wifi, the mac802.11 calls > hw_config, as it recognise a change in tx-power from maximum(19dBm) to > user_txpower(17dBm) > > -this leads in case of ath5k to call the ops representative function > ath5k_config... which transforms 1 dBm steps in 2 half-dBm steps ... > concrete: ath5k_hw_set_txpower_limit(..34 half-dBm..) is called which > calls ath5k_hw_txpower(..34 half-dBm..) .. and in here .. just before > writing the power to registers.. it calls > ath5k_setup_rate_powertable(..34 half-dBm..) > > - now lets jump into ath5k_setup_rate_powertable(..34 half-dBm..): > our 34 half-dBm is now transformed to 68 quarter-dBm stored in max_pwr. > max_pwr is re-assigned as to be the minimum of (our user_txpower 68 > quarter_dBm and ah->ah_txpower.txp_max_pwr) / 2 ... so it is set back to > 34 half_dBm > > -one step further: > rates[0..5] = min(max_pwr (34 half dBm), rate_info->target_power_6to24 > (38 half-dBm); > > -now it gets interesting: > for i [1..16] rates[i] = rates[i] + OFFSET (line 3560) > { > .. and OFFSET is in my case 18 .. so rates[0] = 34+21 = 55 > } > ah->ah_txpower.txp_cur_pwr = 2 * rates[0] = 2 * 55 half_dBm = 110 half_dBm > > +BREAK: ah->ah_txpower.txp_cur_pwr is at 110 half_dBm but not yet used. > +in ALL cases where ath5k_hw_phy_init() is called ... queue reset, wifi > up/down, iwconfig set same txpower .. we reuse this wrong set txpower > variable which does not represent the user setting > > -now our ah->ah_txpower.txp_cur_pwr is just reused in the function > ath5k_hw_phy_init(): > ret = ath5k_hw_txpower(ah, channel, ah->ah_txpower.txp_cur_pwr (110 > half_dBm)? > ah->ah_txpower.txp_cur_pwr / 2 (55 half_dBm): AR5K_TUNE_MAX_TXPOWER (63 > half_dBm)); > > .. which leads to the call ath5k_hw_txpower(..AR5K_TUNE_MAX_TXPOWER (63 > half-dBm)..) > > .. to its end ... txpower is changed from 17 dBm to the maximum. So the > OFFSET summation is the cause of a wrong ah->ah_txpower.txp_cur_pwr > usage. My path fixes exactly that and keeps ah->ah_txpower.txp_cur_pwr > unused for Felix :) > > Greetings Thomas > > There is nothing in your patch that suggests that's related to this. Anyway there's a simple way to fix this: Just move this: 3575 /* Min/max in 0.25dB units */ 3576 ah->ah_txpower.txp_min_pwr = 2 * rates[7]; 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; 3578 ah->ah_txpower.txp_ofdm = rates[7]; above the for loop and you are done. Note rates[i] don't hold tx power values, they hold indices to the channel powertable. -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-25 23:23 ` Nick Kossifidis @ 2012-07-25 23:40 ` Thomas Huehn 2012-07-26 10:20 ` Nick Kossifidis 0 siblings, 1 reply; 30+ messages in thread From: Thomas Huehn @ 2012-07-25 23:40 UTC (permalink / raw) To: Nick Kossifidis Cc: jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville, nbd Hi Nick, Nick Kossifidis schrieb: > 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > There is nothing in your patch that suggests that's related to this. > Anyway there's a simple way to fix this: > > Just move this: > > 3575 /* Min/max in 0.25dB units */ > 3576 ah->ah_txpower.txp_min_pwr = 2 * rates[7]; > 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; > 3578 ah->ah_txpower.txp_ofdm = rates[7]; > > above the for loop and you are done. > > Note rates[i] don't hold tx power values, they hold indices to the > channel powertable. > Are you agreeing that current ath5k txpower handling set from user space is not working and we need to fix it ? Beside that, what about a channel switch and wrongly re-use txp_cur_pwr on a channel witch other max power levels ? Lets wait on Felix opinion what he has in mind. Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-25 23:40 ` [ath5k-devel] " Thomas Huehn @ 2012-07-26 10:20 ` Nick Kossifidis 2012-07-26 10:28 ` Felix Fietkau 0 siblings, 1 reply; 30+ messages in thread From: Nick Kossifidis @ 2012-07-26 10:20 UTC (permalink / raw) To: Thomas Huehn Cc: jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville, nbd 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi Nick, > > Nick Kossifidis schrieb: > >> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > >> There is nothing in your patch that suggests that's related to this. >> Anyway there's a simple way to fix this: >> >> Just move this: >> >> 3575 /* Min/max in 0.25dB units */ >> 3576 ah->ah_txpower.txp_min_pwr = 2 * rates[7]; >> 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; >> 3578 ah->ah_txpower.txp_ofdm = rates[7]; >> >> above the for loop and you are done. >> >> Note rates[i] don't hold tx power values, they hold indices to the >> channel powertable. >> > > > Are you agreeing that current ath5k txpower handling set from user space > is not working and we need to fix it ? Is that a rhetorical question ? Just check out the TODOs on wireless.kernel.org. The current code does not preserve the tx power value across resets, thats the problem and the change I mentioned above fixes it (patch on the way, it's just that where I am right now I don't have bw to download wireless-testing) but other than that when we set tx power without reseting at least it does what it's supposed to do (and the result is the same with madwifi, using a spectrum analyzer or another client/monitor interface you see some power levels supported or only the max txpower supported, it's really up to the vendor, not all of them follow Atheros's reference designs). Your patch passes 1dbm units on a function that expects 0.5dbm units, you fix the problem with preserving tx power but you break the tx power setting. The change I mentioned above fixes the problem without introducing new variables just because "Felix will use the other one", I don't understand why you have a problem with that and why you think I don't want this to get fixed... > Beside that, what about a channel switch and wrongly re-use txp_cur_pwr > on a channel witch other max power levels ? That won't be a problem, when channel changes we 'll call reset -> phy_init -> set_txpower -> (calibration) -> set rate target power and then it's going to get limited before it's written on hw here: max_pwr = min(max_pwr, (u16) ah->ah_txpower.txp_max_pwr) / 2; txp_max_pwr is initialized on calibration (the max power for this channel), then it gets limited by CTL edge information on EEPROM, then by max_pwr and then max_pwr is limited by rate_info->target_power_X from EEPROM to create rates[i]. We write rates[i] on hw, not txp_cur_pwr. > Lets wait on Felix opinion what he has in mind. > > Greetings Thomas I 'd rather wait for Felix's patches instead to understand what is he up to. -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-26 10:20 ` Nick Kossifidis @ 2012-07-26 10:28 ` Felix Fietkau 2012-07-26 10:31 ` Nick Kossifidis 0 siblings, 1 reply; 30+ messages in thread From: Felix Fietkau @ 2012-07-26 10:28 UTC (permalink / raw) To: Nick Kossifidis Cc: Thomas Huehn, jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville On 2012-07-26 12:20 PM, Nick Kossifidis wrote: > 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >> Hi Nick, >> >> Nick Kossifidis schrieb: >> >>> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >> >>> There is nothing in your patch that suggests that's related to this. >>> Anyway there's a simple way to fix this: >>> >>> Just move this: >>> >>> 3575 /* Min/max in 0.25dB units */ >>> 3576 ah->ah_txpower.txp_min_pwr = 2 * rates[7]; >>> 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; >>> 3578 ah->ah_txpower.txp_ofdm = rates[7]; >>> >>> above the for loop and you are done. >>> >>> Note rates[i] don't hold tx power values, they hold indices to the >>> channel powertable. >>> >> >> >> Are you agreeing that current ath5k txpower handling set from user space >> is not working and we need to fix it ? > > Is that a rhetorical question ? Just check out the TODOs on wireless.kernel.org. > > The current code does not preserve the tx power value across resets, > thats the problem and the change I mentioned above fixes it (patch on > the way, it's just that where I am right now I don't have bw to > download wireless-testing) but other than that when we set tx power > without reseting at least it does what it's supposed to do (and the > result is the same with madwifi, using a spectrum analyzer or another > client/monitor interface you see some power levels supported or only > the max txpower supported, it's really up to the vendor, not all of > them follow Atheros's reference designs). Your patch passes 1dbm units > on a function that expects 0.5dbm units, you fix the problem with > preserving tx power but you break the tx power setting. > > The change I mentioned above fixes the problem without introducing new > variables just because "Felix will use the other one", I don't > understand why you have a problem with that and why you think I don't > want this to get fixed... > >> Beside that, what about a channel switch and wrongly re-use txp_cur_pwr >> on a channel witch other max power levels ? > > That won't be a problem, when channel changes we 'll call > > reset -> phy_init -> set_txpower -> (calibration) -> set rate target power > > and then it's going to get limited before it's written on hw here: > > max_pwr = min(max_pwr, (u16) ah->ah_txpower.txp_max_pwr) / 2; > > txp_max_pwr is initialized on calibration (the max power for this > channel), then it gets limited by CTL edge information on EEPROM, then > by max_pwr and then max_pwr is limited by rate_info->target_power_X > from EEPROM to create rates[i]. We write rates[i] on hw, not > txp_cur_pwr. I think it's a bad idea to store the user's choice of txpower in a variable that internally gets reused to store the hw limit. Even when the offset isn't added to it, it's still fragile. A problem with this is that different channels have different max power values, so if you switch to a channel with a lower power, and then switch back (without explicitly changing txpower inbetween), don't you then end up with less power than you configured? This can be easily avoided by storing the user's txpower choice separately from the actual hw limit... - Felix ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-26 10:28 ` Felix Fietkau @ 2012-07-26 10:31 ` Nick Kossifidis 2012-07-26 10:41 ` Felix Fietkau 0 siblings, 1 reply; 30+ messages in thread From: Nick Kossifidis @ 2012-07-26 10:31 UTC (permalink / raw) To: Felix Fietkau Cc: Thomas Huehn, jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville 2012/7/26 Felix Fietkau <nbd@openwrt.org>: > On 2012-07-26 12:20 PM, Nick Kossifidis wrote: >> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>> Hi Nick, >>> >>> Nick Kossifidis schrieb: >>> >>>> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>> >>>> There is nothing in your patch that suggests that's related to this. >>>> Anyway there's a simple way to fix this: >>>> >>>> Just move this: >>>> >>>> 3575 /* Min/max in 0.25dB units */ >>>> 3576 ah->ah_txpower.txp_min_pwr = 2 * rates[7]; >>>> 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; >>>> 3578 ah->ah_txpower.txp_ofdm = rates[7]; >>>> >>>> above the for loop and you are done. >>>> >>>> Note rates[i] don't hold tx power values, they hold indices to the >>>> channel powertable. >>>> >>> >>> >>> Are you agreeing that current ath5k txpower handling set from user space >>> is not working and we need to fix it ? >> >> Is that a rhetorical question ? Just check out the TODOs on wireless.kernel.org. >> >> The current code does not preserve the tx power value across resets, >> thats the problem and the change I mentioned above fixes it (patch on >> the way, it's just that where I am right now I don't have bw to >> download wireless-testing) but other than that when we set tx power >> without reseting at least it does what it's supposed to do (and the >> result is the same with madwifi, using a spectrum analyzer or another >> client/monitor interface you see some power levels supported or only >> the max txpower supported, it's really up to the vendor, not all of >> them follow Atheros's reference designs). Your patch passes 1dbm units >> on a function that expects 0.5dbm units, you fix the problem with >> preserving tx power but you break the tx power setting. >> >> The change I mentioned above fixes the problem without introducing new >> variables just because "Felix will use the other one", I don't >> understand why you have a problem with that and why you think I don't >> want this to get fixed... >> >>> Beside that, what about a channel switch and wrongly re-use txp_cur_pwr >>> on a channel witch other max power levels ? >> >> That won't be a problem, when channel changes we 'll call >> >> reset -> phy_init -> set_txpower -> (calibration) -> set rate target power >> >> and then it's going to get limited before it's written on hw here: >> >> max_pwr = min(max_pwr, (u16) ah->ah_txpower.txp_max_pwr) / 2; >> >> txp_max_pwr is initialized on calibration (the max power for this >> channel), then it gets limited by CTL edge information on EEPROM, then >> by max_pwr and then max_pwr is limited by rate_info->target_power_X >> from EEPROM to create rates[i]. We write rates[i] on hw, not >> txp_cur_pwr. > I think it's a bad idea to store the user's choice of txpower in a > variable that internally gets reused to store the hw limit. Even when > the offset isn't added to it, it's still fragile. > > A problem with this is that different channels have different max power > values, so if you switch to a channel with a lower power, and then > switch back (without explicitly changing txpower inbetween), don't you > then end up with less power than you configured? > > This can be easily avoided by storing the user's txpower choice > separately from the actual hw limit... > > - Felix That's what ah->power_level already does, what's the point of introducing another one. Do you think power_level is baldy used/implemented ? -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-26 10:31 ` Nick Kossifidis @ 2012-07-26 10:41 ` Felix Fietkau 2012-07-26 17:48 ` Nick Kossifidis 0 siblings, 1 reply; 30+ messages in thread From: Felix Fietkau @ 2012-07-26 10:41 UTC (permalink / raw) To: Nick Kossifidis Cc: Thomas Huehn, jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville On 2012-07-26 12:31 PM, Nick Kossifidis wrote: > 2012/7/26 Felix Fietkau <nbd@openwrt.org>: >> On 2012-07-26 12:20 PM, Nick Kossifidis wrote: >>> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>>> Hi Nick, >>>> >>>> Nick Kossifidis schrieb: >>>> >>>>> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>>> >>>>> There is nothing in your patch that suggests that's related to this. >>>>> Anyway there's a simple way to fix this: >>>>> >>>>> Just move this: >>>>> >>>>> 3575 /* Min/max in 0.25dB units */ >>>>> 3576 ah->ah_txpower.txp_min_pwr = 2 * rates[7]; >>>>> 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; >>>>> 3578 ah->ah_txpower.txp_ofdm = rates[7]; >>>>> >>>>> above the for loop and you are done. >>>>> >>>>> Note rates[i] don't hold tx power values, they hold indices to the >>>>> channel powertable. >>>>> >>>> >>>> >>>> Are you agreeing that current ath5k txpower handling set from user space >>>> is not working and we need to fix it ? >>> >>> Is that a rhetorical question ? Just check out the TODOs on wireless.kernel.org. >>> >>> The current code does not preserve the tx power value across resets, >>> thats the problem and the change I mentioned above fixes it (patch on >>> the way, it's just that where I am right now I don't have bw to >>> download wireless-testing) but other than that when we set tx power >>> without reseting at least it does what it's supposed to do (and the >>> result is the same with madwifi, using a spectrum analyzer or another >>> client/monitor interface you see some power levels supported or only >>> the max txpower supported, it's really up to the vendor, not all of >>> them follow Atheros's reference designs). Your patch passes 1dbm units >>> on a function that expects 0.5dbm units, you fix the problem with >>> preserving tx power but you break the tx power setting. >>> >>> The change I mentioned above fixes the problem without introducing new >>> variables just because "Felix will use the other one", I don't >>> understand why you have a problem with that and why you think I don't >>> want this to get fixed... >>> >>>> Beside that, what about a channel switch and wrongly re-use txp_cur_pwr >>>> on a channel witch other max power levels ? >>> >>> That won't be a problem, when channel changes we 'll call >>> >>> reset -> phy_init -> set_txpower -> (calibration) -> set rate target power >>> >>> and then it's going to get limited before it's written on hw here: >>> >>> max_pwr = min(max_pwr, (u16) ah->ah_txpower.txp_max_pwr) / 2; >>> >>> txp_max_pwr is initialized on calibration (the max power for this >>> channel), then it gets limited by CTL edge information on EEPROM, then >>> by max_pwr and then max_pwr is limited by rate_info->target_power_X >>> from EEPROM to create rates[i]. We write rates[i] on hw, not >>> txp_cur_pwr. >> I think it's a bad idea to store the user's choice of txpower in a >> variable that internally gets reused to store the hw limit. Even when >> the offset isn't added to it, it's still fragile. >> >> A problem with this is that different channels have different max power >> values, so if you switch to a channel with a lower power, and then >> switch back (without explicitly changing txpower inbetween), don't you >> then end up with less power than you configured? >> >> This can be easily avoided by storing the user's txpower choice >> separately from the actual hw limit... >> >> - Felix > > That's what ah->power_level already does, what's the point of > introducing another one. Do you think power_level is baldy > used/implemented ? Unless I'm missing something here, it does not seem to get used at reset time, only when mac80211 requests a txpower change (and for TPC values in descriptors). On every reset, ah->ah_txpower.txp_cur_pwr gets reused as configured tx power value (and subsequently clamped to the hw channel power limit). I guess this patch could be simplified to use ah->power_level in ath5k_hw_phy_init instead of ah->ah_txpower.txp_cur_pwr, but I do see an issue in the code as it is without this patch. - Felix ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-26 10:41 ` Felix Fietkau @ 2012-07-26 17:48 ` Nick Kossifidis 2012-07-26 20:56 ` Thomas Huehn 0 siblings, 1 reply; 30+ messages in thread From: Nick Kossifidis @ 2012-07-26 17:48 UTC (permalink / raw) To: Felix Fietkau Cc: Thomas Huehn, jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville 2012/7/26 Felix Fietkau <nbd@openwrt.org>: > On 2012-07-26 12:31 PM, Nick Kossifidis wrote: >> 2012/7/26 Felix Fietkau <nbd@openwrt.org>: >>> On 2012-07-26 12:20 PM, Nick Kossifidis wrote: >>>> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>>>> Hi Nick, >>>>> >>>>> Nick Kossifidis schrieb: >>>>> >>>>>> 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: >>>>> >>>>>> There is nothing in your patch that suggests that's related to this. >>>>>> Anyway there's a simple way to fix this: >>>>>> >>>>>> Just move this: >>>>>> >>>>>> 3575 /* Min/max in 0.25dB units */ >>>>>> 3576 ah->ah_txpower.txp_min_pwr = 2 * rates[7]; >>>>>> 3577 ah->ah_txpower.txp_cur_pwr = 2 * rates[0]; >>>>>> 3578 ah->ah_txpower.txp_ofdm = rates[7]; >>>>>> >>>>>> above the for loop and you are done. >>>>>> >>>>>> Note rates[i] don't hold tx power values, they hold indices to the >>>>>> channel powertable. >>>>>> >>>>> >>>>> >>>>> Are you agreeing that current ath5k txpower handling set from user space >>>>> is not working and we need to fix it ? >>>> >>>> Is that a rhetorical question ? Just check out the TODOs on wireless.kernel.org. >>>> >>>> The current code does not preserve the tx power value across resets, >>>> thats the problem and the change I mentioned above fixes it (patch on >>>> the way, it's just that where I am right now I don't have bw to >>>> download wireless-testing) but other than that when we set tx power >>>> without reseting at least it does what it's supposed to do (and the >>>> result is the same with madwifi, using a spectrum analyzer or another >>>> client/monitor interface you see some power levels supported or only >>>> the max txpower supported, it's really up to the vendor, not all of >>>> them follow Atheros's reference designs). Your patch passes 1dbm units >>>> on a function that expects 0.5dbm units, you fix the problem with >>>> preserving tx power but you break the tx power setting. >>>> >>>> The change I mentioned above fixes the problem without introducing new >>>> variables just because "Felix will use the other one", I don't >>>> understand why you have a problem with that and why you think I don't >>>> want this to get fixed... >>>> >>>>> Beside that, what about a channel switch and wrongly re-use txp_cur_pwr >>>>> on a channel witch other max power levels ? >>>> >>>> That won't be a problem, when channel changes we 'll call >>>> >>>> reset -> phy_init -> set_txpower -> (calibration) -> set rate target power >>>> >>>> and then it's going to get limited before it's written on hw here: >>>> >>>> max_pwr = min(max_pwr, (u16) ah->ah_txpower.txp_max_pwr) / 2; >>>> >>>> txp_max_pwr is initialized on calibration (the max power for this >>>> channel), then it gets limited by CTL edge information on EEPROM, then >>>> by max_pwr and then max_pwr is limited by rate_info->target_power_X >>>> from EEPROM to create rates[i]. We write rates[i] on hw, not >>>> txp_cur_pwr. >>> I think it's a bad idea to store the user's choice of txpower in a >>> variable that internally gets reused to store the hw limit. Even when >>> the offset isn't added to it, it's still fragile. >>> >>> A problem with this is that different channels have different max power >>> values, so if you switch to a channel with a lower power, and then >>> switch back (without explicitly changing txpower inbetween), don't you >>> then end up with less power than you configured? >>> >>> This can be easily avoided by storing the user's txpower choice >>> separately from the actual hw limit... >>> >>> - Felix >> >> That's what ah->power_level already does, what's the point of >> introducing another one. Do you think power_level is baldy >> used/implemented ? > Unless I'm missing something here, it does not seem to get used at reset > time, only when mac80211 requests a txpower change (and for TPC values > in descriptors). > On every reset, ah->ah_txpower.txp_cur_pwr gets reused as configured tx > power value (and subsequently clamped to the hw channel power limit). > I guess this patch could be simplified to use ah->power_level in > ath5k_hw_phy_init instead of ah->ah_txpower.txp_cur_pwr, but I do see an > issue in the code as it is without this patch. > > - Felix > I think this is a better approach (I'll prepare a proper patch as soon as I have some bandwidth to work with wireless-testing, maybe tomorrow)... --- old/phy.c 2012-07-26 20:40:00.869150187 +0300 +++ new/phy.c 2012-07-26 20:43:25.074710577 +0300 @@ -3562,6 +3562,12 @@ for (i = 8; i <= 15; i++) rates[i] -= ah->ah_txpower.txp_cck_ofdm_gainf_delta; + + /* 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]; + /* 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 +3577,6 @@ 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]; } @@ -3789,8 +3790,8 @@ * 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; Works for you ? BTW is there a way to pass the actual tx power set back to mac80211/cfg80211 so that user knows what power his card is actually transmitting at ? -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-26 17:48 ` Nick Kossifidis @ 2012-07-26 20:56 ` Thomas Huehn 2012-07-28 11:45 ` Nick Kossifidis 0 siblings, 1 reply; 30+ messages in thread From: Thomas Huehn @ 2012-07-26 20:56 UTC (permalink / raw) To: Nick Kossifidis Cc: Felix Fietkau, jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville Hi Nick, Nick Kossifidis schrieb: > I think this is a better approach (I'll prepare a proper patch as soon > as I have some bandwidth to work with wireless-testing, maybe > tomorrow)... > > --- old/phy.c 2012-07-26 20:40:00.869150187 +0300 > +++ new/phy.c 2012-07-26 20:43:25.074710577 +0300 > @@ -3562,6 +3562,12 @@ > for (i = 8; i <= 15; i++) > rates[i] -= ah->ah_txpower.txp_cck_ofdm_gainf_delta; > > + > + /* 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]; > + > /* 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 +3577,6 @@ > 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]; > } > > @@ -3789,8 +3790,8 @@ > * 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; > > Works for you ? works as well. There are now 2 unused variables as left over: ah->ah_txpower.txp_cur_pwr ah->ah_txpower.txp_min_pwr Do we need them anymore to check for hw chan limit ? > BTW is there a way to pass the actual tx power set back to > mac80211/cfg80211 so that user knows what power his card is actually > transmitting at ? I think that on point of the todo list. Greetings Thomas ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [ath5k-devel] [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes 2012-07-26 20:56 ` Thomas Huehn @ 2012-07-28 11:45 ` Nick Kossifidis 0 siblings, 0 replies; 30+ messages in thread From: Nick Kossifidis @ 2012-07-28 11:45 UTC (permalink / raw) To: Thomas Huehn Cc: Felix Fietkau, jirislaby, johannes.berg, ath5k-devel, linux-wireless, linville 2012/7/26 Thomas Huehn <thomas@net.t-labs.tu-berlin.de>: > Hi Nick, > > Nick Kossifidis schrieb: > >> I think this is a better approach (I'll prepare a proper patch as soon >> as I have some bandwidth to work with wireless-testing, maybe >> tomorrow)... >> >> --- old/phy.c 2012-07-26 20:40:00.869150187 +0300 >> +++ new/phy.c 2012-07-26 20:43:25.074710577 +0300 >> @@ -3562,6 +3562,12 @@ >> for (i = 8; i <= 15; i++) >> rates[i] -= ah->ah_txpower.txp_cck_ofdm_gainf_delta; >> >> + >> + /* 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]; >> + >> /* 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 +3577,6 @@ >> 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]; >> } >> > >> @@ -3789,8 +3790,8 @@ >> * 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; >> >> Works for you ? > > works as well. > > There are now 2 unused variables as left over: > ah->ah_txpower.txp_cur_pwr > ah->ah_txpower.txp_min_pwr > > Do we need them anymore to check for hw chan limit ? > >> BTW is there a way to pass the actual tx power set back to >> mac80211/cfg80211 so that user knows what power his card is actually >> transmitting at ? > > > I think that on point of the todo list. > That's what txp_cur_pwr will be used for, min_pwr is left there. I'll do a cleanup too sometime :-) -- GPG ID: 0xEE878588 As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2012-08-05 18:26 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-23 16:01 [PATCH 0/2] ath5k: fixing broken power gain calibration at 5GHz and txpower handling Thomas Huehn 2012-07-23 16:01 ` [PATCH 1/2] ath5k: fix wrong per rate target power eeprom reads for AR5K_EEPROM_MODE_11A Thomas Huehn 2012-07-25 18:42 ` Nick Kossifidis 2012-07-25 18:55 ` Felix Fietkau 2012-07-25 22:22 ` Nick Kossifidis 2012-07-25 22:01 ` [ath5k-devel] " Thomas Huehn 2012-07-25 22:31 ` Nick Kossifidis 2012-08-04 8:14 ` Thomas Huehn 2012-08-04 15:28 ` Nick Kossifidis 2012-08-05 11:06 ` Thomas Huehn 2012-08-05 11:56 ` Nick Kossifidis 2012-08-05 12:37 ` Thomas Huehn 2012-08-05 12:59 ` Nick Kossifidis 2012-08-05 18:26 ` Nick Kossifidis 2012-07-23 16:01 ` [PATCH 2/2] ath5k: fix phy_init() to respect user txpower changes Thomas Huehn 2012-07-23 16:42 ` [ath5k-devel] " Bob Copeland 2012-07-23 18:25 ` Thomas Huehn 2012-07-23 18:29 ` Thomas Huehn 2012-07-23 19:20 ` Bob Copeland 2012-07-25 19:22 ` Nick Kossifidis 2012-07-25 23:07 ` Thomas Huehn 2012-07-25 23:23 ` Nick Kossifidis 2012-07-25 23:40 ` [ath5k-devel] " Thomas Huehn 2012-07-26 10:20 ` Nick Kossifidis 2012-07-26 10:28 ` Felix Fietkau 2012-07-26 10:31 ` Nick Kossifidis 2012-07-26 10:41 ` Felix Fietkau 2012-07-26 17:48 ` Nick Kossifidis 2012-07-26 20:56 ` Thomas Huehn 2012-07-28 11:45 ` Nick Kossifidis
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.