During scans the full calibration usually does not make much sense,
PAPD probing and IQ calibration should be deferred until there is
enough time to complete them. Adding 100 ms to the initial full
calibration delay should be enough to do this.
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index a413aa7..efec14f 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2728,7 +2728,7 @@ ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan,
ath5k_ani_init(ah, ani_mode);
- ah->ah_cal_next_full = jiffies;
+ ah->ah_cal_next_full = jiffies + msecs_to_jiffies(100);
ah->ah_cal_next_ani = jiffies;
ah->ah_cal_next_nf = jiffies;
ewma_init(&ah->ah_beacon_rssi_avg, 1024, 8);
--
1.7.3.2
2011/7/4 Kalle Valo <[email protected]>:
> Nick Kossifidis <[email protected]> writes:
>
>> O.K. let's comment it out then and add some information that it's
>> disabled by default on available HALs...
>
> Leaving dead code lying around is bad. Why not add a comment with some
> pointers how to get the old code if someone wants to do that? For
> example git commit id is a good reference and I'm sure it will work
> even after 10 years.
>
OK but either we remove the whole thing (ath5k_hw_set_sleep_cloc also,
it's not being called anywhere else) or go the other way and leave it
there with a comment or a debugfs/modparam. I believe having it as a
debugfs/modparam option is better, after all it worked so far for most
cards, we didn't had any bug reports related to 32KHz operation (at
least I don't remember any).
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On 2011-07-04 4:58 PM, Nick Kossifidis wrote:
> 2011/7/4 Felix Fietkau<[email protected]>:
>> While 32 KHz sleep clock might provide some power saving benefits,
>> it is also a major source of stability issues, on OpenWrt it produced
>> some reproducible data bus errors on register accesses on several
>> different MIPS platforms.
>>
>> All the Atheros drivers that I can find do not enable this feature,
>> so it makes sense to leave it disabled in ath5k as well.
>>
>> Signed-off-by: Felix Fietkau<[email protected]>
>> ---
>> drivers/net/wireless/ath/ath5k/reset.c | 9 ---------
>> 1 files changed, 0 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
>> index 55276ce..192c0cb 100644
>> --- a/drivers/net/wireless/ath/ath5k/reset.c
>> +++ b/drivers/net/wireless/ath/ath5k/reset.c
>> @@ -1285,15 +1285,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
>> */
>> ath5k_hw_dma_init(ah);
>>
>> -
>> - /* Enable 32KHz clock function for AR5212+ chips
>> - * Set clocks to 32KHz operation and use an
>> - * external 32KHz crystal when sleeping if one
>> - * exists */
>> - if (ah->ah_version == AR5K_AR5212&&
>> - op_mode != NL80211_IFTYPE_AP)
>> - ath5k_hw_set_sleep_clock(ah, true);
>> -
>> /*
>> * Disable beacons and reset the TSF
>> */
>> --
>> 1.7.3.2
>>
>
> a) LegacyHAL and Sam's HAL both enable it
At least in the Legacy HAL (and in all other HALs that I looked a) I
found this in the attach function:
ahp->ah_enable32kHzClock = DONT_USE_32KHZ;/* XXX */
> b) Not many cards have a 32KHz crystal anyway (disabled on EEPROM)
> c) Please don't just remove code, if you want to disable it you can
> always comment it out
OK.
> d) Why not make it a module parameter instead ?
I'm not sure 32 KHz has been tested properly and found to be stable
anywhere, so I'm not convinced it will be useful to anybody. Also, I
think a debugfs parameter might be better because then it can be
enabled/disabled per card.
- Felix
While 32 KHz sleep clock might provide some power saving benefits,
it is also a major source of stability issues, on OpenWrt it produced
some reproducible data bus errors on register accesses on several
different MIPS platforms.
All the Atheros drivers that I can find do not enable this feature,
so it makes sense to leave it disabled in ath5k as well.
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath5k/reset.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 55276ce..192c0cb 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -1285,15 +1285,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
*/
ath5k_hw_dma_init(ah);
-
- /* Enable 32KHz clock function for AR5212+ chips
- * Set clocks to 32KHz operation and use an
- * external 32KHz crystal when sleeping if one
- * exists */
- if (ah->ah_version == AR5K_AR5212 &&
- op_mode != NL80211_IFTYPE_AP)
- ath5k_hw_set_sleep_clock(ah, true);
-
/*
* Disable beacons and reset the TSF
*/
--
1.7.3.2
Nick Kossifidis <[email protected]> writes:
> O.K. let's comment it out then and add some information that it's
> disabled by default on available HALs...
Leaving dead code lying around is bad. Why not add a comment with some
pointers how to get the old code if someone wants to do that? For
example git commit id is a good reference and I'm sure it will work
even after 10 years.
--
Kalle Valo
2011/7/4 Felix Fietkau <[email protected]>:
> On 2011-07-04 4:58 PM, Nick Kossifidis wrote:
>>
>> 2011/7/4 Felix Fietkau<[email protected]>:
>>>
>>> While 32 KHz sleep clock might provide some power saving benefits,
>>> it is also a major source of stability issues, on OpenWrt it produced
>>> some reproducible data bus errors on register accesses on several
>>> different MIPS platforms.
>>>
>>> All the Atheros drivers that I can find do not enable this feature,
>>> so it makes sense to leave it disabled in ath5k as well.
>>>
>>> Signed-off-by: Felix Fietkau<[email protected]>
>>> ---
>>> drivers/net/wireless/ath/ath5k/reset.c | 9 ---------
>>> 1 files changed, 0 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath5k/reset.c
>>> b/drivers/net/wireless/ath/ath5k/reset.c
>>> index 55276ce..192c0cb 100644
>>> --- a/drivers/net/wireless/ath/ath5k/reset.c
>>> +++ b/drivers/net/wireless/ath/ath5k/reset.c
>>> @@ -1285,15 +1285,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum
>>> nl80211_iftype op_mode,
>>> */
>>> ath5k_hw_dma_init(ah);
>>>
>>> -
>>> - /* Enable 32KHz clock function for AR5212+ chips
>>> - * Set clocks to 32KHz operation and use an
>>> - * external 32KHz crystal when sleeping if one
>>> - * exists */
>>> - if (ah->ah_version == AR5K_AR5212&&
>>> - op_mode != NL80211_IFTYPE_AP)
>>> - ath5k_hw_set_sleep_clock(ah, true);
>>> -
>>> /*
>>> * Disable beacons and reset the TSF
>>> */
>>> --
>>> 1.7.3.2
>>>
>>
>> a) LegacyHAL and Sam's HAL both enable it
>
> At least in the Legacy HAL (and in all other HALs that I looked a) I found
> this in the attach function:
> ahp->ah_enable32kHzClock = DONT_USE_32KHZ;/* XXX */
>
Ouch, I missed that part...
>> b) Not many cards have a 32KHz crystal anyway (disabled on EEPROM)
>> c) Please don't just remove code, if you want to disable it you can
>> always comment it out
>
> OK.
>
>> d) Why not make it a module parameter instead ?
>
> I'm not sure 32 KHz has been tested properly and found to be stable
> anywhere, so I'm not convinced it will be useful to anybody. Also, I think a
> debugfs parameter might be better because then it can be enabled/disabled
> per card.
>
> - Felix
>
O.K. let's comment it out then and add some information that it's
disabled by default on available HALs...
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
enabling the sleep clock alters the AR5K_USEC_32 field, but disabling
it didn't restore it.
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath5k/reset.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 1676a3e..55276ce 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -233,7 +233,7 @@ static void ath5k_hw_init_core_clock(struct ath5k_hw *ah)
static void ath5k_hw_set_sleep_clock(struct ath5k_hw *ah, bool enable)
{
struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
- u32 scal, spending;
+ u32 scal, spending, sclock;
/* Only set 32KHz settings if we have an external
* 32KHz crystal present */
@@ -317,6 +317,15 @@ static void ath5k_hw_set_sleep_clock(struct ath5k_hw *ah, bool enable)
/* Set up tsf increment on each cycle */
AR5K_REG_WRITE_BITS(ah, AR5K_TSF_PARM, AR5K_TSF_PARM_INC, 1);
+
+ if ((ah->ah_radio == AR5K_RF5112) ||
+ (ah->ah_radio == AR5K_RF5413) ||
+ (ah->ah_radio == AR5K_RF2316) ||
+ (ah->ah_radio == AR5K_RF2317))
+ sclock = 40 - 1;
+ else
+ sclock = 32 - 1;
+ AR5K_REG_WRITE_BITS(ah, AR5K_USEC_5211, AR5K_USEC_32, sclock);
}
}
--
1.7.3.2
Instead of using ieee80211_stop_queue, check the configured tx queue
limit before calling ieee80211_get_buffered_bc.
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index efec14f..d889f33 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1555,7 +1555,8 @@ ath5k_tx_queue(struct ieee80211_hw *hw, struct sk_buff *skb,
goto drop_packet;
}
- if (txq->txq_len >= txq->txq_max)
+ if (txq->txq_len >= txq->txq_max &&
+ txq->qnum <= AR5K_TX_QUEUE_ID_DATA_MAX)
ieee80211_stop_queue(hw, txq->qnum);
spin_lock_irqsave(&sc->txbuflock, flags);
@@ -1931,6 +1932,10 @@ ath5k_beacon_send(struct ath5k_softc *sc)
skb = ieee80211_get_buffered_bc(sc->hw, vif);
while (skb) {
ath5k_tx_queue(sc->hw, skb, sc->cabq);
+
+ if (sc->cabq->txq_len >= sc->cabq->txq_max)
+ break;
+
skb = ieee80211_get_buffered_bc(sc->hw, vif);
}
--
1.7.3.2
2011/7/4 Felix Fietkau <[email protected]>:
> During scans the full calibration usually does not make much sense,
> PAPD probing and IQ calibration should be deferred until there is
> enough time to complete them. Adding 100 ms to the initial full
> calibration delay should be enough to do this.
>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> drivers/net/wireless/ath/ath5k/base.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index a413aa7..efec14f 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -2728,7 +2728,7 @@ ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan,
>
> ath5k_ani_init(ah, ani_mode);
>
> - ah->ah_cal_next_full = jiffies;
> + ah->ah_cal_next_full = jiffies + msecs_to_jiffies(100);
> ah->ah_cal_next_ani = jiffies;
> ah->ah_cal_next_nf = jiffies;
> ewma_init(&ah->ah_beacon_rssi_avg, 1024, 8);
> --
> 1.7.3.2
>
BTW have you checked this one ->
http://www.kernel.org/pub/linux/kernel/people/mickflemm/calibtest.patch
?
Acked-by: Nick Kossifidis <[email protected]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
2011/7/4 Felix Fietkau <[email protected]>:
> While 32 KHz sleep clock might provide some power saving benefits,
> it is also a major source of stability issues, on OpenWrt it produced
> some reproducible data bus errors on register accesses on several
> different MIPS platforms.
>
> All the Atheros drivers that I can find do not enable this feature,
> so it makes sense to leave it disabled in ath5k as well.
>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> drivers/net/wireless/ath/ath5k/reset.c | 9 ---------
> 1 files changed, 0 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 55276ce..192c0cb 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -1285,15 +1285,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
> */
> ath5k_hw_dma_init(ah);
>
> -
> - /* Enable 32KHz clock function for AR5212+ chips
> - * Set clocks to 32KHz operation and use an
> - * external 32KHz crystal when sleeping if one
> - * exists */
> - if (ah->ah_version == AR5K_AR5212 &&
> - op_mode != NL80211_IFTYPE_AP)
> - ath5k_hw_set_sleep_clock(ah, true);
> -
> /*
> * Disable beacons and reset the TSF
> */
> --
> 1.7.3.2
>
a) LegacyHAL and Sam's HAL both enable it
b) Not many cards have a 32KHz crystal anyway (disabled on EEPROM)
c) Please don't just remove code, if you want to disable it you can
always comment it out
d) Why not make it a module parameter instead ?
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
2011/7/4 Felix Fietkau <[email protected]>:
> enabling the sleep clock alters the AR5K_USEC_32 field, but disabling
> it didn't restore it.
>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> drivers/net/wireless/ath/ath5k/reset.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 1676a3e..55276ce 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -233,7 +233,7 @@ static void ath5k_hw_init_core_clock(struct ath5k_hw *ah)
> static void ath5k_hw_set_sleep_clock(struct ath5k_hw *ah, bool enable)
> {
> struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
> - u32 scal, spending;
> + u32 scal, spending, sclock;
>
> /* Only set 32KHz settings if we have an external
> * 32KHz crystal present */
> @@ -317,6 +317,15 @@ static void ath5k_hw_set_sleep_clock(struct ath5k_hw *ah, bool enable)
>
> /* Set up tsf increment on each cycle */
> AR5K_REG_WRITE_BITS(ah, AR5K_TSF_PARM, AR5K_TSF_PARM_INC, 1);
> +
> + if ((ah->ah_radio == AR5K_RF5112) ||
> + (ah->ah_radio == AR5K_RF5413) ||
> + (ah->ah_radio == AR5K_RF2316) ||
> + (ah->ah_radio == AR5K_RF2317))
> + sclock = 40 - 1;
> + else
> + sclock = 32 - 1;
> + AR5K_REG_WRITE_BITS(ah, AR5K_USEC_5211, AR5K_USEC_32, sclock);
> }
> }
>
> --
> 1.7.3.2
>
Acked-by: Nick Kossifidis <[email protected]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick