Re: [RFC] mac80211: convert HW flags to unsigned long bitmap
On Tue, 2015-06-02 at 23:35 -0700, Joe Perches wrote:
+ IEEE80211_HW_HAS_RATE_CONTROL,
+ IEEE80211_HW_RX_INCLUDES_FCS,
It may be nicer to use specified bit numbers here.
It may make compatibility easier and maybe it should be
written down that new entries are only to be added at
the bottom of the enum and not inserted in the middle.
There's no reason for that, since it's pure kernel internal API any
renumbering of these bits is perfectly fine.
This is similar to the broadcom tg3 driver, but a little different.
The mechanism in tg3 compared to ieee80211_hw is
tg3_flagieee80211_hw_check
tg3_flag_setieee80211_hw_set
tg3_flag_clear ?
Would a ieee80211_hw_clear be useful?
See my reply to Julian.
Would it be clearer without the _check?
Well, I thought it would read as
ieee80211_hw check has_rate_control
with this in a sense, for example.
static ssize_t hwflags_read(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
[]
+ for (i = 0; i NUM_IEEE80211_HW_FLAGS; i++) {
+ if (test_bit(i, local-hw.flags))
Maybe use the ieee80211_hw_check() function?
Obviously that won't work. :) I could use the _ieee80211 one, but I'm
considering just getting rid of that one and inlining the test_bit into
the macro instead.
johannes
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] mac80211: convert HW flags to unsigned long bitmap
Hi Johannes, On Wed, Jun 3, 2015 at 5:02 PM, Johannes Berg johan...@sipsolutions.net wrote: On Wed, 2015-06-03 at 10:57 +1000, Julian Calaby wrote: Why not add a ieee80211_hw_clear() function? Yes, it'd only be used in 3 places, but it'd look neater. Well, this was actually intentional. Drivers are not *supposed* to be doing this, the hardware flags are supposed to be constant hardware description flags. The three drivers clearing this specific but currently work OK due to the way this particular flag is treated, but that wouldn't necessarily the case for all of the flags; adding a _clear() function would seem to imply that it's OK to do this, when it's really not. Ok, fair enough then. It just looked really odd to be setting it with a function in one branch of an if statement and clearing it open coded in the other. Thanks, -- Julian Calaby Email: julian.cal...@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] mac80211: convert HW flags to unsigned long bitmap
On Wed, 2015-06-03 at 10:57 +1000, Julian Calaby wrote: Why not add a ieee80211_hw_clear() function? Yes, it'd only be used in 3 places, but it'd look neater. Well, this was actually intentional. Drivers are not *supposed* to be doing this, the hardware flags are supposed to be constant hardware description flags. The three drivers clearing this specific but currently work OK due to the way this particular flag is treated, but that wouldn't necessarily the case for all of the flags; adding a _clear() function would seem to imply that it's OK to do this, when it's really not. johannes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] mac80211: convert HW flags to unsigned long bitmap
On Tue, 2015-06-02 at 21:39 +0200, Johannes Berg wrote:
From: Johannes Berg johannes.b...@intel.com
As we're running out of hardware capability flags pretty quickly,
convert them to use the regular test_bit() style unsigned long
bitmaps.
I think this is nice, thanks.
diff --git a/drivers/net/wireless/adm8211.c b/drivers/net/wireless/adm8211.c
[]
@@ -1369,9 +1369,9 @@ static void adm8211_configure_filter(struct
ieee80211_hw *dev,
[]
- dev-flags |= IEEE80211_HW_RX_INCLUDES_FCS;
+ ieee80211_hw_set(dev, RX_INCLUDES_FCS);
[]
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
[]
@@ -1889,35 +1889,38 @@ struct ieee80211_txq {
[]
enum ieee80211_hw_flags {
- IEEE80211_HW_HAS_RATE_CONTROL = 10,
- IEEE80211_HW_RX_INCLUDES_FCS= 11,
[]
+ IEEE80211_HW_HAS_RATE_CONTROL,
+ IEEE80211_HW_RX_INCLUDES_FCS,
It may be nicer to use specified bit numbers here.
It may make compatibility easier and maybe it should be
written down that new entries are only to be added at
the bottom of the enum and not inserted in the middle.
[]
+static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
+enum ieee80211_hw_flags flg)
+{
+ return test_bit(flg, hw-flags);
+}
+#define ieee80211_hw_check(hw, flg) _ieee80211_hw_check(hw,
IEEE80211_HW_##flg)
+
+static inline void _ieee80211_hw_set(struct ieee80211_hw *hw,
+ enum ieee80211_hw_flags flg)
+{
+ return __set_bit(flg, hw-flags);
+}
This is similar to the broadcom tg3 driver, but a little different.
The mechanism in tg3 compared to ieee80211_hw is
tg3_flagieee80211_hw_check
tg3_flag_setieee80211_hw_set
tg3_flag_clear ?
Would a ieee80211_hw_clear be useful?
Would it be clearer without the _check?
static ssize_t hwflags_read(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
[]
+ for (i = 0; i NUM_IEEE80211_HW_FLAGS; i++) {
+ if (test_bit(i, local-hw.flags))
Maybe use the ieee80211_hw_check() function?
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] mac80211: convert HW flags to unsigned long bitmap
From: Johannes Berg johannes.b...@intel.com As we're running out of hardware capability flags pretty quickly, convert them to use the regular test_bit() style unsigned long bitmaps. This introduces a number of helper functions/macros to set and to test the bits, along with new debugfs code. Signed-off-by: Johannes Berg johannes.b...@intel.com --- drivers/net/wireless/adm8211.c | 8 +- drivers/net/wireless/at76c50x-usb.c| 4 +- drivers/net/wireless/ath/ar5523/ar5523.c | 6 +- drivers/net/wireless/ath/ath10k/mac.c | 24 +++--- drivers/net/wireless/ath/ath5k/base.c | 12 +-- drivers/net/wireless/ath/ath9k/htc_drv_init.c | 20 ++--- drivers/net/wireless/ath/ath9k/init.c | 24 +++--- drivers/net/wireless/ath/carl9170/fw.c | 2 +- drivers/net/wireless/ath/carl9170/main.c | 20 ++--- drivers/net/wireless/ath/wcn36xx/main.c| 12 +-- drivers/net/wireless/b43/main.c| 4 +- drivers/net/wireless/b43legacy/main.c | 5 +- .../net/wireless/brcm80211/brcmsmac/mac80211_if.c | 7 +- drivers/net/wireless/cw1200/main.c | 16 ++-- drivers/net/wireless/iwlegacy/3945-mac.c | 6 +- drivers/net/wireless/iwlegacy/4965-mac.c | 12 +-- drivers/net/wireless/iwlwifi/dvm/mac80211.c| 24 +++--- drivers/net/wireless/iwlwifi/mvm/mac-ctxt.c| 4 +- drivers/net/wireless/iwlwifi/mvm/mac80211.c| 30 +++ drivers/net/wireless/libertas_tf/main.c| 2 +- drivers/net/wireless/mac80211_hwsim.c | 26 +++--- drivers/net/wireless/mwl8k.c | 9 +- drivers/net/wireless/p54/main.c| 12 +-- drivers/net/wireless/rsi/rsi_91x_mac80211.c| 7 +- drivers/net/wireless/rt2x00/rt2400pci.c| 8 +- drivers/net/wireless/rt2x00/rt2500pci.c| 8 +- drivers/net/wireless/rt2x00/rt2500usb.c| 9 +- drivers/net/wireless/rt2x00/rt2800lib.c| 16 ++-- drivers/net/wireless/rt2x00/rt61pci.c | 9 +- drivers/net/wireless/rt2x00/rt73usb.c | 9 +- drivers/net/wireless/rtl818x/rtl8180/dev.c | 9 +- drivers/net/wireless/rtl818x/rtl8187/dev.c | 6 +- drivers/net/wireless/rtlwifi/base.c| 22 +++-- drivers/net/wireless/ti/wl1251/main.c | 3 +- drivers/net/wireless/ti/wlcore/main.c | 26 +++--- drivers/net/wireless/zd1211rw/zd_mac.c | 8 +- drivers/staging/vt6655/device_main.c | 8 +- drivers/staging/vt6656/main_usb.c | 8 +- include/net/mac80211.h | 77 ++--- net/mac80211/agg-tx.c | 4 +- net/mac80211/cfg.c | 10 +-- net/mac80211/debugfs.c | 96 -- net/mac80211/driver-ops.h | 2 +- net/mac80211/iface.c | 10 +-- net/mac80211/key.c | 4 +- net/mac80211/main.c| 14 ++-- net/mac80211/mlme.c| 63 +++--- net/mac80211/offchannel.c | 2 +- net/mac80211/pm.c | 4 +- net/mac80211/rate.c| 6 +- net/mac80211/rc80211_minstrel_ht.c | 2 +- net/mac80211/rx.c | 26 +++--- net/mac80211/scan.c| 10 +-- net/mac80211/sta_info.c| 14 ++-- net/mac80211/status.c | 13 ++- net/mac80211/tdls.c| 2 +- net/mac80211/tx.c | 34 net/mac80211/util.c| 6 +- 58 files changed, 433 insertions(+), 411 deletions(-) diff --git a/drivers/net/wireless/adm8211.c b/drivers/net/wireless/adm8211.c index 413528295d72..ad6ead9a8acb 100644 --- a/drivers/net/wireless/adm8211.c +++ b/drivers/net/wireless/adm8211.c @@ -1369,9 +1369,9 @@ static void adm8211_configure_filter(struct ieee80211_hw *dev, ADM8211_CSR_READ(NAR); if (priv-nar ADM8211_NAR_PR) - dev-flags |= IEEE80211_HW_RX_INCLUDES_FCS; + ieee80211_hw_set(dev, RX_INCLUDES_FCS); else - dev-flags = ~IEEE80211_HW_RX_INCLUDES_FCS; + __clear_bit(IEEE80211_HW_RX_INCLUDES_FCS, dev-flags); if (*total_flags FIF_BCN_PRBRESP_PROMISC) adm8211_set_bssid(dev, bcast); @@ -1857,8 +1857,8 @@ static int adm8211_probe(struct pci_dev *pdev, SET_IEEE80211_PERM_ADDR(dev, perm_addr); dev-extra_tx_headroom = sizeof(struct adm8211_tx_hdr); - /* dev-flags = IEEE80211_HW_RX_INCLUDES_FCS in promisc mode */ - dev-flags =
Re: [RFC] mac80211: convert HW flags to unsigned long bitmap
Hi Johannes,
Minor nit:
On Wed, Jun 3, 2015 at 5:39 AM, Johannes Berg johan...@sipsolutions.net wrote:
From: Johannes Berg johannes.b...@intel.com
As we're running out of hardware capability flags pretty quickly,
convert them to use the regular test_bit() style unsigned long
bitmaps.
This introduces a number of helper functions/macros to set and to
test the bits, along with new debugfs code.
Signed-off-by: Johannes Berg johannes.b...@intel.com
---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index e09a32cb139f..dc56734d0154 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2050,6 +2053,20 @@ struct ieee80211_hw {
int txq_ac_max_pending;
};
+static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
+ enum ieee80211_hw_flags flg)
+{
+ return test_bit(flg, hw-flags);
+}
+#define ieee80211_hw_check(hw, flg)_ieee80211_hw_check(hw,
IEEE80211_HW_##flg)
+
+static inline void _ieee80211_hw_set(struct ieee80211_hw *hw,
+enum ieee80211_hw_flags flg)
+{
+ return __set_bit(flg, hw-flags);
+}
+#define ieee80211_hw_set(hw, flg) _ieee80211_hw_set(hw,
IEEE80211_HW_##flg)
+
Why not add a ieee80211_hw_clear() function? Yes, it'd only be used in
3 places, but it'd look neater.
Thanks,
--
Julian Calaby
Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
