Re: [2.6 patch] iwlwifi: move the selects to the tristate drivers

Previous thread: [PATCH] sched: fair-group: fix a Div0 error of the fair group scheduler by Miao Xie on Sunday, April 27, 2008 - 9:54 pm. (6 messages)

Next thread: [PATCH -next] docbook: x86 bitops file renamed by Randy Dunlap on Sunday, April 27, 2008 - 10:46 pm. (1 message)
From: Carlos R. Mafra
Date: Sunday, April 27, 2008 - 10:47 pm

I've just tried to compile v2.6.25-5561-g064922a and it failed with
these messages:

drivers/built-in.o: In function `iwl_leds_unregister_led':
iwl-led.c:(.text+0x7f14a): undefined reference to `led_classdev_unregister'
drivers/built-in.o: In function `iwl_leds_register_led':
iwl-led.c:(.text+0x7f220): undefined reference to `led_classdev_register'
make: *** [.tmp_vmlinux1] Error 1

and I checked that it was due to CONFIG_IWL4965_LEDS=y and 
CONFIG_LEDS_CLASS=m in my .config.

If I CONFIG_LEDS_CLASS is also =y then it builds fine.

So I tried to cook up a patch to avoid that. It doesn't look
elegant but it avoids this problem for me :-)


--
From 7116035343f4d031807aa73c92434eab3863de56 Mon Sep 17 00:00:00 2001
From: Carlos R. Mafra <crmafra@ift.unesp.br>
Date: Mon, 28 Apr 2008 02:12:22 -0300
Subject: [PATCH] iwlwifi: Fix build failure due to CONFIG_LEDS_CLASS=m

The compilation of v2.6.25-5561-g064922a fails with

drivers/built-in.o: In function `iwl_leds_unregister_led':
iwl-led.c:(.text+0x7f14a): undefined reference to `led_classdev_unregister'
drivers/built-in.o: In function `iwl_leds_register_led':
iwl-led.c:(.text+0x7f220): undefined reference to `led_classdev_register'
make: *** [.tmp_vmlinux1] Error 1

and this is the relevant part of .config which caused it

CONFIG_MAC80211_LEDS=y
CONFIG_IWLWIFI_LEDS=y
CONFIG_IWL4965_LEDS=y
CONFIG_LEDS_CLASS=m

The problem was that CONFIG_IWL_4965_LEDS was compiled in and
CONFIG_LEDS_CLASS was modular. To avoid this build failure
make IWL_4965_LEDS depend on LEDS_CLASS=y.

Signed-off-by: Carlos R. Mafra <crmafra@ift.unesp.br>
---
 drivers/net/wireless/iwlwifi/Kconfig |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
index c4e631d..6081f92 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -49,9 +49,8 @@ config IWL4965_HT
 
 config IWL4965_LEDS
 	bool "Enable LEDS ...
From: David Miller
Date: Sunday, April 27, 2008 - 11:35 pm

From: "Carlos R. Mafra" <crmafra2@gmail.com>

Already fixed in net-2.6 by:

commit bd8fd21dfddf51299d782f598cb776b15b7d64cc
Author: Luca Tettamanti <kronos.it@gmail.com>
Date:   Sun Apr 27 15:34:55 2008 -0700

    wireless: Fix compile error with wifi & leds
    
    Fix build error caused by commit
    e82404ad612ebabc65d15c3d59b971cb35c3ff36 ("iwlwifi: Select
    LEDS_CLASS.") from David Miller:
    
    Since MAC80211_LEDS is selected by wireless drivers it must select its
    own dependencies otherwise a build error may occur (kbuild will select
    the symbol regardless of "depends" constraints).
    
    Signed-off-By: Luca Tettamanti <kronos.it@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index 520a518..a24b459 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -73,7 +73,9 @@ config MAC80211_MESH
 
 config MAC80211_LEDS
 	bool "Enable LED triggers"
-	depends on MAC80211 && LEDS_TRIGGERS
+	depends on MAC80211
+	select NEW_LEDS
+	select LEDS_TRIGGERS
 	---help---
 	  This option enables a few LED triggers for different
 	  packet receive/transmit events.
--

From: Carlos R. Mafra
Date: Monday, April 28, 2008 - 12:32 am

Hmm, I just tested this patch and it does not solve the problem. 

I could create a .config using 'make menuconfig' in which I had

CONFIG_MAC80211_LEDS=y
CONFIG_RFKILL_LEDS=y
CONFIG_IWLWIFI_LEDS=y
CONFIG_IWL4965_LEDS=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=m

So I recompiled the kernel and got the same error message as before.

What my patch does is to make a .config with CONFIG_LEDS_CLASS=m and 
CONFIG_IWL4965_LEDS=y be impossible to exist, because it
appears that this combination is what causes the build error.

But anyways, I've just read about the Kconfig language a few hours
ago when I had this problem and tried to solve it, so my patch is
probably missing something. But it works for me in the sense that
IWL4965_LEDS doesn't even show as an option in 'make menuconfig' 
if LEDS_CLASS=m, so I user like me can not generate a .config which 
does not build correctly.



--

From: Adrian Bunk
Date: Monday, April 28, 2008 - 6:15 am

Please send your .config

Thanks
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: Carlos R. Mafra
Date: Monday, April 28, 2008 - 7:21 am

Find it below. It was generated through make menuconfig with Luca's patch
applied, and the build fails like before.

Is there something wrong with the patch I wrote yesterday? 
It fixes this issue for me...but I would like to take this 
opportunity to learn something new (for me) and useful for the future :-)

Thanks for taking a look at it!

#
# Automatically generated make config: don't edit
# Linux kernel version: 2.6.25
# Mon Apr 28 10:53:15 2008
#
CONFIG_64BIT=y
# CONFIG_X86_32 is not set
CONFIG_X86_64=y
CONFIG_X86=y
# CONFIG_GENERIC_LOCKBREAK is not set
CONFIG_GENERIC_TIME=y
CONFIG_GENERIC_CMOS_UPDATE=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_FAST_CMPXCHG_LOCAL=y
CONFIG_MMU=y
CONFIG_ZONE_DMA=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_IOMAP=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_HWEIGHT=y
# CONFIG_GENERIC_GPIO is not set
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_DMI=y
CONFIG_RWSEM_GENERIC_SPINLOCK=y
# CONFIG_RWSEM_XCHGADD_ALGORITHM is not set
# CONFIG_ARCH_HAS_ILOG2_U32 is not set
# CONFIG_ARCH_HAS_ILOG2_U64 is not set
CONFIG_ARCH_HAS_CPU_IDLE_WAIT=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_HAVE_CPUMASK_OF_CPU_MAP=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ZONE_DMA32=y
CONFIG_ARCH_POPULATES_NODE_MAP=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_AOUT=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_X86_SMP=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_X86_TRAMPOLINE=y
# CONFIG_KTIME_SCALAR is not set
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"

#
# General ...
From: Adrian Bunk
Date: Monday, April 28, 2008 - 1:58 pm

The actual problem was CONFIG_IWLCORE=y, CONFIG_IWL4965=m.



It would require users to manually set CONFIG_LEDS_CLASS only for 
getting their net driver, which is an implementation detail we shouldn't 
bother them with.

And it would require a built-in CONFIG_LEDS_CLASS even for modular 

I'm not sure whether the best opportunity for you to learn something 
useful for the future is to learn about the nastier parts of kconfig...  ;-)

Fellow kernel developers have claimed I was crazy after I told that my 

Thanks for reporting it!

cu
Adrian


<--  snip  -->


This patch moves the following select's:
- RFKILL        : IWLWIFI_RFKILL -> IWLCORE
- RFKILL_INPUT  : IWLWIFI_RFKILL -> IWLCORE
- MAC80211_LEDS : IWL4965_LEDS   -> IWLCORE
- LEDS_CLASS    : IWL4965_LEDS   -> IWLCORE
- MAC80211_LEDS : IWL3945_LEDS   -> IWL3945
- LEDS_CLASS    : IWL3945_LEDS   -> IWL3945

The effects are:
- with IWLCORE=m and/or IWL3945=m RFKILL/RFKILL_INPUT/MAC80211_LEDS/LEDS_CLASS
  are no longer wrongly forced to y
- fixes a build error with IWLCORE=y, IWL4965=m
  might be a bug in kconfig causing it, but doing this change that is 
  anyway the right thing fixes it

Reported-by: Carlos R. Mafra <crmafra2@gmail.com>
Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

BTW: There's no correlation between IWL3945_LEDS and IWLWIFI_LEDS.
     That seems to be intended?

 drivers/net/wireless/iwlwifi/Kconfig |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

cebeaa10898371bd3bf038e796c832ae71ce5f14 diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
index 9a25f55..2a18377 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -6,6 +6,10 @@ config IWLCORE
 	tristate "Intel Wireless Wifi Core"
 	depends on PCI && MAC80211 && WLAN_80211 && EXPERIMENTAL
 	select IWLWIFI
+	select MAC80211_LEDS if IWLWIFI_LEDS
+	select LEDS_CLASS if IWLWIFI_LEDS
+	select RFKILL if IWLWIFI_RFKILL
+	select ...
From: John W. Linville
Date: Monday, April 28, 2008 - 4:45 pm

This seems sane to me -- I'm sorry I let so much Kconfig mess slip

Yes.  IWLWIFI_LEDS used to be IWL4965_LEDS, and I have the impression
that eventually it will subsume IWL3945_LEDS as well.

Dave, are you going to snarf this one too?  If not (and presuming no
one points out yet another problem) I'll just include it in the next
round of fix patches, probably on tomorrow.

John
-- 
John W. Linville
linville@tuxdriver.com
--

From: Tomas Winkler
Date: Monday, April 28, 2008 - 6:07 pm

I'm sorry too, this is just so no intuitive... I'm really amazed where
it get into yet constrain computation is always complex.  Next time I
put more effort in this.

True, not yet.



Thanks
Tomas
--

From: David Miller
Date: Tuesday, April 29, 2008 - 10:30 pm

From: "John W. Linville" <linville@tuxdriver.com>

I pulled in Adrian's patch.
--

From: David Miller
Date: Tuesday, April 29, 2008 - 10:30 pm

From: Adrian Bunk <bunk@kernel.org>

Applied, thanks a lot Adrian.
--

From: Carlos R. Mafra
Date: Wednesday, April 30, 2008 - 6:31 am

Yeah Adrian, thank you very much!

It is really useful for the kernel to have someone like you cleaning 
up these Kconfig mistakes. My dumb patch was not good and now at 
least I know how it should be done.
Keep up your great work!

--

Previous thread: [PATCH] sched: fair-group: fix a Div0 error of the fair group scheduler by Miao Xie on Sunday, April 27, 2008 - 9:54 pm. (6 messages)

Next thread: [PATCH -next] docbook: x86 bitops file renamed by Randy Dunlap on Sunday, April 27, 2008 - 10:46 pm. (1 message)