This is the current set of rfkill changes that I'd like to see
merged for submission in the next merge window (2.6.28).
It contains revised versions of the patches from set 1 [just the ones
that were not merged into wireless-testing yet] as per the comments
received.
I have included Felipe Balbi's patch as well, to keep them all in one
place. I have removed the "net: " from Felipe's patch *title* to keep
it in line with the prefix that is being used for rfkill (just
"rfkill: ").
If there are no problems with the patches and Ivo is okay with them, I'd
like to see them merged into wireless-testing.
Shortlog:
Felipe Balbi (1):
rfkill: add missing line break
Henrique de Moraes Holschuh (3):
rfkill: add WARN and BUG_ON paranoia (v2)
rfkill: rename rfkill_mutex to rfkill_global_mutex
rfkill: remove transmitter blocking on suspend
Also available for PULL from:
git://repo.or.cz/linux-2.6/linux-acpi-2.6/ibm-acpi-2.6.git rfkill
The patches are based on the latest wireless-testing master.
Thank you.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
From: Felipe Balbi <[email protected]>
Trivial patch adding a missing line break on
rfkill_claim_show().
Signed-off-by: Felipe Balbi <[email protected]>
Acked-by: Henrique de Moraes Holschuh <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
---
net/rfkill/rfkill.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 173d039..b630f35 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -431,7 +431,7 @@ static ssize_t rfkill_claim_show(struct device *dev,
{
struct rfkill *rfkill = to_rfkill(dev);
- return sprintf(buf, "%d", rfkill->user_claim);
+ return sprintf(buf, "%d\n", rfkill->user_claim);
}
static ssize_t rfkill_claim_store(struct device *dev,
--
1.5.6.3
On Tue, 26 Aug 2008, Greg KH wrote:
> > Drivers that need rfkill's older behaviour will have to implement it by
> > themselves in their own suspend handling.
[...]
> > The following drivers need to check if they need to explicitly block
> > their transmitters in their own suspend handlers (maintainers Cc'd):
>
> Normally when you change a kernel-api to operate differently, you also
> fix up the relevant parts of the kernel as well. Have you looked into
That requires one to actually KNOW the details of the relevant parts of the
kernel. In this case, the particular needs of several devices and their
device drivers.
Which I don't, for many of them.
> these drivers to see if they do have this problem or not?
Those I know enough to be 100% certain that do not need it (or actually
REQUIRE it not to happen, like libertas) have not been added to the CC.
There were none I could be 100% certain that would need the toggle_radio
call (if there were, they would have been fixed by the patch as well).
For all the others that I am not sure of, I CC'd the maintainers. If any of
them reply indicating that they need the explicit toggle_radio() call, we
can add such changes to the patch (or in a future patch). I consider this
equivalent to emailing everyone and asking about it.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Thursday 28 August 2008, Henrique de Moraes Holschuh wrote:
> On Tue, 26 Aug 2008, Ivo van Doorn wrote:
> > > drivers/net/wireless/rt2x00/* (USB might need it?)
> >
> > rt2x00 doesn't have rfkill support for the USB drivers, only the PCI drivers,
> > (because only those cards could have an actual rfkill switch.
>
> Well, rfkill support is not related only to input devices, but also to
> transmitters. And EVERY transmitter MUST have one (and a single one) rfkill
> class attached to it, unless the hardware is completely and utterly unable
> to block its transmitter(s). I hope no rt2x00 device is like that...
True, but at the moment the rt2x00 implementation is limited to the PCI devices
which indicate they have the input device. Extending it to register the rfkill
structure for all rt2x00 hardware is on my todo list.
> > Other then that no changes are required in rt2x00 with this patch.
>
> Which I will take to mean that all rt2x00 drivers, be them USB or PCI, will
> do whatever is needed (including shutting down transmitters explicitly, when
> the hardware doesn't do it implicitly because of something else the driver
> is doing) on their own suspend() methods.
>
> Is that correct? Because that's all patch 5 of 5 is concerned with.
That is correct, all rt2x00 drivers (with or without rfkill) will do this correctly.
Ivo
Currently, rfkill would stand in the way of properly supporting wireless
devices that are capable of waking the system up from sleep or hibernation
when they receive a special wireless message. It would also get in the way
of mesh devices that need to remain operational even during platform
suspend.
To avoid that, stop trying to block the transmitters on the rfkill class
suspend handler.
Drivers that need rfkill's older behaviour will have to implement it by
themselves in their own suspend handling.
Do note that rfkill *will* attempt to restore the transmitter state on
resume in any situation. This happens after the driver's resume method is
called by the suspend core (class devices resume after the devices they are
attached to have been resumed).
The following drivers need to check if they need to explicitly block
their transmitters in their own suspend handlers (maintainers Cc'd):
arch/arm/mach-pxa/tosa-bt.c
drivers/net/usb/hso.c
drivers/net/wireless/rt2x00/* (USB might need it?)
drivers/net/wireless/b43/ (SSB over USB might need it?)
drivers/misc/hp-wmi.c
eeepc-laptop w/rfkill support (not in mainline yet)
Compal laptop w/rfkill support (not in mainline yet)
toshiba-acpi w/rfkill support (not in mainline yet)
Signed-off-by: Henrique de Moraes Holschuh <[email protected]>
Cc: Ivo van Doorn <[email protected]>
Cc: Matthew Garrett <[email protected]>
Cc: Andrew Bird <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Cezary Jackiewicz <[email protected]>
Cc: Philip Langdale <[email protected]>
---
Documentation/rfkill.txt | 32 ++++++++++++++++++++++++++++----
net/rfkill/rfkill.c | 16 ++--------------
2 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 6fcb306..b65f079 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -341,6 +341,8 @@ key that does nothing by itself, as well as any hot key that is type-specific
3.1 Guidelines for wireless device drivers
------------------------------------------
+(in this text, rfkill->foo means the foo field of struct rfkill).
+
1. Each independent transmitter in a wireless device (usually there is only one
transmitter per device) should have a SINGLE rfkill class attached to it.
@@ -363,10 +365,32 @@ This rule exists because users of the rfkill subsystem expect to get (and set,
when possible) the overall transmitter rfkill state, not of a particular rfkill
line.
-5. During suspend, the rfkill class will attempt to soft-block the radio
-through a call to rfkill->toggle_radio, and will try to restore its previous
-state during resume. After a rfkill class is suspended, it will *not* call
-rfkill->toggle_radio until it is resumed.
+5. The wireless device driver MUST NOT leave the transmitter enabled during
+suspend and hibernation unless:
+
+ 5.1. The transmitter has to be enabled for some sort of functionality
+ like wake-on-wireless-packet or autonomous packed forwarding in a mesh
+ network, and that functionality is enabled for this suspend/hibernation
+ cycle.
+
+AND
+
+ 5.2. The device was not on a user-requested BLOCKED state before
+ the suspend (i.e. the driver must NOT unblock a device, not even
+ to support wake-on-wireless-packet or remain in the mesh).
+
+In other words, there is absolutely no allowed scenario where a driver can
+automatically take action to unblock a rfkill controller (obviously, this deals
+with scenarios where soft-blocking or both soft and hard blocking is happening.
+Scenarios where hardware rfkill lines are the only ones blocking the
+transmitter are outside of this rule, since the wireless device driver does not
+control its input hardware rfkill lines in the first place).
+
+6. During resume, rfkill will try to restore its previous state.
+
+7. After a rfkill class is suspended, it will *not* call rfkill->toggle_radio
+until it is resumed.
+
Example of a WLAN wireless driver connected to the rfkill subsystem:
--------------------------------------------------------------------
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index d573579..ea0dc04 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -512,21 +512,9 @@ static void rfkill_release(struct device *dev)
#ifdef CONFIG_PM
static int rfkill_suspend(struct device *dev, pm_message_t state)
{
- struct rfkill *rfkill = to_rfkill(dev);
-
- if (dev->power.power_state.event != state.event) {
- if (state.event & PM_EVENT_SLEEP) {
- /* Stop transmitter, keep state, no notifies */
- update_rfkill_state(rfkill);
-
- mutex_lock(&rfkill->mutex);
- rfkill->toggle_radio(rfkill->data,
- RFKILL_STATE_SOFT_BLOCKED);
- mutex_unlock(&rfkill->mutex);
- }
-
+ /* mark class device as suspended */
+ if (dev->power.power_state.event != state.event)
dev->power.power_state = state;
- }
return 0;
}
--
1.5.6.3
Switch sysfs parsing to something that actually works properly.
Signed-off-by: Henrique de Moraes Holschuh <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
---
net/rfkill/rfkill.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 47e0b2d..173d039 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -402,12 +402,16 @@ static ssize_t rfkill_state_store(struct device *dev,
const char *buf, size_t count)
{
struct rfkill *rfkill = to_rfkill(dev);
- unsigned int state = simple_strtoul(buf, NULL, 0);
+ unsigned long state;
int error;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ error = strict_strtoul(buf, 0, &state);
+ if (error)
+ return error;
+
/* RFKILL_STATE_HARD_BLOCKED is illegal here... */
if (state != RFKILL_STATE_UNBLOCKED &&
state != RFKILL_STATE_SOFT_BLOCKED)
@@ -435,7 +439,8 @@ static ssize_t rfkill_claim_store(struct device *dev,
const char *buf, size_t count)
{
struct rfkill *rfkill = to_rfkill(dev);
- bool claim = !!simple_strtoul(buf, NULL, 0);
+ unsigned long claim_tmp;
+ bool claim;
int error;
if (!capable(CAP_NET_ADMIN))
@@ -444,6 +449,11 @@ static ssize_t rfkill_claim_store(struct device *dev,
if (rfkill->user_claim_unsupported)
return -EOPNOTSUPP;
+ error = strict_strtoul(buf, 0, &claim_tmp);
+ if (error)
+ return error;
+ claim = !!claim_tmp;
+
/*
* Take the global lock to make sure the kernel is not in
* the middle of rfkill_switch_all
--
1.5.6.3
On Tue, Aug 26, 2008 at 7:13 PM, Ivo van Doorn <[email protected]> wrote:
> On Tuesday 26 August 2008, Henrique de Moraes Holschuh wrote:
>> Currently, rfkill would stand in the way of properly supporting wireless
>> devices that are capable of waking the system up from sleep or hibernation
>> when they receive a special wireless message. It would also get in the way
>> of mesh devices that need to remain operational even during platform
>> suspend.
>>
>> To avoid that, stop trying to block the transmitters on the rfkill class
>> suspend handler.
>>
>> Drivers that need rfkill's older behaviour will have to implement it by
>> themselves in their own suspend handling.
>>
>> Do note that rfkill *will* attempt to restore the transmitter state on
>> resume in any situation. This happens after the driver's resume method is
>> called by the suspend core (class devices resume after the devices they are
>> attached to have been resumed).
>>
>> The following drivers need to check if they need to explicitly block
>> their transmitters in their own suspend handlers (maintainers Cc'd):
>> arch/arm/mach-pxa/tosa-bt.c
>> drivers/net/usb/hso.c
>> drivers/net/wireless/rt2x00/* (USB might need it?)
>
> rt2x00 doesn't have rfkill support for the USB drivers, only the PCI drivers,
> (because only those cards could have an actual rfkill switch.
>
> Other then that no changes are required in rt2x00 with this patch.
>
>> drivers/net/wireless/b43/ (SSB over USB might need it?)
>> drivers/misc/hp-wmi.c
>> eeepc-laptop w/rfkill support (not in mainline yet)
>> Compal laptop w/rfkill support (not in mainline yet)
>> toshiba-acpi w/rfkill support (not in mainline yet)
>>
>> Signed-off-by: Henrique de Moraes Holschuh <[email protected]>
>> Cc: Ivo van Doorn <[email protected]>
>> Cc: Matthew Garrett <[email protected]>
>> Cc: Andrew Bird <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Cezary Jackiewicz <[email protected]>
>> Cc: Philip Langdale <[email protected]>
>
> Acked-by: Ivo van Doorn <[email protected]>
Acked-by: Tomas Winkler <[email protected]>
>> ---
>> Documentation/rfkill.txt | 32 ++++++++++++++++++++++++++++----
>> net/rfkill/rfkill.c | 16 ++--------------
>> 2 files changed, 30 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
>> index 6fcb306..b65f079 100644
>> --- a/Documentation/rfkill.txt
>> +++ b/Documentation/rfkill.txt
>> @@ -341,6 +341,8 @@ key that does nothing by itself, as well as any hot key that is type-specific
>> 3.1 Guidelines for wireless device drivers
>> ------------------------------------------
>>
>> +(in this text, rfkill->foo means the foo field of struct rfkill).
>> +
>> 1. Each independent transmitter in a wireless device (usually there is only one
>> transmitter per device) should have a SINGLE rfkill class attached to it.
>>
>> @@ -363,10 +365,32 @@ This rule exists because users of the rfkill subsystem expect to get (and set,
>> when possible) the overall transmitter rfkill state, not of a particular rfkill
>> line.
>>
>> -5. During suspend, the rfkill class will attempt to soft-block the radio
>> -through a call to rfkill->toggle_radio, and will try to restore its previous
>> -state during resume. After a rfkill class is suspended, it will *not* call
>> -rfkill->toggle_radio until it is resumed.
>> +5. The wireless device driver MUST NOT leave the transmitter enabled during
>> +suspend and hibernation unless:
>> +
>> + 5.1. The transmitter has to be enabled for some sort of functionality
>> + like wake-on-wireless-packet or autonomous packed forwarding in a mesh
>> + network, and that functionality is enabled for this suspend/hibernation
>> + cycle.
>> +
>> +AND
>> +
>> + 5.2. The device was not on a user-requested BLOCKED state before
>> + the suspend (i.e. the driver must NOT unblock a device, not even
>> + to support wake-on-wireless-packet or remain in the mesh).
>> +
>> +In other words, there is absolutely no allowed scenario where a driver can
>> +automatically take action to unblock a rfkill controller (obviously, this deals
>> +with scenarios where soft-blocking or both soft and hard blocking is happening.
>> +Scenarios where hardware rfkill lines are the only ones blocking the
>> +transmitter are outside of this rule, since the wireless device driver does not
>> +control its input hardware rfkill lines in the first place).
>> +
>> +6. During resume, rfkill will try to restore its previous state.
>> +
>> +7. After a rfkill class is suspended, it will *not* call rfkill->toggle_radio
>> +until it is resumed.
>> +
>>
>> Example of a WLAN wireless driver connected to the rfkill subsystem:
>> --------------------------------------------------------------------
>> diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
>> index d573579..ea0dc04 100644
>> --- a/net/rfkill/rfkill.c
>> +++ b/net/rfkill/rfkill.c
>> @@ -512,21 +512,9 @@ static void rfkill_release(struct device *dev)
>> #ifdef CONFIG_PM
>> static int rfkill_suspend(struct device *dev, pm_message_t state)
>> {
>> - struct rfkill *rfkill = to_rfkill(dev);
>> -
>> - if (dev->power.power_state.event != state.event) {
>> - if (state.event & PM_EVENT_SLEEP) {
>> - /* Stop transmitter, keep state, no notifies */
>> - update_rfkill_state(rfkill);
>> -
>> - mutex_lock(&rfkill->mutex);
>> - rfkill->toggle_radio(rfkill->data,
>> - RFKILL_STATE_SOFT_BLOCKED);
>> - mutex_unlock(&rfkill->mutex);
>> - }
>> -
>> + /* mark class device as suspended */
>> + if (dev->power.power_state.event != state.event)
>> dev->power.power_state = state;
>> - }
>>
>> return 0;
>> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Tue, 26 Aug 2008 11:58:01 -0300, Henrique de Moraes Holschuh
<[email protected]> wrote:
> Currently, rfkill would stand in the way of properly supporting wireless
> devices that are capable of waking the system up from sleep or
hibernation
> when they receive a special wireless message. It would also get in the
> way
> of mesh devices that need to remain operational even during platform
> suspend.
>
> To avoid that, stop trying to block the transmitters on the rfkill class
> suspend handler.
>
> Drivers that need rfkill's older behaviour will have to implement it by
> themselves in their own suspend handling.
>
> Do note that rfkill *will* attempt to restore the transmitter state on
> resume in any situation. This happens after the driver's resume method
is
> called by the suspend core (class devices resume after the devices they
> are
> attached to have been resumed).
>
> The following drivers need to check if they need to explicitly block
> their transmitters in their own suspend handlers (maintainers Cc'd):
> toshiba-acpi w/rfkill support (not in mainline yet)
The toshiba bluetooth device is automatically disconnected and powered down
at suspend - and I'm glad to know that rfkill will still attempt to restore
the state - because the hardware definitely won't. :-)
So, no new work here.
--phil
BUG_ON() and WARN() the heck out of buggy drivers calling into the rfkill
subsystem.
Also switch from WARN_ON(1) to the new descriptive WARN().
Signed-off-by: Henrique de Moraes Holschuh <[email protected]>
Cc: Ivo van Doorn <[email protected]>
Cc: Johannes Berg <[email protected]>
---
net/rfkill/rfkill.c | 50 +++++++++++++++++++++++++++++++++++++-------------
1 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index b630f35..910699c 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -76,6 +76,7 @@ static BLOCKING_NOTIFIER_HEAD(rfkill_notifier_list);
*/
int register_rfkill_notifier(struct notifier_block *nb)
{
+ BUG_ON(!nb);
return blocking_notifier_chain_register(&rfkill_notifier_list, nb);
}
EXPORT_SYMBOL_GPL(register_rfkill_notifier);
@@ -91,6 +92,7 @@ EXPORT_SYMBOL_GPL(register_rfkill_notifier);
*/
int unregister_rfkill_notifier(struct notifier_block *nb)
{
+ BUG_ON(!nb);
return blocking_notifier_chain_unregister(&rfkill_notifier_list, nb);
}
EXPORT_SYMBOL_GPL(unregister_rfkill_notifier);
@@ -202,6 +204,9 @@ static int rfkill_toggle_radio(struct rfkill *rfkill,
* RFKILL_STATE_HARD_BLOCKED */
break;
default:
+ WARN(1, KERN_WARNING
+ "rfkill: illegal state %d passed as parameter "
+ "to rfkill_toggle_radio\n", state);
return -EINVAL;
}
@@ -236,7 +241,11 @@ static void __rfkill_switch_all(const enum rfkill_type type,
{
struct rfkill *rfkill;
- if (unlikely(state >= RFKILL_STATE_MAX))
+ if (WARN((state >= RFKILL_STATE_MAX || type >= RFKILL_TYPE_MAX),
+ KERN_WARNING
+ "rfkill: illegal state %d or type %d "
+ "passed as parameter to __rfkill_switch_all\n",
+ state, type))
return;
rfkill_global_states[type].current_state = state;
@@ -334,7 +343,11 @@ int rfkill_force_state(struct rfkill *rfkill, enum rfkill_state state)
{
enum rfkill_state oldstate;
- if (unlikely(state >= RFKILL_STATE_MAX))
+ BUG_ON(!rfkill);
+ if (WARN((state >= RFKILL_STATE_MAX),
+ KERN_WARNING
+ "rfkill: illegal state %d passed as parameter "
+ "to rfkill_force_state\n", state))
return -EINVAL;
mutex_lock(&rfkill->mutex);
@@ -593,10 +606,10 @@ static int rfkill_check_duplicity(const struct rfkill *rfkill)
memset(seen, 0, sizeof(seen));
list_for_each_entry(p, &rfkill_list, node) {
- if (p == rfkill) {
- WARN_ON(1);
+ if (WARN((p == rfkill), KERN_WARNING
+ "rfkill: illegal attempt to register "
+ "an already registered rfkill struct\n"))
return -EEXIST;
- }
set_bit(p->type, seen);
}
@@ -664,6 +677,12 @@ struct rfkill * __must_check rfkill_allocate(struct device *parent,
struct rfkill *rfkill;
struct device *dev;
+ if (WARN((type >= RFKILL_TYPE_MAX),
+ KERN_WARNING
+ "rfkill: illegal type %d passed as parameter "
+ "to rfkill_allocate\n", type))
+ return NULL;
+
rfkill = kzalloc(sizeof(struct rfkill), GFP_KERNEL);
if (!rfkill)
return NULL;
@@ -736,11 +755,12 @@ int __must_check rfkill_register(struct rfkill *rfkill)
struct device *dev = &rfkill->dev;
int error;
- if (!rfkill->toggle_radio)
- return -EINVAL;
- if (rfkill->type >= RFKILL_TYPE_MAX)
- return -EINVAL;
- if (rfkill->state >= RFKILL_STATE_MAX)
+ if (WARN((!rfkill || !rfkill->toggle_radio ||
+ rfkill->type >= RFKILL_TYPE_MAX ||
+ rfkill->state >= RFKILL_STATE_MAX),
+ KERN_WARNING
+ "rfkill: attempt to register a "
+ "badly initialized rfkill struct\n"))
return -EINVAL;
snprintf(dev->bus_id, sizeof(dev->bus_id),
@@ -775,6 +795,7 @@ EXPORT_SYMBOL(rfkill_register);
*/
void rfkill_unregister(struct rfkill *rfkill)
{
+ BUG_ON(!rfkill);
device_del(&rfkill->dev);
rfkill_remove_switch(rfkill);
rfkill_led_trigger_unregister(rfkill);
@@ -811,9 +832,12 @@ int rfkill_set_default(enum rfkill_type type, enum rfkill_state state)
{
int error;
- if (type >= RFKILL_TYPE_MAX ||
- (state != RFKILL_STATE_SOFT_BLOCKED &&
- state != RFKILL_STATE_UNBLOCKED))
+ if (WARN((type >= RFKILL_TYPE_MAX ||
+ (state != RFKILL_STATE_SOFT_BLOCKED &&
+ state != RFKILL_STATE_UNBLOCKED)),
+ KERN_WARNING
+ "rfkill: illegal state %d or type %d passed as "
+ "parameter to rfkill_set_default\n", state, type))
return -EINVAL;
mutex_lock(&rfkill_mutex);
--
1.5.6.3
On Tuesday 26 August 2008, Henrique de Moraes Holschuh wrote:
> Currently, rfkill would stand in the way of properly supporting wireless
> devices that are capable of waking the system up from sleep or hibernation
> when they receive a special wireless message. It would also get in the way
> of mesh devices that need to remain operational even during platform
> suspend.
>
> To avoid that, stop trying to block the transmitters on the rfkill class
> suspend handler.
>
> Drivers that need rfkill's older behaviour will have to implement it by
> themselves in their own suspend handling.
>
> Do note that rfkill *will* attempt to restore the transmitter state on
> resume in any situation. This happens after the driver's resume method is
> called by the suspend core (class devices resume after the devices they are
> attached to have been resumed).
>
> The following drivers need to check if they need to explicitly block
> their transmitters in their own suspend handlers (maintainers Cc'd):
> arch/arm/mach-pxa/tosa-bt.c
> drivers/net/usb/hso.c
> drivers/net/wireless/rt2x00/* (USB might need it?)
rt2x00 doesn't have rfkill support for the USB drivers, only the PCI drivers,
(because only those cards could have an actual rfkill switch.
Other then that no changes are required in rt2x00 with this patch.
> drivers/net/wireless/b43/ (SSB over USB might need it?)
> drivers/misc/hp-wmi.c
> eeepc-laptop w/rfkill support (not in mainline yet)
> Compal laptop w/rfkill support (not in mainline yet)
> toshiba-acpi w/rfkill support (not in mainline yet)
>
> Signed-off-by: Henrique de Moraes Holschuh <[email protected]>
> Cc: Ivo van Doorn <[email protected]>
> Cc: Matthew Garrett <[email protected]>
> Cc: Andrew Bird <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Cezary Jackiewicz <[email protected]>
> Cc: Philip Langdale <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
> ---
> Documentation/rfkill.txt | 32 ++++++++++++++++++++++++++++----
> net/rfkill/rfkill.c | 16 ++--------------
> 2 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
> index 6fcb306..b65f079 100644
> --- a/Documentation/rfkill.txt
> +++ b/Documentation/rfkill.txt
> @@ -341,6 +341,8 @@ key that does nothing by itself, as well as any hot key that is type-specific
> 3.1 Guidelines for wireless device drivers
> ------------------------------------------
>
> +(in this text, rfkill->foo means the foo field of struct rfkill).
> +
> 1. Each independent transmitter in a wireless device (usually there is only one
> transmitter per device) should have a SINGLE rfkill class attached to it.
>
> @@ -363,10 +365,32 @@ This rule exists because users of the rfkill subsystem expect to get (and set,
> when possible) the overall transmitter rfkill state, not of a particular rfkill
> line.
>
> -5. During suspend, the rfkill class will attempt to soft-block the radio
> -through a call to rfkill->toggle_radio, and will try to restore its previous
> -state during resume. After a rfkill class is suspended, it will *not* call
> -rfkill->toggle_radio until it is resumed.
> +5. The wireless device driver MUST NOT leave the transmitter enabled during
> +suspend and hibernation unless:
> +
> + 5.1. The transmitter has to be enabled for some sort of functionality
> + like wake-on-wireless-packet or autonomous packed forwarding in a mesh
> + network, and that functionality is enabled for this suspend/hibernation
> + cycle.
> +
> +AND
> +
> + 5.2. The device was not on a user-requested BLOCKED state before
> + the suspend (i.e. the driver must NOT unblock a device, not even
> + to support wake-on-wireless-packet or remain in the mesh).
> +
> +In other words, there is absolutely no allowed scenario where a driver can
> +automatically take action to unblock a rfkill controller (obviously, this deals
> +with scenarios where soft-blocking or both soft and hard blocking is happening.
> +Scenarios where hardware rfkill lines are the only ones blocking the
> +transmitter are outside of this rule, since the wireless device driver does not
> +control its input hardware rfkill lines in the first place).
> +
> +6. During resume, rfkill will try to restore its previous state.
> +
> +7. After a rfkill class is suspended, it will *not* call rfkill->toggle_radio
> +until it is resumed.
> +
>
> Example of a WLAN wireless driver connected to the rfkill subsystem:
> --------------------------------------------------------------------
> diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
> index d573579..ea0dc04 100644
> --- a/net/rfkill/rfkill.c
> +++ b/net/rfkill/rfkill.c
> @@ -512,21 +512,9 @@ static void rfkill_release(struct device *dev)
> #ifdef CONFIG_PM
> static int rfkill_suspend(struct device *dev, pm_message_t state)
> {
> - struct rfkill *rfkill = to_rfkill(dev);
> -
> - if (dev->power.power_state.event != state.event) {
> - if (state.event & PM_EVENT_SLEEP) {
> - /* Stop transmitter, keep state, no notifies */
> - update_rfkill_state(rfkill);
> -
> - mutex_lock(&rfkill->mutex);
> - rfkill->toggle_radio(rfkill->data,
> - RFKILL_STATE_SOFT_BLOCKED);
> - mutex_unlock(&rfkill->mutex);
> - }
> -
> + /* mark class device as suspended */
> + if (dev->power.power_state.event != state.event)
> dev->power.power_state = state;
> - }
>
> return 0;
> }
On Tue, 26 Aug 2008, Ivo van Doorn wrote:
> > drivers/net/wireless/rt2x00/* (USB might need it?)
>
> rt2x00 doesn't have rfkill support for the USB drivers, only the PCI drivers,
> (because only those cards could have an actual rfkill switch.
Well, rfkill support is not related only to input devices, but also to
transmitters. And EVERY transmitter MUST have one (and a single one) rfkill
class attached to it, unless the hardware is completely and utterly unable
to block its transmitter(s). I hope no rt2x00 device is like that...
> Other then that no changes are required in rt2x00 with this patch.
Which I will take to mean that all rt2x00 drivers, be them USB or PCI, will
do whatever is needed (including shutting down transmitters explicitly, when
the hardware doesn't do it implicitly because of something else the driver
is doing) on their own suspend() methods.
Is that correct? Because that's all patch 5 of 5 is concerned with.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
rfkill_mutex and rfkill->mutex are too easy to confuse with each other.
Rename rfkill_mutex to rfkill_global_mutex, so that they are easier to tell
apart with just one glance.
Signed-off-by: Henrique de Moraes Holschuh <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
Cc: Michael Buesch <[email protected]>
---
net/rfkill/rfkill.c | 38 ++++++++++++++++++++------------------
1 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 910699c..d573579 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -37,7 +37,7 @@ MODULE_DESCRIPTION("RF switch support");
MODULE_LICENSE("GPL");
static LIST_HEAD(rfkill_list); /* list of registered rf switches */
-static DEFINE_MUTEX(rfkill_mutex);
+static DEFINE_MUTEX(rfkill_global_mutex);
static unsigned int rfkill_default_state = RFKILL_STATE_UNBLOCKED;
module_param_named(default_state, rfkill_default_state, uint, 0444);
@@ -234,7 +234,7 @@ static int rfkill_toggle_radio(struct rfkill *rfkill,
* unless a specific switch is claimed by userspace (in which case,
* that switch is left alone) or suspended.
*
- * Caller must have acquired rfkill_mutex.
+ * Caller must have acquired rfkill_global_mutex.
*/
static void __rfkill_switch_all(const enum rfkill_type type,
const enum rfkill_state state)
@@ -263,14 +263,14 @@ static void __rfkill_switch_all(const enum rfkill_type type,
* @type: type of interfaces to be affected
* @state: the new state
*
- * Acquires rfkill_mutex and calls __rfkill_switch_all(@type, @state).
+ * Acquires rfkill_global_mutex and calls __rfkill_switch_all(@type, @state).
* Please refer to __rfkill_switch_all() for details.
*/
void rfkill_switch_all(enum rfkill_type type, enum rfkill_state state)
{
- mutex_lock(&rfkill_mutex);
+ mutex_lock(&rfkill_global_mutex);
__rfkill_switch_all(type, state);
- mutex_unlock(&rfkill_mutex);
+ mutex_unlock(&rfkill_global_mutex);
}
EXPORT_SYMBOL(rfkill_switch_all);
@@ -278,7 +278,7 @@ EXPORT_SYMBOL(rfkill_switch_all);
* rfkill_epo - emergency power off all transmitters
*
* This kicks all non-suspended rfkill devices to RFKILL_STATE_SOFT_BLOCKED,
- * ignoring everything in its path but rfkill_mutex and rfkill->mutex.
+ * ignoring everything in its path but rfkill_global_mutex and rfkill->mutex.
*
* The global state before the EPO is saved and can be restored later
* using rfkill_restore_states().
@@ -288,7 +288,8 @@ void rfkill_epo(void)
struct rfkill *rfkill;
int i;
- mutex_lock(&rfkill_mutex);
+ mutex_lock(&rfkill_global_mutex);
+
list_for_each_entry(rfkill, &rfkill_list, node) {
mutex_lock(&rfkill->mutex);
rfkill_toggle_radio(rfkill, RFKILL_STATE_SOFT_BLOCKED, 1);
@@ -300,7 +301,7 @@ void rfkill_epo(void)
rfkill_global_states[i].current_state =
RFKILL_STATE_SOFT_BLOCKED;
}
- mutex_unlock(&rfkill_mutex);
+ mutex_unlock(&rfkill_global_mutex);
}
EXPORT_SYMBOL_GPL(rfkill_epo);
@@ -315,10 +316,11 @@ void rfkill_restore_states(void)
{
int i;
- mutex_lock(&rfkill_mutex);
+ mutex_lock(&rfkill_global_mutex);
+
for (i = 0; i < RFKILL_TYPE_MAX; i++)
__rfkill_switch_all(i, rfkill_global_states[i].default_state);
- mutex_unlock(&rfkill_mutex);
+ mutex_unlock(&rfkill_global_mutex);
}
EXPORT_SYMBOL_GPL(rfkill_restore_states);
@@ -471,7 +473,7 @@ static ssize_t rfkill_claim_store(struct device *dev,
* Take the global lock to make sure the kernel is not in
* the middle of rfkill_switch_all
*/
- error = mutex_lock_interruptible(&rfkill_mutex);
+ error = mutex_lock_interruptible(&rfkill_global_mutex);
if (error)
return error;
@@ -486,7 +488,7 @@ static ssize_t rfkill_claim_store(struct device *dev,
rfkill->user_claim = claim;
}
- mutex_unlock(&rfkill_mutex);
+ mutex_unlock(&rfkill_global_mutex);
return error ? error : count;
}
@@ -621,7 +623,7 @@ static int rfkill_add_switch(struct rfkill *rfkill)
{
int error;
- mutex_lock(&rfkill_mutex);
+ mutex_lock(&rfkill_global_mutex);
error = rfkill_check_duplicity(rfkill);
if (error < 0)
@@ -642,16 +644,16 @@ static int rfkill_add_switch(struct rfkill *rfkill)
error = 0;
unlock_out:
- mutex_unlock(&rfkill_mutex);
+ mutex_unlock(&rfkill_global_mutex);
return error;
}
static void rfkill_remove_switch(struct rfkill *rfkill)
{
- mutex_lock(&rfkill_mutex);
+ mutex_lock(&rfkill_global_mutex);
list_del_init(&rfkill->node);
- mutex_unlock(&rfkill_mutex);
+ mutex_unlock(&rfkill_global_mutex);
mutex_lock(&rfkill->mutex);
rfkill_toggle_radio(rfkill, RFKILL_STATE_SOFT_BLOCKED, 1);
@@ -840,7 +842,7 @@ int rfkill_set_default(enum rfkill_type type, enum rfkill_state state)
"parameter to rfkill_set_default\n", state, type))
return -EINVAL;
- mutex_lock(&rfkill_mutex);
+ mutex_lock(&rfkill_global_mutex);
if (!test_and_set_bit(type, rfkill_states_lockdflt)) {
rfkill_global_states[type].default_state = state;
@@ -848,7 +850,7 @@ int rfkill_set_default(enum rfkill_type type, enum rfkill_state state)
} else
error = -EPERM;
- mutex_unlock(&rfkill_mutex);
+ mutex_unlock(&rfkill_global_mutex);
return error;
}
EXPORT_SYMBOL_GPL(rfkill_set_default);
--
1.5.6.3
On Tuesday 26 August 2008, Henrique de Moraes Holschuh wrote:
> BUG_ON() and WARN() the heck out of buggy drivers calling into the rfkill
> subsystem.
>
> Also switch from WARN_ON(1) to the new descriptive WARN().
The below usage of WARN() were correct when used as WARN_ON(),
(I only complained about the litteral WARN_ON(1) usage.
But apparently the WARN arguments are now the same as WARN_ON(),
so I have no objections against this patch.
> Signed-off-by: Henrique de Moraes Holschuh <[email protected]>
> Cc: Ivo van Doorn <[email protected]>
> Cc: Johannes Berg <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
> ---
> net/rfkill/rfkill.c | 50 +++++++++++++++++++++++++++++++++++++-------------
> 1 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
> index b630f35..910699c 100644
> --- a/net/rfkill/rfkill.c
> +++ b/net/rfkill/rfkill.c
> @@ -76,6 +76,7 @@ static BLOCKING_NOTIFIER_HEAD(rfkill_notifier_list);
> */
> int register_rfkill_notifier(struct notifier_block *nb)
> {
> + BUG_ON(!nb);
> return blocking_notifier_chain_register(&rfkill_notifier_list, nb);
> }
> EXPORT_SYMBOL_GPL(register_rfkill_notifier);
> @@ -91,6 +92,7 @@ EXPORT_SYMBOL_GPL(register_rfkill_notifier);
> */
> int unregister_rfkill_notifier(struct notifier_block *nb)
> {
> + BUG_ON(!nb);
> return blocking_notifier_chain_unregister(&rfkill_notifier_list, nb);
> }
> EXPORT_SYMBOL_GPL(unregister_rfkill_notifier);
> @@ -202,6 +204,9 @@ static int rfkill_toggle_radio(struct rfkill *rfkill,
> * RFKILL_STATE_HARD_BLOCKED */
> break;
> default:
> + WARN(1, KERN_WARNING
> + "rfkill: illegal state %d passed as parameter "
> + "to rfkill_toggle_radio\n", state);
> return -EINVAL;
> }
>
> @@ -236,7 +241,11 @@ static void __rfkill_switch_all(const enum rfkill_type type,
> {
> struct rfkill *rfkill;
>
> - if (unlikely(state >= RFKILL_STATE_MAX))
> + if (WARN((state >= RFKILL_STATE_MAX || type >= RFKILL_TYPE_MAX),
> + KERN_WARNING
> + "rfkill: illegal state %d or type %d "
> + "passed as parameter to __rfkill_switch_all\n",
> + state, type))
> return;
>
> rfkill_global_states[type].current_state = state;
> @@ -334,7 +343,11 @@ int rfkill_force_state(struct rfkill *rfkill, enum rfkill_state state)
> {
> enum rfkill_state oldstate;
>
> - if (unlikely(state >= RFKILL_STATE_MAX))
> + BUG_ON(!rfkill);
> + if (WARN((state >= RFKILL_STATE_MAX),
> + KERN_WARNING
> + "rfkill: illegal state %d passed as parameter "
> + "to rfkill_force_state\n", state))
> return -EINVAL;
>
> mutex_lock(&rfkill->mutex);
> @@ -593,10 +606,10 @@ static int rfkill_check_duplicity(const struct rfkill *rfkill)
> memset(seen, 0, sizeof(seen));
>
> list_for_each_entry(p, &rfkill_list, node) {
> - if (p == rfkill) {
> - WARN_ON(1);
> + if (WARN((p == rfkill), KERN_WARNING
> + "rfkill: illegal attempt to register "
> + "an already registered rfkill struct\n"))
> return -EEXIST;
> - }
> set_bit(p->type, seen);
> }
>
> @@ -664,6 +677,12 @@ struct rfkill * __must_check rfkill_allocate(struct device *parent,
> struct rfkill *rfkill;
> struct device *dev;
>
> + if (WARN((type >= RFKILL_TYPE_MAX),
> + KERN_WARNING
> + "rfkill: illegal type %d passed as parameter "
> + "to rfkill_allocate\n", type))
> + return NULL;
> +
> rfkill = kzalloc(sizeof(struct rfkill), GFP_KERNEL);
> if (!rfkill)
> return NULL;
> @@ -736,11 +755,12 @@ int __must_check rfkill_register(struct rfkill *rfkill)
> struct device *dev = &rfkill->dev;
> int error;
>
> - if (!rfkill->toggle_radio)
> - return -EINVAL;
> - if (rfkill->type >= RFKILL_TYPE_MAX)
> - return -EINVAL;
> - if (rfkill->state >= RFKILL_STATE_MAX)
> + if (WARN((!rfkill || !rfkill->toggle_radio ||
> + rfkill->type >= RFKILL_TYPE_MAX ||
> + rfkill->state >= RFKILL_STATE_MAX),
> + KERN_WARNING
> + "rfkill: attempt to register a "
> + "badly initialized rfkill struct\n"))
> return -EINVAL;
>
> snprintf(dev->bus_id, sizeof(dev->bus_id),
> @@ -775,6 +795,7 @@ EXPORT_SYMBOL(rfkill_register);
> */
> void rfkill_unregister(struct rfkill *rfkill)
> {
> + BUG_ON(!rfkill);
> device_del(&rfkill->dev);
> rfkill_remove_switch(rfkill);
> rfkill_led_trigger_unregister(rfkill);
> @@ -811,9 +832,12 @@ int rfkill_set_default(enum rfkill_type type, enum rfkill_state state)
> {
> int error;
>
> - if (type >= RFKILL_TYPE_MAX ||
> - (state != RFKILL_STATE_SOFT_BLOCKED &&
> - state != RFKILL_STATE_UNBLOCKED))
> + if (WARN((type >= RFKILL_TYPE_MAX ||
> + (state != RFKILL_STATE_SOFT_BLOCKED &&
> + state != RFKILL_STATE_UNBLOCKED)),
> + KERN_WARNING
> + "rfkill: illegal state %d or type %d passed as "
> + "parameter to rfkill_set_default\n", state, type))
> return -EINVAL;
>
> mutex_lock(&rfkill_mutex);
On Tue, Aug 26, 2008 at 11:58:01AM -0300, Henrique de Moraes Holschuh wrote:
> Currently, rfkill would stand in the way of properly supporting wireless
> devices that are capable of waking the system up from sleep or hibernation
> when they receive a special wireless message. It would also get in the way
> of mesh devices that need to remain operational even during platform
> suspend.
>
> To avoid that, stop trying to block the transmitters on the rfkill class
> suspend handler.
>
> Drivers that need rfkill's older behaviour will have to implement it by
> themselves in their own suspend handling.
>
> Do note that rfkill *will* attempt to restore the transmitter state on
> resume in any situation. This happens after the driver's resume method is
> called by the suspend core (class devices resume after the devices they are
> attached to have been resumed).
>
> The following drivers need to check if they need to explicitly block
> their transmitters in their own suspend handlers (maintainers Cc'd):
Normally when you change a kernel-api to operate differently, you also
fix up the relevant parts of the kernel as well. Have you looked into
these drivers to see if they do have this problem or not?
thanks,
greg k-h
On Tue, Aug 26, 2008 at 11:58:01AM -0300, Henrique de Moraes Holschuh wrote:
> Currently, rfkill would stand in the way of properly supporting wireless
> devices that are capable of waking the system up from sleep or hibernation
> when they receive a special wireless message. It would also get in the way
> of mesh devices that need to remain operational even during platform
> suspend.
>
> To avoid that, stop trying to block the transmitters on the rfkill class
> suspend handler.
>
> Drivers that need rfkill's older behaviour will have to implement it by
> themselves in their own suspend handling.
>
> Do note that rfkill *will* attempt to restore the transmitter state on
> resume in any situation. This happens after the driver's resume method is
> called by the suspend core (class devices resume after the devices they are
> attached to have been resumed).
>
> The following drivers need to check if they need to explicitly block
> their transmitters in their own suspend handlers (maintainers Cc'd):
> arch/arm/mach-pxa/tosa-bt.c
> drivers/net/usb/hso.c
> drivers/net/wireless/rt2x00/* (USB might need it?)
> drivers/net/wireless/b43/ (SSB over USB might need it?)
> drivers/misc/hp-wmi.c
> eeepc-laptop w/rfkill support (not in mainline yet)
> Compal laptop w/rfkill support (not in mainline yet)
> toshiba-acpi w/rfkill support (not in mainline yet)
Are we now satisfied that the in-kernel drivers above are all prepared
for this patch?
John
--
John W. Linville
[email protected]
On Wed, 03 Sep 2008, John W. Linville wrote:
> > The following drivers need to check if they need to explicitly block
> > their transmitters in their own suspend handlers (maintainers Cc'd):
> > arch/arm/mach-pxa/tosa-bt.c
> > drivers/net/usb/hso.c
> > drivers/net/wireless/rt2x00/* (USB might need it?)
> > drivers/net/wireless/b43/ (SSB over USB might need it?)
> > drivers/misc/hp-wmi.c
> > eeepc-laptop w/rfkill support (not in mainline yet)
> > Compal laptop w/rfkill support (not in mainline yet)
> > toshiba-acpi w/rfkill support (not in mainline yet)
>
> Are we now satisfied that the in-kernel drivers above are all prepared
> for this patch?
I didn't receive a single response stating that a driver would need special
care, but then, I didn't receive many responses at all...
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Wed, Sep 03, 2008 at 06:20:21PM -0300, Henrique de Moraes Holschuh wrote:
> On Wed, 03 Sep 2008, John W. Linville wrote:
> > > The following drivers need to check if they need to explicitly block
> > > their transmitters in their own suspend handlers (maintainers Cc'd):
> > > arch/arm/mach-pxa/tosa-bt.c
> > > drivers/net/usb/hso.c
> > > drivers/net/wireless/rt2x00/* (USB might need it?)
> > > drivers/net/wireless/b43/ (SSB over USB might need it?)
> > > drivers/misc/hp-wmi.c
> > > eeepc-laptop w/rfkill support (not in mainline yet)
> > > Compal laptop w/rfkill support (not in mainline yet)
> > > toshiba-acpi w/rfkill support (not in mainline yet)
> >
> > Are we now satisfied that the in-kernel drivers above are all prepared
> > for this patch?
>
> I didn't receive a single response stating that a driver would need special
> care, but then, I didn't receive many responses at all...
OK, I will plan on merging this shortly (let's say on Friday 12
September 2008) unless I hear serious objections.
John
On Wednesday 03 September 2008 23:20:21 Henrique de Moraes Holschuh wrote:
> On Wed, 03 Sep 2008, John W. Linville wrote:
> > > The following drivers need to check if they need to explicitly block
> > > their transmitters in their own suspend handlers (maintainers Cc'd):
> > > arch/arm/mach-pxa/tosa-bt.c
> > > drivers/net/usb/hso.c
> > > drivers/net/wireless/rt2x00/* (USB might need it?)
> > > drivers/net/wireless/b43/ (SSB over USB might need it?)
> > > drivers/misc/hp-wmi.c
> > > eeepc-laptop w/rfkill support (not in mainline yet)
> > > Compal laptop w/rfkill support (not in mainline yet)
> > > toshiba-acpi w/rfkill support (not in mainline yet)
> >
> > Are we now satisfied that the in-kernel drivers above are all prepared
> > for this patch?
The b43/b43legacy chips are completely shutdown on suspend and all
clocks are disabled (Including PHY/radio). We don't support WoL.
--
Greetings Michael.