linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ar9170: implement frequency calibration for one-stage/openfw
@ 2009-09-03 18:25 Christian Lamparter
  2009-09-19  0:02 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Lamparter @ 2009-09-03 18:25 UTC (permalink / raw)
  To: linux-wireless; +Cc: John Linville

This patch ports some code from the vendor driver, which is
supposed to upload the right calibration values for the
chosen frequency.

In theory, this should give a better range and throughput
for all users with the open, or one-stage firmware.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
index df86f70..be96f50 100644
--- a/drivers/net/wireless/ath/ar9170/phy.c
+++ b/drivers/net/wireless/ath/ar9170/phy.c
@@ -987,6 +987,124 @@ static u8 ar9170_interpolate_u8(u8 x, u8 x1, u8 y1, u8 x2, u8 y2)
 #undef SHIFT
 }
 
+static u8 ar9170_interpolate_val(u8 x, u8 *x_array, u8 *y_array)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		if (x <= x_array[i + 1])
+			break;
+
+	return ar9170_interpolate_u8(x,
+				     x_array[i],
+				     y_array[i],
+				     x_array[i + 1],
+				     y_array[i + 1]);
+}
+
+static int ar9170_set_freq_cal_data(struct ar9170 *ar,
+				    struct ieee80211_channel *channel)
+{
+	u8 *cal_freq_pier;
+	u8 vpds[2][AR5416_PD_GAIN_ICEPTS];
+	u8 pwrs[2][AR5416_PD_GAIN_ICEPTS];
+	int chain, idx, i;
+	u8 f;
+
+	switch (channel->band) {
+	case IEEE80211_BAND_2GHZ:
+		f = channel->center_freq - 2300;
+		cal_freq_pier = ar->eeprom.cal_freq_pier_2G;
+		i = AR5416_NUM_2G_CAL_PIERS - 1;
+		break;
+
+	case IEEE80211_BAND_5GHZ:
+		f = (channel->center_freq - 4800) / 5;
+		cal_freq_pier = ar->eeprom.cal_freq_pier_5G;
+		i = AR5416_NUM_5G_CAL_PIERS - 1;
+		break;
+
+	default:
+		return -EINVAL;
+		break;
+	}
+
+	for (; i >= 0; i--) {
+		if (cal_freq_pier[i] != 0xff)
+			break;
+	}
+	if (i < 0)
+		return -EINVAL;
+
+	idx = ar9170_find_freq_idx(i, cal_freq_pier, f);
+
+	ar9170_regwrite_begin(ar);
+
+	for (chain = 0; chain < AR5416_MAX_CHAINS; chain++) {
+		for (i = 0; i < AR5416_PD_GAIN_ICEPTS; i++) {
+			struct ar9170_calibration_data_per_freq *cal_pier_data;
+			int j;
+
+			switch (channel->band) {
+			case IEEE80211_BAND_2GHZ:
+				cal_pier_data = &ar->eeprom.
+					cal_pier_data_2G[chain][idx];
+				break;
+
+			case IEEE80211_BAND_5GHZ:
+				cal_pier_data = &ar->eeprom.
+					cal_pier_data_5G[chain][idx];
+				break;
+
+			default:
+				return -EINVAL;
+			}
+
+			for (j = 0; j < 2; j++) {
+				vpds[j][i] = ar9170_interpolate_u8(f,
+					cal_freq_pier[idx],
+					cal_pier_data->vpd_pdg[j][i],
+					cal_freq_pier[idx + 1],
+					cal_pier_data[1].vpd_pdg[j][i]);
+
+				pwrs[j][i] = ar9170_interpolate_u8(f,
+					cal_freq_pier[idx],
+					cal_pier_data->pwr_pdg[j][i],
+					cal_freq_pier[idx + 1],
+					cal_pier_data[1].pwr_pdg[j][i]) / 2;
+			}
+		}
+
+		for (i = 0; i < 76; i++) {
+			u32 phy_data;
+			u8 tmp;
+
+			if (i < 25) {
+				tmp = ar9170_interpolate_val(i, &pwrs[0][0],
+							     &vpds[0][0]);
+			} else {
+				tmp = ar9170_interpolate_val(i - 12,
+							     &pwrs[1][0],
+							     &vpds[1][0]);
+			}
+
+			phy_data |= tmp << ((i & 3) << 3);
+			if ((i & 3) == 3) {
+				ar9170_regwrite(0x1c6280 + chain * 0x1000 +
+						(i & ~3), phy_data);
+				phy_data = 0;
+			}
+		}
+
+		for (i = 19; i < 32; i++)
+			ar9170_regwrite(0x1c6280 + chain * 0x1000 + (i << 2),
+					0x0);
+	}
+
+	ar9170_regwrite_finish();
+	return ar9170_regwrite_result();
+}
+
 static int ar9170_set_power_cal(struct ar9170 *ar, u32 freq, enum ar9170_bw bw)
 {
 	struct ar9170_calibration_target_power_legacy *ctpl;
@@ -1207,6 +1325,10 @@ int ar9170_set_channel(struct ar9170 *ar, struct ieee80211_channel *channel,
 	if (err)
 		return err;
 
+	err = ar9170_set_freq_cal_data(ar, channel);
+	if (err)
+		return err;
+
 	err = ar9170_set_power_cal(ar, channel->center_freq, bw);
 	if (err)
 		return err;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ar9170: implement frequency calibration for one-stage/openfw
  2009-09-03 18:25 [PATCH] ar9170: implement frequency calibration for one-stage/openfw Christian Lamparter
@ 2009-09-19  0:02 ` Andrew Morton
  2009-09-19 12:21   ` Joerg Albert
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2009-09-19  0:02 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless, linville, David S. Miller

On Thu, 3 Sep 2009 20:25:31 +0200
Christian Lamparter <chunkeey@googlemail.com> wrote:

> This patch ports some code from the vendor driver, which is
> supposed to upload the right calibration values for the
> chosen frequency.
> 
> In theory, this should give a better range and throughput
> for all users with the open, or one-stage firmware.
> 
> ...
>
> +		for (i = 0; i < 76; i++) {
> +			u32 phy_data;
> +			u8 tmp;
> +
> +			if (i < 25) {
> +				tmp = ar9170_interpolate_val(i, &pwrs[0][0],
> +							     &vpds[0][0]);
> +			} else {
> +				tmp = ar9170_interpolate_val(i - 12,
> +							     &pwrs[1][0],
> +							     &vpds[1][0]);
> +			}
> +
> +			phy_data |= tmp << ((i & 3) << 3);

Clearly buggy and the compiler warns.  The value of phy_data is unknown
here.

How did this get all the way into mainline?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ar9170: implement frequency calibration for one-stage/openfw
  2009-09-19  0:02 ` Andrew Morton
@ 2009-09-19 12:21   ` Joerg Albert
  2009-09-28 18:41     ` John W. Linville
  0 siblings, 1 reply; 6+ messages in thread
From: Joerg Albert @ 2009-09-19 12:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christian Lamparter, linux-wireless, linville, David S. Miller

On 09/19/2009 02:02 AM, Andrew Morton wrote:
> On Thu, 3 Sep 2009 20:25:31 +0200
> Christian Lamparter <chunkeey@googlemail.com> wrote:
> 
>> This patch ports some code from the vendor driver, which is
>> supposed to upload the right calibration values for the
>> chosen frequency.
>>
>> In theory, this should give a better range and throughput
>> for all users with the open, or one-stage firmware.
>>
>> ...
>>
>> +		for (i = 0; i < 76; i++) {
>> +			u32 phy_data;
>> +			u8 tmp;
>> +
>> +			if (i < 25) {
>> +				tmp = ar9170_interpolate_val(i, &pwrs[0][0],
>> +							     &vpds[0][0]);
>> +			} else {
>> +				tmp = ar9170_interpolate_val(i - 12,
>> +							     &pwrs[1][0],
>> +							     &vpds[1][0]);
>> +			}
>> +
>> +			phy_data |= tmp << ((i & 3) << 3);
> 
> Clearly buggy and the compiler warns.  The value of phy_data is unknown
> here.
> 
> How did this get all the way into mainline?

Strangely it compiles without any warning for me with the latest linux-wireless:

 joerg@nc10:~/src/wireless.gits/wireless-testing$ gcc --version
gcc (Ubuntu 4.3.3-5ubuntu4) 4.3.3
joerg@nc10:~/src/wireless.gits/wireless-testing$ git describe
v2.6.31-38258-g389ce02
joerg@nc10:~/src/wireless.gits/wireless-testing/drivers/net/wireless/ath/ar9170$ grep phy_data phy.c
                        u32 phy_data;
                        phy_data |= tmp << ((i & 3) << 3);
                                                (i & ~3), phy_data);
                                phy_data = 0;

joerg@nc10:~/src/wireless.gits/wireless-testing$ make modules
...
  CC [M]  drivers/net/wireless/ath/ar9170/mac.o
  CC [M]  drivers/net/wireless/ath/ar9170/phy.o
  CC [M]  drivers/net/wireless/ath/ar9170/led.o

make V=1 says:

  gcc -Wp,-MD,drivers/net/wireless/ath/ar9170/.phy.o.d  -nostdinc -isystem 
/usr/lib/gcc/i486-linux-gnu/4.3.3/include -Iinclude  
-I/home/joerg/src/wireless.gits/wireless-testing/arch/x86/include -include 
include/linux/autoconf.h -D__KERNEL__ -Wall -Wundef -Wstrict-prototypes 
-Wno-trigraphs -fno-strict-aliasing -fno-common 
-Werror-implicit-function-declaration -Wno-format-security 
-fno-delete-null-pointer-checks -O2 -m32 -msoft-float -mregparm=3 
-freg-struct-return -mpreferred-stack-boundary=2 -march=i686 -mtune=pentium3 
-mtune=generic -Wa,-mtune=generic32 -ffreestanding -pipe -Wno-sign-compare 
-fno-asynchronous-unwind-tables -mno-sse -mno-mmx -mno-sse2 -mno-3dnow 
-fno-stack-protector -fno-omit-frame-pointer -fno-optimize-sibling-calls -g 
-Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow  -DMODULE 
-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(phy)"  
-D"KBUILD_MODNAME=KBUILD_STR(ar9170usb)"  -c -o 
drivers/net/wireless/ath/ar9170/.tmp_phy.o drivers/net/wireless/ath/ar9170/phy.c


Adding "-Wuninitialized" didn't help, acc. to the doc it is switched on
with -Wall for -O2.
Is this a regression of gcc?

Regards,
Jörg.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ar9170: implement frequency calibration for one-stage/openfw
  2009-09-19 12:21   ` Joerg Albert
@ 2009-09-28 18:41     ` John W. Linville
  2009-09-28 19:45       ` Christian Lamparter
  0 siblings, 1 reply; 6+ messages in thread
From: John W. Linville @ 2009-09-28 18:41 UTC (permalink / raw)
  To: Joerg Albert
  Cc: Andrew Morton, Christian Lamparter, linux-wireless,
	David S. Miller

On Sat, Sep 19, 2009 at 02:21:36PM +0200, Joerg Albert wrote:
> On 09/19/2009 02:02 AM, Andrew Morton wrote:
> > On Thu, 3 Sep 2009 20:25:31 +0200
> > Christian Lamparter <chunkeey@googlemail.com> wrote:
> > 
> >> This patch ports some code from the vendor driver, which is
> >> supposed to upload the right calibration values for the
> >> chosen frequency.
> >>
> >> In theory, this should give a better range and throughput
> >> for all users with the open, or one-stage firmware.
> >>
> >> ...
> >>
> >> +		for (i = 0; i < 76; i++) {
> >> +			u32 phy_data;
> >> +			u8 tmp;
> >> +
> >> +			if (i < 25) {
> >> +				tmp = ar9170_interpolate_val(i, &pwrs[0][0],
> >> +							     &vpds[0][0]);
> >> +			} else {
> >> +				tmp = ar9170_interpolate_val(i - 12,
> >> +							     &pwrs[1][0],
> >> +							     &vpds[1][0]);
> >> +			}
> >> +
> >> +			phy_data |= tmp << ((i & 3) << 3);
> > 
> > Clearly buggy and the compiler warns.  The value of phy_data is unknown
> > here.
> > 
> > How did this get all the way into mainline?
> 
> Strangely it compiles without any warning for me with the latest linux-wireless:

That is strange -- no warning here (F-11) either...

Christian, care to propose a patch?  Or maybe just something like this?

diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
index b3e5cf3..49c10cb 100644
--- a/drivers/net/wireless/ath/ar9170/phy.c
+++ b/drivers/net/wireless/ath/ar9170/phy.c
@@ -1220,7 +1220,7 @@ static int ar9170_set_freq_cal_data(struct ar9170 *ar,
 							     &vpds[1][0]);
 			}
 
-			phy_data |= tmp << ((i & 3) << 3);
+			phy_data = tmp << ((i & 3) << 3);
 			if ((i & 3) == 3) {
 				ar9170_regwrite(0x1c6280 + chain * 0x1000 +
 						(i & ~3), phy_data);

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ar9170: implement frequency calibration for one-stage/openfw
  2009-09-28 18:41     ` John W. Linville
@ 2009-09-28 19:45       ` Christian Lamparter
  2009-09-28 19:50         ` John W. Linville
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Lamparter @ 2009-09-28 19:45 UTC (permalink / raw)
  To: John W. Linville
  Cc: Joerg Albert, Andrew Morton, linux-wireless, David S. Miller

2009/9/28 John W. Linville <linville@tuxdriver.com>:
> On Sat, Sep 19, 2009 at 02:21:36PM +0200, Joerg Albert wrote:
>> On 09/19/2009 02:02 AM, Andrew Morton wrote:
>> > On Thu, 3 Sep 2009 20:25:31 +0200
>> > Christian Lamparter <chunkeey@googlemail.com> wrote:
>> >
>> >> This patch ports some code from the vendor driver, which is
>> >> supposed to upload the right calibration values for the
>> >> chosen frequency.
>> >>
>> >> In theory, this should give a better range and throughput
>> >> for all users with the open, or one-stage firmware.
>> >>
>> >> ...
>> >>
>> >> +          for (i = 0; i < 76; i++) {
>> >> +                  u32 phy_data;
>> >> +                  u8 tmp;
>> >> +
>> >> +                  if (i < 25) {
>> >> +                          tmp = ar9170_interpolate_val(i, &pwrs[0][0],
>> >> +                                                       &vpds[0][0]);
>> >> +                  } else {
>> >> +                          tmp = ar9170_interpolate_val(i - 12,
>> >> +                                                       &pwrs[1][0],
>> >> +                                                       &vpds[1][0]);
>> >> +                  }
>> >> +
>> >> +                  phy_data |= tmp << ((i & 3) << 3);
>> >
>> > Clearly buggy and the compiler warns.  The value of phy_data is unknown
>> > here.
>> >
>> > How did this get all the way into mainline?
>>
>> Strangely it compiles without any warning for me with the latest linux-wireless:
>
> That is strange -- no warning here (F-11) either...
>
> Christian, care to propose a patch?  Or maybe just something like this?
>
> diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
> index b3e5cf3..49c10cb 100644
> --- a/drivers/net/wireless/ath/ar9170/phy.c
> +++ b/drivers/net/wireless/ath/ar9170/phy.c
> @@ -1220,7 +1220,7 @@ static int ar9170_set_freq_cal_data(struct ar9170 *ar,
>                                                             &vpds[1][0]);
>                        }
>
> -                       phy_data |= tmp << ((i & 3) << 3);
> +                       phy_data = tmp << ((i & 3) << 3);
>                        if ((i & 3) == 3) {
>                                ar9170_regwrite(0x1c6280 + chain * 0x1000 +
>                                                (i & ~3), phy_data);
>
> John
no? Andrew Morton's mail included a tiny patch which initializes the
phy_data to 0. I assumed you would pick it up right away...
Or is there something which would prevent it from including?

Regards,
         Chr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ar9170: implement frequency calibration for one-stage/openfw
  2009-09-28 19:45       ` Christian Lamparter
@ 2009-09-28 19:50         ` John W. Linville
  0 siblings, 0 replies; 6+ messages in thread
From: John W. Linville @ 2009-09-28 19:50 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Joerg Albert, Andrew Morton, linux-wireless, David S. Miller

On Mon, Sep 28, 2009 at 09:45:28PM +0200, Christian Lamparter wrote:
> 2009/9/28 John W. Linville <linville@tuxdriver.com>:
> > On Sat, Sep 19, 2009 at 02:21:36PM +0200, Joerg Albert wrote:
> >> On 09/19/2009 02:02 AM, Andrew Morton wrote:

> >> > How did this get all the way into mainline?
> >>
> >> Strangely it compiles without any warning for me with the latest linux-wireless:
> >
> > That is strange -- no warning here (F-11) either...
> >
> > Christian, care to propose a patch?  Or maybe just something like this?
> >
> > diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
> > index b3e5cf3..49c10cb 100644
> > --- a/drivers/net/wireless/ath/ar9170/phy.c
> > +++ b/drivers/net/wireless/ath/ar9170/phy.c
> > @@ -1220,7 +1220,7 @@ static int ar9170_set_freq_cal_data(struct ar9170 *ar,
> >                                                             &vpds[1][0]);
> >                        }
> >
> > -                       phy_data |= tmp << ((i & 3) << 3);
> > +                       phy_data = tmp << ((i & 3) << 3);
> >                        if ((i & 3) == 3) {
> >                                ar9170_regwrite(0x1c6280 + chain * 0x1000 +
> >                                                (i & ~3), phy_data);
> >
> > John
> no? Andrew Morton's mail included a tiny patch which initializes the
> phy_data to 0. I assumed you would pick it up right away...
> Or is there something which would prevent it from including?

Don't know what email you got, but there was no patch in my inbox...

So, I presume you have no objection to the patch above?  Initializing
phy_data just to unconditionally assign to it doesn't seem any better.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-09-28 20:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-03 18:25 [PATCH] ar9170: implement frequency calibration for one-stage/openfw Christian Lamparter
2009-09-19  0:02 ` Andrew Morton
2009-09-19 12:21   ` Joerg Albert
2009-09-28 18:41     ` John W. Linville
2009-09-28 19:45       ` Christian Lamparter
2009-09-28 19:50         ` John W. Linville

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).