On Tue, Dec 15, 2009 at 01:35:01PM -0800, Lukáš Turek wrote: > On 15.12.2009 19:50 you wrote: > > Can you move this last line setting the sc->ah->ah_coverage_class > > to ath5k_hw_set_coverage_class() instead? > ath5k_hw_set_coverage_class() is also called after interface reset: > ath5k_hw_set_coverage_class(ah, ah->ah_coverage_class); > > So it would be quite strange if it set the value again... > But I see your point, and don't see a better solution. What I meant was not for you to pass the ah->ah_coverage_class to ath5k_hw_set_coverage_class() but instead to pass the value obtained from mac80211 and let ath5k_hw_set_coverage_class() itself set it on ah. > > Also -- if an ath_hw_set_coverage_class(common, coverage_class) can be > > defined on drivers/net/wireless/ath/hw.c along with: > > > > ath_hw_set_slot_time(common, slot_time), > > ath_hw_set_ack_timeout(common, ack_timeout); > > ath_hw_set_cts_timeout(common, cts_timeout); > > Unfortunately the functions are really hardware specific, they write a > register that even differs between chip versions, and also depend on chip > clock rate. Thanks for checking, I just wanted to elaborate as to how we can share code and to because of that it seems like a good idea to remain consistant and treat settings ah-> variables in hw code alone. The less we muck with ah-> settings on base.c for ath5k the more well kept and separated the actual hw code is. This really will only serve purpose to later merge mow hw code between ath9k/ath5k. How much we end up sharing remains to be seen, I don't mind the current split we have but if we do see something obviously common it makes sense to just stuff it in when we get a chance to ath. > It would be possible to move ath_hw_set_coverage_class() to ath to share the > calculation, but that would require exporting six functions from the modules. > > > Wonder if ar9170 can support setting these too. > > It seems it supports setting slot time, but I don't see a register with ACK > timeout, which is required too. Thanks for checking, I was lazy, and just curious. Reason for asking was if it could use at least caching the coverage class a hardware parameter we can stuff it into ath_common just as we do with the bssid_mask and stuff. Even if its just shared between ath5k and ath9k its still worth stashing it there if both will use it. Since tuning cts timeout, ack timouet and slot time may not be an operation which we do that often it would still not be so bad to just stuff a generic helper for ath5k/ath9k into ath which will handle the different hw revisions itself. Luis -- 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