All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver
@ 2015-04-27 20:32 Azael Avalos
  2015-04-27 20:32 ` [PATCH 1/6] toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev Azael Avalos
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Azael Avalos @ 2015-04-27 20:32 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/6] toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev
  2015-04-27 20:32 [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver Azael Avalos
@ 2015-04-27 20:32 ` Azael Avalos
  2015-04-27 20:32 ` [PATCH 2/6] toshiba_bluetooth: Add RFKill handler functions Azael Avalos
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Azael Avalos @ 2015-04-27 20:32 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

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 <coproscefalo@gmail.com>
---
 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 <Jes.Sorensen@gmail.com>");
 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/6] toshiba_bluetooth: Add RFKill handler functions
  2015-04-27 20:32 [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver Azael Avalos
  2015-04-27 20:32 ` [PATCH 1/6] toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev Azael Avalos
@ 2015-04-27 20:32 ` Azael Avalos
  2015-04-27 20:32 ` [PATCH 3/6] toshiba_bluetooth: Clean toshiba_bluetooth_enable function Azael Avalos
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Azael Avalos @ 2015-04-27 20:32 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

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 <coproscefalo@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/6] toshiba_bluetooth: Clean toshiba_bluetooth_enable function
  2015-04-27 20:32 [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver Azael Avalos
  2015-04-27 20:32 ` [PATCH 1/6] toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev Azael Avalos
  2015-04-27 20:32 ` [PATCH 2/6] toshiba_bluetooth: Add RFKill handler functions Azael Avalos
@ 2015-04-27 20:32 ` Azael Avalos
  2015-05-03 19:43   ` Darren Hart
  2015-04-27 20:32 ` [PATCH 4/6] toshiba_bluetooth: Adapt *_notify and *_resume functions to rfkill Azael Avalos
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Azael Avalos @ 2015-04-27 20:32 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

This patch removes unneeded code from the toshiba_bluetooth_enable
function as propper rfkill code as been added now.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/6] toshiba_bluetooth: Adapt *_notify and *_resume functions to rfkill
  2015-04-27 20:32 [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver Azael Avalos
                   ` (2 preceding siblings ...)
  2015-04-27 20:32 ` [PATCH 3/6] toshiba_bluetooth: Clean toshiba_bluetooth_enable function Azael Avalos
@ 2015-04-27 20:32 ` Azael Avalos
  2015-04-27 20:32 ` [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug Azael Avalos
  2015-04-27 20:32 ` [PATCH 6/6] platform/x86: Add RFKILL dependency to toshiba_bluetooth driver Azael Avalos
  5 siblings, 0 replies; 15+ messages in thread
From: Azael Avalos @ 2015-04-27 20:32 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

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 <coproscefalo@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug
  2015-04-27 20:32 [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver Azael Avalos
                   ` (3 preceding siblings ...)
  2015-04-27 20:32 ` [PATCH 4/6] toshiba_bluetooth: Adapt *_notify and *_resume functions to rfkill Azael Avalos
@ 2015-04-27 20:32 ` Azael Avalos
  2015-04-28  7:36   ` Bjørn Mork
  2015-05-03 19:49   ` Darren Hart
  2015-04-27 20:32 ` [PATCH 6/6] platform/x86: Add RFKILL dependency to toshiba_bluetooth driver Azael Avalos
  5 siblings, 2 replies; 15+ messages in thread
From: Azael Avalos @ 2015-04-27 20:32 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

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 <coproscefalo@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/6] platform/x86: Add RFKILL dependency to toshiba_bluetooth driver
  2015-04-27 20:32 [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver Azael Avalos
                   ` (4 preceding siblings ...)
  2015-04-27 20:32 ` [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug Azael Avalos
@ 2015-04-27 20:32 ` Azael Avalos
  2015-05-03 19:47   ` Darren Hart
  5 siblings, 1 reply; 15+ messages in thread
From: Azael Avalos @ 2015-04-27 20:32 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

This patch simply adds the RFKILL dependency to Kconfig, as we are
now using rfkill code on the driver.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug
  2015-04-27 20:32 ` [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug Azael Avalos
@ 2015-04-28  7:36   ` Bjørn Mork
  2015-04-28 15:08       ` Azael Avalos
  2015-05-03 19:49   ` Darren Hart
  1 sibling, 1 reply; 15+ messages in thread
From: Bjørn Mork @ 2015-04-28  7:36 UTC (permalink / raw)
  To: Azael Avalos; +Cc: Darren Hart, platform-driver-x86, linux-kernel

Azael Avalos <coproscefalo@gmail.com> 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 <coproscefalo@gmail.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug
  2015-04-28  7:36   ` Bjørn Mork
@ 2015-04-28 15:08       ` Azael Avalos
  0 siblings, 0 replies; 15+ messages in thread
From: Azael Avalos @ 2015-04-28 15:08 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Darren Hart, platform-driver-x86, linux-kernel

Hi there,

2015-04-28 1:36 GMT-06:00 Bjørn Mork <bjorn@mork.no>:
> Azael Avalos <coproscefalo@gmail.com> 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 <coproscefalo@gmail.com>
>> ---
>>  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 --

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug
@ 2015-04-28 15:08       ` Azael Avalos
  0 siblings, 0 replies; 15+ messages in thread
From: Azael Avalos @ 2015-04-28 15:08 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Darren Hart, platform-driver-x86, linux-kernel

Hi there,

2015-04-28 1:36 GMT-06:00 Bjørn Mork <bjorn@mork.no>:
> Azael Avalos <coproscefalo@gmail.com> 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 <coproscefalo@gmail.com>
>> ---
>>  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 --

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug
  2015-04-28 15:08       ` Azael Avalos
@ 2015-04-28 15:16         ` Bjørn Mork
  -1 siblings, 0 replies; 15+ messages in thread
From: Bjørn Mork @ 2015-04-28 15:16 UTC (permalink / raw)
  To: Azael Avalos; +Cc: Darren Hart, platform-driver-x86, linux-kernel

Azael Avalos <coproscefalo@gmail.com> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug
@ 2015-04-28 15:16         ` Bjørn Mork
  0 siblings, 0 replies; 15+ messages in thread
From: Bjørn Mork @ 2015-04-28 15:16 UTC (permalink / raw)
  To: Azael Avalos; +Cc: Darren Hart, platform-driver-x86, linux-kernel

Azael Avalos <coproscefalo@gmail.com> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/6] toshiba_bluetooth: Clean toshiba_bluetooth_enable function
  2015-04-27 20:32 ` [PATCH 3/6] toshiba_bluetooth: Clean toshiba_bluetooth_enable function Azael Avalos
@ 2015-05-03 19:43   ` Darren Hart
  0 siblings, 0 replies; 15+ messages in thread
From: Darren Hart @ 2015-05-03 19:43 UTC (permalink / raw)
  To: Azael Avalos; +Cc: platform-driver-x86, linux-kernel

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] platform/x86: Add RFKILL dependency to toshiba_bluetooth driver
  2015-04-27 20:32 ` [PATCH 6/6] platform/x86: Add RFKILL dependency to toshiba_bluetooth driver Azael Avalos
@ 2015-05-03 19:47   ` Darren Hart
  0 siblings, 0 replies; 15+ messages in thread
From: Darren Hart @ 2015-05-03 19:47 UTC (permalink / raw)
  To: Azael Avalos; +Cc: platform-driver-x86, linux-kernel

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug
  2015-04-27 20:32 ` [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug Azael Avalos
  2015-04-28  7:36   ` Bjørn Mork
@ 2015-05-03 19:49   ` Darren Hart
  1 sibling, 0 replies; 15+ messages in thread
From: Darren Hart @ 2015-05-03 19:49 UTC (permalink / raw)
  To: Azael Avalos; +Cc: platform-driver-x86, linux-kernel

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-05-03 19:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-27 20:32 [PATCH 0/6] toshiba_bluetooth: Add rfkill code to driver Azael Avalos
2015-04-27 20:32 ` [PATCH 1/6] toshiba_bluetooth: Add a container struct named toshiba_bluetooth_dev Azael Avalos
2015-04-27 20:32 ` [PATCH 2/6] toshiba_bluetooth: Add RFKill handler functions Azael Avalos
2015-04-27 20:32 ` [PATCH 3/6] toshiba_bluetooth: Clean toshiba_bluetooth_enable function Azael Avalos
2015-05-03 19:43   ` Darren Hart
2015-04-27 20:32 ` [PATCH 4/6] toshiba_bluetooth: Adapt *_notify and *_resume functions to rfkill Azael Avalos
2015-04-27 20:32 ` [PATCH 5/6] toshiba_bluetooth: Change BT status message to debug Azael Avalos
2015-04-28  7:36   ` Bjørn Mork
2015-04-28 15:08     ` Azael Avalos
2015-04-28 15:08       ` Azael Avalos
2015-04-28 15:16       ` Bjørn Mork
2015-04-28 15:16         ` Bjørn Mork
2015-05-03 19:49   ` Darren Hart
2015-04-27 20:32 ` [PATCH 6/6] platform/x86: Add RFKILL dependency to toshiba_bluetooth driver Azael Avalos
2015-05-03 19:47   ` Darren Hart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.