2009-01-24 09:46:11

by Alina Friedrichsen

[permalink] [raw]
Subject: [PATCH] ath5k: Set TSF fix

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

Signed-off-by: Alina Friedrichsen <[email protected]>
---
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


2009-01-25 16:45:24

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

On Sun, 2009-01-25 at 11:41 -0500, Bob Copeland wrote:
> On Sat, Jan 24, 2009 at 3:12 AM, Alina Friedrichsen <[email protected]> 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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-01-25 16:47:33

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

On Sun, Jan 25, 2009 at 11:45 AM, Johannes Berg
<[email protected]> 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 %% http://www.bobcopeland.com

2009-01-27 21:30:48

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

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
[email protected] might be all we have. Be ready.

2009-01-26 14:01:33

by Alina Friedrichsen

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

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

2009-01-25 16:41:11

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

On Sat, Jan 24, 2009 at 3:12 AM, Alina Friedrichsen <[email protected]> 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 %% http://www.bobcopeland.com

2009-03-02 23:31:53

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

On Mon, Mar 2, 2009 at 6:02 PM, Alina Friedrichsen <[email protected]> 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 %% http://www.bobcopeland.com

2009-03-02 23:02:35

by Alina Friedrichsen

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

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

2009-03-02 22:44:10

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

On Mon, Mar 2, 2009 at 5:29 PM, Alina Friedrichsen <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>

--=20
Bob Copeland %% http://www.bobcopeland.com

2009-03-03 21:45:15

by Alina Friedrichsen

[permalink] [raw]
Subject: Re: [PATCH] ath5k: Set TSF fix

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