wimax: Kbuild / rfkill-build fixes
Subsystems (2)
cover wimax: Kbuild / rfkill-build fixes
These three patches fix issues reported by Randy Dunlap on the linux-next tree for Jan 6:
http://lkml.org/lkml/2009/1/6/340
The issue fixed by patch #3 wasn't reported on the emails but was uncovered while discussing the fixes.
Inaky Perez-Gonzalez (3):
wimax: fix '#ifdef CONFIG_BUG' layout to avoid warning
wimax: fix kconfig interactions with rfkill and input layers
wimax: testing for rfkill support should also test for
CONFIG_RFKILL_MODULE
net/wimax/Kconfig | 14 ++++++++++++++ net/wimax/id-table.c | 10 ++++++---- net/wimax/op-rfkill.c | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-)
PATCH 1/3 wimax: fix '#ifdef CONFIG_BUG' layout to avoid warning
Reported by Randy Dunlap:
Quoted 4 lines
Also, this warning needs to be fixed:
linux-next-20090106/net/wimax/id-table.c:133: warning: ISO C90 forbids mixed declarations and code
Move the return on #defined(CONFIG_BUG) below the variable declarations so it doesn't violate ISO C90.
On wimax_id_table_release() we want to do a debug check if CONFIG_BUG is enabled. However, we also want the debug code to be always compiled to ensure there is no bitrot. It will be optimized out by the compiler when CONFIG_BUG is disabled.
Added a note to the function header stating this.
--- net/wimax/id-table.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)
net/wimax/id-table.c +6 -4
@@ -123,15 +123,17 @@ void wimax_id_table_rm(struct wimax_dev *wimax_dev)/* * Release the gennetlink family id / mapping table * -* On debug, verify that the table is empty upon removal. +* On debug, verify that the table is empty upon removal. We want the +* code always compiled, to ensure it doesn't bit rot. It will be +* compiled out if CONFIG_BUG is disabled. */ void wimax_id_table_release (void ) { -#ifndef CONFIG_BUG -return ; -#endif struct wimax_dev *wimax_dev; +#ifdef CONFIG_BUG +return ; +#endif spin_lock (&wimax_id_table_lock);list_for_each_entry (wimax_dev, &wimax_id_table, id_table_node) { printk(KERN_ERR "BUG: %s wimax_dev %p ifindex %d not cleared\n",
Signature · 1 line
1.5.6.5
Re: [PATCH 1/3] wimax: fix '#ifdef CONFIG_BUG' layout to avoid warning
Quoted 7 lines · Inaky Perez-Gonzalez
Reported by Randy Dunlap:
Quoted 4 lines
Also, this warning needs to be fixed:
linux-next-20090106/net/wimax/id-table.c:133: warning: ISO C90 forbids mixed declarations and code
Quoted 6 lines
Move the return on #defined(CONFIG_BUG) below the variable declarations so it doesn't violate ISO C90.
On
wimax_id_table_release()we want to do a debug check ifCONFIG_BUGis enabled. However, we also want the debug code to be always compiled to ensure there is no bitrot.
I hope this kind of solution won't add some warnings? Besides, this seems rather strange reasoning as CONFIG_BUG is mostly enabled anyway?
Quoted 21 lines
It will be optimized out by the compiler
whenCONFIG_BUGis disabled.Added a note to the function header stating this.
Signed-off-by Inaky Perez-Gonzalez <[email protected]>--- net/wimax/id-table.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)net/wimax/id-table.c +2
-1@@ -123,15 +123,17 @@ void wimax_id_table_rm(struct wimax_dev *wimax_dev)/* * Release the gennetlink family id / mapping table * -* On debug, verify that the table is empty upon removal. +* On debug, verify that the table is empty upon removal. We want the +* code always compiled, to ensure it doesn't bit rot. It will be
net/wimax/id-table.c void wimax_id_table_rm(struct wimax_dev *wimax_dev) · line 134
Quoted 10 lines
net/wimax/id-table.c +2
-3+* compiled out if CONFIG_BUG is disabled. */ void wimax_id_table_release (void ) { -#ifndef CONFIG_BUG -return ; -#endif struct wimax_dev *wimax_dev; +#ifdef CONFIG_BUG
Did you perhaps mean ifndef here??? :-)
Quoted 6 lines
net/wimax/id-table.c +2
-0+return ; +#endif spin_lock (&wimax_id_table_lock);list_for_each_entry (wimax_dev, &wimax_id_table, id_table_node) { printk(KERN_ERR "BUG: %s wimax_dev %p ifindex %d not cleared\n",
Signature · 1 line
i.
Re: [PATCH 1/3] wimax: fix '#ifdef CONFIG_BUG' layout to avoid warning
Quoted 10 lines · Ilpo Järvinen
Quoted 9 lines · Inaky Perez-Gonzalez
Reported by Randy Dunlap:
Quoted 4 lines
Also, this warning needs to be fixed:
linux-next-20090106/net/wimax/id-table.c:133: warning: ISO C90 forbids mixed declarations and codeMove the return on #defined(CONFIG_BUG) below the variable declarations so it doesn't violate ISO C90.
Quoted 6 lines
Quoted 3 lines
On
wimax_id_table_release()we want to do a debug check ifCONFIG_BUGis enabled. However, we also want the debug code to be always compiled to ensure there is no bitrot.I hope this kind of solution won't add some warnings? Besides, this seems rather strange reasoning as
CONFIG_BUGis mostly enabled anyway?
Well, it is legal code -- short of 'if (1) return'. It doesn't warn (and it should not).
Quoted 8 lines
net/wimax/id-table.c void wimax_id_table_rm(struct wimax_dev *wimax_dev) · line 134
Quoted 6 lines
net/wimax/id-table.c +1
-3-#ifndef CONFIG_BUG -return ; -#endif struct wimax_dev *wimax_dev; +#ifdef CONFIG_BUG Did you perhaps mean ifndef here??? :-)
Sigh ... you are right ... good thing I triple checked.
Sending updated patch series.
Thanks,
Signature · 1 line
Inaky
Re: [PATCH 1/3] wimax: fix '#ifdef CONFIG_BUG' layout to avoid warning
Quoted 15 lines · Inaky Perez-Gonzalez
Quoted 14 lines · Ilpo Järvinen
Quoted 12 lines · Inaky Perez-Gonzalez
Reported by Randy Dunlap:
Quoted 4 lines
Also, this warning needs to be fixed:
linux-next-20090106/net/wimax/id-table.c:133: warning: ISO C90 forbids mixed declarations and codeMove the return on #defined(CONFIG_BUG) below the variable declarations so it doesn't violate ISO C90.
On
wimax_id_table_release()we want to do a debug check ifCONFIG_BUGis enabled. However, we also want the debug code to be always compiled to ensure there is no bitrot.
Quoted 5 lines
Quoted 2 lines
I hope this kind of solution won't add some warnings? Besides, this seems rather strange reasoning as
CONFIG_BUGis mostly enabled anyway?Well, it is legal code -- short of 'if (1) return'. It doesn't warn (and it should not).
Obviously, but I was concerned on the other lines than that particular one, e.g., gcc might think that wimax_dev is unused variable and emit a warning or along those lines...?
Signature · 1 line
i.
Re: [PATCH 1/3] wimax: fix '#ifdef CONFIG_BUG' layout to avoid warning
Quoted 19 lines · Ilpo Järvinen
Quoted 18 lines · Inaky Perez-Gonzalez
Quoted 16 lines · Ilpo Järvinen
Quoted 6 lines · Inaky Perez-Gonzalez
Reported by Randy Dunlap:
Quoted 4 lines
Also, this warning needs to be fixed:
linux-next-20090106/net/wimax/id-table.c:133: warning: ISO C90 forbids mixed declarations and codeQuoted 6 lines
Move the return on #defined(CONFIG_BUG) below the variable declarations so it doesn't violate ISO C90.
On
wimax_id_table_release()we want to do a debug check ifCONFIG_BUGis enabled. However, we also want the debug code to be always compiled to ensure there is no bitrot.I hope this kind of solution won't add some warnings? Besides, this seems rather strange reasoning as
CONFIG_BUGis mostly enabled anyway?
Quoted 6 lines
Quoted 2 lines
Well, it is legal code -- short of 'if (1) return'. It doesn't warn (and it should not).
Obviously, but I was concerned on the other lines than that particular one, e.g., gcc might think that
wimax_devis unused variable and emit a warning or along those lines...?
Ah, I see -- no, it won't. [disclaimer: not know much about compiler optimization] In theory, as we were saying, it works just as in a case where you have
int somevar;
if (1)
return;
somevar = call_some_func();
with 1 being the result of a compile time evaluation. The compiler sees that somevar is being used, but the code path is never executed, so everything gets dumped.
If it ever did, it'd be a matter of changing that return to an if (1) return. It'd look uglier though.
Signature · 1 line
Inaky
PATCH 2/3 wimax: fix kconfig interactions with rfkill and input layers
In reply to Inaky Perez-GonzalezWiMAX can work without RFKILL, but it was missing a check to make sure RFKILL is not being made a module with wimax compiled into the kernel. This caused failed builds in s390, where CONFIG_INPUT is always off.
When RFKILL is enabled, the code uses the input layer to report hardware switch changes; thus, if RFKILL is enabled, INPUT has to be too. It also needs to display some message when INPUT is disabled that explains why WiMAX is not selectable.
(issues found by Randy Dunlap in the linux-next tree).
--- net/wimax/Kconfig | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
net/wimax/Kconfig +14 -0
@@ -1,9 +1,23 @@ # # WiMAX LAN device configuration # +# Note the ugly 'depends on' on WIMAX: that disallows RFKILL to be a +# module if WIMAX is to be linked in. The WiMAX code is done in such a +# way that it doesn't require and explicit dependency on RFKILL in +# case an embedded system wants to rip it out. +# +# As well, enablement of the RFKILL code means we need the INPUT layer +# support to inject events coming from hw rfkill switches. That +# dependency could be killed if input.h provided appropiate means to +# work when input is disabled. + +comment "WiMAX Wireless Broadband support requires CONFIG_INPUT enabled" + depends on INPUT = n && RFKILL != n menuconfig WIMAX tristate "WiMAX Wireless Broadband support" + depends on (y && RFKILL != m) || m + depends on (INPUT && RFKILL != n) || RFKILL = n help Select to configure support for devices that provide
Signature · 1 line
1.5.6.5
PATCH 3/3 wimax: testing for rfkill support should also test for CONFIG_RFKILL_MODULE
In reply to Inaky Perez-GonzalezCurrent WiMAX rfkill code is missing the case where rfkill is compiled in as modules and works only when rfkill is compiled in. This is not correct. Fixed to test for CONFIG_RFKILL or CONFIG_RKILL_MODULE.
--- net/wimax/op-rfkill.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
net/wimax/op-rfkill.c +1 -1
@@ -71,7 +71,7 @@#define D_SUBMODULE op_rfkill#include "debug-levels.h" -#ifdef CONFIG_RFKILL +#if defined(CONFIG_RFKILL ) || defined(CONFIG_RFKILL_MODULE )/**
Signature · 1 line
1.5.6.5