Re: [PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

2018-03-27 Thread Johannes Berg
On Sat, 2018-03-24 at 16:02 -0700, Quytelda Kahja wrote:
> The "document" refers to the file in which the changes were made
> ('include/linux/ieee80211.h').

You're the first person ever to do that, to my knowledge :)

Either way, I don't really want to merge this since it would break
staging, and I don't want to merge a staging fix either... so you'd have
to get that sorted out first.

johannes


Re: [PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

2018-03-26 Thread Quytelda Kahja
This is a problem with the rtl8723bs driver in staging; it's source
tree has a custom IEEE80211 header which imports 'linux/ieee80211.h',
but redefines many of the #define's from the original header.
Functionally, they are the same, but I will submit a patch in reply to
this email which removes the duplicate #defines from
drivers/staging/rtl8723bs/include/ieee80211.h.  It looks like there's
also some #defines there that shadow enum members in
'linux/ieee80211.h', but I will address that in separate patch(es)
when I have a chance.

Thank you,
Quytelda Kahja

On Sun, Mar 25, 2018 at 7:05 AM, kbuild test robot  wrote:
> Hi Quytelda,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on mac80211-next/master]
> [also build test WARNING on v4.16-rc6 next-20180323]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Quytelda-Kahja/ieee80211-Replace-bit-shifts-with-the-BIT-macro-for-WLAN_CAPABILITY_/20180325-211645
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
> config: i386-randconfig-s1-03251817 (attached as .config)
> compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All warnings (new ones prefixed by >>):
>
>In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>>> drivers/staging/rtl8723bs/include/ieee80211.h:444:0: warning: 
>>> "WLAN_CAPABILITY_IBSS" redefined
> #define WLAN_CAPABILITY_IBSS (1<<1)
>
>In file included from include/net/cfg80211.h:23:0,
> from 
> drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
> from drivers/staging/rtl8723bs/include/osdep_service.h:23,
> from drivers/staging/rtl8723bs/include/drv_types.h:29,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>include/linux/ieee80211.h:1593:0: note: this is the location of the 
> previous definition
> #define WLAN_CAPABILITY_IBSS  BIT(1)
>
>In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>>> drivers/staging/rtl8723bs/include/ieee80211.h:445:0: warning: 
>>> "WLAN_CAPABILITY_CF_POLLABLE" redefined
> #define WLAN_CAPABILITY_CF_POLLABLE (1<<2)
>
>In file included from include/net/cfg80211.h:23:0,
> from 
> drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
> from drivers/staging/rtl8723bs/include/osdep_service.h:23,
> from drivers/staging/rtl8723bs/include/drv_types.h:29,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>include/linux/ieee80211.h:1603:0: note: this is the location of the 
> previous definition
> #define WLAN_CAPABILITY_CF_POLLABLE BIT(2)
>
>In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>>> drivers/staging/rtl8723bs/include/ieee80211.h:446:0: warning: 
>>> "WLAN_CAPABILITY_CF_POLL_REQUEST" redefined
> #define WLAN_CAPABILITY_CF_POLL_REQUEST (1<<3)
>
>In file included from include/net/cfg80211.h:23:0,
> from 
> drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
> from drivers/staging/rtl8723bs/include/osdep_service.h:23,
> from drivers/staging/rtl8723bs/include/drv_types.h:29,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>include/linux/ieee80211.h:1604:0: note: this is the location of the 
> previous definition
> #define WLAN_CAPABILITY_CF_POLL_REQUEST BIT(3)
>
>In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>>> drivers/staging/rtl8723bs/include/ieee80211.h:447:0: warning: 
>>> "WLAN_CAPABILITY_PRIVACY" redefined
> #define WLAN_CAPABILITY_PRIVACY (1<<4)
>
>In file included from include/net/cfg80211.h:23:0,
> from 
> drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
> from drivers/staging/rtl8723bs/include/osdep_service.h:23,
> from drivers/staging/rtl8723bs/include/drv_types.h:29,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>include/linux/ieee80211.h:1605:0: note: this is the location of the 
> previous definition
> #define WLAN_CAPABILITY_PRIVACY  BIT(4)
>
>In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
> from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>>> drivers/staging/rtl8723bs/include/ieee80211.h:448:0: warning: 

Re: [PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

2018-03-25 Thread kbuild test robot
Hi Quytelda,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mac80211-next/master]
[also build test WARNING on v4.16-rc6 next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Quytelda-Kahja/ieee80211-Replace-bit-shifts-with-the-BIT-macro-for-WLAN_CAPABILITY_/20180325-211645
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git 
master
config: i386-randconfig-s1-03251817 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>> drivers/staging/rtl8723bs/include/ieee80211.h:444:0: warning: 
>> "WLAN_CAPABILITY_IBSS" redefined
#define WLAN_CAPABILITY_IBSS (1<<1)

   In file included from include/net/cfg80211.h:23:0,
from 
drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
from drivers/staging/rtl8723bs/include/osdep_service.h:23,
from drivers/staging/rtl8723bs/include/drv_types.h:29,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
   include/linux/ieee80211.h:1593:0: note: this is the location of the previous 
definition
#define WLAN_CAPABILITY_IBSS  BIT(1)

   In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>> drivers/staging/rtl8723bs/include/ieee80211.h:445:0: warning: 
>> "WLAN_CAPABILITY_CF_POLLABLE" redefined
#define WLAN_CAPABILITY_CF_POLLABLE (1<<2)

   In file included from include/net/cfg80211.h:23:0,
from 
drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
from drivers/staging/rtl8723bs/include/osdep_service.h:23,
from drivers/staging/rtl8723bs/include/drv_types.h:29,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
   include/linux/ieee80211.h:1603:0: note: this is the location of the previous 
definition
#define WLAN_CAPABILITY_CF_POLLABLE BIT(2)

   In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>> drivers/staging/rtl8723bs/include/ieee80211.h:446:0: warning: 
>> "WLAN_CAPABILITY_CF_POLL_REQUEST" redefined
#define WLAN_CAPABILITY_CF_POLL_REQUEST (1<<3)

   In file included from include/net/cfg80211.h:23:0,
from 
drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
from drivers/staging/rtl8723bs/include/osdep_service.h:23,
from drivers/staging/rtl8723bs/include/drv_types.h:29,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
   include/linux/ieee80211.h:1604:0: note: this is the location of the previous 
definition
#define WLAN_CAPABILITY_CF_POLL_REQUEST BIT(3)

   In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>> drivers/staging/rtl8723bs/include/ieee80211.h:447:0: warning: 
>> "WLAN_CAPABILITY_PRIVACY" redefined
#define WLAN_CAPABILITY_PRIVACY (1<<4)

   In file included from include/net/cfg80211.h:23:0,
from 
drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
from drivers/staging/rtl8723bs/include/osdep_service.h:23,
from drivers/staging/rtl8723bs/include/drv_types.h:29,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
   include/linux/ieee80211.h:1605:0: note: this is the location of the previous 
definition
#define WLAN_CAPABILITY_PRIVACY  BIT(4)

   In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
>> drivers/staging/rtl8723bs/include/ieee80211.h:448:0: warning: 
>> "WLAN_CAPABILITY_SHORT_PREAMBLE" redefined
#define WLAN_CAPABILITY_SHORT_PREAMBLE (1<<5)

   In file included from include/net/cfg80211.h:23:0,
from 
drivers/staging/rtl8723bs/include/osdep_service_linux.h:50,
from drivers/staging/rtl8723bs/include/osdep_service.h:23,
from drivers/staging/rtl8723bs/include/drv_types.h:29,
from drivers/staging/rtl8723bs/hal/rtl8723b_dm.c:20:
   include/linux/ieee80211.h:1606:0: note: this is the location of the previous 
definition
#define WLAN_CAPABILITY_SHORT_PREAMBLE BIT(5)

   In file included from drivers/staging/rtl8723bs/include/drv_types.h:33:0,
from 

Re: [PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

2018-03-24 Thread Quytelda Kahja
The "document" refers to the file in which the changes were made
('include/linux/ieee80211.h').

I tend to try to split my commits into the smallest logically related
changes possible, hence the three patch series.  This particular case
may be a little on the extreme side, but if the maintainer desires,
they can always squash them together or ask me to resubmit as one
patch.

On 3/24/18, Larry Finger  wrote:
> On 03/23/2018 11:10 PM, Quytelda Kahja wrote:
>> It is neater and more consistent with the rest of the document to use the
>> BIT() macro from 'linux/bitops.h' to define the WLAN_CAPABILITY_*
>> bitmasks.  In the case of WLAN_CAPABILITY_DMG_TYPE_{IBSS, PBSS, AP},
>> bitshifting integers by 0 does nothing, so there is no reason to do it in
>> the code; replace these values with plain integers.
>>
>> Signed-off-by: Quytelda Kahja 
>
> In the commit message for all of these, what is the "document" to which you
> refer?
>
> I'm not quite sure why you split these changes into 3 parts, but I guess
> that is OK.
>
> Larry
>
-- 
Thank you,
Quytelda Kahja


Re: [PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

2018-03-24 Thread Larry Finger

On 03/23/2018 11:10 PM, Quytelda Kahja wrote:

It is neater and more consistent with the rest of the document to use the
BIT() macro from 'linux/bitops.h' to define the WLAN_CAPABILITY_*
bitmasks.  In the case of WLAN_CAPABILITY_DMG_TYPE_{IBSS, PBSS, AP},
bitshifting integers by 0 does nothing, so there is no reason to do it in
the code; replace these values with plain integers.

Signed-off-by: Quytelda Kahja 


In the commit message for all of these, what is the "document" to which you 
refer?

I'm not quite sure why you split these changes into 3 parts, but I guess that 
is OK.

Larry


[PATCH 1/3] ieee80211: Replace bit shifts with the BIT() macro for WLAN_CAPABILITY_*.

2018-03-23 Thread Quytelda Kahja
It is neater and more consistent with the rest of the document to use the
BIT() macro from 'linux/bitops.h' to define the WLAN_CAPABILITY_*
bitmasks.  In the case of WLAN_CAPABILITY_DMG_TYPE_{IBSS, PBSS, AP},
bitshifting integers by 0 does nothing, so there is no reason to do it in
the code; replace these values with plain integers.

Signed-off-by: Quytelda Kahja 
---
 include/linux/ieee80211.h | 56 +++
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index ee6657a0ed69..58069176b432 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1588,8 +1588,8 @@ struct ieee80211_vht_operation {
 
 #define WLAN_AUTH_CHALLENGE_LEN 128
 
-#define WLAN_CAPABILITY_ESS(1<<0)
-#define WLAN_CAPABILITY_IBSS   (1<<1)
+#define WLAN_CAPABILITY_ESSBIT(0)
+#define WLAN_CAPABILITY_IBSS   BIT(1)
 
 /*
  * A mesh STA sets the ESS and IBSS capability bits to zero.
@@ -1599,37 +1599,37 @@ struct ieee80211_vht_operation {
 #define WLAN_CAPABILITY_IS_STA_BSS(cap)\
(!((cap) & (WLAN_CAPABILITY_ESS | WLAN_CAPABILITY_IBSS)))
 
-#define WLAN_CAPABILITY_CF_POLLABLE(1<<2)
-#define WLAN_CAPABILITY_CF_POLL_REQUEST(1<<3)
-#define WLAN_CAPABILITY_PRIVACY(1<<4)
-#define WLAN_CAPABILITY_SHORT_PREAMBLE (1<<5)
-#define WLAN_CAPABILITY_PBCC   (1<<6)
-#define WLAN_CAPABILITY_CHANNEL_AGILITY(1<<7)
+#define WLAN_CAPABILITY_CF_POLLABLEBIT(2)
+#define WLAN_CAPABILITY_CF_POLL_REQUESTBIT(3)
+#define WLAN_CAPABILITY_PRIVACYBIT(4)
+#define WLAN_CAPABILITY_SHORT_PREAMBLE BIT(5)
+#define WLAN_CAPABILITY_PBCC   BIT(6)
+#define WLAN_CAPABILITY_CHANNEL_AGILITYBIT(7)
 
 /* 802.11h */
-#define WLAN_CAPABILITY_SPECTRUM_MGMT  (1<<8)
-#define WLAN_CAPABILITY_QOS(1<<9)
-#define WLAN_CAPABILITY_SHORT_SLOT_TIME(1<<10)
-#define WLAN_CAPABILITY_APSD   (1<<11)
-#define WLAN_CAPABILITY_RADIO_MEASURE  (1<<12)
-#define WLAN_CAPABILITY_DSSS_OFDM  (1<<13)
-#define WLAN_CAPABILITY_DEL_BACK   (1<<14)
-#define WLAN_CAPABILITY_IMM_BACK   (1<<15)
+#define WLAN_CAPABILITY_SPECTRUM_MGMT  BIT(8)
+#define WLAN_CAPABILITY_QOSBIT(9)
+#define WLAN_CAPABILITY_SHORT_SLOT_TIMEBIT(10)
+#define WLAN_CAPABILITY_APSD   BIT(11)
+#define WLAN_CAPABILITY_RADIO_MEASURE  BIT(12)
+#define WLAN_CAPABILITY_DSSS_OFDM  BIT(13)
+#define WLAN_CAPABILITY_DEL_BACK   BIT(14)
+#define WLAN_CAPABILITY_IMM_BACK   BIT(15)
 
 /* DMG (60gHz) 802.11ad */
 /* type - bits 0..1 */
-#define WLAN_CAPABILITY_DMG_TYPE_MASK  (3<<0)
-#define WLAN_CAPABILITY_DMG_TYPE_IBSS  (1<<0) /* Tx by: STA */
-#define WLAN_CAPABILITY_DMG_TYPE_PBSS  (2<<0) /* Tx by: PCP */
-#define WLAN_CAPABILITY_DMG_TYPE_AP(3<<0) /* Tx by: AP */
-
-#define WLAN_CAPABILITY_DMG_CBAP_ONLY  (1<<2)
-#define WLAN_CAPABILITY_DMG_CBAP_SOURCE(1<<3)
-#define WLAN_CAPABILITY_DMG_PRIVACY(1<<4)
-#define WLAN_CAPABILITY_DMG_ECPAC  (1<<5)
-
-#define WLAN_CAPABILITY_DMG_SPECTRUM_MGMT  (1<<8)
-#define WLAN_CAPABILITY_DMG_RADIO_MEASURE  (1<<12)
+#define WLAN_CAPABILITY_DMG_TYPE_MASK   (BIT(0) | BIT(1))
+#define WLAN_CAPABILITY_DMG_TYPE_IBSS   1 /* Tx by: STA */
+#define WLAN_CAPABILITY_DMG_TYPE_PBSS   2 /* Tx by: PCP */
+#define WLAN_CAPABILITY_DMG_TYPE_AP 3 /* Tx by: AP */
+
+#define WLAN_CAPABILITY_DMG_CBAP_ONLY  BIT(2)
+#define WLAN_CAPABILITY_DMG_CBAP_SOURCEBIT(3)
+#define WLAN_CAPABILITY_DMG_PRIVACYBIT(4)
+#define WLAN_CAPABILITY_DMG_ECPAC  BIT(5)
+
+#define WLAN_CAPABILITY_DMG_SPECTRUM_MGMT  BIT(8)
+#define WLAN_CAPABILITY_DMG_RADIO_MEASURE  BIT(12)
 
 /* measurement */
 #define IEEE80211_SPCT_MSR_RPRT_MODE_LATE  (1<<0)
-- 
2.16.2