Re: [PATCH] iwlwifi: do not register bands with no supported channels

On Tue, Mar 25, 2008 at 11:24 PM, John W. Linville
[off-list ref] wrote:

On Tue, Mar 25, 2008 at 10:31:27PM +0200, Tomas Winkler wrote:
 > On Tue, Mar 25, 2008 at 9:58 PM, John W. Linville
 > [off-list ref] wrote:
 > > Signed-off-by: John W. Linville [off-list ref]
 > >  ---
 > >  Otherwise, b/g-only devices fail in wiphy_register.  Tested in Fedora
 > >  kernels for the last week or so...
 > >
 > >   drivers/net/wireless/iwlwifi/iwl3945-base.c |    8 ++++++--
 > >   drivers/net/wireless/iwlwifi/iwl4965-base.c |    8 ++++++--
 > >   2 files changed, 12 insertions(+), 4 deletions(-)

quoted

quoted

 diff --git a/drivers/net/wireless/iwlwifi/iwl4965-base.c b/drivers/net/wireless/iwlwifi/iwl4965-base.c

 > >  index 44cfd02..9ae3d29 100644
 > >  --- a/drivers/net/wireless/iwlwifi/iwl4965-base.c
 > >  +++ b/drivers/net/wireless/iwlwifi/iwl4965-base.c
 > >  @@ -5001,8 +5001,12 @@ int iwl4965_init_geos(struct iwl_priv *priv)
 > >                priv->bands[IEEE80211_BAND_2GHZ].n_channels,
 > >                priv->bands[IEEE80211_BAND_5GHZ].n_channels);
 > >
 > >  -       priv->hw->wiphy->bands[IEEE80211_BAND_2GHZ] = &priv->bands[IEEE80211_BAND_2GHZ];
 > >  -       priv->hw->wiphy->bands[IEEE80211_BAND_5GHZ] = &priv->bands[IEEE80211_BAND_5GHZ];
 > >  +       if (priv->bands[IEEE80211_BAND_2GHZ].n_channels)
 > >  +               priv->hw->wiphy->bands[IEEE80211_BAND_2GHZ] =
 > >  +                       &priv->bands[IEEE80211_BAND_2GHZ];
 > >  +       if (priv->bands[IEEE80211_BAND_5GHZ].n_channels)
 > >  +               priv->hw->wiphy->bands[IEEE80211_BAND_5GHZ] =
 > >  +                       &priv->bands[IEEE80211_BAND_5GHZ];
 > >
 > >         set_bit(STATUS_GEO_CONFIGURED, &priv->status);
 > >
 >
 > As there already exists condition testing this I would prefer
 > something like this

 I don't think it is the same -- yours is checking IWL_SKU_A.


 > if ((priv->bands[IEEE80211_BAND_5GHZ].n_channels == 0) &&
 >            priv->cfg->sku & IWL_SKU_A) {
 >               printk(KERN_INFO DRV_NAME
 >                      ": Incorrectly detected BG card as ABG.  Please send "
 >                      "your PCI ID 0x%04X:0x%04X to maintainer.\n",
 >                      priv->pci_dev->device, priv->pci_dev->subsystem_device);
 >               priv->cfg->sku &= ~IWL_SKU_A;
 >                 priv->hw->wiphy->bands[IEEE80211_BAND_5GHZ] = NULL;
 > }

 A clause just like this exists a few lines above, yet it does not
 fire for the device that prompted this patch:

        https://bugzilla.redhat.com/attachment.cgi?id=298613


 > else
 >    priv->hw->wiphy->bands[IEEE80211_BAND_5GHZ] =
 > &priv->bands[IEEE80211_BAND_5GHZ];

 I don't think your version will resolve the issue.  Am I missing
 something?

Yep I was a bit snappy. my suggestion doesn't resolve the problem.
Anyhow, let me look at this tomorrow if we have other SKU issues so it
can be rolled in together.

Thanks
TOmas

 John
 --


John W. Linville
 linville@tuxdriver.com