Bug 490892
Summary: | Review Request: b43-openfwwf - Open FirmWare for Broadcom 43xx series WLAN chip | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Peter Lemenkov <lemenkov> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | 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
Peter, we need to get this reviewed as it's an important package in my personal opinion. I'll try and rally the troops. 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 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. *** Bug 508320 has been marked as a duplicate of this bug. *** 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. (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. (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. 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 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. (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? Legal has been poked, via Blocks: FE-Legal above. Lifting FE-Legal, everything seems okay here. Thanks, spot. APPROVED 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: cvs done. 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 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 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 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 Awesome work! Looks like the "s" is missing from the end of the summary. 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. 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. (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. |