Hi Luis,
This is more or less what I need from ath9k's regulatory domain code.
What's your opinion? Would you accept the changes, or do you see
a potential conflict/problem with the design?
Regards,
Chr
---
diff --git a/drivers/net/wireless/ath9k/hw.c b/drivers/net/wireless/ath9k/hw.c
index 9818945..334a20f 100644
--- a/drivers/net/wireless/ath9k/hw.c
+++ b/drivers/net/wireless/ath9k/hw.c
@@ -1366,7 +1366,7 @@ static int ath9k_hw_process_ini(struct ath_hw *ah,
ath9k_olc_init(ah);
status = ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(ah, chan),
+ ath9k_regd_get_ctl(&ah->regulatory, chan),
channel->max_antenna_gain * 2,
channel->max_power * 2,
min((u32) MAX_RATE_POWER,
@@ -1706,7 +1706,7 @@ static bool ath9k_hw_channel_change(struct ath_hw *ah,
}
if (ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(ah, chan),
+ ath9k_regd_get_ctl(&ah->regulatory, chan),
channel->max_antenna_gain * 2,
channel->max_power * 2,
min((u32) MAX_RATE_POWER,
@@ -3768,7 +3768,7 @@ bool ath9k_hw_set_txpowerlimit(struct ath_hw *ah, u32 limit)
ah->regulatory.power_limit = min(limit, (u32) MAX_RATE_POWER);
if (ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(ah, chan),
+ ath9k_regd_get_ctl(&ah->regulatory, chan),
channel->max_antenna_gain * 2,
channel->max_power * 2,
min((u32) MAX_RATE_POWER,
diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index c13e4e5..8db019f 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -1362,6 +1362,16 @@ void ath_detach(struct ath_softc *sc)
ath9k_ps_restore(sc);
}
+struct ath9k_regulatory *ath9k_reg_get_from_wiphy(struct wiphy *wiphy)
+{
+ struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+ struct ath_wiphy *aphy = hw->priv;
+ struct ath_softc *sc = aphy->sc;
+ struct ath_hw *ah = sc->sc_ah;
+
+ return &ah->regulatory;
+}
+
static int ath_init(u16 devid, struct ath_softc *sc)
{
struct ath_hw *ah = NULL;
@@ -1416,7 +1426,8 @@ static int ath_init(u16 devid, struct ath_softc *sc)
for (i = 0; i < sc->keymax; i++)
ath9k_hw_keyreset(ah, (u16) i);
- if (ath9k_regd_init(sc->sc_ah))
+ sc->sc_ah->regulatory.debug = sc;
+ if (ath9k_regd_init(&sc->sc_ah->regulatory))
goto bad;
/* default to MONITOR mode */
@@ -1666,10 +1677,10 @@ int ath_attach(u16 devid, struct ath_softc *sc)
goto error_attach;
#endif
- if (ath9k_is_world_regd(sc->sc_ah)) {
+ if (ath9k_is_world_regd(&sc->sc_ah->regulatory)) {
/* Anything applied here (prior to wiphy registration) gets
* saved on the wiphy orig_* parameters */
- regd = ath9k_world_regdomain(sc->sc_ah);
+ regd = ath9k_world_regdomain(&sc->sc_ah->regulatory);
hw->wiphy->custom_regulatory = true;
hw->wiphy->strict_regulatory = false;
} else {
@@ -1688,7 +1699,7 @@ int ath_attach(u16 devid, struct ath_softc *sc)
error = ieee80211_register_hw(hw);
- if (!ath9k_is_world_regd(sc->sc_ah)) {
+ if (!ath9k_is_world_regd(&sc->sc_ah->regulatory)) {
error = regulatory_hint(hw->wiphy,
sc->sc_ah->regulatory.alpha2);
if (error)
diff --git a/drivers/net/wireless/ath9k/regd.c b/drivers/net/wireless/ath9k/regd.c
index 4ca6251..017a05f 100644
--- a/drivers/net/wireless/ath9k/regd.c
+++ b/drivers/net/wireless/ath9k/regd.c
@@ -112,14 +112,14 @@ static inline bool is_wwr_sku(u16 regd)
(regd == WORLD);
}
-static u16 ath9k_regd_get_eepromRD(struct ath_hw *ah)
+static u16 ath9k_regd_get_eepromRD(struct ath9k_regulatory *reg)
{
- return ah->regulatory.current_rd & ~WORLDWIDE_ROAMING_FLAG;
+ return reg->current_rd & ~WORLDWIDE_ROAMING_FLAG;
}
-bool ath9k_is_world_regd(struct ath_hw *ah)
+bool ath9k_is_world_regd(struct ath9k_regulatory *reg)
{
- return is_wwr_sku(ath9k_regd_get_eepromRD(ah));
+ return is_wwr_sku(ath9k_regd_get_eepromRD(reg));
}
const struct ieee80211_regdomain *ath9k_default_world_regdomain(void)
@@ -128,9 +128,9 @@ const struct ieee80211_regdomain *ath9k_default_world_regdomain(void)
return &ath9k_world_regdom_64;
}
-const struct ieee80211_regdomain *ath9k_world_regdomain(struct ath_hw *ah)
+const struct ieee80211_regdomain *ath9k_world_regdomain(struct ath9k_regulatory *reg)
{
- switch (ah->regulatory.regpair->regDmnEnum) {
+ switch (reg->regpair->regDmnEnum) {
case 0x60:
case 0x61:
case 0x62:
@@ -313,12 +313,9 @@ void ath9k_reg_apply_radar_flags(struct wiphy *wiphy)
void ath9k_reg_apply_world_flags(struct wiphy *wiphy,
enum nl80211_reg_initiator initiator)
{
- struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
- struct ath_wiphy *aphy = hw->priv;
- struct ath_softc *sc = aphy->sc;
- struct ath_hw *ah = sc->sc_ah;
+ struct ath9k_regulatory *reg = ath9k_reg_get_from_wiphy(wiphy);
- switch (ah->regulatory.regpair->regDmnEnum) {
+ switch (reg->regpair->regDmnEnum) {
case 0x60:
case 0x63:
case 0x66:
@@ -335,9 +332,7 @@ void ath9k_reg_apply_world_flags(struct wiphy *wiphy,
int ath9k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
{
- struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
- struct ath_wiphy *aphy = hw->priv;
- struct ath_softc *sc = aphy->sc;
+ struct ath9k_regulatory *reg = ath9k_reg_get_from_wiphy(wiphy);
/* We always apply this */
ath9k_reg_apply_radar_flags(wiphy);
@@ -348,7 +343,7 @@ int ath9k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
case NL80211_REGDOM_SET_BY_USER:
break;
case NL80211_REGDOM_SET_BY_COUNTRY_IE:
- if (ath9k_is_world_regd(sc->sc_ah))
+ if (ath9k_is_world_regd(reg))
ath9k_reg_apply_world_flags(wiphy, request->initiator);
break;
}
@@ -356,9 +351,9 @@ int ath9k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
return 0;
}
-bool ath9k_regd_is_eeprom_valid(struct ath_hw *ah)
+bool ath9k_regd_is_eeprom_valid(struct ath9k_regulatory *reg)
{
- u16 rd = ath9k_regd_get_eepromRD(ah);
+ u16 rd = ath9k_regd_get_eepromRD(reg);
int i;
if (rd & COUNTRY_ERD_FLAG) {
@@ -373,8 +368,8 @@ bool ath9k_regd_is_eeprom_valid(struct ath_hw *ah)
if (regDomainPairs[i].regDmnEnum == rd)
return true;
}
- DPRINTF(ah->ah_sc, ATH_DBG_REGULATORY,
- "invalid regulatory domain/country code 0x%x\n", rd);
+ DPRINTF(reg->debug, ATH_DBG_REGULATORY,
+ "invalid regulatory domain/country code 0x%x\n", rd);
return false;
}
@@ -433,42 +428,42 @@ ath9k_get_regpair(int regdmn)
return NULL;
}
-int ath9k_regd_init(struct ath_hw *ah)
+int ath9k_regd_init(struct ath9k_regulatory *reg)
{
struct country_code_to_enum_rd *country = NULL;
u16 regdmn;
- if (!ath9k_regd_is_eeprom_valid(ah)) {
- DPRINTF(ah->ah_sc, ATH_DBG_REGULATORY,
+ if (!ath9k_regd_is_eeprom_valid(reg)) {
+ DPRINTF(reg->debug, ATH_DBG_FATAL,
"Invalid EEPROM contents\n");
return -EINVAL;
}
- regdmn = ath9k_regd_get_eepromRD(ah);
- ah->regulatory.country_code = ath9k_regd_get_default_country(regdmn);
+ regdmn = ath9k_regd_get_eepromRD(reg);
+ reg->country_code = ath9k_regd_get_default_country(regdmn);
- if (ah->regulatory.country_code == CTRY_DEFAULT &&
+ if (reg->country_code == CTRY_DEFAULT &&
regdmn == CTRY_DEFAULT)
- ah->regulatory.country_code = CTRY_UNITED_STATES;
+ reg->country_code = CTRY_UNITED_STATES;
- if (ah->regulatory.country_code == CTRY_DEFAULT) {
+ if (reg->country_code == CTRY_DEFAULT) {
country = NULL;
} else {
- country = ath9k_regd_find_country(ah->regulatory.country_code);
+ country = ath9k_regd_find_country(reg->country_code);
if (country == NULL) {
- DPRINTF(ah->ah_sc, ATH_DBG_REGULATORY,
+ DPRINTF(reg->debug, ATH_DBG_FATAL,
"Country is NULL!!!!, cc= %d\n",
- ah->regulatory.country_code);
+ reg->country_code);
return -EINVAL;
} else
regdmn = country->regDmnEnum;
}
- ah->regulatory.regpair = ath9k_get_regpair(regdmn);
+ reg->regpair = ath9k_get_regpair(regdmn);
- if (!ah->regulatory.regpair) {
- DPRINTF(ah->ah_sc, ATH_DBG_FATAL,
- "No regulatory domain pair found, cannot continue\n");
+ if (!reg->regpair) {
+ DPRINTF(reg->debug, ATH_DBG_FATAL, "No regulatory domain "
+ "pair found, cannot continue\n");
return -EINVAL;
}
@@ -476,29 +471,29 @@ int ath9k_regd_init(struct ath_hw *ah)
country = ath9k_regd_find_country_by_rd(regdmn);
if (country) {
- ah->regulatory.alpha2[0] = country->isoName[0];
- ah->regulatory.alpha2[1] = country->isoName[1];
+ reg->alpha2[0] = country->isoName[0];
+ reg->alpha2[1] = country->isoName[1];
} else {
- ah->regulatory.alpha2[0] = '0';
- ah->regulatory.alpha2[1] = '0';
+ reg->alpha2[0] = '0';
+ reg->alpha2[1] = '0';
}
- DPRINTF(ah->ah_sc, ATH_DBG_REGULATORY,
+ DPRINTF(reg->debug, ATH_DBG_REGULATORY,
"Country alpha2 being used: %c%c\n"
"Regulatory.Regpair detected: 0x%0x\n",
- ah->regulatory.alpha2[0], ah->regulatory.alpha2[1],
- ah->regulatory.regpair->regDmnEnum);
+ reg->alpha2[0], reg->alpha2[1],
+ reg->regpair->regDmnEnum);
return 0;
}
-u32 ath9k_regd_get_ctl(struct ath_hw *ah, struct ath9k_channel *chan)
+u32 ath9k_regd_get_ctl(struct ath9k_regulatory *reg,
+ struct ath9k_channel *chan)
{
u32 ctl = NO_CTL;
- if (!ah->regulatory.regpair ||
- (ah->regulatory.country_code == CTRY_DEFAULT &&
- is_wwr_sku(ath9k_regd_get_eepromRD(ah)))) {
+ if (!reg->regpair || (reg->country_code == CTRY_DEFAULT &&
+ is_wwr_sku(ath9k_regd_get_eepromRD(reg)))) {
if (IS_CHAN_B(chan))
ctl = SD_NO_CTL | CTL_11B;
else if (IS_CHAN_G(chan))
@@ -509,11 +504,11 @@ u32 ath9k_regd_get_ctl(struct ath_hw *ah, struct ath9k_channel *chan)
}
if (IS_CHAN_B(chan))
- ctl = ah->regulatory.regpair->reg_2ghz_ctl | CTL_11B;
+ ctl = reg->regpair->reg_2ghz_ctl | CTL_11B;
else if (IS_CHAN_G(chan))
- ctl = ah->regulatory.regpair->reg_2ghz_ctl | CTL_11G;
+ ctl = reg->regpair->reg_2ghz_ctl | CTL_11G;
else
- ctl = ah->regulatory.regpair->reg_5ghz_ctl | CTL_11A;
+ ctl = reg->regpair->reg_5ghz_ctl | CTL_11A;
return ctl;
}
diff --git a/drivers/net/wireless/ath9k/regd.h b/drivers/net/wireless/ath9k/regd.h
index 9f5fbd4..dc23883 100644
--- a/drivers/net/wireless/ath9k/regd.h
+++ b/drivers/net/wireless/ath9k/regd.h
@@ -49,6 +49,7 @@ struct ath9k_regulatory {
u16 current_rd_ext;
int16_t power_limit;
struct reg_dmn_pair_mapping *regpair;
+ void *debug;
};
enum CountryCode {
@@ -233,15 +234,17 @@ enum CountryCode {
CTRY_BELGIUM2 = 5002
};
-bool ath9k_is_world_regd(struct ath_hw *ah);
-const struct ieee80211_regdomain *ath9k_world_regdomain(struct ath_hw *ah);
+struct ath9k_channel;
+
+bool ath9k_is_world_regd(struct ath9k_regulatory *reg);
+const struct ieee80211_regdomain *ath9k_world_regdomain(struct ath9k_regulatory *reg);
const struct ieee80211_regdomain *ath9k_default_world_regdomain(void);
void ath9k_reg_apply_world_flags(struct wiphy *wiphy,
enum nl80211_reg_initiator initiator);
void ath9k_reg_apply_radar_flags(struct wiphy *wiphy);
-int ath9k_regd_init(struct ath_hw *ah);
-bool ath9k_regd_is_eeprom_valid(struct ath_hw *ah);
-u32 ath9k_regd_get_ctl(struct ath_hw *ah, struct ath9k_channel *chan);
+int ath9k_regd_init(struct ath9k_regulatory *reg);
+bool ath9k_regd_is_eeprom_valid(struct ath9k_regulatory *reg);
+u32 ath9k_regd_get_ctl(struct ath9k_regulatory *reg, struct ath9k_channel *chan);
int ath9k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request);
-
+struct ath9k_regulatory *ath9k_reg_get_from_wiphy(struct wiphy *wiphy);
#endif
---
of course, here are the ar9170 changes:
---
diff --git a/drivers/net/wireless/ar9170/Makefile b/drivers/net/wireless/ar9170/Makefile
index 8d91c7e..66e89ce 100644
--- a/drivers/net/wireless/ar9170/Makefile
+++ b/drivers/net/wireless/ar9170/Makefile
@@ -1,3 +1,3 @@
-ar9170usb-objs := usb.o main.o cmd.o mac.o phy.o led.o
+ar9170usb-objs := usb.o ../ath9k/regd.o main.o cmd.o mac.o phy.o led.o
obj-$(CONFIG_AR9170_USB) += ar9170usb.o
diff --git a/drivers/net/wireless/ar9170/ar9170.h b/drivers/net/wireless/ar9170/ar9170.h
index f4fb2e9..49d4995 100644
--- a/drivers/net/wireless/ar9170/ar9170.h
+++ b/drivers/net/wireless/ar9170/ar9170.h
@@ -48,6 +48,9 @@
#include "eeprom.h"
#include "hw.h"
+/* ath9k definitions */
+#include "../ath9k/regd.h"
+
#define PAYLOAD_MAX (AR9170_MAX_CMD_LEN/4 - 1)
enum ar9170_bw {
@@ -156,6 +159,9 @@ struct ar9170 {
struct sk_buff_head global_tx_status;
struct sk_buff_head global_tx_status_waste;
struct delayed_work tx_status_janitor;
+
+ /* regulatory domain */
+ struct ath9k_regulatory regulatory;
};
struct ar9170_sta_info {
diff --git a/drivers/net/wireless/ar9170/main.c b/drivers/net/wireless/ar9170/main.c
index 5996ff9..c5a2012 100644
--- a/drivers/net/wireless/ar9170/main.c
+++ b/drivers/net/wireless/ar9170/main.c
@@ -149,6 +149,13 @@ static struct ieee80211_supported_band ar9170_band_2GHz = {
.n_bitrates = ar9170_g_ratetable_size,
};
+static struct ieee80211_supported_band ar9170_band_5GHz = {
+ .channels = ar9170_5ghz_chantable,
+ .n_channels = ARRAY_SIZE(ar9170_5ghz_chantable),
+ .bitrates = ar9170_a_ratetable,
+ .n_bitrates = ar9170_a_ratetable_size,
+};
+
#ifdef AR9170_QUEUE_DEBUG
/*
* In case some wants works with AR9170's crazy tx_status queueing techniques.
@@ -190,12 +197,30 @@ static void ar9170_dump_station_tx_status_queue(struct ar9170 *ar,
}
#endif /* AR9170_QUEUE_DEBUG */
-static struct ieee80211_supported_band ar9170_band_5GHz = {
- .channels = ar9170_5ghz_chantable,
- .n_channels = ARRAY_SIZE(ar9170_5ghz_chantable),
- .bitrates = ar9170_a_ratetable,
- .n_bitrates = ar9170_a_ratetable_size,
-};
+/* regulatory domain glue code */
+struct ath9k_regulatory *ath9k_reg_get_from_wiphy(struct wiphy *wiphy)
+{
+ struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+ struct ar9170 *ar = hw->priv;
+
+ return &ar->regulatory;
+}
+
+void DPRINTF(struct ar9170 *ar, int dbg_mask, const char *fmt, ...)
+{
+ if (!ar)
+ return;
+
+ /* print fatal errors */
+ if (dbg_mask & 0x8000) {
+ va_list args;
+
+ va_start(args, fmt);
+ printk(KERN_ERR "%s: ", wiphy_name(ar->hw->wiphy));
+ vprintk(fmt, args);
+ va_end(args);
+ }
+}
void ar9170_handle_tx_status(struct ar9170 *ar, struct sk_buff *skb,
bool valid_status, u16 tx_status)
@@ -1557,6 +1582,11 @@ void *ar9170_alloc(size_t priv_size)
ar->hw->max_rates = 1;
ar->hw->max_rate_tries = 3;
+ ar->regulatory.debug = ar;
+ ar->regulatory.country_code = 0; /* CTRY_DEFAULT */
+ ar->regulatory.power_limit = 63; /* MAX_RATE_POWER */
+ ar->regulatory.tp_scale = 0; /* ATH9K_TP_SCALE_MAX */
+
for (i = 0; i < ARRAY_SIZE(ar->noise); i++)
ar->noise[i] = -95; /* ATH_DEFAULT_NOISE_FLOOR */
@@ -1607,6 +1637,10 @@ static int ar9170_read_eeprom(struct ar9170 *ar)
ar->hw->wiphy->bands[IEEE80211_BAND_5GHZ] = &ar9170_band_5GHz;
bands++;
}
+
+ ar->regulatory.current_rd = le16_to_cpu(ar->eeprom.reg_domain[0]);
+ ar->regulatory.current_rd_ext = le16_to_cpu(ar->eeprom.reg_domain[1]);
+
/*
* I measured this, a bandswitch takes roughly
* 135 ms and a frequency switch about 80.
@@ -1627,6 +1661,7 @@ static int ar9170_read_eeprom(struct ar9170 *ar)
int ar9170_register(struct ar9170 *ar, struct device *pdev)
{
+ const struct ieee80211_regdomain *regd;
int err;
/* try to read EEPROM, init MAC addr */
@@ -1634,10 +1669,38 @@ int ar9170_register(struct ar9170 *ar, struct device *pdev)
if (err)
goto err_out;
+ err = ath9k_regd_init(&ar->regulatory);
+ if (err)
+ goto err_out;
+
+ if (ath9k_is_world_regd(&ar->regulatory)) {
+ /* Anything applied here (prior to wiphy registration) gets
+ * saved on the wiphy orig_* parameters */
+ regd = ath9k_world_regdomain(&ar->regulatory);
+ ar->hw->wiphy->custom_regulatory = true;
+ ar->hw->wiphy->strict_regulatory = false;
+ } else {
+ /* This gets applied in the case of the absense of CRDA,
+ * it's our own custom world regulatory domain, similar to
+ * cfg80211's but we enable passive scanning */
+ regd = ath9k_default_world_regdomain();
+ }
+ wiphy_apply_custom_regulatory(ar->hw->wiphy, regd);
+ ath9k_reg_apply_radar_flags(ar->hw->wiphy);
+ ath9k_reg_apply_world_flags(ar->hw->wiphy,
+ NL80211_REGDOM_SET_BY_DRIVER);
+
err = ieee80211_register_hw(ar->hw);
if (err)
goto err_out;
+ if (!ath9k_is_world_regd(&ar->regulatory)) {
+ err = regulatory_hint(ar->hw->wiphy,
+ ar->regulatory.alpha2);
+ if (err)
+ goto err_unreg;
+ }
+
err = ar9170_init_leds(ar);
if (err)
goto err_unreg;
---
On Tuesday 24 March 2009 21:10:21 Luis R. Rodriguez wrote:
> On Tue, Mar 24, 2009 at 1:03 PM, Bob Copeland <[email protected]> wrote:
> > On Tue, Mar 24, 2009 at 2:41 PM, Luis R. Rodriguez
> > <[email protected]> wrote:
> >> Do any of you guys have time to pick it up?
> >
> > Well my patch #2 is pretty similar to what Christian has, and I'd like to
> > upstream the first few patches soon (especially the ath.ko creation, it is a
> > beast to rebase as you saw). That would bring up the first contentious
> > point of the merge: is "ath/ath.ko" ok with everyone?
>
> Yes.
Yes.
> > ath5k/9k specific
> > bits can eventually go in ath/{5k,9k} subdirs... not sure where 9170
> > lives then :)
>
> Wherever Christian wants it. If he finds re-usable bits then great.
maybe ath/9kusb ?
> > I probably can't get to it until the weekend, but if you have more time,
> > Christian, feel free to adopt whatever you want from mine. Otherwise I'll
> > pick up where Luis left off on the weekend.
Done! Patch attached, please add to the tree ;)
> > I didn't push it yet because ath5k was having problems once I turned it
> > on for ath5k,
>
> So this is why I recommend to trim the channel list down of ath5k. The
> other reason is you won't have users spending what may be 3/4 of their
> time scanning on channels that they won't ever find APs in. What can
> be done here is add flag or command as we had reviewed a while back
> to allow the user to enable these channels if he knows what he is
> doing so that way we know we don't mind bothering scanning on the
> gazillion channels. But that is just my advice. My concern first is to
> get users working happily and quickly associated to an AP and I think
> that parallels those goals and puts as secondary the bells and
> whistles of adding every single possible channel.
>
> > and I have no other hardware to test with so wanted to
> > figure out what was broken first.
> > Channel list is a good hint, maybe
> > it's time to fix iw/nl80211 to send back all the channels :)
>
> So that is the other option. I tried looking at that yesterday but
> wasn't able to find a way to do this. I went through the netlink
> documenation and even tried modifying the skb on the kernel side as
> its a dump. Not sure what to do here to fix this.
>
> So yes, both are possible options. Just keep in mind that without
> being able to see the channel list with 'iw list' debugging regulatory
> is a real royal pain in the ass. So I'd try to fix that first either
> through channel reduction (you get another bonus enhancement for
> scanning time for users) or through fixing this through iw/nl80211.
>
> Johannes probably can advise best on the second path.
>
> Luis
---
From: Christian Lamparter <[email protected]>
Date: Tue, 24 Mar 2009 21:24:14 +0100
Subject: [PATCH] ar9170: use regulatory infrastructure
To: [email protected]
Make ar9170 select the ath module and add in the hooks to make the
eeprom regulatory hint and reg notifier take effect.
Signed-off-by: Christian Lamparter <[email protected]>
---
diff --git a/drivers/net/wireless/ar9170/Kconfig b/drivers/net/wireless/ar9170/Kconfig
index de4281f..b99e326 100644
--- a/drivers/net/wireless/ar9170/Kconfig
+++ b/drivers/net/wireless/ar9170/Kconfig
@@ -2,6 +2,7 @@ config AR9170_USB
tristate "Atheros AR9170 802.11n USB support"
depends on USB && MAC80211 && WLAN_80211 && EXPERIMENTAL
select FW_LOADER
+ select ATH_COMMON
help
This is a driver for the Atheros "otus" 802.11n USB devices.
diff --git a/drivers/net/wireless/ar9170/ar9170.h b/drivers/net/wireless/ar9170/ar9170.h
index f4fb2e9..87c1985 100644
--- a/drivers/net/wireless/ar9170/ar9170.h
+++ b/drivers/net/wireless/ar9170/ar9170.h
@@ -48,6 +48,8 @@
#include "eeprom.h"
#include "hw.h"
+#include "../ath/regd.h"
+
#define PAYLOAD_MAX (AR9170_MAX_CMD_LEN/4 - 1)
enum ar9170_bw {
@@ -151,6 +153,7 @@ struct ar9170 {
/* EEPROM */
struct ar9170_eeprom eeprom;
+ struct ath_regulatory regulatory;
/* global tx status for unregistered Stations. */
struct sk_buff_head global_tx_status;
diff --git a/drivers/net/wireless/ar9170/main.c b/drivers/net/wireless/ar9170/main.c
index 5996ff9..4a251fc 100644
--- a/drivers/net/wireless/ar9170/main.c
+++ b/drivers/net/wireless/ar9170/main.c
@@ -1619,12 +1619,24 @@ static int ar9170_read_eeprom(struct ar9170 *ar)
else
ar->hw->channel_change_time = 80 * 1000;
+ ar->regulatory.current_rd = le16_to_cpu(ar->eeprom.reg_domain[0]);
+ ar->regulatory.current_rd_ext = le16_to_cpu(ar->eeprom.reg_domain[1]);
+
/* second part of wiphy init */
SET_IEEE80211_PERM_ADDR(ar->hw, addr);
return bands ? 0 : -EINVAL;
}
+int ar9170_reg_notifier(struct wiphy *wiphy,
+ struct regulatory_request *request)
+{
+ struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+ struct ar9170 *ar = hw->priv;
+
+ return ath_reg_notifier_apply(wiphy, request, &ar->regulatory);
+}
+
int ar9170_register(struct ar9170 *ar, struct device *pdev)
{
int err;
@@ -1634,10 +1646,16 @@ int ar9170_register(struct ar9170 *ar, struct device *pdev)
if (err)
goto err_out;
+ err = ath_regd_init(&ar->regulatory, ar->hw->wiphy,
+ ar9170_reg_notifier);
+
err = ieee80211_register_hw(ar->hw);
if (err)
goto err_out;
+ if (!ath_is_world_regd(&ar->regulatory))
+ regulatory_hint(ar->hw->wiphy, ar->regulatory.alpha2);
+
err = ar9170_init_leds(ar);
if (err)
goto err_unreg;
On Tue, Mar 24, 2009 at 08:45:53PM -0700, Luis R. Rodriguez wrote:
> > Fixed..
> >
> > http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v5.patch.txt
>
> Nope.. no luck, something got messed up along the way... needs a closer look.
So I tested this series and it works fine, with a caveat:
Currently, ath5k exposes tons of 5 GHz channels. In addition to the other
problems I mentioned in http://marc.info/?l=linux-wireless&m=123825852320853,
it also essentially breaks with regulatory hints from the driver. This is
because because the Atheros world regdomain allows most channels in the
5 GHz range for passive scanning, whereas the default world regdomain
disables many of these. Consequently, scanning takes long enough that
NM/wpa_supplicant bails.
So, I think the way forward is to include the patch linked above with the
rest of the series. Although perhaps the last patch moving ath5k/ath9k etc
under the ath/ directory can be put off a while in case people have patches
sitting around against the old dir structure.
Anyone who still wants to use all the channels with ath5k can use a custom
regd or configure the supplicant to scan the specific channels they are
interested in.
--
Bob Copeland %% http://www.bobcopeland.com
On Tue, Mar 24, 2009 at 1:31 PM, Johannes Berg
<[email protected]> wrote:
> On Tue, 2009-03-24 at 13:10 -0700, Luis R. Rodriguez wrote:
>
>> > Channel list is a good hint, maybe
>> > it's time to fix iw/nl80211 to send back all the channels :)
>>
>> So that is the other option. I tried looking at that yesterday but
>> wasn't able to find a way to do this. I went through the netlink
>> documenation and even tried modifying the skb on the kernel side as
>> its a dump. Not sure what to do here to fix this.
>
> I looked into yesterday actually, and it's completely non-trivial and
> requires userspace interface changes.
Heh, there goes that idea then. Then in that case I'd advocate even
more trimming the channels down.
Luis
On Tue, Mar 24, 2009 at 4:17 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, Mar 24, 2009 at 4:13 PM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Tue, Mar 24, 2009 at 3:30 PM, Bob Copeland <[email protected]> wrote:
>>> On Tue, 24 Mar 2009 23:24:33 +0100, Christian Lamparter wrote
>>>
>>>> > Nice....... so it seems all we need now is ath5k figured out and properly
>>>> > tested.
>>>> uhh, looks like ath5k/regd.c got dropped by accident?!
>>>
>>> Well, it wasn't working when I had it either :)
>>
>> My brain is fuzzy here but I think I had decided to not have an
>> ath5k/regd.c. Hm but I see the file there now in my tree.
>
> Ah yes, brain_works_still++, the file was not in the commit log and I
> did move things into the driver elsewhere.
Alright here's a quick concat of all these patches (also one small
sparse fix on Christian's last patch). It also now throws ath5k, ath9k
and ar9170 into ath/. We can rename the drivers after, was lazy to do
that. Think its easier for review to separate that too.
http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v3.patch.txt
Bob, where was that switch break error?
Luis
On Tue, Mar 24, 2009 at 7:59 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, Mar 24, 2009 at 7:30 PM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Tue, Mar 24, 2009 at 6:06 PM, Bob Copeland <[email protected]> w=
rote:
>>> On Tue, Mar 24, 2009 at 04:52:09PM -0700, Luis R. Rodriguez wrote:
>>>> Alright here's a quick concat of all these patches (also one small
>>>> sparse fix on Christian's last patch). It also now throws ath5k, a=
th9k
>>>> and ar9170 into ath/. We can rename the drivers after, was lazy to=
do
>>>> that. Think its easier for review to separate that too.
>>>>
>>>> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v3.pa=
tch.txt
>>>>
>>>> Bob, where was that switch break error?
>>>
>>> 1st patch:
>>>
>>>> + =C2=A0 =C2=A0 switch (band) {
>>>> + =C2=A0 =C2=A0 case IEEE80211_BAND_2GHZ:
>>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulatory=
=2Eregpair->reg_2ghz_ctl;
>>>> + =C2=A0 =C2=A0 case IEEE80211_BAND_5GHZ:
>>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulatory=
=2Eregpair->reg_5ghz_ctl;
>>>> + =C2=A0 =C2=A0 default:
>>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D NO_CTL;
>>>> + =C2=A0 =C2=A0 }
>>>> + =C2=A0 =C2=A0 return ctl;
>>>> +}
>>>
>>> I think I had it like the following and changed it -- actually with
>>> the returns it's probably better...
>>>
>>> switch (band) {
>>> case XXX:
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0return ah->...;
>>> case YYY:
>>> ...
>>> }
>>
>> Alright thanks, I've fixed that here, it also now does not introduce
>> any new checkpatch or sparse complaints.
>>
>> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v4.patc=
h.txt
>>
>> My shiny new wl rebased on 29 kernel is compiling, will let you know
>> in the morning how it goes.
>
> Ah poo, ath5k_reg_notifier() got lost.
=46ixed..
http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v5.patch.t=
xt
Luis
On Tue, Mar 24, 2009 at 01:33:27PM -0700, Christian Lamparter wrote:
> On Tuesday 24 March 2009 21:10:21 Luis R. Rodriguez wrote:
> > On Tue, Mar 24, 2009 at 1:03 PM, Bob Copeland <[email protected]> wrote:
> > > On Tue, Mar 24, 2009 at 2:41 PM, Luis R. Rodriguez
> > > <[email protected]> wrote:
> > >> Do any of you guys have time to pick it up?
> > >
> > > Well my patch #2 is pretty similar to what Christian has, and I'd like to
> > > upstream the first few patches soon (especially the ath.ko creation, it is a
> > > beast to rebase as you saw). That would bring up the first contentious
> > > point of the merge: is "ath/ath.ko" ok with everyone?
> >
> > Yes.
> Yes.
>
> > > ath5k/9k specific
> > > bits can eventually go in ath/{5k,9k} subdirs... not sure where 9170
> > > lives then :)
> >
> > Wherever Christian wants it. If he finds re-usable bits then great.
> maybe ath/9kusb ?
In that case maybe 9170/, we do have future USB plans.
> > > I probably can't get to it until the weekend, but if you have more time,
> > > Christian, feel free to adopt whatever you want from mine. Otherwise I'll
> > > pick up where Luis left off on the weekend.
> Done! Patch attached, please add to the tree ;)
Nice....... so it seems all we need now is ath5k figured out and properly
tested.
Luis
On Tue, Mar 24, 2009 at 04:52:09PM -0700, Luis R. Rodriguez wrote:
> Alright here's a quick concat of all these patches (also one small
> sparse fix on Christian's last patch). It also now throws ath5k, ath9k
> and ar9170 into ath/. We can rename the drivers after, was lazy to do
> that. Think its easier for review to separate that too.
>
> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v3.patch.txt
>
> Bob, where was that switch break error?
1st patch:
> + switch (band) {
> + case IEEE80211_BAND_2GHZ:
> + ctl = ah->regulatory.regpair->reg_2ghz_ctl;
> + case IEEE80211_BAND_5GHZ:
> + ctl = ah->regulatory.regpair->reg_5ghz_ctl;
> + default:
> + ctl = NO_CTL;
> + }
> + return ctl;
> +}
I think I had it like the following and changed it -- actually with
the returns it's probably better...
switch (band) {
case XXX:
return ah->...;
case YYY:
...
}
--
Bob Copeland %% http://www.bobcopeland.com
On Tue, Mar 24, 2009 at 11:59:37AM -0700, Bob Copeland wrote:
> On Tue, Mar 24, 2009 at 2:38 PM, Christian Lamparter <[email protected]> wrote:
> > Hi Luis,
> >
> > This is more or less what I need from ath9k's regulatory domain code.
> > What's your opinion? Would you accept the changes, or do you see
> > a potential conflict/problem with the design?
>
> FWIW I had done a small bit of rework for sharing with ath5k, and
> Luis has those patches now in somewhat better shape. The last
> patch set I made is here:
>
> http://bobcopeland.com/kernel/wl/20090309/
I rebased these on top of the latest wl, and slightly modified them
to apply cleanly with checkpatch and I think I had a few very minor changes.
You can find the latest here:
http://bombadil.infradead.org/~mcgrof/patches/ath5k/ath5k-reg.patch.txt
I thought I was going to be able to work on this last week but that wasn't the
case and it seems I won't get to it this week either. Do any of you guys have
time to pick it up? Only change I'd recommend is to first make ath5k use a
small set of channels as ath9k and ar9170 has, then you can actually debug
regulatory stuff using iw by calling 'iw list' and checking the channel info.
I was going to start with that but saw its a little pain. Essentially the way
I'd do it is to modify ath5k_setup_bands() to remove the channel hw specific
settings and move that stuff to a routine which can be called dynamically upon
channel change (IEEE80211_CONF_CHANGE_CHANNEL) through the mac80211 config
callback. In ath9k we do this with the ath9k_update_ichannel(). The idea
was to eventually condense that down to a ath9k_update_ht_mode() as I think
that's all that would be required. But do do that we'd need to slowly completley
remove ath9k_channel. That's a bit of a challenge, provided you want to make
sure everything else works. But anyway that should give you an idea of where
I was heading if you want to try something similar for ath5k.
Technically speaking it'd be even nice to not have to compute this upon channel
change so that way we save time during a channel change, but that again is a nice
challenge. For example it'd be hot of we wouldn't have to compute the hw value for
the frequency on ath9k every single time as its a static value.
Anyway that's my feedback.
Luis
On Tue, Mar 24, 2009 at 3:30 PM, Bob Copeland <[email protected]> wrote:
> On Tue, 24 Mar 2009 23:24:33 +0100, Christian Lamparter wrote
>
>> > Nice....... so it seems all we need now is ath5k figured out and properly
>> > tested.
>> uhh, looks like ath5k/regd.c got dropped by accident?!
>
> Well, it wasn't working when I had it either :)
My brain is fuzzy here but I think I had decided to not have an
ath5k/regd.c. Hm but I see the file there now in my tree.
Anyway we should get a newer shinier patch which has all these changes
so far + ar9170 changes and test them. Whoever gets to it first please
post one.
Luis
On Tue, 2009-03-24 at 13:10 -0700, Luis R. Rodriguez wrote:
> > Channel list is a good hint, maybe
> > it's time to fix iw/nl80211 to send back all the channels :)
>
> So that is the other option. I tried looking at that yesterday but
> wasn't able to find a way to do this. I went through the netlink
> documenation and even tried modifying the skb on the kernel side as
> its a dump. Not sure what to do here to fix this.
I looked into yesterday actually, and it's completely non-trivial and
requires userspace interface changes.
johannes
On Tue, Mar 24, 2009 at 2:38 PM, Christian Lamparter <[email protected]> wrote:
> Hi Luis,
>
> This is more or less what I need from ath9k's regulatory domain code.
> What's your opinion? Would you accept the changes, or do you see
> a potential conflict/problem with the design?
FWIW I had done a small bit of rework for sharing with ath5k, and
Luis has those patches now in somewhat better shape. The last
patch set I made is here:
http://bobcopeland.com/kernel/wl/20090309/
--
Bob Copeland %% http://www.bobcopeland.com
On Tue, Mar 24, 2009 at 03:09:12PM -0700, Bob Copeland wrote:
> On Tue, 24 Mar 2009 13:58:47 -0700, Luis R. Rodriguez wrote
> > > > > pick up where Luis left off on the weekend.
> > > Done! Patch attached, please add to the tree ;)
> >
> > Nice....... so it seems all we need now is ath5k figured out and properly
> > tested.
>
> BTW there's a dumb bug in my first patch, missing 'break' in the switch
> statements. Oops. But ath5k wasn't using any of that since it didn't
> do the CTL stuff yet.
>
> I'll go ahead and rebase the stuff and repost it later in the week,
> as long as you guys can test ath9k/ar9170 then I can do the debugging
> for 5k..
Nice, I think I can at least commit now to reviewing and testing ath9k
stuff. The good news is this is all going for a .31 so we'll have a good
amount of time to test.
Luis
On Tue, Mar 24, 2009 at 8:15 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, Mar 24, 2009 at 7:59 PM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Tue, Mar 24, 2009 at 7:30 PM, Luis R. Rodriguez
>> <[email protected]> wrote:
>>> On Tue, Mar 24, 2009 at 6:06 PM, Bob Copeland <[email protected]> =
wrote:
>>>> On Tue, Mar 24, 2009 at 04:52:09PM -0700, Luis R. Rodriguez wrote:
>>>>> Alright here's a quick concat of all these patches (also one smal=
l
>>>>> sparse fix on Christian's last patch). It also now throws ath5k, =
ath9k
>>>>> and ar9170 into ath/. We can rename the drivers after, was lazy t=
o do
>>>>> that. Think its easier for review to separate that too.
>>>>>
>>>>> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v3.p=
atch.txt
>>>>>
>>>>> Bob, where was that switch break error?
>>>>
>>>> 1st patch:
>>>>
>>>>> + =C2=A0 =C2=A0 switch (band) {
>>>>> + =C2=A0 =C2=A0 case IEEE80211_BAND_2GHZ:
>>>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulator=
y.regpair->reg_2ghz_ctl;
>>>>> + =C2=A0 =C2=A0 case IEEE80211_BAND_5GHZ:
>>>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulator=
y.regpair->reg_5ghz_ctl;
>>>>> + =C2=A0 =C2=A0 default:
>>>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D NO_CTL;
>>>>> + =C2=A0 =C2=A0 }
>>>>> + =C2=A0 =C2=A0 return ctl;
>>>>> +}
>>>>
>>>> I think I had it like the following and changed it -- actually wit=
h
>>>> the returns it's probably better...
>>>>
>>>> switch (band) {
>>>> case XXX:
>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0return ah->...;
>>>> case YYY:
>>>> ...
>>>> }
>>>
>>> Alright thanks, I've fixed that here, it also now does not introduc=
e
>>> any new checkpatch or sparse complaints.
>>>
>>> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v4.pat=
ch.txt
>>>
>>> My shiny new wl rebased on 29 kernel is compiling, will let you kno=
w
>>> in the morning how it goes.
>>
>> Ah poo, ath5k_reg_notifier() got lost.
>
> Fixed..
>
> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v5.patch=
=2Etxt
Nope.. no luck, something got messed up along the way... needs a closer=
look.
Luis
On Tue, Mar 24, 2009 at 7:30 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, Mar 24, 2009 at 6:06 PM, Bob Copeland <[email protected]> wr=
ote:
>> On Tue, Mar 24, 2009 at 04:52:09PM -0700, Luis R. Rodriguez wrote:
>>> Alright here's a quick concat of all these patches (also one small
>>> sparse fix on Christian's last patch). It also now throws ath5k, at=
h9k
>>> and ar9170 into ath/. We can rename the drivers after, was lazy to =
do
>>> that. Think its easier for review to separate that too.
>>>
>>> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v3.pat=
ch.txt
>>>
>>> Bob, where was that switch break error?
>>
>> 1st patch:
>>
>>> + =C2=A0 =C2=A0 switch (band) {
>>> + =C2=A0 =C2=A0 case IEEE80211_BAND_2GHZ:
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulatory.=
regpair->reg_2ghz_ctl;
>>> + =C2=A0 =C2=A0 case IEEE80211_BAND_5GHZ:
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulatory.=
regpair->reg_5ghz_ctl;
>>> + =C2=A0 =C2=A0 default:
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D NO_CTL;
>>> + =C2=A0 =C2=A0 }
>>> + =C2=A0 =C2=A0 return ctl;
>>> +}
>>
>> I think I had it like the following and changed it -- actually with
>> the returns it's probably better...
>>
>> switch (band) {
>> case XXX:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0return ah->...;
>> case YYY:
>> ...
>> }
>
> Alright thanks, I've fixed that here, it also now does not introduce
> any new checkpatch or sparse complaints.
>
> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v4.patch=
=2Etxt
>
> My shiny new wl rebased on 29 kernel is compiling, will let you know
> in the morning how it goes.
Ah poo, ath5k_reg_notifier() got lost.
Luis
On Tue, 24 Mar 2009 13:58:47 -0700, Luis R. Rodriguez wrote
> > > > pick up where Luis left off on the weekend.
> > Done! Patch attached, please add to the tree ;)
>
> Nice....... so it seems all we need now is ath5k figured out and properly
> tested.
BTW there's a dumb bug in my first patch, missing 'break' in the switch
statements. Oops. But ath5k wasn't using any of that since it didn't
do the CTL stuff yet.
I'll go ahead and rebase the stuff and repost it later in the week,
as long as you guys can test ath9k/ar9170 then I can do the debugging
for 5k..
--
Bob Copeland %% http://www.bobcopeland.com
On Tue, Mar 24, 2009 at 1:03 PM, Bob Copeland <[email protected]> wrot=
e:
> On Tue, Mar 24, 2009 at 2:41 PM, Luis R. Rodriguez
> <[email protected]> wrote:
>> Do any of you guys have time to pick it up?
>
> Well my patch #2 is pretty similar to what Christian has, and I'd lik=
e to
> upstream the first few patches soon (especially the ath.ko creation, =
it is a
> beast to rebase as you saw). =C2=A0That would bring up the first cont=
entious
> point of the merge: is "ath/ath.ko" ok with everyone?
Yes.
> ath5k/9k specific
> bits can eventually go in ath/{5k,9k} subdirs... not sure where 9170
> lives then :)
Wherever Christian wants it. If he finds re-usable bits then great.
> I probably can't get to it until the weekend, but if you have more ti=
me,
> Christian, feel free to adopt whatever you want from mine. =C2=A0Othe=
rwise I'll
> pick up where Luis left off on the weekend.
>
> I didn't push it yet because ath5k was having problems once I turned =
it
> on for ath5k,
So this is why I recommend to trim the channel list down of ath5k. The
other reason is you won't have users spending what may be 3/4 of their
time scanning on channels that they won't ever find APs in. What can
be done here is add flag or command as we had reviewed a while back
to allow the user to enable these channels if he knows what he is
doing so that way we know we don't mind bothering scanning on the
gazillion channels. But that is just my advice. My concern first is to
get users working happily and quickly associated to an AP and I think
that parallels those goals and puts as secondary the bells and
whistles of adding every single possible channel.
> and I have no other hardware to test with so wanted to
> figure out what was broken first.
> Channel list is a good hint, maybe
> it's time to fix iw/nl80211 to send back all the channels :)
So that is the other option. I tried looking at that yesterday but
wasn't able to find a way to do this. I went through the netlink
documenation and even tried modifying the skb on the kernel side as
its a dump. Not sure what to do here to fix this.
So yes, both are possible options. Just keep in mind that without
being able to see the channel list with 'iw list' debugging regulatory
is a real royal pain in the ass. So I'd try to fix that first either
through channel reduction (you get another bonus enhancement for
scanning time for users) or through fixing this through iw/nl80211.
Johannes probably can advise best on the second path.
Luis
On Tue, Mar 24, 2009 at 03:04:59PM -0700, Luis R. Rodriguez wrote:
> >> > Channel list is a good hint, maybe
> >> > it's time to fix iw/nl80211 to send back all the channels :)
> >
> > I looked into yesterday actually, and it's completely non-trivial and
> > requires userspace interface changes.
>
> Heh, there goes that idea then. Then in that case I'd advocate even
> more trimming the channels down.
Nick, what do you think of this? I think it's the best compromise
for now - people can still get all the channels, normal users get a
nice speedup in the default case.
--------------------
>From 7289f60a3acf3bb1cfcc2a7ef7c4d88ffd0e13f1 Mon Sep 17 00:00:00 2001
From: Bob Copeland <[email protected]>
Date: Sat, 28 Mar 2009 12:27:31 -0400
Subject: [PATCH] ath5k: reduce exported channel list
Claiming every available 5 ghz channel has a couple of negative
side-effects: scanning takes a long time, and the channel list
overflows the available buffer space for netlink commands,
resulting in:
$ iw phy phy0 info
command failed: No buffer space available (-105)
This patch adds a modparam so people who want to see all the channels
can do so by passing all_channels=1. By default users will see a
smaller list of channels that works with iw. This also halves scan
time, from 10 seconds down to less than 5.
Changes-licensed-under: 3-Clause-BSD
Signed-off-by: Bob Copeland <[email protected]>
---
drivers/net/wireless/ath5k/base.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 5d57d77..a018106 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -64,6 +64,10 @@ static int modparam_nohwcrypt;
module_param_named(nohwcrypt, modparam_nohwcrypt, int, 0444);
MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption.");
+static int modparam_all_channels;
+module_param_named(all_channels, modparam_all_channels, int, 0444);
+MODULE_PARM_DESC(all_channels, "Expose all channels the device can use.");
+
/******************\
* Internal defines *
@@ -862,6 +866,20 @@ ath5k_ieee2mhz(short chan)
return 2212 + chan * 20;
}
+/*
+ * Returns true for the channel numbers user without all_channels modparam.
+ */
+static bool ath5k_is_standard_channel(short chan)
+{
+ return ((chan <= 14) ||
+ /* UNII 1,2 */
+ ((chan & 3) == 0 && chan >= 36 && chan <= 64) ||
+ /* midband */
+ ((chan & 3) == 0 && chan >= 100 && chan <= 140) ||
+ /* UNII-3 */
+ ((chan & 3) == 1 && chan >= 149 && chan <= 165));
+}
+
static unsigned int
ath5k_copy_channels(struct ath5k_hw *ah,
struct ieee80211_channel *channels,
@@ -899,6 +917,9 @@ ath5k_copy_channels(struct ath5k_hw *ah,
if (!ath5k_channel_ok(ah, freq, chfreq))
continue;
+ if (!modparam_all_channels && !ath5k_is_standard_channel(ch))
+ continue;
+
/* Write channel info and increment counter */
channels[count].center_freq = freq;
channels[count].band = (chfreq == CHANNEL_2GHZ) ?
--
1.6.0.6
--
Bob Copeland %% http://www.bobcopeland.com
2009/3/30 Nick Kossifidis <[email protected]>:
> 2009/3/28 Bob Copeland <[email protected]>:
>> On Tue, Mar 24, 2009 at 03:04:59PM -0700, Luis R. Rodriguez wrote:
>>> >> > Channel list is a good hint, maybe
>>> >> > it's time to fix iw/nl80211 to send back all the channels :)
>>> >
>>> > I looked into yesterday actually, and it's completely non-trivial=
and
>>> > requires userspace interface changes.
>>>
>>> Heh, there goes that idea then. Then in that case I'd advocate even
>>> more trimming the channels down.
>>
>> Nick, what do you think of this? =C2=A0I think it's the best comprom=
ise
>> for now - people can still get all the channels, normal users get a
>> nice speedup in the default case.
>>
By the way it would be nice if you could also clean up CHANNEL_DEBUG
stuff from ath5k.h etc ;-)
--=20
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On Tue, Mar 24, 2009 at 4:13 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, Mar 24, 2009 at 3:30 PM, Bob Copeland <[email protected]> wrote:
>> On Tue, 24 Mar 2009 23:24:33 +0100, Christian Lamparter wrote
>>
>>> > Nice....... so it seems all we need now is ath5k figured out and properly
>>> > tested.
>>> uhh, looks like ath5k/regd.c got dropped by accident?!
>>
>> Well, it wasn't working when I had it either :)
>
> My brain is fuzzy here but I think I had decided to not have an
> ath5k/regd.c. Hm but I see the file there now in my tree.
Ah yes, brain_works_still++, the file was not in the commit log and I
did move things into the driver elsewhere.
Luis
On Tue, 24 Mar 2009 23:24:33 +0100, Christian Lamparter wrote
> > Nice....... so it seems all we need now is ath5k figured out and properly
> > tested.
> uhh, looks like ath5k/regd.c got dropped by accident?!
Well, it wasn't working when I had it either :)
--
Bob Copeland %% http://www.bobcopeland.com
2009/3/28 Bob Copeland <[email protected]>:
> On Tue, Mar 24, 2009 at 03:04:59PM -0700, Luis R. Rodriguez wrote:
>> >> > Channel list is a good hint, maybe
>> >> > it's time to fix iw/nl80211 to send back all the channels :)
>> >
>> > I looked into yesterday actually, and it's completely non-trivial =
and
>> > requires userspace interface changes.
>>
>> Heh, there goes that idea then. Then in that case I'd advocate even
>> more trimming the channels down.
>
> Nick, what do you think of this? =C2=A0I think it's the best compromi=
se
> for now - people can still get all the channels, normal users get a
> nice speedup in the default case.
>
> --------------------
> From 7289f60a3acf3bb1cfcc2a7ef7c4d88ffd0e13f1 Mon Sep 17 00:00:00 200=
1
> From: Bob Copeland <[email protected]>
> Date: Sat, 28 Mar 2009 12:27:31 -0400
> Subject: [PATCH] ath5k: reduce exported channel list
>
> Claiming every available 5 ghz channel has a couple of negative
> side-effects: scanning takes a long time, and the channel list
> overflows the available buffer space for netlink commands,
> resulting in:
>
> =C2=A0 =C2=A0$ iw phy phy0 info
> =C2=A0 =C2=A0command failed: No buffer space available (-105)
>
> This patch adds a modparam so people who want to see all the channels
> can do so by passing all_channels=3D1. =C2=A0By default users will se=
e a
> smaller list of channels that works with iw. =C2=A0This also halves s=
can
> time, from 10 seconds down to less than 5.
>
> Changes-licensed-under: 3-Clause-BSD
>
> Signed-off-by: Bob Copeland <[email protected]>
> ---
> =C2=A0drivers/net/wireless/ath5k/base.c | =C2=A0 21 +++++++++++++++++=
++++
> =C2=A01 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless=
/ath5k/base.c
> index 5d57d77..a018106 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -64,6 +64,10 @@ static int modparam_nohwcrypt;
> =C2=A0module_param_named(nohwcrypt, modparam_nohwcrypt, int, 0444);
> =C2=A0MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption.");
>
> +static int modparam_all_channels;
> +module_param_named(all_channels, modparam_all_channels, int, 0444);
> +MODULE_PARM_DESC(all_channels, "Expose all channels the device can u=
se.");
> +
>
> =C2=A0/******************\
> =C2=A0* Internal defines *
> @@ -862,6 +866,20 @@ ath5k_ieee2mhz(short chan)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return 2212 + =
chan * 20;
> =C2=A0}
>
> +/*
> + * Returns true for the channel numbers user without all_channels mo=
dparam.
> + */
> +static bool ath5k_is_standard_channel(short chan)
> +{
> + =C2=A0 =C2=A0 =C2=A0 return ((chan <=3D 14) ||
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* UNII 1,2 */
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ((chan & 3) =3D=3D=
0 && chan >=3D 36 && chan <=3D 64) ||
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* midband */
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ((chan & 3) =3D=3D=
0 && chan >=3D 100 && chan <=3D 140) ||
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* UNII-3 */
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ((chan & 3) =3D=3D=
1 && chan >=3D 149 && chan <=3D 165));
> +}
> +
> =C2=A0static unsigned int
> =C2=A0ath5k_copy_channels(struct ath5k_hw *ah,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct ieee802=
11_channel *channels,
> @@ -899,6 +917,9 @@ ath5k_copy_channels(struct ath5k_hw *ah,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!ath5k_cha=
nnel_ok(ah, freq, chfreq))
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0continue;
>
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!modparam_all_=
channels && !ath5k_is_standard_channel(ch))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 continue;
> +
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Write chann=
el info and increment counter */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0channels[count=
].center_freq =3D freq;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0channels[count=
].band =3D (chfreq =3D=3D CHANNEL_2GHZ) ?
Seems fine ;-)
Acked-by: Nick Kossifidis <[email protected]>
--=20
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On Tuesday 24 March 2009 21:58:47 Luis R. Rodriguez wrote:
> On Tue, Mar 24, 2009 at 01:33:27PM -0700, Christian Lamparter wrote:
> > On Tuesday 24 March 2009 21:10:21 Luis R. Rodriguez wrote:
> > > On Tue, Mar 24, 2009 at 1:03 PM, Bob Copeland <[email protected]> wrote:
> > > > On Tue, Mar 24, 2009 at 2:41 PM, Luis R. Rodriguez <[email protected]> wrote:
> > > > ath5k/9k specific
> > > > bits can eventually go in ath/{5k,9k} subdirs... not sure where 9170
> > > > lives then :)
> > >
> > > Wherever Christian wants it. If he finds re-usable bits then great.
> > maybe ath/9kusb ?
>
> In that case maybe 9170/, we do have future USB plans.
alright, ath/9170/ it is!
> > > > I probably can't get to it until the weekend, but if you have more time,
> > > > Christian, feel free to adopt whatever you want from mine. Otherwise I'll
> > > > pick up where Luis left off on the weekend.
> > Done! Patch attached, please add to the tree ;)
>
> Nice....... so it seems all we need now is ath5k figured out and properly
> tested.
uhh, looks like ath5k/regd.c got dropped by accident?!
Regards,
Chr
On Mon, Mar 30, 2009 at 02:15:34AM +0300, Nick Kossifidis wrote:
> >> Nick, what do you think of this? ??I think it's the best compromise
> >> for now - people can still get all the channels, normal users get a
> >> nice speedup in the default case.
> >>
>
> By the way it would be nice if you could also clean up CHANNEL_DEBUG
> stuff from ath5k.h etc ;-)
Ah, sure, will do that in a followup (also ATH_CHAN_MAX & friends).
--
Bob Copeland %% http://www.bobcopeland.com
On Tue, Mar 24, 2009 at 6:06 PM, Bob Copeland <[email protected]> wrot=
e:
> On Tue, Mar 24, 2009 at 04:52:09PM -0700, Luis R. Rodriguez wrote:
>> Alright here's a quick concat of all these patches (also one small
>> sparse fix on Christian's last patch). It also now throws ath5k, ath=
9k
>> and ar9170 into ath/. We can rename the drivers after, was lazy to d=
o
>> that. Think its easier for review to separate that too.
>>
>> http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v3.patc=
h.txt
>>
>> Bob, where was that switch break error?
>
> 1st patch:
>
>> + =C2=A0 =C2=A0 switch (band) {
>> + =C2=A0 =C2=A0 case IEEE80211_BAND_2GHZ:
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulatory.r=
egpair->reg_2ghz_ctl;
>> + =C2=A0 =C2=A0 case IEEE80211_BAND_5GHZ:
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D ah->regulatory.r=
egpair->reg_5ghz_ctl;
>> + =C2=A0 =C2=A0 default:
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ctl =3D NO_CTL;
>> + =C2=A0 =C2=A0 }
>> + =C2=A0 =C2=A0 return ctl;
>> +}
>
> I think I had it like the following and changed it -- actually with
> the returns it's probably better...
>
> switch (band) {
> case XXX:
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return ah->...;
> case YYY:
> ...
> }
Alright thanks, I've fixed that here, it also now does not introduce
any new checkpatch or sparse complaints.
http://bombadil.infradead.org/~mcgrof/patches/ath/ath-common-v4.patch.t=
xt
My shiny new wl rebased on 29 kernel is compiling, will let you know
in the morning how it goes.
Luis
On Tue, Mar 24, 2009 at 2:41 PM, Luis R. Rodriguez
<[email protected]> wrote:
> Do any of you guys have time to pick it up?
Well my patch #2 is pretty similar to what Christian has, and I'd like to
upstream the first few patches soon (especially the ath.ko creation, it is a
beast to rebase as you saw). That would bring up the first contentious
point of the merge: is "ath/ath.ko" ok with everyone? ath5k/9k specific
bits can eventually go in ath/{5k,9k} subdirs... not sure where 9170
lives then :)
I probably can't get to it until the weekend, but if you have more time,
Christian, feel free to adopt whatever you want from mine. Otherwise I'll
pick up where Luis left off on the weekend.
I didn't push it yet because ath5k was having problems once I turned it
on for ath5k, and I have no other hardware to test with so wanted to
figure out what was broken first. Channel list is a good hint, maybe
it's time to fix iw/nl80211 to send back all the channels :)
--
Bob Copeland %% http://www.bobcopeland.com