From patchwork Fri May 27 07:46:47 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Adrian Chadd X-Patchwork-Id: 823182 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p4R7koCd025425 for ; Fri, 27 May 2011 07:46:51 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752748Ab1E0Hqs (ORCPT ); Fri, 27 May 2011 03:46:48 -0400 Received: from mail-gx0-f174.google.com ([209.85.161.174]:54200 "EHLO mail-gx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751953Ab1E0Hqs convert rfc822-to-8bit (ORCPT ); Fri, 27 May 2011 03:46:48 -0400 Received: by gxk21 with SMTP id 21so593840gxk.19 for ; Fri, 27 May 2011 00:46:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=VYnKpXt+tdeUFHYR+pDPCqie5+u5QzyrduWd48RB0F4=; b=B6Ph8Tuz7vleraHKKumPSaFOBHu33uWtHIUCXNoYL4F59HzuVf9m82hwnF1yUpRh9a 6o0TNbggYWDWCNr3l2B8/CREi4IzfDSOKhck7UNdcW8aqgjNWu9gxcolyAoaxQOCgkwr DlXRx0v6SpCpwUZhCLDqnECwXC3NeHJhsLxbY= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=IExwLvTaxkGNq/m18ZHLBQGoy/wkEjz8GqvEBPVxVSRuTC0yAGP1XahKC91ZzWBCuJ jQUtA9K6r2tQE5Vzi+NlWEpB816pANJNRa0544FMaO3OYUzttrRHZKOK1IiX4Bv8IPa3 r1Jrh/0HKFAr1/UTbz1d427M3EEyDkhQXKbr8= MIME-Version: 1.0 Received: by 10.151.82.16 with SMTP id j16mr1892534ybl.356.1306482407422; Fri, 27 May 2011 00:46:47 -0700 (PDT) Received: by 10.151.107.15 with HTTP; Fri, 27 May 2011 00:46:47 -0700 (PDT) In-Reply-To: References: <24ECC725-EA49-48CA-8B9D-FDD71BD257A0@cs.washington.edu> Date: Fri, 27 May 2011 15:46:47 +0800 X-Google-Sender-Auth: dP0Rlz_5z0nB6lklACqlvwQKX_k Message-ID: Subject: Re: [PATCH/RFC] ath9k: fix two more bugs in tx power From: Adrian Chadd To: Mohammed Shafi Cc: Daniel Halperin , linux-wireless , stable@kernel.org, blaise@willowgarage.com Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Fri, 27 May 2011 07:46:55 +0000 (UTC) That should only really hit 0 if you're configuring a low TX power level.. ? Hm, I spy with my little eye a small bug with the eeprom power handling for the v14 eeprom. The modal header has: u8 pwrDecreaseFor2Chain; u8 pwrDecreaseFor3Chain; but the EEPROM code is hard-coded to use -6 for 2-chain reduction and -9 for 3-chain reduction. A completely untested patch (but FreeBSD does this) : @@ -924,6 +925,11 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah, twiceLargestAntenna = (int16_t)min(AntennaReduction - twiceLargestAntenna, 0); + pwrDecreaseFor2Chain = pEepData->modalHeader + [IS_CHAN_2GHZ(chan)].pwrDecreaseFor2Chain; + pwrDecreaseFor3Chain = pEepData->modalHeader + [IS_CHAN_2GHZ(chan)].pwrDecreaseFor3Chain; + maxRegAllowedPower = twiceMaxRegulatoryPower + twiceLargestAntenna; if (regulatory->tp_scale != ATH9K_TP_SCALE_MAX) { @@ -937,14 +943,14 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah, case 1: break; case 2: - if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN) - scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN; + if (scaledPower > pwrDecreaseFor2Chain) + scaledPower -= pwrDecreaseFor2Chain; else scaledPower = 0; break; case 3: - if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN) - scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN; + if (scaledPower > pwrDecreaseFor3Chain) + scaledPower -= pwrDecreaseFor3Chain; else scaledPower = 0; break; On 27 May 2011 15:18, Mohammed Shafi wrote: > On Thu, May 26, 2011 at 10:13 PM, Daniel Halperin > wrote: >> This is the same fix as >> >>    commit 841051602e3fa18ea468fe5a177aa92b6eb44b56 >>    Author: Matteo Croce >>    Date:   Fri Dec 3 02:25:08 2010 +0100 >> >>    The ath9k driver subtracts 3 dBm to the txpower as with two radios the >>    signal power is doubled. >>    The resulting value is assigned in an u16 which overflows and makes >>    the card work at full power. >> >> in two more places. I grepped the ath tree and didn't find any others. >> >> Cc: stable@kernel.org >> Signed-off-by: Daniel Halperin >> --- >> RFC: Blaise Gassend actually pointed these two bugs out on 12/7/2010 but no >> one fixed. There was some discussion about refactoring/improving this code, >> but it never seemed to get anywhere. See this thread: >> >>    http://comments.gmane.org/gmane.linux.drivers.ath9k.devel/4601 >> --- >>  drivers/net/wireless/ath/ath9k/ar9003_eeprom.c |   10 ++++++++-- >>  drivers/net/wireless/ath/ath9k/eeprom_9287.c   |   10 ++++++++-- >>  2 files changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c >> index 729534c..934e419 100644 >> --- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c >> +++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c >> @@ -4645,10 +4645,16 @@ static void ar9003_hw_set_power_per_rate_table(struct ath_hw *ah, >>        case 1: >>                break; >>        case 2: >> -               scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN; >> +               if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN) >> +                       scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN; >> +               else >> +                       scaledPower = 0; > > should we make the scaledPower as '0' lets have the first check if it > fails, let us retain the  scaledPower obtained by >  scaledPower = min(powerLimit, maxRegAllowedPower); > > am I missing something ? > making scaledPower affects this >   minCtlPower = (u8)min(twiceMaxEdgePower, scaledPower); > > which in turn affects >  pPwrArray[i] = >                                          (u8)min((u16)pPwrArray[i], >                                                  minCtlPower); > > which in turn affects target Power values > > > > > >>                break; >>        case 3: >> -               scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN; >> +               if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN) >> +                       scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN; >> +               else >> +                       scaledPower = 0; >>                break; >>        } >> >> diff --git a/drivers/net/wireless/ath/ath9k/eeprom_9287.c b/drivers/net/wireless/ath/ath9k/eeprom_9287.c >> index 7856f0d..343fc9f 100644 >> --- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c >> +++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c >> @@ -524,10 +524,16 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah, >>        case 1: >>                break; >>        case 2: >> -               scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN; >> +               if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN) >> +                       scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN; >> +               else >> +                       scaledPower = 0; >>                break; >>        case 3: >> -               scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN; >> +               if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN) >> +                       scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN; >> +               else >> +                       scaledPower = 0; >>                break; >>        } >>        scaledPower = max((u16)0, scaledPower); >> -- >> 1.7.0.4 >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at  http://vger.kernel.org/majordomo-info.html >> > > > > -- > shafi > --- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/wireless/ath/ath9k/eeprom_def.c b/drivers/net/wireless/ath/ath9k/eeprom_def.c index 17f0a68..09f587b 100644 --- a/drivers/net/wireless/ath/ath9k/eeprom_def.c +++ b/drivers/net/wireless/ath/ath9k/eeprom_def.c @@ -906,6 +906,7 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah, struct chan_centers centers; int tx_chainmask; u16 twiceMinEdgePower; + u8 pwrDecreaseFor2Chain, pwrDecreaseFor3Chain; tx_chainmask = ah->txchainmask;