These patches add support to use the rfkill core functionality to the
Toshiba bluetooth driver and adapting the existing code to it.
Azael Avalos (6):
toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev
toshiba_bluetooth: Add RFKill handler functions
toshiba_bluetooth: Clean toshiba_bluetooth_enable function
toshiba_bluetooth: Adapt *_notify and *_resume functions to rfkill
toshiba_bluetooth: Change BT status message to debug
platform/x86: Add RFKILL dependency to toshiba_bluetooth driver
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/toshiba_bluetooth.c | 175 ++++++++++++++++++++++++-------
2 files changed, 137 insertions(+), 39 deletions(-)
--
2.3.5
This patch adds a struct named toshiba_bluetooth_dev, which will be
used to contain the acpi_device struct and bluetooth status booleans.
This struct will also be used by later patches to store the rfkill
struct as well.
Also, a helper function named toshiba_bluetooth_sync_status was added
to be also used by upcomming patches.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_bluetooth.c | 47 +++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index 2498007..a619ba6 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -34,6 +34,14 @@ MODULE_AUTHOR("Jes Sorensen <[email protected]>");
MODULE_DESCRIPTION("Toshiba Laptop ACPI Bluetooth Enable Driver");
MODULE_LICENSE("GPL");
+struct toshiba_bluetooth_dev {
+ struct acpi_device *acpi_dev;
+
+ bool killswitch;
+ bool plugged;
+ bool powered;
+};
+
static int toshiba_bt_rfkill_add(struct acpi_device *device);
static int toshiba_bt_rfkill_remove(struct acpi_device *device);
static void toshiba_bt_rfkill_notify(struct acpi_device *device, u32 event);
@@ -165,6 +173,25 @@ static int toshiba_bluetooth_disable(acpi_handle handle)
return 0;
}
+/* Helper function */
+static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
+{
+ int status;
+
+ status = toshiba_bluetooth_status(bt_dev->acpi_dev->handle);
+ if (status < 0) {
+ pr_err("Could not sync bluetooth device status\n");
+ return status;
+ }
+
+ bt_dev->killswitch = (status & BT_KILLSWITCH_MASK) ? true : false;
+ bt_dev->plugged = (status & BT_PLUGGED_MASK) ? true : false;
+ bt_dev->powered = (status & BT_POWER_MASK) ? true : false;
+
+ return 0;
+}
+
+/* ACPI driver functions */
static void toshiba_bt_rfkill_notify(struct acpi_device *device, u32 event)
{
toshiba_bluetooth_enable(device->handle);
@@ -179,6 +206,7 @@ static int toshiba_bt_resume(struct device *dev)
static int toshiba_bt_rfkill_add(struct acpi_device *device)
{
+ struct toshiba_bluetooth_dev *bt_dev;
int result;
result = toshiba_bluetooth_present(device->handle);
@@ -187,17 +215,34 @@ static int toshiba_bt_rfkill_add(struct acpi_device *device)
pr_info("Toshiba ACPI Bluetooth device driver\n");
+ bt_dev = kzalloc(sizeof(*bt_dev), GFP_KERNEL);
+ if (!bt_dev)
+ return -ENOMEM;
+ bt_dev->acpi_dev = device;
+ device->driver_data = bt_dev;
+ dev_set_drvdata(&device->dev, bt_dev);
+
+ result = toshiba_bluetooth_sync_status(bt_dev);
+ if (result) {
+ kfree(bt_dev);
+ return result;
+ }
+
/* Enable the BT device */
result = toshiba_bluetooth_enable(device->handle);
if (result)
- return result;
+ kfree(bt_dev);
return result;
}
static int toshiba_bt_rfkill_remove(struct acpi_device *device)
{
+ struct toshiba_bluetooth_dev *bt_dev = acpi_driver_data(device);
+
/* clean up */
+ kfree(bt_dev);
+
return toshiba_bluetooth_disable(device->handle);
}
--
2.3.5
This patch adds RFKill handler functions to the driver, allowing it
to register and update the rfkill switch.
Also, a comment block was moved from the header to the poll function,
as it explains why we need to poll the killswitch on older devices.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_bluetooth.c | 77 ++++++++++++++++++++++++++++----
1 file changed, 68 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index a619ba6..a3b2d38 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -10,12 +10,6 @@
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
- *
- * Note the Toshiba Bluetooth RFKill switch seems to be a strange
- * fish. It only provides a BT event when the switch is flipped to
- * the 'on' position. When flipping it to 'off', the USB device is
- * simply pulled away underneath us, without any BT event being
- * delivered.
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -25,6 +19,7 @@
#include <linux/init.h>
#include <linux/types.h>
#include <linux/acpi.h>
+#include <linux/rfkill.h>
#define BT_KILLSWITCH_MASK 0x01
#define BT_PLUGGED_MASK 0x40
@@ -36,6 +31,7 @@ MODULE_LICENSE("GPL");
struct toshiba_bluetooth_dev {
struct acpi_device *acpi_dev;
+ struct rfkill *rfk;
bool killswitch;
bool plugged;
@@ -191,6 +187,49 @@ static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
return 0;
}
+/* RFKill handlers */
+static int bt_rfkill_set_block(void *data, bool blocked)
+{
+ struct toshiba_bluetooth_dev *bt_dev = data;
+ int ret;
+
+ ret = toshiba_bluetooth_sync_status(bt_dev);
+ if (ret)
+ return ret;
+
+ if (!bt_dev->killswitch)
+ return 0;
+
+ if (blocked)
+ ret = toshiba_bluetooth_disable(bt_dev->acpi_dev->handle);
+ else
+ ret = toshiba_bluetooth_enable(bt_dev->acpi_dev->handle);
+
+ return ret;
+}
+
+static void bt_rfkill_poll(struct rfkill *rfkill, void *data)
+{
+ struct toshiba_bluetooth_dev *bt_dev = data;
+
+ if (toshiba_bluetooth_sync_status(bt_dev))
+ return;
+
+ /*
+ * Note the Toshiba Bluetooth RFKill switch seems to be a strange
+ * fish. It only provides a BT event when the switch is flipped to
+ * the 'on' position. When flipping it to 'off', the USB device is
+ * simply pulled away underneath us, without any BT event being
+ * delivered.
+ */
+ rfkill_set_hw_state(bt_dev->rfk, !bt_dev->killswitch);
+}
+
+static const struct rfkill_ops rfk_ops = {
+ .set_block = bt_rfkill_set_block,
+ .poll = bt_rfkill_poll,
+};
+
/* ACPI driver functions */
static void toshiba_bt_rfkill_notify(struct acpi_device *device, u32 event)
{
@@ -228,10 +267,25 @@ static int toshiba_bt_rfkill_add(struct acpi_device *device)
return result;
}
- /* Enable the BT device */
- result = toshiba_bluetooth_enable(device->handle);
- if (result)
+ bt_dev->rfk = rfkill_alloc("Toshiba Bluetooth",
+ &device->dev,
+ RFKILL_TYPE_BLUETOOTH,
+ &rfk_ops,
+ bt_dev);
+ if (!bt_dev->rfk) {
+ pr_err("Unable to allocate rfkill device\n");
+ kfree(bt_dev);
+ return -ENOMEM;
+ }
+
+ rfkill_set_hw_state(bt_dev->rfk, !bt_dev->killswitch);
+
+ result = rfkill_register(bt_dev->rfk);
+ if (result) {
+ pr_err("Unable to register rfkill device\n");
+ rfkill_destroy(bt_dev->rfk);
kfree(bt_dev);
+ }
return result;
}
@@ -241,6 +295,11 @@ static int toshiba_bt_rfkill_remove(struct acpi_device *device)
struct toshiba_bluetooth_dev *bt_dev = acpi_driver_data(device);
/* clean up */
+ if (bt_dev->rfk) {
+ rfkill_unregister(bt_dev->rfk);
+ rfkill_destroy(bt_dev->rfk);
+ }
+
kfree(bt_dev);
return toshiba_bluetooth_disable(device->handle);
--
2.3.5
This patch removes unneeded code from the toshiba_bluetooth_enable
function as propper rfkill code as been added now.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_bluetooth.c | 27 ---------------------------
1 file changed, 27 deletions(-)
diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index a3b2d38..875ff6c 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -107,33 +107,6 @@ static int toshiba_bluetooth_status(acpi_handle handle)
static int toshiba_bluetooth_enable(acpi_handle handle)
{
acpi_status result;
- bool killswitch;
- bool powered;
- bool plugged;
- int status;
-
- /*
- * Query ACPI to verify RFKill switch is set to 'on'.
- * If not, we return silently, no need to report it as
- * an error.
- */
- status = toshiba_bluetooth_status(handle);
- if (status < 0)
- return status;
-
- killswitch = (status & BT_KILLSWITCH_MASK) ? true : false;
- powered = (status & BT_POWER_MASK) ? true : false;
- plugged = (status & BT_PLUGGED_MASK) ? true : false;
-
- if (!killswitch)
- return 0;
- /*
- * This check ensures to only enable the device if it is powered
- * off or detached, as some recent devices somehow pass the killswitch
- * test, causing a loop enabling/disabling the device, see bug 93911.
- */
- if (powered || plugged)
- return 0;
result = acpi_evaluate_object(handle, "AUSB", NULL, NULL);
if (ACPI_FAILURE(result)) {
--
2.3.5
This patch adapts toshiba_bt_rfkill_notify and toshiba_bt_resume
functions to update the rfkill status, as they were only calling
toshiba_bluetooth_enable.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_bluetooth.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index 875ff6c..9867ccd 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -206,13 +206,29 @@ static const struct rfkill_ops rfk_ops = {
/* ACPI driver functions */
static void toshiba_bt_rfkill_notify(struct acpi_device *device, u32 event)
{
- toshiba_bluetooth_enable(device->handle);
+ struct toshiba_bluetooth_dev *bt_dev = acpi_driver_data(device);
+
+ if (toshiba_bluetooth_sync_status(bt_dev))
+ return;
+
+ rfkill_set_hw_state(bt_dev->rfk, !bt_dev->killswitch);
}
#ifdef CONFIG_PM_SLEEP
static int toshiba_bt_resume(struct device *dev)
{
- return toshiba_bluetooth_enable(to_acpi_device(dev)->handle);
+ struct toshiba_bluetooth_dev *bt_dev;
+ int ret;
+
+ bt_dev = acpi_driver_data(to_acpi_device(dev));
+
+ ret = toshiba_bluetooth_sync_status(bt_dev);
+ if (ret)
+ return ret;
+
+ rfkill_set_hw_state(bt_dev->rfk, !bt_dev->killswitch);
+
+ return 0;
}
#endif
--
2.3.5
The function toshiba_bluetooth_status s currently printing the status
of the device whenever it is queried, but since the introduction of
the rfkill poll code, this value will get printed everytime the poll
occurs.
This patch changes the level of the printed message from info to
debug, and also adds a few more debug messages printing the
killswitch, plug and power status of the device as well.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/toshiba_bluetooth.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index 9867ccd..93b9688 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -99,7 +99,7 @@ static int toshiba_bluetooth_status(acpi_handle handle)
return -ENXIO;
}
- pr_info("Bluetooth status %llu\n", status);
+ pr_debug("Bluetooth status %llu\n", status);
return status;
}
@@ -157,6 +157,10 @@ static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
bt_dev->plugged = (status & BT_PLUGGED_MASK) ? true : false;
bt_dev->powered = (status & BT_POWER_MASK) ? true : false;
+ pr_debug("killswitch %d\n", bt_dev->killswitch);
+ pr_debug("plugged %d\n", bt_dev->plugged);
+ pr_debug("powered %d\n", bt_dev->powered);
+
return 0;
}
--
2.3.5
This patch simply adds the RFKILL dependency to Kconfig, as we are
now using rfkill code on the driver.
Signed-off-by: Azael Avalos <[email protected]>
---
drivers/platform/x86/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 9752761..681b0c5 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -642,6 +642,7 @@ config ACPI_TOSHIBA
config TOSHIBA_BT_RFKILL
tristate "Toshiba Bluetooth RFKill switch support"
depends on ACPI
+ depends on RFKILL || RFKILL = n
---help---
This driver adds support for Bluetooth events for the RFKill
switch on modern Toshiba laptops with full ACPI support and
--
2.3.5
Azael Avalos <[email protected]> writes:
> The function toshiba_bluetooth_status s currently printing the status
> of the device whenever it is queried, but since the introduction of
> the rfkill poll code, this value will get printed everytime the poll
> occurs.
>
> This patch changes the level of the printed message from info to
> debug, and also adds a few more debug messages printing the
> killswitch, plug and power status of the device as well.
>
> Signed-off-by: Azael Avalos <[email protected]>
> ---
> drivers/platform/x86/toshiba_bluetooth.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
> index 9867ccd..93b9688 100644
> --- a/drivers/platform/x86/toshiba_bluetooth.c
> +++ b/drivers/platform/x86/toshiba_bluetooth.c
> @@ -99,7 +99,7 @@ static int toshiba_bluetooth_status(acpi_handle handle)
> return -ENXIO;
> }
>
> - pr_info("Bluetooth status %llu\n", status);
> + pr_debug("Bluetooth status %llu\n", status);
>
> return status;
> }
> @@ -157,6 +157,10 @@ static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
> bt_dev->plugged = (status & BT_PLUGGED_MASK) ? true : false;
> bt_dev->powered = (status & BT_POWER_MASK) ? true : false;
>
> + pr_debug("killswitch %d\n", bt_dev->killswitch);
> + pr_debug("plugged %d\n", bt_dev->plugged);
> + pr_debug("powered %d\n", bt_dev->powered);
Those are terribly generic messages. I don't think I would have guessed
which device was trying to tell me "powered 1" if I found it in the
logs...
How about using e.g dev_dbg() to get a bit more context here?
You might also want to put all three into a single call, so that they
make a single dynamic debug entry when dynamic debugging is enabled.
And looking at toshiba_bluetooth_status() I see that all callers have a
device. How about propagating the device to be able to use the dev_*
printk's there as well? Let the device identify itself instead of
having to guess.
Bjørn
Hi there,
2015-04-28 1:36 GMT-06:00 Bjørn Mork <[email protected]>:
> Azael Avalos <[email protected]> writes:
>
>> The function toshiba_bluetooth_status s currently printing the status
>> of the device whenever it is queried, but since the introduction of
>> the rfkill poll code, this value will get printed everytime the poll
>> occurs.
>>
>> This patch changes the level of the printed message from info to
>> debug, and also adds a few more debug messages printing the
>> killswitch, plug and power status of the device as well.
>>
>> Signed-off-by: Azael Avalos <[email protected]>
>> ---
>> drivers/platform/x86/toshiba_bluetooth.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
>> index 9867ccd..93b9688 100644
>> --- a/drivers/platform/x86/toshiba_bluetooth.c
>> +++ b/drivers/platform/x86/toshiba_bluetooth.c
>> @@ -99,7 +99,7 @@ static int toshiba_bluetooth_status(acpi_handle handle)
>> return -ENXIO;
>> }
>>
>> - pr_info("Bluetooth status %llu\n", status);
>> + pr_debug("Bluetooth status %llu\n", status);
>>
>> return status;
>> }
>> @@ -157,6 +157,10 @@ static int toshiba_bluetooth_sync_status(struct toshiba_bluetooth_dev *bt_dev)
>> bt_dev->plugged = (status & BT_PLUGGED_MASK) ? true : false;
>> bt_dev->powered = (status & BT_POWER_MASK) ? true : false;
>>
>> + pr_debug("killswitch %d\n", bt_dev->killswitch);
>> + pr_debug("plugged %d\n", bt_dev->plugged);
>> + pr_debug("powered %d\n", bt_dev->powered);
>
>
> Those are terribly generic messages. I don't think I would have guessed
> which device was trying to tell me "powered 1" if I found it in the
> logs...
Well, I was under the impression that what really gets printed is:
toshiba_bluetooth: killswitch 1
and then a siple "dmesg | grep toshiba_bluetooth" would suffice,
but yeah, they are quite non obvious.
>
> How about using e.g dev_dbg() to get a bit more context here?
>
> You might also want to put all three into a single call, so that they
> make a single dynamic debug entry when dynamic debugging is enabled.
>
Alright, will do, I'll just wait on Darren's (or someone else) comments and
send a v2.
> And looking at toshiba_bluetooth_status() I see that all callers have a
> device. How about propagating the device to be able to use the dev_*
> printk's there as well? Let the device identify itself instead of
> having to guess.
>
>
> Bjørn
Cheers
Azael
--
-- El mundo apesta y vosotros apestais tambien --
Azael Avalos <[email protected]> writes:
> Well, I was under the impression that what really gets printed is:
> toshiba_bluetooth: killswitch 1
Ah, right. Yes, I missed the
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
you already had in the module. That helps a lot.
Bjørn
On Mon, Apr 27, 2015 at 02:32:47PM -0600, Azael Avalos wrote:
> This patch removes unneeded code from the toshiba_bluetooth_enable
> function as propper rfkill code as been added now.
>
Seems like this should just be part of 2 of 3 since it essentially moves the
status code?
--
Darren Hart
Intel Open Source Technology Center
On Mon, Apr 27, 2015 at 02:32:50PM -0600, Azael Avalos wrote:
> This patch simply adds the RFKILL dependency to Kconfig, as we are
> now using rfkill code on the driver.
This should be added at the same time, or before, the dependency was added - not
after as it leaves a small window of broken dependencies in the history.
--
Darren Hart
Intel Open Source Technology Center
On Mon, Apr 27, 2015 at 02:32:49PM -0600, Azael Avalos wrote:
> The function toshiba_bluetooth_status s currently printing the status
A nit since there will be v2: ^ is
--
Darren Hart
Intel Open Source Technology Center