[PATCH 00/12] Bluetooth: Asynchronous HCI transaction API

[PATCH 00/12] Bluetooth: Asynchronous HCI transaction API

[Johan Hedberg]

Hi,

There are several places in the kernel code where a way of grouping
together commands, sending them asynchronously and getting notified when
they've completed (or a command failed) would be very useful. We already
have the hci_request() API which allows doing this in a blocking fashion
but there's nothing for asynchronous needs.

One example is a long outstanding bug with powering adapters on through
the Management API where some HCI commands are sent after we already
replied with "done" to user space. These commands need to be monitored
in some clean way for completion but so far there hasn't been any good
API for that.

This patch set introduces an API for doing this kind of asynchronous
operations (called "transactions" in the code). It also contains
cleanups enabled by this API such as converting the hci_request()
implementation to use transactions (e.g. removing the hci_req_done()
public function) and unnecessary HCI event callback handlers.

Patch 07/12 is the first example of an actual fix enabled by this new
API, fixing the mgmt bug described above. When applied the 1 second
artificial delay in the user space mgmt-tester can be removed.

I've tested this patch set with various adapters, ranging from old 1.1
ones to LE capable 4.0 ones. I've also tested that legacy APIs such as
the ioctls for doing inquiry and other operations still work as they
should, as well as that device discovery, pairing and connecting still
works fine through bluetoothd. This still doesn't guarantee that there
aren't bugs so any review/testing/feedback is welcome!

Johan Hedberg (12):
      Bluetooth: Add initial hooks for HCI transaction support
      Bluetooth: Add basic start/complete HCI transaction functions
      Bluetooth: Add hci_transaction_cmd_complete function
      Bluetooth: Add hci_transaction_from_skb function
      Bluetooth: Switch from hdev->cmd_q to using transactions
      Bluetooth: Remove unused hdev->cmd_q HCI command queue
      Bluetooth: Fix mgmt powered indication by using a HCI transaction
      Bluetooth: Enable HCI transaction support cmd_status 0
      Bluetooth: Add HCI init sequence support for HCI transactions
      Bluetooth: Convert hci_request to use HCI transactions
      Bluetooth: Remove unused hdev->init_last_cmd
      Bluetooth: Remove empty HCI event handlers

 include/net/bluetooth/hci_core.h |   30 +++++--
 net/bluetooth/hci_core.c         |  391 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 net/bluetooth/hci_event.c        |  225 ++++-------------------------------------------------
 net/bluetooth/hci_sock.c         |    5 +-
 net/bluetooth/mgmt.c             |   43 ++++++++---
 5 files changed, 424 insertions(+), 270 deletions(-)

Johan

[PATCH 01/12] Bluetooth: Add initial hooks for HCI transaction support

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds the initial context variables and functions for HCI
transaction support. HCI transactions are essentially just groups of HCI
commands with an optional callback for notifying the completion of the
transaction.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |   17 +++++++++++++++++
 net/bluetooth/hci_core.c         |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 90cf75a..0e53032 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -134,6 +134,15 @@ struct amp_assoc {
 	__u8	data[HCI_MAX_AMP_ASSOC_SIZE];
 };
 
+struct hci_dev;
+
+struct hci_transaction {
+	struct list_head	list;
+	struct sk_buff_head	cmd_q;
+	void			(*complete)(struct hci_dev *hdev,
+					    __u16 last_cmd, int hci_err);
+};
+
 #define NUM_REASSEMBLY 4
 struct hci_dev {
 	struct list_head list;
@@ -240,6 +249,11 @@ struct hci_dev {
 	struct sk_buff_head	raw_q;
 	struct sk_buff_head	cmd_q;
 
+	struct mutex		transaction_lock;
+	struct hci_transaction	*build_transaction;
+	struct hci_transaction	*current_transaction;
+	struct list_head	transaction_q;
+
 	struct sk_buff		*sent_cmd;
 	struct sk_buff		*reassembly[NUM_REASSEMBLY];
 
@@ -1041,6 +1055,9 @@ static inline u16 eir_append_data(u8 *eir, u16 eir_len, u8 type, u8 *data,
 int hci_register_cb(struct hci_cb *hcb);
 int hci_unregister_cb(struct hci_cb *hcb);
 
+#define hci_transaction_lock(d)		mutex_lock(&d->transaction_lock)
+#define hci_transaction_unlock(d)	mutex_unlock(&d->transaction_lock)
+
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
 void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
 void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 22e77a7..05e2e8b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -689,6 +689,36 @@ unlock:
 	return err;
 }
 
+static void __transaction_free(struct hci_transaction *transaction)
+{
+	skb_queue_purge(&transaction->cmd_q);
+	kfree(transaction);
+}
+
+static void hci_transaction_cleanup(struct hci_dev *hdev)
+{
+	struct hci_transaction *transact, *tmp;
+
+	hci_transaction_lock(hdev);
+
+	if (hdev->build_transaction) {
+		__transaction_free(hdev->build_transaction);
+		hdev->build_transaction = NULL;
+	}
+
+	if (hdev->current_transaction) {
+		__transaction_free(hdev->current_transaction);
+		hdev->current_transaction = NULL;
+	}
+
+	list_for_each_entry_safe(transact, tmp, &hdev->transaction_q, list) {
+		list_del(&transact->list);
+		__transaction_free(transact);
+	}
+
+	hci_transaction_unlock(hdev);
+}
+
 /* ---- HCI ioctl helpers ---- */
 
 int hci_dev_open(__u16 dev)
@@ -759,6 +789,7 @@ int hci_dev_open(__u16 dev)
 		flush_work(&hdev->cmd_work);
 		flush_work(&hdev->rx_work);
 
+		hci_transaction_cleanup(hdev);
 		skb_queue_purge(&hdev->cmd_q);
 		skb_queue_purge(&hdev->rx_q);
 
@@ -823,6 +854,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 		hdev->flush(hdev);
 
 	/* Reset device */
+	hci_transaction_cleanup(hdev);
 	skb_queue_purge(&hdev->cmd_q);
 	atomic_set(&hdev->cmd_cnt, 1);
 	if (!test_bit(HCI_RAW, &hdev->flags) &&
@@ -838,6 +870,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	/* Drop queues */
 	skb_queue_purge(&hdev->rx_q);
 	skb_queue_purge(&hdev->cmd_q);
+	hci_transaction_cleanup(hdev);
 	skb_queue_purge(&hdev->raw_q);
 
 	/* Drop last sent command */
@@ -908,6 +941,7 @@ int hci_dev_reset(__u16 dev)
 	/* Drop queues */
 	skb_queue_purge(&hdev->rx_q);
 	skb_queue_purge(&hdev->cmd_q);
+	hci_transaction_cleanup(hdev);
 
 	hci_dev_lock(hdev);
 	inquiry_cache_flush(hdev);
@@ -1710,6 +1744,7 @@ struct hci_dev *hci_alloc_dev(void)
 
 	mutex_init(&hdev->lock);
 	mutex_init(&hdev->req_lock);
+	mutex_init(&hdev->transaction_lock);
 
 	INIT_LIST_HEAD(&hdev->mgmt_pending);
 	INIT_LIST_HEAD(&hdev->blacklist);
@@ -1718,6 +1753,7 @@ struct hci_dev *hci_alloc_dev(void)
 	INIT_LIST_HEAD(&hdev->long_term_keys);
 	INIT_LIST_HEAD(&hdev->remote_oob_data);
 	INIT_LIST_HEAD(&hdev->conn_hash.list);
+	INIT_LIST_HEAD(&hdev->transaction_q);
 
 	INIT_WORK(&hdev->rx_work, hci_rx_work);
 	INIT_WORK(&hdev->cmd_work, hci_cmd_work);
-- 
1.7.10.4

[PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

With these functions it will be possible to declare the start and end of
a transaction. hci_start_transaction() creates hdev->build_transaction
which will be used by hci_send_command() to construct a transaction.
hci_complete_transaction() indicates the end of a transaction with an
optional complete callback to be called once the transaction is
complete. The transaction is either moved to hdev->current_transaction
(if no other transaction is in progress) or appended to
hdev->transaction_q.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    5 +++
 net/bluetooth/hci_core.c         |   81 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0e53032..5cd58f5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1058,6 +1058,11 @@ int hci_unregister_cb(struct hci_cb *hcb);
 #define hci_transaction_lock(d)		mutex_lock(&d->transaction_lock)
 #define hci_transaction_unlock(d)	mutex_unlock(&d->transaction_lock)
 
+int hci_start_transaction(struct hci_dev *hdev);
+int hci_complete_transaction(struct hci_dev *hdev,
+			     void (*complete)(struct hci_dev *hdev,
+					      __u16 last_cmd, int status));
+
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
 void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
 void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 05e2e8b..13c064e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2196,6 +2196,87 @@ static int hci_send_frame(struct sk_buff *skb)
 	return hdev->send(skb);
 }
 
+int hci_start_transaction(struct hci_dev *hdev)
+{
+	struct hci_transaction *transaction;
+	int err;
+
+	hci_transaction_lock(hdev);
+
+	/* We can't start a new transaction if there's another one in
+	 * progress of being built.
+	 */
+	if (hdev->build_transaction) {
+		err = -EBUSY;
+		goto unlock;
+	}
+
+	transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
+	if (!transaction) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	memset(transaction, 0, sizeof(*transaction));
+	skb_queue_head_init(&transaction->cmd_q);
+
+	hdev->build_transaction = transaction;
+
+	err = 0;
+
+unlock:
+	hci_transaction_unlock(hdev);
+
+	return err;
+}
+
+static void __transaction_add(struct hci_dev *hdev,
+			      struct hci_transaction *transaction)
+{
+	if (hdev->current_transaction || !list_empty(&hdev->transaction_q)) {
+		list_add_tail(&transaction->list, &hdev->transaction_q);
+	} else {
+		hdev->current_transaction = transaction;
+		queue_work(hdev->workqueue, &hdev->cmd_work);
+	}
+}
+
+int hci_complete_transaction(struct hci_dev *hdev,
+			     void (*complete)(struct hci_dev *hdev,
+					      __u16 last_cmd, int status))
+{
+	struct hci_transaction *transaction;
+	int err;
+
+	hci_transaction_lock(hdev);
+
+	transaction = hdev->build_transaction;
+	if (!transaction) {
+		err = -EINVAL;
+		goto unlock;
+	}
+
+	hdev->build_transaction = NULL;
+
+	/* Do not allow empty transactions */
+	if (skb_queue_empty(&transaction->cmd_q)) {
+		__transaction_free(transaction);
+		err = -EINVAL;
+		goto unlock;
+	}
+
+	transaction->complete = complete;
+
+	__transaction_add(hdev, transaction);
+
+	err = 0;
+
+unlock:
+	hci_transaction_unlock(hdev);
+
+	return err;
+}
+
 /* Send HCI command */
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 {
-- 
1.7.10.4

[PATCH 03/12] Bluetooth: Add hci_transaction_cmd_complete function

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This function is used to process the HCI transaction state, including
thigs like picking the next command to send, calling the complete
callback for the current transaction and moving the next transaction
from the queue (if any) to hdev->current_transaction.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_core.c         |   65 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5cd58f5..54efaa2 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1062,6 +1062,7 @@ int hci_start_transaction(struct hci_dev *hdev);
 int hci_complete_transaction(struct hci_dev *hdev,
 			     void (*complete)(struct hci_dev *hdev,
 					      __u16 last_cmd, int status));
+bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
 
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
 void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 13c064e..dec84a1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2977,6 +2977,71 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	kfree_skb(skb);
 }
 
+static void __transaction_next(struct hci_dev *hdev, u16 opcode, int status)
+{
+	struct hci_transaction *transaction;
+
+	transaction = hdev->current_transaction;
+	if (!transaction)
+		goto next_in_queue;
+
+	if (status || skb_queue_empty(&transaction->cmd_q)) {
+		hdev->current_transaction = NULL;
+
+		/* We need to give up the transaction lock temporarily
+		 * since the complete callback might trigger a deadlock
+		 */
+		hci_transaction_unlock(hdev);
+		if (transaction->complete)
+			transaction->complete(hdev, opcode, status);
+		__transaction_free(transaction);
+		hci_transaction_lock(hdev);
+
+		transaction = hdev->current_transaction;
+	}
+
+	if (transaction)
+		return;
+
+next_in_queue:
+	if (list_empty(&hdev->transaction_q))
+		return;
+
+	transaction = list_first_entry(&hdev->transaction_q,
+				       struct hci_transaction, list);
+	if (transaction)
+		list_del(&transaction->list);
+
+	hdev->current_transaction = transaction;
+}
+
+bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status)
+{
+	bool queue_empty;
+
+	/* Ignore this event if it doesn't match the last HCI command
+	 * that was sent
+	 */
+	if (!hci_sent_cmd_data(hdev, opcode))
+		return false;
+
+	hci_transaction_lock(hdev);
+
+	__transaction_next(hdev, opcode, status);
+
+	if (!hdev->current_transaction) {
+		queue_empty = true;
+		goto unlock;
+	}
+
+	queue_empty = skb_queue_empty(&hdev->current_transaction->cmd_q);
+
+unlock:
+	hci_transaction_unlock(hdev);
+
+	return queue_empty;
+}
+
 static void hci_rx_work(struct work_struct *work)
 {
 	struct hci_dev *hdev = container_of(work, struct hci_dev, rx_work);
-- 
1.7.10.4

[PATCH 04/12] Bluetooth: Add hci_transaction_from_skb function

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This function is needed for situations where a transaction needs to be
constructed straight from an skb. This happens e.g. when going through
the HCI driver initialization commands in hci_init_req() or when
receiving commands from user space through a raw HCI socket.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_core.c         |   29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 54efaa2..b5c6f99 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1063,6 +1063,7 @@ int hci_complete_transaction(struct hci_dev *hdev,
 			     void (*complete)(struct hci_dev *hdev,
 					      __u16 last_cmd, int status));
 bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
+int hci_transaction_from_skb(struct hci_dev *hdev, struct sk_buff *skb);
 
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
 void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dec84a1..72abf9c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2277,6 +2277,35 @@ unlock:
 	return err;
 }
 
+static int __transaction_from_skb(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_transaction *transaction;
+
+	transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
+	if (!transaction)
+		return -ENOMEM;
+
+	memset(transaction, 0, sizeof(*transaction));
+	skb_queue_head_init(&transaction->cmd_q);
+
+	skb_queue_tail(&transaction->cmd_q, skb);
+
+	__transaction_add(hdev, transaction);
+
+	return 0;
+}
+
+int hci_transaction_from_skb(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	int err;
+
+	hci_transaction_lock(hdev);
+	err = __transaction_from_skb(hdev, skb);
+	hci_transaction_unlock(hdev);
+
+	return err;
+}
+
 /* Send HCI command */
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 {
-- 
1.7.10.4

[PATCH 05/12] Bluetooth: Switch from hdev->cmd_q to using transactions

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch converts the code from using a single hdev->cmd_q HCI command
queue to use the HCI transaction infrastructure.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_core.c  |   87 ++++++++++++++++++++++++++++++++++++++-------
 net/bluetooth/hci_event.c |   11 ++++--
 net/bluetooth/hci_sock.c  |    5 +--
 3 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 72abf9c..2c0ad61 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -80,10 +80,18 @@ void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
 			return;
 
 		skb = skb_clone(hdev->sent_cmd, GFP_ATOMIC);
+		hci_transaction_lock(hdev);
 		if (skb) {
-			skb_queue_head(&hdev->cmd_q, skb);
-			queue_work(hdev->workqueue, &hdev->cmd_work);
+			struct hci_transaction *transaction =
+						hdev->current_transaction;
+			if (transaction) {
+				skb_queue_head(&transaction->cmd_q, skb);
+				queue_work(hdev->workqueue, &hdev->cmd_work);
+			} else {
+				kfree_skb(skb);
+			}
 		}
+		hci_transaction_unlock(hdev);
 
 		return;
 	}
@@ -203,22 +211,30 @@ static void amp_init(struct hci_dev *hdev)
 
 static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 {
+	struct hci_transaction *transaction;
 	struct sk_buff *skb;
 
 	BT_DBG("%s %ld", hdev->name, opt);
 
 	/* Driver initialization */
 
+	if (hci_start_transaction(hdev) < 0)
+		return;
+
+	hci_transaction_lock(hdev);
+
+	transaction = hdev->build_transaction;
+
 	/* Special commands */
 	while ((skb = skb_dequeue(&hdev->driver_init))) {
 		bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
 		skb->dev = (void *) hdev;
-
-		skb_queue_tail(&hdev->cmd_q, skb);
-		queue_work(hdev->workqueue, &hdev->cmd_work);
+		skb_queue_tail(&transaction->cmd_q, skb);
 	}
 	skb_queue_purge(&hdev->driver_init);
 
+	hci_transaction_unlock(hdev);
+
 	/* Reset */
 	if (!test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks))
 		hci_reset_req(hdev, 0);
@@ -236,6 +252,8 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 		BT_ERR("Unknown device type %d", hdev->dev_type);
 		break;
 	}
+
+	hci_complete_transaction(hdev, NULL);
 }
 
 static void hci_scan_req(struct hci_dev *hdev, unsigned long opt)
@@ -2312,6 +2330,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 	int len = HCI_COMMAND_HDR_SIZE + plen;
 	struct hci_command_hdr *hdr;
 	struct sk_buff *skb;
+	int err;
 
 	BT_DBG("%s opcode 0x%4.4x plen %d", hdev->name, opcode, plen);
 
@@ -2336,10 +2355,41 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 	if (test_bit(HCI_INIT, &hdev->flags))
 		hdev->init_last_cmd = opcode;
 
-	skb_queue_tail(&hdev->cmd_q, skb);
-	queue_work(hdev->workqueue, &hdev->cmd_work);
+	err = 0;
 
-	return 0;
+	hci_transaction_lock(hdev);
+
+	/* If this is part of a multi-command transaction (i.e.
+	 * hci_start_transaction() has been called) just add the skb to
+	 * the end of the transaction being built.
+	 */
+	if (hdev->build_transaction) {
+		skb_queue_tail(&hdev->build_transaction->cmd_q, skb);
+		goto unlock;
+	}
+
+	/* If we're in the middle of a hci_request the req lock will be
+	 * held and our only choice is to append to the request
+	 * transaction.
+	 */
+	if (hdev->req_status && hdev->current_transaction) {
+		skb_queue_tail(&hdev->current_transaction->cmd_q, skb);
+		goto unlock;
+	}
+
+	/* This is neither a multi-command transaction nor a hci_request
+	 * situation, but simply hci_send_cmd being called without any
+	 * existing context. Create a simple one-command transaction out
+	 * of the skb
+	 */
+	err = __transaction_from_skb(hdev, skb);
+	if (err < 0)
+		kfree_skb(skb);
+
+unlock:
+	hci_transaction_unlock(hdev);
+
+	return err;
 }
 
 /* Get data from the previously sent command */
@@ -3129,16 +3179,26 @@ static void hci_rx_work(struct work_struct *work)
 static void hci_cmd_work(struct work_struct *work)
 {
 	struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_work);
+	struct hci_transaction *transaction;
 	struct sk_buff *skb;
 
+	hci_transaction_lock(hdev);
+
+	__transaction_next(hdev, 0, 0);
+	transaction = hdev->current_transaction;
+
 	BT_DBG("%s cmd_cnt %d cmd queued %d", hdev->name,
-	       atomic_read(&hdev->cmd_cnt), skb_queue_len(&hdev->cmd_q));
+	       atomic_read(&hdev->cmd_cnt),
+	       transaction ? skb_queue_len(&transaction->cmd_q) : 0);
+
+	if (!transaction)
+		goto unlock;
 
 	/* Send queued commands */
 	if (atomic_read(&hdev->cmd_cnt)) {
-		skb = skb_dequeue(&hdev->cmd_q);
+		skb = skb_dequeue(&transaction->cmd_q);
 		if (!skb)
-			return;
+			goto unlock;
 
 		kfree_skb(hdev->sent_cmd);
 
@@ -3152,10 +3212,13 @@ static void hci_cmd_work(struct work_struct *work)
 				mod_timer(&hdev->cmd_timer,
 					  jiffies + HCI_CMD_TIMEOUT);
 		} else {
-			skb_queue_head(&hdev->cmd_q, skb);
+			skb_queue_head(&transaction->cmd_q, skb);
 			queue_work(hdev->workqueue, &hdev->cmd_work);
 		}
 	}
+
+unlock:
+	hci_transaction_unlock(hdev);
 }
 
 int hci_do_inquiry(struct hci_dev *hdev, u8 length)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 477726a..13d0f1a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2505,6 +2505,8 @@ static void hci_qos_setup_complete_evt(struct hci_dev *hdev,
 static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_cmd_complete *ev = (void *) skb->data;
+	u8 status = skb->data[sizeof(*ev)];
+	bool queue_empty;
 	__u16 opcode;
 
 	skb_pull(skb, sizeof(*ev));
@@ -2748,9 +2750,11 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	if (ev->opcode != HCI_OP_NOP)
 		del_timer(&hdev->cmd_timer);
 
+	queue_empty = hci_transaction_cmd_complete(hdev, ev->opcode, status);
+
 	if (ev->ncmd && !test_bit(HCI_RESET, &hdev->flags)) {
 		atomic_set(&hdev->cmd_cnt, 1);
-		if (!skb_queue_empty(&hdev->cmd_q))
+		if (!queue_empty)
 			queue_work(hdev->workqueue, &hdev->cmd_work);
 	}
 }
@@ -2758,6 +2762,7 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_cmd_status *ev = (void *) skb->data;
+	bool queue_empty;
 	__u16 opcode;
 
 	skb_pull(skb, sizeof(*ev));
@@ -2841,9 +2846,11 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	if (ev->opcode != HCI_OP_NOP)
 		del_timer(&hdev->cmd_timer);
 
+	queue_empty = hci_transaction_cmd_complete(hdev, ev->opcode, ev->status);
+
 	if (ev->ncmd && !test_bit(HCI_RESET, &hdev->flags)) {
 		atomic_set(&hdev->cmd_cnt, 1);
-		if (!skb_queue_empty(&hdev->cmd_q))
+		if (!queue_empty)
 			queue_work(hdev->workqueue, &hdev->cmd_work);
 	}
 }
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 07f0739..bf008f9 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -859,8 +859,9 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 			skb_queue_tail(&hdev->raw_q, skb);
 			queue_work(hdev->workqueue, &hdev->tx_work);
 		} else {
-			skb_queue_tail(&hdev->cmd_q, skb);
-			queue_work(hdev->workqueue, &hdev->cmd_work);
+			err = hci_transaction_from_skb(hdev, skb);
+			if (err < 0)
+				goto drop;
 		}
 	} else {
 		if (!capable(CAP_NET_RAW)) {
-- 
1.7.10.4

[PATCH 06/12] Bluetooth: Remove unused hdev->cmd_q HCI command queue

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch removes the now unused hdev->cmd_q HCI command queue. The HCI
transaction framework takes care of the same functionality.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    1 -
 net/bluetooth/hci_core.c         |    5 -----
 2 files changed, 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b5c6f99..fd8d305 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -247,7 +247,6 @@ struct hci_dev {
 
 	struct sk_buff_head	rx_q;
 	struct sk_buff_head	raw_q;
-	struct sk_buff_head	cmd_q;
 
 	struct mutex		transaction_lock;
 	struct hci_transaction	*build_transaction;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2c0ad61..da532372 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -808,7 +808,6 @@ int hci_dev_open(__u16 dev)
 		flush_work(&hdev->rx_work);
 
 		hci_transaction_cleanup(hdev);
-		skb_queue_purge(&hdev->cmd_q);
 		skb_queue_purge(&hdev->rx_q);
 
 		if (hdev->flush)
@@ -873,7 +872,6 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 
 	/* Reset device */
 	hci_transaction_cleanup(hdev);
-	skb_queue_purge(&hdev->cmd_q);
 	atomic_set(&hdev->cmd_cnt, 1);
 	if (!test_bit(HCI_RAW, &hdev->flags) &&
 	    test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks)) {
@@ -887,7 +885,6 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 
 	/* Drop queues */
 	skb_queue_purge(&hdev->rx_q);
-	skb_queue_purge(&hdev->cmd_q);
 	hci_transaction_cleanup(hdev);
 	skb_queue_purge(&hdev->raw_q);
 
@@ -958,7 +955,6 @@ int hci_dev_reset(__u16 dev)
 
 	/* Drop queues */
 	skb_queue_purge(&hdev->rx_q);
-	skb_queue_purge(&hdev->cmd_q);
 	hci_transaction_cleanup(hdev);
 
 	hci_dev_lock(hdev);
@@ -1785,7 +1781,6 @@ struct hci_dev *hci_alloc_dev(void)
 
 	skb_queue_head_init(&hdev->driver_init);
 	skb_queue_head_init(&hdev->rx_q);
-	skb_queue_head_init(&hdev->cmd_q);
 	skb_queue_head_init(&hdev->raw_q);
 
 	init_waitqueue_head(&hdev->req_wait_q);
-- 
1.7.10.4

[PATCH 07/12] Bluetooth: Fix mgmt powered indication by using a HCI transaction

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The response to mgmt_set_powered should be returned only when all
related HCI commands have completed. To properly do this make use of a
HCI transaction with a callback to indicate its completion.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 39395c7..042a6c7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3058,19 +3058,33 @@ static int set_bredr_scan(struct hci_dev *hdev)
 	return hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
 }
 
+static void powered_complete(struct hci_dev *hdev, u16 opcode, int status)
+{
+	struct cmd_lookup match = { NULL, hdev };
+
+	mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
+
+	new_settings(hdev, match.sk);
+
+	if (match.sk)
+		sock_put(match.sk);
+}
+
 int mgmt_powered(struct hci_dev *hdev, u8 powered)
 {
 	struct cmd_lookup match = { NULL, hdev };
+	u8 status = MGMT_STATUS_NOT_POWERED;
+	u8 zero_cod[] = { 0, 0, 0 };
 	int err;
 
 	if (!test_bit(HCI_MGMT, &hdev->dev_flags))
 		return 0;
 
-	mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
-
 	if (powered) {
 		u8 link_sec;
 
+		hci_start_transaction(hdev);
+
 		if (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags) &&
 		    !lmp_host_ssp_capable(hdev)) {
 			u8 ssp = 1;
@@ -3105,17 +3119,28 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 			update_name(hdev, hdev->dev_name);
 			update_eir(hdev);
 		}
-	} else {
-		u8 status = MGMT_STATUS_NOT_POWERED;
-		u8 zero_cod[] = { 0, 0, 0 };
 
-		mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
+		err = hci_complete_transaction(hdev, powered_complete);
+		if (err == 0)
+			return 0;
 
-		if (memcmp(hdev->dev_class, zero_cod, sizeof(zero_cod)) != 0)
-			mgmt_event(MGMT_EV_CLASS_OF_DEV_CHANGED, hdev,
-				   zero_cod, sizeof(zero_cod), NULL);
+		/* If hci_complete_transaction fails it means no HCI
+		 * commands were pushed to the queue, i.e. we can go
+		 * ahead and indicate success directly.
+		 */
+		mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp,
+				     &match);
+		goto new_settings;
 	}
 
+	mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
+	mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
+
+	if (memcmp(hdev->dev_class, zero_cod, sizeof(zero_cod)) != 0)
+		mgmt_event(MGMT_EV_CLASS_OF_DEV_CHANGED, hdev,
+			   zero_cod, sizeof(zero_cod), NULL);
+
+new_settings:
 	err = new_settings(hdev, match.sk);
 
 	if (match.sk)
-- 
1.7.10.4

[PATCH 08/12] Bluetooth: Enable HCI transaction support cmd_status 0

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

Some HCI commands do not result in a command complete event and generate
an intermediate command status 0 in between. Inquiry is one of these
procedures and needs to be handled properly since the legacy ioctl for
it uses hci_request which in turn will make use of the the HCI
transaction framework.

If the ncmd HCI event parameter indicates that we can send more commands
to the controller we should do it if we have any commands in our queue.
However, for the ongoing HCI transaction to be properly notified for
completion we need to hold off this notification if possible when the
command status event comes.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_core.c         |   28 ++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |    7 ++++++-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fd8d305..ce7fbf7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1062,6 +1062,7 @@ int hci_complete_transaction(struct hci_dev *hdev,
 			     void (*complete)(struct hci_dev *hdev,
 					      __u16 last_cmd, int status));
 bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
+bool hci_transaction_cmd_status(struct hci_dev *hdev, u16 opcode, u8 status);
 int hci_transaction_from_skb(struct hci_dev *hdev, struct sk_buff *skb);
 
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index da532372..05914d8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3093,6 +3093,8 @@ bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status)
 {
 	bool queue_empty;
 
+	BT_DBG("opcode 0x%04x status 0x%02x", opcode, status);
+
 	/* Ignore this event if it doesn't match the last HCI command
 	 * that was sent
 	 */
@@ -3116,6 +3118,32 @@ unlock:
 	return queue_empty;
 }
 
+bool hci_transaction_cmd_status(struct hci_dev *hdev, u16 opcode, u8 status)
+{
+	struct hci_transaction *transaction = hdev->current_transaction;
+
+	BT_DBG("opcode 0x%04x status 0x%02x", opcode, status);
+
+	if (status)
+		return hci_transaction_cmd_complete(hdev, opcode, status);
+
+	if (!transaction)
+		return true;
+
+	/* If there are no more commands for this transaction and it *
+	 * doesn't have a complete callback or there are other
+	 * commands/transactions in the hdev queue we consider this
+	 * transaction as completed. Otherwise reply that the queue is
+	 * empty so that we wait for the event that really indicates
+	 * that the pending command is complete.
+	 */
+	if (skb_queue_empty(&transaction->cmd_q) &&
+	    (!transaction->complete || !list_empty(&hdev->transaction_q)))
+		return hci_transaction_cmd_complete(hdev, opcode, status);
+
+	return true;
+}
+
 static void hci_rx_work(struct work_struct *work)
 {
 	struct hci_dev *hdev = container_of(work, struct hci_dev, rx_work);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 13d0f1a..1045a31 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -40,6 +40,8 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
+	hci_transaction_cmd_complete(hdev, HCI_OP_INQUIRY, status);
+
 	if (status) {
 		hci_dev_lock(hdev);
 		mgmt_stop_discovery_failed(hdev, status);
@@ -1944,6 +1946,7 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
+	hci_transaction_cmd_complete(hdev, HCI_OP_INQUIRY, status);
 
 	hci_conn_check_pending(hdev);
 
@@ -2846,7 +2849,9 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	if (ev->opcode != HCI_OP_NOP)
 		del_timer(&hdev->cmd_timer);
 
-	queue_empty = hci_transaction_cmd_complete(hdev, ev->opcode, ev->status);
+	queue_empty = hci_transaction_cmd_status(hdev, ev->opcode, ev->status);
+
+	BT_DBG("queue_empty %u, ev->ncmd %u", queue_empty, ev->ncmd);
 
 	if (ev->ncmd && !test_bit(HCI_RESET, &hdev->flags)) {
 		atomic_set(&hdev->cmd_cnt, 1);
-- 
1.7.10.4

[PATCH 09/12] Bluetooth: Add HCI init sequence support for HCI transactions

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

There's a special quirk needed for unexpected HCI reset command
completes during the init process. This patch adds this quirk to the HCI
transaction code so that hci_init_req can be converted to use the HCI
transaction framework.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_core.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 05914d8..c75acb2 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3089,17 +3089,56 @@ next_in_queue:
 	hdev->current_transaction = transaction;
 }
 
+static void hci_resend_last(struct hci_dev *hdev)
+{
+	struct hci_transaction *transaction;
+	struct hci_command_hdr *sent;
+	struct sk_buff *skb;
+	u16 opcode;
+
+	if (!hdev->sent_cmd)
+		return;
+
+	sent = (void *) hdev->sent_cmd->data;
+	opcode = __le16_to_cpu(sent->opcode);
+
+	skb = skb_clone(hdev->sent_cmd, GFP_ATOMIC);
+	if (!skb)
+		return;
+
+	hci_transaction_lock(hdev);
+
+	transaction = hdev->current_transaction;
+	if (transaction) {
+		skb_queue_head(&transaction->cmd_q, skb);
+		queue_work(hdev->workqueue, &hdev->cmd_work);
+	} else {
+		kfree_skb(skb);
+	}
+
+	hci_transaction_unlock(hdev);
+}
+
 bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status)
 {
 	bool queue_empty;
 
 	BT_DBG("opcode 0x%04x status 0x%02x", opcode, status);
 
-	/* Ignore this event if it doesn't match the last HCI command
-	 * that was sent
+	/* If the completed command doesn't match the last one that was
+	 * sent we need to do special handling of it.
 	 */
-	if (!hci_sent_cmd_data(hdev, opcode))
+	if (!hci_sent_cmd_data(hdev, opcode)) {
+		/* Some CSR based controllers generate a spontaneous
+		 * reset complete event during init and any pending
+		 * command will never be completed. In such a case we
+		 * need to resend whatever was the last sent
+		 * command.
+		 */
+		if (test_bit(HCI_INIT, &hdev->flags) && opcode == HCI_OP_RESET)
+			hci_resend_last(hdev);
 		return false;
+	}
 
 	hci_transaction_lock(hdev);
 
-- 
1.7.10.4

[PATCH 10/12] Bluetooth: Convert hci_request to use HCI transactions

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch converts the hci_request procedure to make use of HCI
transactions. Since the completion callback is now private to hci_core.c
there is no need to have a public hci_req_complete() function anymore.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    2 -
 net/bluetooth/hci_core.c         |   49 ++++--------------------
 net/bluetooth/hci_event.c        |   76 ++------------------------------------
 3 files changed, 11 insertions(+), 116 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ce7fbf7..fe7df9e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1177,8 +1177,6 @@ struct hci_sec_filter {
 #define hci_req_lock(d)		mutex_lock(&d->req_lock)
 #define hci_req_unlock(d)	mutex_unlock(&d->req_lock)
 
-void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result);
-
 void hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max,
 					u16 latency, u16 to_multiplier);
 void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index c75acb2..ea4937b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -57,45 +57,10 @@ static void hci_notify(struct hci_dev *hdev, int event)
 
 /* ---- HCI requests ---- */
 
-void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
+static void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
 {
 	BT_DBG("%s command 0x%4.4x result 0x%2.2x", hdev->name, cmd, result);
 
-	/* If this is the init phase check if the completed command matches
-	 * the last init command, and if not just return.
-	 */
-	if (test_bit(HCI_INIT, &hdev->flags) && hdev->init_last_cmd != cmd) {
-		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
-		u16 opcode = __le16_to_cpu(sent->opcode);
-		struct sk_buff *skb;
-
-		/* Some CSR based controllers generate a spontaneous
-		 * reset complete event during init and any pending
-		 * command will never be completed. In such a case we
-		 * need to resend whatever was the last sent
-		 * command.
-		 */
-
-		if (cmd != HCI_OP_RESET || opcode == HCI_OP_RESET)
-			return;
-
-		skb = skb_clone(hdev->sent_cmd, GFP_ATOMIC);
-		hci_transaction_lock(hdev);
-		if (skb) {
-			struct hci_transaction *transaction =
-						hdev->current_transaction;
-			if (transaction) {
-				skb_queue_head(&transaction->cmd_q, skb);
-				queue_work(hdev->workqueue, &hdev->cmd_work);
-			} else {
-				kfree_skb(skb);
-			}
-		}
-		hci_transaction_unlock(hdev);
-
-		return;
-	}
-
 	if (hdev->req_status == HCI_REQ_PEND) {
 		hdev->req_result = result;
 		hdev->req_status = HCI_REQ_DONE;
@@ -124,12 +89,19 @@ static int __hci_request(struct hci_dev *hdev,
 
 	BT_DBG("%s start", hdev->name);
 
+	err = hci_start_transaction(hdev);
+	if (err < 0)
+		return err;
+
 	hdev->req_status = HCI_REQ_PEND;
 
 	add_wait_queue(&hdev->req_wait_q, &wait);
 	set_current_state(TASK_INTERRUPTIBLE);
 
 	req(hdev, opt);
+
+	hci_complete_transaction(hdev, hci_req_complete);
+
 	schedule_timeout(timeout);
 
 	remove_wait_queue(&hdev->req_wait_q, &wait);
@@ -218,9 +190,6 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 
 	/* Driver initialization */
 
-	if (hci_start_transaction(hdev) < 0)
-		return;
-
 	hci_transaction_lock(hdev);
 
 	transaction = hdev->build_transaction;
@@ -252,8 +221,6 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 		BT_ERR("Unknown device type %d", hdev->dev_type);
 		break;
 	}
-
-	hci_complete_transaction(hdev, NULL);
 }
 
 static void hci_scan_req(struct hci_dev *hdev, unsigned long opt)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1045a31..805580e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -55,8 +55,6 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
 	hci_dev_unlock(hdev);
 
-	hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);
-
 	hci_conn_check_pending(hdev);
 }
 
@@ -185,8 +183,6 @@ static void hci_cc_write_def_link_policy(struct hci_dev *hdev,
 
 	if (!status)
 		hdev->link_policy = get_unaligned_le16(sent);
-
-	hci_req_complete(hdev, HCI_OP_WRITE_DEF_LINK_POLICY, status);
 }
 
 static void hci_cc_reset(struct hci_dev *hdev, struct sk_buff *skb)
@@ -197,8 +193,6 @@ static void hci_cc_reset(struct hci_dev *hdev, struct sk_buff *skb)
 
 	clear_bit(HCI_RESET, &hdev->flags);
 
-	hci_req_complete(hdev, HCI_OP_RESET, status);
-
 	/* Reset all non-persistent flags */
 	hdev->dev_flags &= ~(BIT(HCI_LE_SCAN) | BIT(HCI_PENDING_CLASS) |
 			     BIT(HCI_PERIODIC_INQ));
@@ -233,8 +227,6 @@ static void hci_cc_write_local_name(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (!status && !test_bit(HCI_INIT, &hdev->flags))
 		hci_update_ad(hdev);
-
-	hci_req_complete(hdev, HCI_OP_WRITE_LOCAL_NAME, status);
 }
 
 static void hci_cc_read_local_name(struct hci_dev *hdev, struct sk_buff *skb)
@@ -272,8 +264,6 @@ static void hci_cc_write_auth_enable(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (test_bit(HCI_MGMT, &hdev->dev_flags))
 		mgmt_auth_enable_complete(hdev, status);
-
-	hci_req_complete(hdev, HCI_OP_WRITE_AUTH_ENABLE, status);
 }
 
 static void hci_cc_write_encrypt_mode(struct hci_dev *hdev, struct sk_buff *skb)
@@ -295,8 +285,6 @@ static void hci_cc_write_encrypt_mode(struct hci_dev *hdev, struct sk_buff *skb)
 		else
 			clear_bit(HCI_ENCRYPT, &hdev->flags);
 	}
-
-	hci_req_complete(hdev, HCI_OP_WRITE_ENCRYPT_MODE, status);
 }
 
 static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
@@ -345,7 +333,6 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 
 done:
 	hci_dev_unlock(hdev);
-	hci_req_complete(hdev, HCI_OP_WRITE_SCAN_ENABLE, status);
 }
 
 static void hci_cc_read_class_of_dev(struct hci_dev *hdev, struct sk_buff *skb)
@@ -442,8 +429,6 @@ static void hci_cc_host_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	hci_req_complete(hdev, HCI_OP_HOST_BUFFER_SIZE, status);
 }
 
 static void hci_cc_write_ssp_mode(struct hci_dev *hdev, struct sk_buff *skb)
@@ -686,7 +671,7 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
 	BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
 
 	if (rp->status)
-		goto done;
+		return;
 
 	hdev->hci_ver = rp->hci_ver;
 	hdev->hci_rev = __le16_to_cpu(rp->hci_rev);
@@ -699,9 +684,6 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (test_bit(HCI_INIT, &hdev->flags))
 		hci_setup(hdev);
-
-done:
-	hci_req_complete(hdev, HCI_OP_READ_LOCAL_VERSION, rp->status);
 }
 
 static void hci_setup_link_policy(struct hci_dev *hdev)
@@ -730,15 +712,12 @@ static void hci_cc_read_local_commands(struct hci_dev *hdev,
 	BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
 
 	if (rp->status)
-		goto done;
+		return;
 
 	memcpy(hdev->commands, rp->commands, sizeof(hdev->commands));
 
 	if (test_bit(HCI_INIT, &hdev->flags) && (hdev->commands[5] & 0x10))
 		hci_setup_link_policy(hdev);
-
-done:
-	hci_req_complete(hdev, HCI_OP_READ_LOCAL_COMMANDS, rp->status);
 }
 
 static void hci_cc_read_local_features(struct hci_dev *hdev,
@@ -821,7 +800,7 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
 	BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
 
 	if (rp->status)
-		goto done;
+		return;
 
 	switch (rp->page) {
 	case 0:
@@ -834,9 +813,6 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
 
 	if (test_bit(HCI_INIT, &hdev->flags) && lmp_le_capable(hdev))
 		hci_set_le_support(hdev);
-
-done:
-	hci_req_complete(hdev, HCI_OP_READ_LOCAL_EXT_FEATURES, rp->status);
 }
 
 static void hci_cc_read_flow_control_mode(struct hci_dev *hdev,
@@ -850,8 +826,6 @@ static void hci_cc_read_flow_control_mode(struct hci_dev *hdev,
 		return;
 
 	hdev->flow_ctl_mode = rp->mode;
-
-	hci_req_complete(hdev, HCI_OP_READ_FLOW_CONTROL_MODE, rp->status);
 }
 
 static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
@@ -888,8 +862,6 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (!rp->status)
 		bacpy(&hdev->bdaddr, &rp->bdaddr);
-
-	hci_req_complete(hdev, HCI_OP_READ_BD_ADDR, rp->status);
 }
 
 static void hci_cc_read_data_block_size(struct hci_dev *hdev,
@@ -910,8 +882,6 @@ static void hci_cc_read_data_block_size(struct hci_dev *hdev,
 
 	BT_DBG("%s blk mtu %d cnt %d len %d", hdev->name, hdev->block_mtu,
 	       hdev->block_cnt, hdev->block_len);
-
-	hci_req_complete(hdev, HCI_OP_READ_DATA_BLOCK_SIZE, rp->status);
 }
 
 static void hci_cc_write_ca_timeout(struct hci_dev *hdev, struct sk_buff *skb)
@@ -919,8 +889,6 @@ static void hci_cc_write_ca_timeout(struct hci_dev *hdev, struct sk_buff *skb)
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	hci_req_complete(hdev, HCI_OP_WRITE_CA_TIMEOUT, status);
 }
 
 static void hci_cc_read_local_amp_info(struct hci_dev *hdev,
@@ -944,8 +912,6 @@ static void hci_cc_read_local_amp_info(struct hci_dev *hdev,
 	hdev->amp_be_flush_to = __le32_to_cpu(rp->be_flush_to);
 	hdev->amp_max_flush_to = __le32_to_cpu(rp->max_flush_to);
 
-	hci_req_complete(hdev, HCI_OP_READ_LOCAL_AMP_INFO, rp->status);
-
 a2mp_rsp:
 	a2mp_send_getinfo_rsp(hdev);
 }
@@ -993,8 +959,6 @@ static void hci_cc_delete_stored_link_key(struct hci_dev *hdev,
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	hci_req_complete(hdev, HCI_OP_DELETE_STORED_LINK_KEY, status);
 }
 
 static void hci_cc_set_event_mask(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1002,8 +966,6 @@ static void hci_cc_set_event_mask(struct hci_dev *hdev, struct sk_buff *skb)
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	hci_req_complete(hdev, HCI_OP_SET_EVENT_MASK, status);
 }
 
 static void hci_cc_write_inquiry_mode(struct hci_dev *hdev,
@@ -1012,8 +974,6 @@ static void hci_cc_write_inquiry_mode(struct hci_dev *hdev,
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	hci_req_complete(hdev, HCI_OP_WRITE_INQUIRY_MODE, status);
 }
 
 static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
@@ -1025,8 +985,6 @@ static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
 
 	if (!rp->status)
 		hdev->inq_tx_power = rp->tx_power;
-
-	hci_req_complete(hdev, HCI_OP_READ_INQ_RSP_TX_POWER, rp->status);
 }
 
 static void hci_cc_set_event_flt(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1034,8 +992,6 @@ static void hci_cc_set_event_flt(struct hci_dev *hdev, struct sk_buff *skb)
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	hci_req_complete(hdev, HCI_OP_SET_EVENT_FLT, status);
 }
 
 static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1097,8 +1053,6 @@ static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
 	hdev->le_cnt = hdev->le_pkts;
 
 	BT_DBG("%s le mtu %d:%d", hdev->name, hdev->le_mtu, hdev->le_pkts);
-
-	hci_req_complete(hdev, HCI_OP_LE_READ_BUFFER_SIZE, rp->status);
 }
 
 static void hci_cc_le_read_local_features(struct hci_dev *hdev,
@@ -1110,8 +1064,6 @@ static void hci_cc_le_read_local_features(struct hci_dev *hdev,
 
 	if (!rp->status)
 		memcpy(hdev->le_features, rp->features, 8);
-
-	hci_req_complete(hdev, HCI_OP_LE_READ_LOCAL_FEATURES, rp->status);
 }
 
 static void hci_cc_le_read_adv_tx_power(struct hci_dev *hdev,
@@ -1126,8 +1078,6 @@ static void hci_cc_le_read_adv_tx_power(struct hci_dev *hdev,
 		if (!test_bit(HCI_INIT, &hdev->flags))
 			hci_update_ad(hdev);
 	}
-
-	hci_req_complete(hdev, HCI_OP_LE_READ_ADV_TX_POWER, rp->status);
 }
 
 static void hci_cc_le_set_event_mask(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1135,8 +1085,6 @@ static void hci_cc_le_set_event_mask(struct hci_dev *hdev, struct sk_buff *skb)
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-	hci_req_complete(hdev, HCI_OP_LE_SET_EVENT_MASK, status);
 }
 
 static void hci_cc_user_confirm_reply(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1237,8 +1185,6 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (!test_bit(HCI_INIT, &hdev->flags))
 		hci_update_ad(hdev);
-
-	hci_req_complete(hdev, HCI_OP_LE_SET_ADV_ENABLE, status);
 }
 
 static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1247,8 +1193,6 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
-	hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_PARAM, status);
-
 	if (status) {
 		hci_dev_lock(hdev);
 		mgmt_start_discovery_failed(hdev, status);
@@ -1271,8 +1215,6 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 	switch (cp->enable) {
 	case LE_SCANNING_ENABLED:
-		hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_ENABLE, status);
-
 		if (status) {
 			hci_dev_lock(hdev);
 			mgmt_start_discovery_failed(hdev, status);
@@ -1323,8 +1265,6 @@ static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
 
 	if (!rp->status)
 		hdev->le_white_list_size = rp->size;
-
-	hci_req_complete(hdev, HCI_OP_LE_READ_WHITE_LIST_SIZE, rp->status);
 }
 
 static void hci_cc_le_ltk_reply(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1335,8 +1275,6 @@ static void hci_cc_le_ltk_reply(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (rp->status)
 		return;
-
-	hci_req_complete(hdev, HCI_OP_LE_LTK_REPLY, rp->status);
 }
 
 static void hci_cc_le_ltk_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1347,8 +1285,6 @@ static void hci_cc_le_ltk_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
 
 	if (rp->status)
 		return;
-
-	hci_req_complete(hdev, HCI_OP_LE_LTK_NEG_REPLY, rp->status);
 }
 
 static void hci_cc_le_read_supported_states(struct hci_dev *hdev,
@@ -1360,8 +1296,6 @@ static void hci_cc_le_read_supported_states(struct hci_dev *hdev,
 
 	if (!rp->status)
 		memcpy(hdev->le_states, rp->le_states, 8);
-
-	hci_req_complete(hdev, HCI_OP_LE_READ_SUPPORTED_STATES, rp->status);
 }
 
 static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
@@ -1391,8 +1325,6 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
 	if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
 	    !test_bit(HCI_INIT, &hdev->flags))
 		mgmt_le_enable_complete(hdev, sent->le, status);
-
-	hci_req_complete(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, status);
 }
 
 static void hci_cc_write_remote_amp_assoc(struct hci_dev *hdev,
@@ -1414,7 +1346,6 @@ static void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
 	if (status) {
-		hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 		hci_conn_check_pending(hdev);
 		hci_dev_lock(hdev);
 		if (test_bit(HCI_MGMT, &hdev->dev_flags))
@@ -1945,7 +1876,6 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
-	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 	hci_transaction_cmd_complete(hdev, HCI_OP_INQUIRY, status);
 
 	hci_conn_check_pending(hdev);
-- 
1.7.10.4

[PATCH 11/12] Bluetooth: Remove unused hdev->init_last_cmd

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This variable is no longer needed as the functionality is provided by
the HCI transaction framework.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    2 --
 net/bluetooth/hci_core.c         |    4 ----
 2 files changed, 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fe7df9e..4c14d31 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,8 +261,6 @@ struct hci_dev {
 	__u32			req_status;
 	__u32			req_result;
 
-	__u16			init_last_cmd;
-
 	struct list_head	mgmt_pending;
 
 	struct discovery_state	discovery;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index ea4937b..a0eb92f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -750,7 +750,6 @@ int hci_dev_open(__u16 dev)
 	if (!test_bit(HCI_RAW, &hdev->flags)) {
 		atomic_set(&hdev->cmd_cnt, 1);
 		set_bit(HCI_INIT, &hdev->flags);
-		hdev->init_last_cmd = 0;
 
 		ret = __hci_request(hdev, hci_init_req, 0, HCI_INIT_TIMEOUT);
 
@@ -2314,9 +2313,6 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 	bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
 	skb->dev = (void *) hdev;
 
-	if (test_bit(HCI_INIT, &hdev->flags))
-		hdev->init_last_cmd = opcode;
-
 	err = 0;
 
 	hci_transaction_lock(hdev);
-- 
1.7.10.4

[PATCH 12/12] Bluetooth: Remove empty HCI event handlers

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

With the removal of hci_req_complete() several HCI event handles have
essentially become empty and can be removed.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c |  133 ---------------------------------------------
 1 file changed, 133 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 805580e..22864f2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -424,13 +424,6 @@ static void hci_cc_write_voice_setting(struct hci_dev *hdev,
 		hdev->notify(hdev, HCI_NOTIFY_VOICE_SETTING);
 }
 
-static void hci_cc_host_buffer_size(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	__u8 status = *((__u8 *) skb->data);
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
 static void hci_cc_write_ssp_mode(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	__u8 status = *((__u8 *) skb->data);
@@ -884,13 +877,6 @@ static void hci_cc_read_data_block_size(struct hci_dev *hdev,
 	       hdev->block_cnt, hdev->block_len);
 }
 
-static void hci_cc_write_ca_timeout(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	__u8 status = *((__u8 *) skb->data);
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
 static void hci_cc_read_local_amp_info(struct hci_dev *hdev,
 				       struct sk_buff *skb)
 {
@@ -953,29 +939,6 @@ a2mp_rsp:
 	a2mp_send_create_phy_link_req(hdev, rp->status);
 }
 
-static void hci_cc_delete_stored_link_key(struct hci_dev *hdev,
-					  struct sk_buff *skb)
-{
-	__u8 status = *((__u8 *) skb->data);
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
-static void hci_cc_set_event_mask(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	__u8 status = *((__u8 *) skb->data);
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
-static void hci_cc_write_inquiry_mode(struct hci_dev *hdev,
-				      struct sk_buff *skb)
-{
-	__u8 status = *((__u8 *) skb->data);
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
 static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
 					 struct sk_buff *skb)
 {
@@ -987,13 +950,6 @@ static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
 		hdev->inq_tx_power = rp->tx_power;
 }
 
-static void hci_cc_set_event_flt(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	__u8 status = *((__u8 *) skb->data);
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
 static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_pin_code_reply *rp = (void *) skb->data;
@@ -1267,26 +1223,6 @@ static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
 		hdev->le_white_list_size = rp->size;
 }
 
-static void hci_cc_le_ltk_reply(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	struct hci_rp_le_ltk_reply *rp = (void *) skb->data;
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
-
-	if (rp->status)
-		return;
-}
-
-static void hci_cc_le_ltk_neg_reply(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	struct hci_rp_le_ltk_neg_reply *rp = (void *) skb->data;
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
-
-	if (rp->status)
-		return;
-}
-
 static void hci_cc_le_read_supported_states(struct hci_dev *hdev,
 					    struct sk_buff *skb)
 {
@@ -1817,11 +1753,6 @@ static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
 	}
 }
 
-static void hci_cs_le_start_enc(struct hci_dev *hdev, u8 status)
-{
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
 static void hci_cs_create_phylink(struct hci_dev *hdev, u8 status)
 {
 	struct hci_cp_create_phy_link *cp;
@@ -1863,11 +1794,6 @@ static void hci_cs_accept_phylink(struct hci_dev *hdev, u8 status)
 	amp_write_remote_assoc(hdev, cp->phy_handle);
 }
 
-static void hci_cs_create_logical_link(struct hci_dev *hdev, u8 status)
-{
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-}
-
 static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	__u8 status = *((__u8 *) skb->data);
@@ -2424,17 +2350,6 @@ unlock:
 	hci_dev_unlock(hdev);
 }
 
-static void hci_remote_version_evt(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	BT_DBG("%s", hdev->name);
-}
-
-static void hci_qos_setup_complete_evt(struct hci_dev *hdev,
-				       struct sk_buff *skb)
-{
-	BT_DBG("%s", hdev->name);
-}
-
 static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_cmd_complete *ev = (void *) skb->data;
@@ -2523,10 +2438,6 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cc_write_voice_setting(hdev, skb);
 		break;
 
-	case HCI_OP_HOST_BUFFER_SIZE:
-		hci_cc_host_buffer_size(hdev, skb);
-		break;
-
 	case HCI_OP_WRITE_SSP_MODE:
 		hci_cc_write_ssp_mode(hdev, skb);
 		break;
@@ -2559,10 +2470,6 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cc_read_data_block_size(hdev, skb);
 		break;
 
-	case HCI_OP_WRITE_CA_TIMEOUT:
-		hci_cc_write_ca_timeout(hdev, skb);
-		break;
-
 	case HCI_OP_READ_FLOW_CONTROL_MODE:
 		hci_cc_read_flow_control_mode(hdev, skb);
 		break;
@@ -2575,26 +2482,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cc_read_local_amp_assoc(hdev, skb);
 		break;
 
-	case HCI_OP_DELETE_STORED_LINK_KEY:
-		hci_cc_delete_stored_link_key(hdev, skb);
-		break;
-
-	case HCI_OP_SET_EVENT_MASK:
-		hci_cc_set_event_mask(hdev, skb);
-		break;
-
-	case HCI_OP_WRITE_INQUIRY_MODE:
-		hci_cc_write_inquiry_mode(hdev, skb);
-		break;
-
 	case HCI_OP_READ_INQ_RSP_TX_POWER:
 		hci_cc_read_inq_rsp_tx_power(hdev, skb);
 		break;
 
-	case HCI_OP_SET_EVENT_FLT:
-		hci_cc_set_event_flt(hdev, skb);
-		break;
-
 	case HCI_OP_PIN_CODE_REPLY:
 		hci_cc_pin_code_reply(hdev, skb);
 		break;
@@ -2655,14 +2546,6 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cc_le_read_white_list_size(hdev, skb);
 		break;
 
-	case HCI_OP_LE_LTK_REPLY:
-		hci_cc_le_ltk_reply(hdev, skb);
-		break;
-
-	case HCI_OP_LE_LTK_NEG_REPLY:
-		hci_cc_le_ltk_neg_reply(hdev, skb);
-		break;
-
 	case HCI_OP_LE_READ_SUPPORTED_STATES:
 		hci_cc_le_read_supported_states(hdev, skb);
 		break;
@@ -2755,10 +2638,6 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cs_le_create_conn(hdev, ev->status);
 		break;
 
-	case HCI_OP_LE_START_ENC:
-		hci_cs_le_start_enc(hdev, ev->status);
-		break;
-
 	case HCI_OP_CREATE_PHY_LINK:
 		hci_cs_create_phylink(hdev, ev->status);
 		break;
@@ -2767,10 +2646,6 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cs_accept_phylink(hdev, ev->status);
 		break;
 
-	case HCI_OP_CREATE_LOGICAL_LINK:
-		hci_cs_create_logical_link(hdev, ev->status);
-		break;
-
 	default:
 		BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
 		break;
@@ -4072,14 +3947,6 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_remote_features_evt(hdev, skb);
 		break;
 
-	case HCI_EV_REMOTE_VERSION:
-		hci_remote_version_evt(hdev, skb);
-		break;
-
-	case HCI_EV_QOS_SETUP_COMPLETE:
-		hci_qos_setup_complete_evt(hdev, skb);
-		break;
-
 	case HCI_EV_CMD_COMPLETE:
 		hci_cmd_complete_evt(hdev, skb);
 		break;
-- 
1.7.10.4

Re: [PATCH 09/12] Bluetooth: Add HCI init sequence support for HCI transactions

[Anderson Lizardo]

Hi Johan,

On Wed, Feb 13, 2013 at 10:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> +static void hci_resend_last(struct hci_dev *hdev)
> +{
> +       struct hci_transaction *transaction;
> +       struct hci_command_hdr *sent;
> +       struct sk_buff *skb;
> +       u16 opcode;
> +
> +       if (!hdev->sent_cmd)
> +               return;
> +
> +       sent = (void *) hdev->sent_cmd->data;
> +       opcode = __le16_to_cpu(sent->opcode);

Where is "opcode" used?

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Andre Guedes]

Hi Johan,

On Wed, Feb 13, 2013 at 11:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@intel.com>
>
> With these functions it will be possible to declare the start and end of
> a transaction. hci_start_transaction() creates hdev->build_transaction
> which will be used by hci_send_command() to construct a transaction.
> hci_complete_transaction() indicates the end of a transaction with an
> optional complete callback to be called once the transaction is
> complete. The transaction is either moved to hdev->current_transaction
> (if no other transaction is in progress) or appended to
> hdev->transaction_q.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    5 +++
>  net/bluetooth/hci_core.c         |   81 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0e53032..5cd58f5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1058,6 +1058,11 @@ int hci_unregister_cb(struct hci_cb *hcb);
>  #define hci_transaction_lock(d)                mutex_lock(&d->transaction_lock)
>  #define hci_transaction_unlock(d)      mutex_unlock(&d->transaction_lock)
>
> +int hci_start_transaction(struct hci_dev *hdev);
> +int hci_complete_transaction(struct hci_dev *hdev,
> +                            void (*complete)(struct hci_dev *hdev,
> +                                             __u16 last_cmd, int status));
> +
>  int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
>  void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
>  void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 05e2e8b..13c064e 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2196,6 +2196,87 @@ static int hci_send_frame(struct sk_buff *skb)
>         return hdev->send(skb);
>  }
>
> +int hci_start_transaction(struct hci_dev *hdev)
> +{
> +       struct hci_transaction *transaction;
> +       int err;
> +
> +       hci_transaction_lock(hdev);
> +
> +       /* We can't start a new transaction if there's another one in
> +        * progress of being built.
> +        */
> +       if (hdev->build_transaction) {
> +               err = -EBUSY;
> +               goto unlock;
> +       }
> +
> +       transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);

I've failed to see why we need GFP_ATOMIC here. As this code is not
running in any atomic section, we can allocate memory using
GFP_KERNEL.

> +       if (!transaction) {
> +               err = -ENOMEM;
> +               goto unlock;
> +       }
> +
> +       memset(transaction, 0, sizeof(*transaction));

We can also use kzalloc instead of kmalloc, making this memset unnecessary.

Regards,

Andre

Re: [PATCH 03/12] Bluetooth: Add hci_transaction_cmd_complete function

[Andre Guedes]

Hi Johan,

On Wed, Feb 13, 2013 at 11:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@intel.com>
>
> This function is used to process the HCI transaction state, including
> thigs like picking the next command to send, calling the complete

Typo: thigs/things

Regards

Andre

Re: [PATCH 08/12] Bluetooth: Enable HCI transaction support cmd_status 0

[Andre Guedes]

Hi Johan,

On Wed, Feb 13, 2013 at 11:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@intel.com>
>
> Some HCI commands do not result in a command complete event and generate
> an intermediate command status 0 in between. Inquiry is one of these
> procedures and needs to be handled properly since the legacy ioctl for
> it uses hci_request which in turn will make use of the the HCI

Typo: the the/the

Regards,

Andre

Re: [PATCH 09/12] Bluetooth: Add HCI init sequence support for HCI transactions

[Johan Hedberg]

Hi Lizardo,

On Wed, Feb 13, 2013, Anderson Lizardo wrote:
> Hi Johan,
> 
> On Wed, Feb 13, 2013 at 10:50 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > +static void hci_resend_last(struct hci_dev *hdev)
> > +{
> > +       struct hci_transaction *transaction;
> > +       struct hci_command_hdr *sent;
> > +       struct sk_buff *skb;
> > +       u16 opcode;
> > +
> > +       if (!hdev->sent_cmd)
> > +               return;
> > +
> > +       sent = (void *) hdev->sent_cmd->data;
> > +       opcode = __le16_to_cpu(sent->opcode);
> 
> Where is "opcode" used?

To retain the old logic in hci_req_complete() the intention here was to
check for HCI_OP_RESET and return. I've fixed it in v2.

Johan

Re: [PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Johan Hedberg]

Hi Andre,

On Thu, Feb 14, 2013, Andre Guedes wrote:
> > +int hci_start_transaction(struct hci_dev *hdev)
> > +{
> > +       struct hci_transaction *transaction;
> > +       int err;
> > +
> > +       hci_transaction_lock(hdev);
> > +
> > +       /* We can't start a new transaction if there's another one in
> > +        * progress of being built.
> > +        */
> > +       if (hdev->build_transaction) {
> > +               err = -EBUSY;
> > +               goto unlock;
> > +       }
> > +
> > +       transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
> 
> I've failed to see why we need GFP_ATOMIC here. As this code is not
> running in any atomic section, we can allocate memory using
> GFP_KERNEL.

Since one of the intentions of this API is to create an async version of
hci_request() I think it's better to keep GFP_ATOMIC here. One situation
where you couldn't for sure use hci_request() is if you're in an atomic
section and then a HCI request would be the only other alternative.

> > +       if (!transaction) {
> > +               err = -ENOMEM;
> > +               goto unlock;
> > +       }
> > +
> > +       memset(transaction, 0, sizeof(*transaction));
> 
> We can also use kzalloc instead of kmalloc, making this memset unnecessary.

Good point. Fixed in v2.

Johan

Re: [PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Szymon Janc]

Hi Johan,

On Friday 15 of February 2013 10:22:36 Johan Hedberg wrote:
> > > +int hci_start_transaction(struct hci_dev *hdev)
> > > +{
> > > +       struct hci_transaction *transaction;
> > > +       int err;
> > > +
> > > +       hci_transaction_lock(hdev);
> > > +
> > > +       /* We can't start a new transaction if there's another one in
> > > +        * progress of being built.
> > > +        */
> > > +       if (hdev->build_transaction) {
> > > +               err = -EBUSY;
> > > +               goto unlock;
> > > +       }
> > > +
> > > +       transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
> > 
> > I've failed to see why we need GFP_ATOMIC here. As this code is not
> > running in any atomic section, we can allocate memory using
> > GFP_KERNEL.
> 
> Since one of the intentions of this API is to create an async version of
> hci_request() I think it's better to keep GFP_ATOMIC here. One situation
> where you couldn't for sure use hci_request() is if you're in an atomic
> section and then a HCI request would be the only other alternative.

You lock mutex in hci_start_transaction so it is non-atomic anyway..

-- 
BR
Szymon Janc

Re: [PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Johan Hedberg]

Hi Szymon,

On Fri, Feb 15, 2013, Szymon Janc wrote:
> On Friday 15 of February 2013 10:22:36 Johan Hedberg wrote:
> > > > +int hci_start_transaction(struct hci_dev *hdev)
> > > > +{
> > > > +       struct hci_transaction *transaction;
> > > > +       int err;
> > > > +
> > > > +       hci_transaction_lock(hdev);
> > > > +
> > > > +       /* We can't start a new transaction if there's another one in
> > > > +        * progress of being built.
> > > > +        */
> > > > +       if (hdev->build_transaction) {
> > > > +               err = -EBUSY;
> > > > +               goto unlock;
> > > > +       }
> > > > +
> > > > +       transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
> > > 
> > > I've failed to see why we need GFP_ATOMIC here. As this code is not
> > > running in any atomic section, we can allocate memory using
> > > GFP_KERNEL.
> > 
> > Since one of the intentions of this API is to create an async version of
> > hci_request() I think it's better to keep GFP_ATOMIC here. One situation
> > where you couldn't for sure use hci_request() is if you're in an atomic
> > section and then a HCI request would be the only other alternative.
> 
> You lock mutex in hci_start_transaction so it is non-atomic anyway..

This has me a bit confused since hci_dev_lock() also uses the same. Does
this mean that there's no code at all in the Bluetooth subsystem anymore
that can run in atomic context? (since most things require locking at
least the hdev struct). I remember seeing some time back (after the
whole workqueue conversion) bugs arising from incorrect GFP_ATOMIC ->
GFP_KERNEL conversions (none of which luckily made it to Linus' tree), so at
least some parts seemed to still be atomic. Do a grep for GFP_ATOMIC in
net/bluetooth/ and you'll find plenty of them.

Johan

Re: [PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Szymon Janc]

Hi Johan,

On Friday 15 of February 2013 11:06:04 Johan Hedberg wrote:
> Hi Szymon,
> 
> On Fri, Feb 15, 2013, Szymon Janc wrote:
> > On Friday 15 of February 2013 10:22:36 Johan Hedberg wrote:
> > > > > +int hci_start_transaction(struct hci_dev *hdev)
> > > > > +{
> > > > > +       struct hci_transaction *transaction;
> > > > > +       int err;
> > > > > +
> > > > > +       hci_transaction_lock(hdev);
> > > > > +
> > > > > +       /* We can't start a new transaction if there's another one in
> > > > > +        * progress of being built.
> > > > > +        */
> > > > > +       if (hdev->build_transaction) {
> > > > > +               err = -EBUSY;
> > > > > +               goto unlock;
> > > > > +       }
> > > > > +
> > > > > +       transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
> > > > 
> > > > I've failed to see why we need GFP_ATOMIC here. As this code is not
> > > > running in any atomic section, we can allocate memory using
> > > > GFP_KERNEL.
> > > 
> > > Since one of the intentions of this API is to create an async version of
> > > hci_request() I think it's better to keep GFP_ATOMIC here. One situation
> > > where you couldn't for sure use hci_request() is if you're in an atomic
> > > section and then a HCI request would be the only other alternative.
> > 
> > You lock mutex in hci_start_transaction so it is non-atomic anyway..
> 
> This has me a bit confused since hci_dev_lock() also uses the same. Does
> this mean that there's no code at all in the Bluetooth subsystem anymore
> that can run in atomic context? (since most things require locking at
> least the hdev struct). I remember seeing some time back (after the
> whole workqueue conversion) bugs arising from incorrect GFP_ATOMIC ->
> GFP_KERNEL conversions (none of which luckily made it to Linus' tree), so at
> least some parts seemed to still be atomic. Do a grep for GFP_ATOMIC in
> net/bluetooth/ and you'll find plenty of them.

The bug (by me btw:P) you are probably referring to was due to rwlock which
is spin lock and raise sleeping in atomic warning even if run e.g. from wq.

-- 
BR
Szymon Janc

Re: [PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Johan Hedberg]

Hi Szymon,

On Fri, Feb 15, 2013, Szymon Janc wrote:
> > > > > > +int hci_start_transaction(struct hci_dev *hdev)
> > > > > > +{
> > > > > > +       struct hci_transaction *transaction;
> > > > > > +       int err;
> > > > > > +
> > > > > > +       hci_transaction_lock(hdev);
> > > > > > +
> > > > > > +       /* We can't start a new transaction if there's another one in
> > > > > > +        * progress of being built.
> > > > > > +        */
> > > > > > +       if (hdev->build_transaction) {
> > > > > > +               err = -EBUSY;
> > > > > > +               goto unlock;
> > > > > > +       }
> > > > > > +
> > > > > > +       transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
> > > > > 
> > > > > I've failed to see why we need GFP_ATOMIC here. As this code is not
> > > > > running in any atomic section, we can allocate memory using
> > > > > GFP_KERNEL.
> > > > 
> > > > Since one of the intentions of this API is to create an async version of
> > > > hci_request() I think it's better to keep GFP_ATOMIC here. One situation
> > > > where you couldn't for sure use hci_request() is if you're in an atomic
> > > > section and then a HCI request would be the only other alternative.
> > > 
> > > You lock mutex in hci_start_transaction so it is non-atomic anyway..
> > 
> > This has me a bit confused since hci_dev_lock() also uses the same. Does
> > this mean that there's no code at all in the Bluetooth subsystem anymore
> > that can run in atomic context? (since most things require locking at
> > least the hdev struct). I remember seeing some time back (after the
> > whole workqueue conversion) bugs arising from incorrect GFP_ATOMIC ->
> > GFP_KERNEL conversions (none of which luckily made it to Linus' tree), so at
> > least some parts seemed to still be atomic. Do a grep for GFP_ATOMIC in
> > net/bluetooth/ and you'll find plenty of them.
> 
> The bug (by me btw:P) you are probably referring to was due to rwlock which
> is spin lock and raise sleeping in atomic warning even if run e.g. from wq.

Alright. I've just sent a v3 of the set which uses GFP_KERNEL for all
allocations.

Johan

Re: [PATCH 12/12] Bluetooth: Remove empty HCI event handlers

[Johan Hedberg]

Hi Andrei,

On Sun, Feb 17, 2013, Andrei Emeltchenko wrote:
> > With the removal of hci_req_complete() several HCI event handles have
> > essentially become empty and can be removed.
> 
> It is still good to have debug traces.

If you need to trace exactly what HCI events are happening btmon and
hcidump are probably much more useful tools for that than kernel debug
logs. And even if you do want debug logs we can just add single BT_DBT()
calls to the hci_cmd_complete() and hci_cmd_status() functions instead
of creating a new function for every single event just for the sake of
having a debug log for them.

Johan