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

[Luis R. Rodriguez]

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 <tiwai@xxxxxxx> 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 <bp@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/