2011-11-21 00:49:14

by Nikolay Martynov

[permalink] [raw]
Subject: [PATCH 0/4] Fix the way ANI is being disabled for ar9100 and ar9340

Currently in ath9k code there is an attempt which is meant to disable ANI for ar9100 and ar9340. But it doesn't really achieve this. All it does is disable ANI init and setup (i.e. calls to ath9k_hw_ani_setup and ath9k_hw_ani_init). Since ath9k_hw_ani_setup is not called ah->config.ani_poll_interval is never initialized (i.e. it is always zero) and ath_ani_calibrate always executes ANI procedures (over uninitialized ANI parameters).
Moreover, ath_ani_calibrate is being called each 1ms because common->ani.timer is set to zero interval because ah->config.ani_poll_interval==0 (and thus smallest value of all intervals). Normally it should not be called this often.
This patchset makes use of config.enable_ani to check if ANI should be performed. It sets config.enable_ani to false for appropriate platforms.
Also last patch in this set enables ANI for ar9100 - it seems to be working fine there (tested on ar9102).

Question: there are common.disable_ani (and corresponding debugfs file) and config.enable_ani in ath9k code. config.enable_ani sets if ANI is supported and common.disable_ani is meant to disable calibration. This is confusing on a sense that disable_ani completely disables all calibration code including short/long calibration and ANI, not ANI alone. Would it make sense to rename disable_ani to disable_calib and enable_ani to ani_supported?

All comments and suggestions are appreciated.
Thanks.

Nikolay Martynov (4):
ath9k: cosmetic fix in calibration debug log
ath9k: change calibration debug to log all calibration types
ath9k: use config.enable_ani to check if ani should be performed
ath9k: enable ani for 9100

drivers/net/wireless/ath/ath9k/ar9002_calib.c | 2 +-
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 3 ++-
drivers/net/wireless/ath/ath9k/hw.c | 12 ++++++++----
drivers/net/wireless/ath/ath9k/main.c | 12 +++++++-----
4 files changed, 18 insertions(+), 11 deletions(-)

--
1.7.4.1



2011-11-21 06:06:19

by Nikolay Martynov

[permalink] [raw]
Subject: Re: [PATCH 3/4] ath9k: use config.enable_ani to check if ani should be performed

Hi,

2011/11/21 Mohammed Shafi <[email protected]>:
> instead of doing checks like the above , can we just prevent
> ath_ani_calibrate being not executed at all by doing something like
> this, please verify if i had missed something

Please see my response to [PATCH 0/4].
Before my patch the 'ath_ani_calibrate' was executed for all
devices. But for certain devices I think the intent was to not execute
'ath9k_hw_ani_monitor' ('ANI-ANI' from my previous email) and execute
everything else.
Considering this your patch seems wrong since it'll disable
'ath_ani_calibrate' completely for certain devices.

Please let me know if I'm missing something.
Thanks.

>
> diff --git a/drivers/net/wireless/ath/ath9k/init.c
> b/drivers/net/wireless/ath/ath9k/init.c
> index e046de9..e0ebccd 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -500,7 +500,9 @@ static void ath9k_init_misc(struct ath_softc *sc)
> {
> struct ath_common *common = ath9k_hw_common(sc->sc_ah);
> int i = 0;
> - setup_timer(&common->ani.timer, ath_ani_calibrate, (unsigned long)sc);
> +
> + if (sc->sc_ah->config.enable_ani)
> + setup_timer(&common->ani.timer, ath_ani_calibrate,
> (unsigned long)sc);
>
> sc->config.txpowlimit = ATH_TXPOWER_MAX;

--
Truthfully yours,
Martynov Nikolay.
Email: [email protected]

2011-11-21 05:31:08

by Mohammed Shafi

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fix the way ANI is being disabled for ar9100 and ar9340

On Mon, Nov 21, 2011 at 6:18 AM, Nikolay Martynov <[email protected]> wrote:
> ?Currently in ath9k code there is an attempt which is meant to disable ANI for ar9100 and ar9340. But it doesn't really achieve this. All it does is disable ANI init and setup (i.e. calls to ath9k_hw_ani_setup and ath9k_hw_ani_init). Since ath9k_hw_ani_setup is not called ah->config.ani_poll_interval is never initialized (i.e. it is always zero) and ath_ani_calibrate always executes ANI procedures (over uninitialized ANI parameters).
> ?Moreover, ath_ani_calibrate is being called each 1ms because common->ani.timer is set to zero interval because ah->config.ani_poll_interval==0 (and thus smallest value of all intervals). Normally it should not be called this often.
> ?This patchset makes use of config.enable_ani to check if ANI should be performed. It sets config.enable_ani to false for appropriate platforms.
> ?Also last patch in this set enables ANI for ar9100 - it seems to be working fine there (tested on ar9102).
>
> ?Question: there are common.disable_ani (and corresponding debugfs file) and config.enable_ani in ath9k code. config.enable_ani sets if ANI is supported and common.disable_ani is meant to disable calibration. This is confusing on a sense that disable_ani completely disables all calibration code including short/long calibration and ANI, not ANI alone. Would it make sense to rename disable_ani to disable_calib and enable_ani to ani_supported?

I understand short and long calibration of ANI, not the complete calibration :)
disable_ani is to stop/start ANI anytime via ath9k debugfs entry.
please let me know if you have further queries

>
> ?All comments and suggestions are appreciated.
> ?Thanks.
>
> Nikolay Martynov (4):
> ?ath9k: cosmetic fix in calibration debug log
> ?ath9k: change calibration debug to log all calibration types
> ?ath9k: use config.enable_ani to check if ani should be performed
> ?ath9k: enable ani for 9100
>
> ?drivers/net/wireless/ath/ath9k/ar9002_calib.c | ? ?2 +-
> ?drivers/net/wireless/ath/ath9k/htc_drv_main.c | ? ?3 ++-
> ?drivers/net/wireless/ath/ath9k/hw.c ? ? ? ? ? | ? 12 ++++++++----
> ?drivers/net/wireless/ath/ath9k/main.c ? ? ? ? | ? 12 +++++++-----
> ?4 files changed, 18 insertions(+), 11 deletions(-)
>
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
shafi

2011-11-21 00:49:25

by Nikolay Martynov

[permalink] [raw]
Subject: [PATCH 2/4] ath9k: change calibration debug to log all calibration types

---
drivers/net/wireless/ath/ath9k/main.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e43c41c..9ce3dff 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -561,7 +561,6 @@ void ath_ani_calibrate(unsigned long data)
/* Long calibration runs independently of short calibration. */
if ((timestamp - common->ani.longcal_timer) >= long_cal_interval) {
longcal = true;
- ath_dbg(common, ATH_DBG_ANI, "longcal @%lu\n", jiffies);
common->ani.longcal_timer = timestamp;
}

@@ -569,8 +568,6 @@ void ath_ani_calibrate(unsigned long data)
if (!common->ani.caldone) {
if ((timestamp - common->ani.shortcal_timer) >= short_cal_interval) {
shortcal = true;
- ath_dbg(common, ATH_DBG_ANI,
- "shortcal @%lu\n", jiffies);
common->ani.shortcal_timer = timestamp;
common->ani.resetcal_timer = timestamp;
}
@@ -605,6 +602,11 @@ void ath_ani_calibrate(unsigned long data)
ah->rxchainmask, longcal);
}

+ ath_dbg(common, ATH_DBG_ANI,
+ "Calibration @%lu finished: %s %s %s, caldone: %s\n", jiffies,
+ longcal ? "long" : "", shortcal ? "short" : "",
+ aniflag ? "ani" : "", common->ani.caldone ? "true" : "false");
+
ath9k_ps_restore(sc);

set_timer:
--
1.7.4.1


2011-11-21 05:15:58

by Mohammed Shafi

[permalink] [raw]
Subject: Re: [PATCH 3/4] ath9k: use config.enable_ani to check if ani should be performed

On Mon, Nov 21, 2011 at 6:18 AM, Nikolay Martynov <[email protected]> wrote:
> ---
> ?drivers/net/wireless/ath/ath9k/htc_drv_main.c | ? ?3 ++-
> ?drivers/net/wireless/ath/ath9k/hw.c ? ? ? ? ? | ? 12 ++++++++----
> ?drivers/net/wireless/ath/ath9k/main.c ? ? ? ? | ? ?4 ++--
> ?3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
> index 0b9a0e8..f8983fd 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
> @@ -808,7 +808,8 @@ void ath9k_htc_ani_work(struct work_struct *work)
> ? ? ? ?}
>
> ? ? ? ?/* Verify whether we must check ANI */
> - ? ? ? if ((timestamp - common->ani.checkani_timer) >= ATH_ANI_POLLINTERVAL) {
> + ? ? ? if (sc->sc_ah->config.enable_ani
> + ? ? ? ? ? ?&& (timestamp - common->ani.checkani_timer) >= ATH_ANI_POLLINTERVAL) {
> ? ? ? ? ? ? ? ?aniflag = true;
> ? ? ? ? ? ? ? ?common->ani.checkani_timer = timestamp;
> ? ? ? ?}
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index 662ab7e..d6a2568 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -504,10 +504,10 @@ static int ath9k_hw_post_init(struct ath_hw *ah)
> ? ? ? ? ? ? ? ?return ecode;
> ? ? ? ?}
>
> - ? ? ? if (!AR_SREV_9100(ah) && !AR_SREV_9340(ah)) {
> - ? ? ? ? ? ? ? ath9k_hw_ani_setup(ah);
> - ? ? ? ? ? ? ? ath9k_hw_ani_init(ah);
> - ? ? ? }
> + ? ? ? ?if (ah->config.enable_ani) {
> + ? ? ? ? ? ? ? ?ath9k_hw_ani_setup(ah);
> + ? ? ? ? ? ? ? ?ath9k_hw_ani_init(ah);
> + ? ? ? ?}
>
> ? ? ? ?return 0;
> ?}
> @@ -610,6 +610,10 @@ static int __ath9k_hw_init(struct ath_hw *ah)
> ? ? ? ?if (!AR_SREV_9300_20_OR_LATER(ah))
> ? ? ? ? ? ? ? ?ah->ani_function &= ~ATH9K_ANI_MRC_CCK;
>
> + ? ? ? ?//disable ANI for 9100 and 9340

please change the comment style to
/*
*disable ANI for AR9100 and AR9340
*/

> + ? ? ? ?if (AR_SREV_9100(ah) || AR_SREV_9340(ah))
> + ? ? ? ? ? ? ? ?ah->config.enable_ani = false;
> +
> ? ? ? ?ath9k_hw_init_mode_regs(ah);
>
> ? ? ? ?if (!ah->is_pciexpress)
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 9ce3dff..4653538 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -581,8 +581,8 @@ void ath_ani_calibrate(unsigned long data)
> ? ? ? ?}
>
> ? ? ? ?/* Verify whether we must check ANI */
> - ? ? ? if ((timestamp - common->ani.checkani_timer) >=
> - ? ? ? ? ? ?ah->config.ani_poll_interval) {
> + ? ? ? if (sc->sc_ah->config.enable_ani
> + ? ? ? ? ? ?&& (timestamp - common->ani.checkani_timer) >= ah->config.ani_poll_interval) {
> ? ? ? ? ? ? ? ?aniflag = true;
> ? ? ? ? ? ? ? ?common->ani.checkani_timer = timestamp;
> ? ? ? ?}

instead of doing checks like the above , can we just prevent
ath_ani_calibrate being not executed at all by doing something like
this, please verify if i had missed something

diff --git a/drivers/net/wireless/ath/ath9k/init.c
b/drivers/net/wireless/ath/ath9k/init.c
index e046de9..e0ebccd 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -500,7 +500,9 @@ static void ath9k_init_misc(struct ath_softc *sc)
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
int i = 0;
- setup_timer(&common->ani.timer, ath_ani_calibrate, (unsigned long)sc);
+
+ if (sc->sc_ah->config.enable_ani)
+ setup_timer(&common->ani.timer, ath_ani_calibrate,
(unsigned long)sc);

sc->config.txpowlimit = ATH_TXPOWER_MAX;



> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
shafi

2011-11-21 05:19:01

by Mohammed Shafi

[permalink] [raw]
Subject: Re: [PATCH 4/4] ath9k: enable ani for 9100

On Mon, Nov 21, 2011 at 6:19 AM, Nikolay Martynov <[email protected]> wrote:
> ---
> ?drivers/net/wireless/ath/ath9k/hw.c | ? ?4 ++--
> ?1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index d6a2568..79c4a0f 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -610,8 +610,8 @@ static int __ath9k_hw_init(struct ath_hw *ah)
> ? ? ? ?if (!AR_SREV_9300_20_OR_LATER(ah))
> ? ? ? ? ? ? ? ?ah->ani_function &= ~ATH9K_ANI_MRC_CCK;
>
> - ? ? ? ?//disable ANI for 9100 and 9340
> - ? ? ? ?if (AR_SREV_9100(ah) || AR_SREV_9340(ah))
> + ? ? ? ?//disable ANI for 9340

please change the comment style to
/*
*
*/

> + ? ? ? ?if (AR_SREV_9340(ah))
> ? ? ? ? ? ? ? ? ah->config.enable_ani = false;

please mention in the commit log why we are enabling ANI for AR9100,
if there is some improvement performance in noisy environment, please
do mention it.
i assume if it is disabled we may not have ANI support for AR9100
(or) the hardware code changes are missing.

>
> ? ? ? ?ath9k_hw_init_mode_regs(ah);
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
shafi

2011-11-21 00:49:29

by Nikolay Martynov

[permalink] [raw]
Subject: [PATCH 3/4] ath9k: use config.enable_ani to check if ani should be performed

---
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 3 ++-
drivers/net/wireless/ath/ath9k/hw.c | 12 ++++++++----
drivers/net/wireless/ath/ath9k/main.c | 4 ++--
3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index 0b9a0e8..f8983fd 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -808,7 +808,8 @@ void ath9k_htc_ani_work(struct work_struct *work)
}

/* Verify whether we must check ANI */
- if ((timestamp - common->ani.checkani_timer) >= ATH_ANI_POLLINTERVAL) {
+ if (sc->sc_ah->config.enable_ani
+ && (timestamp - common->ani.checkani_timer) >= ATH_ANI_POLLINTERVAL) {
aniflag = true;
common->ani.checkani_timer = timestamp;
}
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 662ab7e..d6a2568 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -504,10 +504,10 @@ static int ath9k_hw_post_init(struct ath_hw *ah)
return ecode;
}

- if (!AR_SREV_9100(ah) && !AR_SREV_9340(ah)) {
- ath9k_hw_ani_setup(ah);
- ath9k_hw_ani_init(ah);
- }
+ if (ah->config.enable_ani) {
+ ath9k_hw_ani_setup(ah);
+ ath9k_hw_ani_init(ah);
+ }

return 0;
}
@@ -610,6 +610,10 @@ static int __ath9k_hw_init(struct ath_hw *ah)
if (!AR_SREV_9300_20_OR_LATER(ah))
ah->ani_function &= ~ATH9K_ANI_MRC_CCK;

+ //disable ANI for 9100 and 9340
+ if (AR_SREV_9100(ah) || AR_SREV_9340(ah))
+ ah->config.enable_ani = false;
+
ath9k_hw_init_mode_regs(ah);

if (!ah->is_pciexpress)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 9ce3dff..4653538 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -581,8 +581,8 @@ void ath_ani_calibrate(unsigned long data)
}

/* Verify whether we must check ANI */
- if ((timestamp - common->ani.checkani_timer) >=
- ah->config.ani_poll_interval) {
+ if (sc->sc_ah->config.enable_ani
+ && (timestamp - common->ani.checkani_timer) >= ah->config.ani_poll_interval) {
aniflag = true;
common->ani.checkani_timer = timestamp;
}
--
1.7.4.1


2011-11-21 00:49:32

by Nikolay Martynov

[permalink] [raw]
Subject: [PATCH 4/4] ath9k: enable ani for 9100

---
drivers/net/wireless/ath/ath9k/hw.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index d6a2568..79c4a0f 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -610,8 +610,8 @@ static int __ath9k_hw_init(struct ath_hw *ah)
if (!AR_SREV_9300_20_OR_LATER(ah))
ah->ani_function &= ~ATH9K_ANI_MRC_CCK;

- //disable ANI for 9100 and 9340
- if (AR_SREV_9100(ah) || AR_SREV_9340(ah))
+ //disable ANI for 9340
+ if (AR_SREV_9340(ah))
ah->config.enable_ani = false;

ath9k_hw_init_mode_regs(ah);
--
1.7.4.1


2011-11-21 06:52:36

by Mohammed Shafi

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fix the way ANI is being disabled for ar9100 and ar9340

On Mon, Nov 21, 2011 at 11:22 AM, Nikolay Martynov <[email protected]> wrote:
> Hi,
>
> 2011/11/21 Mohammed Shafi <[email protected]>:
>
>>> ?Question: there are common.disable_ani (and corresponding debugfs file) and config.enable_ani in ath9k code. config.enable_ani sets if ANI is supported and common.disable_ani is meant to disable calibration. This is confusing on a sense that disable_ani completely disables all calibration code including short/long calibration and ANI, not ANI alone. Would it make sense to rename disable_ani to disable_calib and enable_ani to ani_supported?
>>
>> I understand short and long calibration of ANI, not the complete calibration :)
>> disable_ani is to stop/start ANI anytime via ath9k debugfs entry.
>> please let me know if you have further queries
>
> ?I'm new to this code so I hope you'll forgive me if my question
> sounds stupidly.
> ?There is 'ath_ani_calibrate' which basically does three things
> (apart from timer management boilerplate):
> ? -- calls ath9k_hw_calibrate to perform 'long calibration'
> ? -- calls ath9k_hw_calibrate to perform 'short calibration'
> ? -- calls ath9k_hw_ani_monitor to get ANI statistics (?)
> ?So there seem to be three major parts of ANI: long calibration,
> short calibration, and third thing which I find hard to name correctly
> so I'll refer to it as 'ANI-ANI'.

thanks! yes yes you are correct, sorry i made a mistake. based on
longcal/shortcal flags we call ath9k_hw_calibrate. true
ath9k_hw_ani_monitor does the real ani and updating ofdm/cck error and
all other ANI statistics


>
> ?From your response I understand that 'short calibration' and 'long
> calibration' generally considered to be part of think called ANI.
> Considering this name 'disable_ani' in a sense that it disables
> 'ath_ani_calibrate' as a whole looks ok.
> ?But config.enable_ani is used to control if 'ANI-ANI' gets executed
> (and this was somewhat true before my patch).
> ?So, having two variables which 'negated' names is confusing by
> itself, i.e. enable_ani vs disable_ani. But more confusing is the fact
> that they actually control different things and have different
> meaning, so disable_ani != (!enable_ani).

true disable_ani completeley disables all the three things long/short
cal and hw_ani_monitor.

>
> ?I do understand that my 'ANI-ANI' probably has proper name and I
> think that 'enable_ani' should probably be renamed using this proper
> name.

true, enable_ani only takes care of ath9k_hw_ani_monitor. thanks for
digging into it.

>
> ?I'd really appreciate if you could let me know where my
> understanding is incorrect.
> ?Thanks!
>
> --
> Truthfully yours,
> Martynov Nikolay.
> Email: [email protected]
>



--
shafi

2011-11-21 05:52:21

by Nikolay Martynov

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fix the way ANI is being disabled for ar9100 and ar9340

Hi,

2011/11/21 Mohammed Shafi <[email protected]>:

>> Question: there are common.disable_ani (and corresponding debugfs file) and config.enable_ani in ath9k code. config.enable_ani sets if ANI is supported and common.disable_ani is meant to disable calibration. This is confusing on a sense that disable_ani completely disables all calibration code including short/long calibration and ANI, not ANI alone. Would it make sense to rename disable_ani to disable_calib and enable_ani to ani_supported?
>
> I understand short and long calibration of ANI, not the complete calibration :)
> disable_ani is to stop/start ANI anytime via ath9k debugfs entry.
> please let me know if you have further queries

I'm new to this code so I hope you'll forgive me if my question
sounds stupidly.
There is 'ath_ani_calibrate' which basically does three things
(apart from timer management boilerplate):
-- calls ath9k_hw_calibrate to perform 'long calibration'
-- calls ath9k_hw_calibrate to perform 'short calibration'
-- calls ath9k_hw_ani_monitor to get ANI statistics (?)
So there seem to be three major parts of ANI: long calibration,
short calibration, and third thing which I find hard to name correctly
so I'll refer to it as 'ANI-ANI'.

From your response I understand that 'short calibration' and 'long
calibration' generally considered to be part of think called ANI.
Considering this name 'disable_ani' in a sense that it disables
'ath_ani_calibrate' as a whole looks ok.
But config.enable_ani is used to control if 'ANI-ANI' gets executed
(and this was somewhat true before my patch).
So, having two variables which 'negated' names is confusing by
itself, i.e. enable_ani vs disable_ani. But more confusing is the fact
that they actually control different things and have different
meaning, so disable_ani != (!enable_ani).

I do understand that my 'ANI-ANI' probably has proper name and I
think that 'enable_ani' should probably be renamed using this proper
name.

I'd really appreciate if you could let me know where my
understanding is incorrect.
Thanks!

--
Truthfully yours,
Martynov Nikolay.
Email: [email protected]

2011-11-21 00:49:22

by Nikolay Martynov

[permalink] [raw]
Subject: [PATCH 1/4] ath9k: cosmetic fix in calibration debug log

---
drivers/net/wireless/ath/ath9k/ar9002_calib.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_calib.c b/drivers/net/wireless/ath/ath9k/ar9002_calib.c
index 88279e3..6cfc393 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_calib.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_calib.c
@@ -203,7 +203,7 @@ static void ar9002_hw_iqcalibrate(struct ath_hw *ah, u8 numChains)
i);

ath_dbg(common, ATH_DBG_CALIBRATE,
- "Orignal: Chn %diq_corr_meas = 0x%08x\n",
+ "Orignal: Chn %d iq_corr_meas = 0x%08x\n",
i, ah->totalIqCorrMeas[i]);

iqCorrNeg = 0;
--
1.7.4.1


2011-11-21 04:36:43

by Mohammed Shafi

[permalink] [raw]
Subject: Re: [PATCH 1/4] ath9k: cosmetic fix in calibration debug log

On Mon, Nov 21, 2011 at 6:18 AM, Nikolay Martynov <[email protected]> wrote:
> ---
> ?drivers/net/wireless/ath/ath9k/ar9002_calib.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ar9002_calib.c b/drivers/net/wireless/ath/ath9k/ar9002_calib.c
> index 88279e3..6cfc393 100644
> --- a/drivers/net/wireless/ath/ath9k/ar9002_calib.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9002_calib.c
> @@ -203,7 +203,7 @@ static void ar9002_hw_iqcalibrate(struct ath_hw *ah, u8 numChains)
> ? ? ? ? ? ? ? ? ? ? ? ?i);
>
> ? ? ? ? ? ? ? ?ath_dbg(common, ATH_DBG_CALIBRATE,
> - ? ? ? ? ? ? ? ? ? ? ? "Orignal: Chn %diq_corr_meas = 0x%08x\n",
> + ? ? ? ? ? ? ? ? ? ? ? "Orignal: Chn %d iq_corr_meas = 0x%08x\n",

thanks, please also change Orignal to Original :)
please make the same changes in ar9003_calib.c

> ? ? ? ? ? ? ? ? ? ? ? ?i, ah->totalIqCorrMeas[i]);
>
> ? ? ? ? ? ? ? ?iqCorrNeg = 0;
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
shafi