Bug 490892

Summary: Review Request: b43-openfwwf - Open FirmWare for Broadcom 43xx series WLAN chip
Product: [Fedora] Fedora Reporter: Peter Lemenkov <lemenkov>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bruno, dave, fedora-package-review, jason294, notting, pahan, pbrobinson, rhughes, sebastian, steve, tcallawa
Target Milestone: ---Flags: j: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 5.1-3.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-07-22 21:49:04 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 490638    
Bug Blocks:    

Description Peter Lemenkov 2009-03-18 13:57:31 UTC
Spec URL: http://peter.fedorapeople.org/openfwwf.spec
SRPM URL: http://peter.fedorapeople.org/openfwwf-5.1-1.fc10.src.rpm
Description: Open source firmware for Broadcom/AirForce chipset based devices.

Comment 1 Richard Hughes 2009-06-27 08:01:55 UTC
Peter, we need to get this reviewed as it's an important package in my personal opinion. I'll try and rally the troops.

Comment 2 Peter Lemenkov 2009-06-27 08:29:46 UTC
I also think so, Richard :)

BTW I think that package should be renamed into b43-openfwwf. What you (and
other people) think about it?


http://peter.fedorapeople.org/b43-openfwwf.spec
http://peter.fedorapeople.org/b43-openfwwf-5.1-2.fc11.src.rpm

Also I added config-file for proper loading b43-module with this firmware

Comment 3 Richard Hughes 2009-06-27 08:34:58 UTC
Are you sure you need the qos=0 parameter? I thought newer kernels disabled it by default if the free firmware was loaded?

Anyway, I much prefer the b43-openfwwf name.

Comment 4 Peter Lemenkov 2009-06-27 15:33:10 UTC
*** Bug 508320 has been marked as a duplicate of this bug. ***

Comment 5 Jason Tibbitts 2009-06-27 19:51:53 UTC
So, here's my primary concern.  The upstream web site (and hence the README file in the package) says that you need to obtain two files from the proprietary driver, yet those files are present in the package, which makes one worry.  Now, it sure looks to me as if those files are assembled from initvals.asm, which has a proper GPL header and contains plenty of comments and such which indicates that it's not simply made from dumping those two files from the proprietary driver.  However, I think it would be good to get an ack from the legal folks to be absolutely sure this is acceptable.  I know we have plenty of instances of random binary dumps appearing in drivers (nouveau in particular) but I don't know if there are legal restrictions on the methods for obtaining those.

How does this package get along with firmware that folks might have installed by other means (b43-fwcutter)?

Now, down to packaging.  This builds fine; rpmlint says:
  b43-openfwwf.noarch: W: non-conffile-in-etc /etc/modprobe.d/openfwwf.conf
which I believe is normal for files in modprobe.d (save any local.conf file).

I would suggest a summary of:
  Open firmware for some Broadcom 43xx series WLAN chips
Adding "some" to avoid the impression that this supports all chips, and pluralizing "chips".  At least, I don't think this actually supports every 43xx chip yet.

If the chipset support is as limited as I think it is, I would consider listing out the supported chips in the %description.  I wouldn't do this if the list is long or if most users would be supported out of the box, but otherwise it would be nice to make it easy for users to know that chipset support is limited.

* source files match upstream.  sha256sum:                         
  d0bf9dbf17255a0c465afb1ff686648a1f6ce4d534f333f06c8f3a472ec18288  openfwwf-
  5.1.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.                                                        
* build root is OK.                                                           
* license field matches the actual license.                                   
* license is open source-compatible.                                          
* license text included in package.                                           
* latest version is being packaged.                                           
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   b43-openfwwf = 5.1-2.fc12
  =
   module-init-tools
   udev

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Comment 6 Peter Lemenkov 2009-06-27 20:18:39 UTC
(In reply to comment #5)
> So, here's my primary concern.  The upstream web site (and hence the README
> file in the package) says that you need to obtain two files from the
> proprietary driver, yet those files are present in the package, which makes one
> worry. 

It's a leftover - they started to provide these files (which contains initial values) few months ago. I'll remove this outdated piece of text from README. 

> How does this package get along with firmware that folks might have installed
> by other means (b43-fwcutter)?

It will definitely overwrite some files, dumped from proprietary blobs. So they are mutually exclusive.

> I would suggest a summary of:
>   Open firmware for some Broadcom 43xx series WLAN chips

Ok. I'll fix it.

> If the chipset support is as limited as I think it is, I would consider listing
> out the supported chips in the %description.  

OK. I'll try to find list of supported chips.

Comment 7 Peter Lemenkov 2009-06-27 20:22:03 UTC
(In reply to comment #3)
> Are you sure you need the qos=0 parameter? I thought newer kernels disabled it
> by default if the free firmware was loaded?

I'm afraid, that we still need to provide config-file for modprobe, since only kernels from 2.6.30 (or even kernels from particular branches) suport this feature.

Comment 8 Peter Lemenkov 2009-06-29 10:46:18 UTC
http://peter.fedorapeople.org/b43-openfwwf.spec
http://peter.fedorapeople.org/b43-openfwwf-5.1-3.fc11.src.rpm

%changelog
* Mon Jun 29 2009 Peter Lemenkov <lemenkov> 5.1-3
- Changed README a lot
- Changed description

Comment 9 Jason Tibbitts 2009-06-29 21:50:52 UTC
I'd say this looks fine.  At this point all I'd like to see is an ack from legal that they're OK with this, and then I think we can go ahead.

Comment 10 Richard Hughes 2009-06-30 07:38:31 UTC
(In reply to comment #9)
> At this point all I'd like to see is an ack from
> legal that they're OK with this, and then I think we can go ahead.  

Great, thanks for the review. Do we need to poke legal, or do they find us?

Comment 11 Jason Tibbitts 2009-06-30 15:24:35 UTC
Legal has been poked, via Blocks: FE-Legal above.

Comment 12 Tom "spot" Callaway 2009-07-06 14:45:13 UTC
Lifting FE-Legal, everything seems okay here.

Comment 13 Jason Tibbitts 2009-07-06 14:59:53 UTC
Thanks, spot.

APPROVED

Comment 14 Peter Lemenkov 2009-07-06 15:04:09 UTC
Great! Thanks, Jason.

New Package CVS Request
=======================
Package Name: b43-openfwwf
Short Description: Open firmware for some Broadcom 43xx series WLAN chip
Owners: peter
Branches: F-10 F-11
InitialCC:

Comment 15 Kevin Fenzi 2009-07-07 04:15:34 UTC
cvs done.

Comment 16 Fedora Update System 2009-07-07 05:59:05 UTC
b43-openfwwf-5.1-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/b43-openfwwf-5.1-3.fc11

Comment 17 Fedora Update System 2009-07-07 05:59:10 UTC
b43-openfwwf-5.1-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/b43-openfwwf-5.1-3.fc10

Comment 18 Fedora Update System 2009-07-11 17:00:26 UTC
b43-openfwwf-5.1-3.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update b43-openfwwf'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-7433

Comment 19 Fedora Update System 2009-07-11 17:21:29 UTC
b43-openfwwf-5.1-3.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update b43-openfwwf'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-7484

Comment 20 Jason Woofenden 2009-07-12 02:29:19 UTC
Awesome work!

Looks like the "s" is missing from the end of the summary.

Comment 21 Fedora Update System 2009-07-22 21:48:57 UTC
b43-openfwwf-5.1-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2009-07-22 22:08:45 UTC
b43-openfwwf-5.1-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Peter Lemenkov 2009-10-11 18:47:13 UTC
(In reply to comment #20)
> Awesome work!
> 
> Looks like the "s" is missing from the end of the summary.  

Jason, sorry for missing your comment. I'll fix this typo in the next release.