2016-01-23 05:35:53

by Nikolay Martynov

[permalink] [raw]
Subject: [PATCH] iwldvm: fix chain gain calibration when firmware return zero values

From: Nikolay Martynov <(none)>

It looks like sometimes firmware returns zero for chain noise and signal
during calibration period. This seems to be a known problem and current
implementation accounts for this by ignoring invalid data when all chains
return zero signal and noise.

The problem is that sometimes firmware returns zero for only one chain for
some (not all) beacons used for calibration. This leads to perfectly valid
chains be disabled and may cause invalid gain settings.
For example this is calibration data taken on laptop with Intel 6300 card
with all three antennas attached:

active_chains: 3
chain_noise_a: 312
chain_noise_b: 297
chain_noise_c: 0
chain_signal_a: 549
chain_signal_b: 513
chain_signal_c: 0
beacon_count: 16
disconn_array: 0 0 1
delta_gain_code: 4 0 0
radio_write: 1
state: 3

This patch changes statistics gathering to make sure that zero noise
results are ignored for valid rx chains. The rationale being that even if
anntenna is not connected we should be able to see non zero noise if rx
chain is present.

This patch fixes the problem of disabling working chains on hardware I
have (6300 and 5300). It also works fine in case one 3-chain hardware has
only two antennas attached.

Signed-off-by: Nikolay Martynov <[email protected]>
---
drivers/net/wireless/iwlwifi/dvm/calib.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/net/wireless/iwlwifi/dvm/calib.c b/drivers/net/wireless/iwlwifi/dvm/calib.c
index 20e6aa9..59e73e2 100644
--- a/drivers/net/wireless/iwlwifi/dvm/calib.c
+++ b/drivers/net/wireless/iwlwifi/dvm/calib.c
@@ -1026,6 +1026,18 @@ void iwl_chain_noise_calibration(struct iwl_priv *priv)

spin_unlock_bh(&priv->statistics.lock);

+ /* Sometimes firmware returns zero for chain noise and signal
+ * even if chain is present and antenna is connected. This
+ * often results in perfectly valid chains being disabled.
+ * Ignore statistics if it contains zero noise for valid rx
+ * chain: even with antenna disconnected we should hear some noise.
+ */
+ if (((priv->nvm_data->valid_rx_ant & (1 << 0)) && chain_noise_a == 0) ||
+ ((priv->nvm_data->valid_rx_ant & (2 << 0)) && chain_noise_b == 0) ||
+ ((priv->nvm_data->valid_rx_ant & (3 << 0)) && chain_noise_c == 0)) {
+ return;
+ }
+
data->beacon_count++;

data->chain_noise_a = (chain_noise_a + data->chain_noise_a);
--
2.7.0



2016-01-24 22:40:11

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH] iwldvm: fix chain gain calibration when firmware return zero values

Hi Nikolay,

A couple of minor points:

On Sat, Jan 23, 2016 at 4:35 PM, Nikolay Martynov <[email protected]> wrote:
> From: Nikolay Martynov <(none)>

You really need an email address here instead of "(none)" as this will
be used for the author field of the eventual commit. Check the "user"
section of your git configuration. (Try git config --list)

> It looks like sometimes firmware returns zero for chain noise and signal
> during calibration period. This seems to be a known problem and current
> implementation accounts for this by ignoring invalid data when all chains
> return zero signal and noise.
>
> The problem is that sometimes firmware returns zero for only one chain for
> some (not all) beacons used for calibration. This leads to perfectly valid
> chains be disabled and may cause invalid gain settings.
> For example this is calibration data taken on laptop with Intel 6300 card
> with all three antennas attached:
>
> active_chains: 3
> chain_noise_a: 312
> chain_noise_b: 297
> chain_noise_c: 0
> chain_signal_a: 549
> chain_signal_b: 513
> chain_signal_c: 0
> beacon_count: 16
> disconn_array: 0 0 1
> delta_gain_code: 4 0 0
> radio_write: 1
> state: 3
>
> This patch changes statistics gathering to make sure that zero noise
> results are ignored for valid rx chains. The rationale being that even if
> anntenna is not connected we should be able to see non zero noise if rx
> chain is present.
>
> This patch fixes the problem of disabling working chains on hardware I
> have (6300 and 5300). It also works fine in case one 3-chain hardware has
> only two antennas attached.
>
> Signed-off-by: Nikolay Martynov <[email protected]>
> ---
> drivers/net/wireless/iwlwifi/dvm/calib.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/wireless/iwlwifi/dvm/calib.c b/drivers/net/wireless/iwlwifi/dvm/calib.c
> index 20e6aa9..59e73e2 100644
> --- a/drivers/net/wireless/iwlwifi/dvm/calib.c
> +++ b/drivers/net/wireless/iwlwifi/dvm/calib.c
> @@ -1026,6 +1026,18 @@ void iwl_chain_noise_calibration(struct iwl_priv *priv)
>
> spin_unlock_bh(&priv->statistics.lock);
>
> + /* Sometimes firmware returns zero for chain noise and signal
> + * even if chain is present and antenna is connected. This
> + * often results in perfectly valid chains being disabled.
> + * Ignore statistics if it contains zero noise for valid rx
> + * chain: even with antenna disconnected we should hear some noise.
> + */
> + if (((priv->nvm_data->valid_rx_ant & (1 << 0)) && chain_noise_a == 0) ||
> + ((priv->nvm_data->valid_rx_ant & (2 << 0)) && chain_noise_b == 0) ||
> + ((priv->nvm_data->valid_rx_ant & (3 << 0)) && chain_noise_c == 0)) {

To expand on Emmanuel's comment, 3 << 0 is equivalent to 3. If you're
trying to set specific bits then you need to use the BIT() macro,
otherwise just get rid of the "<< 0"s.

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/

2016-01-24 20:07:32

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: [PATCH] iwldvm: fix chain gain calibration when firmware return zero values

Hi,

Note that Wey-Yi has stepped down from maintaining iwlwifi for a while now.

On Sat, Jan 23, 2016 at 7:35 AM, Nikolay Martynov <[email protected]> wrote:
> From: Nikolay Martynov <(none)>
>
> It looks like sometimes firmware returns zero for chain noise and signal
> during calibration period. This seems to be a known problem and current
> implementation accounts for this by ignoring invalid data when all chains
> return zero signal and noise.
>
> The problem is that sometimes firmware returns zero for only one chain for
> some (not all) beacons used for calibration. This leads to perfectly valid
> chains be disabled and may cause invalid gain settings.
> For example this is calibration data taken on laptop with Intel 6300 card
> with all three antennas attached:
>
> active_chains: 3
> chain_noise_a: 312
> chain_noise_b: 297
> chain_noise_c: 0
> chain_signal_a: 549
> chain_signal_b: 513
> chain_signal_c: 0
> beacon_count: 16
> disconn_array: 0 0 1
> delta_gain_code: 4 0 0
> radio_write: 1
> state: 3
>
> This patch changes statistics gathering to make sure that zero noise
> results are ignored for valid rx chains. The rationale being that even if
> anntenna is not connected we should be able to see non zero noise if rx
> chain is present.
>
> This patch fixes the problem of disabling working chains on hardware I
> have (6300 and 5300). It also works fine in case one 3-chain hardware has
> only two antennas attached.

So how will that work if an antenna is disconnected? You always seem
to ignore the sampling that returned 0.

>
> Signed-off-by: Nikolay Martynov <[email protected]>
> ---
> drivers/net/wireless/iwlwifi/dvm/calib.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/wireless/iwlwifi/dvm/calib.c b/drivers/net/wireless/iwlwifi/dvm/calib.c
> index 20e6aa9..59e73e2 100644
> --- a/drivers/net/wireless/iwlwifi/dvm/calib.c
> +++ b/drivers/net/wireless/iwlwifi/dvm/calib.c
> @@ -1026,6 +1026,18 @@ void iwl_chain_noise_calibration(struct iwl_priv *priv)
>
> spin_unlock_bh(&priv->statistics.lock);
>
> + /* Sometimes firmware returns zero for chain noise and signal
> + * even if chain is present and antenna is connected. This
> + * often results in perfectly valid chains being disabled.
> + * Ignore statistics if it contains zero noise for valid rx
> + * chain: even with antenna disconnected we should hear some noise.
> + */
> + if (((priv->nvm_data->valid_rx_ant & (1 << 0)) && chain_noise_a == 0) ||
> + ((priv->nvm_data->valid_rx_ant & (2 << 0)) && chain_noise_b == 0) ||
> + ((priv->nvm_data->valid_rx_ant & (3 << 0)) && chain_noise_c == 0)) {
> + return;
> + }

I guess this is a typo, you wanted BIT(1), BIT(2) and BIT(3), right?

> +
> data->beacon_count++;
>
> data->chain_noise_a = (chain_noise_a + data->chain_noise_a);
> --
> 2.7.0
>
> --
> 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

2016-01-25 22:19:42

by Nikolay Martynov

[permalink] [raw]
Subject: [PATCH v2] iwldvm: fix chain gain calibration when firmware return zero values

It looks like sometimes firmware returns zero for chain noise and signal
during calibration period. This seems to be a known problem and current
implementation accounts for this by ignoring invalid data when all chains
return zero signal and noise.

The problem is that sometimes firmware returns zero for only one chain for
some (not all) beacons used for calibration. This leads to perfectly valid
chains be disabled and may cause invalid gain settings.
For example this is calibration data taken on laptop with Intel 6300 card
with all three antennas attached:

active_chains: 3
chain_noise_a: 312
chain_noise_b: 297
chain_noise_c: 0
chain_signal_a: 549
chain_signal_b: 513
chain_signal_c: 0
beacon_count: 16
disconn_array: 0 0 1
delta_gain_code: 4 0 0
radio_write: 1
state: 3

This patch changes statistics gathering to make sure that zero noise
results are ignored for valid rx chains. The rationale being that even if
anntenna is not connected we should be able to see non zero noise if rx
chain is present.

This patch fixes the problem of disabling working chains on hardware I
have (6300 and 5300). With three connected antennas statistics in
chain_noise looks like this (6300):

active_chains: 7
chain_noise_a: 321
chain_noise_b: 243
chain_noise_c: 311
chain_signal_a: 559
chain_signal_b: 452
chain_signal_c: 512
beacon_count: 16
disconn_array: 0 0 0
delta_gain_code: 4 3 0
radio_write: 1
state: 3

It also works fine in case one 3-chain hardware has only two antennas
attached (tested in 5300):

active_chains: 3
chain_noise_a: 238
chain_noise_b: 234
chain_noise_c: 219
chain_signal_a: 533
chain_signal_b: 514
chain_signal_c: 206
beacon_count: 16
disconn_array: 0 0 1
delta_gain_code: 4 0 0
radio_write: 1
state: 3

Signed-off-by: Nikolay Martynov <[email protected]>
---
drivers/net/wireless/iwlwifi/dvm/calib.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/net/wireless/iwlwifi/dvm/calib.c b/drivers/net/wireless/iwlwifi/dvm/calib.c
index 20e6aa9..cdaa88d 100644
--- a/drivers/net/wireless/iwlwifi/dvm/calib.c
+++ b/drivers/net/wireless/iwlwifi/dvm/calib.c
@@ -1026,6 +1026,18 @@ void iwl_chain_noise_calibration(struct iwl_priv *priv)

spin_unlock_bh(&priv->statistics.lock);

+ /* Sometimes firmware returns zero for chain noise and signal
+ * even if chain is present and antenna is connected. This
+ * often results in perfectly valid chains being disabled.
+ * Ignore statistics if it contains zero noise for valid rx
+ * chain: even with antenna disconnected we should hear some noise.
+ */
+ if ((priv->nvm_data->valid_rx_ant & ANT_A && chain_noise_a == 0) ||
+ (priv->nvm_data->valid_rx_ant & ANT_B && chain_noise_b == 0) ||
+ (priv->nvm_data->valid_rx_ant & ANT_C && chain_noise_c == 0)) {
+ return;
+ }
+
data->beacon_count++;

data->chain_noise_a = (chain_noise_a + data->chain_noise_a);
--
2.7.0