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