From: Adrian Bunk <bunk@kernel.org> Date: 2008-10-15 04:13:57
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
From: Adrian Bunk <bunk@kernel.org> Date: 2008-10-15 05:06:15
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
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
From: Adrian Bunk <bunk@kernel.org> Date: 2008-10-15 07:30:27
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(-)
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
From: Adrian Bunk <bunk@kernel.org> Date: 2008-10-15 10:21:31
On Wed, Oct 15, 2008 at 09:43:18AM +0200, Jiri Kosina wrote:
On Wed, 15 Oct 2008, Adrian Bunk wrote:
quoted
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
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
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
From: Adrian Bunk <bunk@kernel.org> Date: 2008-10-15 17:32:58
On Wed, Oct 15, 2008 at 10:17:27AM -0700, Linus Torvalds wrote:
On Wed, 15 Oct 2008, Linus Torvalds wrote:
quoted
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
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
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
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
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
From: Adrian Bunk <bunk@kernel.org> Date: 2008-10-16 19:31:24
On Thu, Oct 16, 2008 at 11:47:55AM -0700, Linus Torvalds wrote:
On Thu, 16 Oct 2008, Geert Uytterhoeven wrote:
quoted
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