Search Linux Wireless

Re: [PATCH 7/8] ath5k: disable 32KHz sleep clock operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/7/4 Felix Fietkau <nbd@xxxxxxxxxxx>:
> On 2011-07-04 4:58 PM, Nick Kossifidis wrote:
>>
>> 2011/7/4 Felix Fietkau<nbd@xxxxxxxxxxx>:
>>>
>>>  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<nbd@xxxxxxxxxxx>
>>>  ---
>>>   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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux