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

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

[Johan Hedberg]

Hi,

v3: Convert to using GFP_KERNEL in two places and another kzmalloc
improvement.

>From the original cover letter (with updated diffstat):

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(-)

[PATCH 01/12 v3] 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 v3] 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         |   80 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 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..0b289f3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2196,6 +2196,86 @@ 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 = kzalloc(sizeof(*transaction), GFP_KERNEL);
+	if (!transaction) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	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 v3] 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
things 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 0b289f3..8923a1f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2976,6 +2976,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 v3] 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         |   28 ++++++++++++++++++++++++++++
 2 files changed, 29 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 8923a1f..6569248 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2276,6 +2276,34 @@ unlock:
 	return err;
 }
 
+static int __transaction_from_skb(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_transaction *transaction;
+
+	transaction = kzalloc(sizeof(*transaction), GFP_KERNEL);
+	if (!transaction)
+		return -ENOMEM;
+
+	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 v3] 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 |   12 +++++--
 net/bluetooth/hci_sock.c  |    5 +--
 3 files changed, 88 insertions(+), 16 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6569248..4f55225 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)
@@ -2310,6 +2328,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);
 
@@ -2334,10 +2353,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 */
@@ -3127,16 +3177,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);
 
@@ -3150,10 +3210,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..7f198ba 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,12 @@ 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 v3] 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 4f55225..bc2d7f2 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 v3] 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 v3] 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 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        |    8 ++++++--
 3 files changed, 35 insertions(+), 2 deletions(-)

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 bc2d7f2..2565bb1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3091,6 +3091,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
 	 */
@@ -3114,6 +3116,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 7f198ba..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,8 +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 v3] 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 |   47 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2565bb1..d16c2e4 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3087,17 +3087,58 @@ 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);
+	if (opcode == HCI_OP_RESET)
+		return;
+
+	skb = skb_clone(hdev->sent_cmd, GFP_KERNEL);
+	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 v3] 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 d16c2e4..ddda5e7 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 v3] 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 ddda5e7..35b9bb5 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);
 
@@ -2312,9 +2311,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 v3] 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 02/12 v3] Bluetooth: Add basic start/complete HCI transaction functions

[Marcel Holtmann]

Hi Johan,

> 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         |   80 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 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..0b289f3 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2196,6 +2196,86 @@ 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;
> +	}

I do not get this part. Why not use a common mutex on
hci_start_transaction and have it close with hci_complete_transaction.

This sounds more like a double protection.

> +
> +	transaction = kzalloc(sizeof(*transaction), GFP_KERNEL);
> +	if (!transaction) {
> +		err = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	skb_queue_head_init(&transaction->cmd_q);
> +
> +	hdev->build_transaction = transaction;
> +

I am bit confused with this build_transaction concept. So we are
expecting to build transaction inside the same function. Meaning that
start and complete functions will be called in the same context.

Maybe some concept of DECLARE_TRANSACTION declaring a local variable and
then start and complete transaction would be better.

On that topic using begin_transaction and finish_transaction sounds a
bit more appealing. Or even run_transaction instead of finish.

Regards

Marcel

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

[Marcel Holtmann]

Hi Johan,

> This function is used to process the HCI transaction state, including
> things 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 0b289f3..8923a1f 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2976,6 +2976,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;
> +	}

why do we need current_transaction again. Can we not just always use the
head of the list?

Regards

Marcel

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

[Marcel Holtmann]

Hi Johan,

> 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 |   12 +++++--
>  net/bluetooth/hci_sock.c  |    5 +--
>  3 files changed, 88 insertions(+), 16 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 6569248..4f55225 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);

if we have to touch this one, then please look at my hdev->setup()
patches I send a while back. We should get these merged so that drivers
can use a transaction within the driver and remove the driver_init queue
actually.

Regards

Marcel

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

[Johan Hedberg]

Hi Marcel,

On Sat, Feb 16, 2013, Marcel Holtmann 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;
> > +	}
> 
> I do not get this part. Why not use a common mutex on
> hci_start_transaction and have it close with hci_complete_transaction.
> 
> This sounds more like a double protection.
> 
> > +
> > +	transaction = kzalloc(sizeof(*transaction), GFP_KERNEL);
> > +	if (!transaction) {
> > +		err = -ENOMEM;
> > +		goto unlock;
> > +	}
> > +
> > +	skb_queue_head_init(&transaction->cmd_q);
> > +
> > +	hdev->build_transaction = transaction;
> > +
> 
> I am bit confused with this build_transaction concept. So we are
> expecting to build transaction inside the same function. Meaning that
> start and complete functions will be called in the same context.
> 
> Maybe some concept of DECLARE_TRANSACTION declaring a local variable and
> then start and complete transaction would be better.

Both of the above two issues are related to the desire to not have to
introduce a new function next to hci_send_cmd() (that would take this
local variable "context") and to be able to keep all current
hci_send_cmd() calls as they are.

In my initial iteration of the patch set I did have the kind of locking
you describe, but keeping the requirement for hci_send_cmd() like I
stated means that you'll then either end up having a potential race
or a deadlock inside the function since sometimes you're entering with
the transaction lock held (when using begin/finish transaction) and
sometimes without the lock held (all existing calls to hci_send_cmd().

So what we're dealing with here is trading less complexity in all our
hci_send_cmd() callers (not having to pass around this extra context)
for slightly more complexity inside the hci_send_cmd() function, which I
maintain is less complexity than removing it would introduce elsewhere,
basically this:

       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);


> On that topic using begin_transaction and finish_transaction sounds a
> bit more appealing. Or even run_transaction instead of finish.

begin/run sounds fine to me.

Johan

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

[Johan Hedberg]

Hi Marcel,

On Sat, Feb 16, 2013, Marcel Holtmann wrote:
> > This function is used to process the HCI transaction state, including
> > things 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 0b289f3..8923a1f 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2976,6 +2976,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;
> > +	}
> 
> why do we need current_transaction again. Can we not just always use the
> head of the list?

Since we give up and re-acquire the lock when calling
transaction->complete() it is possible that a new transaction has
already been moved to current_transaction and started running when
complete() returns. If this is the case the __transaction_next() should
just return (like it does).

Johan

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

[Johan Hedberg]

Hi Marcel,

On Sat, Feb 16, 2013, Marcel Holtmann wrote:
> >  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);
> 
> if we have to touch this one, then please look at my hdev->setup()
> patches I send a while back. We should get these merged so that drivers
> can use a transaction within the driver and remove the driver_init queue
> actually.

Alright. I'll look into it.

Johan

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

[Marcel Holtmann]

Hi Johan,

> > > +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;
> > > +	}
> > 
> > I do not get this part. Why not use a common mutex on
> > hci_start_transaction and have it close with hci_complete_transaction.
> > 
> > This sounds more like a double protection.
> > 
> > > +
> > > +	transaction = kzalloc(sizeof(*transaction), GFP_KERNEL);
> > > +	if (!transaction) {
> > > +		err = -ENOMEM;
> > > +		goto unlock;
> > > +	}
> > > +
> > > +	skb_queue_head_init(&transaction->cmd_q);
> > > +
> > > +	hdev->build_transaction = transaction;
> > > +
> > 
> > I am bit confused with this build_transaction concept. So we are
> > expecting to build transaction inside the same function. Meaning that
> > start and complete functions will be called in the same context.
> > 
> > Maybe some concept of DECLARE_TRANSACTION declaring a local variable and
> > then start and complete transaction would be better.
> 
> Both of the above two issues are related to the desire to not have to
> introduce a new function next to hci_send_cmd() (that would take this
> local variable "context") and to be able to keep all current
> hci_send_cmd() calls as they are.

not having to modify hci_send_cmd seems fine. I was more thinking of
tracking this stuff. Since it only requires to know start and end of the
transaction. So that tracking could be localized.

I do like the idea of replacing cmd_q with a more advanced queue that
also has the ability for callbacks when a command finished. That is
pretty neat. The API itself seems also fine, but the actual
implementation feels rather hackish to have this all in hdev struct
while in reality it should be localized to the calling functions (except
for the queue itself of course).

The way I see this, we just need to take a proper lock protecting the
queue. And then mark the begin of a transaction and when it finishes.
When you call finish the markers get supplied to the queue.

Of course the queue can not be processed at the same time you create a
transaction, but that should be just fine.

> In my initial iteration of the patch set I did have the kind of locking
> you describe, but keeping the requirement for hci_send_cmd() like I
> stated means that you'll then either end up having a potential race
> or a deadlock inside the function since sometimes you're entering with
> the transaction lock held (when using begin/finish transaction) and
> sometimes without the lock held (all existing calls to hci_send_cmd().

I think we need to protect the access the queue against the queue
processing and that should be enough. I can see the desire to make
hci_send_cmd essentially a single transaction without callback. That is
a nice idea and should be most likely kept. However I am not sure if not
better use new command to add commands to a transaction. It does not
feel right to overload hci_send_cmd with two semantics.

The kernel historically makes the caller being aware of what it is doing
and what context it is in. See GFP_KERNEL and alike and locked vs
unlocked functions. If we are trying to be super smart now seems going
against that.

Regards

Marcel

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

[Andre Guedes]

Hi Johan,

On Fri, Feb 15, 2013 at 7:51 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> 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);
> +

What if this hci_start_transaction returns error? For example, if we
have another transaction being built.

Regards,

Andre