2008-09-23 00:14:31

by Tomas Winkler

[permalink] [raw]
Subject: [PATCH 1/1] iwlwifi 2.6.27: send all the calibration results instead of the last one only

From: Emmanuel Grumbach <[email protected]>

This patch solves a bug in calibration. We get three results from the
initial microcode, but we freed all the results upon each reception leading
to zero two of them, only the last one survived. This fix is user visible in
HT rates.

This patch simplifed version for 2.6.27 and will create conflict with
86316f2fe474b019cc445211980f5394977b34d5 iwlwifi: generic init calibrations framework

Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
---
Note this patch effects only 5000 HW (nor 3946 or 4965) so doesn't resolve or create any
regression. Anyhow it was thoroughly tested.

drivers/net/wireless/iwlwifi/iwl-5000.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index b08036a..0d027c1 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -512,23 +512,24 @@ static void iwl5000_rx_calib_result(struct iwl_priv *priv,
struct iwl5000_calib_hdr *hdr = (struct iwl5000_calib_hdr *)pkt->u.raw;
int len = le32_to_cpu(pkt->len) & FH_RSCSR_FRAME_SIZE_MSK;

- iwl_free_calib_results(priv);
-
/* reduce the size of the length field itself */
len -= 4;

switch (hdr->op_code) {
case IWL5000_PHY_CALIBRATE_LO_CMD:
+ kfree(priv->calib_results.lo_res);
priv->calib_results.lo_res = kzalloc(len, GFP_ATOMIC);
priv->calib_results.lo_res_len = len;
memcpy(priv->calib_results.lo_res, pkt->u.raw, len);
break;
case IWL5000_PHY_CALIBRATE_TX_IQ_CMD:
+ kfree(priv->calib_results.tx_iq_res);
priv->calib_results.tx_iq_res = kzalloc(len, GFP_ATOMIC);
priv->calib_results.tx_iq_res_len = len;
memcpy(priv->calib_results.tx_iq_res, pkt->u.raw, len);
break;
case IWL5000_PHY_CALIBRATE_TX_IQ_PERD_CMD:
+ kfree(priv->calib_results.tx_iq_perd_res);
priv->calib_results.tx_iq_perd_res = kzalloc(len, GFP_ATOMIC);
priv->calib_results.tx_iq_perd_res_len = len;
memcpy(priv->calib_results.tx_iq_perd_res, pkt->u.raw, len);
--
1.5.4.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.



2008-09-23 01:41:37

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/1] iwlwifi 2.6.27: send all the calibration results instead of the last one only

From: Tomas Winkler <[email protected]>
Date: Tue, 23 Sep 2008 03:14:28 +0300

> Note this patch effects only 5000 HW (nor 3946 or 4965) so doesn't
> resolve or create any regression. Anyhow it was thoroughly tested.

You haven't learned the first time.

This kind of stuff is not appropriate outside of the merge window, I'm
not pulling this change into my tree if John tries to send it to me.

2008-09-24 19:17:08

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 1/1] iwlwifi 2.6.27: send all the calibration results instead of the last one only

On Tue, Sep 23, 2008 at 03:14:28AM +0300, Tomas Winkler wrote:
> From: Emmanuel Grumbach <[email protected]>
>
> This patch solves a bug in calibration. We get three results from the
> initial microcode, but we freed all the results upon each reception leading
> to zero two of them, only the last one survived. This fix is user visible in
> HT rates.
>
> This patch simplifed version for 2.6.27 and will create conflict with
> 86316f2fe474b019cc445211980f5394977b34d5 iwlwifi: generic init calibrations framework

The code is different in wireless-next-2.6. Does this problem exist there?

John
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-09-25 10:45:31

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 1/1] iwlwifi 2.6.27: send all the calibration results instead of the last one only

On Wed, Sep 24, 2008 at 10:16 PM, John W. Linville
<[email protected]> wrote:
> On Tue, Sep 23, 2008 at 03:14:28AM +0300, Tomas Winkler wrote:
>> From: Emmanuel Grumbach <[email protected]>
>>
>> This patch solves a bug in calibration. We get three results from the
>> initial microcode, but we freed all the results upon each reception leading
>> to zero two of them, only the last one survived. This fix is user visible in
>> HT rates.
>>
>> This patch simplifed version for 2.6.27 and will create conflict with
>> 86316f2fe474b019cc445211980f5394977b34d5 iwlwifi: generic init calibrations framework
>
> The code is different in wireless-next-2.6. Does this problem exist there?

Oh sorry for confusion, the proper fix is
86316f2fe474b019cc445211980f5394977b34d5
but it was said it's too much code for 2.6.27 so this patch was to
address that and is minimal as possible.. If this is not in 2.6.27
just drop it.

Thanks
Tomas

2008-09-23 02:19:37

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 1/1] iwlwifi 2.6.27: send all the calibration results instead of the last one only

On Tue, Sep 23, 2008 at 4:41 AM, David Miller <[email protected]> wrote:
> From: Tomas Winkler <[email protected]>
> Date: Tue, 23 Sep 2008 03:14:28 +0300
>
>> Note this patch effects only 5000 HW (nor 3946 or 4965) so doesn't
>> resolve or create any regression. Anyhow it was thoroughly tested.
>
> You haven't learned the first time.
>
> This kind of stuff is not appropriate outside of the merge window, I'm
> not pulling this change into my tree if John tries to send it to me.

I really really really really don't understand what is wrong with this.
This is new hardware portion of the driver in 2.6.27 which can be even
compiled out with IWL5000. This was not part of 2.6.26. Sorry for
asking again how this is differing from ath9k patches just accepted.
Please answer this even if you have to be rude your way.

This is serious bug as radio is not configured properly affect greatly
radio performance up to disconnection.
We've limited the number of lines to minimum just to address THE
specific problem and tested it properly.
This bug was introduced in the merging window and discovered after it
was closed there is nothing I can do about that unfortunate timing.

Even if you don't accept this patch here it is, it fixes the problem
in 2.6.27 this is the bottom line, maybe someone find it useful.

Best regards
Tomas