| Message ID | 19103.25725.707217.596352@gargle.gargle.HOWL (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 2009-09-03 at 12:08 +0530, Sujith wrote: > CHANNEL_G has to be set for 2GHZ channels since > IS_CHAN_G() checks for this in channelFlags and not in > chanmode. To make things messier, ath9k_hw_process_ini() > checks for CHANNEL_G in chanmode and not in channelFlags. > The supreme, brain-searing fix is to set the > flag in both cases. My impression is that's a workaround for a bug that lies elsewhere. I don't think we should apply such patches. chanmode and channelFlags seem to duplicate each other. I would use an enum for distinctive allowed modes and macros to query if the particular mode has a specific feature (band, modulation, bandwidth etc). The enum could use ORed constants for optimization, but such constants should be used only through the macros.
On Thu, Sep 3, 2009 at 8:19 AM, Pavel Roskin<proski@gnu.org> wrote: > On Thu, 2009-09-03 at 12:08 +0530, Sujith wrote: >> CHANNEL_G has to be set for 2GHZ channels since >> IS_CHAN_G() checks for this in channelFlags and not in >> chanmode. To make things messier, ath9k_hw_process_ini() >> checks for CHANNEL_G in chanmode and not in channelFlags. >> The supreme, brain-searing fix is to set the >> flag in both cases. > > My impression is that's a workaround for a bug that lies elsewhere. Â I > don't think we should apply such patches. > > chanmode and channelFlags seem to duplicate each other. Â I would use an > enum for distinctive allowed modes and macros to query if the particular > mode has a specific feature (band, modulation, bandwidth etc). > > The enum could use ORed constants for optimization, but such constants > should be used only through the macros. I should mention ath9k_update_ichannel() was the last thing I intended on porting for the new regulatory changes we did on ath9k, when we ditched the old regulatory code, but I left it as it did involve quite a lot more changes in the code. When I reviewed this it seemed we could just rely on the hw->conf.channel->band for many things but in some cases we needed some defines for hw. So -- I'd personally welcome this change as a better alternative needs better planning, and more testing. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pavel Roskin wrote: > My impression is that's a workaround for a bug that lies elsewhere. I > don't think we should apply such patches. The internal channelFlags/related macros are extremely messy. The ugliness arose from the requirement for differentiating between modes (11a, 11g, 11ng etc..) and legacy cruft. mac80211 has no concept of modes, it has bands instead. I do understand your concern though. But fixing this cleanly would mean that we rewrite the entire channel handling mechanism, any band-aid can only be a hack at best. But yes, maybe we can hold this until everything is cleaned up. And this has been in the ath9k TODO page for quite some time. :) Sujith -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Luis R. Rodriguez wrote: > I should mention ath9k_update_ichannel() was the last thing I intended > on porting for the new regulatory changes we did on ath9k, when we > ditched the old regulatory code, but I left it as it did involve quite > a lot more changes in the code. When I reviewed this it seemed we > could just rely on the hw->conf.channel->band for many things but in > some cases we needed some defines for hw. > > So -- I'd personally welcome this change as a better alternative needs > better planning, and more testing. Agreed. Sujith -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index b1d189c..a69fda8 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -1879,7 +1879,7 @@ void ath9k_update_ichannel(struct ath_softc *sc, struct ieee80211_hw *hw, if (chan->band == IEEE80211_BAND_2GHZ) { ichan->chanmode = CHANNEL_G; - ichan->channelFlags = CHANNEL_2GHZ | CHANNEL_OFDM; + ichan->channelFlags = CHANNEL_2GHZ | CHANNEL_OFDM | CHANNEL_G; } else { ichan->chanmode = CHANNEL_A; ichan->channelFlags = CHANNEL_5GHZ | CHANNEL_OFDM;
CHANNEL_G has to be set for 2GHZ channels since IS_CHAN_G() checks for this in channelFlags and not in chanmode. To make things messier, ath9k_hw_process_ini() checks for CHANNEL_G in chanmode and not in channelFlags. The supreme, brain-searing fix is to set the flag in both cases. Signed-off-by: Sujith <Sujith.Manoharan@atheros.com> --- drivers/net/wireless/ath/ath9k/main.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)