LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Jan Beulich" <jbeulich@novell.com>
To: "David Brownell" <david-b@pacbell.net>,
	"Randy Dunlap" <rdunlap@xenotime.net>
Cc: "lkml" <linux-kernel@vger.kernel.org>
Subject: Re: 2.6.23-git Kconfig regression
Date: Wed, 16 Jan 2008 09:29:13 +0000	[thread overview]
Message-ID: <478DDC79.76E4.0078.0@novell.com> (raw)
In-Reply-To: <20071019202140.ecbc4a0d.rdunlap@xenotime.net>

>>> Randy Dunlap <rdunlap@xenotime.net> 20.10.07 05:21 >>>

Sorry for only now getting back to this.

>On Fri, 19 Oct 2007 19:55:35 -0700 Randy Dunlap wrote:
>
>> On Fri, 19 Oct 2007 19:01:09 -0700 Randy Dunlap wrote:
>> 
>> > >>> I noticed a regression, visible in the drivers/usb/gadget Kconfig;
>> > >>> it seems to be quite recent.
>> > >>>
>> > >>> ...
>> > >> Hm, it does look very odd.  It looks like it has something to
>> > >> do with <choice> working differently for some reason.
>> > >>
>> > >> In xconfig, I set all of the View Options and when I click on one
>> > >> of the periph. controllers, it says
>> > >>
>> > >> 	depends on =y && PCI
>> > > 
>> > > That's what I saw too.  Looked dubious ... 
>> > > 
>> > > 
>> > >> but if I back up to -git7, it says
>> > >>
>> > >> 	depends on <choice> && PCI
>> > > 
>> > > And that git7 thing doesn't look _quite_ so odd.  Did git7 actually

But it still doesn't look like it ought to be that way. Still, I'll try to make
this display e.g. <choice>=y instead.

>> > > let you configure a modular GOKU (for example), i.e. work correctly?
>> > 
>> > Yes, -git9 does.
>> > 
>> > Looks to me like it broke on -git10.  -git9 is OK.
>> > 
>> > >> I'll keep looking.
>> > > 
>> > > Thanks.  Kconfig is one of the areas I prefer to let others
>> > > be the experts.  :)
>> 
>> [hm, odd email problems, changing SMTP]
>> 
>> David,
>> 
>> Just a small update.
>> 
>> If I set USB gadget support to Y instead of M and peripheral
>> controller menu item to Y instead of M, then I can select any of the
>> 4 periph. controllers that are available to me.  (on -git14)
>> I don't know why it's like this though.
>
>David,
>
>My (quick, meaning that I may have missed something) testing
>indicates that the problem is in the patches attached.
>
>Can you revert them and test that?
>
>From git:
>http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=a5bf3d891a6a0fb5aa122792d965e3774108b923 
>
>Change kconfig behavior so that mixing bool and tristate config settings in
>a choice is possible and has the desired effect of offering just the
>tristate options individually if the choice gets set to M, and a normal
>boolean selection if the choice gets set to Y.
>
>Odd, I don't recall seeing this patch...

I'm pretty convinced that drivers/usb/gadget/Kconfig isn't really
written properly:

The first choice contains a mixture of boolean and tristate values; all
of the tristate ones are auto-select values (i.e. such with no prompt),
which results in all of them being unavailable when the choice itself is
set to M (the default with the patch - previously, the choice type got
derived from just the first contained value's type, which was clearly
wrong). These prompt-less items should go after the choice (resulting
in the choice to become a boolean one), if they're needed at all (I
think, especially with the intended change, these options are just
pointless except for avoiding

#if defined(CONFIG_...) || defined(CONFIG_..._MODULE)'

in C sources. In that latter case, the choice could become a tristate
one, allowing all of the selections to be built at once as modules
(which really seems to be the way distro kernels would want to use
it) or any one of them to be built in (the current behavior, except
that at present even when using these as a module only a single
one can be selected).

The second choice appears suspicious altogether - is it really that
just any one of these items can be selected to be built into the
kernel? And again, there are items inside that choice that really
seem to belong outside - I think this is an abuse of the choice
construct, which just happened to work when the choice type (as
described above) was derived from only its first contained value.

Bottom line is, in order to suggest an appropriate adjustment to this
Kconfig file I need clarification on its intentions. Meanwhile I'll scan
the tree for other suspicious choices...

Thanks, Jan


  parent reply	other threads:[~2008-01-16  9:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-20  1:22 David Brownell
2007-10-20  1:46 ` Randy Dunlap
2007-10-20  1:57   ` David Brownell
2007-10-20  2:01     ` Randy Dunlap
2007-10-20  2:55       ` Randy Dunlap
2007-10-20  3:21         ` Randy Dunlap
2007-10-20  4:11           ` David Brownell
2007-10-20  4:25             ` Linus Torvalds
2007-10-20 16:50               ` Sam Ravnborg
2008-01-16  9:29           ` Jan Beulich [this message]
2008-01-17  0:02             ` David Brownell
2008-01-17  8:16               ` Jan Beulich
2008-01-17  8:32                 ` David Brownell
2007-10-20 18:27 Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=478DDC79.76E4.0078.0@novell.com \
    --to=jbeulich@novell.com \
    --cc=david-b@pacbell.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdunlap@xenotime.net \
    --subject='Re: 2.6.23-git Kconfig regression' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).