RFC: kconfig select warnings bogus?

RFC: kconfig select warnings bogus?

[Sam Ravnborg]

We see a lot of these lately:
  GEN     /home/bor/build/linux-2.6.22/Makefile
scripts/kconfig/conf -s arch/i386/Kconfig
drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'


Do this warning really add any value or should we just ignore them like this?

	Sam

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index f14aeac..c0c84f2 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -200,11 +200,6 @@ void sym_check_prop(struct symbol *sym)
 				prop_warn(prop,
 				    "config symbol '%s' uses select, but is "
 				    "not boolean or tristate", sym->name);
-			else if (sym2->type == S_UNKNOWN)
-				prop_warn(prop,
-				    "'select' used by config symbol '%s' "
-				    "refers to undefined symbol '%s'",
-				    sym->name, sym2->name);
 			else if (sym2->type != S_BOOLEAN && sym2->type != S_TRISTATE)
 				prop_warn(prop,
 				    "'%s' has wrong type. 'select' only "

Re: RFC: kconfig select warnings bogus?

[Andrew Morton]

On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> wrote:

> We see a lot of these lately:
>   GEN     /home/bor/build/linux-2.6.22/Makefile
> scripts/kconfig/conf -s arch/i386/Kconfig
> drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
> drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> 
> 
> Do this warning really add any value or should we just ignore them like this?
> 

They always indicate Kconfig bugs, don't they?  If so, we should keep the
warning.

We should also fix the Kconfig bugs but for some reason they tend to hang
around for ages and ages.

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/19/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> wrote:
>
> > We see a lot of these lately:
> >   GEN     /home/bor/build/linux-2.6.22/Makefile
> > scripts/kconfig/conf -s arch/i386/Kconfig
> > drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
> > drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> >
> >
> > Do this warning really add any value or should we just ignore them like this?
> >
>
> They always indicate Kconfig bugs, don't they?  If so, we should keep the
> warning.
>
> We should also fix the Kconfig bugs but for some reason they tend to hang
> around for ages and ages.

I remember coming across a thread couple weeks back that contained
patches that eliminated all four of these .. perhaps nobody picked them
up. Adding the authors of those patches to CC list (those that I
remember, at least).

(later)

I think this is the thread, in any case:

http://groups.google.com/group/linux.kernel/browse_frm/thread/d46d5dcecb84424a/1b586d67a1205d86?tvc=1

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> On 5/19/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > > We see a lot of these lately:
> > >   GEN     /home/bor/build/linux-2.6.22/Makefile
> > > scripts/kconfig/conf -s arch/i386/Kconfig
> > > drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
> > > drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> > > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > > drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > >
> > >
> > > Do this warning really add any value or should we just ignore them like this?
> > >
> >
> > They always indicate Kconfig bugs, don't they?  If so, we should keep the
> > warning.
> >
> > We should also fix the Kconfig bugs but for some reason they tend to hang
> > around for ages and ages.
>
> I remember coming across a thread couple weeks back that contained
> patches that eliminated all four of these .. perhaps nobody picked them
> up. Adding the authors of those patches to CC list (those that I
> remember, at least).
>
> (later)
>
> I think this is the thread, in any case:
>
> http://groups.google.com/group/linux.kernel/browse_frm/thread/d46d5dcecb84424a/1b586d67a1205d86?tvc=1


A combined patch for all of those on top of 22-rc2 (adding Signed-off-by's
for original authors too):

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>

---

 arch/m68k/Kconfig                  |    1 +
 arch/powerpc/Kconfig               |    1 +
 arch/powerpc/sysdev/qe_lib/Kconfig |    1 +
 drivers/input/keyboard/Kconfig     |    1 -
 drivers/input/mouse/Kconfig        |    1 -
 drivers/macintosh/Kconfig          |    1 -
 drivers/net/Kconfig                |    1 -
 7 files changed, 3 insertions(+), 4 deletions(-)

---

diff -ruNp a/arch/m68k/Kconfig b/arch/m68k/Kconfig
--- a/arch/m68k/Kconfig	2007-05-20 04:11:47.000000000 +0530
+++ b/arch/m68k/Kconfig	2007-05-20 04:23:57.000000000 +0530
@@ -411,6 +411,7 @@ config STRAM_PROC

 config ATARI_KBD_CORE
 	bool
+	default y if KEYBOARD_ATARI || MOUSE_ATARI

 config HEARTBEAT
 	bool "Use power LED as a heartbeat" if AMIGA || APOLLO || ATARI || MAC ||Q40
diff -ruNp a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
--- a/arch/powerpc/Kconfig	2007-05-20 04:11:48.000000000 +0530
+++ b/arch/powerpc/Kconfig	2007-05-20 04:18:38.000000000 +0530
@@ -119,6 +119,7 @@ config GENERIC_BUG

 config SYS_SUPPORTS_APM_EMULATION
 	bool
+	default y if PMAC_APM_EMU

 config DEFAULT_UIMAGE
 	bool
diff -ruNp a/arch/powerpc/sysdev/qe_lib/Kconfig
b/arch/powerpc/sysdev/qe_lib/Kconfig
--- a/arch/powerpc/sysdev/qe_lib/Kconfig	2007-05-20 04:11:49.000000000 +0530
+++ b/arch/powerpc/sysdev/qe_lib/Kconfig	2007-05-20 04:20:43.000000000 +0530
@@ -12,6 +12,7 @@ config UCC_SLOW

 config UCC_FAST
 	bool
+	default y if UCC_GETH
 	default n
 	select UCC
 	help
diff -ruNp a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
--- a/drivers/input/keyboard/Kconfig	2007-05-20 04:11:55.000000000 +0530
+++ b/drivers/input/keyboard/Kconfig	2007-05-20 04:24:54.000000000 +0530
@@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
 config KEYBOARD_ATARI
 	tristate "Atari keyboard"
 	depends on ATARI
-	select ATARI_KBD_CORE
 	help
 	  Say Y here if you are running Linux on any Atari and have a keyboard
 	  attached.
diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
--- a/drivers/input/mouse/Kconfig	2007-05-20 04:11:55.000000000 +0530
+++ b/drivers/input/mouse/Kconfig	2007-05-20 04:25:09.000000000 +0530
@@ -179,7 +179,6 @@ config MOUSE_AMIGA
 config MOUSE_ATARI
 	tristate "Atari mouse"
 	depends on ATARI
-	select ATARI_KBD_CORE
 	help
 	  Say Y here if you have an Atari and want its native mouse
 	  supported by the kernel.
diff -ruNp a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
--- a/drivers/macintosh/Kconfig	2007-05-20 04:11:55.000000000 +0530
+++ b/drivers/macintosh/Kconfig	2007-05-20 04:18:57.000000000 +0530
@@ -113,7 +113,6 @@ config PMAC_SMU

 config PMAC_APM_EMU
 	tristate "APM emulation"
-	select SYS_SUPPORTS_APM_EMULATION
 	select APM_EMULATION
 	depends on ADB_PMU && PM

diff -ruNp a/drivers/net/Kconfig b/drivers/net/Kconfig
--- a/drivers/net/Kconfig	2007-05-20 04:11:56.000000000 +0530
+++ b/drivers/net/Kconfig	2007-05-20 04:22:15.000000000 +0530
@@ -2280,7 +2280,6 @@ config GFAR_NAPI
 config UCC_GETH
 	tristate "Freescale QE Gigabit Ethernet"
 	depends on QUICC_ENGINE
-	select UCC_FAST
 	help
 	  This driver supports the Gigabit Ethernet mode of the QUICC Engine,
 	  which is available on some Freescale SOCs.

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 04:19:12AM +0530, Satyam Sharma wrote:
>...
> A combined patch for all of those on top of 22-rc2 (adding Signed-off-by's
> for original authors too):
>...
> --- a/arch/m68k/Kconfig	2007-05-20 04:11:47.000000000 +0530
> +++ b/arch/m68k/Kconfig	2007-05-20 04:23:57.000000000 +0530
> @@ -411,6 +411,7 @@ config STRAM_PROC
>
> config ATARI_KBD_CORE
> 	bool
> +	default y if KEYBOARD_ATARI || MOUSE_ATARI
>...
> --- a/drivers/input/keyboard/Kconfig	2007-05-20 04:11:55.000000000 +0530
> +++ b/drivers/input/keyboard/Kconfig	2007-05-20 04:24:54.000000000 +0530
> @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
> config KEYBOARD_ATARI
> 	tristate "Atari keyboard"
> 	depends on ATARI
> -	select ATARI_KBD_CORE
> 	help
> 	  Say Y here if you are running Linux on any Atari and have a keyboard
> 	  attached.
> diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> --- a/drivers/input/mouse/Kconfig	2007-05-20 04:11:55.000000000 +0530
> +++ b/drivers/input/mouse/Kconfig	2007-05-20 04:25:09.000000000 +0530
> @@ -179,7 +179,6 @@ config MOUSE_AMIGA
> config MOUSE_ATARI
> 	tristate "Atari mouse"
> 	depends on ATARI
> -	select ATARI_KBD_CORE
> 	help
> 	  Say Y here if you have an Atari and want its native mouse
> 	  supported by the kernel.
>...

This is a good example how "fixing" the warnings makes things worse...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, May 20, 2007 at 04:19:12AM +0530, Satyam Sharma wrote:
> >...
> > A combined patch for all of those on top of 22-rc2 (adding Signed-off-by's
> > for original authors too):
> >...
> > --- a/arch/m68k/Kconfig       2007-05-20 04:11:47.000000000 +0530
> > +++ b/arch/m68k/Kconfig       2007-05-20 04:23:57.000000000 +0530
> > @@ -411,6 +411,7 @@ config STRAM_PROC
> >
> > config ATARI_KBD_CORE
> >       bool
> > +     default y if KEYBOARD_ATARI || MOUSE_ATARI
> >...
> > --- a/drivers/input/keyboard/Kconfig  2007-05-20 04:11:55.000000000 +0530
> > +++ b/drivers/input/keyboard/Kconfig  2007-05-20 04:24:54.000000000 +0530
> > @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
> > config KEYBOARD_ATARI
> >       tristate "Atari keyboard"
> >       depends on ATARI
> > -     select ATARI_KBD_CORE
> >       help
> >         Say Y here if you are running Linux on any Atari and have a keyboard
> >         attached.
> > diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> > --- a/drivers/input/mouse/Kconfig     2007-05-20 04:11:55.000000000 +0530
> > +++ b/drivers/input/mouse/Kconfig     2007-05-20 04:25:09.000000000 +0530
> > @@ -179,7 +179,6 @@ config MOUSE_AMIGA
> > config MOUSE_ATARI
> >       tristate "Atari mouse"
> >       depends on ATARI
> > -     select ATARI_KBD_CORE
> >       help
> >         Say Y here if you have an Atari and want its native mouse
> >         supported by the kernel.
> >...
>
> This is a good example how "fixing" the warnings makes things worse...

How?

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 04:47:52AM +0530, Satyam Sharma wrote:
> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>> On Sun, May 20, 2007 at 04:19:12AM +0530, Satyam Sharma wrote:
>> >...
>> > A combined patch for all of those on top of 22-rc2 (adding 
>> Signed-off-by's
>> > for original authors too):
>> >...
>> > --- a/arch/m68k/Kconfig       2007-05-20 04:11:47.000000000 +0530
>> > +++ b/arch/m68k/Kconfig       2007-05-20 04:23:57.000000000 +0530
>> > @@ -411,6 +411,7 @@ config STRAM_PROC
>> >
>> > config ATARI_KBD_CORE
>> >       bool
>> > +     default y if KEYBOARD_ATARI || MOUSE_ATARI
>> >...
>> > --- a/drivers/input/keyboard/Kconfig  2007-05-20 04:11:55.000000000 
>> +0530
>> > +++ b/drivers/input/keyboard/Kconfig  2007-05-20 04:24:54.000000000 
>> +0530
>> > @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
>> > config KEYBOARD_ATARI
>> >       tristate "Atari keyboard"
>> >       depends on ATARI
>> > -     select ATARI_KBD_CORE
>> >       help
>> >         Say Y here if you are running Linux on any Atari and have a 
>> keyboard
>> >         attached.
>> > diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
>> > --- a/drivers/input/mouse/Kconfig     2007-05-20 04:11:55.000000000 
>> +0530
>> > +++ b/drivers/input/mouse/Kconfig     2007-05-20 04:25:09.000000000 
>> +0530
>> > @@ -179,7 +179,6 @@ config MOUSE_AMIGA
>> > config MOUSE_ATARI
>> >       tristate "Atari mouse"
>> >       depends on ATARI
>> > -     select ATARI_KBD_CORE
>> >       help
>> >         Say Y here if you have an Atari and want its native mouse
>> >         supported by the kernel.
>> >...
>>
>> This is a good example how "fixing" the warnings makes things worse...
>
> How?

Consider ATARI_KBD_CORE was used by 20 drivers.

Using select for such not user visible helper variables is a really nice 
thing, and much more readable (and therefore much less likely to contain 
bugs) than dependencies with tons of "||"'s.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, May 20, 2007 at 04:47:52AM +0530, Satyam Sharma wrote:
> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> >> On Sun, May 20, 2007 at 04:19:12AM +0530, Satyam Sharma wrote:
> >> >...
> >> > A combined patch for all of those on top of 22-rc2 (adding
> >> Signed-off-by's
> >> > for original authors too):
> >> >...
> >> > --- a/arch/m68k/Kconfig       2007-05-20 04:11:47.000000000 +0530
> >> > +++ b/arch/m68k/Kconfig       2007-05-20 04:23:57.000000000 +0530
> >> > @@ -411,6 +411,7 @@ config STRAM_PROC
> >> >
> >> > config ATARI_KBD_CORE
> >> >       bool
> >> > +     default y if KEYBOARD_ATARI || MOUSE_ATARI
> >> >...
> >> > --- a/drivers/input/keyboard/Kconfig  2007-05-20 04:11:55.000000000
> >> +0530
> >> > +++ b/drivers/input/keyboard/Kconfig  2007-05-20 04:24:54.000000000
> >> +0530
> >> > @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
> >> > config KEYBOARD_ATARI
> >> >       tristate "Atari keyboard"
> >> >       depends on ATARI
> >> > -     select ATARI_KBD_CORE
> >> >       help
> >> >         Say Y here if you are running Linux on any Atari and have a
> >> keyboard
> >> >         attached.
> >> > diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> >> > --- a/drivers/input/mouse/Kconfig     2007-05-20 04:11:55.000000000
> >> +0530
> >> > +++ b/drivers/input/mouse/Kconfig     2007-05-20 04:25:09.000000000
> >> +0530
> >> > @@ -179,7 +179,6 @@ config MOUSE_AMIGA
> >> > config MOUSE_ATARI
> >> >       tristate "Atari mouse"
> >> >       depends on ATARI
> >> > -     select ATARI_KBD_CORE
> >> >       help
> >> >         Say Y here if you have an Atari and want its native mouse
> >> >         supported by the kernel.
> >> >...
> >>
> >> This is a good example how "fixing" the warnings makes things worse...
> >
> > How?
>
> Consider ATARI_KBD_CORE was used by 20 drivers.
>
> Using select for such not user visible helper variables is a really nice
> thing, and much more readable (and therefore much less likely to contain
> bugs) than dependencies with tons of "||"'s.

Well, the "default .. if .." kind of idiom is fairly common (I could say
almost standard), in arch/.../config's. It's been used for some time,
and for several symbols over there. But you're right that if 20 drivers
used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
perhaps we do need some kind of fix for this.

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > On Sun, May 20, 2007 at 04:47:52AM +0530, Satyam Sharma wrote:
> > > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > >> On Sun, May 20, 2007 at 04:19:12AM +0530, Satyam Sharma wrote:
> > >> >...
> > >> > A combined patch for all of those on top of 22-rc2 (adding
> > >> Signed-off-by's
> > >> > for original authors too):
> > >> >...
> > >> > --- a/arch/m68k/Kconfig       2007-05-20 04:11:47.000000000 +0530
> > >> > +++ b/arch/m68k/Kconfig       2007-05-20 04:23:57.000000000 +0530
> > >> > @@ -411,6 +411,7 @@ config STRAM_PROC
> > >> >
> > >> > config ATARI_KBD_CORE
> > >> >       bool
> > >> > +     default y if KEYBOARD_ATARI || MOUSE_ATARI
> > >> >...
> > >> > --- a/drivers/input/keyboard/Kconfig  2007-05-20 04:11:55.000000000
> > >> +0530
> > >> > +++ b/drivers/input/keyboard/Kconfig  2007-05-20 04:24:54.000000000
> > >> +0530
> > >> > @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
> > >> > config KEYBOARD_ATARI
> > >> >       tristate "Atari keyboard"
> > >> >       depends on ATARI
> > >> > -     select ATARI_KBD_CORE
> > >> >       help
> > >> >         Say Y here if you are running Linux on any Atari and have a
> > >> keyboard
> > >> >         attached.
> > >> > diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> > >> > --- a/drivers/input/mouse/Kconfig     2007-05-20 04:11:55.000000000
> > >> +0530
> > >> > +++ b/drivers/input/mouse/Kconfig     2007-05-20 04:25:09.000000000
> > >> +0530
> > >> > @@ -179,7 +179,6 @@ config MOUSE_AMIGA
> > >> > config MOUSE_ATARI
> > >> >       tristate "Atari mouse"
> > >> >       depends on ATARI
> > >> > -     select ATARI_KBD_CORE
> > >> >       help
> > >> >         Say Y here if you have an Atari and want its native mouse
> > >> >         supported by the kernel.
> > >> >...
> > >>
> > >> This is a good example how "fixing" the warnings makes things worse...
> > >
> > > How?
> >
> > Consider ATARI_KBD_CORE was used by 20 drivers.
> >
> > Using select for such not user visible helper variables is a really nice
> > thing, and much more readable (and therefore much less likely to contain
> > bugs) than dependencies with tons of "||"'s.
>
> Well, the "default .. if .." kind of idiom is fairly common (I could say
> almost standard), in arch/.../config's. It's been used for some time,
> and for several symbols over there. But you're right that if 20 drivers
> used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
> perhaps we do need some kind of fix for this.

[ BTW ] I hope nobody gets the wrong message with the word "fix"
above, what you're pointing out is only a "readability" issue, not a
potential build breakage or any critical problem ...

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 05:11:50AM +0530, Satyam Sharma wrote:
> On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
>> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>> > On Sun, May 20, 2007 at 04:47:52AM +0530, Satyam Sharma wrote:
>> > > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>> > >> On Sun, May 20, 2007 at 04:19:12AM +0530, Satyam Sharma wrote:
>> > >> >...
>> > >> > A combined patch for all of those on top of 22-rc2 (adding
>> > >> Signed-off-by's
>> > >> > for original authors too):
>> > >> >...
>> > >> > --- a/arch/m68k/Kconfig       2007-05-20 04:11:47.000000000 +0530
>> > >> > +++ b/arch/m68k/Kconfig       2007-05-20 04:23:57.000000000 +0530
>> > >> > @@ -411,6 +411,7 @@ config STRAM_PROC
>> > >> >
>> > >> > config ATARI_KBD_CORE
>> > >> >       bool
>> > >> > +     default y if KEYBOARD_ATARI || MOUSE_ATARI
>> > >> >...
>> > >> > --- a/drivers/input/keyboard/Kconfig  2007-05-20 04:11:55.000000000
>> > >> +0530
>> > >> > +++ b/drivers/input/keyboard/Kconfig  2007-05-20 04:24:54.000000000
>> > >> +0530
>> > >> > @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
>> > >> > config KEYBOARD_ATARI
>> > >> >       tristate "Atari keyboard"
>> > >> >       depends on ATARI
>> > >> > -     select ATARI_KBD_CORE
>> > >> >       help
>> > >> >         Say Y here if you are running Linux on any Atari and have a
>> > >> keyboard
>> > >> >         attached.
>> > >> > diff -ruNp a/drivers/input/mouse/Kconfig 
>> b/drivers/input/mouse/Kconfig
>> > >> > --- a/drivers/input/mouse/Kconfig     2007-05-20 04:11:55.000000000
>> > >> +0530
>> > >> > +++ b/drivers/input/mouse/Kconfig     2007-05-20 04:25:09.000000000
>> > >> +0530
>> > >> > @@ -179,7 +179,6 @@ config MOUSE_AMIGA
>> > >> > config MOUSE_ATARI
>> > >> >       tristate "Atari mouse"
>> > >> >       depends on ATARI
>> > >> > -     select ATARI_KBD_CORE
>> > >> >       help
>> > >> >         Say Y here if you have an Atari and want its native mouse
>> > >> >         supported by the kernel.
>> > >> >...
>> > >>
>> > >> This is a good example how "fixing" the warnings makes things 
>> worse...
>> > >
>> > > How?
>> >
>> > Consider ATARI_KBD_CORE was used by 20 drivers.
>> >
>> > Using select for such not user visible helper variables is a really nice
>> > thing, and much more readable (and therefore much less likely to contain
>> > bugs) than dependencies with tons of "||"'s.
>>
>> Well, the "default .. if .." kind of idiom is fairly common (I could say
>> almost standard), in arch/.../config's. It's been used for some time,
>> and for several symbols over there. But you're right that if 20 drivers
>> used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
>> perhaps we do need some kind of fix for this.
>
> [ BTW ] I hope nobody gets the wrong message with the word "fix"
> above, what you're pointing out is only a "readability" issue, not a
> potential build breakage or any critical problem ...

And what it would "fix" is only a bogus warning, not a potential build 
breakage or any critical problem ...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, May 20, 2007 at 05:11:50AM +0530, Satyam Sharma wrote:
> > On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> >> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> >> > On Sun, May 20, 2007 at 04:47:52AM +0530, Satyam Sharma wrote:
> >> > > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> >> > >> On Sun, May 20, 2007 at 04:19:12AM +0530, Satyam Sharma wrote:
> >> > >> >...
> >> > >> > A combined patch for all of those on top of 22-rc2 (adding
> >> > >> Signed-off-by's
> >> > >> > for original authors too):
> >> > >> >...
> >> > >> > --- a/arch/m68k/Kconfig       2007-05-20 04:11:47.000000000 +0530
> >> > >> > +++ b/arch/m68k/Kconfig       2007-05-20 04:23:57.000000000 +0530
> >> > >> > @@ -411,6 +411,7 @@ config STRAM_PROC
> >> > >> >
> >> > >> > config ATARI_KBD_CORE
> >> > >> >       bool
> >> > >> > +     default y if KEYBOARD_ATARI || MOUSE_ATARI
> >> > >> >...
> >> > >> > --- a/drivers/input/keyboard/Kconfig  2007-05-20 04:11:55.000000000
> >> > >> +0530
> >> > >> > +++ b/drivers/input/keyboard/Kconfig  2007-05-20 04:24:54.000000000
> >> > >> +0530
> >> > >> > @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
> >> > >> > config KEYBOARD_ATARI
> >> > >> >       tristate "Atari keyboard"
> >> > >> >       depends on ATARI
> >> > >> > -     select ATARI_KBD_CORE
> >> > >> >       help
> >> > >> >         Say Y here if you are running Linux on any Atari and have a
> >> > >> keyboard
> >> > >> >         attached.
> >> > >> > diff -ruNp a/drivers/input/mouse/Kconfig
> >> b/drivers/input/mouse/Kconfig
> >> > >> > --- a/drivers/input/mouse/Kconfig     2007-05-20 04:11:55.000000000
> >> > >> +0530
> >> > >> > +++ b/drivers/input/mouse/Kconfig     2007-05-20 04:25:09.000000000
> >> > >> +0530
> >> > >> > @@ -179,7 +179,6 @@ config MOUSE_AMIGA
> >> > >> > config MOUSE_ATARI
> >> > >> >       tristate "Atari mouse"
> >> > >> >       depends on ATARI
> >> > >> > -     select ATARI_KBD_CORE
> >> > >> >       help
> >> > >> >         Say Y here if you have an Atari and want its native mouse
> >> > >> >         supported by the kernel.
> >> > >> >...
> >> > >>
> >> > >> This is a good example how "fixing" the warnings makes things
> >> worse...
> >> > >
> >> > > How?
> >> >
> >> > Consider ATARI_KBD_CORE was used by 20 drivers.
> >> >
> >> > Using select for such not user visible helper variables is a really nice
> >> > thing, and much more readable (and therefore much less likely to contain
> >> > bugs) than dependencies with tons of "||"'s.
> >>
> >> Well, the "default .. if .." kind of idiom is fairly common (I could say
> >> almost standard), in arch/.../config's. It's been used for some time,
> >> and for several symbols over there. But you're right that if 20 drivers
> >> used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
> >> perhaps we do need some kind of fix for this.
> >
> > [ BTW ] I hope nobody gets the wrong message with the word "fix"
> > above, what you're pointing out is only a "readability" issue, not a
> > potential build breakage or any critical problem ...
>
> And what it would "fix" is only a bogus warning, not a potential build
> breakage or any critical problem ...

Yeah, as I said already, if you're saying that all four of these warnings
are bogus and don't lead to any build failures, then perhaps the patch
I sent isn't any "critical" either. Again, I can't really confirm, because
I've never tried to build these drivers, because I don't have access to
powerpc / m68k machines.

But the patch I sent does suppress bogus warnings, and does it in a
way that is fairly common / similar to the way most other similar helper
stuff are handled in arch/.../config's.

Sam's patch will also suppress warnings, but the problem there is that
it kills off checking for "select"ion of non-existent symbols _completely_,
and thus may allow build breakages to occur.

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 05:06:33AM +0530, Satyam Sharma wrote:
> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>>...
>> Consider ATARI_KBD_CORE was used by 20 drivers.
>>
>> Using select for such not user visible helper variables is a really nice
>> thing, and much more readable (and therefore much less likely to contain
>> bugs) than dependencies with tons of "||"'s.
>
> Well, the "default .. if .." kind of idiom is fairly common (I could say
> almost standard), in arch/.../config's. It's been used for some time,
> and for several symbols over there. But you're right that if 20 drivers
> used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
> perhaps we do need some kind of fix for this.

And the fix is to use select.

Compare the handling of options like IRQ_CPU in arch/mips/Kconfig in 
current kernels with the handling in kernel 2.6.0 .

Or as an exercise, change drivers/net/Kconfig to no longer use
"select MII". When you are finished, ensure that you are handling it 
properly although the option is user visible...

There are cases where "default .. if .." is the right idiom, but there 
are also cases where "select" is the right idiom. And for helper code 
like ATARI_KBD_CORE, "select" is the right idiom.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, May 20, 2007 at 05:06:33AM +0530, Satyam Sharma wrote:
> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> >>...
> >> Consider ATARI_KBD_CORE was used by 20 drivers.
> >>
> >> Using select for such not user visible helper variables is a really nice
> >> thing, and much more readable (and therefore much less likely to contain
> >> bugs) than dependencies with tons of "||"'s.
> >
> > Well, the "default .. if .." kind of idiom is fairly common (I could say
> > almost standard), in arch/.../config's. It's been used for some time,
> > and for several symbols over there. But you're right that if 20 drivers
> > used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
> > perhaps we do need some kind of fix for this.
>
> And the fix is to use select.
>
> Compare the handling of options like IRQ_CPU in arch/mips/Kconfig in
> current kernels with the handling in kernel 2.6.0 .
>
> Or as an exercise, change drivers/net/Kconfig to no longer use
> "select MII". When you are finished, ensure that you are handling it
> properly although the option is user visible...

"config MII" and "select MII" are _not_ equivalent to the case at hand.
MII is defined in drivers/net/Kconfig itself so does not print any "symbol
unknown kind of warnings" ... so clearly no probs in "select" for it ...

> There are cases where "default .. if .." is the right idiom, but there
> are also cases where "select" is the right idiom. And for helper code
> like ATARI_KBD_CORE, "select" is the right idiom.

ATARI_KBD_CORE, unlike MII, is defined only by some archs. And the
correct (most widely used or standard, in any case) idiom for that is
"default .. if ..". Or perhaps you can convert those helper code options in
arch/.../config's over to select too, as an exercise? :-)

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 05:25:24AM +0530, Satyam Sharma wrote:
> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>> On Sun, May 20, 2007 at 05:06:33AM +0530, Satyam Sharma wrote:
>> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>> >>...
>> >> Consider ATARI_KBD_CORE was used by 20 drivers.
>> >>
>> >> Using select for such not user visible helper variables is a really 
>> nice
>> >> thing, and much more readable (and therefore much less likely to 
>> contain
>> >> bugs) than dependencies with tons of "||"'s.
>> >
>> > Well, the "default .. if .." kind of idiom is fairly common (I could say
>> > almost standard), in arch/.../config's. It's been used for some time,
>> > and for several symbols over there. But you're right that if 20 drivers
>> > used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
>> > perhaps we do need some kind of fix for this.
>>
>> And the fix is to use select.
>>
>> Compare the handling of options like IRQ_CPU in arch/mips/Kconfig in
>> current kernels with the handling in kernel 2.6.0 .
>>
>> Or as an exercise, change drivers/net/Kconfig to no longer use
>> "select MII". When you are finished, ensure that you are handling it
>> properly although the option is user visible...
>
> "config MII" and "select MII" are _not_ equivalent to the case at hand.
> MII is defined in drivers/net/Kconfig itself so does not print any "symbol
> unknown kind of warnings" ... so clearly no probs in "select" for it ...

Then move the "config MII" to arch/i386/Kconfig and assume all drivers  
select'ing it would depend on X86_32.

This is not a problem in the kernel sources, it is an example for you to 
see why select is the correct idiom in such cases.

Please try it if you want to understand the problem you are talking 
about.

>> There are cases where "default .. if .." is the right idiom, but there
>> are also cases where "select" is the right idiom. And for helper code
>> like ATARI_KBD_CORE, "select" is the right idiom.
>
> ATARI_KBD_CORE, unlike MII, is defined only by some archs. And the
> correct (most widely used or standard, in any case) idiom for that is
> "default .. if ..". Or perhaps you can convert those helper code options in
> arch/.../config's over to select too, as an exercise? :-)

Perhaps not as an exercise, but actually for real.

We had "fixed" such warnings in the past similar to your patch, but that 
was actually a mistake.

And "correct" can easily be the opposite of "most widely used or standard"
if you discover that you did it wrong in the past.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, May 20, 2007 at 05:25:24AM +0530, Satyam Sharma wrote:
> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> >> On Sun, May 20, 2007 at 05:06:33AM +0530, Satyam Sharma wrote:
> >> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> >> >>...
> >> >> Consider ATARI_KBD_CORE was used by 20 drivers.
> >> >>
> >> >> Using select for such not user visible helper variables is a really
> >> nice
> >> >> thing, and much more readable (and therefore much less likely to
> >> contain
> >> >> bugs) than dependencies with tons of "||"'s.
> >> >
> >> > Well, the "default .. if .." kind of idiom is fairly common (I could say
> >> > almost standard), in arch/.../config's. It's been used for some time,
> >> > and for several symbols over there. But you're right that if 20 drivers
> >> > used ATARI_KBD_CORE, we'd get tons of ugly "||"'s there, so
> >> > perhaps we do need some kind of fix for this.
> >>
> >> And the fix is to use select.
> >>
> >> Compare the handling of options like IRQ_CPU in arch/mips/Kconfig in
> >> current kernels with the handling in kernel 2.6.0 .
> >>
> >> Or as an exercise, change drivers/net/Kconfig to no longer use
> >> "select MII". When you are finished, ensure that you are handling it
> >> properly although the option is user visible...
> >
> > "config MII" and "select MII" are _not_ equivalent to the case at hand.
> > MII is defined in drivers/net/Kconfig itself so does not print any "symbol
> > unknown kind of warnings" ... so clearly no probs in "select" for it ...
>
> Then move the "config MII" to arch/i386/Kconfig and assume all drivers
> select'ing it would depend on X86_32.

Why should we do that?

> >> There are cases where "default .. if .." is the right idiom, but there
> >> are also cases where "select" is the right idiom. And for helper code
> >> like ATARI_KBD_CORE, "select" is the right idiom.
> >
> > ATARI_KBD_CORE, unlike MII, is defined only by some archs. And the
> > correct (most widely used or standard, in any case) idiom for that is
> > "default .. if ..". Or perhaps you can convert those helper code options in
> > arch/.../config's over to select too, as an exercise? :-)
>
> Perhaps not as an exercise, but actually for real.
>
> We had "fixed" such warnings in the past similar to your patch, but that
> was actually a mistake.
>
> And "correct" can easily be the opposite of "most widely used or standard"
> if you discover that you did it wrong in the past.

In that case the correct approach here too would be to _shift_
ATARI_KBD_CORE from arch/m68k/Kconfig to drivers/input/Kconfig
and *then* use "select" from the config options that require it in
drivers/input/keyboard/Kconfig and drivers/input/mouse/Kconfig

(and repeat for other such cases in arch/...)

Re: RFC: kconfig select warnings bogus?

[Trent Piepho]

On Sun, 20 May 2007, Satyam Sharma wrote:
> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > On Sun, May 20, 2007 at 05:25:24AM +0530, Satyam Sharma wrote:
> > > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > >> On Sun, May 20, 2007 at 05:06:33AM +0530, Satyam Sharma wrote:
> > >> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > >> There are cases where "default .. if .." is the right idiom, but there
> > >> are also cases where "select" is the right idiom. And for helper code
> > >> like ATARI_KBD_CORE, "select" is the right idiom.
> > >
> > > ATARI_KBD_CORE, unlike MII, is defined only by some archs. And the
> > > correct (most widely used or standard, in any case) idiom for that is
> > > "default .. if ..". Or perhaps you can convert those helper code options in
> > > arch/.../config's over to select too, as an exercise? :-)
> >
> > Perhaps not as an exercise, but actually for real.
> >
> > We had "fixed" such warnings in the past similar to your patch, but that
> > was actually a mistake.
> >
> > And "correct" can easily be the opposite of "most widely used or standard"
> > if you discover that you did it wrong in the past.
>
> In that case the correct approach here too would be to _shift_
> ATARI_KBD_CORE from arch/m68k/Kconfig to drivers/input/Kconfig
> and *then* use "select" from the config options that require it in
> drivers/input/keyboard/Kconfig and drivers/input/mouse/Kconfig
>
> (and repeat for other such cases in arch/...)

You don't need a huge '||' chain, you can always have more than one default
line:

config ATARI_KBD_CORE
	bool
	default y if KEYBOARD_ATARI
	default y if MOUSE_ATARI

Basically a line "config A \n select B" is transformed into "config B \n
default y if A".  It's the same number of lines, they're just in a new place.

Another alternative would be to shift the arch specific drivers to a file
that's arch specific.  e.g. move MOUSE_ATARI from drivers/input/mouse/Kconfig
to arch/m68k/Kconfig.

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Trent Piepho wrote:
> config ATARI_KBD_CORE
> 	bool
> 	default y if KEYBOARD_ATARI
> 	default y if MOUSE_ATARI
> 
> Basically a line "config A \n select B" is transformed into "config B \n
> default y if A".  It's the same number of lines, they're just in a new place.

Basically you replace

	A... depends on B

by

	B... serves A

The latter variant is a pain to maintain.  Dependencies change over
time, therefore we should note the dependency always at the dependent
option, not at the serving option.

Iterating upwards and downwards the dependency graph is the duty of
"make snafuconfig", not of the maintainers.

Besides, the "serves" form cannot stand in for constructs like

	A... depends on (B && C) || D
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Trent Piepho]

On Sun, 20 May 2007, Stefan Richter wrote:
> Trent Piepho wrote:
> > config ATARI_KBD_CORE
> > 	bool
> > 	default y if KEYBOARD_ATARI
> > 	default y if MOUSE_ATARI
> >
> > Basically a line "config A \n select B" is transformed into "config B \n
> > default y if A".  It's the same number of lines, they're just in a new place.
>
> Basically you replace
>
> 	A... depends on B
>
> by
>
> 	B... serves A
>
> The latter variant is a pain to maintain.  Dependencies change over
> time, therefore we should note the dependency always at the dependent
> option, not at the serving option.

The problem is that "B" will not exist on some architectures.  If you put the
dependency with "A", the dependency still exists when "B" is gone.  If the
dependency is with "B", it nicely goes away when "B" is gone.

> Iterating upwards and downwards the dependency graph is the duty of
> "make snafuconfig", not of the maintainers.
>
> Besides, the "serves" form cannot stand in for constructs like
>
> 	A... depends on (B && C) || D

This is for replacing "select" lines, which can't look like that.

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Trent Piepho wrote:
> On Sun, 20 May 2007, Stefan Richter wrote:
>> Basically you replace
>>
>> 	A... depends on B
>>
>> by
>>
>> 	B... serves A
>>
>> The latter variant is a pain to maintain.  Dependencies change over
>> time, therefore we should note the dependency always at the dependent
>> option, not at the serving option.
> 
> The problem is that "B" will not exist on some architectures.  If you put the
> dependency with "A", the dependency still exists when "B" is gone.  If the
> dependency is with "B", it nicely goes away when "B" is gone.

If "make whateverconfig" works correctly,...

>> Iterating upwards and downwards the dependency graph is the duty of
>> "make snafuconfig", not of the maintainers.

...multi-level dependencies are no problem for it.

There is nothing wrong with

	A... depends on B

	B... depends on C

	# CONFIG_C is not set

-> A is unavailable.
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Trent Piepho]

On Sun, 20 May 2007, Stefan Richter wrote:
>
> >> Iterating upwards and downwards the dependency graph is the duty of
> >> "make snafuconfig", not of the maintainers.
>
> ...multi-level dependencies are no problem for it.
>
> There is nothing wrong with
>
> 	A... depends on B
>
> 	B... depends on C
>
> 	# CONFIG_C is not set
>
> -> A is unavailable.

select doesn't appear to work quite like this.  For example:

config A
	bool "A"

config B
	bool "B"
	depends on A

config C
	bool "C"
	select B

In this case, it's possible to turn C on and A off.  B will be on, even
though it depends on A and A is off.

The kconfig docs say that "B..  depends on A" sets the maximum value of B
to be that of A.  Since A=0, the max value of B is 0.

The kconfig docs also say that "C..  select B" sets the minimum value of B
to be that of C.  Since C=2, the minimum value of B is 2.

So we have B>=2 and B<=0, which is obviously impossible.  Yet *config has
no problem with this, and will set B=2 even the 'depends' means B must be
0.  It seems like "select" will override any other dependencies.

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Trent Piepho wrote:
> config A
> 	bool "A"
> 
> config B
> 	bool "B"
> 	depends on A
> 
> config C
> 	bool "C"
> 	select B
> 
> In this case, it's possible to turn C on and A off.  B will be on, even
> though it depends on A and A is off.
> 
> The kconfig docs say that "B..  depends on A" sets the maximum value of B
> to be that of A.  Since A=0, the max value of B is 0.
> 
> The kconfig docs also say that "C..  select B" sets the minimum value of B
> to be that of C.  Since C=2, the minimum value of B is 2.
> 
> So we have B>=2 and B<=0, which is obviously impossible.  Yet *config has
> no problem with this, and will set B=2 even the 'depends' means B must be
> 0.  It seems like "select" will override any other dependencies.

If that's so, then we have /a/ an incomplete definition of the Kconfig
language (what is supposed to happen if "select" attempts to set an
impossible value?) and /b/ a bug in the make xyzconfig programs (they
generate invalid configs).
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Trent Piepho]

On Sun, 20 May 2007, Stefan Richter wrote:
> Trent Piepho wrote:
> > config A
> > 	bool "A"
> >
> > config B
> > 	bool "B"
> > 	depends on A
> >
> > config C
> > 	bool "C"
> > 	select B
> >
> > In this case, it's possible to turn C on and A off.  B will be on, even
> > though it depends on A and A is off.
> >
> > The kconfig docs say that "B..  depends on A" sets the maximum value of B
> > to be that of A.  Since A=0, the max value of B is 0.
> >
> > The kconfig docs also say that "C..  select B" sets the minimum value of B
> > to be that of C.  Since C=2, the minimum value of B is 2.
> >
> > So we have B>=2 and B<=0, which is obviously impossible.  Yet *config has
> > no problem with this, and will set B=2 even the 'depends' means B must be
> > 0.  It seems like "select" will override any other dependencies.
>
> If that's so, then we have /a/ an incomplete definition of the Kconfig
> language (what is supposed to happen if "select" attempts to set an
> impossible value?) and /b/ a bug in the make xyzconfig programs (they
> generate invalid configs).

This came up when I was working on the v4l-dvb tree's out of kernel build
system.  It uses the same Kconfig files, but I've created a config system in
perl to parse and evaluate them.  It will disable options that the kernel
config programs allow, and it is because I'm treating this situation
differently:  Since "B" is disabled because "A" is off, "C" must also be
disabled.

Another way of dealing with this would be to have 'select' follow the
dependency chain back up.  Turning on C selects B, B depends on A, so A is
also selected.

One problem with this is that "depends on" can take complex expressions.
Finding the solution is NP complete, which likely isn't a problem for the
sizes of realistic Kconfig files.  But there could easily be multiple
solutions, so which one is the right one?

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

Hi Trent,

On 5/21/07, Trent Piepho <xyzzy@speakeasy.org> wrote:
> On Sun, 20 May 2007, Stefan Richter wrote:
> > Trent Piepho wrote:
> > > config A
> > >     bool "A"
> > >
> > > config B
> > >     bool "B"
> > >     depends on A
> > >
> > > config C
> > >     bool "C"
> > >     select B
> > >
> > > In this case, it's possible to turn C on and A off.  B will be on, even
> > > though it depends on A and A is off.
> > >
> > > The kconfig docs say that "B..  depends on A" sets the maximum value of B
> > > to be that of A.  Since A=0, the max value of B is 0.
> > >
> > > The kconfig docs also say that "C..  select B" sets the minimum value of B
> > > to be that of C.  Since C=2, the minimum value of B is 2.
> > >
> > > So we have B>=2 and B<=0, which is obviously impossible.  Yet *config has
> > > no problem with this, and will set B=2 even the 'depends' means B must be
> > > 0.  It seems like "select" will override any other dependencies.
> >
> > If that's so, then we have /a/ an incomplete definition of the Kconfig
> > language (what is supposed to happen if "select" attempts to set an
> > impossible value?) and /b/ a bug in the make xyzconfig programs (they
> > generate invalid configs).
>
> This came up when I was working on the v4l-dvb tree's out of kernel build
> system.  It uses the same Kconfig files, but I've created a config system in
> perl to parse and evaluate them.  It will disable options that the kernel
> config programs allow, and it is because I'm treating this situation
> differently:  Since "B" is disabled because "A" is off, "C" must also be
> disabled.

Yeah, this is the classic "select" trap.

> Another way of dealing with this would be to have 'select' follow the
> dependency chain back up.  Turning on C selects B, B depends on A, so A is
> also selected.

You're right ... we'd discussed this earlier in:
http://lkml.org/lkml/2007/5/16/257
and http://lkml.org/lkml/2007/5/16/237

Also, because dependencies are of two types, those that _can_ be
meaningfully selected and those that can't, so we could select _all_
grand-parent dependencies A's of C when user picks C if they are all
selectable, but just refuse to proceed to even pick C itself if _any_
grand-parent dependency A of C is not a "select"able dependency.

i.e., a mix of the two solutions you mentioned above.

> One problem with this is that "depends on" can take complex expressions.
> Finding the solution is NP complete, which likely isn't a problem for the
> sizes of realistic Kconfig files.  But there could easily be multiple
> solutions, so which one is the right one?

Yes, the real problem is when a "depends on" somewhere along the
dependency tree has a || somewhere in it -- so we don't exactly know which
branch to follow from that point when creating the dependency tree. But I
suspect this shouldn't really be such a show-stopper ... I'll see if I can look
into scripts/kconfig/*.c to try and implement something of this sort.

Cheers,
Satyam

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 02:52:14AM -0700, Trent Piepho wrote:
> On Sun, 20 May 2007, Satyam Sharma wrote:
> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > > On Sun, May 20, 2007 at 05:25:24AM +0530, Satyam Sharma wrote:
> > > > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > > >> On Sun, May 20, 2007 at 05:06:33AM +0530, Satyam Sharma wrote:
> > > >> > On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > > >> There are cases where "default .. if .." is the right idiom, but there
> > > >> are also cases where "select" is the right idiom. And for helper code
> > > >> like ATARI_KBD_CORE, "select" is the right idiom.
> > > >
> > > > ATARI_KBD_CORE, unlike MII, is defined only by some archs. And the
> > > > correct (most widely used or standard, in any case) idiom for that is
> > > > "default .. if ..". Or perhaps you can convert those helper code options in
> > > > arch/.../config's over to select too, as an exercise? :-)
> > >
> > > Perhaps not as an exercise, but actually for real.
> > >
> > > We had "fixed" such warnings in the past similar to your patch, but that
> > > was actually a mistake.
> > >
> > > And "correct" can easily be the opposite of "most widely used or standard"
> > > if you discover that you did it wrong in the past.
> >
> > In that case the correct approach here too would be to _shift_
> > ATARI_KBD_CORE from arch/m68k/Kconfig to drivers/input/Kconfig
> > and *then* use "select" from the config options that require it in
> > drivers/input/keyboard/Kconfig and drivers/input/mouse/Kconfig
> >
> > (and repeat for other such cases in arch/...)
> 
> You don't need a huge '||' chain, you can always have more than one default
> line:
> 
> config ATARI_KBD_CORE
> 	bool
> 	default y if KEYBOARD_ATARI
> 	default y if MOUSE_ATARI
> 
> Basically a line "config A \n select B" is transformed into "config B \n
> default y if A".  It's the same number of lines, they're just in a new place.

Only if the select'ed option is a bool.
With tristates they are doubled.

And for user visible options like MII you need an additional helper 
option.

And the "just in a new place" makes a big difference in readability.
Compare the version of arch/mips/Kconfig in kernel 2.6.0 with the 
version in current kernels.
E.g. tell me whether there's any system that doesn't select any 
SYS_SUPPORTS_*_ENDIAN. There are possible errors that can be spotted 
much easier when using "select" at the right places.

> Another alternative would be to shift the arch specific drivers to a file
> that's arch specific.  e.g. move MOUSE_ATARI from drivers/input/mouse/Kconfig
> to arch/m68k/Kconfig.

Where is for a Kconfig user (person compiling her own kernel) the 
logical place to search for the Atari mouse driver option?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> A combined patch for all of those on top of 22-rc2 (adding Signed-off-by's
> for original authors too):
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>

[...]

> diff -ruNp a/drivers/input/keyboard/Kconfig
> b/drivers/input/keyboard/Kconfig
> --- a/drivers/input/keyboard/Kconfig    2007-05-20 04:11:55.000000000 +0530
> +++ b/drivers/input/keyboard/Kconfig    2007-05-20 04:24:54.000000000 +0530
> @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
> config KEYBOARD_ATARI
>     tristate "Atari keyboard"
>     depends on ATARI
> -    select ATARI_KBD_CORE
>     help
>       Say Y here if you are running Linux on any Atari and have a keyboard
>       attached.
> diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> --- a/drivers/input/mouse/Kconfig    2007-05-20 04:11:55.000000000 +0530
> +++ b/drivers/input/mouse/Kconfig    2007-05-20 04:25:09.000000000 +0530
> @@ -179,7 +179,6 @@ config MOUSE_AMIGA
> config MOUSE_ATARI
>     tristate "Atari mouse"
>     depends on ATARI
> -    select ATARI_KBD_CORE
>     help
>       Say Y here if you have an Atari and want its native mouse
>       supported by the kernel.
> diff -ruNp a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
> --- a/drivers/macintosh/Kconfig    2007-05-20 04:11:55.000000000 +0530
> +++ b/drivers/macintosh/Kconfig    2007-05-20 04:18:57.000000000 +0530
> @@ -113,7 +113,6 @@ config PMAC_SMU
> 
> config PMAC_APM_EMU
>     tristate "APM emulation"
> -    select SYS_SUPPORTS_APM_EMULATION
>     select APM_EMULATION
>     depends on ADB_PMU && PM
> 
> diff -ruNp a/drivers/net/Kconfig b/drivers/net/Kconfig
> --- a/drivers/net/Kconfig    2007-05-20 04:11:56.000000000 +0530
> +++ b/drivers/net/Kconfig    2007-05-20 04:22:15.000000000 +0530
> @@ -2280,7 +2280,6 @@ config GFAR_NAPI
> config UCC_GETH
>     tristate "Freescale QE Gigabit Ethernet"
>     depends on QUICC_ENGINE
> -    select UCC_FAST
>     help
>       This driver supports the Gigabit Ethernet mode of the QUICC Engine,
>       which is available on some Freescale SOCs.

Adrian already commented on this.  Let me just add that these above
changes are in so far *totally wrong* as they remove the "depends on
XYZ" which is implied by "select XYZ".

Remember, "select" is like "depends on", just with the added twist that
the 'make snafuconfig' scripts are instructed to (a) not hide the
depending option from the user and (b) try to enable the dependency for
the user if he tries to enable the depending option.
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Satyam Sharma wrote:
> > A combined patch for all of those on top of 22-rc2 (adding Signed-off-by's
> > for original authors too):
> >
> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
>
> [...]
>
> > diff -ruNp a/drivers/input/keyboard/Kconfig
> > b/drivers/input/keyboard/Kconfig
> > --- a/drivers/input/keyboard/Kconfig    2007-05-20 04:11:55.000000000 +0530
> > +++ b/drivers/input/keyboard/Kconfig    2007-05-20 04:24:54.000000000 +0530
> > @@ -167,7 +167,6 @@ config KEYBOARD_AMIGA
> > config KEYBOARD_ATARI
> >     tristate "Atari keyboard"
> >     depends on ATARI
> > -    select ATARI_KBD_CORE
> >     help
> >       Say Y here if you are running Linux on any Atari and have a keyboard
> >       attached.
> > diff -ruNp a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> > --- a/drivers/input/mouse/Kconfig    2007-05-20 04:11:55.000000000 +0530
> > +++ b/drivers/input/mouse/Kconfig    2007-05-20 04:25:09.000000000 +0530
> > @@ -179,7 +179,6 @@ config MOUSE_AMIGA
> > config MOUSE_ATARI
> >     tristate "Atari mouse"
> >     depends on ATARI
> > -    select ATARI_KBD_CORE
> >     help
> >       Say Y here if you have an Atari and want its native mouse
> >       supported by the kernel.
> > diff -ruNp a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
> > --- a/drivers/macintosh/Kconfig    2007-05-20 04:11:55.000000000 +0530
> > +++ b/drivers/macintosh/Kconfig    2007-05-20 04:18:57.000000000 +0530
> > @@ -113,7 +113,6 @@ config PMAC_SMU
> >
> > config PMAC_APM_EMU
> >     tristate "APM emulation"
> > -    select SYS_SUPPORTS_APM_EMULATION
> >     select APM_EMULATION
> >     depends on ADB_PMU && PM
> >
> > diff -ruNp a/drivers/net/Kconfig b/drivers/net/Kconfig
> > --- a/drivers/net/Kconfig    2007-05-20 04:11:56.000000000 +0530
> > +++ b/drivers/net/Kconfig    2007-05-20 04:22:15.000000000 +0530
> > @@ -2280,7 +2280,6 @@ config GFAR_NAPI
> > config UCC_GETH
> >     tristate "Freescale QE Gigabit Ethernet"
> >     depends on QUICC_ENGINE
> > -    select UCC_FAST
> >     help
> >       This driver supports the Gigabit Ethernet mode of the QUICC Engine,
> >       which is available on some Freescale SOCs.
>
> Adrian already commented on this.  Let me just add that these above
> changes are in so far *totally wrong* as they remove the "depends on
> XYZ" which is implied by "select XYZ".

Did you even try this patch?

Did you try to select UCC_GETH and see if UCC_FAST got
automatically selected or not?

Did you get any build problems (or bogus warnings, for that matter)?

Have you ever looked into arch/.../Kconfig's?

> Remember, "select" is like "depends on", just with the added twist that
> the 'make snafuconfig' scripts are instructed to (a) not hide the
> depending option from the user and (b) try to enable the dependency for
> the user if he tries to enable the depending option.

This is precisely what happens with this patch too.

Believe me, just go ahead and try it.

Report any errors, dissatisfaction, etc, if any.

Thanks.

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> > Satyam Sharma wrote:
> > > A combined patch for all of those on top of 22-rc2 (adding Signed-off-by's
> > > for original authors too):
> > >
> > > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
> >
> > [...]
> >
> > Adrian already commented on this.  Let me just add that these above
> > changes are in so far *totally wrong* as they remove the "depends on
> > XYZ" which is implied by "select XYZ".
>
> Did you even try this patch?
>
> Did you try to select UCC_GETH and see if UCC_FAST got
> automatically selected or not?
>
> Did you get any build problems (or bogus warnings, for that matter)?
>
> Have you ever looked into arch/.../Kconfig's?
>
> > Remember, "select" is like "depends on", just with the added twist that
> > the 'make snafuconfig' scripts are instructed to (a) not hide the
> > depending option from the user and (b) try to enable the dependency for
> > the user if he tries to enable the depending option.
>
> This is precisely what happens with this patch too.
>
> Believe me, just go ahead and try it.
>
> Report any errors, dissatisfaction, etc, if any.
>
> Thanks.

[ Just to add: ]

You'll learn about the "default .. if .." Kconfig idiom after you try this, but
when this patch works for you, an Acked-by would also be nice, thanks.

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> You'll learn about the "default .. if .." Kconfig idiom after you try
> this,

I have seen these in the rest of the patch which I didn't quote.

However you fix it --- don't remove "depends on" or "select".  You can
interchange them, but not remove them, unless there wasn't a dependency
to begin with.

Games with default values will break the next time a genius patch
submitter comes around with his ideas how people configure kernels.

PS:  I still believe that the saner approach would be to carry only the
dependencies, prompt texts and help texts in Kconfig files, maybe
amended by new machine-readable markers regarding the role of an option.
The presentation and ways to select and deselect should be entirely left
to UIs.
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Satyam Sharma wrote:
> > You'll learn about the "default .. if .." Kconfig idiom after you try
> > this,
>
> I have seen these in the rest of the patch which I didn't quote.
>
> However you fix it --- don't remove "depends on" or "select".  You can
> interchange them, but not remove them, unless there wasn't a dependency
> to begin with.

They _can_ be removed, with absolutely no build failures at all, because
of the "default y if ..." on the dependency that is defined in some
arch/.../Kconfig.

In fact the whole _idea_ is to remove any mention of the arch-specific
dependency from arch-agnostic stuff in drivers/.../Kconfig's (be it
"depends" or "select") so as not to generate those "unknown symbols"
warnings that would come on other archs. Hence, the normal practice to
use the "default y if ..." idiom (for "bool" helper code like the original
examples on this thread) in arch/.../Kconfig files.

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> However you fix it --- don't remove "depends on" or "select".  You can
>> interchange them, but not remove them, unless there wasn't a dependency
>> to begin with.
> 
> They _can_ be removed, with absolutely no build failures at all, because
> of the "default y if ..." on the dependency that is defined in some
> arch/.../Kconfig.

You are right, they can be removed, and the Kconfig files can be turned
into an unmaintainable mess.

	A requires B

Maintainable.

	B serves A

Logically equivalent, yet unmaintainable.
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Satyam Sharma wrote:
> > On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> However you fix it --- don't remove "depends on" or "select".  You can
> >> interchange them, but not remove them, unless there wasn't a dependency
> >> to begin with.
> >
> > They _can_ be removed, with absolutely no build failures at all, because
> > of the "default y if ..." on the dependency that is defined in some
> > arch/.../Kconfig.
>
> You are right, they can be removed, and the Kconfig files can be turned
> into an unmaintainable mess.
>
>         A requires B
>
> Maintainable.
>
>         B serves A
>
> Logically equivalent, yet unmaintainable.

Well, whether it is readable / maintainable is subjective and hence
debatable. I was only answering your *completely misplaced and
incorrect* original comment against the patch where you claimed
that the patch was "totally wrong" because of the way it removed
the "select" ... etc ...

And remember, like I said already, the whole _idea_ is such arch-
specific helper code be not mentioned from arch-agnostic Kconfig
files. You may not like it, but this is the standard / most common way
such cases are handled for tons of other cases in arch/.... Which
is why Adrian's way of solving this (shifting all such arch-specific
helper symbols also to drivers/... and then using depends on select
on it) is not viable.

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> I was only answering your *completely misplaced and
> incorrect* original comment against the patch where you claimed
> that the patch was "totally wrong" because of the way it removed
> the "select" ... etc ...

I believe I have explained why I labeled it as totally wrong.

> And remember, like I said already, the whole _idea_ is such arch-
> specific helper code be not mentioned from arch-agnostic Kconfig
> files. You may not like it, but this is the standard / most common way
> such cases are handled for tons of other cases in arch/...

The standard and maintainable way (for drivers at least) is

config A
	bool-or-tristate "option A"
	depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X

It is not a matter of me disliking something.  It's a matter of that we
can easily determine what A needs, but have a hard time to determine and
maintain the dependencies in the reverse way.

BTW, if A needs platform-specific helper N, then A is not
platform-agnostic.  If A is desired to be platform-agnostic, then either
A has to be implemented independently of N or N has to be made available
on all platforms.

> Which is why Adrian's way of solving this (shifting all such
> arch-specific helper symbols also to drivers/... and then using depends
> on select on it) is not viable.

I'm not advocating any specific fixes or pseudo-fixes.  I'm advocating
the notation of dependencies in the direction "A requires B".

Since you mention "select":  My opinion about the "select" dialect of
"depends on" is that the UIs should be improved and "select" should be
removed from the Kconfig language.  What do we "select"?  Typically we
"select" an option on which /n/ other options depend on but which itself
does depend on none or few options higher up.  The UIs could be able to
figure this out for themselves, or if necessary by a hint tacked onto
library-type options.  That is, instead of

	config A
		tristate "driver A"
		select L

	config B
		tristate "driver B"
		select L

	config L
		tristate "library L"

write

	config A
		tristate "driver A"
		depends on L

	config B
		tristate "driver B"
		depends on L

	config L
		tristate "library L"
		hint THIS_IS_A_LIBRARY

Now let UIs "make oldconfig", "make menuconfig", "make randconfig" deal
with the hint or ignore the hint --- according to the purpose and
usability requirements of the respective UI.  The "hint
THIS_IS_SOMETHING" isn't even necessary in many cases to detect roles of
options, because their position in the dependency graph is already
saying something about it.

These things really should be shifted into the UIs as much as possible,
because we can have a number of special-purpose UIs but we want
all-purpose Kconfig files.
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Satyam Sharma wrote:
> > I was only answering your *completely misplaced and
> > incorrect* original comment against the patch where you claimed
> > that the patch was "totally wrong" because of the way it removed
> > the "select" ... etc ...
>
> I believe I have explained why I labeled it as totally wrong.

Because it is not "readable"? Umm, "*totally wrong*" is a pretty strong
comment to make just because _you_ find something less readable /
maintainable! Anyway, your initial post did give the impression that
you weren't aware of the "default y if ..." mechanism at all, and so
somehow thought the removal of select would cause some graver
problems because somehow the "dependency on XYZ which was
implied by select XYZ" had been lost by the removal of select ... etc.

> > And remember, like I said already, the whole _idea_ is such arch-
> > specific helper code be not mentioned from arch-agnostic Kconfig
> > files. You may not like it, but this is the standard / most common way
> > such cases are handled for tons of other cases in arch/...
>
> The standard and maintainable way (for drivers at least) is
>
> config A
>         bool-or-tristate "option A"
>         depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X

??? I didn't get this entry, can you give a solid example (you can consider
the case at hand, MOUSE_ATARI and ATARI_KBD_CORE itself). Better
still, if you really think that the above is a better way to solve the
problem at
hand, why don't you submit a patch instead?

> It is not a matter of me disliking something.  It's a matter of that we
> can easily determine what A needs, but have a hard time to determine and
> maintain the dependencies in the reverse way.
>
> BTW, if A needs platform-specific helper N, then A is not
> platform-agnostic.  If A is desired to be platform-agnostic, then either
> A has to be implemented independently of N or N has to be made available
> on all platforms.

What you are suggesting is not practically possible / convenient. See, we
_want_ MOUSE_ATARI to appear in the arch-agnostic area which is along
with the other input devices and hence it is placed in drivers/input/Kconfig.
However, it does depend on arch-specific helper code. This is a real issue,
and we can't simply wish it away.

> > Which is why Adrian's way of solving this (shifting all such
> > arch-specific helper symbols also to drivers/... and then using depends
> > on select on it) is not viable.
>
> I'm not advocating any specific fixes or pseudo-fixes.  I'm advocating
> the notation of dependencies in the direction "A requires B".

See, if you really _still_ think this could / should be done some better
way (which is more maintainable, does not cause build failures and
also does not cause those bogus warnings to be printed etc etc),
then please submit a patch instead.

> These things really should be shifted into the UIs as much as possible,
> because we can have a number of special-purpose UIs but we want
> all-purpose Kconfig files.

Well, the shortcomings of Kconfig / build scripts that you mentioned are
are somewhat valid indeed, but as I said, unless you or someone submits
patches to clear up the above issues, we do need to solve problems within
that framework itself, isn't it?

Thanks,
Satyam

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> config A
>>         bool-or-tristate "option A"
>>         depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X
> 
> ??? I didn't get this entry,

"A is available if N is there or if it's a platform other than X."

That would be adequate if N is only present and required on platform X.

> can you give a solid example

Nothing exactly of this sort, but compare for example kernel/power/Kconfig:

config SOFTWARE_SUSPEND
	bool "Software Suspend (Hibernation)"
	depends on PM && SWAP && (((X86 || PPC64_SWSUSP) &&
		(!SMP || SUSPEND_SMP)) || ((FRV || PPC32) && !SMP))

Of course this could be written in a clearer fashion, for example as

	depends on PM
	depends on SWAP
	depends on (X86 && !SMP) ||
	           (X86 && SUSPEND_SMP) ||
	           (PPC64_SWSUSP && !SMP) ||
	           (PPC64_SWSUSP && SUSPEND_SMP) ||
	           (FRV   && !SMP) ||
	           (PPC32 && !SMP)

(Untested.)  Anyway, the dependencies which we are looking at here
should not (and partially even cannot) be declared in reverse.

> (you can consider
> the case at hand, MOUSE_ATARI and ATARI_KBD_CORE itself). Better
> still, if you really think that the above is a better way to solve the
> problem at hand, why don't you submit a patch instead?

Because I am lazy and trust that the platform maintainers know best
about the dependencies of MOUSE_ATARI.  Could also be related to that I
as an Amiga owner cannot really relate to Atari. ;-)  Could also have
something to do with my being busy enough fixing bugs in the drivers I
am familiar with.  (Granted, I should shut up, ignore how Kconfig is
more and more turned into a mess, and concentrate on my primary business.)
-- 
Stefan Richter
-=====-=-=== -=-= =-=--
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

[ I was wondering whether to reply or not (this has become sort of a
a dead thread and subsumed by Sam's proposal to scan all Kconfig
files, but still) ... ]

> Satyam Sharma wrote:
> > On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> config A
> >>         bool-or-tristate "option A"
> >>         depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X
> >
> > ??? I didn't get this entry,
>
> "A is available if N is there or if it's a platform other than X."
>
> That would be adequate if N is only present and required on platform X.

Umm, if A requires helper code N (which is available only on platform X),
then why/how do we want A to depend on platforms _other_ than X?
That "||" OR there is totally incorrect, because it allows A to be selected
even _without_ N, which won't even allow A to build properly! Remember,
something "depends on" or wishes to "select" some other thing only if
it *reuses code exported* by the dependency.

Beginning to wonder if you even _understood_ the problem that was
being solved there ... I seriously recommend that if / when you _really_
think you have some good idea to solve some problem, then submitting
a working patch is invariably always better (and causes lesser noise :-)

> > can you give a solid example
>
> Nothing exactly of this sort, but compare for example kernel/power/Kconfig:
>
> config SOFTWARE_SUSPEND
>         bool "Software Suspend (Hibernation)"
>         depends on PM && SWAP && (((X86 || PPC64_SWSUSP) &&
>                 (!SMP || SUSPEND_SMP)) || ((FRV || PPC32) && !SMP))
>
> Of course this could be written in a clearer fashion, for example as
>
>         depends on PM
>         depends on SWAP
>         depends on (X86 && !SMP) ||
>                    (X86 && SUSPEND_SMP) ||
>                    (PPC64_SWSUSP && !SMP) ||
>                    (PPC64_SWSUSP && SUSPEND_SMP) ||
>                    (FRV   && !SMP) ||
>                    (PPC32 && !SMP)

Ok, so perhaps you actually meant X && N_ON_X above. But your
suggestion is _still_ wrong. Using "depends on" instead of "select" does
get rid of the warnings, but that conversion would *not* maintain current
behaviour (that of the config option being visible and automatically
selecting its dependency -- "default y if ..." otoh does preserve current
behaviour and hence _can_ replace select).

This is precisely what Trent meant that the kind of dependencies you
were suggesting in your mails can't be "select"ed in the first place.

Anyway, I've had enough of this thread. The patch that I had sent was
a 7-line *triviality* that merely (1) preserved current behaviour, (2) did
not cause _any_ build problems, and (3) got rid of bogus warnings in
a way that was completely standard when dealing with arch/.../Kconfig's
and better than the initial suggestion by Sam. Can't quite understand
how this became such a fiasco that ruined my Sunday :-)

Satyam

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> > On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> >> config A
>> >>         bool-or-tristate "option A"
>> >>         depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X
[...]
> Umm, if A requires helper code N (which is available only on platform X),
[And, I assumed, 'A' works without helper 'N' on all platforms except
'X'.  This is a synthetic example which might not have a real-world
application.]
> then why/how do we want A to depend on platforms _other_ than X?

The expression means:  'A' can be enabled on all platforms.  Except on
platform_X, there it can only be enabled if helper_N is built too.

[...]
>>         depends on PM
>>         depends on SWAP
>>         depends on (X86 && !SMP) ||
>>                    (X86 && SUSPEND_SMP) ||
>>                    (PPC64_SWSUSP && !SMP) ||
>>                    (PPC64_SWSUSP && SUSPEND_SMP) ||
>>                    (FRV   && !SMP) ||
>>                    (PPC32 && !SMP)
> 
> Ok, so perhaps you actually meant X && N_ON_X above.

No.  I meant "(X && N_ON_X) || Any_one_platform_other_than_X" above.

The 2nd example is a whitelist, while the 1st example is some kind of
blacklist.  A real-world example of blacklist style:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=a73df4dfdb0e01a1cbf119416a00e520a5e22306
(Lived only 2 days in Linus' tree.)
-- 
Stefan Richter
-=====-=-=== -=-= =-==-
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/22/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Satyam Sharma wrote:
> > On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> > On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> >> config A
> >> >>         bool-or-tristate "option A"
> >> >>         depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X
> [...]
> > Umm, if A requires helper code N (which is available only on platform X),
> [And, I assumed, 'A' works without helper 'N' on all platforms except
> 'X'.
> This is a synthetic example which might not have a real-world application.]

Well, it is not relevant / equivalent to any of the four symbols that caused the
warnings that this thread is about, at least.

> > then why/how do we want A to depend on platforms _other_ than X?
>
> The expression means:  'A' can be enabled on all platforms.  Except on
> platform_X, there it can only be enabled if helper_N is built too.
> [...]
> No.  I meant "(X && N_ON_X) || Any_one_platform_other_than_X" above.

Yeah, and as I said, this was totally *not* the problem being discussed /
solved in this thread (and by that patch).

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> On 5/22/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> >> > On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> >> >>         depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X
...
>> This is a synthetic example which might not have a real-world application.]
> 
> Well, it is not relevant / equivalent to any of the four symbols that caused the
> warnings that this thread is about, at least.

(Have I ever said that this concrete expression can be used in whatever
fix?)

...
> and as I said, this was totally *not* the problem being discussed /
> solved in this thread (and by that patch).

My point was not about that particular expression. My point was:

	I'm not advocating any specific fixes or pseudo-fixes.
	I'm advocating the notation of dependencies in the direction
	"A requires B".

When I said "The standard and maintainable way (for drivers at least)
is..." I didn't mean the example expression, I meant the *direction* in
which the example was stating dependencies.
-- 
Stefan Richter
-=====-=-=== -=-= =-==-
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/22/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Satyam Sharma wrote:
> > On 5/22/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> >> > On 5/20/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> >> >>         depends on !PLATFORM_X || HELPER_N_ON_PLATFORM_X
> ...
> >> This is a synthetic example which might not have a real-world application.]
> >
> > Well, it is not relevant / equivalent to any of the four symbols that caused the
> > warnings that this thread is about, at least.
>
> (Have I ever said that this concrete expression can be used in whatever
> fix?)
>
> ...
> > and as I said, this was totally *not* the problem being discussed /
> > solved in this thread (and by that patch).
>
> My point was not about that particular expression. My point was:
>
>         I'm not advocating any specific fixes or pseudo-fixes.
>         I'm advocating the notation of dependencies in the direction
>         "A requires B".
>
> When I said "The standard and maintainable way (for drivers at least)
> is..." I didn't mean the example expression, I meant the *direction* in
> which the example was stating dependencies.

In that case I wish "the points" you make on threads are relevant to them.

Re: RFC: kconfig select warnings bogus?

[Stefan Richter]

Satyam Sharma wrote:
> On 5/22/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> When I said "The standard and maintainable way (for drivers at least)
>> is..." I didn't mean the example expression, I meant the *direction* in
>> which the example was stating dependencies.
> 
> In that case I wish "the points" you make on threads are relevant to them.

My initial reply was on a patch which removed the A-to-B declaration of
dependency.
-- 
Stefan Richter
-=====-=-=== -=-= =-==-
http://arcgraph.de/sr/

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/22/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Satyam Sharma wrote:
> > On 5/22/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> When I said "The standard and maintainable way (for drivers at least)
> >> is..." I didn't mean the example expression, I meant the *direction* in
> >> which the example was stating dependencies.
> >
> > In that case I wish "the points" you make on threads are relevant to them.
>
> My initial reply was on a patch which removed the A-to-B declaration of
> dependency.

And if you think you can have the "direction of declaration of
dependencies" the way you like it, and also (1) preserve current
"select"-like behaviour, (2) introduce no build issues, and (3) get rid of
the bogus warnings, then: Patches are welcome. (No, suggestions that
don't work, or not fully applicable to the case at hand in this thread will
NOT do).

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 03:09:10PM +0200, Stefan Richter wrote:
>...
> Since you mention "select":  My opinion about the "select" dialect of
> "depends on" is that the UIs should be improved and "select" should be
> removed from the Kconfig language.  What do we "select"?  Typically we
> "select" an option on which /n/ other options depend on but which itself
> does depend on none or few options higher up.  The UIs could be able to
> figure this out for themselves, or if necessary by a hint tacked onto
> library-type options.  That is, instead of
> 
> 	config A
> 		tristate "driver A"
> 		select L
> 
> 	config B
> 		tristate "driver B"
> 		select L
> 
> 	config L
> 		tristate "library L"
> 
> write
> 
> 	config A
> 		tristate "driver A"
> 		depends on L
> 
> 	config B
> 		tristate "driver B"
> 		depends on L
> 
> 	config L
> 		tristate "library L"
> 		hint THIS_IS_A_LIBRARY
> 
> Now let UIs "make oldconfig", "make menuconfig", "make randconfig" deal
> with the hint or ignore the hint --- according to the purpose and
> usability requirements of the respective UI.  The "hint
> THIS_IS_SOMETHING" isn't even necessary in many cases to detect roles of
> options, because their position in the dependency graph is already
> saying something about it.
>...

Example where your proposal wouldn't work well:

config MIPS_ATLAS
        bool "MIPS Atlas board"
        select SYS_SUPPORTS_LITTLE_ENDIAN
        select SYS_SUPPORTS_BIG_ENDIAN

config SYS_SUPPORTS_BIG_ENDIAN
        bool

config CPU_BIG_ENDIAN
        bool "Big endian"
        depends on SYS_SUPPORTS_BIG_ENDIAN


Where's the value of allowing an UI to ignore hints? If an UI would 
ignore all hints on SYS_SUPPORTS_BIG_ENDIAN you would no longer be able 
to configure a kernel on this architecture.

And there's no way for an UI to figure out the direction of the options - 
with "select" the only reasonable direction is fixed for all UIs.

"select" might have room for improvements especially in the area of 
dependencies of select'ed options, but replacing it with overloading
"depends on" plus hints wouldn't be a good solution.

> Stefan Richter

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sat, May 19, 2007 at 11:09:44AM -0700, Andrew Morton wrote:
> On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > We see a lot of these lately:
> >   GEN     /home/bor/build/linux-2.6.22/Makefile
> > scripts/kconfig/conf -s arch/i386/Kconfig
> > drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
> > drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > 
> > 
> > Do this warning really add any value or should we just ignore them like this?
> > 
> 
> They always indicate Kconfig bugs, don't they?  If so, we should keep the
> warning.
>...

No, they aren't always.

Look for example at the last one in drivers/input/mouse/Kconfig:

config MOUSE_ATARI
        tristate "Atari mouse"
        depends on ATARI
        select ATARI_KBD_CORE

This is perfectly correct (the select'ed symbol is only unavailable when 
the dependency can't be fulfilled), and all things to "fix" the warning 
will make it worse.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sat, May 19, 2007 at 11:09:44AM -0700, Andrew Morton wrote:
> > On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > > We see a lot of these lately:
> > >   GEN     /home/bor/build/linux-2.6.22/Makefile
> > > scripts/kconfig/conf -s arch/i386/Kconfig
> > > drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
> > > drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> > > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > > drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > >
> > >
> > > Do this warning really add any value or should we just ignore them like this?
> > >
> >
> > They always indicate Kconfig bugs, don't they?  If so, we should keep the
> > warning.
> >...
>
> No, they aren't always.
>
> Look for example at the last one in drivers/input/mouse/Kconfig:
>
> config MOUSE_ATARI
>         tristate "Atari mouse"
>         depends on ATARI
>         select ATARI_KBD_CORE
>
> This is perfectly correct (the select'ed symbol is only unavailable when
> the dependency can't be fulfilled), and all things to "fix" the warning
> will make it worse.

Not sure what you mean here. The select'ed symbol here is unavailable
because not all arch's define it. However, the symbol that selects it is
in drivers/input/... and hence run for all arch's. That's the simple issue
here that is resolved by this fix.

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> > On Sat, May 19, 2007 at 11:09:44AM -0700, Andrew Morton wrote:
> > > On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> wrote:
> > >
> > > > We see a lot of these lately:
> > > >   GEN     /home/bor/build/linux-2.6.22/Makefile
> > > > scripts/kconfig/conf -s arch/i386/Kconfig
> > > > drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
> > > > drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> > > > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > > > drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> > > >
> > > >
> > > > Do this warning really add any value or should we just ignore them like this?
> > > >
> > >
> > > They always indicate Kconfig bugs, don't they?  If so, we should keep the
> > > warning.
> > >...
> >
> > No, they aren't always.
> >
> > Look for example at the last one in drivers/input/mouse/Kconfig:
> >
> > config MOUSE_ATARI
> >         tristate "Atari mouse"
> >         depends on ATARI
> >         select ATARI_KBD_CORE
> >
> > This is perfectly correct (the select'ed symbol is only unavailable when
> > the dependency can't be fulfilled), and all things to "fix" the warning
> > will make it worse.
>
> Not sure what you mean here. The select'ed symbol here is unavailable
> because not all arch's define it. However, the symbol that selects it is
> in drivers/input/... and hence run for all arch's. That's the simple issue
> here that is resolved by this fix.

And by the way, this is precisely the case for _all_ the select issues in this
patch (the four that were raised by Sam) -- i.e. only one arch defining a
config option that is selected by stuff in drivers/... which is common for
all archs, and hence the warnings on other archs.

I don't see any problem at all that is introduced / "made worse" by the
patch, it just rectifies the problem to get rid of the warnings.

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 04:51:44AM +0530, Satyam Sharma wrote:
> On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
>> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>> > On Sat, May 19, 2007 at 11:09:44AM -0700, Andrew Morton wrote:
>> > > On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> 
>> wrote:
>> > >
>> > > > We see a lot of these lately:
>> > > >   GEN     /home/bor/build/linux-2.6.22/Makefile
>> > > > scripts/kconfig/conf -s arch/i386/Kconfig
>> > > > drivers/macintosh/Kconfig:116:warning: 'select' used by config 
>> symbol 'PMAC_APM_EMU' refers to undefined symbol 
>> 'SYS_SUPPORTS_APM_EMULATION'
>> > > > drivers/net/Kconfig:2283:warning: 'select' used by config symbol 
>> 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
>> > > > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config 
>> symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
>> > > > drivers/input/mouse/Kconfig:182:warning: 'select' used by config 
>> symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
>> > > >
>> > > >
>> > > > Do this warning really add any value or should we just ignore them 
>> like this?
>> > > >
>> > >
>> > > They always indicate Kconfig bugs, don't they?  If so, we should keep 
>> the
>> > > warning.
>> > >...
>> >
>> > No, they aren't always.
>> >
>> > Look for example at the last one in drivers/input/mouse/Kconfig:
>> >
>> > config MOUSE_ATARI
>> >         tristate "Atari mouse"
>> >         depends on ATARI
>> >         select ATARI_KBD_CORE
>> >
>> > This is perfectly correct (the select'ed symbol is only unavailable when
>> > the dependency can't be fulfilled), and all things to "fix" the warning
>> > will make it worse.
>>
>> Not sure what you mean here. The select'ed symbol here is unavailable
>> because not all arch's define it. However, the symbol that selects it is
>> in drivers/input/... and hence run for all arch's. That's the simple issue
>> here that is resolved by this fix.
>
> And by the way, this is precisely the case for _all_ the select issues in 
> this
> patch (the four that were raised by Sam) -- i.e. only one arch defining a
> config option that is selected by stuff in drivers/... which is common for
> all archs, and hence the warnings on other archs.
>
> I don't see any problem at all that is introduced / "made worse" by the
> patch, it just rectifies the problem to get rid of the warnings.

There's no doubt that it gets rid of the warnings.

But the warnings don't point to problems and the "fixes" for the 
warnings make the Kconfig's worse.

If fixing warnings makes something not better but worse, it's always 
worth a thought whether to disable the warning.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, May 20, 2007 at 04:51:44AM +0530, Satyam Sharma wrote:
> > On 5/20/07, Satyam Sharma <satyam.sharma@gmail.com> wrote:
> >> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> >> > On Sat, May 19, 2007 at 11:09:44AM -0700, Andrew Morton wrote:
> >> > > On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org>
> >> wrote:
> >> > >
> >> > > > We see a lot of these lately:
> >> > > >   GEN     /home/bor/build/linux-2.6.22/Makefile
> >> > > > scripts/kconfig/conf -s arch/i386/Kconfig
> >> > > > drivers/macintosh/Kconfig:116:warning: 'select' used by config
> >> symbol 'PMAC_APM_EMU' refers to undefined symbol
> >> 'SYS_SUPPORTS_APM_EMULATION'
> >> > > > drivers/net/Kconfig:2283:warning: 'select' used by config symbol
> >> 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> >> > > > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config
> >> symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> >> > > > drivers/input/mouse/Kconfig:182:warning: 'select' used by config
> >> symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> >> > > >
> >> > > >
> >> > > > Do this warning really add any value or should we just ignore them
> >> like this?
> >> > > >
> >> > >
> >> > > They always indicate Kconfig bugs, don't they?  If so, we should keep
> >> the
> >> > > warning.
> >> > >...
> >> >
> >> > No, they aren't always.
> >> >
> >> > Look for example at the last one in drivers/input/mouse/Kconfig:
> >> >
> >> > config MOUSE_ATARI
> >> >         tristate "Atari mouse"
> >> >         depends on ATARI
> >> >         select ATARI_KBD_CORE
> >> >
> >> > This is perfectly correct (the select'ed symbol is only unavailable when
> >> > the dependency can't be fulfilled), and all things to "fix" the warning
> >> > will make it worse.
> >>
> >> Not sure what you mean here. The select'ed symbol here is unavailable
> >> because not all arch's define it. However, the symbol that selects it is
> >> in drivers/input/... and hence run for all arch's. That's the simple issue
> >> here that is resolved by this fix.
> >
> > And by the way, this is precisely the case for _all_ the select issues in
> > this
> > patch (the four that were raised by Sam) -- i.e. only one arch defining a
> > config option that is selected by stuff in drivers/... which is common for
> > all archs, and hence the warnings on other archs.
> >
> > I don't see any problem at all that is introduced / "made worse" by the
> > patch, it just rectifies the problem to get rid of the warnings.
>
> There's no doubt that it gets rid of the warnings.

Ah, ok, so you're saying that these warnings would not lead to build
failures in any case, so we might as well not print the warnings at all?
I can't confirm that, have never tried to build these drivers here (don't
have m68k / powerpc boxen), so perhaps you're right.

> But the warnings don't point to problems and the "fixes" for the
> warnings make the Kconfig's worse.
>
> If fixing warnings makes something not better but worse, it's always
> worth a thought whether to disable the warning.

There is nothing that is "made worse" by the "select -> default .. if .."
kind of conversion here.

In any case, the problem with Sam's patch is that it gets rid of
warnings for all cases where someone is trying to "select" a config
option that does not exist at all (and perhaps some of those cases
_could_ lead to build breakages).

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sun, May 20, 2007 at 04:47:01AM +0530, Satyam Sharma wrote:
> On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
>> On Sat, May 19, 2007 at 11:09:44AM -0700, Andrew Morton wrote:
>> > On Sat, 19 May 2007 17:15:23 +0200 Sam Ravnborg <sam@ravnborg.org> 
>> wrote:
>> >
>> > > We see a lot of these lately:
>> > >   GEN     /home/bor/build/linux-2.6.22/Makefile
>> > > scripts/kconfig/conf -s arch/i386/Kconfig
>> > > drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 
>> 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
>> > > drivers/net/Kconfig:2283:warning: 'select' used by config symbol 
>> 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
>> > > drivers/input/keyboard/Kconfig:170:warning: 'select' used by config 
>> symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
>> > > drivers/input/mouse/Kconfig:182:warning: 'select' used by config 
>> symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
>> > >
>> > >
>> > > Do this warning really add any value or should we just ignore them 
>> like this?
>> > >
>> >
>> > They always indicate Kconfig bugs, don't they?  If so, we should keep 
>> the
>> > warning.
>> >...
>>
>> No, they aren't always.
>>
>> Look for example at the last one in drivers/input/mouse/Kconfig:
>>
>> config MOUSE_ATARI
>>         tristate "Atari mouse"
>>         depends on ATARI
>>         select ATARI_KBD_CORE
>>
>> This is perfectly correct (the select'ed symbol is only unavailable when
>> the dependency can't be fulfilled), and all things to "fix" the warning
>> will make it worse.
>
> Not sure what you mean here. The select'ed symbol here is unavailable
> because not all arch's define it. However, the symbol that selects it is
> in drivers/input/... and hence run for all arch's. That's the simple issue
> here that is resolved by this fix.

This is OK, because when the select'ed symbol is not available, the 
dependencies can't be fulfilled.

There is no issue here that needs to be fixed.
And that's exactly what Sam's patch is about.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Andrew Morton]

On Sun, 20 May 2007 01:05:37 +0200 Adrian Bunk <bunk@stusta.de> wrote:

> Look for example at the last one in drivers/input/mouse/Kconfig:
> 
> config MOUSE_ATARI
>         tristate "Atari mouse"
>         depends on ATARI
>         select ATARI_KBD_CORE
> 
> This is perfectly correct (the select'ed symbol is only unavailable when 
> the dependency can't be fulfilled), and all things to "fix" the warning 
> will make it worse.

If ATARI is unset then we shouldn't be generating the "'select' used by
config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'"
warnings, should we?

Re: RFC: kconfig select warnings bogus?

[Adrian Bunk]

On Sat, May 19, 2007 at 04:22:39PM -0700, Andrew Morton wrote:
> On Sun, 20 May 2007 01:05:37 +0200 Adrian Bunk <bunk@stusta.de> wrote:
> 
> > Look for example at the last one in drivers/input/mouse/Kconfig:
> > 
> > config MOUSE_ATARI
> >         tristate "Atari mouse"
> >         depends on ATARI
> >         select ATARI_KBD_CORE
> > 
> > This is perfectly correct (the select'ed symbol is only unavailable when 
> > the dependency can't be fulfilled), and all things to "fix" the warning 
> > will make it worse.
> 
> If ATARI is unset then we shouldn't be generating the "'select' used by
> config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'"
> warnings, should we?

Exactly.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: RFC: kconfig select warnings bogus?

[Satyam Sharma]

On 5/20/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sat, May 19, 2007 at 04:22:39PM -0700, Andrew Morton wrote:
> > On Sun, 20 May 2007 01:05:37 +0200 Adrian Bunk <bunk@stusta.de> wrote:
> >
> > > Look for example at the last one in drivers/input/mouse/Kconfig:
> > >
> > > config MOUSE_ATARI
> > >         tristate "Atari mouse"
> > >         depends on ATARI
> > >         select ATARI_KBD_CORE
> > >
> > > This is perfectly correct (the select'ed symbol is only unavailable when
> > > the dependency can't be fulfilled), and all things to "fix" the warning
> > > will make it worse.
> >
> > If ATARI is unset then we shouldn't be generating the "'select' used by
> > config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'"
> > warnings, should we?
>
> Exactly.

But I suspect we do wish to get rid of the bogus warnings? (similar to
other places in arch/.../Kconfig's where such cases are handled) ...
_that_ (suppressing these four bogus warnings) was the purpose of that
patch in the first place .....

Re: RFC: kconfig select warnings bogus?

[Russell King]

On Sun, May 20, 2007 at 02:02:35AM +0200, Adrian Bunk wrote:
> On Sat, May 19, 2007 at 04:22:39PM -0700, Andrew Morton wrote:
> > On Sun, 20 May 2007 01:05:37 +0200 Adrian Bunk <bunk@stusta.de> wrote:
> > 
> > > Look for example at the last one in drivers/input/mouse/Kconfig:
> > > 
> > > config MOUSE_ATARI
> > >         tristate "Atari mouse"
> > >         depends on ATARI
> > >         select ATARI_KBD_CORE
> > > 
> > > This is perfectly correct (the select'ed symbol is only unavailable when 
> > > the dependency can't be fulfilled), and all things to "fix" the warning 
> > > will make it worse.
> > 
> > If ATARI is unset then we shouldn't be generating the "'select' used by
> > config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'"
> > warnings, should we?
> 
> Exactly.

If we do that we need to try a million and one Kconfig option combinations
to find undefined symbols.  Not practical.

(And no, allyconfig really doesn't hack it on non-x86 platforms, no matter
how much people whinge that it should do.  It's a pipedream to make it so.)

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: RFC: kconfig select warnings bogus?

[Russell King]

On Sat, May 19, 2007 at 04:22:39PM -0700, Andrew Morton wrote:
> On Sun, 20 May 2007 01:05:37 +0200 Adrian Bunk <bunk@stusta.de> wrote:
> 
> > Look for example at the last one in drivers/input/mouse/Kconfig:
> > 
> > config MOUSE_ATARI
> >         tristate "Atari mouse"
> >         depends on ATARI
> >         select ATARI_KBD_CORE
> > 
> > This is perfectly correct (the select'ed symbol is only unavailable when 
> > the dependency can't be fulfilled), and all things to "fix" the warning 
> > will make it worse.
> 
> If ATARI is unset then we shouldn't be generating the "'select' used by
> config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'"
> warnings, should we?

Playing devils advocate here.  What if "ATARI_KBD_CORE" never exists?
Let's say you run 'make kconfig' and you select the ATARI option.  When
does the lack of ATARI_KBD_CORE get noticed and what is the expected
result?

Let's put it another way.  Given the complexities of the configuration
system as it is today, if we do not generate a warning at parse time,
how do we find things like:

config SHARPSL_PM
        bool
        select APM_EMULATION

config PXA_SHARP_C7xx
        bool
        select PXA_SSP
        select SHARPSL_PM

config MACH_CORGI
        bool "Enable Sharp SL-C700 (Corgi) Support"
        depends on PXA_SHARPSL_25x
        select PXA_SHARP_C7xx

and (lets say for the sake of argument) APM_EMULATION were to go away.

Do we really need an exhaustive set of configuration combinations to
run through Kconfig to find possible missing symbols?  Or do we need a
Kconfig lint to find them?

If we're going to make Kconfig warn on missing symbols only when they're
attempted to be selected, you'll have to choose one of those two options.
Choosing none is not an option.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: RFC: kconfig select warnings bogus?

[Sam Ravnborg]

On Sun, May 20, 2007 at 10:40:33AM +0100, Russell King wrote:
> 
> Do we really need an exhaustive set of configuration combinations to
> run through Kconfig to find possible missing symbols?  Or do we need a
> Kconfig lint to find them?
> 
> If we're going to make Kconfig warn on missing symbols only when they're
> attempted to be selected, you'll have to choose one of those two options.
> Choosing none is not an option.
The best solution would be to give kconfig full knowledge of all
Kconfig files. Then the "undefined symbol" is no longer a warning
but an error.

That would require a bit of kconfig surgery that we should do one day.
But giving kconfig full knowledge would gain a lot in several scenarios.
Turning the undefined symbol to an error is just one part of it.

	Sam

Re: RFC: kconfig select warnings bogus?

[Geert Uytterhoeven]

On Sun, 20 May 2007, Russell King wrote:
> On Sat, May 19, 2007 at 04:22:39PM -0700, Andrew Morton wrote:
> > On Sun, 20 May 2007 01:05:37 +0200 Adrian Bunk <bunk@stusta.de> wrote:
> > > Look for example at the last one in drivers/input/mouse/Kconfig:
> > > 
> > > config MOUSE_ATARI
> > >         tristate "Atari mouse"
> > >         depends on ATARI
> > >         select ATARI_KBD_CORE
> > > 
> > > This is perfectly correct (the select'ed symbol is only unavailable when 
> > > the dependency can't be fulfilled), and all things to "fix" the warning 
> > > will make it worse.
> > 
> > If ATARI is unset then we shouldn't be generating the "'select' used by
> > config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'"
> > warnings, should we?
> 
> Playing devils advocate here.  What if "ATARI_KBD_CORE" never exists?
> Let's say you run 'make kconfig' and you select the ATARI option.  When
> does the lack of ATARI_KBD_CORE get noticed and what is the expected
> result?
> 
> Let's put it another way.  Given the complexities of the configuration
> system as it is today, if we do not generate a warning at parse time,
> how do we find things like:
> 
> config SHARPSL_PM
>         bool
>         select APM_EMULATION
> 
> config PXA_SHARP_C7xx
>         bool
>         select PXA_SSP
>         select SHARPSL_PM
> 
> config MACH_CORGI
>         bool "Enable Sharp SL-C700 (Corgi) Support"
>         depends on PXA_SHARPSL_25x
>         select PXA_SHARP_C7xx
> 
> and (lets say for the sake of argument) APM_EMULATION were to go away.

It will probably fail to build :-)

> Do we really need an exhaustive set of configuration combinations to
> run through Kconfig to find possible missing symbols?  Or do we need a
> Kconfig lint to find them?

Now let's say PXA_SHARPSL_25x goes away (or someone made a typo
in her `depends on')...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: RFC: kconfig select warnings bogus?

[Roman Zippel]

Hi,

On Sat, 19 May 2007, Sam Ravnborg wrote:

> We see a lot of these lately:
>   GEN     /home/bor/build/linux-2.6.22/Makefile
> scripts/kconfig/conf -s arch/i386/Kconfig
> drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION'
> drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST'
> drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE'
> 
> 
> Do this warning really add any value or should we just ignore them like this?

The problem is that a select operation on a non bool/tristate symbol is 
undefined. A runtime error would probably be even more ignored than this.
"Proving" that this select can't be activated is theoretically possible, 
but not really practical. An alternative might be to reverse the 
dependency again and let it act like a "depends on".
Just removing the warning is definitely not the right answer.

bye, Roman