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 --
| Greg Kroah-Hartman | [PATCH 027/196] tifm: Convert from class_device to device for TI flash media |
| Kok, Auke | Re: Linux 2.6.21-rc1 |
| Trent Piepho | Re: [PATCH] [POWERPC] Improve (in|out)_beXX() asm code |
| Greg KH | Re: Dual-Licensing Linux Kernel with GPL V2 and GPL V3 |
git: | |
| Gerrit Renker | [PATCH 27/37] dccp: Integration of dynamic feature activation - part 2 (server side) |
| Arjan van de Ven | Re: [GIT]: Networking |
| Ingo Molnar | Re: [PATCH 01/10] x86: add Kconfig entry for DMA-API debugging |
| Jarek Poplawski | [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock(). |
