usbhid_set_leds problems

usbhid_set_leds problems

[Adrian Bunk]

With CONFIG_USB_HID=n I'm getting the following build error:

<--  snip  -->

...
  LD      .tmp_vmlinux1
drivers/built-in.o: In function `bright_probe':
drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
drivers/built-in.o: In function `dell_probe':
drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
drivers/built-in.o: In function `lg_probe':
drivers/hid/hid-lg.c:252: undefined reference to `usbhid_set_leds'
drivers/hid/hid-lg.c:252: more undefined references to `usbhid_set_leds' follow
...
make[1]: *** [.tmp_vmlinux1] Error 1

<--  snip  -->


And I'm not sure commit 6edfa8dc33803a49ad936ead9840e453bee6ca3b
(HID: move reset leds quirk) was a good idea at all since I don't
see cases like HID_DELL=y, CONFIG_USB_HID=m could now work at all.


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: usbhid_set_leds problems

[Adrian Bunk]

On Wed, Oct 15, 2008 at 07:13:46AM +0300, Adrian Bunk wrote:
> With CONFIG_USB_HID=n I'm getting the following build error:
> 
> <--  snip  -->
> 
> ...
>   LD      .tmp_vmlinux1
> drivers/built-in.o: In function `bright_probe':
> drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> drivers/built-in.o: In function `dell_probe':
> drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> drivers/built-in.o: In function `lg_probe':
> drivers/hid/hid-lg.c:252: undefined reference to `usbhid_set_leds'
> drivers/hid/hid-lg.c:252: more undefined references to `usbhid_set_leds' follow
> ...
> make[1]: *** [.tmp_vmlinux1] Error 1
> 
> <--  snip  -->
> 
> 
> And I'm not sure commit 6edfa8dc33803a49ad936ead9840e453bee6ca3b
> (HID: move reset leds quirk) was a good idea at all since I don't
> see cases like HID_DELL=y, CONFIG_USB_HID=m could now work at all.

Scrap that mail, I shouldn't send patches before the first cup of tea...

I just found the actual 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: usbhid_set_leds problems

[Jiri Kosina]

On Wed, 15 Oct 2008, Adrian Bunk wrote:

> >   LD      .tmp_vmlinux1
> > drivers/built-in.o: In function `bright_probe':
> > drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> > drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> > drivers/built-in.o: In function `dell_probe':
> > drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> > drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> > drivers/built-in.o: In function `lg_probe':
> > drivers/hid/hid-lg.c:252: undefined reference to `usbhid_set_leds'
> > drivers/hid/hid-lg.c:252: more undefined references to `usbhid_set_leds' follow
> > ...
> > make[1]: *** [.tmp_vmlinux1] Error 1
> > And I'm not sure commit 6edfa8dc33803a49ad936ead9840e453bee6ca3b
> > (HID: move reset leds quirk) was a good idea at all since I don't
> > see cases like HID_DELL=y, CONFIG_USB_HID=m could now work at all.
> Scrap that mail, I shouldn't send patches before the first cup of tea...
> I just found the actual problem.

Hmm, so what was the culprit actually? The build error shouldn't be there.

Thanks,

-- 
Jiri Kosina
SUSE Labs

[2.6 patch] HID: fix default building of all quirky devices

[Adrian Bunk]

Commit 9be7bbd54df3c9c393ccd19acc49f90c517d1291
(HID: build drivers for all quirky devices by default)
was wrong in that it select'ed the drivers, causing
build errors like the following with CONFIG_USB_HID=n:

<--  snip  -->

...
  LD      .tmp_vmlinux1
drivers/built-in.o: In function `bright_probe':
drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
drivers/built-in.o: In function `dell_probe':
drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
drivers/built-in.o: In function `lg_probe':
drivers/hid/hid-lg.c:252: undefined reference to `usbhid_set_leds'
drivers/hid/hid-lg.c:252: more undefined references to `usbhid_set_leds' follow
drivers/built-in.o: In function `sony_set_operational':
drivers/hid/hid-sony.c:42: undefined reference to `usb_control_msg'
drivers/hid/hid-sony.c:42: undefined reference to `usb_control_msg'
make[1]: *** [.tmp_vmlinux1] Error 1

<--  snip  -->


Implement it in a better way.


Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

 drivers/hid/Kconfig |   91 +++++++++++++++++---------------------------
 1 file changed, 36 insertions(+), 55 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index da64108..eec9b73 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -17,25 +17,6 @@ config HID
 	tristate "Generic HID support"
 	depends on INPUT
 	default y
-        select HID_A4TECH if !EMBEDDED
-        select HID_APPLE if !EMBEDDED
-        select HID_BELKIN if !EMBEDDED
-        select HID_BRIGHT if !EMBEDDED
-        select HID_CHERRY if !EMBEDDED
-        select HID_CHICONY if !EMBEDDED
-        select HID_CYPRESS if !EMBEDDED
-        select HID_DELL if !EMBEDDED
-        select HID_EZKEY if !EMBEDDED
-        select HID_GYRATION if !EMBEDDED
-        select HID_LOGITECH if !EMBEDDED
-        select HID_MICROSOFT if !EMBEDDED
-        select HID_MONTEREY if !EMBEDDED
-        select HID_PANTHERLORD if !EMBEDDED
-        select HID_PETALYNX if !EMBEDDED
-        select HID_SAMSUNG if !EMBEDDED
-        select HID_SONY if !EMBEDDED
-        select HID_SUNPLUS if !EMBEDDED
-
 	---help---
 	  A human interface device (HID) is a type of computer device that
 	  interacts directly with and takes input from humans. The term "HID"
@@ -102,15 +83,15 @@ config HID_COMPAT
 	If unsure, say Y.
 
 config HID_A4TECH
-	tristate "A4 tech"
-	default m
+	tristate "A4 tech" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for A4 tech X5 and WOP-35 / Trust 450L mice.
 
 config HID_APPLE
-	tristate "Apple"
-	default m
+	tristate "Apple" if EMBEDDED
+	default (USB_HID || BT_HIDP)
 	depends on (USB_HID || BT_HIDP)
 	---help---
 	Support for some Apple devices which less or more break
@@ -123,64 +104,64 @@ config HID_APPLE
 	If unsure, say M.
 
 config HID_BELKIN
-	tristate "Belkin"
-	default m
+	tristate "Belkin" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Belkin Flip KVM and Wireless keyboard.
 
 config HID_BRIGHT
-	tristate "Bright"
-	default m
+	tristate "Bright" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Bright ABNT-2 keyboard.
 
 config HID_CHERRY
-	tristate "Cherry"
-	default m
+	tristate "Cherry" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Cherry Cymotion.
 
 config HID_CHICONY
-	tristate "Chicony"
-	default m
+	tristate "Chicony" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Chicony Tactical pad.
 
 config HID_CYPRESS
-	tristate "Cypress"
-	default m
+	tristate "Cypress" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Cypress mouse and barcodes.
 
 config HID_DELL
-	tristate "Dell"
-	default m
+	tristate "Dell" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Dell W7658.
 
 config HID_EZKEY
-	tristate "Ezkey"
-	default m
+	tristate "Ezkey" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Ezkey mouse and barcodes.
 
 config HID_GYRATION
-	tristate "Gyration"
-	default m
+	tristate "Gyration" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Gyration remote.
 
 config HID_LOGITECH
-	tristate "Logitech"
-	default m
+	tristate "Logitech" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for some Logitech devices which breaks less or more
@@ -211,23 +192,23 @@ config LOGIRUMBLEPAD2_FF
 	  Rumblepad 2 devices.
 
 config HID_MICROSOFT
-	tristate "Microsoft"
-	default m
+	tristate "Microsoft" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for some Microsoft devices which breaks less or more
 	HID specification.
 
 config HID_MONTEREY
-	tristate "Monterey"
-	default m
+	tristate "Monterey" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Monterey Genius KB29E.
 
 config HID_PANTHERLORD
-	tristate "Pantherlord devices support"
-	default m
+	tristate "Pantherlord devices support" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for PantherLord/GreenAsia based device support.
@@ -242,29 +223,29 @@ config PANTHERLORD_FF
 	  or adapter and want to enable force feedback support for it.
 
 config HID_PETALYNX
-	tristate "Petalynx"
-	default m
+	tristate "Petalynx" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Petalynx Maxter remote.
 
 config HID_SAMSUNG
-	tristate "Samsung"
-	default m
+	tristate "Samsung" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Samsung IR remote.
 
 config HID_SONY
-	tristate "Sony"
-	default m
+	tristate "Sony" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Sony PS3 controller.
 
 config HID_SUNPLUS
-	tristate "Sunplus"
-	default m
+	tristate "Sunplus" if EMBEDDED
+	default USB_HID
 	depends on USB_HID
 	---help---
 	Support for Sunplus WDesktop input device.

Re: [2.6 patch] HID: fix default building of all quirky devices

[Jiri Kosina]

On Wed, 15 Oct 2008, Adrian Bunk wrote:

> Commit 9be7bbd54df3c9c393ccd19acc49f90c517d1291
> (HID: build drivers for all quirky devices by default)
> was wrong in that it select'ed the drivers, causing
> build errors like the following with CONFIG_USB_HID=n:
> <--  snip  -->
> 
> ...
>   LD      .tmp_vmlinux1
> drivers/built-in.o: In function `bright_probe':
> drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> drivers/built-in.o: In function `dell_probe':
> drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> drivers/built-in.o: In function `lg_probe':
> drivers/hid/hid-lg.c:252: undefined reference to `usbhid_set_leds'
> drivers/hid/hid-lg.c:252: more undefined references to `usbhid_set_leds' follow
> drivers/built-in.o: In function `sony_set_operational':
> drivers/hid/hid-sony.c:42: undefined reference to `usb_control_msg'
> drivers/hid/hid-sony.c:42: undefined reference to `usb_control_msg'
> make[1]: *** [.tmp_vmlinux1] Error 1

Hmm ... I am not really sure, isn't this a bug in Kconfig?

What we have right now:

- CONFIG_HID selects all the individual quirk-drivers automatically
- the individual quirk drivers have dependency on CONFIG_USB_HID

Therefore I'd expect when CONFIG_HID is selected and CONFIG_USB_HID 
deselected the individual drivers not to get built (due to their 
dependency on unset option).

But currently what happens is when I de-select CONFIG_USB_HID in 
menuconfig, the configuration options for individual drivers vanish from 
the menuconfig menu (which is expected, they depend on unset option), but 
after saving .config they are still there. Is this how it is expected to 
behave? (i.e. select having preference over dependency).

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [2.6 patch] HID: fix default building of all quirky devices

[Adrian Bunk]

On Wed, Oct 15, 2008 at 09:43:18AM +0200, Jiri Kosina wrote:
> On Wed, 15 Oct 2008, Adrian Bunk wrote:
> 
> > Commit 9be7bbd54df3c9c393ccd19acc49f90c517d1291
> > (HID: build drivers for all quirky devices by default)
> > was wrong in that it select'ed the drivers, causing
> > build errors like the following with CONFIG_USB_HID=n:
> > <--  snip  -->
> > 
> > ...
> >   LD      .tmp_vmlinux1
> > drivers/built-in.o: In function `bright_probe':
> > drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> > drivers/hid/hid-bright.c:38: undefined reference to `usbhid_set_leds'
> > drivers/built-in.o: In function `dell_probe':
> > drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> > drivers/hid/hid-dell.c:41: undefined reference to `usbhid_set_leds'
> > drivers/built-in.o: In function `lg_probe':
> > drivers/hid/hid-lg.c:252: undefined reference to `usbhid_set_leds'
> > drivers/hid/hid-lg.c:252: more undefined references to `usbhid_set_leds' follow
> > drivers/built-in.o: In function `sony_set_operational':
> > drivers/hid/hid-sony.c:42: undefined reference to `usb_control_msg'
> > drivers/hid/hid-sony.c:42: undefined reference to `usb_control_msg'
> > make[1]: *** [.tmp_vmlinux1] Error 1
> 
> Hmm ... I am not really sure, isn't this a bug in Kconfig?

No.

> What we have right now:
> 
> - CONFIG_HID selects all the individual quirk-drivers automatically
> - the individual quirk drivers have dependency on CONFIG_USB_HID
> 
> Therefore I'd expect when CONFIG_HID is selected and CONFIG_USB_HID 
> deselected the individual drivers not to get built (due to their 
> dependency on unset option).

Different people have different expectations.

Many kernel developers seem to wrongly assume kconfig was something 
trivial and everything that does not work as expected was a bug in 
kconfig. But it is not, and there is no magic bullet like changing
how select behaves in some way that will suddenly solve all problems.

> But currently what happens is when I de-select CONFIG_USB_HID in 
> menuconfig, the configuration options for individual drivers vanish from 
> the menuconfig menu (which is expected, they depend on unset option), but 
> after saving .config they are still there. Is this how it is expected to 
> behave? (i.e. select having preference over dependency).

Select ignores dependencies on the selected options.

That's the documented behavior.

> Thanks,

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: [2.6 patch] HID: fix default building of all quirky devices

[Linus Torvalds]

On Wed, 15 Oct 2008, Jiri Kosina wrote:
> 
> Hmm ... I am not really sure, isn't this a bug in Kconfig?

Not really, more of a misfeature.

That said, even if we were to consider it a bug, Adrian's patch is 
obviously the better way to do it. You shouldn't do non-local dependencies 
(have one config option select fifty other ones), when the local ones are 
clearer and more readable (have one config option just describe its *own* 
dependencies).

Don't get me wrong - select is very useful, but not for "should I ask this 
question or not". The point to use select is when you have some library or 
other common infrastructure that isn't worth a question of its own (eg 
"Why the hell would the user want to care whether he needs CRC32 
routines?"), and some code says "I will need this infrastructure" by just 
saying "select CRC32" to let the build system know that it needs that 
particular piece of functionality.

In other words, "select" is kind of a "depends on", but for things that it 
is insane to ask. It would be totally _idiotic_ to ask a user "do you want 
to have CRC32 routines in the kernel?" and then based on that say "ok, you 
didn't ask for CRC32, so now you cannot use the AX88796 network driver".

See? THAT is what "select" is for. And when you use select, the way we do 
things now, you have to select everything you need. You cannot assume that 
it will recursively select whatever it needs.

And notice how you mis-use select. That's not how to disable a question. A 
question gets disabled by just doing an "if xyz" on the question itself, 
like Adrian did.

			Linus

Re: [2.6 patch] HID: fix default building of all quirky devices

[Linus Torvalds]

On Wed, 15 Oct 2008, Linus Torvalds wrote:
> 
> And notice how you mis-use select. That's not how to disable a question. A 
> question gets disabled by just doing an "if xyz" on the question itself, 
> like Adrian did.

Side note: Adrian too has a very odd and non-obvious way of doing this.

His patch did config entries like

	config HID_BRIGHT
		tristate "Bright" if EMBEDDED
		default USB_HID
		depends on USB_HID

which is a really odd way to express this. The much more natural one is

	config HID_BRIGHT
		tristate "Bright" if EMBEDDED
		depends on USB_HID
		default y

since there's no point in saying "default USB_HID" + "depends on USB_HID", 
and that's just confusing. Since it depends on USB_HID, the "default y" 
will automatically degrade to whatever USB_HID was set to.

			Linus

Re: [2.6 patch] HID: fix default building of all quirky devices

[Adrian Bunk]

On Wed, Oct 15, 2008 at 10:17:27AM -0700, Linus Torvalds wrote:
> 
> 
> On Wed, 15 Oct 2008, Linus Torvalds wrote:
> > 
> > And notice how you mis-use select. That's not how to disable a question. A 
> > question gets disabled by just doing an "if xyz" on the question itself, 
> > like Adrian did.
> 
> Side note: Adrian too has a very odd and non-obvious way of doing this.
> 
> His patch did config entries like
> 
> 	config HID_BRIGHT
> 		tristate "Bright" if EMBEDDED
> 		default USB_HID
> 		depends on USB_HID
> 
> which is a really odd way to express this. The much more natural one is
> 
> 	config HID_BRIGHT
> 		tristate "Bright" if EMBEDDED
> 		depends on USB_HID
> 		default y
> 
> since there's no point in saying "default USB_HID" + "depends on USB_HID", 
> and that's just confusing. Since it depends on USB_HID, the "default y" 
> will automatically degrade to whatever USB_HID was set to.

For me the more obvious way was what I did, but I agree that your 
solution looks better.

> 			Linus

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: [2.6 patch] HID: fix default building of all quirky devices

[Linus Torvalds]

On Wed, 15 Oct 2008, Adrian Bunk wrote:
> 
> For me the more obvious way was what I did, but I agree that your 
> solution looks better.

I guess the "more obvious" part simply depends on what you're used to. The 
"default y" + "depends on XYZ" thing is a rather common model, so I find 
it to not just be shorter, but also "more obvious", exactly because I've 
seen it before.

Of course, the behaviour of 'default y' is much simpler (and _much_ more 
common!) for a bool than for a tristate, and the rules for tristate logic 
in general are obviously often a bit non-intuitive considering that we're 
almost always used to the binary kind, and then the fact that Kconfgi uses 
ternary logic has caused no end of hiccups.

		Linus

Re: [2.6 patch] HID: fix default building of all quirky devices

[Jiri Kosina]

On Wed, 15 Oct 2008, Linus Torvalds wrote:

> > Hmm ... I am not really sure, isn't this a bug in Kconfig?
> Not really, more of a misfeature.
> That said, even if we were to consider it a bug, Adrian's patch is 
> obviously the better way to do it. You shouldn't do non-local dependencies 
> (have one config option select fifty other ones), when the local ones are 
> clearer and more readable (have one config option just describe its *own* 
> dependencies).

I agree. 

And even though I really think this is at least a misfeature, it's indeed 
properly documented in Documentation/kbuild/kconfig-language.txt, so I am 
really the only one to blame here.

On Wed, 15 Oct 2008, Linus Torvalds wrote:

> Side note: Adrian too has a very odd and non-obvious way of doing this.
> 
> His patch did config entries like
> 
> 	config HID_BRIGHT
> 		tristate "Bright" if EMBEDDED
> 		default USB_HID
> 		depends on USB_HID
> 
> which is a really odd way to express this. The much more natural one is
> 
> 	config HID_BRIGHT
> 		tristate "Bright" if EMBEDDED
> 		depends on USB_HID
> 		default y
> 
> since there's no point in saying "default USB_HID" + "depends on USB_HID", 
> and that's just confusing. Since it depends on USB_HID, the "default y" 
> will automatically degrade to whatever USB_HID was set to.

Hmm, yes, looks slightly better.

I see you haven't pulled Adrian's fix from my tree yet. So please drop my 
latest pull request, I will do it the "depens on + default y" way and send 
you a new one shortly. There have been some more VID/PID additions 
accumulated in the meantime anyway, so I'll send them too within a new 
pull request.

-- 
Jiri Kosina

select validation (was: Re: [2.6 patch] HID: fix default building of all quirky devices)

[Geert Uytterhoeven]

On Wed, 15 Oct 2008, Linus Torvalds wrote:
> On Wed, 15 Oct 2008, Jiri Kosina wrote:
> > Hmm ... I am not really sure, isn't this a bug in Kconfig?
> 
> Not really, more of a misfeature.
> 
> That said, even if we were to consider it a bug, Adrian's patch is 
> obviously the better way to do it. You shouldn't do non-local dependencies 
> (have one config option select fifty other ones), when the local ones are 
> clearer and more readable (have one config option just describe its *own* 
> dependencies).
> 
> Don't get me wrong - select is very useful, but not for "should I ask this 
> question or not". The point to use select is when you have some library or 
> other common infrastructure that isn't worth a question of its own (eg 
> "Why the hell would the user want to care whether he needs CRC32 
> routines?"), and some code says "I will need this infrastructure" by just 
> saying "select CRC32" to let the build system know that it needs that 
> particular piece of functionality.
> 
> In other words, "select" is kind of a "depends on", but for things that it 
> is insane to ask. It would be totally _idiotic_ to ask a user "do you want 
> to have CRC32 routines in the kernel?" and then based on that say "ok, you 
> didn't ask for CRC32, so now you cannot use the AX88796 network driver".
> 
> See? THAT is what "select" is for. And when you use select, the way we do 
> things now, you have to select everything you need. You cannot assume that 
> it will recursively select whatever it needs.
> 
> And notice how you mis-use select. That's not how to disable a question. A 
> question gets disabled by just doing an "if xyz" on the question itself, 
> like Adrian did.

Would it be possible for kconfig to check for invalid usage of select?

Like you should not use select to enable something that has
dependencies. However, that would fail in case both the selector option
and the selected option depend on the same.

And doing it dynamically (you should not use select to enable something that
has dependencies that are (currently) not fulfilled) wouldn't find all
incorrect usages.

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: select validation (was: Re: [2.6 patch] HID: fix default building of all quirky devices)

[Linus Torvalds]

On Thu, 16 Oct 2008, Geert Uytterhoeven wrote:
> 
> Would it be possible for kconfig to check for invalid usage of select?
> 
> Like you should not use select to enable something that has
> dependencies. However, that would fail in case both the selector option
> and the selected option depend on the same.

Well, at least in theory you may actually want to select something that 
has dependencies, even if you don't want to select the dependencies.

For example, some feature may be enabled by default on some architecture 
or with some config. Example:

	bool SUPPORT_FEATURE
		default y
		depends on EXPERIMENTAL
		depends on !EMBEDDED
		depends on X86

which is just another way of saying

	bool SUPPORT_FEATURE
		default X86 && !EMBEDDED && EXPERIMENTAL

and it's still possible that some code wants to do a

	select SUPPORT_FEATURE

because the dependency isn't a _code_ dependency, it's a default-value 
dependency.

Now, you could say that then you should use that second version (ie the 
"default X86 && !EMBEDDED && EXPERIMENTAL" version), but the thing is, 
"depends on" is actually a more powerful and can result in more readable 
setup (because you can have multiple "depends on" lines and they are all 
logically anded together.

Do we do that? I dunno. But I wouldn't be surprised if we do.

		Linus

Re: select validation (was: Re: [2.6 patch] HID: fix default building of all quirky devices)

[Adrian Bunk]

On Thu, Oct 16, 2008 at 11:47:55AM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 16 Oct 2008, Geert Uytterhoeven wrote:
> > 
> > Would it be possible for kconfig to check for invalid usage of select?
> > 
> > Like you should not use select to enable something that has
> > dependencies. However, that would fail in case both the selector option
> > and the selected option depend on the same.
> 
> Well, at least in theory you may actually want to select something that 
> has dependencies, even if you don't want to select the dependencies.
> 
> For example, some feature may be enabled by default on some architecture 
> or with some config. Example:
> 
> 	bool SUPPORT_FEATURE
> 		default y
> 		depends on EXPERIMENTAL
> 		depends on !EMBEDDED
> 		depends on X86
> 
> which is just another way of saying
> 
> 	bool SUPPORT_FEATURE
> 		default X86 && !EMBEDDED && EXPERIMENTAL

Such constructs without a prompt are for a different usecase than what 
you have in mind, there wouldn't be EXPERIMENTAL or EMBEDDED in the 
dependencies, this is used for stuff like

config X86_HT
        bool
        depends on SMP
        depends on (X86_32 && !X86_VOYAGER) || X86_64
        default y

And that's usually not the kind of symbol that should get selected.


In reality your example would be:

config SUPPORT_FEATURE
        bool "support feature" if EMBEDDED
        default y
        depends on X86 && EXPERIMENTAL


> and it's still possible that some code wants to do a
> 
> 	select SUPPORT_FEATURE
> 
> because the dependency isn't a _code_ dependency, it's a default-value 
> dependency.
>...

I'd be more concerned about dependencies that are used to make the 
kconfig UI better.

Dependencies on options like NETDEV_1000 are not code dependencies.

> 		Linus

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