[PATCH 2/2] ath9k: Fix channelFlags for 2GHZ

[PATCH 2/2] ath9k: Fix channelFlags for 2GHZ

[Sujith]

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(-)

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;
-- 
1.6.4.2

Re: [PATCH 2/2] ath9k: Fix channelFlags for 2GHZ

[Pavel Roskin]

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.

-- 
Regards,
Pavel Roskin

Re: [PATCH 2/2] ath9k: Fix channelFlags for 2GHZ

[Luis R. Rodriguez]

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

Re: [PATCH 2/2] ath9k: Fix channelFlags for 2GHZ

[Sujith]

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

Re: [PATCH 2/2] ath9k: Fix channelFlags for 2GHZ

[Sujith]

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