[PATCH 1/3] Bluetooth: Clean up logic in hci_cc_write_scan_enable

[PATCH 1/3] Bluetooth: Clean up logic in hci_cc_write_scan_enable

[johan.hedberg]

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

This patch adds a new label to hci_cc_write_scan_enable to avoid
unnecessary indentation. This will be convenient especially when new
code for the discoverable timeout gets added.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c |   46 +++++++++++++++++++++++---------------------
 1 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d8fa657..8c81a75 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -274,7 +274,8 @@ static void hci_cc_write_encrypt_mode(struct hci_dev *hdev, struct sk_buff *skb)
 
 static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 {
-	__u8 status = *((__u8 *) skb->data);
+	__u8 param, status = *((__u8 *) skb->data);
+	int old_pscan, old_iscan;
 	void *sent;
 
 	BT_DBG("%s status 0x%x", hdev->name, status);
@@ -283,28 +284,29 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 	if (!sent)
 		return;
 
-	if (!status) {
-		__u8 param = *((__u8 *) sent);
-		int old_pscan, old_iscan;
+	if (status != 0)
+		goto done;
 
-		old_pscan = test_and_clear_bit(HCI_PSCAN, &hdev->flags);
-		old_iscan = test_and_clear_bit(HCI_ISCAN, &hdev->flags);
+	param = *((__u8 *) sent);
 
-		if (param & SCAN_INQUIRY) {
-			set_bit(HCI_ISCAN, &hdev->flags);
-			if (!old_iscan)
-				mgmt_discoverable(hdev->id, 1);
-		} else if (old_iscan)
-			mgmt_discoverable(hdev->id, 0);
+	old_pscan = test_and_clear_bit(HCI_PSCAN, &hdev->flags);
+	old_iscan = test_and_clear_bit(HCI_ISCAN, &hdev->flags);
 
-		if (param & SCAN_PAGE) {
-			set_bit(HCI_PSCAN, &hdev->flags);
-			if (!old_pscan)
-				mgmt_connectable(hdev->id, 1);
-		} else if (old_pscan)
-			mgmt_connectable(hdev->id, 0);
-	}
+	if (param & SCAN_INQUIRY) {
+		set_bit(HCI_ISCAN, &hdev->flags);
+		if (!old_iscan)
+			mgmt_discoverable(hdev->id, 1);
+	} else if (old_iscan)
+		mgmt_discoverable(hdev->id, 0);
+
+	if (param & SCAN_PAGE) {
+		set_bit(HCI_PSCAN, &hdev->flags);
+		if (!old_pscan)
+			mgmt_connectable(hdev->id, 1);
+	} else if (old_pscan)
+		mgmt_connectable(hdev->id, 0);
 
+done:
 	hci_req_complete(hdev, HCI_OP_WRITE_SCAN_ENABLE, status);
 }
 
-- 
1.7.7.1

[PATCH 2/3] Bluetooth: Add timeout field to mgmt_set_discoverable

[johan.hedberg]

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

Based on the revised mgmt API set_discoverable has a timeout parameter
to specify how long the adapter will remain discoverable. A value of 0
means "indefinitively".

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    3 +++
 include/net/bluetooth/mgmt.h     |    4 ++++
 net/bluetooth/hci_core.c         |   25 +++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |    5 +++++
 net/bluetooth/mgmt.c             |    5 ++++-
 5 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f97792c..5079598 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -196,6 +196,9 @@ struct hci_dev {
 	struct work_struct	power_off;
 	struct timer_list	off_timer;
 
+	__u16			discov_timeout;
+	struct delayed_work	discov_off;
+
 	struct timer_list	cmd_timer;
 	struct tasklet_struct	cmd_task;
 	struct tasklet_struct	rx_task;
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 3062fd3..b5320aa 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -69,6 +69,10 @@ struct mgmt_mode {
 #define MGMT_OP_SET_POWERED		0x0005
 
 #define MGMT_OP_SET_DISCOVERABLE	0x0006
+struct mgmt_cp_set_discoverable {
+	__u8 val;
+	__u16 timeout;
+} __packed;
 
 #define MGMT_OP_SET_CONNECTABLE		0x0007
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b7f6b5b..0c75770 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -595,6 +595,11 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	tasklet_kill(&hdev->rx_task);
 	tasklet_kill(&hdev->tx_task);
 
+	if (hdev->discov_timeout > 0) {
+		cancel_delayed_work_sync(&hdev->discov_off);
+		hdev->discov_timeout = 0;
+	}
+
 	hci_dev_lock_bh(hdev);
 	inquiry_cache_flush(hdev);
 	hci_conn_hash_flush(hdev);
@@ -968,6 +973,24 @@ void hci_del_off_timer(struct hci_dev *hdev)
 	del_timer(&hdev->off_timer);
 }
 
+static void hci_discov_off(struct work_struct *work)
+{
+	struct hci_dev *hdev;
+	u8 scan = SCAN_PAGE;
+
+	hdev = container_of(work, struct hci_dev, discov_off.work);
+
+	BT_DBG("%s", hdev->name);
+
+	hci_dev_lock_bh(hdev);
+
+	hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
+
+	hdev->discov_timeout = 0;
+
+	hci_dev_unlock_bh(hdev);
+}
+
 int hci_uuids_clear(struct hci_dev *hdev)
 {
 	struct list_head *p, *n;
@@ -1485,6 +1508,8 @@ int hci_register_dev(struct hci_dev *hdev)
 	INIT_WORK(&hdev->power_off, hci_power_off);
 	setup_timer(&hdev->off_timer, hci_auto_off, (unsigned long) hdev);
 
+	INIT_DELAYED_WORK(&hdev->discov_off, hci_discov_off);
+
 	memset(&hdev->stat, 0, sizeof(struct hci_dev_stats));
 
 	atomic_set(&hdev->promisc, 0);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8c81a75..dd24d6e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -296,6 +296,11 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 		set_bit(HCI_ISCAN, &hdev->flags);
 		if (!old_iscan)
 			mgmt_discoverable(hdev->id, 1);
+		if (hdev->discov_timeout > 0) {
+			int to = msecs_to_jiffies(hdev->discov_timeout * 1000);
+			queue_delayed_work(hdev->workqueue, &hdev->discov_off,
+									to);
+		}
 	} else if (old_iscan)
 		mgmt_discoverable(hdev->id, 0);
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 747366a..efcde95 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -348,7 +348,7 @@ failed:
 static int set_discoverable(struct sock *sk, u16 index, unsigned char *data,
 									u16 len)
 {
-	struct mgmt_mode *cp;
+	struct mgmt_cp_set_discoverable *cp;
 	struct hci_dev *hdev;
 	struct pending_cmd *cmd;
 	u8 scan;
@@ -399,6 +399,9 @@ static int set_discoverable(struct sock *sk, u16 index, unsigned char *data,
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
+	if (cp->val)
+		hdev->discov_timeout = get_unaligned_le16(&cp->timeout);
+
 failed:
 	hci_dev_unlock_bh(hdev);
 	hci_dev_put(hdev);
-- 
1.7.7.1

[PATCH 3/3] Bluetooth: Fix mgmt response when HCI_Write_Scan_Enable fails

[johan.hedberg]

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

A proper mgmt_command_status should be returned to user-space if either
discoverable or connectable enabling fails.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_event.c        |    9 ++++++---
 net/bluetooth/mgmt.c             |   13 +++++++++++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5079598..6a17e5f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -911,6 +911,7 @@ int mgmt_index_removed(u16 index);
 int mgmt_powered(u16 index, u8 powered);
 int mgmt_discoverable(u16 index, u8 discoverable);
 int mgmt_connectable(u16 index, u8 connectable);
+int mgmt_write_scan_failed(u16 index, u8 scan, u8 status);
 int mgmt_new_key(u16 index, struct link_key *key, u8 persistent);
 int mgmt_connected(u16 index, bdaddr_t *bdaddr, u8 link_type);
 int mgmt_disconnected(u16 index, bdaddr_t *bdaddr);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index dd24d6e..a601baf 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -284,11 +284,14 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 	if (!sent)
 		return;
 
-	if (status != 0)
-		goto done;
-
 	param = *((__u8 *) sent);
 
+	if (status != 0) {
+		mgmt_write_scan_failed(hdev->id, param, status);
+		hdev->discov_timeout = 0;
+		goto done;
+	}
+
 	old_pscan = test_and_clear_bit(HCI_PSCAN, &hdev->flags);
 	old_iscan = test_and_clear_bit(HCI_ISCAN, &hdev->flags);
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index efcde95..56e64dc 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2059,6 +2059,19 @@ int mgmt_connectable(u16 index, u8 connectable)
 	return ret;
 }
 
+int mgmt_write_scan_failed(u16 index, u8 scan, u8 status)
+{
+	if (scan & SCAN_PAGE)
+		mgmt_pending_foreach(MGMT_OP_SET_CONNECTABLE, index,
+						cmd_status_rsp, &status);
+
+	if (scan & SCAN_INQUIRY)
+		mgmt_pending_foreach(MGMT_OP_SET_DISCOVERABLE, index,
+						cmd_status_rsp, &status);
+
+	return 0;
+}
+
 int mgmt_new_key(u16 index, struct link_key *key, u8 persistent)
 {
 	struct mgmt_ev_new_key ev;
-- 
1.7.7.1

Re: [PATCH 2/3] Bluetooth: Add timeout field to mgmt_set_discoverable

[Gustavo Padovan]

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2011-11-04 00:17:46 +0200]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> Based on the revised mgmt API set_discoverable has a timeout parameter
> to specify how long the adapter will remain discoverable. A value of 0
> means "indefinitively".
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    3 +++
>  include/net/bluetooth/mgmt.h     |    4 ++++
>  net/bluetooth/hci_core.c         |   25 +++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |    5 +++++
>  net/bluetooth/mgmt.c             |    5 ++++-
>  5 files changed, 41 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index f97792c..5079598 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -196,6 +196,9 @@ struct hci_dev {
>  	struct work_struct	power_off;
>  	struct timer_list	off_timer;
>  
> +	__u16			discov_timeout;
> +	struct delayed_work	discov_off;
> +
>  	struct timer_list	cmd_timer;
>  	struct tasklet_struct	cmd_task;
>  	struct tasklet_struct	rx_task;
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 3062fd3..b5320aa 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -69,6 +69,10 @@ struct mgmt_mode {
>  #define MGMT_OP_SET_POWERED		0x0005
>  
>  #define MGMT_OP_SET_DISCOVERABLE	0x0006
> +struct mgmt_cp_set_discoverable {
> +	__u8 val;
> +	__u16 timeout;
> +} __packed;
>  
>  #define MGMT_OP_SET_CONNECTABLE		0x0007
>  
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index b7f6b5b..0c75770 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -595,6 +595,11 @@ static int hci_dev_do_close(struct hci_dev *hdev)
>  	tasklet_kill(&hdev->rx_task);
>  	tasklet_kill(&hdev->tx_task);
>  
> +	if (hdev->discov_timeout > 0) {
> +		cancel_delayed_work_sync(&hdev->discov_off);
> +		hdev->discov_timeout = 0;
> +	}
> +
>  	hci_dev_lock_bh(hdev);
>  	inquiry_cache_flush(hdev);
>  	hci_conn_hash_flush(hdev);
> @@ -968,6 +973,24 @@ void hci_del_off_timer(struct hci_dev *hdev)
>  	del_timer(&hdev->off_timer);
>  }
>  
> +static void hci_discov_off(struct work_struct *work)
> +{
> +	struct hci_dev *hdev;
> +	u8 scan = SCAN_PAGE;
> +
> +	hdev = container_of(work, struct hci_dev, discov_off.work);
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	hci_dev_lock_bh(hdev);
> +
> +	hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
> +
> +	hdev->discov_timeout = 0;
> +
> +	hci_dev_unlock_bh(hdev);
> +}
> +
>  int hci_uuids_clear(struct hci_dev *hdev)
>  {
>  	struct list_head *p, *n;
> @@ -1485,6 +1508,8 @@ int hci_register_dev(struct hci_dev *hdev)
>  	INIT_WORK(&hdev->power_off, hci_power_off);
>  	setup_timer(&hdev->off_timer, hci_auto_off, (unsigned long) hdev);
>  
> +	INIT_DELAYED_WORK(&hdev->discov_off, hci_discov_off);
> +
>  	memset(&hdev->stat, 0, sizeof(struct hci_dev_stats));
>  
>  	atomic_set(&hdev->promisc, 0);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 8c81a75..dd24d6e 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -296,6 +296,11 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
>  		set_bit(HCI_ISCAN, &hdev->flags);
>  		if (!old_iscan)
>  			mgmt_discoverable(hdev->id, 1);
> +		if (hdev->discov_timeout > 0) {
> +			int to = msecs_to_jiffies(hdev->discov_timeout * 1000);
> +			queue_delayed_work(hdev->workqueue, &hdev->discov_off,
> +									to);
> +		}
>  	} else if (old_iscan)
>  		mgmt_discoverable(hdev->id, 0);

If we call set discoverable FALSE after call set discoverable TRUE and
_before_ it timeouts expires we are leaking the delayed work. A
cancel_delayed_work_sync() call seems to be missing in this case.

	Gustavo

Re: [PATCH 1/3] Bluetooth: Clean up logic in hci_cc_write_scan_enable

[Gustavo Padovan]

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2011-11-04 00:17:45 +0200]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds a new label to hci_cc_write_scan_enable to avoid
> unnecessary indentation. This will be convenient especially when new
> code for the discoverable timeout gets added.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/hci_event.c |   46 +++++++++++++++++++++++---------------------
>  1 files changed, 24 insertions(+), 22 deletions(-)

Applied, thanks.

	Gustavo

Re: [PATCH 3/3] Bluetooth: Fix mgmt response when HCI_Write_Scan_Enable fails

[Gustavo Padovan]

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2011-11-04 00:17:47 +0200]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> A proper mgmt_command_status should be returned to user-space if either
> discoverable or connectable enabling fails.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/hci_event.c        |    9 ++++++---
>  net/bluetooth/mgmt.c             |   13 +++++++++++++
>  3 files changed, 20 insertions(+), 3 deletions(-)

Applied, thanks.

	Gustavo