Discussion:
[PATCH 1/8] rfkill: clarify meaning of rfkill states
(too old to reply)
Henrique de Moraes Holschuh
2008-04-11 20:37:17 UTC
Permalink
rfkill really should have been named rfswitch. As it is, one can get
confused whether RFKILL_STATE_ON means the KILL switch is on (and
therefore, the radio is being *blocked* from operating), or whether it
means the RADIO rf output is on.

Clearly state that RFKILL_STATE_ON means the radio is *unblocked* from
operating (i.e. there is no rf killing going on).

Signed-off-by: Henrique de Moraes Holschuh <***@hmh.eng.br>
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: Dmitry Torokhov <***@mail.ru>
---
include/linux/rfkill.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index e3ab21d..ca89ae1 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -44,8 +44,8 @@ enum rfkill_type {
};

enum rfkill_state {
- RFKILL_STATE_OFF = 0,
- RFKILL_STATE_ON = 1,
+ RFKILL_STATE_OFF = 0, /* Radio output blocked */
+ RFKILL_STATE_ON = 1, /* Radio output active */
};

/**
@@ -53,7 +53,7 @@ enum rfkill_state {
* @name: Name of the switch.
* @type: Radio type which the button controls, the value stored
* here should be a value from enum rfkill_type.
- * @state: State of the switch (on/off).
+ * @state: State of the switch, "ON" means radio can operate.
* @user_claim_unsupported: Whether the hardware supports exclusive
* RF-kill control by userspace. Set this before registering.
* @user_claim: Set when the switch is controlled exlusively by userspace.
--
1.5.4.4
Henrique de Moraes Holschuh
2008-04-11 20:37:18 UTC
Permalink
Fix a minor typo in an exported function documentation

Signed-off-by: Henrique de Moraes Holschuh <***@hmh.eng.br>
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: Dmitry Torokhov <***@mail.ru>
---
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 140a0a8..1601e50 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -412,7 +412,7 @@ int rfkill_register(struct rfkill *rfkill)
EXPORT_SYMBOL(rfkill_register);

/**
- * rfkill_unregister - Uegister a rfkill structure.
+ * rfkill_unregister - Unregister a rfkill structure.
* @rfkill: rfkill structure to be unregistered
*
* This function should be called by the network driver during device
--
1.5.4.4
Henrique de Moraes Holschuh
2008-04-11 20:37:20 UTC
Permalink
Currently, rfkill supports only write-only rfkill switches. There is no
provision for querying the current switch state from the hardware/firmware.

This is bad on hardware where the switch state can change behind the
kernel's back (which is rather common). There is no reason to keep kernel
state incorrect, when we have the possibility to match reality.

There is also the issue of read-only rfkill switches (support to be added
in a later patch), which absolutely requires support to read the current
state from the hardware in order to be implemented.

In order to implement the read/write functionality:

Add a get_state() hook that is called by the class every time it needs to
fetch the current state of the switch. Add a call to this hook every time
the *current* state of the radio plays a role in a decision.

Also add a force_state() method that can be used to forcefully syncronize
the class' idea of the current state of the switch. This allows for a
faster implementation of the read/write functionality, as a driver which
get events on switch changes can avoid the need for a get_state() hook.

If the get_state() hook is left as NULL, current behaviour is maintained,
so this change is fully backwards compatible with the current rfkill
drivers.

If the firmware is event driven, leave get_state() NULL in the driver, set
the initial state properly before registering the rfkill class, and use the
force_state() method in the driver to keep the class up-to-date.

get_state() can be called by the class from atomic context. It must not
sleep.

Signed-off-by: Henrique de Moraes Holschuh <***@hmh.eng.br>
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: Dmitry Torokhov <***@mail.ru>
---
include/linux/rfkill.h | 5 ++++
net/rfkill/rfkill.c | 49 +++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index ca89ae1..844e961 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -61,6 +61,8 @@ enum rfkill_state {
* @data: Pointer to the RF button drivers private data which will be
* passed along when toggling radio state.
* @toggle_radio(): Mandatory handler to control state of the radio.
+ * @get_state(): handler to read current radio state from hardware,
+ * may be called from atomic context, should return 0 on success.
* @led_trigger: A LED trigger for this button's LED.
* @dev: Device structure integrating the switch into device tree.
* @node: Used to place switch into list of all switches known to the
@@ -80,6 +82,7 @@ struct rfkill {

void *data;
int (*toggle_radio)(void *data, enum rfkill_state state);
+ int (*get_state)(void *data, enum rfkill_state *state);

#ifdef CONFIG_RFKILL_LEDS
struct led_trigger led_trigger;
@@ -95,6 +98,8 @@ void rfkill_free(struct rfkill *rfkill);
int rfkill_register(struct rfkill *rfkill);
void rfkill_unregister(struct rfkill *rfkill);

+int rfkill_force_state(struct rfkill *rfkill, enum rfkill_state state);
+
/**
* rfkill_get_led_name - Get the LED trigger name for the button's LED.
* This function might return a NULL pointer if registering of the
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 1601e50..88b0558 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -57,19 +57,39 @@ static void rfkill_led_trigger(struct rfkill *rfkill,
#endif /* CONFIG_RFKILL_LEDS */
}

+static void update_rfkill_state(struct rfkill *rfkill)
+{
+ enum rfkill_state newstate;
+
+ if (rfkill->get_state) {
+ mutex_lock(&rfkill->mutex);
+ if (!rfkill->get_state(rfkill->data, &newstate))
+ rfkill->state = newstate;
+ mutex_unlock(&rfkill->mutex);
+ }
+}
+
static int rfkill_toggle_radio(struct rfkill *rfkill,
enum rfkill_state state)
{
int retval = 0;
+ enum rfkill_state oldstate, newstate;
+
+ oldstate = rfkill->state;
+
+ if (rfkill->get_state &&
+ !rfkill->get_state(rfkill->data, &newstate))
+ rfkill->state = newstate;

if (state != rfkill->state) {
retval = rfkill->toggle_radio(rfkill->data, state);
- if (!retval) {
+ if (!retval)
rfkill->state = state;
- rfkill_led_trigger(rfkill, state);
- }
}

+ if (rfkill->state != oldstate)
+ rfkill_led_trigger(rfkill, rfkill->state);
+
return retval;
}

@@ -100,6 +120,28 @@ void rfkill_switch_all(enum rfkill_type type, enum rfkill_state state)
}
EXPORT_SYMBOL(rfkill_switch_all);

+/**
+ * rfkill_force_state - Force the internal rfkill radio state
+ * @rfkill: pointer to the rfkill class to modify.
+ * @state: the current radio state the class should be forced to.
+ *
+ * This function updates the internal state of the radio cached
+ * by the rfkill class. It should be used when the driver gets
+ * a notification by the firmware/hardware of the current *real*
+ * state of the radio rfkill switch.
+ *
+ * It may not be called from an atomic context.
+ */
+int rfkill_force_state(struct rfkill *rfkill, enum rfkill_state state)
+{
+ mutex_lock(&rfkill->mutex);
+ rfkill->state = state;
+ mutex_unlock(&rfkill->mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL(rfkill_force_state);
+
static ssize_t rfkill_name_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -142,6 +184,7 @@ static ssize_t rfkill_state_show(struct device *dev,
{
struct rfkill *rfkill = to_rfkill(dev);

+ update_rfkill_state(rfkill);
return sprintf(buf, "%d\n", rfkill->state);
}
--
1.5.4.4
Henrique de Moraes Holschuh
2008-04-14 01:20:38 UTC
Permalink
Post by Henrique de Moraes Holschuh
Currently, rfkill supports only write-only rfkill switches. There is no
provision for querying the current switch state from the hardware/firmware.
No that is by design, the input_polled_dev interface is there for polling.
I have looked into this, now. And I have to say I am not impressed with
the use of the input system for this. My guess is that somehow the need
to issue state changes for "the stuff the user slides/presses" got
confused with the need to keep the state of the "hardware and software
that enables/disables radios", just because both are called "rfkill
switches".

The current flow for read/write switches (i.e. using input-polldev) is
this:

The players:
A: RFKILL class code: write-only with a state cache
B: rfkill-input
C: the code supporting the device registered with the rfkill class
D: something else, like a hardware signal line or firmware

The flow:
C: sets initial state, and registers rfkill device with A.
A: keeps state cache in sync with any changes that goes through it
D: toggles radio state directly, A doesn't know about it.
C: using pooling or some other method, finds out what D did.
C: issues an INPUT EVENT(!!) to force A to resync itself
B: traps the INPUT EVENT, and tells A to change radio state FOR ALL
RADIOS of that type
A: toggles the radio to the state B wanted.

It is no surprise that b43 can't work right if it is using that flow.

Here's what is wrong with it:

1. Usage of input events that matter to an entire group of drivers
to syncronize something that is specific to a single device

2. Dependency of an OPTIONAL input handler to do it, and let's not
forget this can become a MAJOR mess if userspace decides to butt
in, which it is indeed *expected* to do

3. (some drivers like b43, not a rfkill class issue): the use of
KEY_ events to set a desired state for a radio is wrong. That
event tells rfkill to toggle the radio state, and NOT to set it
to a particular state. One would need EV_SW events for this.

4. Extremely heavy-weight, convoluted, complex information flow
path, prone to races when there are multiple rfkill devices of
the same type involved.

5. Breaks completely if user_claim is used.

There might be more issues with this approach, but the above is damn
good enough reason to change the approach, already.

IMPORTANT: do note that there is nothing wrong with using input-polldev
to monitor a hardware GPIO pin or somesuch, and issue input events every
time there is a change in the state of a rfkill switch (in the "the
stuff the user slides/presses" sense). That's not what I am talking
about.
Post by Henrique de Moraes Holschuh
This is bad on hardware where the switch state can change behind the
kernel's back (which is rather common). There is no reason to keep kernel
state incorrect, when we have the possibility to match reality.
There is also the issue of read-only rfkill switches (support to be added
in a later patch), which absolutely requires support to read the current
state from the hardware in order to be implemented.
See rt2x00 and b43 in driver-wireless for an example implementation
of pollable input device and rfkill
They're broken by design, at least in b43's case. Someone (Carlos, I
believe?) posted an example of a ping-pong scenario using b43.
I'm going to NACK this patch.
Given the above, would you ACK the idea that we implement a read path
for rfkill state (in the "hardware and software that enables/disables
radios" sense), which might as well be something very close to what this
patch did, AND leave input-polldev for the input device side of things?
--
"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
Ivo van Doorn
2008-04-14 12:00:41 UTC
Permalink
Post by Henrique de Moraes Holschuh
Post by Henrique de Moraes Holschuh
Currently, rfkill supports only write-only rfkill switches. There is no
provision for querying the current switch state from the hardware/firmware.
No that is by design, the input_polled_dev interface is there for polling.
I have looked into this, now. And I have to say I am not impressed with
the use of the input system for this. My guess is that somehow the need
to issue state changes for "the stuff the user slides/presses" got
confused with the need to keep the state of the "hardware and software
that enables/disables radios", just because both are called "rfkill
switches".
The current flow for read/write switches (i.e. using input-polldev) is
A: RFKILL class code: write-only with a state cache
B: rfkill-input
C: the code supporting the device registered with the rfkill class
D: something else, like a hardware signal line or firmware
C: sets initial state, and registers rfkill device with A.
A: keeps state cache in sync with any changes that goes through it
D: toggles radio state directly, A doesn't know about it.
C: using pooling or some other method, finds out what D did.
C: issues an INPUT EVENT(!!) to force A to resync itself
B: traps the INPUT EVENT, and tells A to change radio state FOR ALL
RADIOS of that type
A: toggles the radio to the state B wanted.
It is no surprise that b43 can't work right if it is using that flow.
1. Usage of input events that matter to an entire group of drivers
to syncronize something that is specific to a single device
2. Dependency of an OPTIONAL input handler to do it, and let's not
forget this can become a MAJOR mess if userspace decides to butt
in, which it is indeed *expected* to do
The input handler is optional, because userspace *should* actually do all
the work. Userspace should listen to the registered input device and
it is userspace that should select the input device it wants to listen to.
(perhaps the laptop has 2 or more keys, but the user wants all interfaces
to be toggled when any key has been pressed).
Post by Henrique de Moraes Holschuh
3. (some drivers like b43, not a rfkill class issue): the use of
KEY_ events to set a desired state for a radio is wrong. That
event tells rfkill to toggle the radio state, and NOT to set it
to a particular state. One would need EV_SW events for this.
That is a rfkill issue due to lack of correct documentation and problems
with the rfkill implementation. I wouldn't blame the drivers for that. ;)
Post by Henrique de Moraes Holschuh
4. Extremely heavy-weight, convoluted, complex information flow
path, prone to races when there are multiple rfkill devices of
the same type involved.
5. Breaks completely if user_claim is used.
There might be more issues with this approach, but the above is damn
good enough reason to change the approach, already.
True.
Post by Henrique de Moraes Holschuh
IMPORTANT: do note that there is nothing wrong with using input-polldev
to monitor a hardware GPIO pin or somesuch, and issue input events every
time there is a change in the state of a rfkill switch (in the "the
stuff the user slides/presses" sense). That's not what I am talking
about.
Right.
But note that network hardware with such keys will have a driver that handles
both features. So such network drivers will have a input device and will send
a signal that the key was pressed regardless of the current state they are in.
Post by Henrique de Moraes Holschuh
Post by Henrique de Moraes Holschuh
This is bad on hardware where the switch state can change behind the
kernel's back (which is rather common). There is no reason to keep kernel
state incorrect, when we have the possibility to match reality.
There is also the issue of read-only rfkill switches (support to be added
in a later patch), which absolutely requires support to read the current
state from the hardware in order to be implemented.
See rt2x00 and b43 in driver-wireless for an example implementation
of pollable input device and rfkill
They're broken by design, at least in b43's case. Someone (Carlos, I
believe?) posted an example of a ping-pong scenario using b43.
Thats true. But was that a scenario where there were 2 keys for the same radio type?
Or was that a matter of the driver indicating a key pressed event while it has no
attached key to it, and only checked the register for the key status and the current radio
status and saw it was different?

For example rt2x00 does:
hasHardwareKey()
yes -> register input_polldev, check key status
no -> don't do anything.

Note that write-only support hasn't been implemented yet, so rt2x00 will do nothing
if the hardware doesn't have a hardware key attached to it.
Post by Henrique de Moraes Holschuh
I'm going to NACK this patch.
Given the above, would you ACK the idea that we implement a read path
for rfkill state (in the "hardware and software that enables/disables
radios" sense), which might as well be something very close to what this
patch did, AND leave input-polldev for the input device side of things?
Depends on the implementation, I think we should at least have the following requirements:
- _all_ wireless (wifi, bluetooth, etc) network drivers register themselves to rfkill
- radio-key driver polls or listens to hardware events and sends them through the input layer
- it must be caught by either:
1) userspace (default): This can either run a lot of scripts of send a notification to rfkill
2) rfkill-input: sens notification to rfkill
- rfkill goes past all registered rfkill instances to toggle the status.

Note that the rfkill and rfkill-input description is incomplete since that is where this thread
is about. So the actions that those 2 layers should exactly do is not defined yet in this list.
However the first 2 points should be the case regardless of how rfkill handles things.

Ivo
Dmitry Torokhov
2008-04-14 14:16:06 UTC
Permalink
Post by Ivo van Doorn
Post by Henrique de Moraes Holschuh
Post by Henrique de Moraes Holschuh
Currently, rfkill supports only write-only rfkill switches. There is no
provision for querying the current switch state from the hardware/firmware.
No that is by design, the input_polled_dev interface is there for polling.
I have looked into this, now. And I have to say I am not impressed with
the use of the input system for this. My guess is that somehow the need
to issue state changes for "the stuff the user slides/presses" got
confused with the need to keep the state of the "hardware and software
that enables/disables radios", just because both are called "rfkill
switches".
The current flow for read/write switches (i.e. using input-polldev) is
A: RFKILL class code: write-only with a state cache
B: rfkill-input
C: the code supporting the device registered with the rfkill class
D: something else, like a hardware signal line or firmware
C: sets initial state, and registers rfkill device with A.
A: keeps state cache in sync with any changes that goes through it
D: toggles radio state directly, A doesn't know about it.
C: using pooling or some other method, finds out what D did.
C: issues an INPUT EVENT(!!) to force A to resync itself
B: traps the INPUT EVENT, and tells A to change radio state FOR ALL
RADIOS of that type
A: toggles the radio to the state B wanted.
It is no surprise that b43 can't work right if it is using that flow.
1. Usage of input events that matter to an entire group of drivers
to syncronize something that is specific to a single device
2. Dependency of an OPTIONAL input handler to do it, and let's not
forget this can become a MAJOR mess if userspace decides to butt
in, which it is indeed *expected* to do
The input handler is optional, because userspace *should* actually do all
the work. Userspace should listen to the registered input device and
it is userspace that should select the input device it wants to listen to.
(perhaps the laptop has 2 or more keys, but the user wants all interfaces
to be toggled when any key has been pressed).
Post by Henrique de Moraes Holschuh
3. (some drivers like b43, not a rfkill class issue): the use of
KEY_ events to set a desired state for a radio is wrong. That
event tells rfkill to toggle the radio state, and NOT to set it
to a particular state. One would need EV_SW events for this.
That is a rfkill issue due to lack of correct documentation and problems
with the rfkill implementation. I wouldn't blame the drivers for that. ;)
Post by Henrique de Moraes Holschuh
4. Extremely heavy-weight, convoluted, complex information flow
path, prone to races when there are multiple rfkill devices of
the same type involved.
5. Breaks completely if user_claim is used.
What exactly breaks with user_claim? I think the general issue is that
rfkill was originally not supposed to be used with switches that can't
be programmatically controlled. If we have a switch that mechanically
cuts off power to the RF transmitter rfkill can't relly do anything.
The input side (input polldev or other kind of button/switch/slider
inmplementation) still can issue KEY_WLAN or something to indicate that
user toggled the state and userspace or kernel may try to shut off the
rest of RF transmitters in the box but as far as this particlular
switch goes it is game over. I expected that here we'd just rely on
netif_carrier_* to notify the network side of things about interface
losing connection.

Unfortunately (or may be fortunately, I am not quite sure yet) rfkill
started to be used for mechanical type switches as well, disabling
user_claim option, so now we do need the "read-only" kind of rfkill
switch support that allows setting switch state directly from the
driver.
--
Dmitry
Henrique de Moraes Holschuh
2008-04-14 14:36:03 UTC
Permalink
Post by Dmitry Torokhov
What exactly breaks with user_claim? I think the general issue is that
The same thing that happens if userspace is in charge and doesn't
cooperate or if rfkill-input is not loaded: the rfkill driver state is
never updated (remember, it is about the radio controller, not the input
device).
Post by Dmitry Torokhov
rfkill was originally not supposed to be used with switches that can't
be programmatically controlled. If we have a switch that mechanically
The input layer does not provide a way for userspace to get access to
the current state of any switches easily (or at all?). It just exports
events reporting these states sometimes.

We could fix it in the input subsystem, instead. How difficult (and
welcome a change?) would it be for the input device infrastructure to
provide pollable (for good measure!) "state" attributes for every EV_SW
event an input device registers in evbits+swbits ?

If the input subsystem exports that information, we can define that
slave read-only radio controllers must NOT be registered as rfkill
devices, and drop support for read-only rfkill devices, as all master
rfkill devices (which are input devices by definition) would get their
state exported to userspace through the input layer.

However, that DOES mean userspace would need to look for rfkill state in
two places. Input layer, to get the current state of the rfkill input
devices, and rfkill class to get the current state of the rfkill
radio controllers.
Post by Dmitry Torokhov
The input side (input polldev or other kind of button/switch/slider
inmplementation) still can issue KEY_WLAN or something to indicate that
user toggled the state and userspace or kernel may try to shut off the
rest of RF transmitters in the box but as far as this particlular
switch goes it is game over. I expected that here we'd just rely on
netif_carrier_* to notify the network side of things about interface
losing connection.
Yes. And that's the plan, actually. But it DOES NOT work at all with
the current mess, which used input events to keep the internal
rfkill->state state in sync with the hardware.
Post by Dmitry Torokhov
Unfortunately (or may be fortunately, I am not quite sure yet) rfkill
Let's decide this NOW, then. But if we decide to not do it in rfkill,
you will have to add attributes to every input device which has
registered itself as a source of EV_SW events.
Post by Dmitry Torokhov
started to be used for mechanical type switches as well, disabling
user_claim option, so now we do need the "read-only" kind of rfkill
switch support that allows setting switch state directly from the
driver.
We always had read/write rfkill controllers, they were just not properly
supported at all, and that is the root cause of the mess.


Anyway, do we fix read-only rfkill input device support in the input
layer (add access to their state using sysfs and make it clear they are
NOT to be registered with rfkill class) or in the rfkill class (add
read-only rfkill controller support)?

I better make it clear that I have NO idea how to properly implement the
required changes in the input layer if we go that way so if it is left
for me to do that work, it won't be ready anytime soon and it might be
supbar, so I'd appreciate lots of help in that case.
--
"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
Dmitry Torokhov
2008-04-14 15:19:17 UTC
Permalink
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
What exactly breaks with user_claim? I think the general issue is that
The same thing that happens if userspace is in charge and doesn't
cooperate or if rfkill-input is not loaded: the rfkill driver state is
never updated (remember, it is about the radio controller, not the input
device).
And it should not - setting user_claim means userspace controls the
transmitter state. If userspace decides to ignore KEY_* event and not
turn the radio on or off - that is fine. rfkill-input simply provides
default kernel policy for reacting to KEY_WLAN, KEY_BLUETOOTH, etc.
If there are drivers sense button presses and alter their actual
transmitter state immediately but then rely on rfkill-input to update
their sysfs rfkill attributes then such drivers should be fixed.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
rfkill was originally not supposed to be used with switches that can't
be programmatically controlled. If we have a switch that mechanically
The input layer does not provide a way for userspace to get access to
the current state of any switches easily (or at all?). It just exports
events reporting these states sometimes.
You can use EVIOCGSW to get state of switches. I'd expect applications
to use it when they open devices first time and then rely on event
delivery to monitor changes in state of input devices.
Post by Henrique de Moraes Holschuh
We could fix it in the input subsystem, instead. How difficult (and
welcome a change?) would it be for the input device infrastructure to
provide pollable (for good measure!) "state" attributes for every EV_SW
event an input device registers in evbits+swbits ?
Given the above I don't see the need.
Post by Henrique de Moraes Holschuh
If the input subsystem exports that information, we can define that
slave read-only radio controllers must NOT be registered as rfkill
devices, and drop support for read-only rfkill devices, as all master
rfkill devices (which are input devices by definition) would get their
state exported to userspace through the input layer.
However, that DOES mean userspace would need to look for rfkill state in
two places. Input layer, to get the current state of the rfkill input
devices, and rfkill class to get the current state of the rfkill
radio controllers.
I think you outlined the problem in your other mail quite well. We
keep confusing button state with the transmitter state. Only for
buttons that are hardwired to transmitters the states are the same,
for programmatically controlled RF transmitters state may be
different. So if you need to see state of transmitters you should look
in /sys/class/rfkill and for input devices - hook into input devices.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
The input side (input polldev or other kind of button/switch/slider
inmplementation) still can issue KEY_WLAN or something to indicate that
user toggled the state and userspace or kernel may try to shut off the
rest of RF transmitters in the box but as far as this particlular
switch goes it is game over. I expected that here we'd just rely on
netif_carrier_* to notify the network side of things about interface
losing connection.
Yes. And that's the plan, actually. But it DOES NOT work at all with
the current mess, which used input events to keep the internal
rfkill->state state in sync with the hardware.
Post by Dmitry Torokhov
Unfortunately (or may be fortunately, I am not quite sure yet) rfkill
Let's decide this NOW, then. But if we decide to not do it in rfkill,
you will have to add attributes to every input device which has
registered itself as a source of EV_SW events.
No, since they may not be tied directly to any RF transmitter.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
started to be used for mechanical type switches as well, disabling
user_claim option, so now we do need the "read-only" kind of rfkill
switch support that allows setting switch state directly from the
driver.
We always had read/write rfkill controllers, they were just not properly
supported at all, and that is the root cause of the mess.
Anyway, do we fix read-only rfkill input device support in the input
layer (add access to their state using sysfs and make it clear they are
NOT to be registered with rfkill class) or in the rfkill class (add
read-only rfkill controller support)?
I better make it clear that I have NO idea how to properly implement the
required changes in the input layer if we go that way so if it is left
for me to do that work, it won't be ready anytime soon and it might be
supbar, so I'd appreciate lots of help in that case.
Well, let's see.. Do we have any issues with transmitters that can be
controlled programmatically? Or is it the hardwired ones that give us
trouble at the moment? I think that exporting all of them in rfkill is
a good idea, even though originally I thought we would not need to do
that. Therefore we do need to allow hardwided drivers updating their
rfkill state directly. The only problem I see is that we need to monitor
this closely, so non-hardwired cases won't try to use this mechanism.
--
Dmitry
Henrique de Moraes Holschuh
2008-04-14 16:33:57 UTC
Permalink
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
What exactly breaks with user_claim? I think the general issue is that
The same thing that happens if userspace is in charge and doesn't
cooperate or if rfkill-input is not loaded: the rfkill driver state is
never updated (remember, it is about the radio controller, not the input
device).
And it should not - setting user_claim means userspace controls the
transmitter state. If userspace decides to ignore KEY_* event and not
Nuh-uh. Here's the root of the bug. The rfkill controller knowing
whether its state is on or off for real has NOTHING to do with the user
telling it to change state.

Something changed the controller's state (the hardware, or the
firmware), and didn't do it using the rfkill class. That's a fact. The
rfkill class needs to know of that change through an rfkill->state
update, or it misbehaves (because some stuff takes decisions depending
on the contents of rfkill->state, AND that state is exposed to userspace
which is allowed to also take decisions based on it). That's another
fact.

And none of it has to do with input events, or what the user wants done.
During this entire process, the kernel did not take ANY active instances
on what should happen to the radio controller. It didn't change the
radio controller state. It is just making sure the kernel doesn't think
it is ON when it is OFF, for a particular, specific, radio rfkill
controller.

I propose that we broadcast (through normal kernel notifier chains) that
the state has been changed, and if something is trying to enforce that
no radio rfkill controllers deviate from the global rfkill state for
that radio rfkill controller state, IT should tell the radios to go back
to the state it wants. But IT is not the rfkill class, nor any input
devices or rfkill devices. Maybe it is userspace, or rfkill-input, or
another platform-specific policy module.
Post by Dmitry Torokhov
turn the radio on or off - that is fine. rfkill-input simply provides
default kernel policy for reacting to KEY_WLAN, KEY_BLUETOOTH, etc.
So far so good, but it must not be used for something else entirely
(namely: keeping rfkill->state consistent to the real hardware state).
That's what I am talking about...
Post by Dmitry Torokhov
If there are drivers sense button presses and alter their actual
transmitter state immediately but then rely on rfkill-input to update
their sysfs rfkill attributes then such drivers should be fixed.
That's the point, yes. Currently *all* drivers have to do this :) and
that's why I did not add thinkpad-acpi rfkill support until I had enough
time to tack on rfkill itself to bring it up to speed with the extra
requirements stuff like thinkpad-acpi and b43 have.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
rfkill was originally not supposed to be used with switches that can't
be programmatically controlled. If we have a switch that mechanically
The input layer does not provide a way for userspace to get access to
the current state of any switches easily (or at all?). It just exports
events reporting these states sometimes.
You can use EVIOCGSW to get state of switches. I'd expect applications
to use it when they open devices first time and then rely on event
delivery to monitor changes in state of input devices.
Good to know. But the usual consumers of those are shell scripts, do we
have a swiss-army-knife-for-the-input-layer utility a shell script can
use to do a EVIOCGSW call? Or to wait for an event?

If we don't, but we provide one to be the companion of lsinput,
input-events and input-kbd, then I agree we can forego the sysfs
attribute.

But DO be warned that userspace developers will see that rfkill exports
a state attribute for the radio rfkill controllers, and will look for
something equally easy to use for the input devices. There is a big
pontential for confusion and misuse of interfaces there, when you recall
that it will be COMMON to have the same module (driver) provide both a
rfkill controller through the rfkill class, and an input device.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
We could fix it in the input subsystem, instead. How difficult (and
welcome a change?) would it be for the input device infrastructure to
provide pollable (for good measure!) "state" attributes for every EV_SW
event an input device registers in evbits+swbits ?
Given the above I don't see the need.
So, we can get the required information using an IOCTL. That means
rfkill class support for read-only devices is no longar mandatory. It
is still not clear to me it is not *desired*, though.

The reasons are:
1. Need to have an easy way to get that information for shell scripts,
and IOCTLs are not (AFAIK). This can be fixed by adding a shell-
script helper to input-utils.

2. It might be better to have all information directly related to
rfkill behaviour (i.e. both rfkill controller state and rfkill
input device state) in the same class, and present it in the same
way to userspace.

Anyone has an opinion about the above? Provided we can do it without
major complexity (we can, AFAIK, since I did get quite close to it with
the first patchset and the code in there is definately NOT complex), I
think both points could be addressed at the same time, and userspace
can get the information it wants in whichever way it is easier for the
application.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
If the input subsystem exports that information, we can define that
slave read-only radio controllers must NOT be registered as rfkill
devices, and drop support for read-only rfkill devices, as all master
rfkill devices (which are input devices by definition) would get their
state exported to userspace through the input layer.
However, that DOES mean userspace would need to look for rfkill state in
two places. Input layer, to get the current state of the rfkill input
devices, and rfkill class to get the current state of the rfkill
radio controllers.
I think you outlined the problem in your other mail quite well. We
keep confusing button state with the transmitter state. Only for
buttons that are hardwired to transmitters the states are the same,
for programmatically controlled RF transmitters state may be
different. So if you need to see state of transmitters you should look
in /sys/class/rfkill and for input devices - hook into input devices.
I am not so sure about that last sentence. See above. And I really
need to have a very clear and fixed picture of which way to go about
this before I start respinning the rfkill patches :)
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Let's decide this NOW, then. But if we decide to not do it in rfkill,
you will have to add attributes to every input device which has
registered itself as a source of EV_SW events.
No, since they may not be tied directly to any RF transmitter.
It is valuable information for userspace. I export the same information
available through SW_TABLET_MODE for example, as a thinkpad-acpi
specific attribute. Doing EVIOCGSW is not a very friendly interface for
userspace shell scripts right now.

So it *would* be useful. Whether it is the right way to go about it
(instead of adding a new utility to input-utils to do the EVIOCGSW for
shell scripts and other script languages that are IOCTL-challenged) is
something else.

If you really don't want that exported over sysfs as well, and EVIOCGSW
is the only way to get that information, I feel the immediate need for
an input-ioctl utility for userspace shell scripting.

And there is the all important question about whether I should still
export that information in thinkpad-acpi over pollable attributes (which
are *really* easy to use) or not. Other driver writers might want to
think about it as well.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
I better make it clear that I have NO idea how to properly implement the
required changes in the input layer if we go that way so if it is left
for me to do that work, it won't be ready anytime soon and it might be
supbar, so I'd appreciate lots of help in that case.
Well, let's see.. Do we have any issues with transmitters that can be
controlled programmatically? Or is it the hardwired ones that give us
trouble at the moment? I think that exporting all of them in rfkill is
Both, I think. Any rfkill controller which is not in complete,
exclusive control by the kernel, is currently a problem. That means
stuff the firmware can change, and it also means any stuff directly tied
to a button or switch that the user can change.

The only things we are handling well right now is pure keys/buttons, and
rfkill radio controllers that are in full control by their kernel driver
(i.e. those with NO sideway control paths that are out of the control of
the rfkill device).
Post by Dmitry Torokhov
a good idea, even though originally I thought we would not need to do
that. Therefore we do need to allow hardwided drivers updating their
rfkill state directly. The only problem I see is that we need to monitor
this closely, so non-hardwired cases won't try to use this mechanism.
Well, I do propose that we export any, and *every* radio controller
state change through kernel notifier chains (and to userspace using
uevents). Those notifications should include the rfkill type (and type
class if we add that), new switch state, and any other relevant
information (e.g. that the switch is a master switch -- those usually
need input events generated, whether the code that produced the
notification will also handle issuing an input event for it or not
(duplicate suppresion), etc).

We can monitor all rfkill radio controllers from userspace or
rfkill-input through those chains.
--
"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
Dmitry Torokhov
2008-04-14 18:05:32 UTC
Permalink
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
What exactly breaks with user_claim? I think the general issue is that
The same thing that happens if userspace is in charge and doesn't
cooperate or if rfkill-input is not loaded: the rfkill driver state is
never updated (remember, it is about the radio controller, not the input
device).
And it should not - setting user_claim means userspace controls the
transmitter state. If userspace decides to ignore KEY_* event and not
Nuh-uh. Here's the root of the bug. The rfkill controller knowing
whether its state is on or off for real has NOTHING to do with the user
telling it to change state.
Nuh-uh. Nothing changed the transmitter sate yet. We just got a
request from user to do something.
Post by Henrique de Moraes Holschuh
Something changed the controller's state (the hardware, or the
firmware), and didn't do it using the rfkill class. That's a fact.
And this is a bug. How come hardware state changes without kernel not
beign aware? The only case is hard-wired switch. Here I agree that if
such switches are exported via rfkill to userspace the driver should
be able to set rfkill state to match its own. In all other cases, when
you can programmatically control RF state you shoudl defer the actual
switch until user[space] tells you to do so. I guess firmware could
work like a hard-wired switch, turnign radio off when battery goes
extremely low, so yes, I agree that allowing drivers to update their
state directly is required.
Post by Henrique de Moraes Holschuh
The
rfkill class needs to know of that change through an rfkill->state
update, or it misbehaves (because some stuff takes decisions depending
on the contents of rfkill->state, AND that state is exposed to userspace
which is allowed to also take decisions based on it). That's another
fact.
And none of it has to do with input events, or what the user wants done.
During this entire process, the kernel did not take ANY active instances
on what should happen to the radio controller. It didn't change the
radio controller state. It is just making sure the kernel doesn't think
it is ON when it is OFF, for a particular, specific, radio rfkill
controller.
I propose that we broadcast (through normal kernel notifier chains) that
the state has been changed, and if something is trying to enforce that
no radio rfkill controllers deviate from the global rfkill state for
that radio rfkill controller state, IT should tell the radios to go back
to the state it wants. But IT is not the rfkill class, nor any input
devices or rfkill devices. Maybe it is userspace, or rfkill-input, or
another platform-specific policy module.
I agree. Here you are talking about hardwired-like scenarios where
kernel did not get a chance to interfere with the RF toggle. Still, if
there was a button involved it needs to be broadcasted so that the
rest of RF transmitters can be adjusted according to the used
policy.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
turn the radio on or off - that is fine. rfkill-input simply provides
default kernel policy for reacting to KEY_WLAN, KEY_BLUETOOTH, etc.
So far so good, but it must not be used for something else entirely
(namely: keeping rfkill->state consistent to the real hardware state).
That's what I am talking about...
Here I agree.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
If there are drivers sense button presses and alter their actual
transmitter state immediately but then rely on rfkill-input to update
their sysfs rfkill attributes then such drivers should be fixed.
That's the point, yes. Currently *all* drivers have to do this :) and
that's why I did not add thinkpad-acpi rfkill support until I had enough
time to tack on rfkill itself to bring it up to speed with the extra
requirements stuff like thinkpad-acpi and b43 have.
No, I dont believe all drivers have to do this. Ivo will correct me if
Im wrong but I believe he worked with drivers that dont necessarily
have the switch hard wired.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
rfkill was originally not supposed to be used with switches that can't
be programmatically controlled. If we have a switch that mechanically
The input layer does not provide a way for userspace to get access to
the current state of any switches easily (or at all?). It just exports
events reporting these states sometimes.
You can use EVIOCGSW to get state of switches. I'd expect applications
to use it when they open devices first time and then rely on event
delivery to monitor changes in state of input devices.
Good to know. But the usual consumers of those are shell scripts, do we
have a swiss-army-knife-for-the-input-layer utility a shell script can
use to do a EVIOCGSW call? Or to wait for an event?
If we don't, but we provide one to be the companion of lsinput,
input-events and input-kbd, then I agree we can forego the sysfs
attribute.
But DO be warned that userspace developers will see that rfkill exports
a state attribute for the radio rfkill controllers, and will look for
something equally easy to use for the input devices.
Should I add attribute for every key on a keyboard just because some
userspace developers what to use shell scripts everywhere? key_a,
key_b, key_c and so on? No.
Post by Henrique de Moraes Holschuh
There is a big
pontential for confusion and misuse of interfaces there, when you recall
that it will be COMMON to have the same module (driver) provide both a
rfkill controller through the rfkill class, and an input device.
As soon as people will realize that button and RF transmitter are
different objects serving different purposes it all goes away. The
very same module also registers network device, why rfkill is so
different.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
We could fix it in the input subsystem, instead. How difficult (and
welcome a change?) would it be for the input device infrastructure to
provide pollable (for good measure!) "state" attributes for every EV_SW
event an input device registers in evbits+swbits ?
Given the above I don't see the need.
So, we can get the required information using an IOCTL. That means
rfkill class support for read-only devices is no longar mandatory. It
is still not clear to me it is not *desired*, though.
1. Need to have an easy way to get that information for shell scripts,
and IOCTLs are not (AFAIK). This can be fixed by adding a shell-
script helper to input-utils.
2. It might be better to have all information directly related to
rfkill behaviour (i.e. both rfkill controller state and rfkill
input device state) in the same class, and present it in the same
way to userspace.
No, we need to keep RF transmitetrs and buttons separate. I can assign
KEY_WLAN to one of the media keys on my laptop (that I dont use
otherwise). Does it make sense to "move" my keyboard to rfkill
vicinity? I dont think so. Its just a button.
Post by Henrique de Moraes Holschuh
Anyone has an opinion about the above? Provided we can do it without
major complexity (we can, AFAIK, since I did get quite close to it with
the first patchset and the code in there is definately NOT complex), I
think both points could be addressed at the same time, and userspace
can get the information it wants in whichever way it is easier for the
application.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
If the input subsystem exports that information, we can define that
slave read-only radio controllers must NOT be registered as rfkill
devices, and drop support for read-only rfkill devices, as all master
rfkill devices (which are input devices by definition) would get their
state exported to userspace through the input layer.
However, that DOES mean userspace would need to look for rfkill state in
two places. Input layer, to get the current state of the rfkill input
devices, and rfkill class to get the current state of the rfkill
radio controllers.
I think you outlined the problem in your other mail quite well. We
keep confusing button state with the transmitter state. Only for
buttons that are hardwired to transmitters the states are the same,
for programmatically controlled RF transmitters state may be
different. So if you need to see state of transmitters you should look
in /sys/class/rfkill and for input devices - hook into input devices.
I am not so sure about that last sentence. See above. And I really
need to have a very clear and fixed picture of which way to go about
this before I start respinning the rfkill patches :)
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Let's decide this NOW, then. But if we decide to not do it in rfkill,
you will have to add attributes to every input device which has
registered itself as a source of EV_SW events.
No, since they may not be tied directly to any RF transmitter.
It is valuable information for userspace. I export the same information
available through SW_TABLET_MODE for example, as a thinkpad-acpi
specific attribute. Doing EVIOCGSW is not a very friendly interface for
userspace shell scripts right now.
So it *would* be useful. Whether it is the right way to go about it
(instead of adding a new utility to input-utils to do the EVIOCGSW for
shell scripts and other script languages that are IOCTL-challenged) is
something else.
If you really don't want that exported over sysfs as well, and EVIOCGSW
is the only way to get that information, I feel the immediate need for
an input-ioctl utility for userspace shell scripting.
And there is the all important question about whether I should still
export that information in thinkpad-acpi over pollable attributes (which
are *really* easy to use) or not. Other driver writers might want to
think about it as well.
If we export switches then we need to export leds. And keys. And
axis... It all adds up very quickly and wastes kernel memory. Just
write ioctl utility for script-users if such is needed and be done
with it.
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
I better make it clear that I have NO idea how to properly implement the
required changes in the input layer if we go that way so if it is left
for me to do that work, it won't be ready anytime soon and it might be
supbar, so I'd appreciate lots of help in that case.
Well, let's see.. Do we have any issues with transmitters that can be
controlled programmatically? Or is it the hardwired ones that give us
trouble at the moment? I think that exporting all of them in rfkill is
Both, I think. Any rfkill controller which is not in complete,
exclusive control by the kernel, is currently a problem. That means
stuff the firmware can change, and it also means any stuff directly tied
to a button or switch that the user can change.
The only things we are handling well right now is pure keys/buttons, and
rfkill radio controllers that are in full control by their kernel driver
(i.e. those with NO sideway control paths that are out of the control of
the rfkill device).
Post by Dmitry Torokhov
a good idea, even though originally I thought we would not need to do
that. Therefore we do need to allow hardwided drivers updating their
rfkill state directly. The only problem I see is that we need to monitor
this closely, so non-hardwired cases won't try to use this mechanism.
Well, I do propose that we export any, and *every* radio controller
state change through kernel notifier chains (and to userspace using
uevents). Those notifications should include the rfkill type (and type
class if we add that), new switch state, and any other relevant
information (e.g. that the switch is a master switch -- those usually
need input events generated, whether the code that produced the
notification will also handle issuing an input event for it or not
(duplicate suppresion), etc).
Notfiers are a good idea for RF transmitter state changes. Not so for
key/button notifiers because these can be ussued by regular PS/2 or
USB keyboards and other devices that have no idea about rfkill and
dont want to know.
Post by Henrique de Moraes Holschuh
We can monitor all rfkill radio controllers from userspace or
rfkill-input through those chains.
--
Dmitry
Henrique de Moraes Holschuh
2008-04-14 21:41:28 UTC
Permalink
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
What exactly breaks with user_claim? I think the general issue is that
The same thing that happens if userspace is in charge and doesn't
cooperate or if rfkill-input is not loaded: the rfkill driver state is
never updated (remember, it is about the radio controller, not the input
device).
And it should not - setting user_claim means userspace controls the
transmitter state. If userspace decides to ignore KEY_* event and not
Nuh-uh. Here's the root of the bug. The rfkill controller knowing
whether its state is on or off for real has NOTHING to do with the user
telling it to change state.
Nuh-uh. Nothing changed the transmitter sate yet. We just got a
request from user to do something.
We are talking about different things. Please re-read the last three or
four messages before this one, thread-wise.
Post by Dmitry Torokhov
switch until user[space] tells you to do so. I guess firmware could
work like a hard-wired switch, turnign radio off when battery goes
extremely low, so yes, I agree that allowing drivers to update their
state directly is required.
Ok, then we DO agree. Let's drop this point and go to the next one...
Post by Dmitry Torokhov
I agree. Here you are talking about hardwired-like scenarios where
kernel did not get a chance to interfere with the RF toggle. Still, if
Actually, I am talking about ANY scenario where the kernel did not get a
chance to interfere with the RF toggle.
Post by Dmitry Torokhov
there was a button involved it needs to be broadcasted so that the
rest of RF transmitters can be adjusted according to the used
policy.
We must only broadcast master events, these switches are often tied to
each other in a tree, you can't report events on the leaves, just the
root. In a ThinkPad T60, you get an rfkill input device (that can be
read), which is hardwired into the WLAN card's rfkill input pin (that
can ALSO be read, and could potentially be registered as a rfkill
controller). The WLAN card has also a second rfkill controller, which
is in a config register.

I kept that in mind, that's why I started talking about master and slave
rfkill switches. Only the master ones can also be used as rfkill input
devices.

So yes, if there is a button involved, it must issue one (and only one)
input event. We agree on this.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
So far so good, but it must not be used for something else entirely
(namely: keeping rfkill->state consistent to the real hardware state).
That's what I am talking about...
Here I agree.
As far as I can see, we actually agree in most issues related to rfkill,
maybe even in everything... but our terminology is NOT in sync, so it is
not immediately obvious that we do.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
If there are drivers sense button presses and alter their actual
transmitter state immediately but then rely on rfkill-input to update
their sysfs rfkill attributes then such drivers should be fixed.
That's the point, yes. Currently *all* drivers have to do this :) and
that's why I did not add thinkpad-acpi rfkill support until I had enough
time to tack on rfkill itself to bring it up to speed with the extra
requirements stuff like thinkpad-acpi and b43 have.
No, I dont believe all drivers have to do this. Ivo will correct me if
Im wrong but I believe he worked with drivers that dont necessarily
have the switch hard wired.
WHEN the kernel is the ONLY possible source of a radio controller state
change, it is not needed. For all other cases, we are either ignoring
the status changes (i.e. the kernel's idea of the current controller
state can be wrong), or using the input layer to resync (b43 tries to do
this).
Post by Dmitry Torokhov
Should I add attribute for every key on a keyboard just because some
userspace developers what to use shell scripts everywhere? key_a,
key_b, key_c and so on? No.
It is a rare application that needs to know if a key is currently
pressed or not. And usually, it is assumed by the user that he should
not expect an application to know a key was depressed before it started,
nor are such applications usually written using shell script :-)

For switches, that's not rare at all.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
There is a big
pontential for confusion and misuse of interfaces there, when you recall
that it will be COMMON to have the same module (driver) provide both a
rfkill controller through the rfkill class, and an input device.
As soon as people will realize that button and RF transmitter are
different objects serving different purposes it all goes away. The
very same module also registers network device, why rfkill is so
different.
It is not so different. But you could make the handling of that
information more generic if you could get it using just the rfkill
interface.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
We could fix it in the input subsystem, instead. How difficult (and
welcome a change?) would it be for the input device infrastructure to
provide pollable (for good measure!) "state" attributes for every EV_SW
event an input device registers in evbits+swbits ?
Given the above I don't see the need.
So, we can get the required information using an IOCTL. That means
rfkill class support for read-only devices is no longar mandatory. It
is still not clear to me it is not *desired*, though.
1. Need to have an easy way to get that information for shell scripts,
and IOCTLs are not (AFAIK). This can be fixed by adding a shell-
script helper to input-utils.
2. It might be better to have all information directly related to
rfkill behaviour (i.e. both rfkill controller state and rfkill
input device state) in the same class, and present it in the same
way to userspace.
No, we need to keep RF transmitetrs and buttons separate. I can assign
KEY_WLAN to one of the media keys on my laptop (that I dont use
otherwise). Does it make sense to "move" my keyboard to rfkill
vicinity? I dont think so. Its just a button.
And it doesn't have much of an existence outside of when it generates an
input event. You don't ask a button where it is ON or OFF, that's a
switch property.

But that's not true for a real switch. You can ask it whether it is in
the ON or OFF state (for two-state switches, they can have a lot more
than just two states, but that's not relevant for rfkill).

I can see why we'd not want to export that through rfkill now that you
brought external keyboards into the picture. If I have an external
keyboard with a radio kill SWITCH (not key), I want it to work just like
if it were built-in the platform.

Which means it needs to be done in the input layer. Well, that gives me
my answer, in a rather definitive way too.
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
And there is the all important question about whether I should still
export that information in thinkpad-acpi over pollable attributes (which
are *really* easy to use) or not. Other driver writers might want to
think about it as well.
If we export switches then we need to export leds. And keys. And
axis... It all adds up very quickly and wastes kernel memory. Just
write ioctl utility for script-users if such is needed and be done
with it.
Actually, no, we don't.

AFAIK, input layer LEDs should go away, they belong in the LED class.

Keys have usage patterns that make it not necessary to export the
information of whether they are currently pressed or not through sysfs.
Utilities to query keyboard state are around for a long time as well.

Stuff that needs an axis position usually needs to track the axis (i.e.
they need a stream of data), so we are doing everyone a big favour if we
never ever let big bad stupid mistakes like that hdaps "position"
attribute's existence happen again.

As for writing the ioctl utility, it looks that it will be needed
without the sysfs attributes. But who will write it? Who will give it
a home? Maybe util-linux would take it in, if we're very lucky...
Post by Dmitry Torokhov
Post by Henrique de Moraes Holschuh
Well, I do propose that we export any, and *every* radio controller
state change through kernel notifier chains (and to userspace using
uevents). Those notifications should include the rfkill type (and type
class if we add that), new switch state, and any other relevant
information (e.g. that the switch is a master switch -- those usually
need input events generated, whether the code that produced the
notification will also handle issuing an input event for it or not
(duplicate suppresion), etc).
Notfiers are a good idea for RF transmitter state changes. Not so for
key/button notifiers because these can be ussued by regular PS/2 or
USB keyboards and other devices that have no idea about rfkill and
dont want to know.
Agreed.
--
"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
Carlos Corbacho
2008-04-14 19:06:50 UTC
Permalink
Post by Ivo van Doorn
Post by Henrique de Moraes Holschuh
See rt2x00 and b43 in driver-wireless for an example implementation
of pollable input device and rfkill
They're broken by design, at least in b43's case. Someone (Carlos, I
believe?) posted an example of a ping-pong scenario using b43.
Thats true. But was that a scenario where there were 2 keys for the same
radio type? Or was that a matter of the driver indicating a key pressed
event while it has no attached key to it, and only checked the register for
the key status and the current radio status and saw it was different?
Two part problem:

1) Bug in rfkill; rfkill is toggling all devices in the same state, and not
actually checking the type.

(I submitted a patch for this on Sunday, although I sent it to linux-wireless
before you sent the MAINTAINERS patch with netdev as the official list; but I
did CC you on it regardless so you should have seen this. If not, let me know
and I'll resend it).

2) The cycle-of-doom bug, as described earlier.

This still applies.

Yes, b43 can also be fixed not to emit a keycode if it cannot control the
rfkill switch; but that doesn't solve the case of what happens if both the
wireless driver and the platform driver can control the radio.

I agree wholeheartedly with Henrique here though - wireless drivers should
_not_ be using key press events as a reporting mechanism.

-Carlos
--
E-Mail: ***@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D
Dmitry Torokhov
2008-04-14 20:23:14 UTC
Permalink
Post by Carlos Corbacho
I agree wholeheartedly with Henrique here though - wireless drivers should
_not_ be using key press events as a reporting mechanism.
Reporting mechanism for what exacly though? Depending on what you are
talking about we either agree or disagree here ;)
--
Dmitry
Carlos Corbacho
2008-04-15 07:27:49 UTC
Permalink
Post by Dmitry Torokhov
Post by Carlos Corbacho
I agree wholeheartedly with Henrique here though - wireless drivers
should _not_ be using key press events as a reporting mechanism.
Reporting mechanism for what exacly though? Depending on what you are
talking about we either agree or disagree here ;)
Reporting state change in this case. For instance, KEY_WLAN means exactly
that - a user pressed a key that was mapped to KEY_WLAN. It's not a generic
notifier for any and all wireless events, state changes, etc.

If wireless drivers want to report a state change, then we need a proper
mechanism for that, as has already been discussed by Henrique.

-Carlos
--
E-Mail: ***@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D
Dmitry Torokhov
2008-04-15 12:58:49 UTC
Permalink
Post by Carlos Corbacho
Post by Dmitry Torokhov
Post by Carlos Corbacho
I agree wholeheartedly with Henrique here though - wireless drivers
should _not_ be using key press events as a reporting mechanism.
Reporting mechanism for what exacly though? Depending on what you are
talking about we either agree or disagree here ;)
Reporting state change in this case. For instance, KEY_WLAN means exactly
that - a user pressed a key that was mapped to KEY_WLAN. It's not a generic
notifier for any and all wireless events, state changes, etc.
If wireless drivers want to report a state change, then we need a proper
mechanism for that, as has already been discussed by Henrique.
I think everyone is in a violent agreement here but prefers to keep
talking to appear that they not agree ;)
--
Dmitry
Ivo van Doorn
2008-04-14 21:04:42 UTC
Permalink
Post by Carlos Corbacho
Post by Ivo van Doorn
Post by Henrique de Moraes Holschuh
See rt2x00 and b43 in driver-wireless for an example implementation
of pollable input device and rfkill
They're broken by design, at least in b43's case. Someone (Carlos, I
believe?) posted an example of a ping-pong scenario using b43.
Thats true. But was that a scenario where there were 2 keys for the same
radio type? Or was that a matter of the driver indicating a key pressed
event while it has no attached key to it, and only checked the register for
the key status and the current radio status and saw it was different?
1) Bug in rfkill; rfkill is toggling all devices in the same state, and not
actually checking the type.
(I submitted a patch for this on Sunday, although I sent it to linux-wireless
before you sent the MAINTAINERS patch with netdev as the official list; but I
did CC you on it regardless so you should have seen this. If not, let me know
and I'll resend it).
Completely overlooked the patch.
I found it in my archives and just acked it. :)

Ivo
Post by Carlos Corbacho
2) The cycle-of-doom bug, as described earlier.
This still applies.
Yes, b43 can also be fixed not to emit a keycode if it cannot control the
rfkill switch; but that doesn't solve the case of what happens if both the
wireless driver and the platform driver can control the radio.
I agree wholeheartedly with Henrique here though - wireless drivers should
_not_ be using key press events as a reporting mechanism.
-Carlos
Henrique de Moraes Holschuh
2008-04-14 21:46:53 UTC
Permalink
Post by Ivo van Doorn
Completely overlooked the patch.
I found it in my archives and just acked it. :)
Unless you want extra trouble and work on your hands when merge time
comes, I would really appreciate a tree with all pending rfkill patches
applied to base my work on. Right now, I am using 2.6.25-rc8-mm2.

May I suggest you open a git tree somewhere that we can use to track the
rfkill pending patches that are not in mainline yet? :-)

If you don't have a git server in mind already, I suggest http://repo.or.cz
--
"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
Ivo Van Doorn
2008-04-15 08:14:22 UTC
Permalink
On Mon, Apr 14, 2008 at 11:46 PM, Henrique de Moraes Holschuh
Post by Henrique de Moraes Holschuh
Post by Ivo van Doorn
Completely overlooked the patch.
I found it in my archives and just acked it. :)
Unless you want extra trouble and work on your hands when merge time
comes, I would really appreciate a tree with all pending rfkill patches
applied to base my work on. Right now, I am using 2.6.25-rc8-mm2.
I've asked john to push the patch to 2.6.25 as well.
Post by Henrique de Moraes Holschuh
May I suggest you open a git tree somewhere that we can use to track the
rfkill pending patches that are not in mainline yet? :-)
Sure, not a problem. Let me see from which tree it will be best to branch from.
Post by Henrique de Moraes Holschuh
If you don't have a git server in mind already, I suggest http://repo.or.cz
I've an account on kernel.org. :)

Ivo
Henrique de Moraes Holschuh
2008-04-11 20:37:19 UTC
Permalink
The *_RADIO input events are related to all radios in a system. There are
two: KEY_RADIO and SW_RADIO.

Teach rfkill-input how to handle them. In particular, SW_RADIO is not a
toggle, but an absolute enable-or-disable command.

Signed-off-by: Henrique de Moraes Holschuh <***@hmh.eng.br>
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: Dmitry Torokhov <***@mail.ru>
---
net/rfkill/rfkill-input.c | 57 +++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index e4b051d..7d1e520 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -55,6 +55,22 @@ static void rfkill_task_handler(struct work_struct *work)
mutex_unlock(&task->mutex);
}

+static void rfkill_schedule_set(struct rfkill_task *task,
+ enum rfkill_state desired_state)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&task->lock, flags);
+
+ if (time_after(jiffies, task->last + msecs_to_jiffies(200))) {
+ task->desired_state = desired_state;
+ task->last = jiffies;
+ schedule_work(&task->work);
+ }
+
+ spin_unlock_irqrestore(&task->lock, flags);
+}
+
static void rfkill_schedule_toggle(struct rfkill_task *task)
{
unsigned long flags;
@@ -87,9 +103,9 @@ static DEFINE_RFKILL_TASK(rfkill_uwb, RFKILL_TYPE_UWB);
static DEFINE_RFKILL_TASK(rfkill_wimax, RFKILL_TYPE_WIMAX);

static void rfkill_event(struct input_handle *handle, unsigned int type,
- unsigned int code, int down)
+ unsigned int code, int data)
{
- if (type == EV_KEY && down == 1) {
+ if (type == EV_KEY && data == 1) {
switch (code) {
case KEY_WLAN:
rfkill_schedule_toggle(&rfkill_wlan);
@@ -103,6 +119,33 @@ static void rfkill_event(struct input_handle *handle, unsigned int type,
case KEY_WIMAX:
rfkill_schedule_toggle(&rfkill_wimax);
break;
+ case KEY_RADIO:
+ /* EVERY radio type */
+ rfkill_schedule_toggle(&rfkill_wimax);
+ rfkill_schedule_toggle(&rfkill_uwb);
+ rfkill_schedule_toggle(&rfkill_bt);
+ rfkill_schedule_toggle(&rfkill_wlan);
+ break;
+ default:
+ break;
+ }
+ } else if (type == EV_SW) {
+ switch (code) {
+ case SW_RADIO:
+ /* EVERY radio type. data != 0 means radios ON */
+ rfkill_schedule_set(&rfkill_wimax,
+ (data)? RFKILL_STATE_ON:
+ RFKILL_STATE_OFF);
+ rfkill_schedule_set(&rfkill_uwb,
+ (data)? RFKILL_STATE_ON:
+ RFKILL_STATE_OFF);
+ rfkill_schedule_set(&rfkill_bt,
+ (data)? RFKILL_STATE_ON:
+ RFKILL_STATE_OFF);
+ rfkill_schedule_set(&rfkill_wlan,
+ (data)? RFKILL_STATE_ON:
+ RFKILL_STATE_OFF);
+ break;
default:
break;
}
@@ -168,6 +211,16 @@ static const struct input_device_id rfkill_ids[] = {
.evbit = { BIT_MASK(EV_KEY) },
.keybit = { [BIT_WORD(KEY_WIMAX)] = BIT_MASK(KEY_WIMAX) },
},
+ {
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_KEYBIT,
+ .evbit = { BIT(EV_KEY) },
+ .keybit = { [BIT_WORD(KEY_RADIO)] = BIT_MASK(KEY_RADIO) },
+ },
+ {
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_SWBIT,
+ .evbit = { BIT(EV_SW) },
+ .swbit = { [BIT_WORD(SW_RADIO)] = BIT_MASK(SW_RADIO) },
+ },
{ }
};
--
1.5.4.4
Henrique de Moraes Holschuh
2008-04-11 20:37:21 UTC
Permalink
Some devices (notably laptops) have read-only rfkill switches. Those
devices are slider or rocker switches that are *physically* manipulated by
the user, most of the time tied to hardware or firmware functions that
automatically rf-kill and/or hot-unplug radio devices when in the "radios
off" position.

Software must not (and in fact, cannot) attempt to change the state of any
such switch. These switches exist because of international regulations
regarding radio emission devices on airplanes and other sensitive areas.
They must never be overriden.

While one can easily report the *change* of state of these switches using
the input layer EV_SW SW_RADIO event, typically userspace needs to have
immediate access of the current switch state. It makes sense to have all
radio kill switches grouped under the rfkill sysfs interface instead of
every driver doing its own thing.

Make the toggle_radio() hook no longer be mandatory. read-only rfkill
switches are those who have their toggle_radio() hook set to NULL.

Signed-off-by: Henrique de Moraes Holschuh <***@hmh.eng.br>
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: Dmitry Torokhov <***@mail.ru>
---
include/linux/rfkill.h | 3 ++-
net/rfkill/rfkill.c | 18 +++++++++++++-----
2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index 844e961..931d32b 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -60,7 +60,8 @@ enum rfkill_state {
* @mutex: Guards switch state transitions
* @data: Pointer to the RF button drivers private data which will be
* passed along when toggling radio state.
- * @toggle_radio(): Mandatory handler to control state of the radio.
+ * @toggle_radio(): handler to control state of the radio. Must be
+ * NULL for read-only switches.
* @get_state(): handler to read current radio state from hardware,
* may be called from atomic context, should return 0 on success.
* @led_trigger: A LED trigger for this button's LED.
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 88b0558..d149d94 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -81,7 +81,7 @@ static int rfkill_toggle_radio(struct rfkill *rfkill,
!rfkill->get_state(rfkill->data, &newstate))
rfkill->state = newstate;

- if (state != rfkill->state) {
+ if (rfkill->toggle_radio && state != rfkill->state) {
retval = rfkill->toggle_radio(rfkill->data, state);
if (!retval)
rfkill->state = state;
@@ -196,6 +196,9 @@ static ssize_t rfkill_state_store(struct device *dev,
unsigned int state = simple_strtoul(buf, NULL, 0);
int error;

+ if (!rfkill->toggle_radio)
+ return -EACCES;
+
if (!capable(CAP_NET_ADMIN))
return -EPERM;

@@ -278,7 +281,8 @@ static int rfkill_suspend(struct device *dev, pm_message_t state)
if (state.event & PM_EVENT_SLEEP) {
mutex_lock(&rfkill->mutex);

- if (rfkill->state == RFKILL_STATE_ON)
+ if (rfkill->state == RFKILL_STATE_ON &&
+ rfkill->toggle_radio)
rfkill->toggle_radio(rfkill->data,
RFKILL_STATE_OFF);

@@ -298,7 +302,8 @@ static int rfkill_resume(struct device *dev)
if (dev->power.power_state.event != PM_EVENT_ON) {
mutex_lock(&rfkill->mutex);

- if (rfkill->state == RFKILL_STATE_ON)
+ if (rfkill->state == RFKILL_STATE_ON &&
+ rfkill->toggle_radio)
rfkill->toggle_radio(rfkill->data, RFKILL_STATE_ON);

mutex_unlock(&rfkill->mutex);
@@ -427,11 +432,14 @@ int 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->toggle_radio) {
+ rfkill->user_claim_unsupported = 1;
+ rfkill->user_claim = 0;
+ }
+
snprintf(dev->bus_id, sizeof(dev->bus_id),
"rfkill%ld", (long)atomic_inc_return(&rfkill_no) - 1);
--
1.5.4.4
Henrique de Moraes Holschuh
2008-04-11 20:37:23 UTC
Permalink
Add a RFKILL_TYPE_ANY switch. This switch can control more than one type
of radio, and it is always subject to toggling by any type of rfkill-input
event. It is suitable to implement kill-all-radios functionality when
coupled with input event EV_SW SW_RADIO.

Signed-off-by: Henrique de Moraes Holschuh <***@hmh.eng.br>
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: Dmitry Torokhov <***@mail.ru>
---
include/linux/rfkill.h | 2 ++
net/rfkill/rfkill-input.c | 9 +++++++++
net/rfkill/rfkill.c | 3 +++
3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index 7650517..ba8a7e1 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -35,6 +35,7 @@
* RFKILL_TYPE_UWB: switch is on a ultra wideband device.
* RFKILL_TYPE_WIMAX: switch is on a WiMAX device.
* RFKILL_TYPE_WWAN: switch is on a wireless WAN device.
+ * RFKILL_TYPE_ANY: switch kills radios regardless of type.
*/
enum rfkill_type {
RFKILL_TYPE_WLAN ,
@@ -42,6 +43,7 @@ enum rfkill_type {
RFKILL_TYPE_UWB,
RFKILL_TYPE_WIMAX,
RFKILL_TYPE_WWAN,
+ RFKILL_TYPE_ANY,
RFKILL_TYPE_MAX,
};

diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index 675651b..ec9112c 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -102,6 +102,7 @@ static DEFINE_RFKILL_TASK(rfkill_bt, RFKILL_TYPE_BLUETOOTH);
static DEFINE_RFKILL_TASK(rfkill_uwb, RFKILL_TYPE_UWB);
static DEFINE_RFKILL_TASK(rfkill_wimax, RFKILL_TYPE_WIMAX);
static DEFINE_RFKILL_TASK(rfkill_wwan, RFKILL_TYPE_WIMAX);
+static DEFINE_RFKILL_TASK(rfkill_any, RFKILL_TYPE_ANY);

static void rfkill_event(struct input_handle *handle, unsigned int type,
unsigned int code, int data)
@@ -110,18 +111,23 @@ static void rfkill_event(struct input_handle *handle, unsigned int type,
switch (code) {
case KEY_WLAN:
rfkill_schedule_toggle(&rfkill_wlan);
+ rfkill_schedule_toggle(&rfkill_any);
break;
case KEY_BLUETOOTH:
rfkill_schedule_toggle(&rfkill_bt);
+ rfkill_schedule_toggle(&rfkill_any);
break;
case KEY_UWB:
rfkill_schedule_toggle(&rfkill_uwb);
+ rfkill_schedule_toggle(&rfkill_any);
break;
case KEY_WIMAX:
rfkill_schedule_toggle(&rfkill_wimax);
+ rfkill_schedule_toggle(&rfkill_any);
break;
case KEY_RADIO:
/* EVERY radio type */
+ rfkill_schedule_toggle(&rfkill_any);
rfkill_schedule_toggle(&rfkill_wwan);
rfkill_schedule_toggle(&rfkill_wimax);
rfkill_schedule_toggle(&rfkill_uwb);
@@ -135,6 +141,9 @@ static void rfkill_event(struct input_handle *handle, unsigned int type,
switch (code) {
case SW_RADIO:
/* EVERY radio type. data != 0 means radios ON */
+ rfkill_schedule_set(&rfkill_any,
+ (data)? RFKILL_STATE_ON:
+ RFKILL_STATE_OFF);
rfkill_schedule_set(&rfkill_wwan,
(data)? RFKILL_STATE_ON:
RFKILL_STATE_OFF);
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 56241a4..9d3bffb 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -174,6 +174,9 @@ static ssize_t rfkill_type_show(struct device *dev,
case RFKILL_TYPE_WWAN:
type = "wwan";
break;
+ case RFKILL_TYPE_ANY:
+ type = "any";
+ break;
default:
BUG();
}
--
1.5.4.4
Henrique de Moraes Holschuh
2008-04-11 20:37:24 UTC
Permalink
Currently, radios are always enabled when their rfkill interface is
registered. This is not optimal, the safest state for a radio is to be
offline unless the user turns it on.

Add a module parameter that causes all radios to be disabled when their
rfkill interface is registered. Add override parameters for each rfkill
switch type as well, just in case the user wants the defaults to be
different for a given radio type.

We don't change the module default, but I'd really recommed doing so in a
future patch.

The parameters are called "default_state" (global), and
"default_<type>_state", where <type> is: "wlan", "bluetooth", "uwb",
"wimax", or "any". Note that "any" realy is a type and does not mean every
radio type (use the global "default_state" parameter for that.

The precedence order is "most specific first".

Signed-off-by: Henrique de Moraes Holschuh <***@hmh.eng.br>
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: Dmitry Torokhov <***@mail.ru>
---
net/rfkill/rfkill.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 9d3bffb..421de8c 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -39,6 +39,34 @@ MODULE_LICENSE("GPL");
static LIST_HEAD(rfkill_list); /* list of registered rf switches */
static DEFINE_MUTEX(rfkill_mutex);

+static unsigned int rfkill_default_state = RFKILL_STATE_ON;
+static unsigned int rfkill_default_state_wlan = UINT_MAX;
+static unsigned int rfkill_default_state_bluetooth = UINT_MAX;
+static unsigned int rfkill_default_state_uwb = UINT_MAX;
+static unsigned int rfkill_default_state_wimax = UINT_MAX;
+static unsigned int rfkill_default_state_wwan = UINT_MAX;
+static unsigned int rfkill_default_state_any = UINT_MAX;
+
+module_param_named(default_state, rfkill_default_state, uint, 0444);
+MODULE_PARM_DESC(default_state,
+ "Default initial state for all radio types, 0 = radio off");
+
+#define RFKILL_RADIO_DEFAULT(__type) \
+ module_param_named(default_##__type##_state, \
+ rfkill_default_state_##__type, uint, 0444); \
+ MODULE_PARM_DESC(default_##__type##_state, \
+ "Override initial state for radios of type " \
+ #__type ", 0 = radio off");
+
+RFKILL_RADIO_DEFAULT(wlan)
+RFKILL_RADIO_DEFAULT(bluetooth)
+RFKILL_RADIO_DEFAULT(uwb)
+RFKILL_RADIO_DEFAULT(wimax)
+RFKILL_RADIO_DEFAULT(wwan)
+RFKILL_RADIO_DEFAULT(any)
+
+#undef RFKILL_RADIO_DEFAULT
+
static enum rfkill_state rfkill_states[RFKILL_TYPE_MAX];


@@ -485,6 +513,17 @@ void rfkill_unregister(struct rfkill *rfkill)
}
EXPORT_SYMBOL(rfkill_unregister);

+static int __init rfkill_init_states(unsigned int state, enum rfkill_type type)
+{
+ if (state == UINT_MAX)
+ return 0;
+ else if (state == RFKILL_STATE_OFF || state == RFKILL_STATE_ON) {
+ rfkill_states[type] = state;
+ return 0;
+ }
+ return 1;
+}
+
/*
* Rfkill module initialization/deinitialization.
*/
@@ -493,8 +532,26 @@ static int __init rfkill_init(void)
int error;
int i;

+ if (rfkill_default_state != RFKILL_STATE_OFF &&
+ rfkill_default_state != RFKILL_STATE_ON)
+ return -EINVAL;
+
for (i = 0; i < ARRAY_SIZE(rfkill_states); i++)
- rfkill_states[i] = RFKILL_STATE_ON;
+ rfkill_states[i] = rfkill_default_state;
+
+ if (rfkill_init_states(rfkill_default_state_wlan,
+ RFKILL_TYPE_WLAN)
+ || rfkill_init_states(rfkill_default_state_bluetooth,
+ RFKILL_TYPE_BLUETOOTH)
+ || rfkill_init_states(rfkill_default_state_uwb,
+ RFKILL_TYPE_UWB)
+ || rfkill_init_states(rfkill_default_state_wimax,
+ RFKILL_TYPE_WIMAX)
+ || rfkill_init_states(rfkill_default_state_wwan,
+ RFKILL_TYPE_WWAN)
+ || rfkill_init_states(rfkill_default_state_any,
+ RFKILL_TYPE_ANY))
+ return -EINVAL;

error = class_register(&rfkill_class);
if (error) {
--
1.5.4.4
Henrique de Moraes Holschuh
2008-04-11 20:37:22 UTC
Permalink
Unfortunately, instead of adding a generic Wireless WAN type, a technol=
ogy-
specific type (WiMAX) was added. That's useless for other WWAN devices=
,
such as EDGE, UMTS, X-RTT and other such radios.

Add a WWAN rfkill type for generic wireless WAN devices. No keys are a=
dded
as most devices use KEY_RADIO for WWAN control and need no specific key=
code
added.

Signed-off-by: Inaky Perez-Gonzalez <***@linux.intel.com>
Cc: I=C3=B1aky P=C3=A9rez-Gonz=C3=A1lez <inaky.perez-***@intel.com=
Cc: Ivo van Doorn <***@gmail.com>
Cc: John W. Linville <***@tuxdriver.com>
Cc: David S. Miller <***@davemloft.net>
---
include/linux/rfkill.h | 2 ++
net/rfkill/rfkill-input.c | 5 +++++
net/rfkill/rfkill.c | 3 +++
3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index 931d32b..7650517 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -34,12 +34,14 @@
* RFKILL_TYPE_BLUETOOTH: switch is on a bluetooth device.
* RFKILL_TYPE_UWB: switch is on a ultra wideband device.
* RFKILL_TYPE_WIMAX: switch is on a WiMAX device.
+ * RFKILL_TYPE_WWAN: switch is on a wireless WAN device.
*/
enum rfkill_type {
RFKILL_TYPE_WLAN ,
RFKILL_TYPE_BLUETOOTH,
RFKILL_TYPE_UWB,
RFKILL_TYPE_WIMAX,
+ RFKILL_TYPE_WWAN,
RFKILL_TYPE_MAX,
};
=20
diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index 7d1e520..675651b 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -101,6 +101,7 @@ static DEFINE_RFKILL_TASK(rfkill_wlan, RFKILL_TYPE_=
WLAN);
static DEFINE_RFKILL_TASK(rfkill_bt, RFKILL_TYPE_BLUETOOTH);
static DEFINE_RFKILL_TASK(rfkill_uwb, RFKILL_TYPE_UWB);
static DEFINE_RFKILL_TASK(rfkill_wimax, RFKILL_TYPE_WIMAX);
+static DEFINE_RFKILL_TASK(rfkill_wwan, RFKILL_TYPE_WIMAX);
=20
static void rfkill_event(struct input_handle *handle, unsigned int typ=
e,
unsigned int code, int data)
@@ -121,6 +122,7 @@ static void rfkill_event(struct input_handle *handl=
e, unsigned int type,
break;
case KEY_RADIO:
/* EVERY radio type */
+ rfkill_schedule_toggle(&rfkill_wwan);
rfkill_schedule_toggle(&rfkill_wimax);
rfkill_schedule_toggle(&rfkill_uwb);
rfkill_schedule_toggle(&rfkill_bt);
@@ -133,6 +135,9 @@ static void rfkill_event(struct input_handle *handl=
e, unsigned int type,
switch (code) {
case SW_RADIO:
/* EVERY radio type. data !=3D 0 means radios ON */
+ rfkill_schedule_set(&rfkill_wwan,
+ (data)? RFKILL_STATE_ON:
+ RFKILL_STATE_OFF);
rfkill_schedule_set(&rfkill_wimax,
(data)? RFKILL_STATE_ON:
RFKILL_STATE_OFF);
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index d149d94..56241a4 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -171,6 +171,9 @@ static ssize_t rfkill_type_show(struct device *dev,
case RFKILL_TYPE_WIMAX:
type =3D "wimax";
break;
+ case RFKILL_TYPE_WWAN:
+ type =3D "wwan";
+ break;
default:
BUG();
}
--=20
1.5.4.4
Inaky Perez-Gonzalez
2008-04-11 20:44:08 UTC
Permalink
Unfortunately, instead of adding a generic Wireless WAN type, a technology-
specific type (WiMAX) was added. That's useless for other WWAN devices,
such as EDGE, UMTS, X-RTT and other such radios.
Add a WWAN rfkill type for generic wireless WAN devices. No keys are added
as most devices use KEY_RADIO for WWAN control and need no specific keycode
added.
I know it is easier to complain than to submit code, but at this
point, shouldn't we make this dynamic? [so that the interested technology
that provides an rfkill switch registers it?].

Something that given a technology name registers a dynamic key and type
number that can be use throughout?
--
Inaky
Henrique de Moraes Holschuh
2008-04-11 20:53:26 UTC
Permalink
Post by Inaky Perez-Gonzalez
Unfortunately, instead of adding a generic Wireless WAN type, a technology-
specific type (WiMAX) was added. That's useless for other WWAN devices,
such as EDGE, UMTS, X-RTT and other such radios.
Add a WWAN rfkill type for generic wireless WAN devices. No keys are added
as most devices use KEY_RADIO for WWAN control and need no specific keycode
added.
I know it is easier to complain than to submit code, but at this
point, shouldn't we make this dynamic? [so that the interested technology
that provides an rfkill switch registers it?].
I wouldn't have anything against that, but we do need to coalesce the
types when possible, otherwise the "type" notion becomes useless for
rfkill-input.
--
"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
Dmitry Torokhov
2008-04-14 04:20:27 UTC
Permalink
Most rfkill hw keys are attached to the device. So I can plug wifi, wimax,
3G, UWB and BT dongles (or cards) into a system and they each provide a HW
switch (or should provide).
If each overrides another one, we have a problem.
Yeah, and we have to avoid such problems.
But let me ask you one thing. Are we talking of different rfkill
switches, OR are we talking about different KEYs or BUTTONS in the
keyboard/keypad/console/remote control?
Because one thing has very little to do with the other. No network
device driver shall generate an input event after I am done with the
next patch set...
Not sure if that will be the desired course of action.
The original rfkill function didn't work with input devices either and just
handled everything in rfkill.
But the input layer people requested the change to use input devices and let
rfkill hook into that, because a rfkill switch/key can be considered an input device.
I've CC'ed Dmitry into this discussion since he was the one who suggested the
input device interaction initialy (and created the rfkill-input module).
In order to avoid a lot of misunderstandings, we better name the
"enable/disable radio apparatus" (the circuit/config register that
causes a radio to disable its RF path) and the "input hardware rf-switch
device" (the thing the user presses/moves) in different ways, at least
when we are talking about them.
Right now both are usually called rfkill switch, and much of the mess
comes from that...
"input hardware rf-switch devices" ARE input devices, belong to the
input layer, and issue input events.
Exactly. And some times they are completely separated from the RF switch
itself, in the same fashion some keyboards have sleep key that is not
directly wired into ACPI. That was the reason I wanted the radio control
and key/switch/sliter to be separated.
"enable/disable radio apparatus" don't, and the input layer should not
be abused as a "status reporting" feature for those. We certainly will
benefit from those issuing kernel notifications through a notification
chain for them, though.
And many devices are both at the same time (e.g. a b43 in certain
hardware configurations), and many drivers have both types (e.g.
thinkpad-acpi's hotkey subsystem has "input hardare rf-switch devices"
functions, and thinkpad-acpi's bluetooth firmware handling have
"enable/disable radio apparatus" functions).
--
Dmitry
Dmitry Torokhov
2008-04-14 04:22:34 UTC
Permalink
Post by Henrique de Moraes Holschuh
rfkill really should have been named rfswitch. As it is, one can get
confused whether RFKILL_STATE_ON means the KILL switch is on (and
therefore, the radio is being *blocked* from operating), or whether it
means the RADIO rf output is on.
Clearly state that RFKILL_STATE_ON means the radio is *unblocked* from
operating (i.e. there is no rf killing going on).
I consider Ivo maintaining rfkill but for what it worth:

Acked-by: Dmitry Torokhov <***@mail.ru>
--
Dmitry
John W. Linville
2008-04-16 18:37:05 UTC
Permalink
This patch series (based on v2.6.25-rc8-mm2) has several improvements to
the rfkill class that I need for thinkpad-acpi, plus two fluff and
documentation fixes.
Who is handling these rfkill patches? I think in the past they have
gone through me, but some may have gone directly to Dave M.

If they are coming to me, please collect Ivo's ACKs and resend them
to linux-***@vger.kernel.org with me CC'ed as well.

Thanks,

John
--
John W. Linville
***@tuxdriver.com
Ivo van Doorn
2008-04-16 19:26:43 UTC
Permalink
Post by John W. Linville
This patch series (based on v2.6.25-rc8-mm2) has several improvements to
the rfkill class that I need for thinkpad-acpi, plus two fluff and
documentation fixes.
Who is handling these rfkill patches? I think in the past they have
gone through me, but some may have gone directly to Dave M.
Yes initial rfkill introduction went through wireless, but subsequent patches
seemed to have gone directly into net-2.6.
I must admit I am not sure what the correct flow for rkfill should be. :S
Post by John W. Linville
If they are coming to me, please collect Ivo's ACKs and resend them
I am going to setup a rfkill git tree from where rfkill can be managed,
but I still had to look into the correct git tree to branch from.

We should sort out how the rfkill patch flow should go,
directly into net-2.6 or to wireless-2.6/wireless-testing
Although rfkill is a wireless tool, one could argue that linux-wireless
is mostly aimed at the 802.11 protocol, whereas rfkill applies to
irda and bluetooth as well.

Ivo
John W. Linville
2008-04-16 19:58:22 UTC
Permalink
Post by Ivo van Doorn
Post by John W. Linville
If they are coming to me, please collect Ivo's ACKs and resend them
I am going to setup a rfkill git tree from where rfkill can be managed,
but I still had to look into the correct git tree to branch from.
We should sort out how the rfkill patch flow should go,
directly into net-2.6 or to wireless-2.6/wireless-testing
Although rfkill is a wireless tool, one could argue that linux-wireless
is mostly aimed at the 802.11 protocol, whereas rfkill applies to
irda and bluetooth as well.
I suppose you could manage it as a separate tree. Before this round of
patches, that would have seemed like overkill. :-) My only argument
would be that there are probably more users of rfkill in the wireless
(i.e. wireless LAN) trees than in the Bluetooth, UWB, or WiMAX trees.
But honestly, it is up to you and Dave. I'm happy to take the patches
in my trees or you guys can arrange something else.

Just let me know if you want me to merge them. Otherwise, I'll drop
them from my queue.

Thanks,

John
--
John W. Linville
***@tuxdriver.com
Ivo van Doorn
2008-04-16 20:40:38 UTC
Permalink
Post by John W. Linville
Post by Ivo van Doorn
Post by John W. Linville
If they are coming to me, please collect Ivo's ACKs and resend them
I am going to setup a rfkill git tree from where rfkill can be managed,
but I still had to look into the correct git tree to branch from.
We should sort out how the rfkill patch flow should go,
directly into net-2.6 or to wireless-2.6/wireless-testing
Although rfkill is a wireless tool, one could argue that linux-wireless
is mostly aimed at the 802.11 protocol, whereas rfkill applies to
irda and bluetooth as well.
I suppose you could manage it as a separate tree. Before this round of
patches, that would have seemed like overkill. :-) My only argument
would be that there are probably more users of rfkill in the wireless
(i.e. wireless LAN) trees than in the Bluetooth, UWB, or WiMAX trees.
But honestly, it is up to you and Dave. I'm happy to take the patches
in my trees or you guys can arrange something else.
Dave, do you want the rfkill patches directly, or should they go through
the wireless tree first?
Post by John W. Linville
Just let me know if you want me to merge them. Otherwise, I'll drop
them from my queue.
Well the patches from Henrique shouldn't be applied yet,
since he is reworking several of those patches.

Ivo
David Miller
2008-04-17 01:29:38 UTC
Permalink
From: Ivo van Doorn <***@gmail.com>
Date: Wed, 16 Apr 2008 22:40:38 +0200
Post by Ivo van Doorn
Dave, do you want the rfkill patches directly, or should they go through
the wireless tree first?
I'm happy if it goes through the wireless tree.

That way experts like John and the others can review it
from a wireless perspective, and it's one less pull
for me to do :-)

Continue reading on narkive:
Loading...