Message-ID: <20150522214357.GA24129@cloud>
Date:	Fri, 22 May 2015 14:43:57 -0700
From:	josh@...htriplett.org
To:	"Luis R. Rodriguez" <mcgrof@...e.com>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Takashi Iwai <tiwai@...e.de>, Paul Bolle <pebolle@...cali.nl>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Borislav Petkov <bp@...en8.de>,
	Greg KH <gregkh@...uxfoundation.org>,
	"David S. Miller" <davem@...emloft.net>, clemens@...isch.de,
	JBottomley@...n.com, David Airlie <airlied@...ux.ie>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Marcel Holtmann <marcel@...tmann.org>,
	"Gustavo F. Padovan" <gustavo@...ovan.org>,
	Johan Hedberg <johan.hedberg@...il.com>,
	Mikael Starvik <starvik@...s.com>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	Imre Kaloz <kaloz@...nwrt.org>, khalasa@...p.pl,
	Ohad Ben-Cohen <ohad@...ery.com>,
	Arnd Bergmann <arnd@...db.de>, 3chas3@...il.com,
	Jiri Slaby <jslaby@...e.cz>, Bryan Wu <cooloney@...il.com>,
	Richard Purdie <rpurdie@...ys.net>,
	Jacek Anaszewski <j.anaszewski@...sung.com>,
	mcgrof@...not-panic.com,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC v1] tree-wide: remove "select FW_LOADER" uses

On Fri, May 22, 2015 at 09:28:14PM +0200, Luis R. Rodriguez wrote:
> Adding Josh, as I know he seems to be on an EXPERT crusade to tuck
> away system calls for tinyconfig, so likely can chime in for some
> insane EXPERT runtime issue that users can run into.

Thanks.

> On Fri, May 22, 2015 at 11:52:07AM -0700, Dmitry Torokhov wrote:
> > On Fri, May 22, 2015 at 08:19:24PM +0200, Luis R. Rodriguez wrote:
> > > 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.
> > 
> > Not major, rather tweaking the driver sets. Like expert V4L or HID
> > configs.
> 
> Driver tweaks seems rather harmless compared to "trust me I know
> what I'm doing" even for all run time cases.
> 
> So the question still stands -- are there any kconfig options which
> depend on EXPERT which enable folks to screw up run time (not just driver
> tweaks? If so, would it be good to separate the two types of expert
> config options, one for tweaks, another for run time possible conflicts?
> 
> Reason I ask this is -- IMO we should not treat the possible EXPERT run time
> conflicts of FW_LOADER specially and try to be consistent, if we are ensuring
> EXPERT run time conflicts will work with FW_LOADER why not for others? If
> we want the option to let an EXPERT kconfig option enable run time conflicts
> maybe we need to split things up further.

I don't think it should be possible, even with EXPERT, to disable an
option that a driver needs at runtime without disabling that driver.  If
the driver *optionally* needs something, but can work without it, it
shouldn't have "depends" or "select".

> > > 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"?
> > 
> > But the user who is not expert might drop it. The premise was "only
> > experts would have CONFIG_EXPERT and thus we do not care about
> > breakage". But if people use distro configs as a starting point they all
> > are "experts".
> 
> Replacing "select FW_LOADER" with "depends on FW_LOADER", although still pegged
> with EXPERT, would not allow EXPERT users (most folks we've determined now) to
> disable FW_LOADER if a driver was selected that needed FW_LOADER, in fact I
> could not also create a tristate conflict of say MICROCODE set to y and
> FW_LOADER to m. Please let me know if I've missed something with testing this
> though.
> 
> So unless I'm missing a corner case, it would seem replacing "select FW_LOADER"
> with "depends on FW_LOADER" is a reasonable option so far. The subtleties lie
> then now in since EXPERT is used by most distributions, everyone is an expert,
> and:
> 
>   a) Do we want to be consistent about ensuring run time sanity of EXPERT?
>   b) Should we split EXPERT into tweak / possible run time break options?

"depends on FW_LOADER" and "select FW_LOADER" are equivalent from the
point of view of what configs they allow; they differ in how the
dependency is shown in UIs.  "select" means you can always see the
drivers, and you can't turn off FW_LOADER until you turn off all of the
drivers.  "depends" means that if you disable FW_LOADER all the drivers
that depend on it disappear, and you can't turn them on unless you turn
on FW_LOADER (which however is always on when !EXPERT).

Either way, if you have FW_LOADER and a pile of drivers using it
enabled, you have to disable all the drivers before disabling FW_LOADER.
The behavior only differs when you have FW_LOADER disabled, which is a
sufficiently uncommon case that it doesn't matter deeply which approach
you take.  Personally, if FW_LOADER doesn't have any dependencies, I'd
favor the select, since it avoids the case of "wait, where is the driver
I expected and why can't I find it?".

However, from the sounds of things, you want to add a dependency to
FW_LOADER.  So in that case, yeah, switch to depends rather than
rewriting Kconfig, unless you're feeling really ambitious.

> > > 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.

By "recursive dependency issue", I'm guessing that someone wants to make
FW_LOADER depend on something else, which breaks because "select"
doesn't know how to recurse?

Ideally, someone should teach Kconfig to handle recursive dependencies,
but in the meantime using "depends" makes sense.

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists