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.
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.
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
--
