Re: [RFC v1] tree-wide: remove "select FW_LOADER" uses

Dmitry Torokhov
2015-05-22T20:22:24Z

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.
>
> 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.
>
> > > 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?
>
> > > 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.
> >
> > So I see your GYM and LOCKER example, or better, the original FW &
> > CRYPTO warning and I wonder why do we actually go and check dependencies
> > of selected symbols when resolving given symbol.
>
> I'm no kconfig expert

Me neither ;)

> and I wondered the same.
>
> Perhaps Paul can elaborate.
>
> > I.e. should we not
> > "cut" the select branches off and evaluate them on their own? I.e. when
> > we try to select a symbol we note the symbols it tries to select and
> > evaluate them on their own (selects still don't cascade down - right?)
> > and then evaluate "depends on" symbols?

By the way, to resolve your FW loader issue can you simply select CRYPTO
instead of depending on it? I believe that certain core facilities (such
as FW loaders, Crypto, etc) should be normally selected rather than
depended upon, just because of workflow. If user works with menuconfig
and has core facility disable he won't event be presented with an
option. So one has to perform several "passes" over menuconfig to slect
everything that is needed.

Thanks.

--
Dmitry