Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757826AbbEVSVf (ORCPT ); Fri, 22 May 2015 14:21:35 -0400 Received: from cantor2.suse.de ([195.135.220.15]:51989 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757254AbbEVSTb (ORCPT ); Fri, 22 May 2015 14:19:31 -0400 Date: Fri, 22 May 2015 20:19:24 +0200 From: "Luis R. Rodriguez" To: Dmitry Torokhov Cc: Takashi Iwai , Paul Bolle , Geert Uytterhoeven , Borislav Petkov , Greg KH , "David S. Miller" , clemens@ladisch.de, JBottomley@odin.com, David Airlie , Mauro Carvalho Chehab , Herbert Xu , Marcel Holtmann , "Gustavo F. Padovan" , Johan Hedberg , Mikael Starvik , Jesper Nilsson , Imre Kaloz , khalasa@piap.pl, Ohad Ben-Cohen , Arnd Bergmann , 3chas3@gmail.com, Jiri Slaby , Bryan Wu , Richard Purdie , Jacek Anaszewski , mcgrof@do-not-panic.com, "linux-kernel@vger.kernel.org" Subject: Re: [RFC v1] tree-wide: remove "select FW_LOADER" uses Message-ID: <20150522181924.GN23057@wotan.suse.de> References: <1432241149-8762-1-git-send-email-mcgrof@do-not-panic.com> <20150521222129.GI3689@pd.tnic> <20150522065346.GA23022@pd.tnic> <1432282668.27695.24.camel@x220> <20150522175711.GE40101@dtor-ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150522175711.GE40101@dtor-ws> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4939 Lines: 97 On Fri, May 22, 2015 at 10:57:11AM -0700, Dmitry Torokhov wrote: > On Fri, May 22, 2015 at 10:43:12AM -0700, Luis R. Rodriguez wrote: > > On Fri, May 22, 2015 at 1:44 AM, Takashi Iwai wrote: > > > At Fri, 22 May 2015 10:17:48 +0200, > > > Paul Bolle wrote: > > >> > > >> On Fri, 2015-05-22 at 09:11 +0200, Geert Uytterhoeven wrote: > > >> > On Fri, May 22, 2015 at 8:53 AM, Borislav Petkov wrote: > > >> > > One thing I forgot last night: what about randconfigs? All that > > >> > > functionality which selects FW_LOADER, won't boot anymore, right? I > > >> > > mean, there are provisions to build fine even with FW_LOADER unset but > > >> > > if you want to boot-test those kernels, you will artificially fail due > > >> > > to missing request_firmware* things... > > >> > > >> Luis also tried to explain to me that disabling FW_LOADER shouldn't make > > >> the build fail. (And, of course, we could decide to not care about > > >> randconfig builds that have EXPERT set. Maybe we could even special case > > >> EXPERT in randconfig. But that would make randconfig builds less useful. > > >> That's a separate issue, anyhow.) > > > > > > But FW_LOADER is a tristate, so it might be inconsistent if selected > > > randomly? Luis' patch doesn't add depends but just removes select. > > > > We could go both ways, either remove the "select" or replace it with > > "depends on". As you note keeping the "depends on" ensures run time > > sanity for the possible tristate mismatches, but this is an EXPERT > > concern. The crux of what option to go with is: > > > > Should we concern ourselves with run time configuration issues when > > folks enable EXPERT? > > Yes. > > dtor@dtor-ws:~/kernel/master$ grep -r CONFIG_EXPERT /boot/config* > /boot/config-3.13.0-49-generic:CONFIG_EXPERT=y > /boot/config-3.13.0-52-generic:CONFIG_EXPERT=y > > This is distro config and that is what many people use as a base for > their own configs. Smells Ubuntu'ish, and hence ChromeOS'ish :) Either way SUSE kernels also have EXPERT=y as well. Would not be surprised if other major distributions also have EXPERT=y. OK so we obviously care about EXPERT run time issues then, seems to kind of defeat the purpose of EXPERT though, no? Makes me wonder what major options are under EXPERT which most distros need. Also, are there no other runtime configuration options tucked under EXPERT that we *do not* resolve right now? Or should all be taken care of? If not then we are being inconsistent. Both of these are side topics -- but since I have a feeling this might come up again, it may be worth evaluation. Following on topic: such distro configs would never have FW_LOADER as n or m, though right? Is that sufficient for us to drop the select and not apply the "depends on" replacement ? Or do we want to stick to the "depend on"? Note that not changing this means we *will* eventually run into the recursive dependency issue later, either with my FW API change patches or some other future feature. Likewise for any other kconfig option with similar semantics. > > Without EXPERT all run time configurations are vetted to run as > > FW_LOADER defaults to y. If we go down the path of removing the select > > completely we'd be taking a position that we could at least ensure > > EXPERT builds will work, but we cannot vet for not run time sanity of > > such build. I favor simplicity so would prefer to nuke the select > > completely and if we're really concerned about EXPERT users tristate > > mismatch misconfiguration why not just replace tristate with bool for > > FW_LOADER. That would do us the service of simplifying that code a > > bit, and leave only in place one way for folks that enable EXPERT to > > shoot themselves in the foot with FW_LOADER? > > I am afraid that we are moving into wrong direction here. Why don't we > look into Kconfig to teach it the difference between forced selection > and dependency instead? That was what I originally hinted we should try to do. See the implications of the issue right now and my explanation of a whitelist [0], granted this is just a description, I tried looking at the code and quickly gave up, it was not clear how such a thing could be implemented :( I gave up after Paul assured me this could not be an issue, or rather that the logic on kconfig was correct. FWIW, I think my original description of implications if this issue is not a real bug is a bit more clearer than the ones me and Paul ended up reviewing. The examples me and Paul reviewed are examples one can experiment if trying to address and fix the issue at hand in the most simplest cases. [0] https://lkml.org/lkml/2015/5/12/597 Luis -- 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/