[Bug 490892] Review Request: b43-openfwwf - Open FirmWare for Broadcom 43xx series WLAN chip

bugzilla at redhat.com bugzilla at redhat.com
Sat Jun 27 19:51:55 UTC 2009


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=490892


Jason Tibbitts <tibbs at math.uh.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |182235(FE-Legal)
            Summary|Review Request: openfwwf -  |Review Request:
                   |Open FirmWare for Broadcom  |b43-openfwwf - Open
                   |43xx series WLAN chip       |FirmWare for Broadcom 43xx
                   |                            |series WLAN chip




--- Comment #5 from Jason Tibbitts <tibbs at math.uh.edu>  2009-06-27 15:51:53 EDT ---
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.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.




More information about the Fedora-package-review mailing list