Cleanup after API changes patch (checkpatch.pl stuff) and on
ath5k_hw_rf5112_channel() make use of the new channel->band and
existing ath5k_channel_ok() instead of re-implementing the checks
again. This was necessary to make the code cleaner and fit
the 80-chars wide limit so sending it within the same patch.
Finally make a note that we should eventually move cap_range stuff
to struct wiphy.
This patch applies ontop of Nick's API changes patch.
Signed-off-by: Luis R. Rodriguez <[email protected]>
drivers/net/wireless/ath5k/ath5k.h: Changes-licensed-under: ISC
drivers/net/wireless/ath5k/base.c: Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/initvals.c: Changes-licensed-under: ISC
drivers/net/wireless/ath5k/phy.c: Changes-licensed-under: ISC
---
drivers/net/wireless/ath5k/ath5k.h | 8 ++-
drivers/net/wireless/ath5k/base.c | 126 +++++++++++++++++++--------------
drivers/net/wireless/ath5k/initvals.c | 6 +-
drivers/net/wireless/ath5k/phy.c | 48 +++++++++----
4 files changed, 118 insertions(+), 70 deletions(-)
diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 3b03600..7b8a1df 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -263,8 +263,10 @@ enum ath5k_driver_mode {
/* adding this flag to rate_code enables short preamble, see ar5212_reg.h */
#define AR5K_SET_SHORT_PREAMBLE 0x04
-#define HAS_SHPREAMBLE(_ix) (rt->rates[_ix].modulation == IEEE80211_RATE_SHORT_PREAMBLE)
-#define SHPREAMBLE_FLAG(_ix) (HAS_SHPREAMBLE(_ix) ? AR5K_SET_SHORT_PREAMBLE : 0)
+#define HAS_SHPREAMBLE(_ix) \
+ (rt->rates[_ix].modulation == IEEE80211_RATE_SHORT_PREAMBLE)
+#define SHPREAMBLE_FLAG(_ix) \
+ (HAS_SHPREAMBLE(_ix) ? AR5K_SET_SHORT_PREAMBLE : 0)
/****************\
TX DEFINITIONS
@@ -892,6 +894,8 @@ enum ath5k_capability_type {
AR5K_CAP_RFSILENT = 20, /* Supports RFsilent */
};
+
+/* XXX: move cap_range stuff to struct wiphy, use these helper for now */
struct ath5k_capabilities {
/*
* Supported PHY modes
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 8ddac36..bd9c9a8 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -80,7 +80,7 @@ MODULE_AUTHOR("Nick Kossifidis");
MODULE_DESCRIPTION("Support for 5xxx series of Atheros 802.11 wireless LAN cards.");
MODULE_SUPPORTED_DEVICE("Atheros 5xxx WLAN cards");
MODULE_LICENSE("Dual BSD/GPL");
-MODULE_VERSION("0.1.1 (EXPERIMENTAL)");
+MODULE_VERSION("0.5.0 (EXPERIMENTAL)");
/* Known PCI ids */
@@ -513,35 +513,46 @@ ath5k_pci_probe(struct pci_dev *pdev,
sc->ah->ah_mac_srev,
sc->ah->ah_phy_revision);
- if(!sc->ah->ah_single_chip){
+ if (!sc->ah->ah_single_chip) {
/* Single chip radio (!RF5111) */
- if(sc->ah->ah_radio_5ghz_revision && !sc->ah->ah_radio_2ghz_revision) {
+ if (sc->ah->ah_radio_5ghz_revision &&
+ !sc->ah->ah_radio_2ghz_revision) {
/* No 5GHz support -> report 2GHz radio */
- if(!test_bit(AR5K_MODE_11A, sc->ah->ah_capabilities.cap_mode)){
+ if (!test_bit(AR5K_MODE_11A,
+ sc->ah->ah_capabilities.cap_mode)) {
ATH5K_INFO(sc, "RF%s 2GHz radio found (0x%x)\n",
- ath5k_chip_name(AR5K_VERSION_RAD,sc->ah->ah_radio_5ghz_revision),
- sc->ah->ah_radio_5ghz_revision);
- /* No 2GHz support (5110 and some 5Ghz only cards) -> report 5Ghz radio */
- } else if(!test_bit(AR5K_MODE_11B, sc->ah->ah_capabilities.cap_mode)){
+ ath5k_chip_name(AR5K_VERSION_RAD,
+ sc->ah->ah_radio_5ghz_revision),
+ sc->ah->ah_radio_5ghz_revision);
+ /* No 2GHz support (5110 and some
+ * 5Ghz only cards) -> report 5Ghz radio */
+ } else if (!test_bit(AR5K_MODE_11B,
+ sc->ah->ah_capabilities.cap_mode)) {
ATH5K_INFO(sc, "RF%s 5GHz radio found (0x%x)\n",
- ath5k_chip_name(AR5K_VERSION_RAD,sc->ah->ah_radio_5ghz_revision),
- sc->ah->ah_radio_5ghz_revision);
+ ath5k_chip_name(AR5K_VERSION_RAD,
+ sc->ah->ah_radio_5ghz_revision),
+ sc->ah->ah_radio_5ghz_revision);
/* Multiband radio */
} else {
ATH5K_INFO(sc, "RF%s multiband radio found"
" (0x%x)\n",
- ath5k_chip_name(AR5K_VERSION_RAD,sc->ah->ah_radio_5ghz_revision),
- sc->ah->ah_radio_5ghz_revision);
+ ath5k_chip_name(AR5K_VERSION_RAD,
+ sc->ah->ah_radio_5ghz_revision),
+ sc->ah->ah_radio_5ghz_revision);
}
}
- /* Multi chip radio (RF5111 - RF2111) -> report both 2GHz/5GHz radios */
- else if(sc->ah->ah_radio_5ghz_revision && sc->ah->ah_radio_2ghz_revision){
+ /* Multi chip radio (RF5111 - RF2111) ->
+ * report both 2GHz/5GHz radios */
+ else if (sc->ah->ah_radio_5ghz_revision &&
+ sc->ah->ah_radio_2ghz_revision){
ATH5K_INFO(sc, "RF%s 5GHz radio found (0x%x)\n",
- ath5k_chip_name(AR5K_VERSION_RAD,sc->ah->ah_radio_5ghz_revision),
- sc->ah->ah_radio_5ghz_revision);
+ ath5k_chip_name(AR5K_VERSION_RAD,
+ sc->ah->ah_radio_5ghz_revision),
+ sc->ah->ah_radio_5ghz_revision);
ATH5K_INFO(sc, "RF%s 2GHz radio found (0x%x)\n",
- ath5k_chip_name(AR5K_VERSION_RAD,sc->ah->ah_radio_2ghz_revision),
- sc->ah->ah_radio_2ghz_revision);
+ ath5k_chip_name(AR5K_VERSION_RAD,
+ sc->ah->ah_radio_2ghz_revision),
+ sc->ah->ah_radio_2ghz_revision);
}
}
@@ -889,13 +900,15 @@ ath5k_copy_channels(struct ath5k_hw *ah,
/* Write channel info and increment counter */
channels[count].center_freq = freq;
- if((mode == AR5K_MODE_11A) ||
- (mode == AR5K_MODE_11G)){
- channels[count].hw_value = chfreq|CHANNEL_OFDM;
- } else if((mode == AR5K_MODE_11A_TURBO) ||
- (mode == AR5K_MODE_11G_TURBO)){
- channels[count].hw_value = chfreq|CHANNEL_OFDM|CHANNEL_TURBO;
- }if(mode == AR5K_MODE_11B) {
+ if ((mode == AR5K_MODE_11A) ||
+ (mode == AR5K_MODE_11G)) {
+ channels[count].hw_value =
+ chfreq | CHANNEL_OFDM;
+ } else if ((mode == AR5K_MODE_11A_TURBO) ||
+ (mode == AR5K_MODE_11G_TURBO)) {
+ channels[count].hw_value =
+ chfreq | CHANNEL_OFDM|CHANNEL_TURBO;
+ } if (mode == AR5K_MODE_11B) {
channels[count].hw_value = CHANNEL_B;
}
@@ -923,15 +936,16 @@ ath5k_getchannels(struct ieee80211_hw *hw)
count_r = count_c = 0;
/* 2GHz band */
- if(!test_bit(AR5K_MODE_11G, sc->ah->ah_capabilities.cap_mode)){
+ if (!test_bit(AR5K_MODE_11G, sc->ah->ah_capabilities.cap_mode)) {
mode2g = AR5K_MODE_11B;
- if(!test_bit(AR5K_MODE_11B, sc->ah->ah_capabilities.cap_mode)){
+ if (!test_bit(AR5K_MODE_11B,
+ sc->ah->ah_capabilities.cap_mode))
mode2g = -1;
- }
}
- if(mode2g > 0){
- struct ieee80211_supported_band *sband = &sbands[IEEE80211_BAND_2GHZ];
+ if (mode2g > 0) {
+ struct ieee80211_supported_band *sband =
+ &sbands[IEEE80211_BAND_2GHZ];
sband->bitrates = sc->rates;
sband->channels = sc->channels;
@@ -942,7 +956,7 @@ ath5k_getchannels(struct ieee80211_hw *hw)
hw_rates = ath5k_hw_get_rate_table(ah, mode2g);
sband->n_bitrates = ath5k_copy_rates(sband->bitrates,
- hw_rates,max_r);
+ hw_rates, max_r);
count_c = sband->n_channels;
count_r = sband->n_bitrates;
@@ -956,8 +970,9 @@ ath5k_getchannels(struct ieee80211_hw *hw)
/* 5GHz band */
- if(test_bit(AR5K_MODE_11A, sc->ah->ah_capabilities.cap_mode)){
- struct ieee80211_supported_band *sband = &sbands[IEEE80211_BAND_5GHZ];
+ if (test_bit(AR5K_MODE_11A, sc->ah->ah_capabilities.cap_mode)) {
+ struct ieee80211_supported_band *sband =
+ &sbands[IEEE80211_BAND_5GHZ];
sband->bitrates = &sc->rates[count_r];
sband->channels = &sc->channels[count_c];
@@ -968,7 +983,7 @@ ath5k_getchannels(struct ieee80211_hw *hw)
hw_rates = ath5k_hw_get_rate_table(ah, AR5K_MODE_11A);
sband->n_bitrates = ath5k_copy_rates(sband->bitrates,
- hw_rates,max_r);
+ hw_rates, max_r);
hw->wiphy->bands[IEEE80211_BAND_5GHZ] = sband;
}
@@ -1106,7 +1121,7 @@ ath5k_setcurmode(struct ath5k_softc *sc, unsigned int mode)
sc->curmode = mode;
- if(mode == AR5K_MODE_11A){
+ if (mode == AR5K_MODE_11A) {
sc->curband = &sc->sbands[IEEE80211_BAND_5GHZ];
} else {
sc->curband = &sc->sbands[IEEE80211_BAND_2GHZ];
@@ -1158,43 +1173,43 @@ ath5k_mode_setup(struct ath5k_softc *sc)
* When hw returns eg. 27 it points to the last 802.11g rate (54Mbits) etc
*/
static void
-ath5k_set_total_hw_rates(struct ath5k_softc *sc){
+ath5k_set_total_hw_rates(struct ath5k_softc *sc) {
struct ath5k_hw *ah = sc->ah;
- if(test_bit(AR5K_MODE_11A, ah->ah_modes))
+ if (test_bit(AR5K_MODE_11A, ah->ah_modes))
sc->a_rates = 8;
- if(test_bit(AR5K_MODE_11B, ah->ah_modes))
+ if (test_bit(AR5K_MODE_11B, ah->ah_modes))
sc->b_rates = 4;
- if(test_bit(AR5K_MODE_11G, ah->ah_modes))
+ if (test_bit(AR5K_MODE_11G, ah->ah_modes))
sc->g_rates = 12;
-
+
/* XXX: Need to see what what happens when
xr disable bits in eeprom are set */
- if(ah->ah_version >= AR5K_AR5212)
+ if (ah->ah_version >= AR5K_AR5212)
sc->xr_rates = 4;
}
static inline int
-ath5k_hw_to_driver_rix(struct ath5k_softc *sc, int hw_rix){
+ath5k_hw_to_driver_rix(struct ath5k_softc *sc, int hw_rix) {
int mac80211_rix;
- if(sc->curband->band == IEEE80211_BAND_2GHZ){
+ if (sc->curband->band == IEEE80211_BAND_2GHZ) {
/* We setup a g ratetable for both b/g modes */
- mac80211_rix = hw_rix - sc->b_rates - sc->a_rates - sc->xr_rates;
+ mac80211_rix =
+ hw_rix - sc->b_rates - sc->a_rates - sc->xr_rates;
} else {
mac80211_rix = hw_rix - sc->xr_rates;
}
/* Something went wrong, fallback to basic rate for this band */
- if((mac80211_rix >= sc->curband->n_bitrates) ||
- (mac80211_rix <= 0 )){
+ if ((mac80211_rix >= sc->curband->n_bitrates) ||
+ (mac80211_rix <= 0))
mac80211_rix = 1;
- }
return mac80211_rix;
}
@@ -1303,7 +1318,8 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
ret = ah->ah_setup_tx_desc(ah, ds, pktlen,
ieee80211_get_hdrlen_from_skb(skb), AR5K_PKT_TYPE_NORMAL,
- (sc->power_level * 2), ctl->tx_rate->hw_value, ctl->retry_limit, keyidx, 0, flags, 0, 0);
+ (sc->power_level * 2), ctl->tx_rate->hw_value,
+ ctl->retry_limit, keyidx, 0, flags, 0, 0);
if (ret)
goto err_unmap;
@@ -1844,7 +1860,8 @@ accept:
rxs.signal = ds->ds_rxstat.rs_rssi * 100 / 64;
rxs.antenna = ds->ds_rxstat.rs_antenna;
- rxs.rate_idx = ath5k_hw_to_driver_rix(sc,ds->ds_rxstat.rs_rate);
+ rxs.rate_idx = ath5k_hw_to_driver_rix(sc,
+ ds->ds_rxstat.rs_rate);
rxs.flag |= ath5k_rx_decrypted(sc, ds, skb);
ath5k_debug_dump_skb(sc, skb, "RX ", 0);
@@ -1991,8 +2008,9 @@ ath5k_beacon_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
ds->ds_data = bf->skbaddr;
ret = ah->ah_setup_tx_desc(ah, ds, skb->len + FCS_LEN,
ieee80211_get_hdrlen_from_skb(skb),
- AR5K_PKT_TYPE_BEACON, (sc->power_level * 2), ctl->tx_rate->hw_value, 1,
- AR5K_TXKEYIX_INVALID, antenna, flags, 0, 0);
+ AR5K_PKT_TYPE_BEACON, (sc->power_level * 2),
+ ctl->tx_rate->hw_value, 1, AR5K_TXKEYIX_INVALID,
+ antenna, flags, 0, 0);
if (ret)
goto err_unmap;
@@ -2480,7 +2498,8 @@ ath5k_calibrate(unsigned long data)
struct ath5k_hw *ah = sc->ah;
ATH5K_DBG(sc, ATH5K_DEBUG_CALIBRATE, "channel %u/%x\n",
- ieee80211_frequency_to_channel(sc->curchan->center_freq), sc->curchan->hw_value);
+ ieee80211_frequency_to_channel(sc->curchan->center_freq),
+ sc->curchan->hw_value);
if (ath5k_hw_get_rf_gain(ah) == AR5K_RFGAIN_NEED_CHANGE) {
/*
@@ -2492,7 +2511,8 @@ ath5k_calibrate(unsigned long data)
}
if (ath5k_hw_phy_calibrate(ah, sc->curchan))
ATH5K_ERR(sc, "calibration of channel %u failed\n",
- ieee80211_frequency_to_channel(sc->curchan->center_freq));
+ ieee80211_frequency_to_channel(
+ sc->curchan->center_freq));
mod_timer(&sc->calib_tim, round_jiffies(jiffies +
msecs_to_jiffies(ath5k_calinterval * 1000)));
diff --git a/drivers/net/wireless/ath5k/initvals.c b/drivers/net/wireless/ath5k/initvals.c
index a255d8b..cfcb1fe 100644
--- a/drivers/net/wireless/ath5k/initvals.c
+++ b/drivers/net/wireless/ath5k/initvals.c
@@ -1317,8 +1317,10 @@ int ath5k_hw_write_initvals(struct ath5k_hw *ah, u8 mode, bool change_channel)
/* For AR5211 */
} else if (ah->ah_version == AR5K_AR5211) {
- if(mode > 2){ /* AR5K_MODE_11B */
- ATH5K_ERR(ah->ah_sc,"unsupported channel mode: %d\n", mode);
+ /* AR5K_MODE_11B */
+ if (mode > 2) {
+ ATH5K_ERR(ah->ah_sc,
+ "unsupported channel mode: %d\n", mode);
return -EINVAL;
}
diff --git a/drivers/net/wireless/ath5k/phy.c b/drivers/net/wireless/ath5k/phy.c
index 8b576b3..221a18c 100644
--- a/drivers/net/wireless/ath5k/phy.c
+++ b/drivers/net/wireless/ath5k/phy.c
@@ -1124,7 +1124,7 @@ static int ath5k_hw_rf5112_rfregs(struct ath5k_hw *ah,
rf = ah->ah_rf_banks;
if (ah->ah_radio_5ghz_revision >= AR5K_SREV_RAD_2112A
- && !test_bit(AR5K_MODE_11A, ah->ah_capabilities.cap_mode)){
+ && !test_bit(AR5K_MODE_11A, ah->ah_capabilities.cap_mode)) {
rf_ini = rfregs_2112a;
rf_size = ARRAY_SIZE(rfregs_5112a);
if (mode < 2) {
@@ -1445,9 +1445,10 @@ static u32 ath5k_hw_rf5110_chan2athchan(struct ieee80211_channel *channel)
* newer chipsets like the AR5212A who have a completely
* different RF/PHY part.
*/
- athchan = (ath5k_hw_bitswap((ieee80211_frequency_to_channel(channel->center_freq) - 24) / 2, 5) << 1) |
- (1 << 6) | 0x1;
-
+ athchan = (ath5k_hw_bitswap(
+ (ieee80211_frequency_to_channel(
+ channel->center_freq) - 24) / 2, 5)
+ << 1) | (1 << 6) | 0x1;
return athchan;
}
@@ -1506,7 +1507,8 @@ static int ath5k_hw_rf5111_channel(struct ath5k_hw *ah,
struct ieee80211_channel *channel)
{
struct ath5k_athchan_2ghz ath5k_channel_2ghz;
- unsigned int ath5k_channel = ieee80211_frequency_to_channel(channel->center_freq);
+ unsigned int ath5k_channel =
+ ieee80211_frequency_to_channel(channel->center_freq);
u32 data0, data1, clock;
int ret;
@@ -1517,8 +1519,9 @@ static int ath5k_hw_rf5111_channel(struct ath5k_hw *ah,
if (channel->hw_value & CHANNEL_2GHZ) {
/* Map 2GHz channel to 5GHz Atheros channel ID */
- ret = ath5k_hw_rf5111_chan2athchan(ieee80211_frequency_to_channel(channel->center_freq),
- &ath5k_channel_2ghz);
+ ret = ath5k_hw_rf5111_chan2athchan(
+ ieee80211_frequency_to_channel(channel->center_freq),
+ &ath5k_channel_2ghz);
if (ret)
return ret;
@@ -1599,17 +1602,36 @@ static int ath5k_hw_rf5112_channel(struct ath5k_hw *ah,
int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel)
{
int ret;
-
/*
* Check bounds supported by the PHY
* (don't care about regulation restrictions at this point)
*/
- if ((channel->center_freq < ah->ah_capabilities.cap_range.range_2ghz_min ||
- channel->center_freq > ah->ah_capabilities.cap_range.range_2ghz_max) &&
- (channel->center_freq < ah->ah_capabilities.cap_range.range_5ghz_min ||
- channel->center_freq > ah->ah_capabilities.cap_range.range_5ghz_max)) {
+ switch (channel->band) {
+ case IEEE80211_BAND_2GHZ:
+ if (!ath5k_channel_ok(ah, channel->center_freq, CHANNEL_2GHZ)) {
+ ATH5K_ERR(ah->ah_sc,
+ "channel frequency (%u MHz) out of supported "
+ "2GHz band range (%u - %u MHz)\n",
+ channel->center_freq,
+ ah->ah_capabilities.cap_range.range_2ghz_min,
+ ah->ah_capabilities.cap_range.range_2ghz_max);
+ return -EINVAL;
+ }
+ break;
+ case IEEE80211_BAND_5GHZ:
+ if (!ath5k_channel_ok(ah, channel->center_freq, CHANNEL_5GHZ)) {
+ ATH5K_ERR(ah->ah_sc,
+ "channel frequency (%u MHz) out of supported "
+ "5GHz band range (%u - %u MHz)\n",
+ channel->center_freq,
+ ah->ah_capabilities.cap_range.range_5ghz_min,
+ ah->ah_capabilities.cap_range.range_5ghz_max);
+ return -EINVAL;
+ }
+ break;
+ default:
ATH5K_ERR(ah->ah_sc,
- "channel out of supported range (%u MHz)\n",
+ "channel frequency (%u MHz) not in a supported band\n",
channel->center_freq);
return -EINVAL;
}
--
1.5.2.5
On Feb 2, 2008 5:20 PM, Jiri Slaby <[email protected]> wrote:
> On 02/02/2008 10:39 PM, Luis R. Rodriguez wrote:
> > On Feb 2, 2008 4:35 PM, Luis R. Rodriguez <[email protected]> wrote:
> >> On Feb 1, 2008 10:46 AM, Jiri Slaby <[email protected]> wrote:
> >>> On 02/01/2008 02:25 AM, Luis R. Rodriguez wrote:
> >>>> @@ -889,13 +900,15 @@ ath5k_copy_channels(struct ath5k_hw *ah,
> >>>> /* Write channel info and increment counter */
> >>>> channels[count].center_freq = freq;
> >>>>
> >>>> - if((mode == AR5K_MODE_11A) ||
> >>>> - (mode == AR5K_MODE_11G)){
> >>>> - channels[count].hw_value = chfreq|CHANNEL_OFDM;
> >>>> - } else if((mode == AR5K_MODE_11A_TURBO) ||
> >>>> - (mode == AR5K_MODE_11G_TURBO)){
> >>>> - channels[count].hw_value = chfreq|CHANNEL_OFDM|CHANNEL_TURBO;
> >>>> - }if(mode == AR5K_MODE_11B) {
> >>>> + if ((mode == AR5K_MODE_11A) ||
> >>>> + (mode == AR5K_MODE_11G)) {
> >>>> + channels[count].hw_value =
> >>>> + chfreq | CHANNEL_OFDM;
> >>>> + } else if ((mode == AR5K_MODE_11A_TURBO) ||
> >>>> + (mode == AR5K_MODE_11G_TURBO)) {
> >>>> + channels[count].hw_value =
> >>>> + chfreq | CHANNEL_OFDM|CHANNEL_TURBO;
> >>>> + } if (mode == AR5K_MODE_11B) {
> >>> 'else' or '\n' before the if, please
> >> Good catch, that should have been an "else if" there. Will repost all
> >> pending patches in a series addressing all posted comments.
> >
> > Going to move this to a switch, its much more legible.
>
> There was switch IIRC, I think either sparse or -W don't like it too much (enum
> <-> int)? Please could you check? (-W doesn't bother us.)
Thanks for the heads up, will do.
Luis
2008/2/1 Johannes Berg <[email protected]>:
>
> > > I guess you could add a helper function that allocates a channels array
> > > based on a frequency range.
> >
> > This is true but also it would be nice as it is the end points which
> > drivers may want
> > to access every now and then. I think its worth the few bytes.
>
> I'm not really worried about the space that needs, but I don't see why
> the wiphy would need that information.
Export capabilities to userspace would be the main reason. The bands
can be iterated but why do iteration when we can just set the upper
and lower limits?
> Couldn't you keep it in your
> driver's per-hw structure as it is?
Sure, we could. But it seems it would be nice to have this information
easily accessible to drivers and to userspace API.
> Would cfg80211 be required to keep
> them updated according to regulatory rules when the allowed channels
> change?
Nope, I'm just thinking in terms of "hw support capabilities".
> I'm much more worried about all the code that would result in :)
Yeah, I'm not thinking of it in terms of regulatory now. Just in terms
of reasonable capability value accessible to drivers and to userspace.
> But since you're not doing that right now anyway I'll just wait and see
> what you come up with ;)
Hey man, its tough when you have 2 laptops poop out on you in less
than 2 months... this is my excuse this month. Its a new months so I
get one.
Luis
> > What is cap_range and why should it be in struct wiphy?
>
> Its the device's frequency capability range on the bands.
Well since we always need channels I decided to not have such a thing
but rather require registering a channels array that is also used for
bookkeeping about enabled/disabled channels etc.
I guess you could add a helper function that allocates a channels array
based on a frequency range.
johannes
> I'm just going to resend all pending patches. For this stuff I've
> shifted a bit like this:
>
> /*
> * Check bounds supported by the PHY (we don't care about regultory
> * restrictions at this point). Note: hw_value already has the band
> * (CHANNEL_2GHZ, or CHANNEL_5GHZ) so we inform ath5k_channel_ok()
> * of the band by that */
> if (!ath5k_channel_ok(ah, channel->center_freq, channel->hw_value)) {
> char bname[5];
> switch (channel->band) {
> case IEEE80211_BAND_2GHZ:
> strcpy(bname, "2 GHz");
> break;
> case IEEE80211_BAND_5GHZ:
> strcpy(bname, "5 GHz");
> break;
> default:
> BUG_ON(1);
> return -EINVAL;
> }
> ATH5K_ERR(ah->ah_sc,
> "channel frequency (%u MHz) out of supported "
> "%s band range (%u - %u MHz)\n",
> channel->center_freq,
> bname,
> ah->ah_capabilities.cap_range.range_2ghz_min,
> ah->ah_capabilities.cap_range.range_2ghz_max);
> return -EINVAL;
> }
>
We really don't need all this IMHO, ath5k_channel_ok is a simple
working function that's supposed to do all needed checks and print any
error msg, if you want to modify something modify ath5k_channel_ok,
don't add more stuff on the caller. Also why use strings for the band
??? You can have an int and set it 2 or 5 if you want (i realy think
strcpy is baaad).
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
> Finally make a note that we should eventually move cap_range stuff
> to struct wiphy.
What is cap_range and why should it be in struct wiphy?
johannes
2008/2/1, Luis R. Rodriguez <[email protected]>:
> 2008/2/1 Johannes Berg <[email protected]>:
> >
> > > > What is cap_range and why should it be in struct wiphy?
> > >
> > > Its the device's frequency capability range on the bands.
> >
> > Well since we always need channels I decided to not have such a thing
> > but rather require registering a channels array that is also used for
> > bookkeeping about enabled/disabled channels etc.
> >
> > I guess you could add a helper function that allocates a channels array
> > based on a frequency range.
>
> This is true but also it would be nice as it is the end points which
> drivers may want
> to access every now and then. I think its worth the few bytes.
>
> Luis
Why mac80211/cfg80211 has to know about phy's channel range ? I mean
driver knows it and that's all we need to create the big (unfiltered)
channel array. These caps are set for some chip revisions (eg. 5210
supports only part of 5GHz band while 5111/5112 support much wider
range) and only used by channel_ok during channel setup (attach), we
don't have any other use for those inside the driver. Other drivers
don't even need such caps because channel ranges are standard for all
their chip revisions, or they get channels from firmware, or use fixed
range/channel table no matter what phy supports etc.
The only use i can imagine for frequency range information on
mac80211/cfg80211 is that the whole channel setup is being done during
wiphy setup (we provide frequency range and supported bands and
mac80211/cfg80211 does the rest) but we'll have (at least for ath5k)
to fill channel flags (hw_value) anyway.
About the patch: You are right that we should use channel_ok in
hw_channel but there is no need for that switch, channel_ok checks the
channel flags (hw_value) and determines the band already (and channel
flags are filled per band -CHANNEL_2GHZ/CHANNEL_5GHZ- during setup so
checking channel->band or channel->hw_vallue is the same).
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On 02/01/2008 02:25 AM, Luis R. Rodriguez wrote:
> Cleanup after API changes patch (checkpatch.pl stuff) and on
> ath5k_hw_rf5112_channel() make use of the new channel->band and
> existing ath5k_channel_ok() instead of re-implementing the checks
> again. This was necessary to make the code cleaner and fit
> the 80-chars wide limit so sending it within the same patch.
>
> Finally make a note that we should eventually move cap_range stuff
> to struct wiphy.
>
> This patch applies ontop of Nick's API changes patch.
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
>
> drivers/net/wireless/ath5k/ath5k.h: Changes-licensed-under: ISC
> drivers/net/wireless/ath5k/base.c: Changes-licensed-under: 3-Clause-BSD
> drivers/net/wireless/ath5k/initvals.c: Changes-licensed-under: ISC
> drivers/net/wireless/ath5k/phy.c: Changes-licensed-under: ISC
> ---
> drivers/net/wireless/ath5k/ath5k.h | 8 ++-
> drivers/net/wireless/ath5k/base.c | 126 +++++++++++++++++++--------------
> drivers/net/wireless/ath5k/initvals.c | 6 +-
> drivers/net/wireless/ath5k/phy.c | 48 +++++++++----
> 4 files changed, 118 insertions(+), 70 deletions(-)
>
[...]
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 8ddac36..bd9c9a8 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
[...]
> @@ -889,13 +900,15 @@ ath5k_copy_channels(struct ath5k_hw *ah,
> /* Write channel info and increment counter */
> channels[count].center_freq = freq;
>
> - if((mode == AR5K_MODE_11A) ||
> - (mode == AR5K_MODE_11G)){
> - channels[count].hw_value = chfreq|CHANNEL_OFDM;
> - } else if((mode == AR5K_MODE_11A_TURBO) ||
> - (mode == AR5K_MODE_11G_TURBO)){
> - channels[count].hw_value = chfreq|CHANNEL_OFDM|CHANNEL_TURBO;
> - }if(mode == AR5K_MODE_11B) {
> + if ((mode == AR5K_MODE_11A) ||
> + (mode == AR5K_MODE_11G)) {
> + channels[count].hw_value =
> + chfreq | CHANNEL_OFDM;
> + } else if ((mode == AR5K_MODE_11A_TURBO) ||
> + (mode == AR5K_MODE_11G_TURBO)) {
> + channels[count].hw_value =
> + chfreq | CHANNEL_OFDM|CHANNEL_TURBO;
> + } if (mode == AR5K_MODE_11B) {
'else' or '\n' before the if, please
> channels[count].hw_value = CHANNEL_B;
> }
>
On Feb 1, 2008 4:45 PM, Luis R. Rodriguez <[email protected]> wrote:
> On Feb 1, 2008 1:47 PM, Nick Kossifidis <[email protected]> wrote:
> > 2008/2/1, Luis R. Rodriguez <[email protected]>:
> >
> > > 2008/2/1 Johannes Berg <[email protected]>:
> > > >
> > > > > > What is cap_range and why should it be in struct wiphy?
> > > > >
> > > > > Its the device's frequency capability range on the bands.
> > > >
> > > > Well since we always need channels I decided to not have such a thing
> > > > but rather require registering a channels array that is also used for
> > > > bookkeeping about enabled/disabled channels etc.
> > > >
> > > > I guess you could add a helper function that allocates a channels array
> > > > based on a frequency range.
> > >
> > > This is true but also it would be nice as it is the end points which
> > > drivers may want
> > > to access every now and then. I think its worth the few bytes.
> > >
> > > Luis
> >
> > Why mac80211/cfg80211 has to know about phy's channel range ? I mean
> > driver knows it and that's all we need to create the big (unfiltered)
> > channel array.
>
> Yes, you are right, from a driver's perspective this is all we need
> this for. But, for example, say user tries to change to a channel by
> freq, and we want to see if we support it, wouldn't it be easier to
> have mac80211 do the check for us against the wiphy's range on the
> band instead of having the driver do it? mac80211 would do it a lot
> quicker by just knowing the range for example.
>
> > These caps are set for some chip revisions (eg. 5210
> > supports only part of 5GHz band while 5111/5112 support much wider
> > range) and only used by channel_ok during channel setup (attach), we
> > don't have any other use for those inside the driver.
>
> Sure.
>
> > Other drivers
> > don't even need such caps because channel ranges are standard for all
> > their chip revisions, or they get channels from firmware, or use fixed
> > range/channel table no matter what phy supports etc.
>
> I just want to make it available for drivers in central place for
> those who *could* use it and for mac80211 for a quick short cut.
> Additionally it seems it would be good to export this to userspace
> instead of having it iterate over the array to simply get the range.
>
> > The only use i can imagine for frequency range information on
> > mac80211/cfg80211 is that the whole channel setup is being done during
> > wiphy setup (we provide frequency range and supported bands and
> > mac80211/cfg80211 does the rest) but we'll have (at least for ath5k)
> > to fill channel flags (hw_value) anyway.
>
> You are right, we discussed this a while back on the list as well and
> came to the same conclusion. I am curious -- are there drivers which
> have the same hw_value as the freq? But disregard this -- this isn't
> the reason why I brought up the cap. I've mentioned the reasons above
> and to johill in my last reply. Please let me know what you think.
>
> > About the patch: You are right that we should use channel_ok in
> > hw_channel but there is no need for that switch, channel_ok checks the
> > channel flags (hw_value) and determines the band already (and channel
> > flags are filled per band -CHANNEL_2GHZ/CHANNEL_5GHZ- during setup so
> > checking channel->band or channel->hw_vallue is the same).
>
> OK I think I see your point. I can send a follow up patch if you don't mind.
I'm just going to resend all pending patches. For this stuff I've
shifted a bit like this:
/*
* Check bounds supported by the PHY (we don't care about regultory
* restrictions at this point). Note: hw_value already has the band
* (CHANNEL_2GHZ, or CHANNEL_5GHZ) so we inform ath5k_channel_ok()
* of the band by that */
if (!ath5k_channel_ok(ah, channel->center_freq, channel->hw_value)) {
char bname[5];
switch (channel->band) {
case IEEE80211_BAND_2GHZ:
strcpy(bname, "2 GHz");
break;
case IEEE80211_BAND_5GHZ:
strcpy(bname, "5 GHz");
break;
default:
BUG_ON(1);
return -EINVAL;
}
ATH5K_ERR(ah->ah_sc,
"channel frequency (%u MHz) out of supported "
"%s band range (%u - %u MHz)\n",
channel->center_freq,
bname,
ah->ah_capabilities.cap_range.range_2ghz_min,
ah->ah_capabilities.cap_range.range_2ghz_max);
return -EINVAL;
}
On 02/02/2008 10:39 PM, Luis R. Rodriguez wrote:
> On Feb 2, 2008 4:35 PM, Luis R. Rodriguez <[email protected]> wrote:
>> On Feb 1, 2008 10:46 AM, Jiri Slaby <[email protected]> wrote:
>>> On 02/01/2008 02:25 AM, Luis R. Rodriguez wrote:
>>>> @@ -889,13 +900,15 @@ ath5k_copy_channels(struct ath5k_hw *ah,
>>>> /* Write channel info and increment counter */
>>>> channels[count].center_freq = freq;
>>>>
>>>> - if((mode == AR5K_MODE_11A) ||
>>>> - (mode == AR5K_MODE_11G)){
>>>> - channels[count].hw_value = chfreq|CHANNEL_OFDM;
>>>> - } else if((mode == AR5K_MODE_11A_TURBO) ||
>>>> - (mode == AR5K_MODE_11G_TURBO)){
>>>> - channels[count].hw_value = chfreq|CHANNEL_OFDM|CHANNEL_TURBO;
>>>> - }if(mode == AR5K_MODE_11B) {
>>>> + if ((mode == AR5K_MODE_11A) ||
>>>> + (mode == AR5K_MODE_11G)) {
>>>> + channels[count].hw_value =
>>>> + chfreq | CHANNEL_OFDM;
>>>> + } else if ((mode == AR5K_MODE_11A_TURBO) ||
>>>> + (mode == AR5K_MODE_11G_TURBO)) {
>>>> + channels[count].hw_value =
>>>> + chfreq | CHANNEL_OFDM|CHANNEL_TURBO;
>>>> + } if (mode == AR5K_MODE_11B) {
>>> 'else' or '\n' before the if, please
>> Good catch, that should have been an "else if" there. Will repost all
>> pending patches in a series addressing all posted comments.
>
> Going to move this to a switch, its much more legible.
There was switch IIRC, I think either sparse or -W don't like it too much (enum
<-> int)? Please could you check? (-W doesn't bother us.)
On Feb 1, 2008 7:38 AM, Johannes Berg <[email protected]> wrote:
>
> > Finally make a note that we should eventually move cap_range stuff
> > to struct wiphy.
>
> What is cap_range and why should it be in struct wiphy?
Its the device's frequency capability range on the bands.
Luis
> > I guess you could add a helper function that allocates a channels array
> > based on a frequency range.
>
> This is true but also it would be nice as it is the end points which
> drivers may want
> to access every now and then. I think its worth the few bytes.
I'm not really worried about the space that needs, but I don't see why
the wiphy would need that information. Couldn't you keep it in your
driver's per-hw structure as it is? Would cfg80211 be required to keep
them updated according to regulatory rules when the allowed channels
change? I'm much more worried about all the code that would result in :)
But since you're not doing that right now anyway I'll just wait and see
what you come up with ;)
johannes
On Feb 1, 2008 1:47 PM, Nick Kossifidis <[email protected]> wrote:
> 2008/2/1, Luis R. Rodriguez <[email protected]>:
>
> > 2008/2/1 Johannes Berg <[email protected]>:
> > >
> > > > > What is cap_range and why should it be in struct wiphy?
> > > >
> > > > Its the device's frequency capability range on the bands.
> > >
> > > Well since we always need channels I decided to not have such a thing
> > > but rather require registering a channels array that is also used for
> > > bookkeeping about enabled/disabled channels etc.
> > >
> > > I guess you could add a helper function that allocates a channels array
> > > based on a frequency range.
> >
> > This is true but also it would be nice as it is the end points which
> > drivers may want
> > to access every now and then. I think its worth the few bytes.
> >
> > Luis
>
> Why mac80211/cfg80211 has to know about phy's channel range ? I mean
> driver knows it and that's all we need to create the big (unfiltered)
> channel array.
Yes, you are right, from a driver's perspective this is all we need
this for. But, for example, say user tries to change to a channel by
freq, and we want to see if we support it, wouldn't it be easier to
have mac80211 do the check for us against the wiphy's range on the
band instead of having the driver do it? mac80211 would do it a lot
quicker by just knowing the range for example.
> These caps are set for some chip revisions (eg. 5210
> supports only part of 5GHz band while 5111/5112 support much wider
> range) and only used by channel_ok during channel setup (attach), we
> don't have any other use for those inside the driver.
Sure.
> Other drivers
> don't even need such caps because channel ranges are standard for all
> their chip revisions, or they get channels from firmware, or use fixed
> range/channel table no matter what phy supports etc.
I just want to make it available for drivers in central place for
those who *could* use it and for mac80211 for a quick short cut.
Additionally it seems it would be good to export this to userspace
instead of having it iterate over the array to simply get the range.
> The only use i can imagine for frequency range information on
> mac80211/cfg80211 is that the whole channel setup is being done during
> wiphy setup (we provide frequency range and supported bands and
> mac80211/cfg80211 does the rest) but we'll have (at least for ath5k)
> to fill channel flags (hw_value) anyway.
You are right, we discussed this a while back on the list as well and
came to the same conclusion. I am curious -- are there drivers which
have the same hw_value as the freq? But disregard this -- this isn't
the reason why I brought up the cap. I've mentioned the reasons above
and to johill in my last reply. Please let me know what you think.
> About the patch: You are right that we should use channel_ok in
> hw_channel but there is no need for that switch, channel_ok checks the
> channel flags (hw_value) and determines the band already (and channel
> flags are filled per band -CHANNEL_2GHZ/CHANNEL_5GHZ- during setup so
> checking channel->band or channel->hw_vallue is the same).
OK I think I see your point. I can send a follow up patch if you don't mind.
Luis
2008/2/1 Johannes Berg <[email protected]>:
>
> > > What is cap_range and why should it be in struct wiphy?
> >
> > Its the device's frequency capability range on the bands.
>
> Well since we always need channels I decided to not have such a thing
> but rather require registering a channels array that is also used for
> bookkeeping about enabled/disabled channels etc.
>
> I guess you could add a helper function that allocates a channels array
> based on a frequency range.
This is true but also it would be nice as it is the end points which
drivers may want
to access every now and then. I think its worth the few bytes.
Luis
On Feb 1, 2008 10:46 AM, Jiri Slaby <[email protected]> wrote:
> On 02/01/2008 02:25 AM, Luis R. Rodriguez wrote:
> > Cleanup after API changes patch (checkpatch.pl stuff) and on
> > ath5k_hw_rf5112_channel() make use of the new channel->band and
> > existing ath5k_channel_ok() instead of re-implementing the checks
> > again. This was necessary to make the code cleaner and fit
> > the 80-chars wide limit so sending it within the same patch.
> >
> > Finally make a note that we should eventually move cap_range stuff
> > to struct wiphy.
> >
> > This patch applies ontop of Nick's API changes patch.
> >
> > Signed-off-by: Luis R. Rodriguez <[email protected]>
> >
> > drivers/net/wireless/ath5k/ath5k.h: Changes-licensed-under: ISC
> > drivers/net/wireless/ath5k/base.c: Changes-licensed-under: 3-Clause-BSD
> > drivers/net/wireless/ath5k/initvals.c: Changes-licensed-under: ISC
> > drivers/net/wireless/ath5k/phy.c: Changes-licensed-under: ISC
> > ---
> > drivers/net/wireless/ath5k/ath5k.h | 8 ++-
> > drivers/net/wireless/ath5k/base.c | 126 +++++++++++++++++++--------------
> > drivers/net/wireless/ath5k/initvals.c | 6 +-
> > drivers/net/wireless/ath5k/phy.c | 48 +++++++++----
> > 4 files changed, 118 insertions(+), 70 deletions(-)
> >
> [...]
> > diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> > index 8ddac36..bd9c9a8 100644
> > --- a/drivers/net/wireless/ath5k/base.c
> > +++ b/drivers/net/wireless/ath5k/base.c
> [...]
> > @@ -889,13 +900,15 @@ ath5k_copy_channels(struct ath5k_hw *ah,
> > /* Write channel info and increment counter */
> > channels[count].center_freq = freq;
> >
> > - if((mode == AR5K_MODE_11A) ||
> > - (mode == AR5K_MODE_11G)){
> > - channels[count].hw_value = chfreq|CHANNEL_OFDM;
> > - } else if((mode == AR5K_MODE_11A_TURBO) ||
> > - (mode == AR5K_MODE_11G_TURBO)){
> > - channels[count].hw_value = chfreq|CHANNEL_OFDM|CHANNEL_TURBO;
> > - }if(mode == AR5K_MODE_11B) {
> > + if ((mode == AR5K_MODE_11A) ||
> > + (mode == AR5K_MODE_11G)) {
> > + channels[count].hw_value =
> > + chfreq | CHANNEL_OFDM;
> > + } else if ((mode == AR5K_MODE_11A_TURBO) ||
> > + (mode == AR5K_MODE_11G_TURBO)) {
> > + channels[count].hw_value =
> > + chfreq | CHANNEL_OFDM|CHANNEL_TURBO;
> > + } if (mode == AR5K_MODE_11B) {
>
> 'else' or '\n' before the if, please
Good catch, that should have been an "else if" there. Will repost all
pending patches in a series addressing all posted comments.
Luis
On Feb 2, 2008 5:28 PM, Luis R. Rodriguez <[email protected]> wrote:
>
> On Feb 2, 2008 5:20 PM, Jiri Slaby <[email protected]> wrote:
> > On 02/02/2008 10:39 PM, Luis R. Rodriguez wrote:
> > > On Feb 2, 2008 4:35 PM, Luis R. Rodriguez <[email protected]> wrote:
> > >> On Feb 1, 2008 10:46 AM, Jiri Slaby <[email protected]> wrote:
> > >>> On 02/01/2008 02:25 AM, Luis R. Rodriguez wrote:
> > >>>> @@ -889,13 +900,15 @@ ath5k_copy_channels(struct ath5k_hw *ah,
> > >>>> /* Write channel info and increment counter */
> > >>>> channels[count].center_freq = freq;
> > >>>>
> > >>>> - if((mode == AR5K_MODE_11A) ||
> > >>>> - (mode == AR5K_MODE_11G)){
> > >>>> - channels[count].hw_value = chfreq|CHANNEL_OFDM;
> > >>>> - } else if((mode == AR5K_MODE_11A_TURBO) ||
> > >>>> - (mode == AR5K_MODE_11G_TURBO)){
> > >>>> - channels[count].hw_value = chfreq|CHANNEL_OFDM|CHANNEL_TURBO;
> > >>>> - }if(mode == AR5K_MODE_11B) {
> > >>>> + if ((mode == AR5K_MODE_11A) ||
> > >>>> + (mode == AR5K_MODE_11G)) {
> > >>>> + channels[count].hw_value =
> > >>>> + chfreq | CHANNEL_OFDM;
> > >>>> + } else if ((mode == AR5K_MODE_11A_TURBO) ||
> > >>>> + (mode == AR5K_MODE_11G_TURBO)) {
> > >>>> + channels[count].hw_value =
> > >>>> + chfreq | CHANNEL_OFDM|CHANNEL_TURBO;
> > >>>> + } if (mode == AR5K_MODE_11B) {
> > >>> 'else' or '\n' before the if, please
> > >> Good catch, that should have been an "else if" there. Will repost all
> > >> pending patches in a series addressing all posted comments.
> > >
> > > Going to move this to a switch, its much more legible.
> >
> > There was switch IIRC, I think either sparse or -W don't like it too much (enum
> > <-> int)? Please could you check? (-W doesn't bother us.)
>
> Thanks for the heads up, will do.
Tested with sparse from git, no complaints.
Luis
On Feb 2, 2008 6:07 PM, Nick Kossifidis <[email protected]> wrote:
>
> > I'm just going to resend all pending patches. For this stuff I've
> > shifted a bit like this:
> >
> > /*
> > * Check bounds supported by the PHY (we don't care about regultory
> > * restrictions at this point). Note: hw_value already has the band
> > * (CHANNEL_2GHZ, or CHANNEL_5GHZ) so we inform ath5k_channel_ok()
> > * of the band by that */
> > if (!ath5k_channel_ok(ah, channel->center_freq, channel->hw_value)) {
> > char bname[5];
> > switch (channel->band) {
> > case IEEE80211_BAND_2GHZ:
> > strcpy(bname, "2 GHz");
> > break;
> > case IEEE80211_BAND_5GHZ:
> > strcpy(bname, "5 GHz");
> > break;
> > default:
> > BUG_ON(1);
> > return -EINVAL;
> > }
> > ATH5K_ERR(ah->ah_sc,
> > "channel frequency (%u MHz) out of supported "
> > "%s band range (%u - %u MHz)\n",
> > channel->center_freq,
> > bname,
> > ah->ah_capabilities.cap_range.range_2ghz_min,
> > ah->ah_capabilities.cap_range.range_2ghz_max);
> > return -EINVAL;
> > }
> >
>
> We really don't need all this IMHO, ath5k_channel_ok is a simple
> working function that's supposed to do all needed checks and print any
> error msg, if you want to modify something modify ath5k_channel_ok,
> don't add more stuff on the caller. Also why use strings for the band
> ??? You can have an int and set it 2 or 5 if you want (i realy think
> strcpy is baaad).
OK fine, I'll edit ath5k_channel_ok in a later patch, for now I'll
just keep the code to match what was there to finish the cleanup.
Luis
On Feb 2, 2008 4:35 PM, Luis R. Rodriguez <[email protected]> wrote:
>
> On Feb 1, 2008 10:46 AM, Jiri Slaby <[email protected]> wrote:
> > On 02/01/2008 02:25 AM, Luis R. Rodriguez wrote:
> > > Cleanup after API changes patch (checkpatch.pl stuff) and on
> > > ath5k_hw_rf5112_channel() make use of the new channel->band and
> > > existing ath5k_channel_ok() instead of re-implementing the checks
> > > again. This was necessary to make the code cleaner and fit
> > > the 80-chars wide limit so sending it within the same patch.
> > >
> > > Finally make a note that we should eventually move cap_range stuff
> > > to struct wiphy.
> > >
> > > This patch applies ontop of Nick's API changes patch.
> > >
> > > Signed-off-by: Luis R. Rodriguez <[email protected]>
> > >
> > > drivers/net/wireless/ath5k/ath5k.h: Changes-licensed-under: ISC
> > > drivers/net/wireless/ath5k/base.c: Changes-licensed-under: 3-Clause-BSD
> > > drivers/net/wireless/ath5k/initvals.c: Changes-licensed-under: ISC
> > > drivers/net/wireless/ath5k/phy.c: Changes-licensed-under: ISC
> > > ---
> > > drivers/net/wireless/ath5k/ath5k.h | 8 ++-
> > > drivers/net/wireless/ath5k/base.c | 126 +++++++++++++++++++--------------
> > > drivers/net/wireless/ath5k/initvals.c | 6 +-
> > > drivers/net/wireless/ath5k/phy.c | 48 +++++++++----
> > > 4 files changed, 118 insertions(+), 70 deletions(-)
> > >
> > [...]
> > > diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> > > index 8ddac36..bd9c9a8 100644
> > > --- a/drivers/net/wireless/ath5k/base.c
> > > +++ b/drivers/net/wireless/ath5k/base.c
> > [...]
> > > @@ -889,13 +900,15 @@ ath5k_copy_channels(struct ath5k_hw *ah,
> > > /* Write channel info and increment counter */
> > > channels[count].center_freq = freq;
> > >
> > > - if((mode == AR5K_MODE_11A) ||
> > > - (mode == AR5K_MODE_11G)){
> > > - channels[count].hw_value = chfreq|CHANNEL_OFDM;
> > > - } else if((mode == AR5K_MODE_11A_TURBO) ||
> > > - (mode == AR5K_MODE_11G_TURBO)){
> > > - channels[count].hw_value = chfreq|CHANNEL_OFDM|CHANNEL_TURBO;
> > > - }if(mode == AR5K_MODE_11B) {
> > > + if ((mode == AR5K_MODE_11A) ||
> > > + (mode == AR5K_MODE_11G)) {
> > > + channels[count].hw_value =
> > > + chfreq | CHANNEL_OFDM;
> > > + } else if ((mode == AR5K_MODE_11A_TURBO) ||
> > > + (mode == AR5K_MODE_11G_TURBO)) {
> > > + channels[count].hw_value =
> > > + chfreq | CHANNEL_OFDM|CHANNEL_TURBO;
> > > + } if (mode == AR5K_MODE_11B) {
> >
> > 'else' or '\n' before the if, please
>
> Good catch, that should have been an "else if" there. Will repost all
> pending patches in a series addressing all posted comments.
Going to move this to a switch, its much more legible.
Luis