Prev: Crypto: Talitos: Support for Async_tx XOR offload
Next: ThinkPad T400 : should kernel option "reboot=pci" be hard coded ?
From: Linus Torvalds on 14 Jul 2010 00:10
On Tue, Jul 13, 2010 at 7:26 PM, Grant Likely <grant.likely(a)secretlab.ca> wrote:
>
> I chose to use -D /dev/null (defconfig from an empty file) instead of
> -n (allnoconfig) so that default values in Kconfig would get
> respected. �For the benefit of everyone else, here's an excerpt from
> our IRC conversation this afternoon:
>
> 19:49 < gcl> sfr: [...] Your patch and my patch are
> � � � � � � essentially doing exactly the same thing, except that I used '-d'
> � � � � � � and you used '-n'.
> 19:50 < gcl> s/-d/-D/
> 19:55 < sfr> right
> 19:55 < sfr> Linus wanted us to use -n

Just a note: Linus doesn't really care.

IOW, I used -n not because of any fundamental belief that it is
correct, but just because ti happened to be how I happened to decide
to solve it. It's entirely possible that starting from the Kconfig
defaults (rather than "no") is the right way to go.

I think either approach is likely fine. The -D /dev/null approach
would presumably give smaller Kconfig.xyz files, assuming our defaults
are sane (an maybe that could be at least a partial validation of the
defaults we do have). While the -n approach is in some ways more
stable, in that the resulting Kconfig.xyz entires would presumably be
more stand-alone, and there wouldn't be any subtle interactions when
somebody changes a default value int he Kconfig file.

So I can see advantages to either model. And either model clearly
would want the improvements to "select" that are independent (ie warn
about unsatisfied 'depends on' constraints, and select recursively.
Maybe we already fixed the recursive select thing, I forget).

I also think we need to allow setting of actual values. I don't know
what the syntax would be. A "set" statement that overrides a default
in the Kconfig file, so that you can do

set NODES_SHIFT 10

which would basically be equivalent to a "select" of a tristate
variable, but instead set the actual value? I dunno.

And quite frankly, maybe somebody comes up with a better model
entirely. I like the Kconfig.xyz model, in that it should be
human-readable/writable and shouldn't introduce any fundamentally new
concepts (except the fairly simple extensions to the Kconfig
language), but maybe there are better models.

Regardless, I don't have anything against either set of patches
(Grant's or Stephen's).

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Grant Likely on 14 Jul 2010 01:50
On Tue, Jul 13, 2010 at 10:04 PM, Linus Torvalds
<torvalds(a)linux-foundation.org> wrote:
> On Tue, Jul 13, 2010 at 7:26 PM, Grant Likely <grant.likely(a)secretlab.ca> wrote:
>>
>> I chose to use -D /dev/null (defconfig from an empty file) instead of
>> -n (allnoconfig) so that default values in Kconfig would get
>> respected. �For the benefit of everyone else, here's an excerpt from
>> our IRC conversation this afternoon:
>>
>> 19:49 < gcl> sfr: [...] Your patch and my patch are
>> � � � � � � essentially doing exactly the same thing, except that I used '-d'
>> � � � � � � and you used '-n'.
>> 19:50 < gcl> s/-d/-D/
>> 19:55 < sfr> right
>> 19:55 < sfr> Linus wanted us to use -n
>
> Just a note: Linus doesn't really care.
>
> IOW, I used -n not because of any fundamental belief that it is
> correct, but just because ti happened to be how I happened to decide
> to solve it. It's entirely possible that starting from the Kconfig
> defaults (rather than "no") is the right way to go.
>
> I think either approach is likely fine. The -D /dev/null approach
> would presumably give smaller Kconfig.xyz files, assuming our defaults
> are sane (an maybe that could be at least a partial validation of the
> defaults we do have). While the -n approach is in some ways more
> stable, in that the resulting Kconfig.xyz entires would presumably be
> more stand-alone, and there wouldn't be any subtle interactions when
> somebody changes a default value int he Kconfig file.

Okay, well I advocate for the -D /dev/null approach then. I think
that validating our defaults, and looking for the subtle interactions
are exactly what we want to be doing when it comes to defconfigs. The
fact that a defconfig does *not* want the default value is exactly
what the defconfigs should be capturing.

> So I can see advantages to either model. And either model clearly
> would want the improvements to "select" that are independent (ie warn
> about unsatisfied 'depends on' constraints, and select recursively.
> Maybe we already fixed the recursive select thing, I forget).
>
> I also think we need to allow setting of actual values. I don't know
> what the syntax would be. A "set" statement that overrides a default
> in the Kconfig file, so that you can do
>
> � � � � �set NODES_SHIFT 10
>
> which would basically be equivalent to a "select" of a tristate
> variable, but instead set the actual value? I dunno.

I'm partial to extending select statements myself because it fits
nicely into the existing grammer; but I can see value in having a set
statement too. It would eliminate the temporary config options that
both my and Stephen's patch would add.

> And quite frankly, maybe somebody comes up with a better model
> entirely. I like the Kconfig.xyz model, in that it should be
> human-readable/writable and shouldn't introduce any fundamentally new
> concepts (except the fairly simple extensions to the Kconfig
> language), but maybe there are better models.

Perhaps, but I can't think of anything and this one is simple,
elegant, and easy to implement.

> Regardless, I don't have anything against either set of patches
> (Grant's or Stephen's).

I think we should run with this. Since the patch has been merged to
warn on unmet Kconfig dependences, the only major hole left is being
able to do negative selects and to select specific values. Stephen,
I'm happy to either keep working on this, or drop my patch in favor of
yours. Whichever you prefer. I'll try to find some time to look at
the Kconfig grammer.

The solver would also be useful and could further reduce the size of
the Kconfig fragments, but it isn't necessary so we don't need to wait
for it to get implemented to take this approach..

Cheers,
g.


--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on 14 Jul 2010 11:40
On Tue, Jul 13, 2010 at 10:47 PM, Grant Likely
<grant.likely(a)secretlab.ca> wrote:
>
> Okay, well I advocate for the -D /dev/null approach then. �I think
> that validating our defaults, and looking for the subtle interactions
> are exactly what we want to be doing when it comes to defconfigs. �The
> fact that a defconfig does *not* want the default value is exactly
> what the defconfigs should be capturing.

Sure. That does have the downside that it absolutely _requires_
changes to the Kconfig language. Because currently "select" can only
set a config variable, it cannot clear it or downgrade it from the
defaults - so if you start from anything but a noconfig, you can't
unset anything.

But that's not a big downside, since honestly either approach will
want it anyway (even -n wants it for setting integer or string options
- never mind the whole issue of making it easier/cleaner to write the
files in the first place).

>> � � � � �set NODES_SHIFT 10
>
> I'm partial to extending select statements myself because it fits
> nicely into the existing grammer; but I can see value in having a set
> statement too.

If it turns out easier to just implement the "set value" as a "select
with a value", that's fine by me too. If the syntax for setting a
particular value ends up being

select NODES_SHIFT 10

that still works (with or without a "=" there). It looks a bit odd to
me, but that's probably just because it's new. It doesn't look _wrong_
by any stretch.

>�It would eliminate the temporary config options that
> both my and Stephen's patch would add.

I don't think there is anything wrong with having the extra config
option just to drive the "select" syntax. In fact, in many cases I
think it would be "documentation", because I think the extra config
option should generally be named by the platform that you're actually
selecting for - even if that name may not be used for anything else.

So having the basic syntax for a Kconfig.xyz file be something like

config SIDEWINDER_PLATFORM
def_bool y
select ...

where that initial config/def_bool is strictly not needed for the
actual end result, but is used to get to the select statements, and to
include that "SIDEWINDER_PLATFORM=y" in the resulting .config as a
kind of top-level comment all looks fine.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: Crypto: Talitos: Support for Async_tx XOR offload
Next: ThinkPad T400 : should kernel option "reboot=pci" be hard coded ?