[PATCH v3] move brcm80211 drivers to mainline

[PATCH v3] move brcm80211 drivers to mainline

[Arend van Spriel]

With number of cleanup patch series merged in by Greg KH, I'd like to
once again propose moving brcm80211 out of staging and into mainline.

I've put together a patch to add a copy of the current sources from
staging-next into drivers/net/wireless/brcm80211 of the wireless-next
repository.

The patch is somewhat large, so I've posted the patch at:

http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=view&target=0001-net-wireless-add-brcm80211-drivers-v3.patch

Changes from the previous version:

V3:
- remove -D line from Makefiles
- use endian annotated structures
- enable sparse endian checking
- remove use of (static) global variables
- remove own buffer printing implementation
- remove static function prototypes
- replace macros by inline functions
- reduce sparse warnings
- remove using string-based iovars
- remove driver internal use of ioctls
- remove (un)likely
- remove uncoditional curly braces for variable scoping
- remove error messages upon alloc failures
- reduced code indentation levels
- cleanup in brcmutil module
- remove changing lock state which is acquired by other layer 
(wpa_supplicant)
- brcmfmac:
     - use ffs() instead of brcmf_find_msb()
     - replace threads with work queues
     - cleanup module parameters
- brcmsmac:
     - not modifying ssn value upon AMPDU start
     - use hweight8() instead of brcmu_bitcount()
     - remove unnecessary mac80211 callbacks
     - remove brcms_c_set_par and get_par functions
     - remove bmac wrapper functions
     - remove lock related macros
     - add debugfs based event tracing (not functional in staging)

The brcmsmac driver has been verified to work on x86 (both 32- and 
64-bit), PPC
(64-bit), SPARC, MIPS BE, and ARM.  The brcmfmac driver has been verified to
work on x86 32-bit and ARM (additional testing is in progress, but getting a
working sdio controller on some of the other platforms remains challenging).

The drivers compile cleanly for x86 (32- and 64-bit), PPC (32- and 64-bit),
SPARC, MIPS BE, MIPS LE, and ARM.

Thanks,
Arend van Spriel

Re: [PATCH v3] move brcm80211 drivers to mainline

[John W. Linville]

On Wed, Oct 05, 2011 at 04:08:47PM +0200, Arend van Spriel wrote:
> With number of cleanup patch series merged in by Greg KH, I'd like to
> once again propose moving brcm80211 out of staging and into mainline.
> 
> I've put together a patch to add a copy of the current sources from
> staging-next into drivers/net/wireless/brcm80211 of the wireless-next
> repository.
> 
> The patch is somewhat large, so I've posted the patch at:
> 
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=view&target=0001-net-wireless-add-brcm80211-drivers-v3.patch

Feel free to complain, but I plan to merge this in time for the merge
window unless someone identifies a true "showstopper" problem with
this patch in the next day or so.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH v3] move brcm80211 drivers to mainline

[Greg KH]

On Wed, Oct 05, 2011 at 10:24:03AM -0400, John W. Linville wrote:
> On Wed, Oct 05, 2011 at 04:08:47PM +0200, Arend van Spriel wrote:
> > With number of cleanup patch series merged in by Greg KH, I'd like to
> > once again propose moving brcm80211 out of staging and into mainline.
> > 
> > I've put together a patch to add a copy of the current sources from
> > staging-next into drivers/net/wireless/brcm80211 of the wireless-next
> > repository.
> > 
> > The patch is somewhat large, so I've posted the patch at:
> > 
> > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=view&target=0001-net-wireless-add-brcm80211-drivers-v3.patch
> 
> Feel free to complain, but I plan to merge this in time for the merge
> window unless someone identifies a true "showstopper" problem with
> this patch in the next day or so.

I have no objection to this, feel free to add:
	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
to the merge patch.

greg k-h

Re: [PATCH v3] move brcm80211 drivers to mainline

[Johannes Berg]

On Wed, 2011-10-05 at 16:08 +0200, Arend van Spriel wrote:
With number of cleanup patch series merged in by Greg KH, I'd like to
> once again propose moving brcm80211 out of staging and into mainline.
> 
> I've put together a patch to add a copy of the current sources from
> staging-next into drivers/net/wireless/brcm80211 of the wireless-next
> repository.
> 
> The patch is somewhat large, so I've posted the patch at:
> 
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=view&target=0001-net-wireless-add-brcm80211-drivers-v3.patch

Some comments.

I sort of understand your fascination with "generic utils" but they're
wasteful if they're in a single driver only. They should either be made
more generic (for all drivers) or dissolved, so:

 * You can get rid of BRCMS_BITSCNT and brcmu_bitcount, they're no
   longer used at all.
 * brcmu_mhz2channel isn't used anywhere either
 * neither is brcmu_chspec_ctlchan
 * brcmu_mw_to_qdbm is used only in a single file,
   it can move there to save the export etc.
 * same for brcmu_qdbm_to_mw, brcmu_chipname, brcmu_parse_tlvs,
   brcmu_chspec_malformed
 * brcmu_mkiovar is used only in fmac, so you can move it there
   to save the export
 * brcmu_format_flags is used only in smac, so same
 * same for brcmu_format_flags

After all the above, bcmutils are left with only the pktq and pktbuf
stuff, which hopefully will be either more generic or dissolved at some
point in the future since it should really just be a few skb queues.

Now the drivers :-)

FMAC:
 * A whole bunch of things in dhd.h still seem to lack endian
   annotations, which I wouldn't be too worried about, if it didn't
   also seem to lack conversions. For example brcmf_pkt_filter_enable,
   brcmf_pkt_filter, brcmf_pkt_filter_pattern.
 * These, along with others like brcmf_bss_info, also seem to lack
   __packed annotations. I thought you fixed all this, what am I
   missing?
 * bss_info[1]; and similar should just be [] I think?
 * some very odd indentation on line 1172 of wl_cfg80211.c

SMAC:
 * do you really want to be 40MHz intolerant all the time? not
   supporting 40 MHz is one thing, but being intolerant?
 * brcms_ops_stop doesn't seem right -- this should really power down
   the device, this shouldn't be done per interface since the monitor
   case will want to RX/TX even when no interface is there; also, you'll
   want multi-vif support soon :-)
 * locking tx() is inefficient, you should be able to go with a lock per
   HW queue
 * brcms_ops_sta_add is strange -- it shouldn't be modifying the station
   parameters, why would it do that? seems like a bug to me, especially
   always assuming the peer can do greenfield etc. This data should
   already be set by mac80211, if not that's a bug, not something to
   "fix" in the driver
 * I still think things like brcms_msleep() are guaranteed to create
   subtle locking bugs that are almost impossible to find since it drops
   the spinlock and if somebody calls the function somewhere they won't
   necessarily keep that in mind. There are a whole bunch of functions
   behaving like that.


Anyway, the code is now readable to me, I see no glaring mac80211 or
cfg80211 interfacing errors, so I guess you can work the rest in
mainline.

johannes

RE: [PATCH v3] move brcm80211 drivers to mainline

[Arend Van Spriel]

PiBGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWlsdG86am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldF0N
Cj4gU2VudDogd29lbnNkYWcgNSBva3RvYmVyIDIwMTEgMTY6NTcNCj4gDQo+IFNvbWUgY29tbWVu
dHMuDQo+IA0KPiBJIHNvcnQgb2YgdW5kZXJzdGFuZCB5b3VyIGZhc2NpbmF0aW9uIHdpdGggImdl
bmVyaWMgdXRpbHMiIGJ1dCB0aGV5J3JlDQo+IHdhc3RlZnVsIGlmIHRoZXkncmUgaW4gYSBzaW5n
bGUgZHJpdmVyIG9ubHkuIFRoZXkgc2hvdWxkIGVpdGhlciBiZSBtYWRlDQo+IG1vcmUgZ2VuZXJp
YyAoZm9yIGFsbCBkcml2ZXJzKSBvciBkaXNzb2x2ZWQsIHNvOg0KPiANCj4gICogWW91IGNhbiBn
ZXQgcmlkIG9mIEJSQ01TX0JJVFNDTlQgYW5kIGJyY211X2JpdGNvdW50LCB0aGV5J3JlIG5vDQo+
ICAgIGxvbmdlciB1c2VkIGF0IGFsbC4NCj4gICogYnJjbXVfbWh6MmNoYW5uZWwgaXNuJ3QgdXNl
ZCBhbnl3aGVyZSBlaXRoZXINCj4gICogbmVpdGhlciBpcyBicmNtdV9jaHNwZWNfY3RsY2hhbg0K
PiAgKiBicmNtdV9td190b19xZGJtIGlzIHVzZWQgb25seSBpbiBhIHNpbmdsZSBmaWxlLA0KPiAg
ICBpdCBjYW4gbW92ZSB0aGVyZSB0byBzYXZlIHRoZSBleHBvcnQgZXRjLg0KPiAgKiBzYW1lIGZv
ciBicmNtdV9xZGJtX3RvX213LCBicmNtdV9jaGlwbmFtZSwgYnJjbXVfcGFyc2VfdGx2cywNCj4g
ICAgYnJjbXVfY2hzcGVjX21hbGZvcm1lZA0KPiAgKiBicmNtdV9ta2lvdmFyIGlzIHVzZWQgb25s
eSBpbiBmbWFjLCBzbyB5b3UgY2FuIG1vdmUgaXQgdGhlcmUNCj4gICAgdG8gc2F2ZSB0aGUgZXhw
b3J0DQo+ICAqIGJyY211X2Zvcm1hdF9mbGFncyBpcyB1c2VkIG9ubHkgaW4gc21hYywgc28gc2Ft
ZQ0KPiAgKiBzYW1lIGZvciBicmNtdV9mb3JtYXRfZmxhZ3MNCj4gDQo+IEFmdGVyIGFsbCB0aGUg
YWJvdmUsIGJjbXV0aWxzIGFyZSBsZWZ0IHdpdGggb25seSB0aGUgcGt0cSBhbmQgcGt0YnVmDQo+
IHN0dWZmLCB3aGljaCBob3BlZnVsbHkgd2lsbCBiZSBlaXRoZXIgbW9yZSBnZW5lcmljIG9yIGRp
c3NvbHZlZCBhdCBzb21lDQo+IHBvaW50IGluIHRoZSBmdXR1cmUgc2luY2UgaXQgc2hvdWxkIHJl
YWxseSBqdXN0IGJlIGEgZmV3IHNrYiBxdWV1ZXMuDQoNCkFncmVlLiBUaGVyZSBpcyBvcHBvcnR1
bml0eSB0byBjbGVhbnVwIG9uIHRoZSBicmNtdXRpbCBtb2R1bGUgb3INCmV2ZW4gZ2V0IHJpZCBv
ZiBpdC4NCg0KPiBOb3cgdGhlIGRyaXZlcnMgOi0pDQo+IA0KPiBGTUFDOg0KPiAgKiBBIHdob2xl
IGJ1bmNoIG9mIHRoaW5ncyBpbiBkaGQuaCBzdGlsbCBzZWVtIHRvIGxhY2sgZW5kaWFuDQo+ICAg
IGFubm90YXRpb25zLCB3aGljaCBJIHdvdWxkbid0IGJlIHRvbyB3b3JyaWVkIGFib3V0LCBpZiBp
dCBkaWRuJ3QNCj4gICAgYWxzbyBzZWVtIHRvIGxhY2sgY29udmVyc2lvbnMuIEZvciBleGFtcGxl
IGJyY21mX3BrdF9maWx0ZXJfZW5hYmxlLA0KPiAgICBicmNtZl9wa3RfZmlsdGVyLCBicmNtZl9w
a3RfZmlsdGVyX3BhdHRlcm4uDQo+ICAqIFRoZXNlLCBhbG9uZyB3aXRoIG90aGVycyBsaWtlIGJy
Y21mX2Jzc19pbmZvLCBhbHNvIHNlZW0gdG8gbGFjaw0KPiAgICBfX3BhY2tlZCBhbm5vdGF0aW9u
cy4gSSB0aG91Z2h0IHlvdSBmaXhlZCBhbGwgdGhpcywgd2hhdCBhbSBJDQo+ICAgIG1pc3Npbmc/
DQo+ICAqIGJzc19pbmZvWzFdOyBhbmQgc2ltaWxhciBzaG91bGQganVzdCBiZSBbXSBJIHRoaW5r
Pw0KPiAgKiBzb21lIHZlcnkgb2RkIGluZGVudGF0aW9uIG9uIGxpbmUgMTE3MiBvZiB3bF9jZmc4
MDIxMS5jDQo+IA0KPiBTTUFDOg0KPiAgKiBkbyB5b3UgcmVhbGx5IHdhbnQgdG8gYmUgNDBNSHog
aW50b2xlcmFudCBhbGwgdGhlIHRpbWU/IG5vdA0KPiAgICBzdXBwb3J0aW5nIDQwIE1IeiBpcyBv
bmUgdGhpbmcsIGJ1dCBiZWluZyBpbnRvbGVyYW50Pw0KPiAgKiBicmNtc19vcHNfc3RvcCBkb2Vz
bid0IHNlZW0gcmlnaHQgLS0gdGhpcyBzaG91bGQgcmVhbGx5IHBvd2VyIGRvd24NCj4gICAgdGhl
IGRldmljZSwgdGhpcyBzaG91bGRuJ3QgYmUgZG9uZSBwZXIgaW50ZXJmYWNlIHNpbmNlIHRoZSBt
b25pdG9yDQo+ICAgIGNhc2Ugd2lsbCB3YW50IHRvIFJYL1RYIGV2ZW4gd2hlbiBubyBpbnRlcmZh
Y2UgaXMgdGhlcmU7IGFsc28sDQo+IHlvdSdsbA0KPiAgICB3YW50IG11bHRpLXZpZiBzdXBwb3J0
IHNvb24gOi0pDQoNCkFncmVlLiBXZSBub3RpY2VkIHJlYWRpbmcgbWFjODAyMTEuaCB3aXRoIHN0
YXRlbWVudCBhYm91dCBhZGRfaW50ZXJmYWNlDQpjYWxsYmFjayBub3QgYmVpbmcgdXNlZCBpbiBt
b25pdG9yIG1vZGUuDQoNCj4gICogbG9ja2luZyB0eCgpIGlzIGluZWZmaWNpZW50LCB5b3Ugc2hv
dWxkIGJlIGFibGUgdG8gZ28gd2l0aCBhIGxvY2sNCj4gcGVyDQo+ICAgIEhXIHF1ZXVlDQo+ICAq
IGJyY21zX29wc19zdGFfYWRkIGlzIHN0cmFuZ2UgLS0gaXQgc2hvdWxkbid0IGJlIG1vZGlmeWlu
ZyB0aGUNCj4gc3RhdGlvbg0KPiAgICBwYXJhbWV0ZXJzLCB3aHkgd291bGQgaXQgZG8gdGhhdD8g
c2VlbXMgbGlrZSBhIGJ1ZyB0byBtZSwgZXNwZWNpYWxseQ0KPiAgICBhbHdheXMgYXNzdW1pbmcg
dGhlIHBlZXIgY2FuIGRvIGdyZWVuZmllbGQgZXRjLiBUaGlzIGRhdGEgc2hvdWxkDQo+ICAgIGFs
cmVhZHkgYmUgc2V0IGJ5IG1hYzgwMjExLCBpZiBub3QgdGhhdCdzIGEgYnVnLCBub3Qgc29tZXRo
aW5nIHRvDQo+ICAgICJmaXgiIGluIHRoZSBkcml2ZXINCj4gICogSSBzdGlsbCB0aGluayB0aGlu
Z3MgbGlrZSBicmNtc19tc2xlZXAoKSBhcmUgZ3VhcmFudGVlZCB0byBjcmVhdGUNCj4gICAgc3Vi
dGxlIGxvY2tpbmcgYnVncyB0aGF0IGFyZSBhbG1vc3QgaW1wb3NzaWJsZSB0byBmaW5kIHNpbmNl
IGl0DQo+IGRyb3BzDQo+ICAgIHRoZSBzcGlubG9jayBhbmQgaWYgc29tZWJvZHkgY2FsbHMgdGhl
IGZ1bmN0aW9uIHNvbWV3aGVyZSB0aGV5IHdvbid0DQo+ICAgIG5lY2Vzc2FyaWx5IGtlZXAgdGhh
dCBpbiBtaW5kLiBUaGVyZSBhcmUgYSB3aG9sZSBidW5jaCBvZiBmdW5jdGlvbnMNCj4gICAgYmVo
YXZpbmcgbGlrZSB0aGF0Lg0KDQpBZ3JlZS4gVGhlIGxvY2tpbmcgc3RyYXRlZ3kgaW4gdGhlIGRy
aXZlciB3b3JrcyBhbmQgaGFzIGJlZW4gcHJvdmVuDQp0byB3b3JrIGZvciB0aGUgY3VycmVudCBj
b2RlIGJhc2UgaW4gbWFueSB0ZXN0IGN5Y2xlcywgYnV0IGl0IHBvc2VzDQphIHJpc2sgZm9yIHVw
Y29taW5nIGZlYXR1cmUgZGV2ZWxvcG1lbnQuDQoNCj4gDQo+IEFueXdheSwgdGhlIGNvZGUgaXMg
bm93IHJlYWRhYmxlIHRvIG1lLCBJIHNlZSBubyBnbGFyaW5nIG1hYzgwMjExIG9yDQo+IGNmZzgw
MjExIGludGVyZmFjaW5nIGVycm9ycywgc28gSSBndWVzcyB5b3UgY2FuIHdvcmsgdGhlIHJlc3Qg
aW4NCj4gbWFpbmxpbmUuDQo+IA0KPiBqb2hhbm5lcw0KPiANCg0KVGhhbmtzIGZvciB0aGUgZmVl
ZGJhY2suIFRoZXNlIGFyZSBhbGwgdmFsaWQgcG9pbnRzIGFuZCBhcyBzdWNoIGRlc2VydmUNCm91
ciBhdHRlbnRpb24uDQoNCkFyZW5kDQo=

Re: [PATCH v3] move brcm80211 drivers to mainline

[Hauke Mehrtens]

On 10/05/2011 04:08 PM, Arend van Spriel wrote:
> With number of cleanup patch series merged in by Greg KH, I'd like to
> once again propose moving brcm80211 out of staging and into mainline.
> 
> I've put together a patch to add a copy of the current sources from
> staging-next into drivers/net/wireless/brcm80211 of the wireless-next
> repository.
> 
> The patch is somewhat large, so I've posted the patch at:
> 
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=view&target=0001-net-wireless-add-brcm80211-drivers-v3.patch
> 
> 
> Changes from the previous version:
> 
> V3:
> - remove -D line from Makefiles
> - use endian annotated structures
> - enable sparse endian checking
> - remove use of (static) global variables
> - remove own buffer printing implementation
> - remove static function prototypes
> - replace macros by inline functions
> - reduce sparse warnings
> - remove using string-based iovars
> - remove driver internal use of ioctls
> - remove (un)likely
> - remove uncoditional curly braces for variable scoping
> - remove error messages upon alloc failures
> - reduced code indentation levels
> - cleanup in brcmutil module
> - remove changing lock state which is acquired by other layer
> (wpa_supplicant)
> - brcmfmac:
>     - use ffs() instead of brcmf_find_msb()
>     - replace threads with work queues
>     - cleanup module parameters
> - brcmsmac:
>     - not modifying ssn value upon AMPDU start
>     - use hweight8() instead of brcmu_bitcount()
>     - remove unnecessary mac80211 callbacks
>     - remove brcms_c_set_par and get_par functions
>     - remove bmac wrapper functions
>     - remove lock related macros
>     - add debugfs based event tracing (not functional in staging)
> 
> The brcmsmac driver has been verified to work on x86 (both 32- and
> 64-bit), PPC
> (64-bit), SPARC, MIPS BE, and ARM.  The brcmfmac driver has been
> verified to
> work on x86 32-bit and ARM (additional testing is in progress, but
> getting a
> working sdio controller on some of the other platforms remains
> challenging).
> 
> The drivers compile cleanly for x86 (32- and 64-bit), PPC (32- and 64-bit),
> SPARC, MIPS BE, MIPS LE, and ARM.
> 
> Thanks,
> Arend van Spriel
> 
Hi Arend,

Most of the things defined in
drivers/net/wireless/brcm80211/include/soc.h are also defined in
drivers/net/wireless/brcm80211/brcmsmac/aiutils.h.

Hauke

RE: [PATCH v3] move brcm80211 drivers to mainline

[Arend Van Spriel]

> From: Hauke Mehrtens [mailto:hauke@hauke-m.de]
> Sent: woensdag 5 oktober 2011 17:07
> 
> Hi Arend,
> 
> Most of the things defined in
> drivers/net/wireless/brcm80211/include/soc.h are also defined in
> drivers/net/wireless/brcm80211/brcmsmac/aiutils.h.
> 
> Hauke

Thanks, Hauke

One of the reasons we wanted to move to mainline is that bcma
is worked on from John's repo and we want our brcmsmac driver
to use that. The aiutils.h file is going away when we are
using bcma.

Gr. AvS

Re: [PATCH v3] move brcm80211 drivers to mainline

[Larry Finger]

On 10/05/2011 10:40 AM, Arend Van Spriel wrote:
>> From: Hauke Mehrtens [mailto:hauke@hauke-m.de]
>> Sent: woensdag 5 oktober 2011 17:07
>>
>> Hi Arend,
>>
>> Most of the things defined in
>> drivers/net/wireless/brcm80211/include/soc.h are also defined in
>> drivers/net/wireless/brcm80211/brcmsmac/aiutils.h.
>>
>> Hauke
>
> Thanks, Hauke
>
> One of the reasons we wanted to move to mainline is that bcma
> is worked on from John's repo and we want our brcmsmac driver
> to use that. The aiutils.h file is going away when we are
> using bcma.

Good to hear that patches to switch to bcma will be coming soon.

Larry

RE: [PATCH v3] move brcm80211 drivers to mainline

[Arend Van Spriel]

> From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry
> Finger
> Sent: woensdag 5 oktober 2011 17:45
> 
> On 10/05/2011 10:40 AM, Arend Van Spriel wrote:
> >> From: Hauke Mehrtens [mailto:hauke@hauke-m.de]
> >> Sent: woensdag 5 oktober 2011 17:07
> >>
> >> Hi Arend,
> >>
> >> Most of the things defined in
> >> drivers/net/wireless/brcm80211/include/soc.h are also defined in
> >> drivers/net/wireless/brcm80211/brcmsmac/aiutils.h.
> >>
> >> Hauke
> >
> > Thanks, Hauke
> >
> > One of the reasons we wanted to move to mainline is that bcma
> > is worked on from John's repo and we want our brcmsmac driver
> > to use that. The aiutils.h file is going away when we are
> > using bcma.
> 
> Good to hear that patches to switch to bcma will be coming soon.
> 
> Larry
> 

Hi Larry,

Reading between the lines? ;-) It will not be there yesterday, but
we will definitely make an effort to make that switch. Work was
started on it but other cleanup was interfering so we decided
to focus on the cleanup.

Gr. AvS