[PATCH] ath5k: Set TSF fix

[PATCH] ath5k: Set TSF fix

[Alina Friedrichsen]

It seems that some Atheros hardware more like this code for setting the=
 TSF.

Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
---
diff -urN wireless-testing.orig/drivers/net/wireless/ath5k/pcu.c wirele=
ss-testing/drivers/net/wireless/ath5k/pcu.c
--- wireless-testing.orig/drivers/net/wireless/ath5k/pcu.c	2009-01-23 2=
2:54:07.000000000 +0100
+++ wireless-testing/drivers/net/wireless/ath5k/pcu.c	2009-01-24 08:47:=
16.000000000 +0100
@@ -657,9 +657,8 @@
 {
 	ATH5K_TRACE(ah->ah_sc);
=20
-	ath5k_hw_reg_write(ah, 0x00000000, AR5K_TSF_L32);
-	ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
 	ath5k_hw_reg_write(ah, tsf64 & 0xffffffff, AR5K_TSF_L32);
+	ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
 }
=20
 /**

--=20
NUR NOCH BIS 31.01.! GMX FreeDSL - Telefonanschluss + DSL=20
f=FCr nur 16,37 EURO/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T4=
569a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ath5k: Set TSF fix

[Bob Copeland]

On Sat, Jan 24, 2009 at 3:12 AM, Alina Friedrichsen <x-alina@gmx.net> wrote:
>  {
>        ATH5K_TRACE(ah->ah_sc);
>
> -       ath5k_hw_reg_write(ah, 0x00000000, AR5K_TSF_L32);
> -       ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
>        ath5k_hw_reg_write(ah, tsf64 & 0xffffffff, AR5K_TSF_L32);
> +       ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
>  }

Perhaps there is an internal latch on this pair; that may explain why
you need to write low->high order.

However, you don't need to write (tsf64 >> 32) & 0xffffffff; it's
unsigned so the mask is unnecessary.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: Set TSF fix

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On Sun, 2009-01-25 at 11:41 -0500, Bob Copeland wrote:
> On Sat, Jan 24, 2009 at 3:12 AM, Alina Friedrichsen <x-alina@gmx.net> wrote:
> >  {
> >        ATH5K_TRACE(ah->ah_sc);
> >
> > -       ath5k_hw_reg_write(ah, 0x00000000, AR5K_TSF_L32);
> > -       ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
> >        ath5k_hw_reg_write(ah, tsf64 & 0xffffffff, AR5K_TSF_L32);
> > +       ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
> >  }
> 
> Perhaps there is an internal latch on this pair; that may explain why
> you need to write low->high order.

Either way, I think the changelog should back up the claim by
_something_. Even if it's just "tested on device XXX, old way doesn't
work, new way does" or "looked at legacy HAL code" or ...

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ath5k: Set TSF fix

[Bob Copeland]

On Sun, Jan 25, 2009 at 11:45 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>> Perhaps there is an internal latch on this pair; that may explain why
>> you need to write low->high order.
>
> Either way, I think the changelog should back up the claim by
> _something_. Even if it's just "tested on device XXX, old way doesn't
> work, new way does" or "looked at legacy HAL code" or ...

Agreed, "tested on..." would be good.  I looked at the HAL and it
doesn't have a set tsf function, only reset.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: Set TSF fix

[Alina Friedrichsen]

Hello!

> > Either way, I think the changelog should back up the claim by
> > _something_. Even if it's just "tested on device XXX, old way doesn=
't
> > work, new way does" or "looked at legacy HAL code" or ...
>=20
> Agreed, "tested on..." would be good.  I looked at the HAL and it
> doesn't have a set tsf function, only reset.

The old code does not work on:
AR5418+AR2122 (ath9k)
AR5416+AR2133 (ath9k)

The new code is tested on:
AR5418+AR2122 (ath9k)
AR5416+AR2133 (ath9k)
AR5001X+ (ath5k)
AR5007EG (ath5k)

The old code was taken from:
https://dev.openwrt.org/browser/trunk/package/madwifi/patches/383-ibss_=
hostap.patch
(by Sven-Ola Tuecke)

Regards
Alina

--=20
NUR NOCH BIS 31.01.! GMX FreeDSL - Telefonanschluss + DSL=20
f=FCr nur 16,37 EURO/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T4=
569a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ath5k: Set TSF fix

[John W. Linville]

On Mon, Jan 26, 2009 at 03:01:30PM +0100, Alina Friedrichsen wrote:
> Hello!
> 
> > > Either way, I think the changelog should back up the claim by
> > > _something_. Even if it's just "tested on device XXX, old way doesn't
> > > work, new way does" or "looked at legacy HAL code" or ...
> > 
> > Agreed, "tested on..." would be good.  I looked at the HAL and it
> > doesn't have a set tsf function, only reset.
> 
> The old code does not work on:
> AR5418+AR2122 (ath9k)
> AR5416+AR2133 (ath9k)
> 
> The new code is tested on:
> AR5418+AR2122 (ath9k)
> AR5416+AR2133 (ath9k)
> AR5001X+ (ath5k)
> AR5007EG (ath5k)
> 
> The old code was taken from:
> https://dev.openwrt.org/browser/trunk/package/madwifi/patches/383-ibss_hostap.patch
> (by Sven-Ola Tuecke)

Could you resubmit this patch with some/all of the above information
incorporated into the changelog?  I'm dropping the current version
(with limited changelog) just to avoid my own confusion.

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

[PATCH] ath5k: Set TSF fix

[Alina Friedrichsen]

The old code doesn't work correctly e.g. on newer chipsets like AR5418+=
AR2122 and AR5416+AR2133.

Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
---
diff -urN wireless-testing.orig/drivers/net/wireless/ath5k/pcu.c wirele=
ss-testing/drivers/net/wireless/ath5k/pcu.c
--- wireless-testing.orig/drivers/net/wireless/ath5k/pcu.c	2009-01-23 2=
2:54:07.000000000 +0100
+++ wireless-testing/drivers/net/wireless/ath5k/pcu.c	2009-01-24 08:47:=
16.000000000 +0100
@@ -657,9 +657,8 @@
 {
 	ATH5K_TRACE(ah->ah_sc);
=20
-	ath5k_hw_reg_write(ah, 0x00000000, AR5K_TSF_L32);
-	ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
 	ath5k_hw_reg_write(ah, tsf64 & 0xffffffff, AR5K_TSF_L32);
+	ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
 }
=20
 /**

--=20
Computer Bild Tarifsieger! GMX FreeDSL - Telefonanschluss + DSL
f=FCr nur 17,95 =BF/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ath5k: Set TSF fix

[Bob Copeland]

On Mon, Mar 2, 2009 at 5:29 PM, Alina Friedrichsen <x-alina@gmx.net> wr=
ote:
> The old code doesn't work correctly e.g. on newer chipsets like AR541=
8+AR2122 and AR5416+AR2133.
>
> Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
> ---
> diff -urN wireless-testing.orig/drivers/net/wireless/ath5k/pcu.c wire=
less-testing/drivers/net/wireless/ath5k/pcu.c
> --- wireless-testing.orig/drivers/net/wireless/ath5k/pcu.c =A0 =A0 =A0=
2009-01-23 22:54:07.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/ath5k/pcu.c =A0 2009-01-24 =
08:47:16.000000000 +0100
> @@ -657,9 +657,8 @@
> =A0{
> =A0 =A0 =A0 =A0ATH5K_TRACE(ah->ah_sc);
>
> - =A0 =A0 =A0 ath5k_hw_reg_write(ah, 0x00000000, AR5K_TSF_L32);
> - =A0 =A0 =A0 ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K=
_TSF_U32);
> =A0 =A0 =A0 =A0ath5k_hw_reg_write(ah, tsf64 & 0xffffffff, AR5K_TSF_L3=
2);
> + =A0 =A0 =A0 ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K=
_TSF_U32);
> =A0}

It's in the original so I guess it's ok, but it'd be nice to have a ver=
sion
without the unnecessary "& 0xffffffff" part.  Also can you use the -p o=
ption
to diff in the future?

Otherwise,

Acked-by: Bob Copeland <me@bobcopeland.com>

--=20
Bob Copeland %% www.bobcopeland.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ath5k: Set TSF fix

[Alina Friedrichsen]

Hello Bob!

> It's in the original so I guess it's ok, but it'd be nice to have a
> version
> without the unnecessary "& 0xffffffff" part. =20

I personally think with the "& 0xffffffff" it's cleaner, because it's t=
ype independent. And the compiler should optimize it out, if the type i=
s really 32 bit wide.

> Also can you use the -p
> option
> to diff in the future?

This is an old patch, I now use git for it.

Regards
Alina

--=20
Computer Bild Tarifsieger! GMX FreeDSL - Telefonanschluss + DSL
f=FCr nur 17,95 =BF/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ath5k: Set TSF fix

[Bob Copeland]

On Mon, Mar 2, 2009 at 6:02 PM, Alina Friedrichsen <x-alina@gmx.net> wrote:
>
> I personally think with the "& 0xffffffff" it's cleaner, because it's type
> independent. And the compiler should optimize it out, if the type is really
> 32 bit wide.

Yes, the compiler will optimize it out, but it looks noisy to me.  It's
declared u64 one line above so it is clearly a no-op...  I guess it is a
personal taste thing.

> This is an old patch, I now use git for it.

Ok, thanks!  I only mentioned it because I wanted to see the parameters
and had to go back to the code.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: Set TSF fix

[Alina Friedrichsen]

Hello Bob!

> Yes, the compiler will optimize it out, but it looks noisy to me.  It=
's
> declared u64 one line above so it is clearly a no-op...  I guess it i=
s a
> personal taste thing.

Yes, in assembly I would agree. In higher languages I like to write mor=
e generally code.

> Ok, thanks!  I only mentioned it because I wanted to see the paramete=
rs
> and had to go back to the code.

ACK

Regards
Alina

--=20
Psssst! Schon vom neuen GMX MultiMessenger geh=F6rt? Der kann`s mit all=
en: http://www.gmx.net/de/go/multimessenger01
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html