b43: rework rfkill code
RFC 1/1 b43: rework rfkill code
I've reworked the rfkill code in b43. This ought to be more consistent with the other drivers and it seems to work on the machines I've tested it on here, but it'd be good to get some feedback.
Firstly, I've replaced the polled input device. It's just some delayed work now. It polls the hardware every second to determine whether the radio has been hardware killed or not. If it has, it sets the rfkill state to HARD_BLOCKED. If the radio is enabled and the previous state was HARD_BLOCKED, it resets the state to UNBLOCKED. If the radio is enabled and the previous state was SOFT_BLOCKED, it leaves the state as is.
I also removed some of the complexity from the rfkill toggle function, since the rfkill core will handle the case of the user requesting a change from HARD_BLOCKED without the driver needing to care.
The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
How does this look to people?
drivers/net/wireless/b43/main.c +0 -15
@@ -3384,21 +3384,6 @@ static int b43_op_config(struct ieee80211_hw *hw, u32 changed) b43_is_mode(wl, NL80211_IFTYPE_MESH_POINT))b43_set_beacon_int (dev, conf->beacon_int); -if (!!conf->radio_enabled != phy->radio_on) { -if (conf->radio_enabled) { -b43_software_rfkill (dev,RFKILL_STATE_UNBLOCKED ); -b43info (dev->wl,"Radio turned on by software\n" ); -if (!dev->radio_hw_enable) { -b43info (dev->wl,"The hardware RF-kill button " -"still turns the radio physically off. " -"Press the button to turn it on.\n" ); - } - }else { -b43_software_rfkill (dev,RFKILL_STATE_SOFT_BLOCKED ); -b43info (dev->wl,"Radio turned off by software\n" ); - } - } -spin_lock_irqsave (&wl->irq_lock, flags);b43_interrupt_enable (dev, savedirqs);mmiowb ();
drivers/net/wireless/b43/rfkill.c +33 -77
160 lines not on this page · raw message
drivers/net/wireless/b43/rfkill.h +2 -2
@@ -13,8 +13,8 @@ struct b43_wldev;struct b43_rfkill {/* The RFKILL subsystem data structure */ struct rfkill *rfkill; -/* The poll device for the RFKILL input button */ -struct input_polled_dev *poll_dev; +/* The work queue for the RFKILL input button */ +struct delayed_work work;/* Did initialization succeed? Used for freeing. */ bool registered;/* The unique name of this rfkill switch */
Signature · 1 line
Matthew Garrett | [email protected]
Johannes Berg
maintainerQuoted 9 lines · Matthew Garrett
The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
We're going to have to integrate rfkill with mac80211, but nobody cares.
johannes
Quoted 1 line · On Wednesday 10 December 2008 16:09:35 Matthew Garrett wrote:
I've reworked the rfkill code in b43. This ought to be more consistent
...
I'm fine with this, as long as you take over the responsibility for the whole b43-rfkill code. I'm not going to touch it anymore. I'm only going to fix it by either forwarding bugreports to somebody else (you, in that case) or by reverting patches until it starts working again.
Signature · 1 line
Greetings, Michael.
Quoted 7 lines · Michael Buesch
Quoted 1 line · On Wednesday 10 December 2008 16:09:35 Matthew Garrett wrote:
I've reworked the rfkill code in b43. This ought to be more consistent
...
I'm fine with this, as long as you take over the responsibility for the whole b43-rfkill code. I'm not going to touch it anymore. I'm only going to fix it by either forwarding bugreports to somebody else (you, in that case) or by reverting patches until it starts working again.
Works for me, though I'd appreciate it if people could give it a quick sanity test. The main thing I'm worried about is LED control, which I think /ought/ to still work but is dependent on everything in the rfkill core working properly.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 12 lines · Johannes Berg
Quoted 9 lines · Matthew Garrett
The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
Quoted 1 line
We're going to have to integrate rfkill with mac80211, but nobody cares.
It's on the rt2x00 todo list since quite some time already in the hope to find a volunteer who was willing to do it.
I can't promise any patches for at least a month in this area, but I'll see if I can take a shot at this in a couple of weeks time.
Ivo
Hi Johannes,
Quoted 11 lines
Quoted 9 lines
The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
We're going to have to integrate rfkill with mac80211, but nobody cares.
if you figured out on how to do it the best way, then let me know, because I have to do the same thing for Bluetooth.
Regards
Marcel
Johannes Berg
maintainer[warning: this is going to be a long mail and will tell you why and how I think rfkill needs to be rewritten]
Quoted 2 lines · Marcel Holtmann
if you figured out on how to do it the best way, then let me know, because I have to do the same thing for Bluetooth.
I'd suggest to start by rewriting rfkill to be not such a mess to use.
We really need to disentangle the state tracking rfkill does. People have said this a million times before, but nobody cares: drivers need to register hard-blocked and get soft-block states independently, not in a single enum; API for drivers needs to be, above all, EASY to use.
For mac80211, we really need to register an rfkill structure for each physical device that mac80211 knows about. Drivers would not be allowed to get access to this structure, because the toggle_radio() callback is assigned to a mac80211 function that calls the driver's ->config() callback and, at some point, also disallows configuring interfaces that belong to a device that is rfkilled.
To properly implement this, however, we need to disentangle rfkill states and provide just a single API function to change the state:
rfkill_hw_kill(struct rfkill *rfkill, bool killed);
this would be shadowed by mac80211 with
ieee80211_hw_rfkill(struct ieee80211_hw *hw, bool killed)
so mac80211 knows about hard-kills too, since they don't call ->toggle_radio() obviously.
I know what you're thinking at this point, but hear me out. This is another thing where I think rfkill's design is fundamentally wrong. If you're not thinking anything, hear me out anyway :)
Let's take a step back here and analyse what we really have and need:
* we have devices with a plethora of rfkill setups:
1) devices disappear from the bus (not handled here, not possible either)
2) devices have hard kill button that just disables the radio (need to know about this so users aren't left in the dark)
3) devices have soft kill buttons that just notify the driver
4) soft kill switches that work through other input methods (ACPI comes to mind, but anything is possible)
* we have various ways to kill the radio:
1) like above
2) through platform methods (toshiba ACPI I think?)
3) through the driver only
The last two cases only differ in the radios that the button is supposed to apply to, but this is more of a policy decision.
The point where I think rfkill's design is totally wrong is that it intimately ties this software kill from case three to the rfkill structure as well.
Instead, we should come to accept that this is just a button as well. A button, however, that is *by default* tied to a particular radio. Therefore, drivers for devices that provide such buttons, should, in my opinion, register an rfkill button driver that
a) by default is tied to their hardware by a pointer to the rfkill struct for the hardware, but this could be possible to override in sysfs like LEDs are, if you want the button to really kill all radios
b) serves as input to the rfkill subsystem which will then look up the right rfkill struct(s) and ask them to kill the radio
This would add new API for a button, but again only a single function, something like
rfkill_button_update(struct rfkill_button *btn, bool killed)
Finally, rfkill-input will boil down to just registering a software button that happens to control all radios by default, by setting the controlled rfkill struct to NULL.
johannes
Johannes Berg
maintainerQuoted 4 lines · Johannes Berg
We really need to disentangle the state tracking rfkill does. People have said this a million times before, but nobody cares: drivers need to register hard-blocked and get soft-block states independently, not in a single enum; API for drivers needs to be, above all, EASY to use.
Also, the rfkill struct itself is a mess. What's get_state() for? Why is this not layered? How can get_state() work correctly, it doesn't poll the device so it doesn't look like software will ever get a state update.
Then there's user_claim_unsupported which is set by all drivers but rt2x00, probably because they have hardware kill switches and thus they have to set it even if it's not strictly true, because of the lacking separation between these things (that I pointed out)
johannes
Johannes Berg
maintainerQuoted 4 lines · Johannes Berg
Then there's
user_claim_unsupportedwhich is set by all drivers but rt2x00, probably because they have hardware kill switches and thus they have to set it even if it's not strictly true, because of the lacking separation between these things (that I pointed out)
IOW, correct me if I'm wrong, it seems to me that user_claim_unsupported really is a wrong name for "has hw kill", which could be avoided if sw and hw kill were a different thing and the rfkill structure was only used, as I'm proposing, for hw kill and sw kill _notifications_, but not the sw kill operation itself.
johannes
Quoted 4 lines · On Wednesday 10 December 2008 18:23:40 Johannes Berg wrote:
Then there's
user_claim_unsupportedwhich is set by all drivers but rt2x00, probably because they have hardware kill switches and thus they have to set it even if it's not strictly true, because of the lacking separation between these things (that I pointed out)
I introduced it when I ported b43 to rfkill.
Well, a lot of semantical changes were made _after_ that.
When I added it there only were two rfkill states and b43 handled these wrt the actual hardware state (and I still think that's the right thing to do. The sw-state intermix is confusing).
So when I added the flag it meant:user_claim_unsupported = True means user cannot change the hardware kill state. So basically it means the device has two states. One software state and one hardware state.
However, I don't know what the semantics for the flag are today. Lots of code changed.
Signature · 1 line
Greetings, Michael.
Johannes Berg
maintainerQuoted 5 lines · Michael Buesch
I introduced it when I ported b43 to rfkill.
Well, a lot of semantical changes were made _after_ that.
When I added it there only were two rfkill states and b43 handled these wrt the actual hardware state (and I still think that's the right thing to do. The sw-state intermix is confusing).
Right.
Quoted 2 lines
So when I added the flag it meant:
user_claim_unsupported= True means user cannot change the hardware kill state.
Yeah, but the assumption that software can change the "hardware kill state" is rather stupid to start with, I think.
Quoted 3 lines
So basically it means the device has two states. One software state and one hardware state.
However, I don't know what the semantics for the flag are today. Lots of code changed.
Ok. I think the fundamental flaw here is assuming that there's just a single state. There isn't. The device can be turned off in hardware (in which case sw won't be able do anything about it, but we want to know) or in software (which we want to handle). Pretending that there's just a single state that's either hw-off, sw-off or on is plain wrong. The device can be hw-off and sw-off at the same time, and then if you turn off the hw-off button it won't turn on (however, unless your system integrator totally screwed up, you won't have a hw and a sw button on your system)
johannes
Quoted 9 lines · Johannes Berg
Ok. I think the fundamental flaw here is assuming that there's just a single state. There isn't. The device can be turned off in hardware (in which case sw won't be able do anything about it, but we want to know) or in software (which we want to handle). Pretending that there's just a single state that's either hw-off, sw-off or on is plain wrong. The device can be hw-off and sw-off at the same time, and then if you turn off the hw-off button it won't turn on (however, unless your system integrator totally screwed up, you won't have a hw and a sw button on your system)
They may not be physical buttons, but we can often control this anyway. For instance, my HP has a button that will perform a hardware disable of the wifi card. However, I can control that button's state through software with the hp-wmi driver. The way we currently handle that (and, I think, the only way we *can* handle that) is to provide two separate rfkill interfaces - one tied to the wireless device, one tied to the platform device.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 12 lines · On Wednesday 10 December 2008 18:51:02 Matthew Garrett wrote:
Quoted 9 lines · Johannes Berg
Ok. I think the fundamental flaw here is assuming that there's just a single state. There isn't. The device can be turned off in hardware (in which case sw won't be able do anything about it, but we want to know) or in software (which we want to handle). Pretending that there's just a single state that's either hw-off, sw-off or on is plain wrong. The device can be hw-off and sw-off at the same time, and then if you turn off the hw-off button it won't turn on (however, unless your system integrator totally screwed up, you won't have a hw and a sw button on your system)
Quoted 1 line
They may not be physical buttons, but we can often control this anyway.
But we do not _want_ it.
If you can do it, keep it private to the driver. Do not export it to other layers. If you need to to sw-rfkill through it, do it in the driver and multiplex the hw-sw-states in the driver.
Signature · 1 line
Greetings, Michael.
Johannes Berg
maintainerQuoted 4 lines · Matthew Garrett
They may not be physical buttons, but we can often control this anyway. For instance, my HP has a button that will perform a hardware disable of the wifi card. However, I can control that button's state through software with the hp-wmi driver.
That's indeed a complication I wasn't aware of.
Quoted 4 lines
The way we currently handle that (and,
I think, the only way we *can* handle that) is to provide two separate rfkill interfaces - one tied to the wireless device, one tied to the platform device.
Yes, but how do we currently do this?
Does the wireless driver get the notification about this from the hardware, like it would if this was a real physical switch? Then it's probably pretty simple: provide a rfkill struct from the driver that updates hard-kill and provide a second rfkill struct for the platform device that doesn't get hard-killed, but also provide a soft-kill input form the platform device. That way, you can toggle that button, but you can also software-enable the platform rfkill device and that in turn re-enables the wifi-rfkill "hw" switch device.
If we need to tie them together in software it gets more complicated though.
johannes
Quoted 2 lines · Johannes Berg
Does the wireless driver get the notification about this from the hardware, like it would if this was a real physical switch?
Yes.
Quoted 6 lines
Then it's probably pretty simple: provide a rfkill struct from the driver that updates hard-kill and provide a second rfkill struct for the platform device that doesn't get hard-killed, but also provide a soft-kill input form the platform device. That way, you can toggle that button, but you can also software-enable the platform rfkill device and that in turn re-enables the wifi-rfkill "hw" switch device.
Right. That's prety close to the current situation.
Quoted 2 lines
If we need to tie them together in software it gets more complicated though.
I think we can avoid that, thankfully.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 16 lines · Johannes Berg
Quoted 4 lines · Matthew Garrett
They may not be physical buttons, but we can often control this anyway. For instance, my HP has a button that will perform a hardware disable of the wifi card. However, I can control that button's state through software with the hp-wmi driver.
That's indeed a complication I wasn't aware of.
Quoted 4 lines
The way we currently handle that (and,
I think, the only way we *can* handle that) is to provide two separate rfkill interfaces - one tied to the wireless device, one tied to the platform device.Yes, but how do we currently do this?
Quoted 8 lines
Does the wireless driver get the notification about this from the hardware, like it would if this was a real physical switch? Then it's probably pretty simple: provide a rfkill struct from the driver that updates hard-kill and provide a second rfkill struct for the platform device that doesn't get hard-killed, but also provide a soft-kill input form the platform device. That way, you can toggle that button, but you can also software-enable the platform rfkill device and that in turn re-enables the wifi-rfkill "hw" switch device.
This sort of sucks for userspace, because we see the actual wifi card as hardblocked, but some other random button as softblocked. There's no indication that changing the softblock one will affect the hardblocked one. What are userspace processes supposed to do here, assume that if a non-radio-associated softblocked switch exists, that it can re-enable a hardblocked radio of some random wifi card?
Dan
Johannes Berg
maintainerQuoted 9 lines · Dan Williams
Quoted 8 lines
Does the wireless driver get the notification about this from the hardware, like it would if this was a real physical switch? Then it's probably pretty simple: provide a rfkill struct from the driver that updates hard-kill and provide a second rfkill struct for the platform device that doesn't get hard-killed, but also provide a soft-kill input form the platform device. That way, you can toggle that button, but you can also software-enable the platform rfkill device and that in turn re-enables the wifi-rfkill "hw" switch device.
Quoted 6 lines
This sort of sucks for userspace, because we see the actual wifi card as hardblocked, but some other random button as softblocked. There's no indication that changing the softblock one will affect the hardblocked one. What are userspace processes supposed to do here, assume that if a non-radio-associated softblocked switch exists, that it can re-enable a hardblocked radio of some random wifi card?
The other question is whether we actually care? So what if the hardware can only be enabled with the button, why does that matter?
johannes
Quoted 18 lines · Johannes Berg
Quoted 9 lines · Dan Williams
Quoted 8 lines
Does the wireless driver get the notification about this from the hardware, like it would if this was a real physical switch? Then it's probably pretty simple: provide a rfkill struct from the driver that updates hard-kill and provide a second rfkill struct for the platform device that doesn't get hard-killed, but also provide a soft-kill input form the platform device. That way, you can toggle that button, but you can also software-enable the platform rfkill device and that in turn re-enables the wifi-rfkill "hw" switch device.
Quoted 6 lines
This sort of sucks for userspace, because we see the actual wifi card as hardblocked, but some other random button as softblocked. There's no indication that changing the softblock one will affect the hardblocked one. What are userspace processes supposed to do here, assume that if a non-radio-associated softblocked switch exists, that it can re-enable a hardblocked radio of some random wifi card?
Quoted 2 lines
The other question is whether we actually care? So what if the hardware can only be enabled with the button, why does that matter?
I guess it doesn't, as long as in Matthew's case, the actual radio rfkill state is only ever softblocked, because it actually *can* be re-enabled with the platform button or something.
Dan
Quoted 26 lines · On Wednesday 10 December 2008 19:29:50 Dan Williams wrote:
Quoted 16 lines · Johannes Berg
Quoted 4 lines · Matthew Garrett
They may not be physical buttons, but we can often control this anyway. For instance, my HP has a button that will perform a hardware disable of the wifi card. However, I can control that button's state through software with the hp-wmi driver.
That's indeed a complication I wasn't aware of.
Quoted 4 lines
The way we currently handle that (and,
I think, the only way we *can* handle that) is to provide two separate rfkill interfaces - one tied to the wireless device, one tied to the platform device.Yes, but how do we currently do this?
Quoted 8 lines
Does the wireless driver get the notification about this from the hardware, like it would if this was a real physical switch? Then it's probably pretty simple: provide a rfkill struct from the driver that updates hard-kill and provide a second rfkill struct for the platform device that doesn't get hard-killed, but also provide a soft-kill input form the platform device. That way, you can toggle that button, but you can also software-enable the platform rfkill device and that in turn re-enables the wifi-rfkill "hw" switch device.
Quoted 6 lines
This sort of sucks for userspace, because we see the actual wifi card as hardblocked, but some other random button as softblocked. There's no indication that changing the softblock one will affect the hardblocked one. What are userspace processes supposed to do here, assume that if a non-radio-associated softblocked switch exists, that it can re-enable a hardblocked radio of some random wifi card?
I don't see the problem.
If userspace wants to enable wifi, it should simply _try_ to do so:
Userspace sees hw-block and sw-block state:
- Unblock the sw state
- Re-fetch hw-block and sw-block state
- If either one is blocked, we can't enable the radio.
- Notify user.
Signature · 1 line
Greetings, Michael.
Quoted 8 lines · Johannes Berg
Quoted 4 lines · Johannes Berg
Then there's
user_claim_unsupportedwhich is set by all drivers but rt2x00, probably because they have hardware kill switches and thus they have to set it even if it's not strictly true, because of the lacking separation between these things (that I pointed out)IOW, correct me if I'm wrong, it seems to me that
user_claim_unsupportedreally is a wrong name for "has hw kill", which could be avoided if sw
I never understood what user_claim_unsupported is for. I left it alone because of that, but it looks like some artifact of the old rfkill that did horrible things to the input layer.
Signature · 4 lines
"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
Quoted 7 lines · On Wednesday 10 December 2008 22:33:34 Henrique de Moraes Holschuh wrote:
Quoted 6 lines · Johannes Berg
Quoted 4 lines · Johannes Berg
Then there's
user_claim_unsupportedwhich is set by all drivers but rt2x00, probably because they have hardware kill switches and thus they have to set it even if it's not strictly true, because of the lacking separation between these things (that I pointed out)
Quoted 6 lines
Quoted 2 lines
IOW, correct me if I'm wrong, it seems to me that
user_claim_unsupportedreally is a wrong name for "has hw kill", which could be avoided if swI never understood what
user_claim_unsupportedis for. I left it alone because of that, but it looks like some artifact of the old rfkill that did horrible things to the input layer.
No, as I just explained. It comes from a time when we didn't have all that input stuff at all. It was a workaround. rfkill basically had a facility to change the hardware rfkill state from userspace. As b43 does not support that, I introduced the flag.
Today we have three states (which is still broken, but you saw the rest of the thread...), so I guess we can remove it again.
We cannot change the hardware state. That's what the flag is (was) for.
Signature · 1 line
Greetings, Michael.
Quoted 12 lines · Johannes Berg
Quoted 9 lines · Matthew Garrett
The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
Quoted 1 line
We're going to have to integrate rfkill with mac80211, but nobody cares.
What strikes me, watching this from the outside - is that rfkill and power saving seem to be doing essentially the same thing: temporarily powering down the radio / card.
If we're going to integrate rfkill and mac80211 or rewrite rfkill, why not hook it into power saving too?
Just my $0.01
Thanks,
Signature · 3 lines
Julian Calaby Email: [email protected]
Quoted 13 lines · On Thursday 11 December 2008 01:32:37 Julian Calaby wrote:
Quoted 12 lines · Johannes Berg
Quoted 9 lines · Matthew Garrett
The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
Quoted 5 lines
Quoted 1 line
We're going to have to integrate rfkill with mac80211, but nobody cares.
What strikes me, watching this from the outside - is that rfkill and power saving seem to be doing essentially the same thing: temporarily powering down the radio / card.
I think it's essentially a different thing.
rfkill means -> turn off the radio; no matter what.
PS means -> turn off the radio for whatever amount of microseconds and periodically wake up to see what's up.
PS-core also takes place in the firmware of the device, where rfkill is a much higher layer thing.
Signature · 1 line
Greetings, Michael.
Quoted 11 lines · Michael Buesch
Quoted 3 lines · On Thursday 11 December 2008 01:32:37 Julian Calaby wrote:
What strikes me, watching this from the outside - is that rfkill and power saving seem to be doing essentially the same thing: temporarily powering down the radio / card.
I think it's essentially a different thing. rfkill means -> turn off the radio; no matter what. PS means -> turn off the radio for whatever amount of microseconds and periodically wake up to see what's up. PS-core also takes place in the firmware of the device, where rfkill is a much higher layer thing.
I agree, they shouldn't be intermixed at all.
Signature · 1 line
Kalle Valo
Quoted 26 lines · Matthew Garrett
I've reworked the rfkill code in b43. This ought to be more consistent with the other drivers and it seems to work on the machines I've tested it on here, but it'd be good to get some feedback.
Firstly, I've replaced the polled input device. It's just some delayed work now. It polls the hardware every second to determine whether the radio has been hardware killed or not. If it has, it sets the rfkill state to
HARD_BLOCKED. If the radio is enabled and the previous state wasHARD_BLOCKED, it resets the state to UNBLOCKED. If the radio is enabled and the previous state wasSOFT_BLOCKED, it leaves the state as is.I also removed some of the complexity from the rfkill toggle function, since the rfkill core will handle the case of the user requesting a change from
HARD_BLOCKEDwithout the driver needing to care.The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
Quoted 1 line
How does this look to people?
All this discussion about hard vs soft rfkill makes my head hurt and I have stopped reading those posts.
With this patch, my b43 device and its LED still work.
Larry
Quoted 26 lines · Matthew Garrett
I've reworked the rfkill code in b43. This ought to be more consistent with the other drivers and it seems to work on the machines I've tested it on here, but it'd be good to get some feedback.
Firstly, I've replaced the polled input device. It's just some delayed work now. It polls the hardware every second to determine whether the radio has been hardware killed or not. If it has, it sets the rfkill state to
HARD_BLOCKED. If the radio is enabled and the previous state wasHARD_BLOCKED, it resets the state to UNBLOCKED. If the radio is enabled and the previous state wasSOFT_BLOCKED, it leaves the state as is.I also removed some of the complexity from the rfkill toggle function, since the rfkill core will handle the case of the user requesting a change from
HARD_BLOCKEDwithout the driver needing to care.The final change is that I removed the code for changing the wireless state in response to the txpower configuration in mac80211. Right now, I can't see any way for this to work correctly - if the user disables the radio via rfkill, mac80211 doesn't flag the radio as disabled. As a result, the next time the configuration callback is called, b43 reenables the radio again, even though the user has explicitly disabled it. I don't think any of the other drivers handle this case, so I'm not really sure what the best way to handle this in future is. The current situation certainly seems broken.
Quoted 1 line
How does this look to people?
All this discussion about hard vs soft rfkill makes my head hurt and I have stopped reading those posts.
Correction to my earlier post. If the system is booted with the RF switch off, the LED is on, whereas it should be off. The original code works correctly.
Larry
Quoted 4 lines · Larry Finger
Quoted 3 lines · Matthew Garrett
I've reworked the rfkill code in b43. This ought to be more consistent with the other drivers and it seems to work on the machines I've tested it on here, but it'd be good to get some feedback.
<snip>
Quoted 7 lines
Quoted 1 line
How does this look to people?
All this discussion about hard vs soft rfkill makes my head hurt and I have stopped reading those posts.
Correction to my earlier post. If the system is booted with the RF switch off, the LED is on, whereas it should be off. The original code works correctly.
Based on the above, I'm dropping this patch. Please submit a non-regressing version! :-)
John
Signature · 2 lines
John W. Linville Linux should be at the core [email protected] of your literate lifestyle.
Johannes Berg
maintainerMarcel,
Quoted 4 lines
Quoted 1 line
We're going to have to integrate rfkill with mac80211, but nobody cares.
if you figured out on how to do it the best way, then let me know, because I have to do the same thing for Bluetooth.
I've now rewritten rfkill [beep] to [beep] -- the plan for mac80211 now is to have _it_ register the rfkill struct, and let the driver only provide the hard-block state (or a function to poll it where necessary), and handle everything else via the regular config call in mac80211 with radio_enabled. Then mac80211 can also react on unblock by reassociating or whatever.
johannes