2008-06-26 21:55:25

by drago01

[permalink] [raw]
Subject: [PATCH] iwl3945: fix up rfkill kconfig and input

The attached patch fixes iwl3945 to build with CONFIG_INPUT=n and
frees the input device on exit.
Additionally it removes the unnecessary dependency on CONFIG_IWLCORE.
It depends on the rfkill subsystem patch, which is in the iwlwifi-2.6 tree.

-----------------
This patch fixes the iwl3945 driver to build with CONFIG_INPUT=n
and remove the dependency on CONFIG_IWLCORE.
It also frees the input device on exit.
Input stuff based on patch from Randy Dunlap for iwl4965/5000.

Signed-off-by: Adel Gadllah <[email protected]>

diff --git a/drivers/net/wireless/iwlwifi/Kconfig
b/drivers/net/wireless/iwlwifi/Kconfig
index a382c00..3148a28 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -105,6 +105,7 @@ config IWL3945
select IWLWIFI
select MAC80211_LEDS if IWL3945_LEDS
select LEDS_CLASS if IWL3945_LEDS
+ select RFKILL_INPUT if (IWL3945_RFKILL && INPUT)
---help---
Select to build the driver supporting the:

@@ -127,6 +128,10 @@ config IWL3945
say M here and read <file:Documentation/kbuild/modules.txt>. The
module will be called iwl3945.ko.

+config IWL3945_RFKILL
+ boolean "Enable RF kill support in iwl3945 drivers"
+ depends on IWL3945
+
config IWL3945_SPECTRUM_MEASUREMENT
bool "Enable Spectrum Measurement in iwl3945 drivers"
depends on IWL3945
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h
b/drivers/net/wireless/iwlwifi/iwl-3945.h
index a774978..4df42ad 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.h
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
@@ -690,7 +690,7 @@ enum {

#endif

-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
struct iwl3945_priv;

struct iwl3945_rfkill_mngr {
@@ -800,7 +800,7 @@ struct iwl3945_priv {
struct iwl3945_init_alive_resp card_alive_init;
struct iwl3945_alive_resp card_alive;

-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
struct iwl3945_rfkill_mngr rfkill_mngr;
#endif

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c
b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 43cb8ff..5449039 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -8282,7 +8282,7 @@ static int iwl3945_pci_resume(struct pci_dev *pdev)
#endif /* CONFIG_PM */

/*************** RFKILL FUNCTIONS **********/
-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
/* software rf-kill from user */
static int iwl3945_rfkill_soft_rf_kill(void *data, enum rfkill_state state)
{
@@ -8340,6 +8340,7 @@ int iwl3945_rfkill_init(struct iwl3945_priv *priv)
priv->rfkill_mngr.rfkill->dev.class->suspend = NULL;
priv->rfkill_mngr.rfkill->dev.class->resume = NULL;

+#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
priv->rfkill_mngr.input_dev = input_allocate_device();
if (!priv->rfkill_mngr.input_dev) {
IWL_ERROR("Unable to allocate rfkill input device.\n");
@@ -8354,6 +8355,7 @@ int iwl3945_rfkill_init(struct iwl3945_priv *priv)
priv->rfkill_mngr.input_dev->dev.parent = device;
priv->rfkill_mngr.input_dev->evbit[0] = BIT(EV_KEY);
set_bit(KEY_WLAN, priv->rfkill_mngr.input_dev->keybit);
+#endif

ret = rfkill_register(priv->rfkill_mngr.rfkill);
if (ret) {
@@ -8361,11 +8363,13 @@ int iwl3945_rfkill_init(struct iwl3945_priv *priv)
goto free_input_dev;
}

+#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
ret = input_register_device(priv->rfkill_mngr.input_dev);
if (ret) {
IWL_ERROR("Unable to register rfkill input device: %d\n", ret);
goto unregister_rfkill;
}
+#endif

IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
return ret;
@@ -8375,8 +8379,10 @@ unregister_rfkill:
priv->rfkill_mngr.rfkill = NULL;

free_input_dev:
+#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
input_free_device(priv->rfkill_mngr.input_dev);
priv->rfkill_mngr.input_dev = NULL;
+#endif

freed_rfkill:
if (priv->rfkill_mngr.rfkill != NULL)
@@ -8391,13 +8397,16 @@ error:
void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
{

+#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
if (priv->rfkill_mngr.input_dev)
input_unregister_device(priv->rfkill_mngr.input_dev);
+ input_free_device(priv->rfkill_mngr.input_dev);
+ priv->rfkill_mngr.input_dev = NULL;
+#endif

if (priv->rfkill_mngr.rfkill)
rfkill_unregister(priv->rfkill_mngr.rfkill);

- priv->rfkill_mngr.input_dev = NULL;
priv->rfkill_mngr.rfkill = NULL;
}


2008-06-26 22:44:23

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH v2] iwl3945: fix up rfkill kconfig and input

On Friday 27 June 2008, drago01 wrote:
> On Fri, Jun 27, 2008 at 12:15 AM, Ivo van Doorn <[email protected]> wrote:
> >> diff --git a/drivers/net/wireless/iwlwifi/Kconfig
> >> b/drivers/net/wireless/iwlwifi/Kconfig
> >> index a382c00..3148a28 100644
> >> --- a/drivers/net/wireless/iwlwifi/Kconfig
> >> +++ b/drivers/net/wireless/iwlwifi/Kconfig
> >> @@ -105,6 +105,7 @@ config IWL3945
> >> select IWLWIFI
> >> select MAC80211_LEDS if IWL3945_LEDS
> >> select LEDS_CLASS if IWL3945_LEDS
> >> + select RFKILL_INPUT if (IWL3945_RFKILL && INPUT)
> >
> > That is a big no. Drivers should only use CONFIG_RFKILL and in no way
> > depend or select RFKILL_INPUT. RFKILL_INPUT is a decision for the user
> > and doesn't change anything for drivers.
>
> v2 is attached it removed the CONFIG_RFKILL_INPUT stuff and made it
> depend on CONFIG_RFKILL.
> That means that the 4965 patch needs to be updated/fixed too.

Not sure if it should depend or select CONFIG_INPUT,
but the RFKILL dependencies are now correct.

Thanks,

Ivo

> ------
> This patch fixes the iwl3945 driver to depend on CONFIG_RFKILL and
> CONFIG_INPUT when RFKILL is used.
> It removes the dependency on CONFIG_IWLCORE and frees the input
> device on exit.
>
> Signed-off-by: Adel Gadllah <[email protected]>
>
> diff --git a/drivers/net/wireless/iwlwifi/Kconfig
> b/drivers/net/wireless/iwlwifi/Kconfig
> index a382c00..c7fd8aa 100644
> --- a/drivers/net/wireless/iwlwifi/Kconfig
> +++ b/drivers/net/wireless/iwlwifi/Kconfig
> @@ -105,6 +105,8 @@ config IWL3945
> select IWLWIFI
> select MAC80211_LEDS if IWL3945_LEDS
> select LEDS_CLASS if IWL3945_LEDS
> + select RFKILL if IWL3945_RFKILL
> + select INPUT if IWL3945_RFKILL
> ---help---
> Select to build the driver supporting the:
>
> @@ -127,6 +129,10 @@ config IWL3945
> say M here and read <file:Documentation/kbuild/modules.txt>. The
> module will be called iwl3945.ko.
>
> +config IWL3945_RFKILL
> + boolean "Enable RF kill support in iwl3945 drivers"
> + depends on IWL3945
> +
> config IWL3945_SPECTRUM_MEASUREMENT
> bool "Enable Spectrum Measurement in iwl3945 drivers"
> depends on IWL3945
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h
> b/drivers/net/wireless/iwlwifi/iwl-3945.h
> index a774978..4df42ad 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
> @@ -690,7 +690,7 @@ enum {
>
> #endif
>
> -#ifdef CONFIG_IWLWIFI_RFKILL
> +#ifdef CONFIG_IWL3945_RFKILL
> struct iwl3945_priv;
>
> struct iwl3945_rfkill_mngr {
> @@ -800,7 +800,7 @@ struct iwl3945_priv {
> struct iwl3945_init_alive_resp card_alive_init;
> struct iwl3945_alive_resp card_alive;
>
> -#ifdef CONFIG_IWLWIFI_RFKILL
> +#ifdef CONFIG_IWL3945_RFKILL
> struct iwl3945_rfkill_mngr rfkill_mngr;
> #endif
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 43cb8ff..e12baaa 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -8282,7 +8282,7 @@ static int iwl3945_pci_resume(struct pci_dev *pdev)
> #endif /* CONFIG_PM */
>
> /*************** RFKILL FUNCTIONS **********/
> -#ifdef CONFIG_IWLWIFI_RFKILL
> +#ifdef CONFIG_IWL3945_RFKILL
> /* software rf-kill from user */
> static int iwl3945_rfkill_soft_rf_kill(void *data, enum rfkill_state state)
> {
> @@ -8391,8 +8391,11 @@ error:
> void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
> {
>
> - if (priv->rfkill_mngr.input_dev)
> + if (priv->rfkill_mngr.input_dev) {
> input_unregister_device(priv->rfkill_mngr.input_dev);
> + input_free_device(priv->rfkill_mngr.input_dev);
> + }
> +
>
> if (priv->rfkill_mngr.rfkill)
> rfkill_unregister(priv->rfkill_mngr.rfkill);
>



2008-06-27 09:23:03

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH v2] iwl3945: fix up rfkill kconfig and input

On Thu, 2008-06-26 at 17:56 -0500, Randy Dunlap wrote:
> and in general select is OK on smallish library-like functions and
> should not be used on entire subsystems. To me that means that it
> should not be used in CONFIG_INPUT....

Please see this patch.

John, please don't apply this one. I'll submit it inside a patch series
later.

Thanks,
-yi

>From 58c5dc44f8f615113d21b3545df2e193bf22cf75 Mon Sep 17 00:00:00 2001
From: Randy Dunlap <[email protected]>
Date: Fri, 27 Jun 2008 16:21:19 +0800
Subject: [PATCH] iwlwifi: fix build for CONFIG_INPUT=n

Fix iwlwifi so that it builds cleanly with CONFIG_INPUT=n.
Also free the input device on exit.

drivers/built-in.o: In function `iwl_rfkill_unregister':
(.text+0xbf430): undefined reference to `input_unregister_device'
drivers/built-in.o: In function `iwl_rfkill_init':
(.text+0xbf51c): undefined reference to `input_allocate_device'
drivers/built-in.o: In function `iwl_rfkill_init':
(.text+0xbf5bf): undefined reference to `input_register_device'
drivers/built-in.o: In function `iwl_rfkill_init':
(.text+0xbf5e9): undefined reference to `input_free_device'
net/built-in.o: In function `rfkill_disconnect':
rfkill-input.c:(.text+0xe71e1): undefined reference to
`input_close_device'
rfkill-input.c:(.text+0xe71e9): undefined reference to
`input_unregister_handle'
net/built-in.o: In function `rfkill_connect':
rfkill-input.c:(.text+0xe723e): undefined reference to
`input_register_handle'
rfkill-input.c:(.text+0xe724d): undefined reference to
`input_open_device'
rfkill-input.c:(.text+0xe725c): undefined reference to
`input_unregister_handle'
net/built-in.o: In function `rfkill_handler_init':
rfkill-input.c:(.init.text+0x36ec): undefined reference to
`input_register_handler'
net/built-in.o: In function `rfkill_handler_exit':
rfkill-input.c:(.exit.text+0x112c): undefined reference to
`input_unregister_handler'
make[1]: *** [.tmp_vmlinux1] Error 1

The iwl3945 portion is done by Adel Gadllah <[email protected]>.
The patch fixes the RFKILL_INPUT dependency bug pointed by
Ivo van Doorn <[email protected]>.

Signed-off-by: Randy Dunlap <[email protected]>
Signed-off-by: Adel Gadllah <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwlwifi/Kconfig | 6 +++-
drivers/net/wireless/iwlwifi/iwl-3945.h | 4 +-
drivers/net/wireless/iwlwifi/iwl-rfkill.c | 34
+++++++++++++++----------
drivers/net/wireless/iwlwifi/iwl3945-base.c | 36
+++++++++++++++-----------
4 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/Kconfig
b/drivers/net/wireless/iwlwifi/Kconfig
index a382c00..82b66a3 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -8,7 +8,6 @@ config IWLCORE
select MAC80211_LEDS if IWLWIFI_LEDS
select LEDS_CLASS if IWLWIFI_LEDS
select RFKILL if IWLWIFI_RFKILL
- select RFKILL_INPUT if IWLWIFI_RFKILL

config IWLWIFI_LEDS
bool
@@ -105,6 +104,7 @@ config IWL3945
select IWLWIFI
select MAC80211_LEDS if IWL3945_LEDS
select LEDS_CLASS if IWL3945_LEDS
+ select RFKILL if IWL3945_RFKILL
---help---
Select to build the driver supporting the:

@@ -127,6 +127,10 @@ config IWL3945
say M here and read <file:Documentation/kbuild/modules.txt>. The
module will be called iwl3945.ko.

+config IWL3945_RFKILL
+ bool "Enable RF kill support in iwl3945 drivers"
+ depends on IWL3945
+
config IWL3945_SPECTRUM_MEASUREMENT
bool "Enable Spectrum Measurement in iwl3945 drivers"
depends on IWL3945
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h
b/drivers/net/wireless/iwlwifi/iwl-3945.h
index a774978..4df42ad 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.h
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
@@ -690,7 +690,7 @@ enum {

#endif

-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
struct iwl3945_priv;

struct iwl3945_rfkill_mngr {
@@ -800,7 +800,7 @@ struct iwl3945_priv {
struct iwl3945_init_alive_resp card_alive_init;
struct iwl3945_alive_resp card_alive;

-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
struct iwl3945_rfkill_mngr rfkill_mngr;
#endif

diff --git a/drivers/net/wireless/iwlwifi/iwl-rfkill.c
b/drivers/net/wireless/iwlwifi/iwl-rfkill.c
index 5f09874..feb3477 100644
--- a/drivers/net/wireless/iwlwifi/iwl-rfkill.c
+++ b/drivers/net/wireless/iwlwifi/iwl-rfkill.c
@@ -95,11 +95,18 @@ int iwl_rfkill_init(struct iwl_priv *priv)
priv->rfkill_mngr.rfkill->dev.class->suspend = NULL;
priv->rfkill_mngr.rfkill->dev.class->resume = NULL;

+ ret = rfkill_register(priv->rfkill_mngr.rfkill);
+ if (ret) {
+ IWL_ERROR("Unable to register rfkill: %d\n", ret);
+ goto freed_rfkill;
+ }
+
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
priv->rfkill_mngr.input_dev = input_allocate_device();
if (!priv->rfkill_mngr.input_dev) {
IWL_ERROR("Unable to allocate rfkill input device.\n");
ret = -ENOMEM;
- goto freed_rfkill;
+ goto unregister_rfkill;
}

priv->rfkill_mngr.input_dev->name = priv->cfg->name;
@@ -110,29 +117,26 @@ int iwl_rfkill_init(struct iwl_priv *priv)
priv->rfkill_mngr.input_dev->evbit[0] = BIT(EV_KEY);
set_bit(KEY_WLAN, priv->rfkill_mngr.input_dev->keybit);

- ret = rfkill_register(priv->rfkill_mngr.rfkill);
- if (ret) {
- IWL_ERROR("Unable to register rfkill: %d\n", ret);
- goto free_input_dev;
- }
-
ret = input_register_device(priv->rfkill_mngr.input_dev);
if (ret) {
IWL_ERROR("Unable to register rfkill input device: %d\n", ret);
- goto unregister_rfkill;
+ goto free_input_dev;
}
+#endif

IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
return ret;

-unregister_rfkill:
- rfkill_unregister(priv->rfkill_mngr.rfkill);
- priv->rfkill_mngr.rfkill = NULL;
-
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
free_input_dev:
input_free_device(priv->rfkill_mngr.input_dev);
priv->rfkill_mngr.input_dev = NULL;

+unregister_rfkill:
+ rfkill_unregister(priv->rfkill_mngr.rfkill);
+ priv->rfkill_mngr.rfkill = NULL;
+#endif
+
freed_rfkill:
if (priv->rfkill_mngr.rfkill != NULL)
rfkill_free(priv->rfkill_mngr.rfkill);
@@ -146,14 +150,16 @@ EXPORT_SYMBOL(iwl_rfkill_init);

void iwl_rfkill_unregister(struct iwl_priv *priv)
{
-
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
if (priv->rfkill_mngr.input_dev)
input_unregister_device(priv->rfkill_mngr.input_dev);
+ input_free_device(priv->rfkill_mngr.input_dev);
+ priv->rfkill_mngr.input_dev = NULL;
+#endif

if (priv->rfkill_mngr.rfkill)
rfkill_unregister(priv->rfkill_mngr.rfkill);

- priv->rfkill_mngr.input_dev = NULL;
priv->rfkill_mngr.rfkill = NULL;
}
EXPORT_SYMBOL(iwl_rfkill_unregister);
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c
b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 73942a4..df74ca5 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -8270,7 +8270,7 @@ static int iwl3945_pci_resume(struct pci_dev
*pdev)
#endif /* CONFIG_PM */

/*************** RFKILL FUNCTIONS **********/
-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
/* software rf-kill from user */
static int iwl3945_rfkill_soft_rf_kill(void *data, enum rfkill_state
state)
{
@@ -8328,11 +8328,18 @@ int iwl3945_rfkill_init(struct iwl3945_priv
*priv)
priv->rfkill_mngr.rfkill->dev.class->suspend = NULL;
priv->rfkill_mngr.rfkill->dev.class->resume = NULL;

+ ret = rfkill_register(priv->rfkill_mngr.rfkill);
+ if (ret) {
+ IWL_ERROR("Unable to register rfkill: %d\n", ret);
+ goto freed_rfkill;
+ }
+
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
priv->rfkill_mngr.input_dev = input_allocate_device();
if (!priv->rfkill_mngr.input_dev) {
IWL_ERROR("Unable to allocate rfkill input device.\n");
ret = -ENOMEM;
- goto freed_rfkill;
+ goto unregister_rfkill;
}

priv->rfkill_mngr.input_dev->name = priv->cfg->name;
@@ -8343,29 +8350,26 @@ int iwl3945_rfkill_init(struct iwl3945_priv
*priv)
priv->rfkill_mngr.input_dev->evbit[0] = BIT(EV_KEY);
set_bit(KEY_WLAN, priv->rfkill_mngr.input_dev->keybit);

- ret = rfkill_register(priv->rfkill_mngr.rfkill);
- if (ret) {
- IWL_ERROR("Unable to register rfkill: %d\n", ret);
- goto free_input_dev;
- }
-
ret = input_register_device(priv->rfkill_mngr.input_dev);
if (ret) {
IWL_ERROR("Unable to register rfkill input device: %d\n", ret);
- goto unregister_rfkill;
+ goto free_input_dev;
}
+#endif

IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
return ret;

-unregister_rfkill:
- rfkill_unregister(priv->rfkill_mngr.rfkill);
- priv->rfkill_mngr.rfkill = NULL;
-
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
free_input_dev:
input_free_device(priv->rfkill_mngr.input_dev);
priv->rfkill_mngr.input_dev = NULL;

+unregister_rfkill:
+ rfkill_unregister(priv->rfkill_mngr.rfkill);
+ priv->rfkill_mngr.rfkill = NULL;
+#endif
+
freed_rfkill:
if (priv->rfkill_mngr.rfkill != NULL)
rfkill_free(priv->rfkill_mngr.rfkill);
@@ -8378,14 +8382,16 @@ error:

void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
{
-
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
if (priv->rfkill_mngr.input_dev)
input_unregister_device(priv->rfkill_mngr.input_dev);
+ input_free_device(priv->rfkill_mngr.input_dev);
+ priv->rfkill_mngr.input_dev = NULL;
+#endif

if (priv->rfkill_mngr.rfkill)
rfkill_unregister(priv->rfkill_mngr.rfkill);

- priv->rfkill_mngr.input_dev = NULL;
priv->rfkill_mngr.rfkill = NULL;
}

--
1.5.3.6



Attachments:
0001-iwlwifi-fix-build-for-CONFIG_INPUT-n.patch (8.97 kB)

2008-06-26 22:13:32

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: fix up rfkill kconfig and input

On Thursday 26 June 2008, drago01 wrote:
> The attached patch fixes iwl3945 to build with CONFIG_INPUT=n and
> frees the input device on exit.
> Additionally it removes the unnecessary dependency on CONFIG_IWLCORE.
> It depends on the rfkill subsystem patch, which is in the iwlwifi-2.6 tree.
>
> -----------------
> This patch fixes the iwl3945 driver to build with CONFIG_INPUT=n
> and remove the dependency on CONFIG_IWLCORE.
> It also frees the input device on exit.
> Input stuff based on patch from Randy Dunlap for iwl4965/5000.
>
> Signed-off-by: Adel Gadllah <[email protected]>
>
> diff --git a/drivers/net/wireless/iwlwifi/Kconfig
> b/drivers/net/wireless/iwlwifi/Kconfig
> index a382c00..3148a28 100644
> --- a/drivers/net/wireless/iwlwifi/Kconfig
> +++ b/drivers/net/wireless/iwlwifi/Kconfig
> @@ -105,6 +105,7 @@ config IWL3945
> select IWLWIFI
> select MAC80211_LEDS if IWL3945_LEDS
> select LEDS_CLASS if IWL3945_LEDS
> + select RFKILL_INPUT if (IWL3945_RFKILL && INPUT)

That is a big no. Drivers should only use CONFIG_RFKILL and in no way
depend or select RFKILL_INPUT. RFKILL_INPUT is a decision for the user
and doesn't change anything for drivers.

See the linux-wireless thread regarding rfkill:
http://thread.gmane.org/gmane.linux.kernel.wireless.general/16653

> +#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
> priv->rfkill_mngr.input_dev = input_allocate_device();
> if (!priv->rfkill_mngr.input_dev) {
> IWL_ERROR("Unable to allocate rfkill input device.\n");
> @@ -8354,6 +8355,7 @@ int iwl3945_rfkill_init(struct iwl3945_priv *priv)
> priv->rfkill_mngr.input_dev->dev.parent = device;
> priv->rfkill_mngr.input_dev->evbit[0] = BIT(EV_KEY);
> set_bit(KEY_WLAN, priv->rfkill_mngr.input_dev->keybit);
> +#endif

This is wrong as well, the input device for the driver has no relation whatsoever with
CONFIG_RFKILL_INPUT.

> ret = rfkill_register(priv->rfkill_mngr.rfkill);
> if (ret) {
> @@ -8361,11 +8363,13 @@ int iwl3945_rfkill_init(struct iwl3945_priv *priv)
> goto free_input_dev;
> }
>
> +#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
> ret = input_register_device(priv->rfkill_mngr.input_dev);
> if (ret) {
> IWL_ERROR("Unable to register rfkill input device: %d\n", ret);
> goto unregister_rfkill;
> }
> +#endif

Same here.

> IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
> return ret;
> @@ -8375,8 +8379,10 @@ unregister_rfkill:
> priv->rfkill_mngr.rfkill = NULL;
>
> free_input_dev:
> +#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
> input_free_device(priv->rfkill_mngr.input_dev);
> priv->rfkill_mngr.input_dev = NULL;
> +#endif

And here.

> freed_rfkill:
> if (priv->rfkill_mngr.rfkill != NULL)
> @@ -8391,13 +8397,16 @@ error:
> void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
> {
>
> +#if defined(CONFIG_RFKILL_INPUT) || defined(CONFIG_RFKILL_INPUT_MODULE)
> if (priv->rfkill_mngr.input_dev)
> input_unregister_device(priv->rfkill_mngr.input_dev);
> + input_free_device(priv->rfkill_mngr.input_dev);
> + priv->rfkill_mngr.input_dev = NULL;
> +#endif

And here.

Ivo


2008-06-27 10:43:25

by Fabien Crespel

[permalink] [raw]
Subject: Re: [PATCH v2] iwl3945: fix up rfkill kconfig and input

Hello,

After looking at these patches about rfkill for iwlwifi, I fail to see
the use of the input layer. To me, it seems the input device is
allocated, registered, and eventually unregistered. But not used at all.

Furthermore, the only key bit set is the KEY_WLAN one, and a wireless
device driver should NOT issue such events, according to the rfkill
documentation:

"Most wireless communication device drivers implementing rfkill
functionality MUST NOT generate these events, and have no reason to
register themselves with the input layer. Doing otherwise is a common
misconception. There is an API to propagate rfkill status change
information, and it is NOT the input layer."

So the only drivers that should register to the input layer for such
events are the laptop platform drivers (thinkpad-acpi, asus-laptop, etc.)

Therefore I would recommend removing the input device completely from
iwlwifi, and of course the dependency on CONFIG_INPUT.

- Fabien.


2008-06-26 22:27:04

by drago01

[permalink] [raw]
Subject: [PATCH v2] iwl3945: fix up rfkill kconfig and input

On Fri, Jun 27, 2008 at 12:15 AM, Ivo van Doorn <[email protected]> wrote:
>> diff --git a/drivers/net/wireless/iwlwifi/Kconfig
>> b/drivers/net/wireless/iwlwifi/Kconfig
>> index a382c00..3148a28 100644
>> --- a/drivers/net/wireless/iwlwifi/Kconfig
>> +++ b/drivers/net/wireless/iwlwifi/Kconfig
>> @@ -105,6 +105,7 @@ config IWL3945
>> select IWLWIFI
>> select MAC80211_LEDS if IWL3945_LEDS
>> select LEDS_CLASS if IWL3945_LEDS
>> + select RFKILL_INPUT if (IWL3945_RFKILL && INPUT)
>
> That is a big no. Drivers should only use CONFIG_RFKILL and in no way
> depend or select RFKILL_INPUT. RFKILL_INPUT is a decision for the user
> and doesn't change anything for drivers.

v2 is attached it removed the CONFIG_RFKILL_INPUT stuff and made it
depend on CONFIG_RFKILL.
That means that the 4965 patch needs to be updated/fixed too.

------
This patch fixes the iwl3945 driver to depend on CONFIG_RFKILL and
CONFIG_INPUT when RFKILL is used.
It removes the dependency on CONFIG_IWLCORE and frees the input
device on exit.

Signed-off-by: Adel Gadllah <[email protected]>

diff --git a/drivers/net/wireless/iwlwifi/Kconfig
b/drivers/net/wireless/iwlwifi/Kconfig
index a382c00..c7fd8aa 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -105,6 +105,8 @@ config IWL3945
select IWLWIFI
select MAC80211_LEDS if IWL3945_LEDS
select LEDS_CLASS if IWL3945_LEDS
+ select RFKILL if IWL3945_RFKILL
+ select INPUT if IWL3945_RFKILL
---help---
Select to build the driver supporting the:

@@ -127,6 +129,10 @@ config IWL3945
say M here and read <file:Documentation/kbuild/modules.txt>. The
module will be called iwl3945.ko.

+config IWL3945_RFKILL
+ boolean "Enable RF kill support in iwl3945 drivers"
+ depends on IWL3945
+
config IWL3945_SPECTRUM_MEASUREMENT
bool "Enable Spectrum Measurement in iwl3945 drivers"
depends on IWL3945
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h
b/drivers/net/wireless/iwlwifi/iwl-3945.h
index a774978..4df42ad 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.h
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
@@ -690,7 +690,7 @@ enum {

#endif

-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
struct iwl3945_priv;

struct iwl3945_rfkill_mngr {
@@ -800,7 +800,7 @@ struct iwl3945_priv {
struct iwl3945_init_alive_resp card_alive_init;
struct iwl3945_alive_resp card_alive;

-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
struct iwl3945_rfkill_mngr rfkill_mngr;
#endif

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c
b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 43cb8ff..e12baaa 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -8282,7 +8282,7 @@ static int iwl3945_pci_resume(struct pci_dev *pdev)
#endif /* CONFIG_PM */

/*************** RFKILL FUNCTIONS **********/
-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
/* software rf-kill from user */
static int iwl3945_rfkill_soft_rf_kill(void *data, enum rfkill_state state)
{
@@ -8391,8 +8391,11 @@ error:
void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
{

- if (priv->rfkill_mngr.input_dev)
+ if (priv->rfkill_mngr.input_dev) {
input_unregister_device(priv->rfkill_mngr.input_dev);
+ input_free_device(priv->rfkill_mngr.input_dev);
+ }
+

if (priv->rfkill_mngr.rfkill)
rfkill_unregister(priv->rfkill_mngr.rfkill);

2008-06-26 22:56:25

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v2] iwl3945: fix up rfkill kconfig and input


--- Original Message ---
> On Friday 27 June 2008, drago01 wrote:
> > On Fri, Jun 27, 2008 at 12:15 AM, Ivo van Doorn <[email protected]> wrote:
> > >> diff --git a/drivers/net/wireless/iwlwifi/Kconfig
> > >> b/drivers/net/wireless/iwlwifi/Kconfig
> > >> index a382c00..3148a28 100644
> > >> --- a/drivers/net/wireless/iwlwifi/Kconfig
> > >> +++ b/drivers/net/wireless/iwlwifi/Kconfig
> > >> @@ -105,6 +105,7 @@ config IWL3945
> > >> select IWLWIFI
> > >> select MAC80211_LEDS if IWL3945_LEDS
> > >> select LEDS_CLASS if IWL3945_LEDS
> > >> + select RFKILL_INPUT if (IWL3945_RFKILL && INPUT)
> > >
> > > That is a big no. Drivers should only use CONFIG_RFKILL and in no way
> > > depend or select RFKILL_INPUT. RFKILL_INPUT is a decision for the user
> > > and doesn't change anything for drivers.
> >
> > v2 is attached it removed the CONFIG_RFKILL_INPUT stuff and made it
> > depend on CONFIG_RFKILL.
> > That means that the 4965 patch needs to be updated/fixed too.
>
> Not sure if it should depend or select CONFIG_INPUT,
> but the RFKILL dependencies are now correct.

I'd say depend on, but it's up to you guys + maintainer.

Here's what Documentation/kbuild/kconfig-language.txt says:

Note:
select should be used with care. select will force
a symbol to a value without visiting the dependencies.
By abusing select you are able to select a symbol FOO even
if FOO depends on BAR that is not set.
In general use select only for non-visible symbols
(no prompts anywhere) and for symbols with no dependencies.
That will limit the usefulness but on the other hand avoid
the illegal configurations all over.
kconfig should one day warn about such things.


and in general select is OK on smallish library-like functions and should
not be used on entire subsystems. To me that means that it should not
be used in CONFIG_INPUT....


> Thanks,
>
> Ivo
>
> > ------
> > This patch fixes the iwl3945 driver to depend on CONFIG_RFKILL and
> > CONFIG_INPUT when RFKILL is used.
> > It removes the dependency on CONFIG_IWLCORE and frees the input
> > device on exit.
> >
> > Signed-off-by: Adel Gadllah <[email protected]>
> >
> > diff --git a/drivers/net/wireless/iwlwifi/Kconfig
> > b/drivers/net/wireless/iwlwifi/Kconfig
> > index a382c00..c7fd8aa 100644
> > --- a/drivers/net/wireless/iwlwifi/Kconfig
> > +++ b/drivers/net/wireless/iwlwifi/Kconfig
> > @@ -105,6 +105,8 @@ config IWL3945
> > select IWLWIFI
> > select MAC80211_LEDS if IWL3945_LEDS
> > select LEDS_CLASS if IWL3945_LEDS
> > + select RFKILL if IWL3945_RFKILL
> > + select INPUT if IWL3945_RFKILL
> > ---help---
> > Select to build the driver supporting the:
> >
> > @@ -127,6 +129,10 @@ config IWL3945
> > say M here and read <file:Documentation/kbuild/modules.txt>. The
> > module will be called iwl3945.ko.
> >
> > +config IWL3945_RFKILL
> > + boolean "Enable RF kill support in iwl3945 drivers"
> > + depends on IWL3945
> > +
> > config IWL3945_SPECTRUM_MEASUREMENT
> > bool "Enable Spectrum Measurement in iwl3945 drivers"
> > depends on IWL3945
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h
> > b/drivers/net/wireless/iwlwifi/iwl-3945.h
> > index a774978..4df42ad 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-3945.h
> > +++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
> > @@ -690,7 +690,7 @@ enum {
> >
> > #endif
> >
> > -#ifdef CONFIG_IWLWIFI_RFKILL
> > +#ifdef CONFIG_IWL3945_RFKILL
> > struct iwl3945_priv;
> >
> > struct iwl3945_rfkill_mngr {
> > @@ -800,7 +800,7 @@ struct iwl3945_priv {
> > struct iwl3945_init_alive_resp card_alive_init;
> > struct iwl3945_alive_resp card_alive;
> >
> > -#ifdef CONFIG_IWLWIFI_RFKILL
> > +#ifdef CONFIG_IWL3945_RFKILL
> > struct iwl3945_rfkill_mngr rfkill_mngr;
> > #endif
> >
> > diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> > b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> > index 43cb8ff..e12baaa 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> > @@ -8282,7 +8282,7 @@ static int iwl3945_pci_resume(struct pci_dev *pdev)
> > #endif /* CONFIG_PM */
> >
> > /*************** RFKILL FUNCTIONS **********/
> > -#ifdef CONFIG_IWLWIFI_RFKILL
> > +#ifdef CONFIG_IWL3945_RFKILL
> > /* software rf-kill from user */
> > static int iwl3945_rfkill_soft_rf_kill(void *data, enum rfkill_state state)
> > {
> > @@ -8391,8 +8391,11 @@ error:
> > void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
> > {
> >
> > - if (priv->rfkill_mngr.input_dev)
> > + if (priv->rfkill_mngr.input_dev) {
> > input_unregister_device(priv->rfkill_mngr.input_dev);
> > + input_free_device(priv->rfkill_mngr.input_dev);
> > + }
> > +
> >
> > if (priv->rfkill_mngr.rfkill)
> > rfkill_unregister(priv->rfkill_mngr.rfkill);
> >
>
>