On Thu, 2008-07-03 at 13:30 -0400, Theodore Tso wrote:Neither is it an adequate review of the actual patch which was submitted. They had to 'make oldconfig' and then actually _choose_ to say 'no' to an option which is fairly clearly documented, that they are the relatively unusual position of wanting to have said 'yes' to. You're getting into Aunt Tillie territory, when you complain about that. Although it does make me wonder if it was better the way I had it originally, with individual options like TIGON3_FIRMWARE_IN_KERNEL attached to each driver, rather than a single FIRMWARE_IN_KERNEL option which controls them all. Perhaps one way to help Aunt Tillie would be to tweak Kbuild to look at the MODULE_FIRMWARE() statements for in-kernel drivers, and to print a warning when the build finishes: "Your static kernel image may require the following firmware files, which are not included: ..." It's wrong to change the CONFIG_FIRMWARE_IN_KERNEL default to 'Y', because the _normal_ setting for that option _really_ should be 'N'. Using request_firmware() satisfied from userspace is best practice these days, and almost all recent drivers do it that way _unconditionally_ anyway. What we're doing now is just cleaning up the older drivers which don't use request_firmware(), to conform to what is now common practice. And while we're retaining the _option_ to continue to build their firmware into the static kernel image, it isn't recommended and really shouldn't be the default configuration. I am content to let Linus decide on what the default for the FIRMWARE_IN_KERNEL option will be. I am adamant that it _should_ be 'N', but it's easy enough for Linus to overrule me with a one-line change. In the meantime, it would be useful if Jeff would quit throwing his toys out of the pram on that issue and actually review the _code_ changes. In particular, are the reports correct that the device operates just fine without the TSO firmware loaded? Should we change the request_firmware() error path to just disable TSO and continue with the initialisation? I can understand why he might not want to answer that if the answer is affirmative, I suppose -- it detracts even _further_ from his already rather dubious argument about 'breaking' the driver, if it'll actually continue to work even when the firmware is completely absent. But it would be nice to get an honest and straightforward review of the code from _someone_ who actually knows the hardware. Less of the ad hominem, please. Especially when it's so misdirected. Updating these drivers to remove large blobs of static unswappable data from the kernel, and having it provided from userspace on demand as modern Linux drivers do, is a perfectly sensible technical goal all on its own. And given the GPL's explicit provisions with regard to collective works there are also entirely reasonable, non-"fundamentalist" grounds for believing that it _may_ pose a licensing problem, and for wanting to err on the side of caution in that respect too. Fedora is almost certain to ship with CONFIG_FIRMWARE_IN_KERNEL=n, and I'd be very surprised if Debian and other major distributions don't follow suit. It is the sensible, pragmatic, technically sound choice. By this argument, shouldn't we include images in the static kernel for _all_ drivers which currently use request_firmware()? Otherwise, it's possible for the user to 'break' them, right? -- dwmw2 --
| Avi Kivity | [PATCH 09/58] KVM: MMU: Respect nonpae pagetable quadrant when zapping ptes |
| Andrew Morton | 2.6.25-rc2-mm1 |
| James Morris | Re: LSM conversion to static interface |
| Eric W. Biederman | Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu |
git: | |
| David Miller | Re: 2.6.25-rc8: FTP transfer errors |
| Gerrit Renker | [PATCH 27/37] dccp: Integration of dynamic feature activation - part 2 (server side) |
| David Miller | Re: [GIT *] Solos PCI ADSL card update |
| David Miller | Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock(). |
