2007-11-23 10:51:55

by Bruno Randolf

[permalink] [raw]
Subject: ath5k signal/noise fixes


hello!

the next 3 patches contain fixes for the signal/noise level calibration and
reporting of ath5k. i split them up because i'm not 100% sure of the second
two.

1) fixes the reading of the noise floor register and puts it into one function.

2) also enables noise floor calibration for rf511x.

3) saves the read noise floor and exports signal/noise and "quality" to
mac80211/wext.

i'm also not sure if what we call noise calibration in this case really is an
calibration or just a reading of the values?

is ath5k_hw the right place to store the noise_floor?

the values reported, especially the noise seems a bit high, it's around -77 in
the A bands and around -92 in G bands (thats with an AR5215/RF5111/2111), so we
might have some more tweaking to do. since i add the RSSI to get the signal
quality in dBm it seems a bit too high too.

this comment from madwifi leads me to believe this is the correct thing to do:

* rx_rssi is in units of dbm above the noise floor. This value
* is measured during the preamble and PLCP; i.e. with the initial
* 4us of detection. The noise floor is typically a consistent
* -96dBm absolute power in a 20MHz channel

at least we have some values to play with now :)

ah, and they are based on my last debugging and logging, patches (although
there are not huge dependencies).

bruno


2007-11-23 11:23:50

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath5k: fix noise floor calibration

2007/11/23, Bruno Randolf <[email protected]>:
> fix noise floor calibration by applying AR5K_PHY_NF_AVAL after
> AR5K_PHY_NF_RVAL. that way the XORed mask and value have the same length and we
> get a reasonable noise value in -dBm.
>
> move duplicate noise floor calibration code from two different places into one
> function ath5k_hw_noise_floor_calibration().
>
> the check for accepted noise_floor values (<= AR5K_TUNE_NOISE_FLOOR) should
> only happen when we have an active reading and a converted value, so move it up
> into the first if.
>
> Changes-licensed-under: ISC
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
> drivers/net/wireless/ath5k/ath5k.h | 1 +
> drivers/net/wireless/ath5k/hw.c | 36 +---------------
> drivers/net/wireless/ath5k/phy.c | 79 +++++++++++++++++++++--------------
> 3 files changed, 51 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
> index 1b8ddd9..5bcbbf9 100644
> --- a/drivers/net/wireless/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath5k/ath5k.h
> @@ -1127,6 +1127,7 @@ extern int ath5k_hw_phy_disable(struct ath5k_hw *ah);
> extern u16 ath5k_hw_radio_revision(struct ath5k_hw *ah, unsigned int chan);
> extern void ath5k_hw_set_def_antenna(struct ath5k_hw *ah, unsigned int ant);
> extern unsigned int ath5k_hw_get_def_antenna(struct ath5k_hw *ah);
> +extern int ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq);
> /* TX power setup */
> extern int ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, unsigned int txpower);
> extern int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, unsigned int power);
> diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
> index 2a826a7..b42fc8f 100644
> --- a/drivers/net/wireless/ath5k/hw.c
> +++ b/drivers/net/wireless/ath5k/hw.c
> @@ -593,7 +593,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
> {
> struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
> u32 data, s_seq, s_ant, s_led[3];
> - s32 noise_floor;
> unsigned int i, mode, freq, ee_mode, ant[2], driver_mode = -1;
> int ret;
>
> @@ -916,38 +915,9 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
> return -EAGAIN;
> }
>
> - /*
> - * Enable noise floor calibration and wait until completion
> - */
> - AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL,
> - AR5K_PHY_AGCCTL_NF);
> -
> - if (ath5k_hw_register_timeout(ah, AR5K_PHY_AGCCTL,
> - AR5K_PHY_AGCCTL_NF, 0, false)) {
> - ATH5K_ERR(ah->ah_sc,
> - "noise floor calibration timeout (%uMHz)\n",
> - channel->freq);
> - return -EAGAIN;
> - }
> -
> - /* Wait until the noise floor is calibrated and read the value */
> - for (i = 20; i > 0; i--) {
> - mdelay(1);
> - noise_floor = ath5k_hw_reg_read(ah, AR5K_PHY_NF);
> -
> - if (AR5K_PHY_NF_RVAL(noise_floor) & AR5K_PHY_NF_ACTIVE)
> - noise_floor = AR5K_PHY_NF_AVAL(noise_floor);
> -
> - if (noise_floor <= AR5K_TUNE_NOISE_FLOOR)
> - break;
> - }
> -
> - if (noise_floor > AR5K_TUNE_NOISE_FLOOR) {
> - ATH5K_ERR(ah->ah_sc,
> - "noise floor calibration failed (%uMHz)\n",
> - channel->freq);
> - return -EIO;
> - }
> + ret = ath5k_hw_noise_floor_calibration(ah, channel->freq);
> + if (ret)
> + return ret;
>
> ah->ah_calibration = false;
>
> diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
> index 3c2a67c..9b91121 100644
> --- a/drivers/net/wireless/ath5k/phy.c
> +++ b/drivers/net/wireless/ath5k/phy.c
> @@ -1519,6 +1519,51 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel)
> /*****************\
> PHY calibration
> \*****************/
> +int
> +ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq)
> +{
> + int ret;
> + unsigned int i;
> + s32 noise_floor;
> +
> + /*
> + * Enable noise floor calibration and wait until completion
> + */
> + AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL,
> + AR5K_PHY_AGCCTL_NF);
> +
> + ret = ath5k_hw_register_timeout(ah, AR5K_PHY_AGCCTL,
> + AR5K_PHY_AGCCTL_NF, 0, false);
> + if (ret) {
> + ATH5K_ERR(ah->ah_sc,
> + "noise floor calibration timeout (%uMHz)\n", freq);
> + return ret;
> + }
> +
> + /* Wait until the noise floor is calibrated and read the value */
> + for (i = 20; i > 0; i--) {
> + mdelay(1);
> + noise_floor = ath5k_hw_reg_read(ah, AR5K_PHY_NF);
> + noise_floor = AR5K_PHY_NF_RVAL(noise_floor);
> + if (noise_floor & AR5K_PHY_NF_ACTIVE) {
> + noise_floor = AR5K_PHY_NF_AVAL(noise_floor);
> +
> + if (noise_floor <= AR5K_TUNE_NOISE_FLOOR)
> + break;
> + }
> + }
> +
> + ATH5K_DBG(ah->ah_sc, ATH5K_DEBUG_CALIBRATE,
> + "noise floor %d, %d\n", noise_floor, AR5K_TUNE_NOISE_FLOOR);
> +
> + if (noise_floor > AR5K_TUNE_NOISE_FLOOR) {
> + ATH5K_ERR(ah->ah_sc,
> + "noise floor calibration failed (%uMHz)\n", freq);
> + return -EIO;
> + }
> +
> + return 0;
> +}
>
> /*
> * Perform a PHY calibration on RF5110
> @@ -1529,8 +1574,6 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
> struct ieee80211_channel *channel)
> {
> u32 phy_sig, phy_agc, phy_sat, beacon;
> - s32 noise_floor;
> - unsigned int i;
> int ret;
>
> /*
> @@ -1612,37 +1655,9 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
> return ret;
> }
>
> - /*
> - * Enable noise floor calibration and wait until completion
> - */
> - AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL, AR5K_PHY_AGCCTL_NF);
> -
> - ret = ath5k_hw_register_timeout(ah, AR5K_PHY_AGCCTL,
> - AR5K_PHY_AGCCTL_NF, 0, false);
> - if (ret) {
> - ATH5K_ERR(ah->ah_sc,
> - "noise floor calibration timeout (%uMHz)\n",
> - channel->freq);
> + ret = ath5k_hw_noise_floor_calibration(ah, channel->freq);
> + if (ret)
> return ret;
> - }
> -
> - /* Wait until the noise floor is calibrated */
> - for (i = 20; i > 0; i--) {
> - mdelay(1);
> - noise_floor = ath5k_hw_reg_read(ah, AR5K_PHY_NF);
> -
> - if (AR5K_PHY_NF_RVAL(noise_floor) & AR5K_PHY_NF_ACTIVE)
> - noise_floor = AR5K_PHY_NF_AVAL(noise_floor);
> -
> - if (noise_floor <= AR5K_TUNE_NOISE_FLOOR)
> - break;
> - }
> -
> - if (noise_floor > AR5K_TUNE_NOISE_FLOOR) {
> - ATH5K_ERR(ah->ah_sc, "noise floor calibration failed (%uMHz)\n",
> - channel->freq);
> - return -EIO;
> - }
>
> /*
> * Re-enable RX/TX and beacons
> --
> 1.5.3.4
>
>

Acked-by: Nick Kossifidis <[email protected]>

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

2007-11-23 11:35:06

by Nick Kossifidis

[permalink] [raw]
Subject: Re: ath5k signal/noise fixes

2007/11/23, Bruno Randolf <[email protected]>:
>
> hello!
>
> the next 3 patches contain fixes for the signal/noise level calibration and
> reporting of ath5k. i split them up because i'm not 100% sure of the second
> two.
>

Good job ;-)

> i'm also not sure if what we call noise calibration in this case really is an
> calibration or just a reading of the values?
>

To enable noise floor calibration on hw we set AGCTL_NF, if we want to
read the value and make checks/update stats etc we read PHY_NF. I
agree with your approach of noise floor calibration + update value in
all cases. This patch aso makes code cleaner because noise floor
calibration belongs inside phy.c




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

2007-11-27 00:40:41

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath5k: export signal quality values

On Nov 23, 2007 6:27 AM, Nick Kossifidis <[email protected]> wrote:
> 2007/11/23, Bruno Randolf <[email protected]>:
>
> > store the last noise floor in ath5k_hw and report signal, noise and link
> > quality values to mac80211, which in turn makes them show up in iwconfig.
> >
> > ath5k.h, phy.c:
> > Changes-licensed-under: ISC
> >
> > base.c:
> > Changes-licensed-under: 3-clause-BSD
> >
> > Signed-off-by: Bruno Randolf <[email protected]>

> Acked-by: Nick Kossifidis <[email protected]>

This patch series depends on your previous debug.[ch] patches. If
that's ACK'ed (I'll review next) then:

Acked-by: Luis R. Rodriguez <[email protected]>

Luis

2007-11-23 10:51:54

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH 2/3] ath5k: noise calibration also for rf511x

also perform full noise calibration in ath5k_hw_rf511x_calibrate() instead of
just writing the bit.

Changes-licensed-under: ISC
Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath5k/phy.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
index 9b91121..f250c2e 100644
--- a/drivers/net/wireless/ath5k/phy.c
+++ b/drivers/net/wireless/ath5k/phy.c
@@ -1702,8 +1702,7 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
((u32)q_coff) | ((u32)i_coff << AR5K_PHY_IQ_CORR_Q_I_COFF_S));

done:
- /* Start noise floor calibration */
- AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL, AR5K_PHY_AGCCTL_NF);
+ ath5k_hw_noise_floor_calibration(ah, channel->freq);

/* Request RF gain */
if (channel->val & CHANNEL_5GHZ) {
--
1.5.3.4


2007-11-23 10:51:55

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH 3/3] ath5k: export signal quality values

store the last noise floor in ath5k_hw and report signal, noise and link
quality values to mac80211, which in turn makes them show up in iwconfig.

ath5k.h, phy.c:
Changes-licensed-under: ISC

base.c:
Changes-licensed-under: 3-clause-BSD

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath5k/ath5k.h | 3 +++
drivers/net/wireless/ath5k/base.c | 22 ++++++++++++++++++++--
drivers/net/wireless/ath5k/phy.c | 2 ++
3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 5bcbbf9..2e13d79 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -1003,6 +1003,9 @@ struct ath5k_hw {
struct ieee80211_channel r_last_channel;
} ah_radar;

+ /* noise floor from last periodic calibration */
+ s32 ah_noise_floor;
+
/*
* Function pointers
*/
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 444e4a7..f288858 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -435,7 +435,10 @@ ath5k_pci_probe(struct pci_dev *pdev,
hw->flags = IEEE80211_HW_RX_INCLUDES_FCS;
hw->extra_tx_headroom = 2;
hw->channel_change_time = 5000;
- hw->max_rssi = 127; /* FIXME: get a real value for this. */
+ /* these names are misleading */
+ hw->max_rssi = -110; /* signal in dBm */
+ hw->max_noise = -110; /* noise in dBm */
+ hw->max_signal = 100; /* we will provide a percentage based on rssi */
sc = hw->priv;
sc->hw = hw;
sc->pdev = pdev;
@@ -1721,7 +1724,22 @@ accept:
rxs.freq = sc->curchan->freq;
rxs.channel = sc->curchan->chan;
rxs.phymode = sc->curmode;
- rxs.ssi = ds->ds_rxstat.rs_rssi;
+
+ /*
+ * signal quality:
+ * the names here are misleading and the usage of these
+ * values by iwconfig makes it even worse
+ */
+ /* noise floor in dBm, from the last noise calibration */
+ rxs.noise = sc->ah->ah_noise_floor;
+ /* signal level in dBm */
+ rxs.ssi = rxs.noise + ds->ds_rxstat.rs_rssi;
+ /*
+ * "signal" is actually displayed as Link Quality by iwconfig
+ * we provide a percentage based on rssi (assuming max rssi 64)
+ */
+ rxs.signal = ds->ds_rxstat.rs_rssi * 100 / 64;
+
rxs.antenna = ds->ds_rxstat.rs_antenna;
rxs.rate = ds->ds_rxstat.rs_rate;
rxs.flag |= ath5k_rx_decrypted(sc, ds, skb);
diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
index f250c2e..52caf8e 100644
--- a/drivers/net/wireless/ath5k/phy.c
+++ b/drivers/net/wireless/ath5k/phy.c
@@ -1562,6 +1562,8 @@ ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq)
return -EIO;
}

+ ah->ah_noise_floor = noise_floor;
+
return 0;
}

--
1.5.3.4


2007-11-27 02:45:51

by Bruno Randolf

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath5k: fix noise floor calibration

On Tuesday 27 November 2007 09:28:02 Luis R. Rodriguez wrote:
> On Nov 23, 2007 5:52 AM, Bruno Randolf <[email protected]> wrote:
> > diff --git a/drivers/net/wireless/ath5k/phy.c
> > b/drivers/net/wireless/ath5k/phy.c index 3c2a67c..9b91121 100644
> > --- a/drivers/net/wireless/ath5k/phy.c
> > +++ b/drivers/net/wireless/ath5k/phy.c
> > @@ -1519,6 +1519,51 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct
> > ieee80211_channel *channel) /*****************\
> > PHY calibration
> > \*****************/
> > +int
> > +ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq)
> > +{
>
> Good idea :) Mind adding kdoc for this? I hope we can add kdoc for
> more hw stuff to eventually export this automatically as developer
> documentation.

sure:

>From c6a21193965b1c6467851c0fa2b9a4a7a63ce8bb Mon Sep 17 00:00:00 2001
From: Bruno Randolf <[email protected]>
Date: Tue, 27 Nov 2007 11:44:02 +0900
Subject: [PATCH 1/1] ath5k: add kerneldoc for ath5k_hw_noise_floor_calibration


Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath5k/phy.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath5k/phy.c
b/drivers/net/wireless/ath5k/phy.c
index e12a7e8..4daccc8 100644
--- a/drivers/net/wireless/ath5k/phy.c
+++ b/drivers/net/wireless/ath5k/phy.c
@@ -1519,6 +1519,27 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct
ieee80211_channel *channel)
/*****************\
PHY calibration
\*****************/
+
+/**
+ * ath5k_hw_noise_floor_calibration - perform PHY noise floor calibration
+ *
+ * @ah: struct ath5k_hw pointer we are operating on
+ * @freq: the channel frequency, just used for error logging
+ *
+ * This function performs a noise floor calibration of the PHY and waits for
+ * it to complete. Then the noise floor value is compared to some maximum
+ * noise floor we consider valid.
+ *
+ * Note that this is different from what the madwifi HAL does: it reads the
+ * noise floor and afterwards initiates the calibration. Since the noise
floor
+ * calibration can take some time to finish, depending on the current channel
+ * use, that avoids the occasional timeout warnings we are seeing now.
+ *
+ * See the following link for an Atheros patent on noise floor calibration:
+ * http://patft.uspto.gov/netacgi/nph-Parser?Sect1=PTO1&Sect2=HITOFF&d=PALL \
+ * &p=1&u=%2Fnetahtml%2FPTO%2Fsrchnum.htm&r=1&f=G&l=50&s1=7245893.PN.&OS=PN/7
+ *
+ */
int
ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq)
{
--
1.5.3.4


2007-11-23 11:27:56

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath5k: export signal quality values

2007/11/23, Bruno Randolf <[email protected]>:
> store the last noise floor in ath5k_hw and report signal, noise and link
> quality values to mac80211, which in turn makes them show up in iwconfig.
>
> ath5k.h, phy.c:
> Changes-licensed-under: ISC
>
> base.c:
> Changes-licensed-under: 3-clause-BSD
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
> drivers/net/wireless/ath5k/ath5k.h | 3 +++
> drivers/net/wireless/ath5k/base.c | 22 ++++++++++++++++++++--
> drivers/net/wireless/ath5k/phy.c | 2 ++
> 3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
> index 5bcbbf9..2e13d79 100644
> --- a/drivers/net/wireless/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath5k/ath5k.h
> @@ -1003,6 +1003,9 @@ struct ath5k_hw {
> struct ieee80211_channel r_last_channel;
> } ah_radar;
>
> + /* noise floor from last periodic calibration */
> + s32 ah_noise_floor;
> +
> /*
> * Function pointers
> */
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 444e4a7..f288858 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -435,7 +435,10 @@ ath5k_pci_probe(struct pci_dev *pdev,
> hw->flags = IEEE80211_HW_RX_INCLUDES_FCS;
> hw->extra_tx_headroom = 2;
> hw->channel_change_time = 5000;
> - hw->max_rssi = 127; /* FIXME: get a real value for this. */
> + /* these names are misleading */
> + hw->max_rssi = -110; /* signal in dBm */
> + hw->max_noise = -110; /* noise in dBm */
> + hw->max_signal = 100; /* we will provide a percentage based on rssi */
> sc = hw->priv;
> sc->hw = hw;
> sc->pdev = pdev;
> @@ -1721,7 +1724,22 @@ accept:
> rxs.freq = sc->curchan->freq;
> rxs.channel = sc->curchan->chan;
> rxs.phymode = sc->curmode;
> - rxs.ssi = ds->ds_rxstat.rs_rssi;
> +
> + /*
> + * signal quality:
> + * the names here are misleading and the usage of these
> + * values by iwconfig makes it even worse
> + */
> + /* noise floor in dBm, from the last noise calibration */
> + rxs.noise = sc->ah->ah_noise_floor;
> + /* signal level in dBm */
> + rxs.ssi = rxs.noise + ds->ds_rxstat.rs_rssi;
> + /*
> + * "signal" is actually displayed as Link Quality by iwconfig
> + * we provide a percentage based on rssi (assuming max rssi 64)
> + */
> + rxs.signal = ds->ds_rxstat.rs_rssi * 100 / 64;
> +
> rxs.antenna = ds->ds_rxstat.rs_antenna;
> rxs.rate = ds->ds_rxstat.rs_rate;
> rxs.flag |= ath5k_rx_decrypted(sc, ds, skb);
> diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
> index f250c2e..52caf8e 100644
> --- a/drivers/net/wireless/ath5k/phy.c
> +++ b/drivers/net/wireless/ath5k/phy.c
> @@ -1562,6 +1562,8 @@ ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq)
> return -EIO;
> }
>
> + ah->ah_noise_floor = noise_floor;
> +
> return 0;
> }
>
> --
> 1.5.3.4
>
>

Acked-by: Nick Kossifidis <[email protected]>

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

2007-11-23 10:51:54

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH 1/3] ath5k: fix noise floor calibration

fix noise floor calibration by applying AR5K_PHY_NF_AVAL after
AR5K_PHY_NF_RVAL. that way the XORed mask and value have the same length and we
get a reasonable noise value in -dBm.

move duplicate noise floor calibration code from two different places into one
function ath5k_hw_noise_floor_calibration().

the check for accepted noise_floor values (<= AR5K_TUNE_NOISE_FLOOR) should
only happen when we have an active reading and a converted value, so move it up
into the first if.

Changes-licensed-under: ISC
Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath5k/ath5k.h | 1 +
drivers/net/wireless/ath5k/hw.c | 36 +---------------
drivers/net/wireless/ath5k/phy.c | 79 +++++++++++++++++++++--------------
3 files changed, 51 insertions(+), 65 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 1b8ddd9..5bcbbf9 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -1127,6 +1127,7 @@ extern int ath5k_hw_phy_disable(struct ath5k_hw *ah);
extern u16 ath5k_hw_radio_revision(struct ath5k_hw *ah, unsigned int chan);
extern void ath5k_hw_set_def_antenna(struct ath5k_hw *ah, unsigned int ant);
extern unsigned int ath5k_hw_get_def_antenna(struct ath5k_hw *ah);
+extern int ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq);
/* TX power setup */
extern int ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel, unsigned int txpower);
extern int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, unsigned int power);
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index 2a826a7..b42fc8f 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -593,7 +593,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
{
struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
u32 data, s_seq, s_ant, s_led[3];
- s32 noise_floor;
unsigned int i, mode, freq, ee_mode, ant[2], driver_mode = -1;
int ret;

@@ -916,38 +915,9 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
return -EAGAIN;
}

- /*
- * Enable noise floor calibration and wait until completion
- */
- AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL,
- AR5K_PHY_AGCCTL_NF);
-
- if (ath5k_hw_register_timeout(ah, AR5K_PHY_AGCCTL,
- AR5K_PHY_AGCCTL_NF, 0, false)) {
- ATH5K_ERR(ah->ah_sc,
- "noise floor calibration timeout (%uMHz)\n",
- channel->freq);
- return -EAGAIN;
- }
-
- /* Wait until the noise floor is calibrated and read the value */
- for (i = 20; i > 0; i--) {
- mdelay(1);
- noise_floor = ath5k_hw_reg_read(ah, AR5K_PHY_NF);
-
- if (AR5K_PHY_NF_RVAL(noise_floor) & AR5K_PHY_NF_ACTIVE)
- noise_floor = AR5K_PHY_NF_AVAL(noise_floor);
-
- if (noise_floor <= AR5K_TUNE_NOISE_FLOOR)
- break;
- }
-
- if (noise_floor > AR5K_TUNE_NOISE_FLOOR) {
- ATH5K_ERR(ah->ah_sc,
- "noise floor calibration failed (%uMHz)\n",
- channel->freq);
- return -EIO;
- }
+ ret = ath5k_hw_noise_floor_calibration(ah, channel->freq);
+ if (ret)
+ return ret;

ah->ah_calibration = false;

diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
index 3c2a67c..9b91121 100644
--- a/drivers/net/wireless/ath5k/phy.c
+++ b/drivers/net/wireless/ath5k/phy.c
@@ -1519,6 +1519,51 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel)
/*****************\
PHY calibration
\*****************/
+int
+ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq)
+{
+ int ret;
+ unsigned int i;
+ s32 noise_floor;
+
+ /*
+ * Enable noise floor calibration and wait until completion
+ */
+ AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL,
+ AR5K_PHY_AGCCTL_NF);
+
+ ret = ath5k_hw_register_timeout(ah, AR5K_PHY_AGCCTL,
+ AR5K_PHY_AGCCTL_NF, 0, false);
+ if (ret) {
+ ATH5K_ERR(ah->ah_sc,
+ "noise floor calibration timeout (%uMHz)\n", freq);
+ return ret;
+ }
+
+ /* Wait until the noise floor is calibrated and read the value */
+ for (i = 20; i > 0; i--) {
+ mdelay(1);
+ noise_floor = ath5k_hw_reg_read(ah, AR5K_PHY_NF);
+ noise_floor = AR5K_PHY_NF_RVAL(noise_floor);
+ if (noise_floor & AR5K_PHY_NF_ACTIVE) {
+ noise_floor = AR5K_PHY_NF_AVAL(noise_floor);
+
+ if (noise_floor <= AR5K_TUNE_NOISE_FLOOR)
+ break;
+ }
+ }
+
+ ATH5K_DBG(ah->ah_sc, ATH5K_DEBUG_CALIBRATE,
+ "noise floor %d, %d\n", noise_floor, AR5K_TUNE_NOISE_FLOOR);
+
+ if (noise_floor > AR5K_TUNE_NOISE_FLOOR) {
+ ATH5K_ERR(ah->ah_sc,
+ "noise floor calibration failed (%uMHz)\n", freq);
+ return -EIO;
+ }
+
+ return 0;
+}

/*
* Perform a PHY calibration on RF5110
@@ -1529,8 +1574,6 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
struct ieee80211_channel *channel)
{
u32 phy_sig, phy_agc, phy_sat, beacon;
- s32 noise_floor;
- unsigned int i;
int ret;

/*
@@ -1612,37 +1655,9 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
return ret;
}

- /*
- * Enable noise floor calibration and wait until completion
- */
- AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL, AR5K_PHY_AGCCTL_NF);
-
- ret = ath5k_hw_register_timeout(ah, AR5K_PHY_AGCCTL,
- AR5K_PHY_AGCCTL_NF, 0, false);
- if (ret) {
- ATH5K_ERR(ah->ah_sc,
- "noise floor calibration timeout (%uMHz)\n",
- channel->freq);
+ ret = ath5k_hw_noise_floor_calibration(ah, channel->freq);
+ if (ret)
return ret;
- }
-
- /* Wait until the noise floor is calibrated */
- for (i = 20; i > 0; i--) {
- mdelay(1);
- noise_floor = ath5k_hw_reg_read(ah, AR5K_PHY_NF);
-
- if (AR5K_PHY_NF_RVAL(noise_floor) & AR5K_PHY_NF_ACTIVE)
- noise_floor = AR5K_PHY_NF_AVAL(noise_floor);
-
- if (noise_floor <= AR5K_TUNE_NOISE_FLOOR)
- break;
- }
-
- if (noise_floor > AR5K_TUNE_NOISE_FLOOR) {
- ATH5K_ERR(ah->ah_sc, "noise floor calibration failed (%uMHz)\n",
- channel->freq);
- return -EIO;
- }

/*
* Re-enable RX/TX and beacons
--
1.5.3.4


2007-11-23 11:25:40

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath5k: noise calibration also for rf511x

2007/11/23, Bruno Randolf <[email protected]>:
> also perform full noise calibration in ath5k_hw_rf511x_calibrate() instead of
> just writing the bit.
>
> Changes-licensed-under: ISC
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
> drivers/net/wireless/ath5k/phy.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
> index 9b91121..f250c2e 100644
> --- a/drivers/net/wireless/ath5k/phy.c
> +++ b/drivers/net/wireless/ath5k/phy.c
> @@ -1702,8 +1702,7 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
> ((u32)q_coff) | ((u32)i_coff << AR5K_PHY_IQ_CORR_Q_I_COFF_S));
>
> done:
> - /* Start noise floor calibration */
> - AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL, AR5K_PHY_AGCCTL_NF);
> + ath5k_hw_noise_floor_calibration(ah, channel->freq);
>
> /* Request RF gain */
> if (channel->val & CHANNEL_5GHZ) {
> --
> 1.5.3.4
>
>

Actually just writing the bit enables nf calibration, what this patch
does is that it also reads the value and check if it's ok.

Acked-by: Nick Kossifidis <[email protected]>

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

2007-11-27 00:31:11

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath5k: noise calibration also for rf511x

On Nov 23, 2007 6:25 AM, Nick Kossifidis <[email protected]> wrote:
> 2007/11/23, Bruno Randolf <[email protected]>:
> > also perform full noise calibration in ath5k_hw_rf511x_calibrate() instead of
> > just writing the bit.
> >
> > Changes-licensed-under: ISC
> > Signed-off-by: Bruno Randolf <[email protected]>
> > ---
> > drivers/net/wireless/ath5k/phy.c | 3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
> > index 9b91121..f250c2e 100644
> > --- a/drivers/net/wireless/ath5k/phy.c
> > +++ b/drivers/net/wireless/ath5k/phy.c
> > @@ -1702,8 +1702,7 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
> > ((u32)q_coff) | ((u32)i_coff << AR5K_PHY_IQ_CORR_Q_I_COFF_S));
> >
> > done:
> > - /* Start noise floor calibration */
> > - AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL, AR5K_PHY_AGCCTL_NF);
> > + ath5k_hw_noise_floor_calibration(ah, channel->freq);
> >
> > /* Request RF gain */
> > if (channel->val & CHANNEL_5GHZ) {
> > --
> > 1.5.3.4
> >
> >
>
> Actually just writing the bit enables nf calibration, what this patch
> does is that it also reads the value and check if it's ok.

Know why it wasn't done before? Have you guys tested this without
impact on performance on the affected hardware? Just curious.

Luis

2007-11-27 00:28:04

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath5k: fix noise floor calibration

On Nov 23, 2007 5:52 AM, Bruno Randolf <[email protected]> wrote:

> diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
> index 3c2a67c..9b91121 100644
> --- a/drivers/net/wireless/ath5k/phy.c
> +++ b/drivers/net/wireless/ath5k/phy.c
> @@ -1519,6 +1519,51 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel)
> /*****************\
> PHY calibration
> \*****************/
> +int
> +ath5k_hw_noise_floor_calibration(struct ath5k_hw *ah, short freq)
> +{

Good idea :) Mind adding kdoc for this? I hope we can add kdoc for
more hw stuff to eventually export this automatically as developer
documentation.

> + int ret;
> + unsigned int i;
> + s32 noise_floor;
> +
> + /*
> + * Enable noise floor calibration and wait until completion
> + */
> + AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL,
> + AR5K_PHY_AGCCTL_NF);

This is minor but you probably noticed ath5k_hw_rf511x_calibrate()
only enables noise floor calibration but does not actually wait and
check for the value. I am not sure why. Anyone? Anyway, until we do
how about wrapping the AR5K_REG_ENABLE_BITS for noise floor into a
small static inline ath5k_hw_enable_noise_floor(). Then your new
routine can call it and so can ath5k_hw_rf511x_calibrate().

> + ATH5K_DBG(ah->ah_sc, ATH5K_DEBUG_CALIBRATE,
> + "noise floor %d, %d\n", noise_floor, AR5K_TUNE_NOISE_FLOOR);

This relies on your previous debug.[ch] patches which are pending.
Just a note, next time you may want to mention this, just so that if
someone wants to test this they'll know how to get to test it without
compilation problems. It'll also help John know if he can apply this
or if he has to wait for pending patches.

Luis

2007-11-27 00:44:29

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: ath5k signal/noise fixes

On Nov 23, 2007 5:52 AM, Bruno Randolf <[email protected]> wrote:
>
> hello!
>
> the next 3 patches contain fixes for the signal/noise level calibration and
> reporting of ath5k. i split them up because i'm not 100% sure of the second
> two.
>
> 1) fixes the reading of the noise floor register and puts it into one function.
>
> 2) also enables noise floor calibration for rf511x.
>
> 3) saves the read noise floor and exports signal/noise and "quality" to
> mac80211/wext.
>
> i'm also not sure if what we call noise calibration in this case really is an
> calibration or just a reading of the values?
>
> is ath5k_hw the right place to store the noise_floor?
>
> the values reported, especially the noise seems a bit high, it's around -77 in
> the A bands and around -92 in G bands (thats with an AR5215/RF5111/2111), so we
> might have some more tweaking to do. since i add the RSSI to get the signal
> quality in dBm it seems a bit too high too.
>
> this comment from madwifi leads me to believe this is the correct thing to do:
>
> * rx_rssi is in units of dbm above the noise floor. This value
> * is measured during the preamble and PLCP; i.e. with the initial
> * 4us of detection. The noise floor is typically a consistent
> * -96dBm absolute power in a 20MHz channel
>
> at least we have some values to play with now :)
>
> ah, and they are based on my last debugging and logging, patches (although
> there are not huge dependencies).

I should have read this first :), sorry I had missed it.

Luis.