According to tests, both TSF lower and upper registers kept counting, so
a rollover of the lower part can happen after the upper part has been
read, as shown in the following log where the upper part is read first
and the lower part next.
tsf = {00000003-fffffffd}
tsf = {00000003-00000001}
tsf = {00000004-0000000b}
This patch corrects this by reading the upper part once again in such
case. It has been tested in an IBSS network where artifical IBSS merges
have been done in order to trigger hundreds of rollover for the TSF
lower part.
Signed-off-by: Benoit Papillault <[email protected]>
---
drivers/net/wireless/ath/ath5k/pcu.c | 21 +++++++++++++++++++--
1 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
index aefe84f..4b24c15 100644
--- a/drivers/net/wireless/ath/ath5k/pcu.c
+++ b/drivers/net/wireless/ath/ath5k/pcu.c
@@ -593,10 +593,27 @@ u32 ath5k_hw_get_tsf32(struct ath5k_hw *ah)
*/
u64 ath5k_hw_get_tsf64(struct ath5k_hw *ah)
{
- u64 tsf = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
+ u32 tsf_lower, tsf_upper;
+
+ /*
+ * While reading TSF upper and then lower part, the clock is still
+ * counting so the lower part can rollover just after reading the
+ * upper part. In this case, we expect the lower part to be quite
+ * small (let's say less than 100us) and we would just need to read
+ * the upper part again to get the correct value.
+ *
+ * Tested on AR2425 (AR5001)
+ */
+
+ tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
+ tsf_lower = ath5k_hw_reg_read(ah, AR5K_TSF_L32);
+
+ if (tsf_lower < 100)
+ tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
+
ATH5K_TRACE(ah->ah_sc);
- return ath5k_hw_reg_read(ah, AR5K_TSF_L32) | (tsf << 32);
+ return (((u64)tsf_upper << 32) | tsf_lower);
}
/**
--
1.5.6.5
Johannes Berg a écrit :
> On Sun, 2010-02-28 at 23:08 +0100, Benoit Papillault wrote:
>
>
>> - u64 tsf = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
>> + u32 tsf_lower, tsf_upper;
>> +
>> + /*
>> + * While reading TSF upper and then lower part, the clock is still
>> + * counting so the lower part can rollover just after reading the
>> + * upper part. In this case, we expect the lower part to be quite
>> + * small (let's say less than 100us) and we would just need to read
>> + * the upper part again to get the correct value.
>> + *
>> + * Tested on AR2425 (AR5001)
>> + */
>> +
>> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
>> + tsf_lower = ath5k_hw_reg_read(ah, AR5K_TSF_L32);
>> +
>> + if (tsf_lower < 100)
>> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
>>
>
> You would typically do
>
> do {
> read upper 1
> read lower
> read upper 2
> } while (upper 1 != upper 2)
>
> or so but that obviously incurs another read in most cases.
>
> johannes
>
>
Indeed. I'll redo the patch. Derek has convinced me that accuracy is
sometimes more important than few register reads. So forget this patch.
Regards,
Benoit
Hi,
No, canot support this.
What happens if there is an interrupt immediately after the first
two reads - before the test on tsf_lower? - and thisinterrupt lasts longer
than 100us ? Most often, this is ok - until the interrupt happens at the
wrong time.
OR, there is a TSF merge event after the tsf_upper has been read, but
before the tsf_lower has been read? The tsf merge (which is done by the
hardware), adjusted both the high and low registers.
Derek.
On Sun, 28 Feb 2010, Benoit Papillault wrote:
> +
> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
> + tsf_lower = ath5k_hw_reg_read(ah, AR5K_TSF_L32);
> +
> + if (tsf_lower < 100)
> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
> +
> ATH5K_TRACE(ah->ah_sc);
>
> - return ath5k_hw_reg_read(ah, AR5K_TSF_L32) | (tsf << 32);
> + return (((u64)tsf_upper << 32) | tsf_lower);
> }
>
> /**
>
--
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
"The only thing IE should be used for is to download Fire Fox"
"My favorite language is call STAR. It's extremely concise. It has
exactly one verb '*', which does exactly what I want at the moment."
--Larry Wall
On Sun, 2010-02-28 at 23:08 +0100, Benoit Papillault wrote:
> - u64 tsf = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
> + u32 tsf_lower, tsf_upper;
> +
> + /*
> + * While reading TSF upper and then lower part, the clock is still
> + * counting so the lower part can rollover just after reading the
> + * upper part. In this case, we expect the lower part to be quite
> + * small (let's say less than 100us) and we would just need to read
> + * the upper part again to get the correct value.
> + *
> + * Tested on AR2425 (AR5001)
> + */
> +
> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
> + tsf_lower = ath5k_hw_reg_read(ah, AR5K_TSF_L32);
> +
> + if (tsf_lower < 100)
> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
You would typically do
do {
read upper 1
read lower
read upper 2
} while (upper 1 != upper 2)
or so but that obviously incurs another read in most cases.
johannes
Derek Smithies a ?crit :
>
> Hi,
> No, canot support this.
>
> What happens if there is an interrupt immediately after the first two
> reads - before the test on tsf_lower? - and thisinterrupt lasts longer
> than 100us ? Most often, this is ok - until the interrupt happens at
> the wrong time.
>
> OR, there is a TSF merge event after the tsf_upper has been read, but
> before the tsf_lower has been read? The tsf merge (which is done by
> the hardware), adjusted both the high and low registers.
>
> Derek.
Hi Derek,
You are indeed right. This was a first step, but like you mentioned, it
does not handle every cases, so let's forget it.
Regards,
Benoit
> On Sun, 28 Feb 2010, Benoit Papillault wrote:
>
>> +
>> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
>> + tsf_lower = ath5k_hw_reg_read(ah, AR5K_TSF_L32);
>> +
>> + if (tsf_lower < 100)
>> + tsf_upper = ath5k_hw_reg_read(ah, AR5K_TSF_U32);
>> +
>> ATH5K_TRACE(ah->ah_sc);
>>
>> - return ath5k_hw_reg_read(ah, AR5K_TSF_L32) | (tsf << 32);
>> + return (((u64)tsf_upper << 32) | tsf_lower);
>> }
>>
>> /**
>>
>