Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756581AbYGCWWv (ORCPT ); Thu, 3 Jul 2008 18:22:51 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754214AbYGCWWj (ORCPT ); Thu, 3 Jul 2008 18:22:39 -0400 Received: from srv5.dvmed.net ([207.36.208.214]:57364 "EHLO mail.dvmed.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754216AbYGCWWi (ORCPT ); Thu, 3 Jul 2008 18:22:38 -0400 Message-ID: <486D511A.9020405@garzik.org> Date: Thu, 03 Jul 2008 18:22:18 -0400 From: Jeff Garzik User-Agent: Thunderbird 2.0.0.14 (X11/20080501) MIME-Version: 1.0 To: David Woodhouse CC: Theodore Tso , Hugh Dickins , Andrew Morton , KOSAKI Motohiro , mchan@broadcom.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin" References: <20080703020236.adaa51fa.akpm@linux-foundation.org> <20080703205548.D6E5.KOSAKI.MOTOHIRO@jp.fujitsu.com> <486CC440.9030909@garzik.org> <486CCFED.7010308@garzik.org> <1215091999.10393.556.camel@pmac.infradead.org> <486CD654.4020605@garzik.org> <1215093175.10393.567.camel@pmac.infradead.org> <20080703173040.GB30506@mit.edu> <1215111362.10393.651.camel@pmac.infradead.org> <486D3E88.9090900@garzik.org> <486D4596.60005@infradead.org> In-Reply-To: <486D4596.60005@infradead.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -4.3 (----) X-Spam-Report: SpamAssassin version 3.2.4 on srv5.dvmed.net summary: Content analysis details: (-4.3 points, 5.0 required) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4452 Lines: 101 David Woodhouse wrote: > Jeff Garzik wrote: >> David Woodhouse wrote: >>> 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. >> >> IMO, individual options would be better. > > They had individual options for a long time, but the consensus was that > I should remove them -- a consensus which was probably right. It was > moderately inconvenient going back through it all and recommitting it > without that, but it was worth it to get it right... > >> Plus, unless I am misunderstanding, the firmware is getting built into >> the kernel image not the tg3 module? > > That's right, although it doesn't really matter when they're both in the > vmlinux. > > When it's actually a module, there really is no good reason not to let > request_firmware() get satisfied from userspace. If you can load > modules, then you can load firmware too -- the required udev stuff has > been there as standard for a _long_ time, as most modern drivers > _require_ it without even giving you the built-in-firmware option at all. > > It makes no more sense to object to that than it does to object to the > module depending on _other_ modules. Both those other modules, and the > required firmware, are _installed_ by the kernel Makefiles, after all. > > It wouldn't be _impossible_ to put firmware blobs into the foo.ko files > themselves and find them there. The firmware blobs in the kernel are > done in a separate section (like initcalls, exceptions tables, pci > fixups, and a bunch of other stuff). It'd just take some work in > module.c to link them into a global list, and some locking shenanigans > in the lookups (and lifetime issues to think about). But it just isn't > worth the added complexity, given that userspace is known to be alive > and working. It's pointless not to just use request_firmware() normally, > from a module. It is pointless -- if you assume everybody wants to run their distro and universe your way. If a firmware is built-in, then 'make firmware_install' is clearly optional and may be omitted. That invalidates several of your assumptions above. Further, all current kernel build and test etc. scripts are unaware of 'make firmware_install', and it is unfair to everybody to force a flag-day build process change on people, just to keep their drivers in the same working state today as it was yesterday. Conclusion - kernel build process must produce a working driver after your changes are applied, even in absence of 'make firmware_install'. That does not, repeat /not/ exclude the desired end goal of course -- separating the firmware from the driver, installing in /lib/firmware via 'make firmware_install', etc. I support your end goal, I really do. But I continue to feel there is a lack of regard for breakage and regressions. You are either ignoring or apparently just not seeing * how many new ways this can produce a non-working driver * how important it is in this specific case to fail-safe, and avoid a broken driver at all costs. As a concrete example, in the above quoted text you assume that a user will never copy kernel modules around. I can tell you that, with tg3.ko being nice and self-contained, yes it does get copied around (to multiple machines, etc.). With the firmware newly separated from tg3.ko, you have introduced breakage for any user that is unaware of this new requirement (kernel module == requires additional file now). Scripts that build install disks must be updated, otherwise a script that builds a boot image will include the drivers it knows about, but /not/ include the crucial firmware. None of this stuff is "pointless", none of this stuff may be dismissed as "making no sense". All these are real world examples where users FOLLOWING THEIR NORMAL, PROSCRIBED KERNEL PROCESSES will produce non-working drivers. The only valid assumption here is to assume that the user is /unaware/ of these new steps they must take in order to continue to have a working system. Regards, Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/