With the latest series of cleanup patches merged in by Greg KH, I'd like to now
propose moving brcm80211 out of staging and into mainline.
Since brcm80211 in staging-next has about ~250 patches that areen't in
wireless-testing yet, I've put together a patch to add a copy of the current
sources from staging-next into wireless-testing:drivers/net/wireless/brcm80211.
The patch is somewhat large, so I've posted the patch at:
http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
Only the necessary Kconfig and Makefile adjustments have been made (the sources
are unchanged from what's currently in staging-next).
- Henry
Hi,
On 7 July 2011 16:46, Henry Ptasinski <[email protected]> wrote:
> We're looking at BE MIPS, and I sent some cards to Benjamin Herrenschmidt so he
> can test on PPC platforms.
BE MIPS currently doesn't compile (at least for me); the R_REG/W_REG
macros try to use ^ with pointers[1], which my gcc flags as errors.
Adding a cast to unsigned long silences the error, but I didn't yet
confirm whether this is the correct way of fixing it, and if it
actually works for 32 and 64 bit.
I would send a patch, but it might take some days until I come around
to creating/testing it - I don't mind if you fix it yourself.
Hopefully I can then even give a short status report how it works on
BE MIPS, after I finally got the PCIe working on my bcm6328 with a
bcm4313 ;-).
Jonas
[1] <http://git.kernel.org/?p=linux/kernel/git/gregkh/staging-2.6.git;a=blob;f=drivers/staging/brcm80211/brcmsmac/types.h;h=bbf21897ae0e32137c7f38bbabf5159a31b4cda7;hb=refs/heads/staging-next#l307>
and below
On Thu, Jul 07, 2011 at 02:55:37PM -0700, Henry Ptasinski wrote:
> Building some cross-compiler toolchains now. SPARC hardware to do anything
> native with seems to be getting very scarce, and trying to fit a PCIe minicard
> into one might be challenging ...
That doesn't matter. The point is that the BE code is obviously wrong
and can't even compile. Your code should be arch-neutral as it's just a
driver.
greg k-h
On 07/06/2011 08:20 PM, Henry Ptasinski wrote:
> With the latest series of cleanup patches merged in by Greg KH, I'd like to now
> propose moving brcm80211 out of staging and into mainline.
>
> Since brcm80211 in staging-next has about ~250 patches that areen't in
> wireless-testing yet, I've put together a patch to add a copy of the current
> sources from staging-next into wireless-testing:drivers/net/wireless/brcm80211.
>
> The patch is somewhat large, so I've posted the patch at:
>
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
>
> Only the necessary Kconfig and Makefile adjustments have been made (the sources
> are unchanged from what's currently in staging-next).
The change to Kconfig seems to be wrong. It would be better to respect
CONFIG_WLAN in drivers/net/wireless/Kconfig, like all other drivers do.
Then drivers/net/wireless/brcm80211/Kconfig should be cleaned up to
remove unneeded dependencies on CONFIG_WLAN.
--
Regards,
Pavel Roskin
On Wed, Jul 06, 2011 at 05:45:43PM -0700, Pavel Roskin wrote:
> On 07/06/2011 08:20 PM, Henry Ptasinski wrote:
> > With the latest series of cleanup patches merged in by Greg KH, I'd like to now
> > propose moving brcm80211 out of staging and into mainline.
> >
> > Since brcm80211 in staging-next has about ~250 patches that areen't in
> > wireless-testing yet, I've put together a patch to add a copy of the current
> > sources from staging-next into wireless-testing:drivers/net/wireless/brcm80211.
> >
> > The patch is somewhat large, so I've posted the patch at:
> >
> > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
> >
> > Only the necessary Kconfig and Makefile adjustments have been made (the sources
> > are unchanged from what's currently in staging-next).
>
> The change to Kconfig seems to be wrong. It would be better to respect
> CONFIG_WLAN in drivers/net/wireless/Kconfig, like all other drivers do.
> Then drivers/net/wireless/brcm80211/Kconfig should be cleaned up to
> remove unneeded dependencies on CONFIG_WLAN.
Definitely. Thanks for pointing that out.
- Henry
On Wed, Jul 06, 2011 at 08:58:11PM -0400, Pavel Roskin wrote:
> On 07/06/2011 08:40 PM, Rafał Miłecki wrote:
> >Short question, without commenting on brcm80211 code yet:
> >
> >Why should we want 2 mainline drivers for the same hardware?
>
> Also, checkpatch.pl finds a lot of bad stuff:
>
> total: 1 errors, 1505 warnings, 105454 lines checked
>
> $ scripts/checkpatch.pl 0001-wireless-testing-add-brcm80211.patch >Log
> $ egrep '^(WARNING|ERROR)' Log |sort |uniq -c
> 1 ERROR: do not use assignment in if condition
> 76 WARNING: braces {} are not necessary for any arm of this statement
> 205 WARNING: braces {} are not necessary for single statement blocks
> 4 WARNING: consider using a completion
> 11 WARNING: consider using kstrto* in preference to simple_strtoul
> 20 WARNING: do not add new typedefs
> 5 WARNING: externs should be avoided in .c files
> 1164 WARNING: line over 80 characters
> 20 WARNING: Use of volatile is usually wrong: see
Almost all of these should be fixed, especially the volatile one, that's
just horrible.
Also, there's the whole big endian mess that needs to be fixed up before
the driver can be moved out of staging. Henry, what's the status of
that?
greg k-h
On Wed, Jul 06, 2011 at 06:45:12PM -0700, Greg KH wrote:
> On Wed, Jul 06, 2011 at 08:58:11PM -0400, Pavel Roskin wrote:
> > On 07/06/2011 08:40 PM, Rafał Miłecki wrote:
> > >Short question, without commenting on brcm80211 code yet:
> > >
> > >Why should we want 2 mainline drivers for the same hardware?
> >
> > Also, checkpatch.pl finds a lot of bad stuff:
> >
> > total: 1 errors, 1505 warnings, 105454 lines checked
> >
> > $ scripts/checkpatch.pl 0001-wireless-testing-add-brcm80211.patch >Log
> > $ egrep '^(WARNING|ERROR)' Log |sort |uniq -c
> > 1 ERROR: do not use assignment in if condition
> > 76 WARNING: braces {} are not necessary for any arm of this statement
> > 205 WARNING: braces {} are not necessary for single statement blocks
> > 4 WARNING: consider using a completion
> > 11 WARNING: consider using kstrto* in preference to simple_strtoul
> > 20 WARNING: do not add new typedefs
> > 5 WARNING: externs should be avoided in .c files
> > 1164 WARNING: line over 80 characters
> > 20 WARNING: Use of volatile is usually wrong: see
>
> Almost all of these should be fixed, especially the volatile one, that's
> just horrible.
Fixes for these are in progress. We'll keep posting patches to address this
cleanup in staging.
> Also, there's the whole big endian mess that needs to be fixed up before
> the driver can be moved out of staging. Henry, what's the status of
> that?
We're looking at BE MIPS, and I sent some cards to Benjamin Herrenschmidt so he
can test on PPC platforms.
- Henry
On 07/06/2011 08:40 PM, Rafał Miłecki wrote:
> Short question, without commenting on brcm80211 code yet:
>
> Why should we want 2 mainline drivers for the same hardware?
Also, checkpatch.pl finds a lot of bad stuff:
total: 1 errors, 1505 warnings, 105454 lines checked
$ scripts/checkpatch.pl 0001-wireless-testing-add-brcm80211.patch >Log
$ egrep '^(WARNING|ERROR)' Log |sort |uniq -c
1 ERROR: do not use assignment in if condition
76 WARNING: braces {} are not necessary for any arm of this statement
205 WARNING: braces {} are not necessary for single statement blocks
4 WARNING: consider using a completion
11 WARNING: consider using kstrto* in preference to simple_strtoul
20 WARNING: do not add new typedefs
5 WARNING: externs should be avoided in .c files
1164 WARNING: line over 80 characters
20 WARNING: Use of volatile is usually wrong: see
Documentation/volatile-considered-harmful.txt
That's the "error". It should be easy to fix.
ERROR: do not use assignment in if condition
#35077: FILE: drivers/net/wireless/brcm80211/brcmsmac/main.c:261:
+ if ((cfg = (wlc)->bsscfg[idx]))
20 typedefs seems excessive to me.
--
Regards,
Pavel Roskin
On Thu, Jul 07, 2011 at 07:58:22AM -0700, Greg KH wrote:
> On Thu, Jul 07, 2011 at 07:46:28AM -0700, Henry Ptasinski wrote:
> > On Wed, Jul 06, 2011 at 06:45:12PM -0700, Greg KH wrote:
> > > On Wed, Jul 06, 2011 at 08:58:11PM -0400, Pavel Roskin wrote:
> > > > On 07/06/2011 08:40 PM, Rafał Miłecki wrote:
> > > > >Short question, without commenting on brcm80211 code yet:
> > > > >
> > > > >Why should we want 2 mainline drivers for the same hardware?
> > > >
> > > > Also, checkpatch.pl finds a lot of bad stuff:
> > > >
> > > > total: 1 errors, 1505 warnings, 105454 lines checked
> > > >
> > > > $ scripts/checkpatch.pl 0001-wireless-testing-add-brcm80211.patch >Log
> > > > $ egrep '^(WARNING|ERROR)' Log |sort |uniq -c
> > > > 1 ERROR: do not use assignment in if condition
> > > > 76 WARNING: braces {} are not necessary for any arm of this statement
> > > > 205 WARNING: braces {} are not necessary for single statement blocks
> > > > 4 WARNING: consider using a completion
> > > > 11 WARNING: consider using kstrto* in preference to simple_strtoul
> > > > 20 WARNING: do not add new typedefs
> > > > 5 WARNING: externs should be avoided in .c files
> > > > 1164 WARNING: line over 80 characters
> > > > 20 WARNING: Use of volatile is usually wrong: see
> > >
> > > Almost all of these should be fixed, especially the volatile one, that's
> > > just horrible.
> >
> > Fixes for these are in progress. We'll keep posting patches to address this
> > cleanup in staging.
>
> Ok, at the least these should be resolved before ever asking for the
> code to be moved out of staging. Sorry for me not checking this
> previously, I wrongly assumed that you had finished this task already.
No worries. We'll have almost all of these cleaned up shortly. The 80 char
lines will be nice and tedious, though, unless somebody can suggest a decent
tool that doesn't make the code unreadable.
>
> > > Also, there's the whole big endian mess that needs to be fixed up before
> > > the driver can be moved out of staging. Henry, what's the status of
> > > that?
> >
> > We're looking at BE MIPS, and I sent some cards to Benjamin Herrenschmidt so he
> > can test on PPC platforms.
>
> Don't forget just building the thing on other arches (SPARC for example)
> and what happens when you try to do that...
Building some cross-compiler toolchains now. SPARC hardware to do anything
native with seems to be getting very scarce, and trying to fit a PCIe minicard
into one might be challenging ...
- Henry
On Thu, Jul 07, 2011 at 07:46:28AM -0700, Henry Ptasinski wrote:
> On Wed, Jul 06, 2011 at 06:45:12PM -0700, Greg KH wrote:
> > On Wed, Jul 06, 2011 at 08:58:11PM -0400, Pavel Roskin wrote:
> > > On 07/06/2011 08:40 PM, Rafał Miłecki wrote:
> > > >Short question, without commenting on brcm80211 code yet:
> > > >
> > > >Why should we want 2 mainline drivers for the same hardware?
> > >
> > > Also, checkpatch.pl finds a lot of bad stuff:
> > >
> > > total: 1 errors, 1505 warnings, 105454 lines checked
> > >
> > > $ scripts/checkpatch.pl 0001-wireless-testing-add-brcm80211.patch >Log
> > > $ egrep '^(WARNING|ERROR)' Log |sort |uniq -c
> > > 1 ERROR: do not use assignment in if condition
> > > 76 WARNING: braces {} are not necessary for any arm of this statement
> > > 205 WARNING: braces {} are not necessary for single statement blocks
> > > 4 WARNING: consider using a completion
> > > 11 WARNING: consider using kstrto* in preference to simple_strtoul
> > > 20 WARNING: do not add new typedefs
> > > 5 WARNING: externs should be avoided in .c files
> > > 1164 WARNING: line over 80 characters
> > > 20 WARNING: Use of volatile is usually wrong: see
> >
> > Almost all of these should be fixed, especially the volatile one, that's
> > just horrible.
>
> Fixes for these are in progress. We'll keep posting patches to address this
> cleanup in staging.
Ok, at the least these should be resolved before ever asking for the
code to be moved out of staging. Sorry for me not checking this
previously, I wrongly assumed that you had finished this task already.
> > Also, there's the whole big endian mess that needs to be fixed up before
> > the driver can be moved out of staging. Henry, what's the status of
> > that?
>
> We're looking at BE MIPS, and I sent some cards to Benjamin Herrenschmidt so he
> can test on PPC platforms.
Don't forget just building the thing on other arches (SPARC for example)
and what happens when you try to do that...
thanks,
greg k-h
2011/7/7 Henry Ptasinski <[email protected]>:
> With the latest series of cleanup patches merged in by Greg KH, I'd like to now
> propose moving brcm80211 out of staging and into mainline.
>
> Since brcm80211 in staging-next has about ~250 patches that areen't in
> wireless-testing yet, I've put together a patch to add a copy of the current
> sources from staging-next into wireless-testing:drivers/net/wireless/brcm80211.
>
> The patch is somewhat large, so I've posted the patch at:
>
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
>
> Only the necessary Kconfig and Makefile adjustments have been made (the sources
> are unchanged from what's currently in staging-next).
Short question, without commenting on brcm80211 code yet:
Why should we want 2 mainline drivers for the same hardware?
--
Rafał
On Wed, Jul 06, 2011 at 05:40:13PM -0700, Rafał Miłecki wrote:
> 2011/7/7 Henry Ptasinski <[email protected]>:
> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
> > now propose moving brcm80211 out of staging and into mainline.
> >
> > Since brcm80211 in staging-next has about ~250 patches that areen't in
> > wireless-testing yet, I've put together a patch to add a copy of the
> > current sources from staging-next into
> > wireless-testing:drivers/net/wireless/brcm80211.
> >
> > The patch is somewhat large, so I've posted the patch at:
> >
> >
> > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
> >
> > Only the necessary Kconfig and Makefile adjustments have been made (the
> > sources are unchanged from what's currently in staging-next).
>
> Short question, without commenting on brcm80211 code yet:
>
> Why should we want 2 mainline drivers for the same hardware?
Rafał,
The brcm80211 tree provides two device drivers:
The brcmsmac driver supports the BCM4313, BCM43224, and BCM43225 chipsets with
full performance (e.g. rate/range comparable to Windows drivers), does not have
any known system stability issues, is fully supported by Broadcom, and provides
an API for addition of newer chipsets that will help speed up the process of
getting support for those chipsets into the kernel.
The brcmfmac driver supports the BCM4329 chipset with full performance, does
not have any known system stability issues, is fully supported by Broadcom, and
provides the framework for the addition of other Broadcom embedded WLAN
chipsets.
There currently isn't any driver in mainline that provides all this, so I'm
proposing that we use the brcm80211 tree to get all this functionality.
- Henry
On 07/07/2011 05:55 PM, Henry Ptasinski wrote:
> Building some cross-compiler toolchains now. SPARC hardware to do anything
> native with seems to be getting very scarce, and trying to fit a PCIe minicard
> into one might be challenging ...
Those two things combined together should solve your challenge:
http://www.google.com/#q=PCI1PEX
http://www.google.com/#q=MP2W
--
Regards,
Pavel Roskin
On Sat, 2011-08-27 at 15:18 +0200, Michael Büsch wrote:
> So, at the end of the day, I do understand the concerns. However, I would
> also like to see broadcom's QA move towards b43 and start some testing
> on it.
If we could at least get the b43 and brcmsmac drivers using the same PHY
code, that would be a big step in the right direction.
I'd like to see just the brcmsmac PHY code moved out of staging and b43
made to use it.
--
dwmw2
2011/8/27 Greg KH <[email protected]>:
> On Sat, Aug 27, 2011 at 05:35:13PM +0300, Dan Carpenter wrote:
>> On Thu, Aug 25, 2011 at 10:55:26PM +0200, Rafał Miłecki wrote:
>> > 2011/8/25 Henry Ptasinski <[email protected]>:
>> > > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> > > once again propose moving brcm80211 out of staging and into mainline.
>> >
>> > Henry: a simple question, please explain it to me, what brcmsmac does
>> > provide that b43 doesn't?
>> >
>>
>> Wow. Why are we only having this discussion now? Somewhere along
>> the line, there has been a massive communications failure. What
>> happened here? Henry, did you know about the b43 driver? Can
>> someone explain what's going on?
>
> I always thought that b43 and the staging driver supported different
> devices and had no overlap, which is why I had no problem with it.
>
> Was I totally mistaken and got this wrong?
Please see my last e-mail in this thread for history overview.
Right now the only card that is supported by brcmsmac and is not
supported by b43 is 14e4:4727. That card is based on PHY type LCN, and
I'm working on adding support for it in b43.
Other cards supported by brcmsmac (14e4:4353, 14e4:4357) are supported
by b43. They still need some work (no HT support, but I'm not sure if
even brcmsmac has support for it), but are usable and seem to work
stable. Plus b43 supports some feature like monitor mode, etc. on that
cards.
I've also crated comparison of driver few days ago:
http://wireless.kernel.org/en/users/Drivers/b43#Comparison_of_recent_drivers
It doesn't cover cards by pci ids, but gives some overview.
--
Rafał
On Wed, Aug 24, 2011 at 03:28:01PM -0700, Henry Ptasinski wrote:
> diff --git a/drivers/staging/brcm80211/brcmsmac/dma.c b/drivers/staging/brcm8021
> index 05dad9f..73e5841 100644
> --- a/drivers/staging/brcm80211/brcmsmac/dma.c
> +++ b/drivers/staging/brcm80211/brcmsmac/dma.c
> @@ -815,7 +815,7 @@ struct sk_buff *dma_rx(struct dma_pub *pub)
> tail = head;
> while ((resid > 0) && (p = _dma_getnextrxp(di, false))) {
> tail->next = p;
> - pkt_len = min(resid, (int)di->rxbufsize);
> + pkt_len = min_t(int, resid, (int)di->rxbufsize);
This isn't right. It should be:
pkt_len = min_t(uint, resid, di->rxbufsize);
Casting it to int would mean that high values of ->rxbufsize would be
treated as lower than "resid".
> __skb_trim(p, pkt_len);
>
> tail = p;
regards,
dan carpenter
On Sat, Aug 27, 2011 at 05:35:13PM +0300, Dan Carpenter wrote:
> On Thu, Aug 25, 2011 at 10:55:26PM +0200, Rafał Miłecki wrote:
> > 2011/8/25 Henry Ptasinski <[email protected]>:
> > > With the latest series of cleanup patches merged in by Greg KH, I'd like to
> > > once again propose moving brcm80211 out of staging and into mainline.
> >
> > Henry: a simple question, please explain it to me, what brcmsmac does
> > provide that b43 doesn't?
> >
>
> Wow. Why are we only having this discussion now? Somewhere along
> the line, there has been a massive communications failure. What
> happened here? Henry, did you know about the b43 driver? Can
> someone explain what's going on?
I always thought that b43 and the staging driver supported different
devices and had no overlap, which is why I had no problem with it.
Was I totally mistaken and got this wrong?
greg k-h
2011/8/25 Henry Ptasinski <[email protected]>:
> With the latest series of cleanup patches merged in by Greg KH, I'd like to
> once again propose moving brcm80211 out of staging and into mainline.
Henry: a simple question, please explain it to me, what brcmsmac does
provide that b43 doesn't?
brcmsmac doesn't support SSB and duplicates BCMA support (doesn't make
use of bcma module)
brmcsmac doesn't support older PHYs like G/LP
brmcsmac doesn't support recent HT-PHY
brmcsmac doesn't support AP or monitor mode
all of that is supported by b43
The one thing brcmsmac is better at, is support for 14e4:4727 card
(BCM4313). I've already started work on support of this card in b43,
hope to get it working soon.
You mostly just duplicate support for cards that are already supported by b43.
--
Rafał
On Wed, Aug 31, 2011 at 01:55:58PM +0200, Hauke Mehrtens wrote:
> If brcmsmac gets merged it should support all braodcom softmac wireless
> devices with ieee80211n support, this includes also the N-PHY devices
> with SB-bus, otherwise ieee80211n support has to be added to b43 and
> then I do not see any advantage over just using b43 and removing
> brcmsmac. Will Broadcom support these older chip in brcmsmac and also
> all new devices still missing now or has the community to add support
> for these devices without any help by broadcom?
This requirement seems rather artificial. Vendors choose which devices
they want to support. Intel has abandoned devices (e.g. ipw2100,
ipw2200, and iwlegacy), and arguably so has Atheros (e.g. ath5k).
Why should Broadcom be compelled to support older devices simply
because there is some overlap between brcmsmac and b43? If they went
down that path, would you then demand that they support the b43legacy
devices as well? Why not?
The strategy of cramming all vaguely similar devices under "one"
driver doesn't have a great track record IMHO. We have had to split
iwlwifi, and may have to do so again. e1000 got split similarly,
as did b43/b43legacy. I see no reason to compel a driver to support
an extended range of hardware when there are reasonable dissimilarities
between the devices.
> What are your plans in updating the PHY code in brcmsmac? As Rafał
> mentioned your closed source linux wifi driver (wl) is ~6 months ahead
> of brcmsmac now.
For the last ~6 months the Broadcom team has been working on getting
their driver out of staging. I have to believe that they would have
rather been working on updating device support during that time. I can
only presume that they would make that a priority in the long run.
How many times has b43 been > ~6 months behind on it's hardware
support? Despite Rafał's recent heroic efforts at improving that,
I can't help but wonder how long will it be before b43 is again
dreadfully behind?
> Are you planing to replace your closed source linux driver with brcmsmac
> on normal x86 desktops and Linux SoCs and will you support brcmsmac as
> you did before with wl?
I'm guessing that their customers will decide this for them, if we
allow the customers to have a reasonable choice.
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
2011/8/30 Henry Ptasinski <[email protected]>:
> On Sat, Aug 27, 2011 at 08:21:44AM -0700, Greg KH wrote:
>> Ok, we don't want/accept duplicate drivers for the same devices (well, I
>> sure don't want that, we had it in the past in the USB subsystem and it
>> was a nightmare).
>>
>> So, as b43 was here first, it looks like brcmfmac is the only part that
>> should really move out of staging, right?
>>
>> Henry, thoughts? Have you all been tracking the b43 support for the
>> past year?
>
> Greg,
>
> The brcmsmac driver supports full-rate 802.11n/HT operation on 20MHz channels,
> and has from the day we released it. This includes 802.11n/HT rates and
> multiple spatial streams, and a number of additional 11n features such as
> A-MPDU and RIFS. Current iperf testing on 20MHz channels with a BCM43224
> achieves greater than 70Mbps TCP throughput, using phy rates up to about
> 130Mbps.
>
> This contrasts with a maximum possible rate of 54Mbps/phy or 24Mbps TCP
> throughput for any driver that is legacy only and/or which doesn't support 11n
> optimizations such as aggregation of layer 2 PDUs (AMPDU). 802.11n operation
> can also achive greater range than legacy operation.
>
> b43 doesn't currently support 802.11n at all, so performance with b43 is
> limited to legacy 11g rates at best.
I agree, that's the biggest feature lacking in b43 and not started at
all. I want to work on this, but I'm just a single man having only 24
hours in his day. Right now I'm focusing on basic support for all the
cards the market has (it's LCN now).
> The brcmsmac driver supports 5GHz channels, including 802.11n operation in
> 5GHz. b43 doesn't appear to currently support 5GHz.
I believe most of the code is in the place, I just don't have right
hardware (router) to test it. Please take a look at our PHY code,
there are proper checks for 2 vs. 5 GHz band and the tables contain
values for PHY / radio. It just needs enabling (in main.c IIRC) and
testing.
> The brcmsmac phy code also has full support for 802.11n/HT operation on 40MHz
> channels. Some of the upper MAC layer settings (e.g. indicating 40MHz support
> to the stack) need updating in order to enable 40MHz channels, but all the
> critical phy support is present.
>
> The brcmsmac phy code is a direct derivative of the phy code used in our other
> drivers, which has been designed and *tested* to work properly over the full
> range of chip operating temperatures and fabrication process corners.
>
> The b43 driver uses a snapshot of the calibration values, that was obtained
> with a single (or few) chips in one environment, and applies those values
> across the board to all chips, regardless of process variations, in all
> environments.
Are you talking about our G-PHY, LP-PHY or N-PHY code? Can you show
where did we hardcode such a values? I believe we didn't so some tip
would be nice.
If you are talking about HT-PHY, this is *of course* true. I didn't
have any specs of reference source to write support for HT-PHY. I
wrote everything from MMIO dumps which is also kind of miracle it's
working at all.
I did it, because there isn't any Linux (at least x86/x86_64) driver
for HT-PHY. Desperate people having that (not-replaceable) card in
their MacBooks were trying ndiswrapper but that was causing frequent
lock ups. HT-PHY support is really experimental and I don't deny it.
Its just better than nothing (or ndiswrapper sometimes).
> The b43 driver doesn't implement transmit power control for the BCM43224 or
> BCM43225, and has various other unimplemented phy functions, e.g.:
>
>> void b43_nphy_set_rxantenna(struct b43_wldev *dev, int antenna)
>> {//TODO
>> }
>>
>> static void b43_nphy_op_adjust_txpower(struct b43_wldev *dev)
>> {//TODO
>> }
>>
>> static enum b43_txpwr_result b43_nphy_op_recalc_txpower(struct b43_wldev *dev,
>> bool ignore_tssi)
>> {//TODO
>> return B43_TXPWR_RES_DONE;
>> }
>>
>> ...
>> b43err(dev->wl, "enabling tx pwr ctrl not implemented yet\n"); ...
>> ...
>
> etc.
I've just checked your wlc_phy_txpower_recalc_target_nphy. It's calling:
1) Trivial wlc_phy_txpwr_limit_to_tbl_nphy
2) More advanced wlc_phy_txpwrctrl_pwr_setup_nphy we have to implement
3) Already-implemented-in-b43 wlapi_bmac_mctrl
4) 50% implemented wlc_phy_txpwrctrl_enable_nphy
I can do this in 1 day.
--
Rafał
On Wed, Aug 31, 2011 at 10:46 AM, Luis R. Rodriguez <[email protected]> wrote:
> At Atheros not only have we provided documentation to help
> support ath5k but we even released firmware for our legacy Otus driver
> -- and documentation.
To be clear, open GPLv2 firmware.
Luis
Hi,
On 25.8.2011, at 2.18, Henry Ptasinski wrote:
> On Wed, Aug 24, 2011 at 04:10:31PM -0700, Aaro Koskinen wrote:
>> On 25.8.2011, at 1.28, Henry Ptasinski wrote:
>>> The drivers compile cleanly for x86 (32- and 64-bit), PPC (32- and
>>> 64-bit),
>>> SPAR, MIPS BE, MIPS LE, and ARM.
>>
>> Are you sure the compilation is even enabled on all of those
>> platforms?
>>
>> E.g.:
>>
>>> +config BRCMSMAC
>>> + tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
>>> + default n
>>> + depends on PCI
>>> + depends on WLAN && MAC80211
>>> + depends on X86 || MIPS
>> ^^^^^^^^^^^^^^^^^^^^^^
>>
>> Why this?
>
> See my other message. Wrong link to the patch. Correct link:
>
> http://linuxwireless.org/en/users/Drivers/brcm80211?
> action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-
> v2.patch
Thanks. The second version builds for ARM, but there's quite a few
sparse warnings....
A.
On Wed, Aug 24, 2011 at 04:05:31PM -0700, Dan Carpenter wrote:
> On Wed, Aug 24, 2011 at 03:28:01PM -0700, Henry Ptasinski wrote:
> > diff --git a/drivers/staging/brcm80211/brcmsmac/dma.c b/drivers/staging/brcm8021
> > index 05dad9f..73e5841 100644
> > --- a/drivers/staging/brcm80211/brcmsmac/dma.c
> > +++ b/drivers/staging/brcm80211/brcmsmac/dma.c
> > @@ -815,7 +815,7 @@ struct sk_buff *dma_rx(struct dma_pub *pub)
> > tail = head;
> > while ((resid > 0) && (p = _dma_getnextrxp(di, false))) {
> > tail->next = p;
> > - pkt_len = min(resid, (int)di->rxbufsize);
> > + pkt_len = min_t(int, resid, (int)di->rxbufsize);
>
> This isn't right. It should be:
> pkt_len = min_t(uint, resid, di->rxbufsize);
>
> Casting it to int would mean that high values of ->rxbufsize would be
> treated as lower than "resid".
Good point. I'll work that change into the next version or send a separate
patch to fix it up.
>
> > __skb_trim(p, pkt_len);
> >
> > tail = p;
>
> regards,
> dan carpenter
Thanks,
- Henry
On Wed, Aug 24, 2011 at 05:52:32PM -0700, Joe Perches wrote:
> On Wed, 2011-08-24 at 17:42 -0700, Henry Ptasinski wrote:
> > On Wed, Aug 24, 2011 at 04:54:28PM -0700, Joe Perches wrote:
> > > On Wed, 2011-08-24 at 16:17 -0700, Henry Ptasinski wrote:
> > > > Augh. I included the wrong link. Correct link:
> > > > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
> > > Can't you post the patch with git format-patch -M instead?
> > This patch drops in a new copy of all the files, as there's been significant
> > change since the last time staging-next and wireless-testing were in sync.
> > (Deleting the existing files from staging will be a separate step.)
> >
> > I can generate a two-patch series instead: one to catch up wireless-testing
> > with all the changes that have been applied to the brcm80211 drivers in
> > staging, and then a second patch with a 'git mv' plus necessary
> > Kconfig/Makefile changes. The first patch would still be quite large, but I'm
> > ok with either approach.
>
> I think that's the better approach, though I wonder
> why you need to update wireless-testing at all.
>
> Can't you do a delete from wireless-testing
> and then a move from staging?
How do you do a move across git repos?
If John could do a pull of just drivers/staging/brcm80211 from staging-next
into wireless-testing to get just those files in sync, then a 'git mv' from
drivers/staging to drivers/net/wireless within wireless-testing would be
trivial. I don't know if such a pull is possible, but maybe somebody else
knows how and can enlighten me.
- Henry
W dniu 27 sierpnia 2011 16:35 użytkownik Dan Carpenter
<[email protected]> napisał:
> On Thu, Aug 25, 2011 at 10:55:26PM +0200, Rafał Miłecki wrote:
>> 2011/8/25 Henry Ptasinski <[email protected]>:
>> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> > once again propose moving brcm80211 out of staging and into mainline.
>>
>> Henry: a simple question, please explain it to me, what brcmsmac does
>> provide that b43 doesn't?
>>
>
> Wow. Why are we only having this discussion now? Somewhere along
> the line, there has been a massive communications failure. What
> happened here? Henry, did you know about the b43 driver? Can
> someone explain what's going on?
[Don't care to read that if you know b43&brcm80211 history]
You've to understand the architecture of Broadcom's cards first. It
less or more (less) like that:
Card -> Bus -> Chipset -> PHY & radio
The biggest part of the code is to support PHY and radio. There are of
course different types of both. It does not matter if PHY is located
on SSB bus or BCMA bus, you program it almost the same way.
brcm80211 (brcmsmac) supports PHY type "N", but it has support for
BCMA bus only.
When brcm80211 appeared, b43 supported PHY type "N", but it was
limited to SSB bus only.
I've added support for BCMA to b43 and this automatically enabled
support for new cards that were based on PHY type "N". Recently I've
added support for PHY type "HT" and now I'm working on LCN.
The code duplication between b43 and brcmsmac was acceptable when b43
didn't support BCMA. brcm80211 was really needed then because we
needed support for cards using BCMA. Now when b43 supports bcma it
sounds less interesting. brcm80211 duplicates a lot of general code,
PHY type N support code, but at the same time it doesn't support all
the N-PHY devices (there ssb based).
--
Rafał
W dniu 25 sierpnia 2011 23:07 użytkownik Rafał Miłecki
<[email protected]> napisał:
> 2011/8/25 Jonas Gorski <[email protected]>:
>> Hi,
>>
>> On 25 August 2011 02:20, Henry Ptasinski <[email protected]> wrote:
>>> On Wed, Aug 24, 2011 at 04:41:54PM -0700, Jonas Gorski wrote:
>>>> Hi Henry,
>>>>
>>>> On 25 August 2011 00:28, Henry Ptasinski <[email protected]> wrote:
>>>> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>>>> > once again propose moving brcm80211 out of staging and into mainline.
>>>>
>>>> While I like the Idea of brcm80211 going mainline, I'd like to throw
>>>> in the suggestion that brcm80211 should be made a bcma/ssb driver
>>>> first (AFACT brcmfmac would use ssb, not bcma, therefore both).
>>>>
>>>> My reasoning is that it needs to be done eventually anyway, and the
>>>> earlier this is done the less work it will be in the long term, also
>>>> it would reduce the duplicate code in bcma, ssb, and brcm80211.
>>>>
>>>> Of course this is just a suggestion, and it's yours and Greg's call
>>>> whether you agree with me or not (since it's quite late in the game to
>>>> add a new TODO, and I suspect a rather big one).
>>>
>>> We started converting brcmsmac to bcma, but bcma is evolving rapidly in the
>>> wireless-testing tree. Since wireless-testing and staging-next only get in
>>> sync during a kernel merge, the version of bcma we have to work with in staging
>>> is usually quit outdated. Unless Greg and John want to come up with a process
>>> for keeping bcma consistent between their two repos, I don't really see how we
>>> can productively use bcma until we cross over. We do intend to switch to using
>>> bcma as soon as possible.
>>
>> Okay, then no objections from me. The keeping in sync is a valid
>> reason. I'm looking forward to seeing your bcma patches :-)
>>
>>> I believe the only SB bus functions that brcmfmac uses are the core reset and
>>> disable functions, and only when initializing the chip to download firmware
>>> (all other management of the bus is handled by the on-chip CPU). Is it
>>> possible to use those funtions from ssb, without the ssb module trying to
>>> manage the bus?
>>
>> I haven't really looked at how much the brcmfmac driver uses ssb; I
>> just saw s(s)b_* stuff in there and remembered that ssb supports SDIO
>> host, so I assumed that there's part of the stack hidden in brcmfmac.
>> But if it's only core reset and disable, then what Michael said
>> applies ;-)
>
> Still, is there any good reason for duplicating that code?
Also what about embedded devices? Shouldn't we have core drivers for
them? I mean, to be able to easily choose driver for a given core?
Let's say I want to use some ssb-core-based driver for Ethernet port
(like gige) but I also want to use brcmsmac fo 80211. Is that possible
without brcmsmac using bcma?
--
Rafał
On Wed, Aug 24, 2011 at 04:54:28PM -0700, Joe Perches wrote:
> On Wed, 2011-08-24 at 16:17 -0700, Henry Ptasinski wrote:
> > Augh. I included the wrong link. Correct link:
> > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
>
> Can't you post the patch with git format-patch -M instead?
This patch drops in a new copy of all the files, as there's been significant
change since the last time staging-next and wireless-testing were in sync.
(Deleting the existing files from staging will be a separate step.)
I can generate a two-patch series instead: one to catch up wireless-testing
with all the changes that have been applied to the brcm80211 drivers in
staging, and then a second patch with a 'git mv' plus necessary
Kconfig/Makefile changes. The first patch would still be quite large, but I'm
ok with either approach.
- Henry
On Wed, Aug 24, 2011 at 04:41:54PM -0700, Jonas Gorski wrote:
> Hi Henry,
>
> On 25 August 2011 00:28, Henry Ptasinski <[email protected]> wrote:
> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
> > once again propose moving brcm80211 out of staging and into mainline.
>
> While I like the Idea of brcm80211 going mainline, I'd like to throw
> in the suggestion that brcm80211 should be made a bcma/ssb driver
> first (AFACT brcmfmac would use ssb, not bcma, therefore both).
>
> My reasoning is that it needs to be done eventually anyway, and the
> earlier this is done the less work it will be in the long term, also
> it would reduce the duplicate code in bcma, ssb, and brcm80211.
>
> Of course this is just a suggestion, and it's yours and Greg's call
> whether you agree with me or not (since it's quite late in the game to
> add a new TODO, and I suspect a rather big one).
We started converting brcmsmac to bcma, but bcma is evolving rapidly in the
wireless-testing tree. Since wireless-testing and staging-next only get in
sync during a kernel merge, the version of bcma we have to work with in staging
is usually quit outdated. Unless Greg and John want to come up with a process
for keeping bcma consistent between their two repos, I don't really see how we
can productively use bcma until we cross over. We do intend to switch to using
bcma as soon as possible.
I believe the only SB bus functions that brcmfmac uses are the core reset and
disable functions, and only when initializing the chip to download firmware
(all other management of the bus is handled by the on-chip CPU). Is it
possible to use those funtions from ssb, without the ssb module trying to
manage the bus?
- Henry
On Wed, Aug 31, 2011 at 7:18 AM, John W. Linville
<[email protected]> wrote:
> On Wed, Aug 31, 2011 at 01:55:58PM +0200, Hauke Mehrtens wrote:
>
>> If brcmsmac gets merged it should support all braodcom softmac wireless
>> devices with ieee80211n support, this includes also the N-PHY devices
>> with SB-bus, otherwise ieee80211n support has to be added to b43 and
>> then I do not see any advantage over just using b43 and removing
>> brcmsmac. Will Broadcom support these older chip in brcmsmac and also
>> all new devices still missing now or has the community to add support
>> for these devices without any help by broadcom?
>
> This requirement seems rather artificial. Vendors choose which devices
> they want to support. Intel has abandoned devices (e.g. ipw2100,
> ipw2200, and iwlegacy), and arguably so has Atheros (e.g. ath5k).
> Why should Broadcom be compelled to support older devices simply
> because there is some overlap between brcmsmac and b43? If they went
> down that path, would you then demand that they support the b43legacy
> devices as well? Why not?
>
> The strategy of cramming all vaguely similar devices under "one"
> driver doesn't have a great track record IMHO.
Agreed.
> We have had to split
> iwlwifi, and may have to do so again. e1000 got split similarly,
> as did b43/b43legacy.
And I would hope someone would even split i915 driver too, having a
regression on every rc1 of the kernel seems rather ridiculous to me.
> I see no reason to compel a driver to support
> an extended range of hardware when there are reasonable dissimilarities
> between the devices.
Agreed here -- but one thing is to dedicate resources to supporting
old devices, which of course no silicon company wants to do, another
is to enable the community to help support older device. The later is
what I argue is reasonable and every silicon provider needs to work
harder at. At Atheros not only have we provided documentation to help
support ath5k but we even released firmware for our legacy Otus driver
-- and documentation. Do not tell me this is not possible, I simply do
not buy it -- you are simply not trying hard enough. Orphaning drivers
can be done better.
Luis
W dniu 26 sierpnia 2011 19:55 użytkownik Henry Ptasinski
<[email protected]> napisał:
> On Thu, Aug 25, 2011 at 01:55:26PM -0700, Rafał Miłecki wrote:
>> 2011/8/25 Henry Ptasinski <[email protected]>:
>> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> > once again propose moving brcm80211 out of staging and into mainline.
>>
>> Henry: a simple question, please explain it to me, what brcmsmac does
>> provide that b43 doesn't?
>
> The brcmsmac driver supports the BCM4313, BCM43224, and BCM43225 chipsets with
> full performance i.e. rate vs. range comparable to what's achived with these
> chips running under other operating systems.
>
> The underlying phy algorithms, especially the dynamic calibrations, have been
> designed and tested, over the full range of chip operating temperatures, and
> across fabrication process corners, to maximize the performance acheived in all
> cases.
Are you aware, that b43's code comes from RE efforts and PHY part is
mostly identical to brcmsmac/wl's one?
We have the same functions, conditions, operations here. Sometimes
called in a different way, sometimes we have different code
structures, but generally code is the same.
So I still don't see the advantages of using brcmsmac instead of b43.
> The driver is fully supported by Broadcom, and provides an API for addition of
> phy support for newer chipsets that will help speed up the process of getting
> support for those chipsets into the kernel.
I'm glad you're so proud of yours flexible API, but b43 has the same
implemented since 2008. You can see we've just easily added support
for HT-PHY using our API and I'm working on LCN-PHY right now (without
modifying the API).
We would like to have b43 supported by Broadcom. It sounds much
better, I've shown you a lot of advantages of such a choice. Switching
to brcmsmac on the other hand needs a lot of work and improvements.
--
Rafał
On Tue, Aug 30, 2011 at 11:14 AM, Greg KH <[email protected]> wrote:
> On Mon, Aug 29, 2011 at 06:42:57PM -0700, Henry Ptasinski wrote:
>> The brcmsmac driver has architectural alignment with our drivers for other
>> operating systems, and we intend to to enhance and maintain this driver in
>> parallel with drivers for other operating systems. Maintaining alignment
>> between our Linux driver and drivers for other operating systems allows us to
>> leverage feature and chip support across all platforms.
>
> Just curious, if you really are going to try to do this, how are you
> going to handle the issue when others change the in-kernel driver in
> ways that you are not going to be allowed to make to your "internal"
> copy of the driver?
What do you mean by this?
> Also, how are you going to handle any GPL-only changes that happen to
> the code as well?
For the broadcom drivers this may be hard if people want to move GPLv2
b43 code to a permissively licensed driver... but ...
> Do you have some process in place to ensure that all
> contributions will have the proper copyright releases on it to allow you
> to make the same changes to your internal versions?
To be clear *new* code going in to a permissively licensed driver
follows the Developer's Certificate of Origin which does state the
contributor follows the file's license. Back in the hay day we thought
this was not enough and introduced a Changes-licensed-under tag but a
few upstream maintainers were not fans of it given that they did
believe the Developer's Certificate of Origin with the Signed-off-by
was enough for it. This is in fact accurate, but only if you educate
your developers to ensure they do know what the Signed-off-by means
and that they have read the Developer's Certificate of Origin. We take
care to repeat this to new contributors to our permissively licensed
drivers and would gladly help Broadcom in doing this as well.
The issues with a large GPLv2 code from b43 and the fact that the
other driver is permissively licensed makes this a bit more
complicated though, but that is only a reflection of not addressing
supporting old hardware. Ouch.
> This all is a very difficult and time-consuming task, are you sure you
> are all up to it and have properly discussed it with your legal team,
> management team, and with the kernel community?
As I see it the only difficult aspect here is large GPLv2 codebase on
b43. The rest requires just education on the Developer's Certificate
or Origin, otherwise we could not share between Linux and the BSD
families, as we had done before with other subsystems, not only
wireless.
Luis
On Thu, Aug 25, 2011 at 10:55:26PM +0200, Rafał Miłecki wrote:
> 2011/8/25 Henry Ptasinski <[email protected]>:
> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
> > once again propose moving brcm80211 out of staging and into mainline.
>
> Henry: a simple question, please explain it to me, what brcmsmac does
> provide that b43 doesn't?
>
Wow. Why are we only having this discussion now? Somewhere along
the line, there has been a massive communications failure. What
happened here? Henry, did you know about the b43 driver? Can
someone explain what's going on?
regards,
dan carpenter
W dniu 26 sierpnia 2011 19:55 użytkownik Henry Ptasinski
<[email protected]> napisał:
> On Thu, Aug 25, 2011 at 01:55:26PM -0700, Rafał Miłecki wrote:
>> 2011/8/25 Henry Ptasinski <[email protected]>:
>> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> > once again propose moving brcm80211 out of staging and into mainline.
>>
>> Henry: a simple question, please explain it to me, what brcmsmac does
>> provide that b43 doesn't?
>
> The brcmsmac driver supports the BCM4313, BCM43224, and BCM43225 chipsets with
> full performance i.e. rate vs. range comparable to what's achived with these
> chips running under other operating systems.
Out of curiosity, do you *really* already support 40 MHz channels in brcmsmac?
According to http://wireless.kernel.org/en/users/Drivers/brcm80211 you don't...
--
Rafał
W dniu 27 sierpnia 2011 17:21 użytkownik Greg KH <[email protected]> napisał:
> So, as b43 was here first, it looks like brcmfmac is the only part that
> should really move out of staging, right?
Yes, I'd like to see brcmfmac improving and leaving staging. I've
offered my help to the Broadcom, but they didn't finally respond to me
about donating me with the hardware.
--
Rafał
On Thu, Aug 25, 2011 at 03:34:52AM -0700, Jonas Gorski wrote:
> > I believe the only SB bus functions that brcmfmac uses are the core reset and
> > disable functions, and only when initializing the chip to download firmware
> > (all other management of the bus is handled by the on-chip CPU). ?Is it
> > possible to use those funtions from ssb, without the ssb module trying to
> > manage the bus?
>
> I haven't really looked at how much the brcmfmac driver uses ssb; I
> just saw s(s)b_* stuff in there and remembered that ssb supports SDIO
> host, so I assumed that there's part of the stack hidden in brcmfmac.
> But if it's only core reset and disable, then what Michael said
> applies ;-)
Yes, I was doubting there would be much benefit, but didn't have a strong
opinion either way.
Thanks for the feedback.
- Henry
W dniu 26 sierpnia 2011 19:55 użytkownik Henry Ptasinski
<[email protected]> napisał:
> On Thu, Aug 25, 2011 at 01:55:26PM -0700, Rafał Miłecki wrote:
>> 2011/8/25 Henry Ptasinski <[email protected]>:
>> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> > once again propose moving brcm80211 out of staging and into mainline.
>>
>> Henry: a simple question, please explain it to me, what brcmsmac does
>> provide that b43 doesn't?
>
> The brcmsmac driver supports the BCM4313, BCM43224, and BCM43225 chipsets with
> full performance i.e. rate vs. range comparable to what's achived with these
> chips running under other operating systems.
Henry, I've made quick simple comparison of wl (which is slightly
newer version of brcmsmac) with b43 for my 14e4:4329 card. I chose
this card, because it's affected by DMA hardware bug we've recently
added fix for in ssb&b43.
You can see attached files for raw results or view chart at my blog:
http://zajec.net/blog/view/2011-b43-dma-fixed-on-some-pci-cards
As you can see, b43 is very comparable with wl/brcmsmac and sometimes
can achieve even higher speeds (for high rates in this case).
I believe this will make you calmer about quality of the b43.
--
Rafał
On Sat, Aug 27, 2011 at 08:21:44AM -0700, Greg KH wrote:
> Ok, we don't want/accept duplicate drivers for the same devices (well, I
> sure don't want that, we had it in the past in the USB subsystem and it
> was a nightmare).
>
> So, as b43 was here first, it looks like brcmfmac is the only part that
> should really move out of staging, right?
>
> Henry, thoughts? Have you all been tracking the b43 support for the
> past year?
Greg,
The brcmsmac driver supports full-rate 802.11n/HT operation on 20MHz channels,
and has from the day we released it. This includes 802.11n/HT rates and
multiple spatial streams, and a number of additional 11n features such as
A-MPDU and RIFS. Current iperf testing on 20MHz channels with a BCM43224
achieves greater than 70Mbps TCP throughput, using phy rates up to about
130Mbps.
This contrasts with a maximum possible rate of 54Mbps/phy or 24Mbps TCP
throughput for any driver that is legacy only and/or which doesn't support 11n
optimizations such as aggregation of layer 2 PDUs (AMPDU). 802.11n operation
can also achive greater range than legacy operation.
b43 doesn't currently support 802.11n at all, so performance with b43 is
limited to legacy 11g rates at best.
The brcmsmac driver supports 5GHz channels, including 802.11n operation in
5GHz. b43 doesn't appear to currently support 5GHz.
The brcmsmac phy code also has full support for 802.11n/HT operation on 40MHz
channels. Some of the upper MAC layer settings (e.g. indicating 40MHz support
to the stack) need updating in order to enable 40MHz channels, but all the
critical phy support is present.
The brcmsmac phy code is a direct derivative of the phy code used in our other
drivers, which has been designed and *tested* to work properly over the full
range of chip operating temperatures and fabrication process corners.
The b43 driver uses a snapshot of the calibration values, that was obtained
with a single (or few) chips in one environment, and applies those values
across the board to all chips, regardless of process variations, in all
environments.
The b43 driver doesn't implement transmit power control for the BCM43224 or
BCM43225, and has various other unimplemented phy functions, e.g.:
> void b43_nphy_set_rxantenna(struct b43_wldev *dev, int antenna)
> {//TODO
> }
>
> static void b43_nphy_op_adjust_txpower(struct b43_wldev *dev)
> {//TODO
> }
>
> static enum b43_txpwr_result b43_nphy_op_recalc_txpower(struct b43_wldev *dev,
> bool ignore_tssi)
> {//TODO
> return B43_TXPWR_RES_DONE;
> }
>
> ...
> b43err(dev->wl, "enabling tx pwr ctrl not implemented yet\n"); ...
> ...
etc.
So while brcmsmac may not yet have monitor or AP mode, it is far more complete
and functional in terms of 802.11n capabilities and phy functionality for the
chips that are currently supported by the driver. As we add new, much more
complicated chips, those chips will also get complete, full-performance,
fully-tested support.
We understand the level of effort that it's taken for b43 to get as far as it
has, and was implemented without access to proprietary information, and it has
been impossible to support advanced chip features. We appreciate the difficulty
of your task and respect your accomplishments tremendously!
The brcmsmac driver has architectural alignment with our drivers for other
operating systems, and we intend to to enhance and maintain this driver in
parallel with drivers for other operating systems. Maintaining alignment
between our Linux driver and drivers for other operating systems allows us to
leverage feature and chip support across all platforms.
Broadcom has made, and is continuing to make, a big investment in open source
and is planning on supporting the brcmsmac driver fully. The benefit to the
community is:
* Complete silicon support, including real calibration
* Full 802.11n standard support
* Increasing features and chips over time.
We understand and respect the goal of 1 driver for 1 piece of hardware.
However, we released the brcmsmac driver with 802.11n support last September,
whereas b43 still doesn't have 802.11n support, so brcmsmac is still the first
and only driver to provide full support for these chips.
- Henry
W dniu 27 sierpnia 2011 17:08 użytkownik Rafał Miłecki
<[email protected]> napisał:
> 2011/8/27 Greg KH <[email protected]>:
>> On Sat, Aug 27, 2011 at 05:35:13PM +0300, Dan Carpenter wrote:
>>> On Thu, Aug 25, 2011 at 10:55:26PM +0200, Rafał Miłecki wrote:
>>> > 2011/8/25 Henry Ptasinski <[email protected]>:
>>> > > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>>> > > once again propose moving brcm80211 out of staging and into mainline.
>>> >
>>> > Henry: a simple question, please explain it to me, what brcmsmac does
>>> > provide that b43 doesn't?
>>> >
>>>
>>> Wow. Why are we only having this discussion now? Somewhere along
>>> the line, there has been a massive communications failure. What
>>> happened here? Henry, did you know about the b43 driver? Can
>>> someone explain what's going on?
>>
>> I always thought that b43 and the staging driver supported different
>> devices and had no overlap, which is why I had no problem with it.
>>
>> Was I totally mistaken and got this wrong?
>
> Please see my last e-mail in this thread for history overview.
>
> Right now the only card that is supported by brcmsmac and is not
> supported by b43 is 14e4:4727. That card is based on PHY type LCN, and
> I'm working on adding support for it in b43.
>
> Other cards supported by brcmsmac (14e4:4353, 14e4:4357) are supported
> by b43.
b43 also works on SoCs (routers) using BCMA and having BCMA core based
on N-PHY. This support was added by Hauke and I think he reported AP
mode was somehow working for him. AFAIR it wasn't stable yet, but some
packets were transferred :)
--
Rafał
On 08/27/2011 05:12 PM, Rafał Miłecki wrote:
> W dniu 27 sierpnia 2011 17:08 użytkownik Rafał Miłecki
> <[email protected]> napisał:
>> 2011/8/27 Greg KH <[email protected]>:
>>> On Sat, Aug 27, 2011 at 05:35:13PM +0300, Dan Carpenter wrote:
>>>> On Thu, Aug 25, 2011 at 10:55:26PM +0200, Rafał Miłecki wrote:
>>>>> 2011/8/25 Henry Ptasinski <[email protected]>:
>>>>>> With the latest series of cleanup patches merged in by Greg KH, I'd like to
>>>>>> once again propose moving brcm80211 out of staging and into mainline.
>>>>>
>>>>> Henry: a simple question, please explain it to me, what brcmsmac does
>>>>> provide that b43 doesn't?
>>>>>
>>>>
>>>> Wow. Why are we only having this discussion now? Somewhere along
>>>> the line, there has been a massive communications failure. What
>>>> happened here? Henry, did you know about the b43 driver? Can
>>>> someone explain what's going on?
>>>
>>> I always thought that b43 and the staging driver supported different
>>> devices and had no overlap, which is why I had no problem with it.
>>>
>>> Was I totally mistaken and got this wrong?
>>
>> Please see my last e-mail in this thread for history overview.
>>
>> Right now the only card that is supported by brcmsmac and is not
>> supported by b43 is 14e4:4727. That card is based on PHY type LCN, and
>> I'm working on adding support for it in b43.
>>
>> Other cards supported by brcmsmac (14e4:4353, 14e4:4357) are supported
>> by b43.
>
> b43 also works on SoCs (routers) using BCMA and having BCMA core based
> on N-PHY. This support was added by Hauke and I think he reported AP
> mode was somehow working for him. AFAIR it wasn't stable yet, but some
> packets were transferred :)
>
Yes AP mode is working with the code in wireless-testing, expect that
the architecture code (mips) does not provide the sprom stored in the
flash to bcma yet. I just set a random mac address for the device and
wireless worked. I have not tested it much, just connected with a client
and got the same speed with iperf as b43 running in client mode. As no
additional changes where needed for AP mode I think b43 should already
support the same functionality when using N-PHY devices as with G-PHY
devices.
Hauke
On Tue, 30 Aug 2011 10:31:08 +0200
Rafał Miłecki <[email protected]> wrote:
> I got lost. You're talking a lot about ucode, but I don't know what do
> you really mean. Do you have some additional info from Broadcom? Are
> they going to release some new ucode not compatible with some older
> cards?
They do so frequently and always did this.
The problem in b43 is, that b43 needs to support _all_ ucode
APIs from 5 to x (Dunno where we are right now. somewhere > 15).
That was relatively easy until now, because the API didn't vary a lot
between cores. I don't know what will/did change for latest wireless
features, though. But you already applied some changes to the affecting
code. I don't know how complete that stuff is.
--
Greetings, Michael.
On Wed, Aug 24, 2011 at 03:28:01PM -0700, Henry Ptasinski wrote:
> With the latest series of cleanup patches 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 wireless-testing:drivers/net/wireless/brcm80211.
>
> The patch is somewhat large, so I've posted the patch at:
>
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
>
> Changes from the previous version:
>
> V2:
> - Resolve checkpatch issues
Really? All of them? What's with all of the use of 'volatile' in the
driver still? Those should all be resolved as they are all wrong from
what I can see.
Wait, those usages are in your above mentioned patch, but are not in the
current driver in the staging-next tree. Did you mess up when creating
that patch and take an older version of the driver?
confused,
greg k-h
W dniu 27 sierpnia 2011 15:18 użytkownik Michael Büsch <[email protected]> napisał:
> On Sat, 27 Aug 2011 14:05:19 +0200
> Rafał Miłecki <[email protected]> wrote:
>> You can see attached files for raw results or view chart at my blog:
>> http://zajec.net/blog/view/2011-b43-dma-fixed-on-some-pci-cards
>>
>> As you can see, b43 is very comparable with wl/brcmsmac and sometimes
>> can achieve even higher speeds (for high rates in this case).
>>
>> I believe this will make you calmer about quality of the b43.
>
> Impressive.
>
> I think the slightly higher speeds come from the different rate control
> algorithms. However, other than that, I do understand if broadcom
> has certain concerns with supporting the b43 driver _officially_.
> I assume broadcom maintains a QA process internally and b43 simply
> isn't tested by that. And testing costs money. It probably is as simple as that.
>
> It may be hard to tell a sales person that QA has to throw money
> out of the window to test a "redundant" driver.
I see. The situation is everyone prefer his own driver. However b43
has better quality (less hacks), better design (ssb&bcma), more
features (AP, mesh, monitor), more hardware support (all the old cards
and ssb). I can not imagine dropping b43 and it doesn't sound like a
good idea to have 2 drivers for the same hardware.
> So, at the end of the day, I do understand the concerns. However, I would
> also like to see broadcom's QA move towards b43 and start some testing
> on it. Nobody wants the QA to officially support the whole driver tomorrow
> afternoon, already. I'd rather see this as a step by step process. Probably
> leaving legacy G-PHY devices completely out. So QA would say we support
> this and that device on b43, but _not_ those and these..
Henry, can you elaborate on that? How could we help Broadcom to
officially support b43? I'm willing to help & co-operate.
> Also, don't be too hard on Henry. He's nice and very cooperative to
> the linux community within the constraints of the company. :)
I'm sorry, it's just about all the experience with Broadcom we all
have. No cooperating with community, hiding the driver (we found wl.o
in WRT54G firmware), refusing to compile it for x86 for years, and so
on...
Henry: I really would be glad you have community & Broadcom
cooperating. I just don't see a single step from Broadcom into that
direction :| You don't seem to want helping b43, you don't respond to
releasing firmware requests, you can't share hardware other than
what's already available on ebay (sometimes not even that).
Do you see a possibility of cooperating with us? I'm really willing to
help, I'm spending a lot of my free time on b43, I offer my help with
brcmfmac. It just doesn't work if the second side doesn't do a step.
--
Rafał
W dniu 25 sierpnia 2011 22:55 użytkownik Rafał Miłecki
<[email protected]> napisał:
> 2011/8/25 Henry Ptasinski <[email protected]>:
>> With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> once again propose moving brcm80211 out of staging and into mainline.
>
> Henry: a simple question, please explain it to me, what brcmsmac does
> provide that b43 doesn't?
>
> brcmsmac doesn't support SSB and duplicates BCMA support (doesn't make
> use of bcma module)
> brmcsmac doesn't support older PHYs like G/LP
> brmcsmac doesn't support recent HT-PHY
> brmcsmac doesn't support AP or monitor mode
> all of that is supported by b43
>
> The one thing brcmsmac is better at, is support for 14e4:4727 card
> (BCM4313). I've already started work on support of this card in b43,
> hope to get it working soon.
>
>
> You mostly just duplicate support for cards that are already supported by b43.
I've another idea. What about adding support for 14e4:4727 (BCM4313)
into b43 (I'll handle that task) and dropping bcrmsmac?
We could focus on improving one driver instead of creating 2 drivers
for the same hardware.
--
Rafał
Hi,
On 25 August 2011 02:20, Henry Ptasinski <[email protected]> wrote:
> On Wed, Aug 24, 2011 at 04:41:54PM -0700, Jonas Gorski wrote:
>> Hi Henry,
>>
>> On 25 August 2011 00:28, Henry Ptasinski <[email protected]> wrote:
>> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> > once again propose moving brcm80211 out of staging and into mainline.
>>
>> While I like the Idea of brcm80211 going mainline, I'd like to throw
>> in the suggestion that brcm80211 should be made a bcma/ssb driver
>> first (AFACT brcmfmac would use ssb, not bcma, therefore both).
>>
>> My reasoning is that it needs to be done eventually anyway, and the
>> earlier this is done the less work it will be in the long term, also
>> it would reduce the duplicate code in bcma, ssb, and brcm80211.
>>
>> Of course this is just a suggestion, and it's yours and Greg's call
>> whether you agree with me or not (since it's quite late in the game to
>> add a new TODO, and I suspect a rather big one).
>
> We started converting brcmsmac to bcma, but bcma is evolving rapidly in the
> wireless-testing tree. Since wireless-testing and staging-next only get in
> sync during a kernel merge, the version of bcma we have to work with in staging
> is usually quit outdated. Unless Greg and John want to come up with a process
> for keeping bcma consistent between their two repos, I don't really see how we
> can productively use bcma until we cross over. We do intend to switch to using
> bcma as soon as possible.
Okay, then no objections from me. The keeping in sync is a valid
reason. I'm looking forward to seeing your bcma patches :-)
> I believe the only SB bus functions that brcmfmac uses are the core reset and
> disable functions, and only when initializing the chip to download firmware
> (all other management of the bus is handled by the on-chip CPU). Is it
> possible to use those funtions from ssb, without the ssb module trying to
> manage the bus?
I haven't really looked at how much the brcmfmac driver uses ssb; I
just saw s(s)b_* stuff in there and remembered that ssb supports SDIO
host, so I assumed that there's part of the stack hidden in brcmfmac.
But if it's only core reset and disable, then what Michael said
applies ;-)
Regards,
Jonas
On Wed, Aug 24, 2011 at 05:42:11PM -0700, Henry Ptasinski wrote:
> On Wed, Aug 24, 2011 at 04:54:28PM -0700, Joe Perches wrote:
> > On Wed, 2011-08-24 at 16:17 -0700, Henry Ptasinski wrote:
> > > Augh. I included the wrong link. Correct link:
> > > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
> >
> > Can't you post the patch with git format-patch -M instead?
>
> This patch drops in a new copy of all the files, as there's been significant
> change since the last time staging-next and wireless-testing were in sync.
> (Deleting the existing files from staging will be a separate step.)
>
> I can generate a two-patch series instead: one to catch up wireless-testing
> with all the changes that have been applied to the brcm80211 drivers in
> staging, and then a second patch with a 'git mv' plus necessary
> Kconfig/Makefile changes. The first patch would still be quite large, but I'm
> ok with either approach.
Don't worry about that now, the real thing is a review of the code.
greg k-h
On Wed, Aug 31, 2011 at 10:55:45AM -0700, Luis R. Rodriguez wrote:
> On Tue, Aug 30, 2011 at 11:14 AM, Greg KH <[email protected]> wrote:
> > On Mon, Aug 29, 2011 at 06:42:57PM -0700, Henry Ptasinski wrote:
> >> The brcmsmac driver has architectural alignment with our drivers for other
> >> operating systems, and we intend to to enhance and maintain this driver in
> >> parallel with drivers for other operating systems. ?Maintaining alignment
> >> between our Linux driver and drivers for other operating systems allows us to
> >> leverage feature and chip support across all platforms.
> >
> > Just curious, if you really are going to try to do this, how are you
> > going to handle the issue when others change the in-kernel driver in
> > ways that you are not going to be allowed to make to your "internal"
> > copy of the driver?
>
> What do you mean by this?
- Infrastructure changes that do not match up with how other operating
systems handle things.
- support for new features
- support for older devices
- license incompatibilities (i.e. new files under GPL-only license
- etc.
> > Also, how are you going to handle any GPL-only changes that happen to
> > the code as well?
>
> For the broadcom drivers this may be hard if people want to move GPLv2
> b43 code to a permissively licensed driver... but ...
>
> >?Do you have some process in place to ensure that all
> > contributions will have the proper copyright releases on it to allow you
> > to make the same changes to your internal versions?
>
> To be clear *new* code going in to a permissively licensed driver
> follows the Developer's Certificate of Origin which does state the
> contributor follows the file's license.
For an existing file, yes.
But for new files, or for copying code from another driver (GPL only)
into this one (dual licensed), that's different, right?
You need to be very careful about this, that's all.
> The issues with a large GPLv2 code from b43 and the fact that the
> other driver is permissively licensed makes this a bit more
> complicated though, but that is only a reflection of not addressing
> supporting old hardware. Ouch.
Yes, this is going to be tricky :)
That is what I am worried about, but I know the lawyers and developers
at Broadcom have already considered this, but they haven't told us how
they are going to handle it, which is what I'm curious about.
Then there the issue that some companies want to keep their internal
copies in a non-dual licensed codebase, to handle the license of the
other operating systems they are working with. Hopefully Broadcom isn't
going to do this, but others have in the past (SGI with xfs, Intel with
the ACPI core, etc.)
thanks,
greg k-h
On 08/30/2011 03:42 AM, Henry Ptasinski wrote:
> On Sat, Aug 27, 2011 at 08:21:44AM -0700, Greg KH wrote:
>> Ok, we don't want/accept duplicate drivers for the same devices (well, I
>> sure don't want that, we had it in the past in the USB subsystem and it
>> was a nightmare).
>>
>> So, as b43 was here first, it looks like brcmfmac is the only part that
>> should really move out of staging, right?
>>
>> Henry, thoughts? Have you all been tracking the b43 support for the
>> past year?
>
> Greg,
>
> The brcmsmac driver supports full-rate 802.11n/HT operation on 20MHz channels,
> and has from the day we released it. This includes 802.11n/HT rates and
> multiple spatial streams, and a number of additional 11n features such as
> A-MPDU and RIFS. Current iperf testing on 20MHz channels with a BCM43224
> achieves greater than 70Mbps TCP throughput, using phy rates up to about
> 130Mbps.
>
> This contrasts with a maximum possible rate of 54Mbps/phy or 24Mbps TCP
> throughput for any driver that is legacy only and/or which doesn't support 11n
> optimizations such as aggregation of layer 2 PDUs (AMPDU). 802.11n operation
> can also achive greater range than legacy operation.
>
If brcmsmac gets merged it should support all braodcom softmac wireless
devices with ieee80211n support, this includes also the N-PHY devices
with SB-bus, otherwise ieee80211n support has to be added to b43 and
then I do not see any advantage over just using b43 and removing
brcmsmac. Will Broadcom support these older chip in brcmsmac and also
all new devices still missing now or has the community to add support
for these devices without any help by broadcom?
What are your plans in updating the PHY code in brcmsmac? As Rafał
mentioned your closed source linux wifi driver (wl) is ~6 months ahead
of brcmsmac now.
Are you planing to replace your closed source linux driver with brcmsmac
on normal x86 desktops and Linux SoCs and will you support brcmsmac as
you did before with wl?
> b43 doesn't currently support 802.11n at all, so performance with b43 is
> limited to legacy 11g rates at best.
>
> The brcmsmac driver supports 5GHz channels, including 802.11n operation in
> 5GHz. b43 doesn't appear to currently support 5GHz.
>
> The brcmsmac phy code also has full support for 802.11n/HT operation on 40MHz
> channels. Some of the upper MAC layer settings (e.g. indicating 40MHz support
> to the stack) need updating in order to enable 40MHz channels, but all the
> critical phy support is present.
>
> The brcmsmac phy code is a direct derivative of the phy code used in our other
> drivers, which has been designed and *tested* to work properly over the full
> range of chip operating temperatures and fabrication process corners.
>
> The b43 driver uses a snapshot of the calibration values, that was obtained
> with a single (or few) chips in one environment, and applies those values
> across the board to all chips, regardless of process variations, in all
> environments.
If you would provide the community with some documentation or recent
code of your tested driver it would help to fix these issues.
Hauke
On Mon, Aug 29, 2011 at 06:42:57PM -0700, Henry Ptasinski wrote:
> On Sat, Aug 27, 2011 at 08:21:44AM -0700, Greg KH wrote:
> > Ok, we don't want/accept duplicate drivers for the same devices (well, I
> > sure don't want that, we had it in the past in the USB subsystem and it
> > was a nightmare).
> >
> > So, as b43 was here first, it looks like brcmfmac is the only part that
> > should really move out of staging, right?
> >
> > Henry, thoughts? Have you all been tracking the b43 support for the
> > past year?
>
> Greg,
>
> The brcmsmac driver supports full-rate 802.11n/HT operation on 20MHz channels,
> and has from the day we released it.
The day you released it, it did not support the same devices that the
in-kernel b43 driver did, which was the only way I accepted it.
Over time, the in-kernel driver has added new device support, which I
was not aware of.
> This includes 802.11n/HT rates and
> multiple spatial streams, and a number of additional 11n features such as
> A-MPDU and RIFS. Current iperf testing on 20MHz channels with a BCM43224
> achieves greater than 70Mbps TCP throughput, using phy rates up to about
> 130Mbps.
>
> This contrasts with a maximum possible rate of 54Mbps/phy or 24Mbps TCP
> throughput for any driver that is legacy only and/or which doesn't support 11n
> optimizations such as aggregation of layer 2 PDUs (AMPDU). 802.11n operation
> can also achive greater range than legacy operation.
>
> b43 doesn't currently support 802.11n at all, so performance with b43 is
> limited to legacy 11g rates at best.
Ok, then why not just help the b43 developers add 11n support to the
driver? Surely that would have been easier than the 1 year development
effort you all have put in trying to clean up the staging driver to the
level of a "mergable" driver.
> The brcmsmac driver supports 5GHz channels, including 802.11n operation in
> 5GHz. b43 doesn't appear to currently support 5GHz.
Surely that's a simple addition, right?
> We understand the level of effort that it's taken for b43 to get as far as it
> has, and was implemented without access to proprietary information, and it has
> been impossible to support advanced chip features. We appreciate the difficulty
> of your task and respect your accomplishments tremendously!
Then why not try to help out here?
All other wireless companies have realized that this is the best way
forward over time. Remember, b43 was here first, you need to play nice
with those developers.
> The brcmsmac driver has architectural alignment with our drivers for other
> operating systems, and we intend to to enhance and maintain this driver in
> parallel with drivers for other operating systems. Maintaining alignment
> between our Linux driver and drivers for other operating systems allows us to
> leverage feature and chip support across all platforms.
No it doesn't.
Really, it doesn't.
And even if it did, that doesn't pertain to anything here that we care
about, it's not a valid argument.
> Broadcom has made, and is continuing to make, a big investment in open source
> and is planning on supporting the brcmsmac driver fully. The benefit to the
> community is:
>
> * Complete silicon support, including real calibration
> * Full 802.11n standard support
> * Increasing features and chips over time.
>
> We understand and respect the goal of 1 driver for 1 piece of hardware.
> However, we released the brcmsmac driver with 802.11n support last September,
> whereas b43 still doesn't have 802.11n support, so brcmsmac is still the first
> and only driver to provide full support for these chips.
That's great, and we appreciate that. But also realize that this
doesn't mean we owe you anything.
You really need to work with the b43 developers here.
Why can't you just do the small changes needed to the b43 driver to add
the missing functionality based on the fact that you know how the
hardware works?
Again, we can't merge a driver that works for the same device.
greg k-h
On Sat, 27 Aug 2011 14:05:19 +0200
Rafał Miłecki <[email protected]> wrote:
> You can see attached files for raw results or view chart at my blog:
> http://zajec.net/blog/view/2011-b43-dma-fixed-on-some-pci-cards
>
> As you can see, b43 is very comparable with wl/brcmsmac and sometimes
> can achieve even higher speeds (for high rates in this case).
>
> I believe this will make you calmer about quality of the b43.
Impressive.
I think the slightly higher speeds come from the different rate control
algorithms. However, other than that, I do understand if broadcom
has certain concerns with supporting the b43 driver _officially_.
I assume broadcom maintains a QA process internally and b43 simply
isn't tested by that. And testing costs money. It probably is as simple as that.
It may be hard to tell a sales person that QA has to throw money
out of the window to test a "redundant" driver.
So, at the end of the day, I do understand the concerns. However, I would
also like to see broadcom's QA move towards b43 and start some testing
on it. Nobody wants the QA to officially support the whole driver tomorrow
afternoon, already. I'd rather see this as a step by step process. Probably
leaving legacy G-PHY devices completely out. So QA would say we support
this and that device on b43, but _not_ those and these..
Also, don't be too hard on Henry. He's nice and very cooperative to
the linux community within the constraints of the company. :)
--
Greetings, Michael.
On Wed, 2011-08-24 at 16:17 -0700, Henry Ptasinski wrote:
> Augh. I included the wrong link. Correct link:
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
Can't you post the patch with git format-patch -M instead?
Hi Henry,
On 25 August 2011 00:28, Henry Ptasinski <[email protected]> wrote:
> With the latest series of cleanup patches merged in by Greg KH, I'd like to
> once again propose moving brcm80211 out of staging and into mainline.
While I like the Idea of brcm80211 going mainline, I'd like to throw
in the suggestion that brcm80211 should be made a bcma/ssb driver
first (AFACT brcmfmac would use ssb, not bcma, therefore both).
My reasoning is that it needs to be done eventually anyway, and the
earlier this is done the less work it will be in the long term, also
it would reduce the duplicate code in bcma, ssb, and brcm80211.
Of course this is just a suggestion, and it's yours and Greg's call
whether you agree with me or not (since it's quite late in the game to
add a new TODO, and I suspect a rather big one).
Regards,
Jonas
On Mon, Aug 29, 2011 at 06:42:57PM -0700, Henry Ptasinski wrote:
> The brcmsmac driver has architectural alignment with our drivers for other
> operating systems, and we intend to to enhance and maintain this driver in
> parallel with drivers for other operating systems. Maintaining alignment
> between our Linux driver and drivers for other operating systems allows us to
> leverage feature and chip support across all platforms.
Just curious, if you really are going to try to do this, how are you
going to handle the issue when others change the in-kernel driver in
ways that you are not going to be allowed to make to your "internal"
copy of the driver?
Also, how are you going to handle any GPL-only changes that happen to
the code as well? Do you have some process in place to ensure that all
contributions will have the proper copyright releases on it to allow you
to make the same changes to your internal versions?
This all is a very difficult and time-consuming task, are you sure you
are all up to it and have properly discussed it with your legal team,
management team, and with the kernel community?
thanks,
greg k-h
On Wed, 2011-08-24 at 17:42 -0700, Henry Ptasinski wrote:
> On Wed, Aug 24, 2011 at 04:54:28PM -0700, Joe Perches wrote:
> > On Wed, 2011-08-24 at 16:17 -0700, Henry Ptasinski wrote:
> > > Augh. I included the wrong link. Correct link:
> > > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
> > Can't you post the patch with git format-patch -M instead?
> This patch drops in a new copy of all the files, as there's been significant
> change since the last time staging-next and wireless-testing were in sync.
> (Deleting the existing files from staging will be a separate step.)
>
> I can generate a two-patch series instead: one to catch up wireless-testing
> with all the changes that have been applied to the brcm80211 drivers in
> staging, and then a second patch with a 'git mv' plus necessary
> Kconfig/Makefile changes. The first patch would still be quite large, but I'm
> ok with either approach.
I think that's the better approach, though I wonder
why you need to update wireless-testing at all.
Can't you do a delete from wireless-testing
and then a move from staging?
On Thu, Aug 25, 2011 at 01:55:26PM -0700, Rafał Miłecki wrote:
> 2011/8/25 Henry Ptasinski <[email protected]>:
> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
> > once again propose moving brcm80211 out of staging and into mainline.
>
> Henry: a simple question, please explain it to me, what brcmsmac does
> provide that b43 doesn't?
The brcmsmac driver supports the BCM4313, BCM43224, and BCM43225 chipsets with
full performance i.e. rate vs. range comparable to what's achived with these
chips running under other operating systems.
The underlying phy algorithms, especially the dynamic calibrations, have been
designed and tested, over the full range of chip operating temperatures, and
across fabrication process corners, to maximize the performance acheived in all
cases.
The driver is fully supported by Broadcom, and provides an API for addition of
phy support for newer chipsets that will help speed up the process of getting
support for those chipsets into the kernel.
- Henry
On Wed, Aug 24, 2011 at 04:10:31PM -0700, Aaro Koskinen wrote:
> Hi,
>
> On 25.8.2011, at 1.28, Henry Ptasinski wrote:
> > 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 has been
> > challenging).
> >
> > The drivers compile cleanly for x86 (32- and 64-bit), PPC (32- and
> > 64-bit),
> > SPAR, MIPS BE, MIPS LE, and ARM.
>
> Are you sure the compilation is even enabled on all of those platforms?
>
> E.g.:
>
> > +config BRCMSMAC
> > + tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
> > + default n
> > + depends on PCI
> > + depends on WLAN && MAC80211
> > + depends on X86 || MIPS
> ^^^^^^^^^^^^^^^^^^^^^^
>
> Why this?
>
> A.
>
>
See my other message. Wrong link to the patch. Correct link:
http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
Kicking myself very much right now ...
- Henry
Hi,
I guess I should review it :)
> + depends on BCMA=n
I guess that has been discussed, and I agree with you that it should be
done after the move, for the given reasons.
> + default n
No reason for that, that's the default.
> +subdir-ccflags-y := -DBCMDMA32
What's the use of that and other options that seem to always be enabled?
Shouldn't they just be run through "unifdef"? Also, typically, we don't
really use -DXYZ, we use #ifdef CONFIG_ (e.g. for SHOW_EVENTS)
Why are the iovar things needed? It seems very odd to me to look things
up by a string name.
> +#define BRCMF_PM_RESUME_WAIT(a, b) do { \
Should that really be a macro? Also BRCMF_PM_RESUME_RETURN_ERROR seems
rather odd to me. Generally, afaict, macros impacting the code flow are
sort of frowned upon.
What's the ioctl layer in dhd.h? That doesn't seem very
confidence-inspiring :) Also, do you really have your own error codes
still? I see BRCMF_E_..., or are those events? Why have events in the
driver?
brcmf_proto_cdc_query_ioctl and friends seem to really actually use
ioctl strings -- that's not really something we want to see in an
upstream driver.
> +/* Spawn a thread for system ioctls (set mac, set mcast) */
> +uint brcmf_sysioc = true;
> +module_param(brcmf_sysioc, uint, 0);
A thread for system ioctls? What does that even mean?
> +/* Network inteface name */
> +char iface_name[IFNAMSIZ] = "wlan";
> +module_param_string(iface_name, iface_name, IFNAMSIZ, 0);
Seriously? What for? No other driver does that.
> +#ifdef SDTEST
I don't think that needs to be part of the driver (for now)?
> brcmf_netdev_ioctl_entry
Am I looking at the wrong patch? You really want to propose private
driver ioctls? :)
> +/* ARM trap handling */
I hope this is for the device, not the host?
Generally, you have tons of static forward declarations that are
unnecessary.
Also generally, there are tons of macros that really shouldn't/needn't
be and probably would be better as inlines or even real functions.
swap_key_from_BE/swap_key_to_BE have the wrong name since evidently they
do _CPU not _BE
> + fs = get_fs();
> + set_fs(get_ds());
> + err = dev->netdev_ops->ndo_do_ioctl(dev, &ifr, SIOCDEVPRIVATE);
> + set_fs(fs);
kidding, right? what does that even do?
I also said this to Marvell so I'll repeat it here: I really don't think
you should have an internal ioctl abstraction layer -- there's no
problem with calling the right function directly instead of calling a
single ioctl() function and then demuxing again, cf. calls to
brcmf_dev_ioctl() -- also, really, this thing expects little endian
bytes in a buffer? Or is there some hidden HW abstraction there in
ioctl()? But even then it'd be much better to make that explicit.
brcmf_find_msb() -- what's wrong with all the bitops that already exist
like ffs()?
> + /*
> + * Make sure WPA_Supplicant receives all the event
> + * generated due to DISASSOC call to the fw to keep
> + * the state fw and WPA_Supplicant state consistent
> + */
> + rtnl_unlock();
> + brcmf_delay(500);
> + rtnl_lock();
Wow, you can't be serious -- this will could cause serious locking
issues. You _never_ want to drop a lock that some other function/layer
acquired.
> +static s32 brcmf_iscan_thread(void *data)
Why do you need a thread instead of using schedule_work from the timer?
Why do you even run iscan periodically? That's not expected; am I
misinterpreting it?
Is cfg80211_dev really global??
> +#define for_each_bss(list, bss, __i) \
There's a bss list in cfg80211 -- use it, and if necessary export
functions for it, but why do you need to walk it anyway? We just had
this with Marvell and it cut ~1.5k LOC. (and besides, a statically sized
array is always a really bad idea)
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/alloc.c
Err... what does this do? And it even has weird things like *err=1003;
> dma_alloc_consistent
That's going to get really confusing once you switch, like you should,
away from the pci wrappers for DMA mapping. The function seems kinda
pointless too since alignment should be OK unless there are special
requirements?
> +#define LOCK(wl) spin_lock_bh(&(wl)->lock)
No way ...
> +static int n_adapters_found;
what for? bound to be racy ...
Again tons of unneeded static forward declarations
Does it just look like brcms_ops_add_interface() accepts any number of
interfaces?
I'm not sure you should have a function called ieee_set_channel. And
what's the "perimeter lock"?
I personally think brcms_c_set_par() should die. And probably
brcms_c_set too, again, why use a single function just to demultiplex
later?
brcms_ops_set_tim -- remove it
brcms_ops_set_tsf -- ditto
brcms_ops_get_stats -- ditto
brcms_ops_sta_notify -- ditto
brcms_ops_get_tsf -- certainly as well with this implementation
brcms_ops_sta_remove -- probably as well, but are you sure you don't
have to tear down the pktq? Oh, and you should probably get rid of pktq
and use skb_queue_head.
> + * Future improvement:
> + * Use the starting sequence number provided ...
Err, well, that's not really an improvement but a requirement, at least
setting it to 0 is totally bogus. either you assign seqno in the driver
for QoS packets, or you don't -- if you do, *ssn needs the value, if you
don't, *ssn needs to be left untouched.
brcms_msleep -- that's ... wtf? You're never going to get locking
correct with that!! Almost impossible anyway, please get rid of it. You
can sleep while holding a mutex anyway.
> struct brcms_timer
Why do you need your own timer abstraction? And it doesn't even use
list_head.
> +#ifdef SUPPORT_HWKEYS
No reason for the ifdef, right?
> brcms_c_wme_initparams_sta
Shouldn't be necessary.
> +#define CHIP_SUPPORTS_11N(wlc) 1
Is there a plan to supports others?
> + /* pull up some info resulting from the low attach */
> + {
> + int i;
> + for (i = 0; i < NFIFO; i++)
> + wlc->core->txavail[i] = wlc->hw->txavail[i];
> + }
Not exactly Linux style to add braces if you need a local var.
> brcms_c_print_txdesc
I suggest to add tracing instead -- much more useful to actually record
everything :)
brcms_c_compute_rtscts_dur -- mac80211 has this and similer things as
helper functions, no?
> + /* Compiler reference to avoid unused variable warning */
> + (void)(frm_tx2);
wtf? I don't think we have warnings for unused args but why don't you
just remove the arg?
> + rxh->RxFrameSize = le16_to_cpu(rxh->RxFrameSize);
NACK -- use proper structs with endian annotations, this will either
generate sparse warnings or not have proper endian annotations. Try make
C=2 CFLAGS=-D__CHECK_ENDIAN__ M=... (or just add check endian to the
Makefile like mac80211, iwlwifi etc.)
> + /* explicitly test bad src address to avoid sending bad deauth */
??
> + /* due to sheer numbers, toss out probe reqs for now */
> + if (ieee80211_is_probe_req(h->frame_control))
> + goto toss;
???
> brcms_c_calc_lsig_len
Ok, I think I like that, can somebody explain to me how to calculate the
length from the ON_AIR_RISE to the timestamp in beacons for HT?
> +/* wrapper BMAC functions to for HIGH driver access */
Not sure that's really necessary? Especially empty ones seem ...
pointless.
Ok I got all the way to
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.h
(not including that file) but I think I've had enough for now :-)
johannes
On Wed, Aug 24, 2011 at 03:53:57PM -0700, Greg KH wrote:
> On Wed, Aug 24, 2011 at 03:28:01PM -0700, Henry Ptasinski wrote:
> > With the latest series of cleanup patches 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 wireless-testing:drivers/net/wireless/brcm80211.
> >
> > The patch is somewhat large, so I've posted the patch at:
> >
> > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
> >
> > Changes from the previous version:
> >
> > V2:
> > - Resolve checkpatch issues
>
> Really? All of them? What's with all of the use of 'volatile' in the
> driver still? Those should all be resolved as they are all wrong from
> what I can see.
>
> Wait, those usages are in your above mentioned patch, but are not in the
> current driver in the staging-next tree. Did you mess up when creating
> that patch and take an older version of the driver?
>
> confused,
>
> greg k-h
>
Augh. I included the wrong link. Correct link:
http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
(Note the '-v2'.)
Very sorry about that.
- Henry
With the latest series of cleanup patches 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 wireless-testing:drivers/net/wireless/brcm80211.
The patch is somewhat large, so I've posted the patch at:
http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211.patch
Changes from the previous version:
V2:
- Resolve checkpatch issues
- Fix issues on big-endian platforms (MIPS BE, PPC, and SPARC)
- Cleanup architecture-specific code
- Fix power save, locking, and scan issues in brcmfmac
- Elimimnate 'void *' casts in brcmsmac
- Fix entry for brcm80211 drivers in drivers/net/wireless/Kconfig
- Use cordic and crc8 kernel library functions
- General code cleanup, restructuring, and dead code removal
The only difference between these sources and the ones in staging-next are:
- necessary Kconfig and Makefile adjustments for the new driver location
- one cleanup of a checkpatch warning (as indicated below)
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 has been challenging).
The drivers compile cleanly for x86 (32- and 64-bit), PPC (32- and 64-bit),
SPAR, MIPS BE, MIPS LE, and ARM.
Thanks,
---
Henry Ptasinski
[email protected]
diff --git a/drivers/staging/brcm80211/brcmsmac/dma.c b/drivers/staging/brcm8021
index 05dad9f..73e5841 100644
--- a/drivers/staging/brcm80211/brcmsmac/dma.c
+++ b/drivers/staging/brcm80211/brcmsmac/dma.c
@@ -815,7 +815,7 @@ struct sk_buff *dma_rx(struct dma_pub *pub)
tail = head;
while ((resid > 0) && (p = _dma_getnextrxp(di, false))) {
tail->next = p;
- pkt_len = min(resid, (int)di->rxbufsize);
+ pkt_len = min_t(int, resid, (int)di->rxbufsize);
__skb_trim(p, pkt_len);
tail = p;
On Wed, Aug 31, 2011 at 11:33 AM, Greg KH <[email protected]> wrote:
> On Wed, Aug 31, 2011 at 10:55:45AM -0700, Luis R. Rodriguez wrote:
>> On Tue, Aug 30, 2011 at 11:14 AM, Greg KH <[email protected]> wrote:
>> > On Mon, Aug 29, 2011 at 06:42:57PM -0700, Henry Ptasinski wrote:
>> >> The brcmsmac driver has architectural alignment with our drivers for other
>> >> operating systems, and we intend to to enhance and maintain this driver in
>> >> parallel with drivers for other operating systems. Maintaining alignment
>> >> between our Linux driver and drivers for other operating systems allows us to
>> >> leverage feature and chip support across all platforms.
>> >
>> > Just curious, if you really are going to try to do this, how are you
>> > going to handle the issue when others change the in-kernel driver in
>> > ways that you are not going to be allowed to make to your "internal"
>> > copy of the driver?
>>
>> What do you mean by this?
>
> - Infrastructure changes that do not match up with how other operating
> systems handle things.
Agreed -- they gotta learn to deal with this. IBM's lesson learnt: You
cannot control Linux, only influence it.
> - support for new features
Ditto.
> - support for older devices
Ditto. But common sense helps here -- just enable the community
through proper support on b43, IMHO.
> - license incompatibilities (i.e. new files under GPL-only license
> - etc.
Agreed.
But -- I will note the typical concern that vendors *should* have is
not that the main driver code will differ from their internal code,
what is of real critical value is to ensure the software that deals
with hardware code doesn't change much to allow us to rapidly add
support for newer chipsets and also take bug fixes back. For Atheros
we've learned to deal with the fact that the only thing we can
possibly try to keep as very similar to our other drivers is what used
to be called the "HAL", and since that is open now I call it "hardware
code", and its in the ath9k_hw module. Mind you -- I still think FOSS
development even helps lead with better architectural designs here, so
if you compare our ath9k_hw with whatever we us with other drivers I
think you'll be pleased with what we've done.
Just my 0.02 CRC.
>> > Also, how are you going to handle any GPL-only changes that happen to
>> > the code as well?
>>
>> For the broadcom drivers this may be hard if people want to move GPLv2
>> b43 code to a permissively licensed driver... but ...
>>
>> > Do you have some process in place to ensure that all
>> > contributions will have the proper copyright releases on it to allow you
>> > to make the same changes to your internal versions?
>>
>> To be clear *new* code going in to a permissively licensed driver
>> follows the Developer's Certificate of Origin which does state the
>> contributor follows the file's license.
>
> For an existing file, yes.
:D
> But for new files, or for copying code from another driver (GPL only)
> into this one (dual licensed), that's different, right?
Yes :)
> You need to be very careful about this, that's all.
Agreed. I think we all stand to win if we do this properly, doing this
properly will help not only share with internal drivers but also the
BSD community and as can be seen with recent efforts by Adrian Chadd
on FreeBSD -- this helps tremendously the community. In the end what
this is proving is internal driver development sucks balls and
collaborative developments are the only real way to sustain support in
the long run for hardware. If we all work together and use common
sense I think we can figure this out.
>> The issues with a large GPLv2 code from b43 and the fact that the
>> other driver is permissively licensed makes this a bit more
>> complicated though, but that is only a reflection of not addressing
>> supporting old hardware. Ouch.
>
> Yes, this is going to be tricky :)
:)
> That is what I am worried about, but I know the lawyers and developers
> at Broadcom have already considered this, but they haven't told us how
> they are going to handle it, which is what I'm curious about.
I really don't expect them to say anything but I'm more curious about
how to address enabling the community with legacy hardware support.
That seems to me the bigger issue.
> Then there the issue that some companies want to keep their internal
> copies in a non-dual licensed codebase, to handle the license of the
> other operating systems they are working with.
To be clear, Broadcom copied Atheros' practice of using a completely
permissively licensed driver under the ISC license, which removes the
ambiguity from the Dual license. The trick to resolving the "other
operating systems" problem is to realize that the "other OSes" do not
*require* you to use *proprietary* licenses, and that *you can* use
permissive licenses as well. This is a whole can of worms in itself
but -- if done properly, I believe architecturally in the long run we
won't have to deal with shit "internal" drivers or license
incompatibilities. In the end I suspect that nice features like RCU,
and Minstrel HT support for example though, will keep innovation on
the edge on Linux though ;)
> Hopefully Broadcom isn't
> going to do this, but others have in the past (SGI with xfs, Intel with
> the ACPI core, etc.)
Good luck to them too :) I hope they copy best practices well.
Luis
On Mon, 2011-08-29 at 21:28 -0700, Greg KH wrote:
> > We understand the level of effort that it's taken for b43 to get as far as it
> > has, and was implemented without access to proprietary information, and it has
> > been impossible to support advanced chip features. We appreciate the difficulty
> > of your task and respect your accomplishments tremendously!
>
> Then why not try to help out here?
>
> All other wireless companies have realized that this is the best way
> forward over time. Remember, b43 was here first, you need to play nice
> with those developers.
Well, truth be told, b43 was there first for older chipsets, and I
played a major role in it existing back then. For the latest generation
chips that are supported by the staging driver, that driver was there
earlier and reverse engineering was ongoing even though code existed and
could have been used. That code may not have been as clean, but the
algorithms etc. are probably more tweaked & tuned to the chips.
> You really need to work with the b43 developers here.
I definitely agree with this. However, given the architectural
differences in the device/ucode combination, I don't think support can
be added to b43 easily.
> Why can't you just do the small changes needed to the b43 driver to add
> the missing functionality based on the fact that you know how the
> hardware works?
I don't think it would be small changes. Let me explain.
What we can hope for is sharing of PHY/Radio code between b43 and
brcmsmac. I'm sure this can be done in some way -- the PHY/Radio code
should be taken from Broadcom's driver most likely, the code in b43
isn't maintained by anyone with access to Si documentation & testing.
Some form of abstraction layer exists in both drivers, but they're
obviously not compatible at this point -- I believe this could be
solved.
However, on a higher level (MAC level) I believe that the drivers are
quite different. There are a bunch of MAC features like multi-MAC
support and probably soon P2P that b43 cannot support across the board
due to the version of the uCode it is tied to, especially on older
devices where such uCode never existed. New features, like P2P, will
likely only be supported by new uCode on new devices -- I don't see
Broadcom backporting & testing their new uCode, in most cases it
probably won't even be possible due to device restrictions (*).
As a result, I believe that attempting to share the MAC level code
between b43 and brcmsmac will lead to a maintenance disaster, b43 would
essentially be fragmented into lots of little code paths that depend on
the uCode API in use. I don't think that's worth it -- we split
b43/b43legacy for precisely that reason before.
I'm not saying we should merge brcmsmac as is, but I do think attempting
to push everything into b43 is bound to lead to lots of issues that we
all don't want to see. Code sharing should be possible on some level,
but at other levels it is fairly likely that code sharing would just
lead to bigger problems down the road.
johannes
(*) keep in mind that older devices only support 4k ucode instructions
and very little memory!
Hi,
On 25.8.2011, at 1.28, Henry Ptasinski wrote:
> 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 has been
> challenging).
>
> The drivers compile cleanly for x86 (32- and 64-bit), PPC (32- and
> 64-bit),
> SPAR, MIPS BE, MIPS LE, and ARM.
Are you sure the compilation is even enabled on all of those platforms?
E.g.:
> +config BRCMSMAC
> + tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
> + default n
> + depends on PCI
> + depends on WLAN && MAC80211
> + depends on X86 || MIPS
^^^^^^^^^^^^^^^^^^^^^^
Why this?
A.
On Wed, 2011-08-24 at 19:23 -0700, Greg KH wrote:
> On Wed, Aug 24, 2011 at 05:42:11PM -0700, Henry Ptasinski wrote:
> > On Wed, Aug 24, 2011 at 04:54:28PM -0700, Joe Perches wrote:
> > > On Wed, 2011-08-24 at 16:17 -0700, Henry Ptasinski wrote:
> > > > Augh. I included the wrong link. Correct link:
> > > > http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
> > >
> > > Can't you post the patch with git format-patch -M instead?
> >
> > This patch drops in a new copy of all the files, as there's been significant
> > change since the last time staging-next and wireless-testing were in sync.
> > (Deleting the existing files from staging will be a separate step.)
> >
> > I can generate a two-patch series instead: one to catch up wireless-testing
> > with all the changes that have been applied to the brcm80211 drivers in
> > staging, and then a second patch with a 'git mv' plus necessary
> > Kconfig/Makefile changes. The first patch would still be quite large, but I'm
> > ok with either approach.
> Don't worry about that now, the real thing is a review of the code.
A 2.8MB patch is _way_ too large to review.
The reviewing should take place of posted patches
to staging, not wireless-testing.
And I believe that's not an RFC patch, but the actual
patch to be applied.
If so, I think that the right way to do that is not
copy but a merge.
W dniu 30 sierpnia 2011 11:28 użytkownik Michael Büsch <[email protected]> napisał:
> On Tue, 30 Aug 2011 10:31:08 +0200
> Rafał Miłecki <[email protected]> wrote:
>
>> I got lost. You're talking a lot about ucode, but I don't know what do
>> you really mean. Do you have some additional info from Broadcom? Are
>> they going to release some new ucode not compatible with some older
>> cards?
>
> They do so frequently and always did this.
> The problem in b43 is, that b43 needs to support _all_ ucode
> APIs from 5 to x (Dunno where we are right now. somewhere > 15).
> That was relatively easy until now, because the API didn't vary a lot
> between cores. I don't know what will/did change for latest wireless
> features, though. But you already applied some changes to the affecting
> code. I don't know how complete that stuff is.
But it sounds pretty, well, stupid, to split Broadcom support into 2
drivers basing on SSB vs. BCMA architecture, if this is just a pure
guess that Broadcom is going to release updated firmware for BCMA
devices only.
The split done by Broadcom could be done anywhere, it could be HT vs.
LCN or LCN vs. LCNXN, we never know what will happen. Let's first wait
for Broadcom to release new firmware not compatible with older
devices, then we can think about any driver split.
--
Rafał
On 08/25/2011 03:55 PM, Rafał Miłecki wrote:
> 2011/8/25 Henry Ptasinski<[email protected]>:
>> With the latest series of cleanup patches merged in by Greg KH, I'd like to
>> once again propose moving brcm80211 out of staging and into mainline.
>
> Henry: a simple question, please explain it to me, what brcmsmac does
> provide that b43 doesn't?
>
> brcmsmac doesn't support SSB and duplicates BCMA support (doesn't make
> use of bcma module)
> brmcsmac doesn't support older PHYs like G/LP
> brmcsmac doesn't support recent HT-PHY
> brmcsmac doesn't support AP or monitor mode
> all of that is supported by b43
>
> The one thing brcmsmac is better at, is support for 14e4:4727 card
> (BCM4313). I've already started work on support of this card in b43,
> hope to get it working soon.
>
>
> You mostly just duplicate support for cards that are already supported by b43.
I am certainly looking forward to the answer to this question.
My %0.02 contribution: When brcmsmac (or its predecessor) first appeared, b43
supported none of the newest Broadcom hardware. Since then, Rafał has done a
super job of extending b43, and as he pointed out, the community driver is
arguably better than the one contributed by Broadcom.
I would like to see the Broadcom engineers concentrate on two things: (1) adding
their new devices to b43, and (2) try to get their lawyers to allow the
community to redistribute their firmware. It would also be nice if they opened
the wl source. I cannot imagine many secrets contained within. Those changes
would overcome a lot of the bad will that has been built up in the community.
There are many users that reject a particular model of computer merely because
it has a Broadcom device included.
Larry
2011/8/25 Jonas Gorski <[email protected]>:
> Hi,
>
> On 25 August 2011 02:20, Henry Ptasinski <[email protected]> wrote:
>> On Wed, Aug 24, 2011 at 04:41:54PM -0700, Jonas Gorski wrote:
>>> Hi Henry,
>>>
>>> On 25 August 2011 00:28, Henry Ptasinski <[email protected]> wrote:
>>> > With the latest series of cleanup patches merged in by Greg KH, I'd like to
>>> > once again propose moving brcm80211 out of staging and into mainline.
>>>
>>> While I like the Idea of brcm80211 going mainline, I'd like to throw
>>> in the suggestion that brcm80211 should be made a bcma/ssb driver
>>> first (AFACT brcmfmac would use ssb, not bcma, therefore both).
>>>
>>> My reasoning is that it needs to be done eventually anyway, and the
>>> earlier this is done the less work it will be in the long term, also
>>> it would reduce the duplicate code in bcma, ssb, and brcm80211.
>>>
>>> Of course this is just a suggestion, and it's yours and Greg's call
>>> whether you agree with me or not (since it's quite late in the game to
>>> add a new TODO, and I suspect a rather big one).
>>
>> We started converting brcmsmac to bcma, but bcma is evolving rapidly in the
>> wireless-testing tree. Since wireless-testing and staging-next only get in
>> sync during a kernel merge, the version of bcma we have to work with in staging
>> is usually quit outdated. Unless Greg and John want to come up with a process
>> for keeping bcma consistent between their two repos, I don't really see how we
>> can productively use bcma until we cross over. We do intend to switch to using
>> bcma as soon as possible.
>
> Okay, then no objections from me. The keeping in sync is a valid
> reason. I'm looking forward to seeing your bcma patches :-)
>
>> I believe the only SB bus functions that brcmfmac uses are the core reset and
>> disable functions, and only when initializing the chip to download firmware
>> (all other management of the bus is handled by the on-chip CPU). Is it
>> possible to use those funtions from ssb, without the ssb module trying to
>> manage the bus?
>
> I haven't really looked at how much the brcmfmac driver uses ssb; I
> just saw s(s)b_* stuff in there and remembered that ssb supports SDIO
> host, so I assumed that there's part of the stack hidden in brcmfmac.
> But if it's only core reset and disable, then what Michael said
> applies ;-)
Still, is there any good reason for duplicating that code?
I think that it's not only about duplicating enable/disable/reset.
What about managing sliding window? Reading available cores? Clocks
management? Requesting & releasing device? Interrupts management?
About the code quality, short question: is the duplication of the code
done on purpose between dhd_siod.c and bcmsdh.c? Two identical
functions:
brcmf_sdcard_set_sbaddr_window
brcmf_sdbrcm_set_siaddr_window
--
Rafał
On Sat, Aug 27, 2011 at 05:08:26PM +0200, Rafał Miłecki wrote:
> 2011/8/27 Greg KH <[email protected]>:
> > On Sat, Aug 27, 2011 at 05:35:13PM +0300, Dan Carpenter wrote:
> >> On Thu, Aug 25, 2011 at 10:55:26PM +0200, Rafał Miłecki wrote:
> >> > 2011/8/25 Henry Ptasinski <[email protected]>:
> >> > > With the latest series of cleanup patches merged in by Greg KH, I'd like to
> >> > > once again propose moving brcm80211 out of staging and into mainline.
> >> >
> >> > Henry: a simple question, please explain it to me, what brcmsmac does
> >> > provide that b43 doesn't?
> >> >
> >>
> >> Wow. Why are we only having this discussion now? Somewhere along
> >> the line, there has been a massive communications failure. What
> >> happened here? Henry, did you know about the b43 driver? Can
> >> someone explain what's going on?
> >
> > I always thought that b43 and the staging driver supported different
> > devices and had no overlap, which is why I had no problem with it.
> >
> > Was I totally mistaken and got this wrong?
>
> Please see my last e-mail in this thread for history overview.
Thanks for this, I appreciate it, and it confirms that when the driver
was originally added, there was no duplication.
> Right now the only card that is supported by brcmsmac and is not
> supported by b43 is 14e4:4727. That card is based on PHY type LCN, and
> I'm working on adding support for it in b43.
That sounds good.
> Other cards supported by brcmsmac (14e4:4353, 14e4:4357) are supported
> by b43. They still need some work (no HT support, but I'm not sure if
> even brcmsmac has support for it), but are usable and seem to work
> stable. Plus b43 supports some feature like monitor mode, etc. on that
> cards.
Very nice.
> I've also crated comparison of driver few days ago:
> http://wireless.kernel.org/en/users/Drivers/b43#Comparison_of_recent_drivers
> It doesn't cover cards by pci ids, but gives some overview.
Ok, we don't want/accept duplicate drivers for the same devices (well, I
sure don't want that, we had it in the past in the USB subsystem and it
was a nightmare).
So, as b43 was here first, it looks like brcmfmac is the only part that
should really move out of staging, right?
Henry, thoughts? Have you all been tracking the b43 support for the
past year?
greg k-h
On Thu, Aug 25, 2011 at 02:09:47PM -0700, Rafał Miłecki wrote:
> Also what about embedded devices? Shouldn't we have core drivers for
> them? I mean, to be able to easily choose driver for a given core?
>
> Let's say I want to use some ssb-core-based driver for Ethernet port
> (like gige) but I also want to use brcmsmac fo 80211. Is that possible
> without brcmsmac using bcma?
Since brcmsmac doesn't attempt to claim any ssb-based devices, why would there
be any conflict between an ssb-based Ethernet device and a brcmsmac device?
The ssb driver claims the ssb-based devices, and brcmsmac only claims the
AI-based chips that it supports.
There is currently a conflict between bcma and brcmsmac (hence the restriction
in drivers/staging/brcm80211/Kconfig), but, as I've said several times, we want
to move to using bcma as soon as possible. But we need to work with the
current version of bcma, which isn't available in staging.
- Henry
2011/8/30 Johannes Berg <[email protected]>:
> On Mon, 2011-08-29 at 21:28 -0700, Greg KH wrote:
>
>> > We understand the level of effort that it's taken for b43 to get as far as it
>> > has, and was implemented without access to proprietary information, and it has
>> > been impossible to support advanced chip features. We appreciate the difficulty
>> > of your task and respect your accomplishments tremendously!
>>
>> Then why not try to help out here?
>>
>> All other wireless companies have realized that this is the best way
>> forward over time. Remember, b43 was here first, you need to play nice
>> with those developers.
>
> Well, truth be told, b43 was there first for older chipsets, and I
> played a major role in it existing back then. For the latest generation
> chips that are supported by the staging driver, that driver was there
> earlier and reverse engineering was ongoing even though code existed and
> could have been used. That code may not have been as clean, but the
> algorithms etc. are probably more tweaked & tuned to the chips.
That's tricky. We started implementing N-PHY code long before
brcm80211. When it was almost ready, Broadcom released brcm80211. Then
we released our version.
The interesting fast is however we duplicated N-PHY code without
supporting the same cards. b43 supported only SSB-based N-PHY cards,
while brcmsmac supported BCMA based N-PHY cards.
When we added BCMA support in b43, we automatically gained support for
BCMA & N-PHY devices like BCM43224 and BCM43225.
If you want to tell, which driver has support for 14e4:xyzq, then yes,
it can be brcmsmac. Which driver has first support for N-PHY? Both.
b43 got ~90% when brcm80211 was released.
>> You really need to work with the b43 developers here.
>
> I definitely agree with this. However, given the architectural
> differences in the device/ucode combination, I don't think support can
> be added to b43 easily.
>
>> Why can't you just do the small changes needed to the b43 driver to add
>> the missing functionality based on the fact that you know how the
>> hardware works?
>
> I don't think it would be small changes. Let me explain.
>
> What we can hope for is sharing of PHY/Radio code between b43 and
> brcmsmac. I'm sure this can be done in some way -- the PHY/Radio code
> should be taken from Broadcom's driver most likely, the code in b43
> isn't maintained by anyone with access to Si documentation & testing.
> Some form of abstraction layer exists in both drivers, but they're
> obviously not compatible at this point -- I believe this could be
> solved.
>
> However, on a higher level (MAC level) I believe that the drivers are
> quite different. There are a bunch of MAC features like multi-MAC
> support and probably soon P2P that b43 cannot support across the board
> due to the version of the uCode it is tied to, especially on older
> devices where such uCode never existed. New features, like P2P, will
> likely only be supported by new uCode on new devices -- I don't see
> Broadcom backporting & testing their new uCode, in most cases it
> probably won't even be possible due to device restrictions (*).
>
> As a result, I believe that attempting to share the MAC level code
> between b43 and brcmsmac will lead to a maintenance disaster, b43 would
> essentially be fragmented into lots of little code paths that depend on
> the uCode API in use. I don't think that's worth it -- we split
> b43/b43legacy for precisely that reason before.
>
> I'm not saying we should merge brcmsmac as is, but I do think attempting
> to push everything into b43 is bound to lead to lots of issues that we
> all don't want to see. Code sharing should be possible on some level,
> but at other levels it is fairly likely that code sharing would just
> lead to bigger problems down the road.
I got lost. You're talking a lot about ucode, but I don't know what do
you really mean. Do you have some additional info from Broadcom? Are
they going to release some new ucode not compatible with some older
cards?
At the moment b43 supports the newest known ucode which is 666.2.
brcmsmac uses older version 610.811.
--
Rafał
On Wed, 24 Aug 2011 17:20:33 -0700
"Henry Ptasinski" <[email protected]> wrote:
> I believe the only SB bus functions that brcmfmac uses are the core reset and
> disable functions, and only when initializing the chip to download firmware
> (all other management of the bus is handled by the on-chip CPU). Is it
> possible to use those funtions from ssb, without the ssb module trying to
> manage the bus?
If that is really the case, why do we care about linking to ssb anyway?
Those functions are less than 100 lines of code. Just copy and paste them.
There's no point in making them work without the ssb core and then linking
the whole unused ssb core to the driver.
--
Greetings, Michael.
On Wed, Aug 24, 2011 at 04:17:46PM -0700, Henry Ptasinski wrote:
> Augh. I included the wrong link. Correct link:
>
> http://linuxwireless.org/en/users/Drivers/brcm80211?action=AttachFile&do=get&target=0001-wireless-testing-add-brcm80211-v2.patch
>
> (Note the '-v2'.)
>
> Very sorry about that.
Ah, much better, nevermind about my comments.
It looks checkpatch clean to me, I'll leave it up to the wireless
network developers to review this now. I have no objections to this
moving if they don't.
thanks,
greg k-h
Henry, you ignored most of the questions and explanations. In this
situation I don't have anything more to add, I'll just post few
comments/updates.
W dniu 30 sierpnia 2011 08:17 użytkownik Rafał Miłecki
<[email protected]> napisał:
> 2011/8/30 Henry Ptasinski <[email protected]>:
>> The b43 driver doesn't implement transmit power control for the BCM43224 or
>> BCM43225, and has various other unimplemented phy functions, e.g.:
>>
>>> void b43_nphy_set_rxantenna(struct b43_wldev *dev, int antenna)
>>> {//TODO
>>> }
>>>
>>> static void b43_nphy_op_adjust_txpower(struct b43_wldev *dev)
>>> {//TODO
>>> }
>>>
>>> static enum b43_txpwr_result b43_nphy_op_recalc_txpower(struct b43_wldev *dev,
>>> bool ignore_tssi)
>>> {//TODO
>>> return B43_TXPWR_RES_DONE;
>>> }
>>>
>>> ...
>>> b43err(dev->wl, "enabling tx pwr ctrl not implemented yet\n"); ...
>>> ...
>>
>> etc.
>
> I've just checked your wlc_phy_txpower_recalc_target_nphy. It's calling:
> 1) Trivial wlc_phy_txpwr_limit_to_tbl_nphy
> 2) More advanced wlc_phy_txpwrctrl_pwr_setup_nphy we have to implement
> 3) Already-implemented-in-b43 wlapi_bmac_mctrl
> 4) 50% implemented wlc_phy_txpwrctrl_enable_nphy
>
> I can do this in 1 day.
If you watch linux-wireless ML, you could see I've started
implementing calibration for N-PHY in b43.
A progress is little slower recently because of:
1) Me having less time in few recent days
2) Problems with kernel.org servers
In any case, *I'm going to finish N-PHY calibration* in b43. I've more
stuff already, just need to test it before posting and make sure
nothing was missed due to problems with git.kernel.org.
I've also finished initialization of static tables for LCN-PHY and
implemented basic operations. Now I'm just going to rewrite other
initialization ops and that should give us working LCN support.
I'm moving to the new place in next week (that's why I've been busy
recently) and will start playing with 5 GHz support in b43.
Too bad you didn't respond to all our questions and comments :| Ppl
started commenting that [0] without seeing the real solution I wanted
to see discussed with you.
[0] http://www.h-online.com/open/features/Kernel-comment-The-obstacle-course-of-cooperation-1340020.html
--
Rafał
PiBGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWlsdG86am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldF0N
Cj4gU2VudDogZG9uZGVyZGFnIDI1IGF1Z3VzdHVzIDIwMTEgNzowMw0KPiANCj4gPiArI2RlZmlu
ZSBMT0NLKHdsKQlzcGluX2xvY2tfYmgoJih3bCktPmxvY2spDQo+IA0KPiBObyB3YXkgLi4uDQo+
IA0KDQpIaSBKb2hhbm5lcywNCg0KWW91ciBjb21tZW50IGlzIGEgYml0IG9wZW4gZm9yIGludGVy
cHJldGF0aW9uIChvciBub3QgOy0pICkuIENvdWxkIHlvdSBlbGFib3JhdGUgb24gdGhpcyBvbmU/
DQoNCkdyLiBBdlMNCg==
On Fri, Sep 30, 2011 at 2:54 PM, Arend Van Spriel <[email protected]> wrote:
>> From: Johannes Berg [mailto:[email protected]]
>> Sent: donderdag 25 augustus 2011 7:03
>>
>> > +#define LOCK(wl) spin_lock_bh(&(wl)->lock)
>>
>> No way ...
>>
>
> Hi Johannes,
>
> Your comment is a bit open for interpretation (or not ;-) ). Could you elaborate on this one?
Dude, just kill the define.
Luis