toshiba_acpi: Add support for bluetooth toggling through rfkill (v2)
PATCH 1/1 toshiba_acpi: Add support for bluetooth toggling through rfkill (v2)
There's been a patch floating around for toshiba_acpi that exports an ad-hoc /proc interface to toggle the bluetooth adapter in a large number of Toshiba laptops. I'm not sure if it's still relevant for the latest models, but it is still required for older models such as my Tecra M3.
This change pulls in the low level Toshiba-specific code from the old patch and sets up an rfkill device and a polled input device to track the state of the hardware kill-switch.
--- Kconfig | 1 toshiba_acpi.c | 327 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 320 insertions(+), 8 deletions(-)
drivers/acpi/Kconfig +1 -0
@@ -260,6 +260,7 @@ config ACPI_ASUS config ACPI_TOSHIBA tristate "Toshiba Laptop Extras" depends on X86 + select INPUT_POLLDEV select BACKLIGHT_CLASS_DEVICE ---help--- This driver adds support for access to certain system settings
drivers/acpi/toshiba_acpi.c +319 -8
422 lines not on this page · raw message
drivers/acpi/toshiba_acpi.c static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result) · line 325
Quoted 9 lines · Philip Langdale
drivers/acpi/toshiba_acpi.c +9
-0+static int bt_rfkill_toggle_radio (void *data,enum rfkill_state state) +{ +u32 result1, result2; +u32 value; +bool radio_state; + +struct toshiba_acpi_dev *dev = data; + + value = state ==RFKILL_STATE_UNBLOCKED ;
value = (state == RFKILL_STATE_UNBLOCKED);
It is a lot easier to read without confusing the == for a =.
drivers/acpi/toshiba_acpi.c static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result) · line 352
Quoted 3 lines
drivers/acpi/toshiba_acpi.c +3
-0+case RFKILL_STATE_HARD_BLOCKED : +printk (MY_NOTICE "Ignored HARD_BLOCKED request by software\n" ); +break ;
You don't really need the above, rfkill won't ever call your toggle_radio callback like that.
If you want paranoid checking, do this instead:
default:
/* maybe WARN(), WARN_ON() or printk here */ return -EINVAL;
drivers/acpi/toshiba_acpi.c static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result) · line 376
Quoted 10 lines
drivers/acpi/toshiba_acpi.c +10
-0+static void bt_acpi_notify (acpi_handle handle,u32 event,void *data) +{ +struct toshiba_acpi_dev *dev = data; + +switch (event) { +case BT_ACPI_SOFT_UNBLOCKED_EVENT : +if (!dev->ignore_next_bt_event) { +bt_rfkill_toggle_radio (data,RFKILL_STATE_UNBLOCKED ); +rfkill_force_state (dev->rfk_dev, +RFKILL_STATE_UNBLOCKED );
This one got me confused. Why do you need that bt_rfkill_toggle_radio call here?
drivers/acpi/toshiba_acpi.c static struct backlight_ops toshiba_backlight_data = { · line 759
Quoted 4 lines
drivers/acpi/toshiba_acpi.c +4
-0+if (toshiba_acpi.rfk_dev) { +rfkill_unregister (toshiba_acpi.rfk_dev); +rfkill_free (toshiba_acpi.rfk_dev); + }
Read the kernel-doc headers of every rfkill function you call at least once... Never rfkill_free() something you rfkill_unregister()'ed.
rfkill_free() is just for the error unwind of a failure between rfkill_allocate() and rfkill_register().
drivers/acpi/toshiba_acpi.c static int __init toshiba_acpi_init(void) · line 859
Quoted 2 lines
drivers/acpi/toshiba_acpi.c +2
-0+ toshiba_acpi.rfk_dev->dev.class->suspend =NULL ; + toshiba_acpi.rfk_dev->dev.class->resume =NULL ;
Why?
Quoted 4 lines
drivers/acpi/toshiba_acpi.c +4
-0+ ret =rfkill_register (toshiba_acpi.rfk_dev); +if (ret) { +printk (MY_ERR "unable to register rfkill device\n" ); +toshiba_acpi_exit ();
drivers/acpi/toshiba_acpi.c static int __init toshiba_acpi_init(void) · line 874
Quoted 10 lines
drivers/acpi/toshiba_acpi.c +10
-0+return -ENOMEM ; + } + +if (hci_get_bt_on (&bt_on) ==HCI_SUCCESS && bt_on) { + toshiba_acpi.rfk_dev->state =RFKILL_STATE_UNBLOCKED ; + }else if (hci_get_radio_state (&radio_on) ==HCI_SUCCESS && radio_on) { + toshiba_acpi.rfk_dev->state =RFKILL_STATE_HARD_BLOCKED ; + }else { + toshiba_acpi.rfk_dev->state =RFKILL_STATE_SOFT_BLOCKED ; + }
Do the above between rfkill_allocate() and rfkill_register().
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 5 lines · Henrique de Moraes Holschuh
drivers/acpi/toshiba_acpi.c static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result) · line 325
Quoted 1 line
drivers/acpi/toshiba_acpi.c +1
-0+ value = state ==RFKILL_STATE_UNBLOCKED ;value = (state ==
RFKILL_STATE_UNBLOCKED);It is a lot easier to read without confusing the == for a =.
Fixed.
Quoted 9 lines
You don't really need the above, rfkill won't ever call your
toggle_radiocallback like that.If you want paranoid checking, do this instead:
default:
/* maybe WARN(), WARN_ON() or printk here */ return -EINVAL;
Fixed.
Quoted 5 lines
Quoted 5 lines
drivers/acpi/toshiba_acpi.c +5
-0+static void bt_acpi_notify (acpi_handle handle,u32 event,void *data) +{ +struct toshiba_acpi_dev *dev = data; + +switch (event) {
Quoted 8 lines
drivers/acpi/toshiba_acpi.c static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result) · line 376
Quoted 5 lines
drivers/acpi/toshiba_acpi.c +5
-0+case BT_ACPI_SOFT_UNBLOCKED_EVENT : +if (!dev->ignore_next_bt_event) { +bt_rfkill_toggle_radio (data,RFKILL_STATE_UNBLOCKED ); +rfkill_force_state (dev->rfk_dev, +RFKILL_STATE_UNBLOCKED );This one got me confused. Why do you need that
bt_rfkill_toggle_radiocall here?
When you turn off the hardware kill switch, the hardware will not reactivate the bluetooth device. it just returns to the SOFT_UNBLOCKED state. I put that in so that it would turn the device back on - a generally more desirable behaviour - otherwise the user has to dig around for a software way to turn it back. All the other hardware I've ever seen (including the wifi device on this laptop) turns it back on, so it seemed sensible to try and make it work as people would expect.
Quoted 6 lines
Read the kernel-doc headers of every rfkill function you call at least once... Never
rfkill_free()something you rfkill_unregister()'ed.
rfkill_free()is just for the error unwind of a failure betweenrfkill_allocate()andrfkill_register().
Fair enough, but it doesn't help that rfkill and input-polldev work in exactly opposite ways; polldev requires you to unregister and then free; some consistency wouldn't hurt.
Quoted 4 lines
drivers/acpi/toshiba_acpi.c static int __init toshiba_acpi_init(void) · line 859
Quoted 2 lines
drivers/acpi/toshiba_acpi.c +2
-0+ toshiba_acpi.rfk_dev->dev.class->suspend =NULL ; + toshiba_acpi.rfk_dev->dev.class->resume =NULL ;Why?
Good question. Gone.
Quoted 3 lines
Do the above between
rfkill_allocate()andrfkill_register().
Moved.
Diff is updated and resent as v3.
Thanks,
--phil
Quoted 14 lines · Philip Langdale
Quoted 5 lines
Quoted 5 lines
drivers/acpi/toshiba_acpi.c +5
-0+static void bt_acpi_notify (acpi_handle handle,u32 event,void *data) +{ +struct toshiba_acpi_dev *dev = data; + +switch (event) {Quoted 8 lines
drivers/acpi/toshiba_acpi.c static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result) · line 376
Quoted 5 lines
drivers/acpi/toshiba_acpi.c +5
-0+case BT_ACPI_SOFT_UNBLOCKED_EVENT : +if (!dev->ignore_next_bt_event) { +bt_rfkill_toggle_radio (data,RFKILL_STATE_UNBLOCKED ); +rfkill_force_state (dev->rfk_dev, +RFKILL_STATE_UNBLOCKED );This one got me confused. Why do you need that
bt_rfkill_toggle_radiocall here?
Quoted 7 lines
When you turn off the hardware kill switch, the hardware will not reactivate the bluetooth device. it just returns to the
SOFT_UNBLOCKEDstate. I put that in so that it would turn the device back on - a generally more desirable behaviour - otherwise the user has to dig around for a software way to turn it back. All the other hardware I've ever seen (including the wifi device on this laptop) turns it back on, so it seemed sensible to try and make it work as people would expect.
Well, you really shouldn't be doing things this way. It hardcodes policy on something that is for either rfkill-input or for userspace to decide.
I think I got the picture of how it works in toshiba, it is similar to the thinkpads.
If I got it right, you have
(1) a input device (a hardware kill switch)
(2) a rfkill controller for bluetooth
And the firmware causes (1) to force-disable (2), but loses state when it does so and leaves it disabled when (1) is not forcing the state to disable anymore.
Well, the textbook way to connect that to rfkill is something like this, please check if it makes sense:
Export (1) as a input device (you have events to hook to when it changes state) or polled input device (you don't have said events). Do NOT register
(1) with a struct rfkill at all. It is purely an input device.
DO NOT expose (2) as an input device at all. Instead, register it to a rfkill struct, of type bluetooth.
On the event handler for (1), you issue the EV_SW SW_RFKILL_ALL input event. Since you *do* know events from (1) are likely to also have messed with the state of (2), you also check (2)'s state at this time and update it through rfkill_force_state().
Due to the interaction of 1 and 2, you need to implement and deal with RFKILL_STATE_HARD_BLOCKED. Since you do know the firmware will be hard-blocking (2) when (1) is active, you return RFKILL_STATE_HARD_BLOCKED for (2)'s state every time (1) is active. Otherwise, you return RFKILL_STATE_SOFT_BLOCKED when (2) is blocked, and RFKILL_STATE_UNBLOCKED otherwise.
This will cause the state of (2) to go to either RFKILL_STATE_HARD_BLOCKED or RFKILL_STATE_SOFT_BLOCKED when (1) changes state.
[Note: the above does assume (1) blocks (2) in some way you cannot override, and that trying to unblock (2) while (1) is blocking it is futile].
rfkill-input (now) or userspace (someday) will take care of kicking the radio to RFKILL_STATE_UNBLOCKED when (1) issues an event that signals that radios don't have to remain blocked. Maybe this is why you see the WLAN going on when you deactivate the radio kill switch?
And rfkill-input will soon be enhanced to let the user configure it to do something different if he wants. Your driver doesn't (and shouldn't) hardcode policy about it.
Quoted 9 lines
Quoted 5 lines
Read the kernel-doc headers of every rfkill function you call at least once... Never
rfkill_free()something you rfkill_unregister()'ed.
rfkill_free()is just for the error unwind of a failure betweenrfkill_allocate()andrfkill_register().Fair enough, but it doesn't help that rfkill and input-polldev work in exactly opposite ways; polldev requires you to unregister and then free; some consistency wouldn't hurt.
That's just how the code was when I started enhancing it, you can ask Ivo (the rfkill maintainer) about the reasons behind it if you want, or you could try to submit a patch to Ivo changing rfkill (and every driver using rfkill) to rfkill_free() after rfkill_unregister().
Quoted 6 lines
Quoted 4 lines
drivers/acpi/toshiba_acpi.c static int __init toshiba_acpi_init(void) · line 859
Quoted 2 lines
drivers/acpi/toshiba_acpi.c +2
-0+ toshiba_acpi.rfk_dev->dev.class->suspend =NULL ; + toshiba_acpi.rfk_dev->dev.class->resume =NULL ;Why?
Good question. Gone.
Thanks. Please take note that rfkill will, right now, try to BLOCK all radios on suspend. That will be changed soon (2.6.28 at the latest), and your driver will have to handle blocking radios on suspend directly if it is needed for toshibas.
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 5 lines · Henrique de Moraes Holschuh
rfkill-input (now) or userspace (someday) will take care of kicking the radio to
RFKILL_STATE_UNBLOCKEDwhen (1) issues an event that signals that radios don't have to remain blocked. Maybe this is why you see the WLAN going on when you deactivate the radio kill switch?
It's all done behind the scenes I think (it's an ipw2200 device). There's no rfkill integration from that driver.
Quoted 3 lines
And rfkill-input will soon be enhanced to let the user configure it to do something different if he wants. Your driver doesn't (and shouldn't) hardcode policy about it.
Ok, that makes things much easier for me :-) But it means that for now the user will have to manually kick the device.
Quoted 5 lines
Thanks. Please take note that rfkill will, right now, try to BLOCK all radios on suspend. That will be changed soon (2.6.28 at the latest), and your driver will have to handle blocking radios on suspend directly if it is needed for toshibas.
Why is this necessary? Doesn't the radio power down as part of the suspend process? How would I tell what the hardware is doing?
I'll update the diff when I can.
--phil
Quoted 8 lines · Philip Langdale
Quoted 4 lines · Henrique de Moraes Holschuh
rfkill-input (now) or userspace (someday) will take care of kicking the radio to
RFKILL_STATE_UNBLOCKEDwhen (1) issues an event that signals that radios don't have to remain blocked. Maybe this is why you see the WLAN going on when you deactivate the radio kill switch?It's all done behind the scenes I think (it's an ipw2200 device). There's no rfkill integration from that driver.
Then it is probably not turning the device on, but rather, reverting its state. This is normal with hardware like the ipw2200 that has a hardware rfkill line.
Quoted 6 lines
Quoted 3 lines
And rfkill-input will soon be enhanced to let the user configure it to do something different if he wants. Your driver doesn't (and shouldn't) hardcode policy about it.
Ok, that makes things much easier for me :-) But it means that for now the user will have to manually kick the device.
Actually, all the users have to do right now is to have rfkill-input loaded. It *already* kicks all devices online when you release the master rfkill switch.
Quoted 7 lines
Quoted 4 lines
Thanks. Please take note that rfkill will, right now, try to BLOCK all radios on suspend. That will be changed soon (2.6.28 at the latest), and your driver will have to handle blocking radios on suspend directly if it is needed for toshibas.
Why is this necessary? Doesn't the radio power down as part of the suspend process? How would I tell what the hardware is doing?
Some network drivers want to do it themselves, so now it will be the job of every driver to know whether it should do something or not.
To test your platform to know if you will need to do something, just comment the rfkill->toggle_radio in rfkill_suspend, and check if your radios are still transmitting when you suspend. I really doubt any laptop has that problem, though.
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 · Henrique de Moraes Holschuh
Some network drivers want to do it themselves, so now it will be the job of every driver to know whether it should do something or not.
To test your platform to know if you will need to do something, just comment the
rfkill->toggle_radioinrfkill_suspend, and check if your radios are still transmitting when you suspend. I really doubt any laptop has that problem, though.
Oh, and if you don't have a way to test, you can just call your own private switch_my_radios_off() function in your driver's suspend() handler. You don't even have to turn them back on on resume(), as rfkill will always do that for you.
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 29 lines · Henrique de Moraes Holschuh
Well, the textbook way to connect that to rfkill is something like this, please check if it makes sense:
Export (1) as a input device (you have events to hook to when it changes state) or polled input device (you don't have said events). Do NOT register
(1) with a struct rfkill at all. It is purely an input device.DO NOT expose (2) as an input device at all. Instead, register it to a rfkill struct, of type bluetooth.
On the event handler for (1), you issue the
EV_SWSW_RFKILL_ALLinput event. Since you *do* know events from (1) are likely to also have messed with the state of (2), you also check (2)'s state at this time and update it throughrfkill_force_state().Due to the interaction of 1 and 2, you need to implement and deal with
RFKILL_STATE_HARD_BLOCKED. Since you do know the firmware will be hard-blocking (2) when (1) is active, you returnRFKILL_STATE_HARD_BLOCKEDfor (2)'s state every time (1) is active. Otherwise, you returnRFKILL_STATE_SOFT_BLOCKEDwhen (2) is blocked, andRFKILL_STATE_UNBLOCKEDotherwise.This will cause the state of (2) to go to either
RFKILL_STATE_HARD_BLOCKEDorRFKILL_STATE_SOFT_BLOCKEDwhen (1) changes state.[Note: the above does assume (1) blocks (2) in some way you cannot override, and that trying to unblock (2) while (1) is blocking it is futile].
Quoted 8 lines
rfkill-input (now) or userspace (someday) will take care of kicking the radio to
RFKILL_STATE_UNBLOCKEDwhen (1) issues an event that signals that radios don't have to remain blocked. Maybe this is why you see the WLAN going on when you deactivate the radio kill switch?And rfkill-input will soon be enhanced to let the user configure it to do something different if he wants. Your driver doesn't (and shouldn't) hardcode policy about it.
Hmm. So, I've updated the diff to do this, but rfkill-input is not kicking the bluetooth device back on after I release the kill switch. it just returns to SOFT_UNBLOCKED and stays there.
--phil
Quoted 34 lines · Philip Langdale
Quoted 33 lines · Henrique de Moraes Holschuh
Well, the textbook way to connect that to rfkill is something like this, please check if it makes sense:
Export (1) as a input device (you have events to hook to when it changes state) or polled input device (you don't have said events). Do NOT register
(1) with a struct rfkill at all. It is purely an input device.DO NOT expose (2) as an input device at all. Instead, register it to a rfkill struct, of type bluetooth.
On the event handler for (1), you issue the
EV_SWSW_RFKILL_ALLinput event. Since you *do* know events from (1) are likely to also have messed with the state of (2), you also check (2)'s state at this time and update it throughrfkill_force_state().Due to the interaction of 1 and 2, you need to implement and deal with
RFKILL_STATE_HARD_BLOCKED. Since you do know the firmware will be hard-blocking (2) when (1) is active, you returnRFKILL_STATE_HARD_BLOCKEDfor (2)'s state every time (1) is active. Otherwise, you returnRFKILL_STATE_SOFT_BLOCKEDwhen (2) is blocked, andRFKILL_STATE_UNBLOCKEDotherwise.This will cause the state of (2) to go to either
RFKILL_STATE_HARD_BLOCKEDorRFKILL_STATE_SOFT_BLOCKEDwhen (1) changes state.[Note: the above does assume (1) blocks (2) in some way you cannot override, and that trying to unblock (2) while (1) is blocking it is futile].
rfkill-input (now) or userspace (someday) will take care of kicking the radio to
RFKILL_STATE_UNBLOCKEDwhen (1) issues an event that signals that radios don't have to remain blocked. Maybe this is why you see the WLAN going on when you deactivate the radio kill switch?
Quoted 7 lines
Quoted 3 lines
And rfkill-input will soon be enhanced to let the user configure it to do something different if he wants. Your driver doesn't (and shouldn't) hardcode policy about it.
Hmm. So, I've updated the diff to do this, but rfkill-input is not kicking the bluetooth device back on after I release the kill switch. it just returns to
SOFT_UNBLOCKEDand stays there.
Hmm? UNBLOCKED means it is not being rfkilled. Why is not working? Put some debug printks on your toggle_radio callback, that might help fix the issue...
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 5 lines · Henrique de Moraes Holschuh
Hmm? UNBLOCKED means it is not being rfkilled. Why is not working? Put some debug printks on your
toggle_radiocallback, that might help fix the issue...
Well, what I see is this:
At modprobe, I get an explicit unblock request. When I modify 'state' in sysfs, I see appropriate requests to soft block and unblock. When I activate the hardware kill switch, the rfkill state goes to hard_block but no one calls toggle_radio and when I release the kill switch, the state goes to soft_block and no one calls toggle_radio.
--phil
Quoted 5 lines · Philip Langdale
Quoted 3 lines · Henrique de Moraes Holschuh
Hmm? UNBLOCKED means it is not being rfkilled. Why is not working? Put some debug printks on your
toggle_radiocallback, that might help fix the issue...
Quoted 7 lines
Well, what I see is this:
At modprobe, I get an explicit unblock request. When I modify 'state' in sysfs, I see appropriate requests to soft block and unblock. When I activate the hardware kill switch, the rfkill state goes to
hard_blockbut no one callstoggle_radioand when I release the kill switch, the state goes tosoft_blockand no one callstoggle_radio.
Looks like either rfkill-input is inactive, or something is binding to the toshiba_acpi input device and setting it to exclusive mode (X evdev does this).
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 4 lines · Henrique de Moraes Holschuh
Looks like either rfkill-input is inactive, or something is binding to the
toshiba_acpiinput device and setting it to exclusive mode (X evdev does this).
Almost. Turns out there were two problems:
1) I was setting SW_RFKILL_ALL as a keybit instead of an swbit - so no events reported. Whoops. :-)
2) Maybe this shouldn't surprise me, but the expected switch semantics are that '1' means radios are on and 0 means they are killed - I thought it was the other way round.
Now, when I release the kill switch, rfkill-input does indeed turn the radio on. Great. The remaining problem is that rfkill-input is setting it to soft-blocked when the switch is registered. I'm not sure why it's deciding to do that. Anyway, I'm almost there. Thanks.
--phil
Quoted 5 lines · Philip Langdale
Now, when I release the kill switch, rfkill-input does indeed turn the radio on. Great. The remaining problem is that rfkill-input is setting it to soft-blocked when the switch is registered. I'm not sure why it's deciding to do that. Anyway, I'm almost there. Thanks.
Finally sorted it out. Updated diff is posted as v5.
--phil