[PATCH v3 0/3] toshiba_bluetooth: Clean up driver plus a bugfix

[PATCH v3 0/3] toshiba_bluetooth: Clean up driver plus a bugfix

[Azael Avalos]

These patches introduce three new functions, cleaning up the driver
code, checking for errors and returning accordingly, and also fixes
bug 93911.

Changes since v2:
- Merged patches 2-3 into one
- Added a comment block explaining a bit why we added an extra check
  to the status of the device

Changes since v1:
- Removed pr_info calls from *_notify function
- Added checks to the status of the BT device removing the TODO
  comment and retaining the previous functionality of the driver

Azael Avalos (3):
  toshiba_bluetooth: Add three new functions to the driver
  toshiba_bluetooth: Clean up *_add function and disable BT device at
    removal
  toshiba_bluetooth: Fix enabling/disabling loop on recent devices

 drivers/platform/x86/toshiba_bluetooth.c | 133 ++++++++++++++++++++++++-------
 1 file changed, 104 insertions(+), 29 deletions(-)

-- 
2.2.2

[PATCH v3 1/3] toshiba_bluetooth: Add three new functions to the driver

[Azael Avalos]

This patch introduces three new functions, which are going to be used
by the next patches.

The functions introduced are toshiba_bluetooth_present,
toshiba_bluetooth_status and toshiba_bluetooth_disable, which queries
the presence of the device, queries the status and disables the
device respectively.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_bluetooth.c | 51 ++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index 2cb1ea6..b479a70 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -57,6 +57,38 @@ static struct acpi_driver toshiba_bt_rfkill_driver = {
 	.drv.pm =	&toshiba_bt_pm,
 };
 
+static int toshiba_bluetooth_present(acpi_handle handle)
+{
+	acpi_status result;
+	u64 bt_present;
+
+	result = acpi_evaluate_integer(handle, "_STA", NULL, &bt_present);
+	if (ACPI_FAILURE(result)) {
+		pr_err("ACPI call to query Bluetooth presence failed");
+		return -ENXIO;
+	} else if (!bt_present) {
+		pr_info("Bluetooth device not present\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int toshiba_bluetooth_status(acpi_handle handle)
+{
+	acpi_status result;
+	u64 status;
+
+	result = acpi_evaluate_integer(handle, "BTST", NULL, &status);
+	if (ACPI_FAILURE(result)) {
+		pr_err("Could not get Bluetooth device status\n");
+		return -ENXIO;
+	}
+
+	pr_info("Bluetooth status %llu\n", status);
+
+	return status;
+}
 
 static int toshiba_bluetooth_enable(acpi_handle handle)
 {
@@ -85,6 +117,25 @@ static int toshiba_bluetooth_enable(acpi_handle handle)
 	return -ENODEV;
 }
 
+static int toshiba_bluetooth_disable(acpi_handle handle)
+{
+	acpi_status result;
+
+	result = acpi_evaluate_object(handle, "BTPF", NULL, NULL);
+	if (ACPI_FAILURE(result)) {
+		pr_err("Could not power OFF Bluetooth device\n");
+		return -ENXIO;
+	}
+
+	result = acpi_evaluate_object(handle, "DUSB", NULL, NULL);
+	if (ACPI_FAILURE(result)) {
+		pr_err("Could not detach USB Bluetooth device\n");
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
 static void toshiba_bt_rfkill_notify(struct acpi_device *device, u32 event)
 {
 	toshiba_bluetooth_enable(device->handle);
-- 
2.2.2

[PATCH v3 2/3] toshiba_bluetooth: Clean up *_add function and disable BT device at removal

[Azael Avalos]

This patch cleans the toshiba_bluetooth_add function by using the
recently introduced function toshiba_bluetooth_present, simplifying
its code and returning appropriate error values.

Also, disables the BT device at the removal of the driver, by using
the function toshiba_bluetooth_disable.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_bluetooth.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index b479a70..b8404c7 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -62,6 +62,11 @@ static int toshiba_bluetooth_present(acpi_handle handle)
 	acpi_status result;
 	u64 bt_present;
 
+	/*
+	 * Some Toshiba laptops may have a fake TOS6205 device in
+	 * their ACPI BIOS, so query the _STA method to see if there
+	 * is really anything there.
+	 */
 	result = acpi_evaluate_integer(handle, "_STA", NULL, &bt_present);
 	if (ACPI_FAILURE(result)) {
 		pr_err("ACPI call to query Bluetooth presence failed");
@@ -150,23 +155,18 @@ static int toshiba_bt_resume(struct device *dev)
 
 static int toshiba_bt_rfkill_add(struct acpi_device *device)
 {
-	acpi_status status;
-	u64 bt_present;
-	int result = -ENODEV;
+	int result;
 
-	/*
-	 * Some Toshiba laptops may have a fake TOS6205 device in
-	 * their ACPI BIOS, so query the _STA method to see if there
-	 * is really anything there, before trying to enable it.
-	 */
-	status = acpi_evaluate_integer(device->handle, "_STA", NULL,
-				       &bt_present);
+	result = toshiba_bluetooth_present(device->handle);
+	if (result)
+		return result;
 
-	if (!ACPI_FAILURE(status) && bt_present) {
-		pr_info("Detected Toshiba ACPI Bluetooth device - "
-			"installing RFKill handler\n");
-		result = toshiba_bluetooth_enable(device->handle);
-	}
+	pr_info("Toshiba ACPI Bluetooth device driver\n");
+
+	/* Enable the BT device */
+	result = toshiba_bluetooth_enable(device->handle);
+	if (result)
+		return result;
 
 	return result;
 }
@@ -174,7 +174,7 @@ static int toshiba_bt_rfkill_add(struct acpi_device *device)
 static int toshiba_bt_rfkill_remove(struct acpi_device *device)
 {
 	/* clean up */
-	return 0;
+	return toshiba_bluetooth_disable(device->handle);
 }
 
 module_acpi_driver(toshiba_bt_rfkill_driver);
-- 
2.2.2

[PATCH v3 3/3] toshiba_bluetooth: Fix enabling/disabling loop on recent devices

[Azael Avalos]

Bug 93911 reported a broken handling of the BT device, causing the
driver to get stuck in a loop enabling/disabling the device whenever
the device is deactivated by the kill switch as follows:

1. The user activated the kill switch, causing the system to generate
   a 0x90 (status change) event and disabling the BT device.
2. The driver catches the event and re-enables the BT device.
3. The system detects the device being activated, but since the kill
   switch is activated, disables the BT device (again) and generates
   a 0x90 event (again).
4. Repeat from 2.

This patch adds an extra check to verify the status of the BT device,
returning silently if it is already activated.

Also, checks and returns appropriate error values while evaluating
the AUSB and BTPO methods.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_bluetooth.c | 50 +++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index b8404c7..2498007 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -2,6 +2,7 @@
  * Toshiba Bluetooth Enable Driver
  *
  * Copyright (C) 2009 Jes Sorensen <Jes.Sorensen@gmail.com>
+ * Copyright (C) 2015 Azael Avalos <coproscefalo@gmail.com>
  *
  * Thanks to Matthew Garrett for background info on ACPI innards which
  * normal people aren't meant to understand :-)
@@ -25,6 +26,10 @@
 #include <linux/types.h>
 #include <linux/acpi.h>
 
+#define BT_KILLSWITCH_MASK	0x01
+#define BT_PLUGGED_MASK		0x40
+#define BT_POWER_MASK		0x80
+
 MODULE_AUTHOR("Jes Sorensen <Jes.Sorensen@gmail.com>");
 MODULE_DESCRIPTION("Toshiba Laptop ACPI Bluetooth Enable Driver");
 MODULE_LICENSE("GPL");
@@ -97,29 +102,48 @@ static int toshiba_bluetooth_status(acpi_handle handle)
 
 static int toshiba_bluetooth_enable(acpi_handle handle)
 {
-	acpi_status res1, res2;
-	u64 result;
+	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.
 	 */
-	res1 = acpi_evaluate_integer(handle, "BTST", NULL, &result);
-	if (ACPI_FAILURE(res1))
-		return res1;
-	if (!(result & 0x01))
-		return 0;
+	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;
 
-	pr_info("Re-enabling Toshiba Bluetooth\n");
-	res1 = acpi_evaluate_object(handle, "AUSB", NULL, NULL);
-	res2 = acpi_evaluate_object(handle, "BTPO", NULL, NULL);
-	if (!ACPI_FAILURE(res1) || !ACPI_FAILURE(res2))
+	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;
 
-	pr_warn("Failed to re-enable Toshiba Bluetooth\n");
+	result = acpi_evaluate_object(handle, "AUSB", NULL, NULL);
+	if (ACPI_FAILURE(result)) {
+		pr_err("Could not attach USB Bluetooth device\n");
+		return -ENXIO;
+	}
 
-	return -ENODEV;
+	result = acpi_evaluate_object(handle, "BTPO", NULL, NULL);
+	if (ACPI_FAILURE(result)) {
+		pr_err("Could not power ON Bluetooth device\n");
+		return -ENXIO;
+	}
+
+	return 0;
 }
 
 static int toshiba_bluetooth_disable(acpi_handle handle)
-- 
2.2.2

Re: [PATCH v3 0/3] toshiba_bluetooth: Clean up driver plus a bugfix

[Darren Hart]

On Thu, Mar 26, 2015 at 02:56:04PM -0600, Azael Avalos wrote:
> These patches introduce three new functions, cleaning up the driver
> code, checking for errors and returning accordingly, and also fixes
> bug 93911.
> 

Queued, thank you Azael.

-- 
Darren Hart
Intel Open Source Technology Center