[PATCH] wl12xx: reset 5ghz num channels on hw init

[PATCH] wl12xx: reset 5ghz num channels on hw init

[Arik Nemtsov]

The number of 5ghz channels is set to 0 when 11a is not supported in the
NVS file. When a single rmmod/insmod of wl12xx_sdio this leads to a
supported band (5ghz) with 0 supported channels, which mac80211
considers illegal.

Fix this by always resetting the number of supported 5ghz channels
before the HW is registered.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 062247e..44cdefd 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2679,6 +2679,10 @@ int wl1271_init_ieee80211(struct wl1271 *wl)
 	wl->hw->wiphy->bands[IEEE80211_BAND_2GHZ] = &wl1271_band_2ghz;
 	wl->hw->wiphy->bands[IEEE80211_BAND_5GHZ] = &wl1271_band_5ghz;
 
+	/* reset the number of channels as this can be changed at runtime */
+	wl->hw->wiphy->bands[IEEE80211_BAND_5GHZ]->n_channels =
+					ARRAY_SIZE(wl1271_channels_5ghz);
+
 	wl->hw->queues = 4;
 	wl->hw->max_rates = 1;
 
-- 
1.7.1

Re: [PATCH] wl12xx: reset 5ghz num channels on hw init

[Johannes Berg]

On Mon, 2011-01-10 at 23:04 +0200, Arik Nemtsov wrote:
> The number of 5ghz channels is set to 0 when 11a is not supported in the
> NVS file. When a single rmmod/insmod of wl12xx_sdio this leads to a
> supported band (5ghz) with 0 supported channels, which mac80211
> considers illegal.

The setting to 0 shouldn't be happening in a global structure anyway
since that way you can't support two different devices in the same
system!!

johannes

Re: [PATCH] wl12xx: reset 5ghz num channels on hw init

[Luciano Coelho]

Hi Johannes,

On Tue, 2011-01-11 at 09:25 +0100, Johannes Berg wrote:
> On Mon, 2011-01-10 at 23:04 +0200, Arik Nemtsov wrote:
> > The number of 5ghz channels is set to 0 when 11a is not supported in the
> > NVS file. When a single rmmod/insmod of wl12xx_sdio this leads to a
> > supported band (5ghz) with 0 supported channels, which mac80211
> > considers illegal.
> 
> The setting to 0 shouldn't be happening in a global structure anyway
> since that way you can't support two different devices in the same
> system!!

This is that hack we discussed some time ago on how to disable the 11a
channels.  We only know that they're not supported after the driver is
loaded and the supported channels are already registered.  We get the
info that the 11a are not supported in the calibration data that we
receive later.

See commit fb6a6819fad0d71b47577a51709440a9f8441f0a.

Earlier I was disabling the channels by setting the DISABLED flag, but
that was bad, because I was touching regulatory stuff and it was causing
other problems.

-- 
Cheers,
Luca.

Re: [PATCH] wl12xx: reset 5ghz num channels on hw init

[Johannes Berg]

On Tue, 2011-01-11 at 10:32 +0200, Luciano Coelho wrote:
> Hi Johannes,
> 
> On Tue, 2011-01-11 at 09:25 +0100, Johannes Berg wrote:
> > On Mon, 2011-01-10 at 23:04 +0200, Arik Nemtsov wrote:
> > > The number of 5ghz channels is set to 0 when 11a is not supported in the
> > > NVS file. When a single rmmod/insmod of wl12xx_sdio this leads to a
> > > supported band (5ghz) with 0 supported channels, which mac80211
> > > considers illegal.
> > 
> > The setting to 0 shouldn't be happening in a global structure anyway
> > since that way you can't support two different devices in the same
> > system!!
> 
> This is that hack we discussed some time ago on how to disable the 11a
> channels. [snip explanation]

You misunderstood: I do remember discussing the hack, and it's still a
hack but it works and I'm OK with it. However, you shouldn't be
modifying a _global_ struct. You should at least copy the sband struct
itself (array actually then), which can still refer to the global
channel array, but then you don't modify global shared data for
device-dependent properties like this.

johannes

Re: [PATCH] wl12xx: reset 5ghz num channels on hw init

[Luciano Coelho]

On Tue, 2011-01-11 at 09:36 +0100, Johannes Berg wrote:
> On Tue, 2011-01-11 at 10:32 +0200, Luciano Coelho wrote:
> > Hi Johannes,
> > 
> > On Tue, 2011-01-11 at 09:25 +0100, Johannes Berg wrote:
> > > On Mon, 2011-01-10 at 23:04 +0200, Arik Nemtsov wrote:
> > > > The number of 5ghz channels is set to 0 when 11a is not supported in the
> > > > NVS file. When a single rmmod/insmod of wl12xx_sdio this leads to a
> > > > supported band (5ghz) with 0 supported channels, which mac80211
> > > > considers illegal.
> > > 
> > > The setting to 0 shouldn't be happening in a global structure anyway
> > > since that way you can't support two different devices in the same
> > > system!!
> > 
> > This is that hack we discussed some time ago on how to disable the 11a
> > channels. [snip explanation]
> 
> You misunderstood: I do remember discussing the hack, and it's still a
> hack but it works and I'm OK with it. However, you shouldn't be
> modifying a _global_ struct. You should at least copy the sband struct
> itself (array actually then), which can still refer to the global
> channel array, but then you don't modify global shared data for
> device-dependent properties like this.

Right! Now I got it.  Yeah, obviously the global shouldn't be modified,
otherwise all the registered hardwares will get their values modified at
the same time, which doesn't make sense.  Well, apparently that was not
so obvious to me when I looked it at first. ;)

Thanks for your comment, I'll fix that.

-- 
Cheers,
Luca.

Re: [PATCH] wl12xx: reset 5ghz num channels on hw init

[Luciano Coelho]

On Mon, 2011-01-10 at 22:04 +0100, Arik Nemtsov wrote:
> The number of 5ghz channels is set to 0 when 11a is not supported in the
> NVS file. When a single rmmod/insmod of wl12xx_sdio this leads to a
> supported band (5ghz) with 0 supported channels, which mac80211
> considers illegal.
> 
> Fix this by always resetting the number of supported 5ghz channels
> before the HW is registered.
> 
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
> ---

Just for the record, this patch is not necessary anymore, once my new
patch that avoids changing the global structs (as suggested by Johannes)
gets in.


-- 
Cheers,
Luca.