[PATCH v3 0/3] Don't send SCO/eSCO request during a mode change from sniff to active

[PATCH v3 0/3] Don't send SCO/eSCO request during a mode change from sniff to active

[Ron Shaffer]

Split original patch in to 3 patches
Fix patch subject lines

-- 
Ron Shaffer
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH 1/3] Bluetooth: Remove extraneous white space

[Ron Shaffer]

Deleted extraneous white space from the end of several lines

Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
---
 include/net/bluetooth/hci_core.h |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e42f6ed..4a664e6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1,4 +1,4 @@
-/* 
+/*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
 
@@ -12,13 +12,13 @@
    OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
    IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
-   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
-   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
-   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
-   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
@@ -380,7 +380,7 @@ static inline void __hci_dev_put(struct hci_dev *d)
 }
 
 static inline void hci_dev_put(struct hci_dev *d)
-{ 
+{
 	__hci_dev_put(d);
 	module_put(d->owner);
 }
-- 
1.7.0.2

--
Ron Shaffer
Qualcomm Innocation Center, Inc.
Qualcomm Innocation Center, Inc. is a member of Code Aurora Forum

[PATCH 2/3] Bluetooth: Reassigned copyright to Code Aurora Forum

[Ron Shaffer]

Qualcomm, Inc. has reassigned rights to Code Aurora Forum. Accordingly,
as files are modified by Code Aurora Forum members, the copyright
statement will be updated.

Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
---
 include/net/bluetooth/hci_core.h |    2 +-
 net/bluetooth/hci_conn.c         |    2 +-
 net/bluetooth/hci_event.c        |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4a664e6..fd53323 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1,6 +1,6 @@
 /*
    BlueZ - Bluetooth protocol stack for Linux
-   Copyright (C) 2000-2001 Qualcomm Incorporated
+   Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b10e3cd..9bf4308 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1,6 +1,6 @@
 /*
    BlueZ - Bluetooth protocol stack for Linux
-   Copyright (C) 2000-2001 Qualcomm Incorporated
+   Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6c57fc7..3af537a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1,6 +1,6 @@
 /*
    BlueZ - Bluetooth protocol stack for Linux
-   Copyright (C) 2000-2001 Qualcomm Incorporated
+   Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
-- 
1.7.0.2

--
Ron Shaffer
Qualcomm Innocation Center, Inc.
Qualcomm Innocation Center, Inc. is a member of Code Aurora Forum

[PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Ron Shaffer]

Certain headsets such as the Motorola H350 will reject SCO and eSCO
connection requests while the ACL is transitioning from sniff mode
to active mode. Add synchronization so that SCO and eSCO connection
requests will wait until the ACL has fully transitioned to active mode.

Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_conn.c         |   18 ++++++++++++++++++
 net/bluetooth/hci_event.c        |   23 ++++++++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fd53323..c4a37fc 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,6 +250,7 @@ enum {
 	HCI_CONN_ENCRYPT_PEND,
 	HCI_CONN_RSWITCH_PEND,
 	HCI_CONN_MODE_CHANGE_PEND,
+	HCI_CONN_SCO_PEND,
 };
 
 static inline void hci_conn_hash_init(struct hci_dev *hdev)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 9bf4308..e900f85 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,9 +117,18 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_add_sco cp;
+	struct hci_conn *acl = conn->link;
 
 	BT_DBG("%p", conn);
 
+	if (acl->mode == HCI_CM_SNIFF &&
+			test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+		set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+		return;
+	}
+
+	clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
 	conn->state = BT_CONNECT;
 	conn->out = 1;
 
@@ -135,9 +144,18 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_setup_sync_conn cp;
+	struct hci_conn *acl = conn->link;
 
 	BT_DBG("%p", conn);
 
+	if (acl->mode == HCI_CM_SNIFF &&
+			test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+		set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+		return;
+	}
+
+	clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
 	conn->state = BT_CONNECT;
 	conn->out = 1;
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3af537a..7692db6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -615,6 +615,7 @@ static void hci_cs_add_sco(struct hci_dev *hdev, __u8 status)
 	acl = hci_conn_hash_lookup_handle(hdev, handle);
 	if (acl && (sco = acl->link)) {
 		sco->state = BT_CLOSED;
+		clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
 
 		hci_proto_connect_cfm(sco, status);
 		hci_conn_del(sco);
@@ -760,6 +761,7 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
 	acl = hci_conn_hash_lookup_handle(hdev, handle);
 	if (acl && (sco = acl->link)) {
 		sco->state = BT_CLOSED;
+		clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
 
 		hci_proto_connect_cfm(sco, status);
 		hci_conn_del(sco);
@@ -795,6 +797,7 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
 {
 	struct hci_cp_exit_sniff_mode *cp;
 	struct hci_conn *conn;
+	struct hci_conn *sco;
 
 	BT_DBG("%s status 0x%x", hdev->name, status);
 
@@ -808,9 +811,17 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
 	hci_dev_lock(hdev);
 
 	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
-	if (conn)
+	if (conn) {
 		clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend);
 
+		sco = conn->link;
+		if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND, &sco->pend)) {
+				hci_proto_connect_cfm(sco, status);
+				hci_conn_del(sco);
+			}
+		}
+	}
+
 	hci_dev_unlock(hdev);
 }
 
@@ -1463,6 +1474,7 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
 {
 	struct hci_ev_mode_change *ev = (void *) skb->data;
 	struct hci_conn *conn;
+	struct hci_conn *sco;
 
 	BT_DBG("%s status %d", hdev->name, ev->status);
 
@@ -1478,6 +1490,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
 				conn->power_save = 1;
 			else
 				conn->power_save = 0;
+		} else {
+			sco = conn->link;
+			if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND,
+					&sco->pend)) {
+				if (lmp_esco_capable(hdev))
+					hci_setup_sync(sco, conn->handle);
+				else
+					hci_add_sco(sco, conn->handle);
+			}
 		}
 	}
 
-- 
1.7.0.2

--
Ron Shaffer
Qualcomm Innocation Center, Inc.
Qualcomm Innocation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH v3 0/3] Don't send SCO/eSCO request during a mode change from sniff to active

[Ron Shaffer]

Hi Marcel:

> Split original patch in to 3 patches
> Fix patch subject lines
> 

Just a ping on this.

-- 
Ron Shaffer
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Marcel Holtmann]

Hi Ron,

> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> connection requests while the ACL is transitioning from sniff mode
> to active mode. Add synchronization so that SCO and eSCO connection
> requests will wait until the ACL has fully transitioned to active mode.
> 
> Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/hci_conn.c         |   18 ++++++++++++++++++
>  net/bluetooth/hci_event.c        |   23 ++++++++++++++++++++++-
>  3 files changed, 41 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index fd53323..c4a37fc 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -250,6 +250,7 @@ enum {
>  	HCI_CONN_ENCRYPT_PEND,
>  	HCI_CONN_RSWITCH_PEND,
>  	HCI_CONN_MODE_CHANGE_PEND,
> +	HCI_CONN_SCO_PEND,
>  };
>  
>  static inline void hci_conn_hash_init(struct hci_dev *hdev)
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 9bf4308..e900f85 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -117,9 +117,18 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
>  {
>  	struct hci_dev *hdev = conn->hdev;
>  	struct hci_cp_add_sco cp;
> +	struct hci_conn *acl = conn->link;
>  
>  	BT_DBG("%p", conn);
>  
> +	if (acl->mode == HCI_CM_SNIFF &&
> +			test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
> +		set_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +		return;
> +	}
> +
> +	clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +
>  	conn->state = BT_CONNECT;
>  	conn->out = 1;
>  
> @@ -135,9 +144,18 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
>  {
>  	struct hci_dev *hdev = conn->hdev;
>  	struct hci_cp_setup_sync_conn cp;
> +	struct hci_conn *acl = conn->link;
>  
>  	BT_DBG("%p", conn);
>  
> +	if (acl->mode == HCI_CM_SNIFF &&
> +			test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
> +		set_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +		return;
> +	}
> +
> +	clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +

I really would prefer test_and_clear_bit() here.

>  	conn->state = BT_CONNECT;
>  	conn->out = 1;
>  
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 3af537a..7692db6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -615,6 +615,7 @@ static void hci_cs_add_sco(struct hci_dev *hdev, __u8 status)
>  	acl = hci_conn_hash_lookup_handle(hdev, handle);
>  	if (acl && (sco = acl->link)) {
>  		sco->state = BT_CLOSED;
> +		clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
>  
>  		hci_proto_connect_cfm(sco, status);
>  		hci_conn_del(sco);
> @@ -760,6 +761,7 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
>  	acl = hci_conn_hash_lookup_handle(hdev, handle);
>  	if (acl && (sco = acl->link)) {
>  		sco->state = BT_CLOSED;
> +		clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
>  
>  		hci_proto_connect_cfm(sco, status);
>  		hci_conn_del(sco);
> @@ -795,6 +797,7 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
>  {
>  	struct hci_cp_exit_sniff_mode *cp;
>  	struct hci_conn *conn;
> +	struct hci_conn *sco;
>  
>  	BT_DBG("%s status 0x%x", hdev->name, status);
>  
> @@ -808,9 +811,17 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
>  	hci_dev_lock(hdev);
>  
>  	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
> -	if (conn)
> +	if (conn) {
>  		clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend);
>  
> +		sco = conn->link;
> +		if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND, &sco->pend)) {
> +				hci_proto_connect_cfm(sco, status);
> +				hci_conn_del(sco);
> +			}
> +		}
> +	}
> +

Something is wrong here. The } are not matching up.

>  	hci_dev_unlock(hdev);
>  }
>  
> @@ -1463,6 +1474,7 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
>  {
>  	struct hci_ev_mode_change *ev = (void *) skb->data;
>  	struct hci_conn *conn;
> +	struct hci_conn *sco;
>  
>  	BT_DBG("%s status %d", hdev->name, ev->status);
>  
> @@ -1478,6 +1490,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
>  				conn->power_save = 1;
>  			else
>  				conn->power_save = 0;
> +		} else {
> +			sco = conn->link;
> +			if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND,
> +					&sco->pend)) {
> +				if (lmp_esco_capable(hdev))
> +					hci_setup_sync(sco, conn->handle);
> +				else
> +					hci_add_sco(sco, conn->handle);
> +			}
>  		}
>  	}
>  

Regards

Marcel

Re: [PATCH 2/3] Bluetooth: Reassigned copyright to Code Aurora Forum

[Marcel Holtmann]

Hi Ron,

> Qualcomm, Inc. has reassigned rights to Code Aurora Forum. Accordingly,
> as files are modified by Code Aurora Forum members, the copyright
> statement will be updated.
> 
> Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
> ---
>  include/net/bluetooth/hci_core.h |    2 +-
>  net/bluetooth/hci_conn.c         |    2 +-
>  net/bluetooth/hci_event.c        |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

patch has been applied. Thanks.

Regards

Marcel

Re: [PATCH 1/3] Bluetooth: Remove extraneous white space

[Marcel Holtmann]

Hi Ron,

> Deleted extraneous white space from the end of several lines
> 
> Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
> ---
>  include/net/bluetooth/hci_core.h |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)

patch has been applied. Thanks.

Regards

Marcel

Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Ron Shaffer]

Certain headsets such as the Motorola H350 will reject SCO and eSCO
connection requests while the ACL is transitioning from sniff mode
to active mode. Add synchronization so that SCO and eSCO connection
requests will wait until the ACL has fully transitioned to active mode.

Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_conn.c         |   18 ++++++++++++++++++
 net/bluetooth/hci_event.c        |   22 +++++++++++++++++++++-
 3 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fd53323..c4a37fc 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,6 +250,7 @@ enum {
 	HCI_CONN_ENCRYPT_PEND,
 	HCI_CONN_RSWITCH_PEND,
 	HCI_CONN_MODE_CHANGE_PEND,
+	HCI_CONN_SCO_PEND,
 };
 
 static inline void hci_conn_hash_init(struct hci_dev *hdev)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 9bf4308..ffc67ac 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,9 +117,18 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_add_sco cp;
+	struct hci_conn *acl = conn->link;
 
 	BT_DBG("%p", conn);
 
+	if (acl->mode == HCI_CM_SNIFF &&
+			test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+		set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+		return;
+	}
+
+	test_and_clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
 	conn->state = BT_CONNECT;
 	conn->out = 1;
 
@@ -135,9 +144,18 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_setup_sync_conn cp;
+	struct hci_conn *acl = conn->link;
 
 	BT_DBG("%p", conn);
 
+	if (acl->mode == HCI_CM_SNIFF &&
+			test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+		set_bit(HCI_CONN_SCO_PEND, &conn->pend);
+		return;
+	}
+
+	test_and_clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
+
 	conn->state = BT_CONNECT;
 	conn->out = 1;
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3af537a..c3a05e1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -615,6 +615,7 @@ static void hci_cs_add_sco(struct hci_dev *hdev, __u8 status)
 	acl = hci_conn_hash_lookup_handle(hdev, handle);
 	if (acl && (sco = acl->link)) {
 		sco->state = BT_CLOSED;
+		clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
 
 		hci_proto_connect_cfm(sco, status);
 		hci_conn_del(sco);
@@ -760,6 +761,7 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
 	acl = hci_conn_hash_lookup_handle(hdev, handle);
 	if (acl && (sco = acl->link)) {
 		sco->state = BT_CLOSED;
+		clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
 
 		hci_proto_connect_cfm(sco, status);
 		hci_conn_del(sco);
@@ -795,6 +797,7 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
 {
 	struct hci_cp_exit_sniff_mode *cp;
 	struct hci_conn *conn;
+	struct hci_conn *sco;
 
 	BT_DBG("%s status 0x%x", hdev->name, status);
 
@@ -808,9 +811,16 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
 	hci_dev_lock(hdev);
 
 	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
-	if (conn)
+	if (conn) {
 		clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend);
 
+		sco = conn->link;
+		if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND, &sco->pend)) {
+				hci_proto_connect_cfm(sco, status);
+				hci_conn_del(sco);
+		}
+	}
+
 	hci_dev_unlock(hdev);
 }
 
@@ -1463,6 +1473,7 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
 {
 	struct hci_ev_mode_change *ev = (void *) skb->data;
 	struct hci_conn *conn;
+	struct hci_conn *sco;
 
 	BT_DBG("%s status %d", hdev->name, ev->status);
 
@@ -1478,6 +1489,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
 				conn->power_save = 1;
 			else
 				conn->power_save = 0;
+		} else {
+			sco = conn->link;
+			if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND,
+					&sco->pend)) {
+				if (lmp_esco_capable(hdev))
+					hci_setup_sync(sco, conn->handle);
+				else
+					hci_add_sco(sco, conn->handle);
+			}
 		}
 	}
 
-- 
1.7.0.2

--
Ron Shaffer
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]

Hi Ron,

> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> connection requests while the ACL is transitioning from sniff mode
> to active mode. Add synchronization so that SCO and eSCO connection
> requests will wait until the ACL has fully transitioned to active mode.

I find your patch actually highly complicated. So I tried to capture
your intend the in the attached patch (only compiled test) and it would
be good if you can try that.

So in general it makes no difference which mode we are in. If a mode
change is pending, then we have to delay the SCO setup. And once we are
done we the mode change we just setup the SCO link. So in theory that
should be enough. Not sure if that is true. I could have overlooked
something since I don't really have tested the patch at all ;)

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 1555 bytes --]

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 600372d..f508d2b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -256,6 +256,7 @@ enum {
 	HCI_CONN_ENCRYPT_PEND,
 	HCI_CONN_RSWITCH_PEND,
 	HCI_CONN_MODE_CHANGE_PEND,
+	HCI_CONN_SCO_SETUP_PEND,
 };
 
 static inline void hci_conn_hash_init(struct hci_dev *hdev)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e9fef83..38c86f6 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -385,6 +385,12 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 		acl->power_save = 1;
 		hci_conn_enter_active_mode(acl);
 
+		if (test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
+			/* defer SCO setup until mode change completed */
+			set_bit(HCI_CONN_SCO_SETUP_PEND, &acl->pend);
+			return sco;
+		}
+
 		if (lmp_esco_capable(hdev))
 			hci_setup_sync(sco, acl->handle);
 		else
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a969800..3500fe8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1481,6 +1481,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
 			else
 				conn->power_save = 0;
 		}
+
+		if (test_and_clear_bit(HCI_CONN_SCO_SETUP_PEND, &conn->pend)) {
+			struct hci_conn *sco = conn->link;
+
+			if (lmp_esco_capable(hdev))
+				hci_setup_sync(sco, conn->handle);
+			else
+				hci_add_sco(sco, conn->handle);
+                }
 	}
 
 	hci_dev_unlock(hdev);

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Perelet, Oleg]

[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]

>I find your patch actually highly complicated. So I tried to capture
>your intend the in the attached patch (only compiled test) and it would
>be good if you can try that.

Marcel, your fix is mode delicate compared to original and has same functionality. 

There's small problem with both yours & Rons - there's no error handling for case when other device will never ACK's unsniff - you pretty much DOS session if there's no reply. No real HS will do that but it may present security flaw. I do not know how severe is that

I attached original conversation when we 1st time seen the problem.
Oleg.
 

-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Marcel Holtmann
Sent: Monday, July 12, 2010 5:08 PM
To: Ron Shaffer
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

Hi Ron,

> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> connection requests while the ACL is transitioning from sniff mode
> to active mode. Add synchronization so that SCO and eSCO connection
> requests will wait until the ACL has fully transitioned to active mode.

I find your patch actually highly complicated. So I tried to capture
your intend the in the attached patch (only compiled test) and it would
be good if you can try that.

So in general it makes no difference which mode we are in. If a mode
change is pending, then we have to delay the SCO setup. And once we are
done we the mode change we just setup the SCO link. So in theory that
should be enough. Not sure if that is true. I could have overlooked
something since I don't really have tested the patch at all ;)

Regards

Marcel


[-- Attachment #2: Type: message/rfc822, Size: 4099 bytes --]

From: "Perelet, Oleg" <operelet@quicinc.com>
To: Nick Pelly <npelly@google.com>, "Shaffer, Ron" <rshaffer@quicinc.com>
Subject: RE: mot hs850
Date: Fri, 19 Mar 2010 10:00:16 -0700

Nick, finally we understand what's wrong with HS830 (I think I called it 850 before:)

It won't accept eSCO(or SCO for that matter, packet types do not apply in this problem) until it ack's unsniff requests - dumb but reality.

This involved a bit more logic after doing:
+               acl->power_save = 1;
+               hci_conn_enter_active_mode(acl);
+
in

http://git.kernel.org/?p=linux/kernel/git/holtmann/bluetooth-2.6.git;a=blobdiff;f=net/bluetooth/hci_conn.c;h=b10e3cdb08f87358ca64d0db8cf83c27f5ad624a;hp=b7c4224f4e7dee01288dd31f4581f7a8821c7a21;hb=c390216b3e868b16d8154939f4b6f8c16dbd9a9f;hpb=88d1a0cf659438a66135661538ae332b23f8635a

you actually need to wait for ack for it before doing hci_setup_sync. Ron extending kernel do to that and we'll mail you patch.

But honestly I think this sort of change should live in userland. Problem is that you do not have libbuetooth up in Java to issue unsniff request and wait for ack for it. Maybe having raw hci going up from java? I do not know - have you thought about this?


Oleg.


-----Original Message-----
From: Nick Pelly [mailto:npelly@google.com]
Sent: Tuesday, March 16, 2010 4:45 PM
To: Shaffer, Ron
Cc: Perelet, Oleg
Subject: Re: mot hs850

On Tue, Mar 16, 2010 at 2:35 PM, Shaffer, Ron <rshaffer@quicinc.com> wrote:
> Hey Nick:
>
>> -----Original Message-----
>> From: Nick Pelly [mailto:npelly@google.com]
>> Sent: Tuesday, March 16, 2010 4:19 PM
>> To: Perelet, Oleg
>> Cc: Shaffer, Ron
>> Subject: Re: mot hs850
>>
>> I haven't tested with that one....what happens?
>
> The issue the we've run into is an eSCO negotiation problem with mot h350.
> It fails to make eSCO or SCO connection after adding in the patch below to
> handle the slow eSCO negotiation when in sniff.
>
> In particular, we request eSCO with 3-EV5, the headset says no let's do 2-EV3,
> then we say how about 3-EV3, and the headset says go pound sand.
>
> It really looks like a headset issue that we're trying to work around.
>
> I took a look at Bluetooth: Allow SCO/eSCO packet type selection for outgoing
> SCO connections,
> http://android.git.kernel.org/?p=kernel/common.git;a=commit;
> h=3b077241e02041b1f03fee912896b8de1d7ac096
>
> and I was wondering if the above behavior is what you saw prior to this patch.

The packet type selection patch was motivated by other headsets, but
can probably be used to resolve this issue as well. I think if you
experiment with disabling 3-EV3 and maybe 3-EV5 packet types with this
headset we might find packets that work. Please let me know if you
find the right packet mask so we can roll it in. With Froyo we are
introducing a config file to set packet type masks with known bad
headsets

I don't want to revert the exit sniff mode patch - it fixes a lot of
slow SCO problems and makes sense.

Nick

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Marcel Holtmann]

Hi Oleg,

> >I find your patch actually highly complicated. So I tried to capture
> >your intend the in the attached patch (only compiled test) and it would
> >be good if you can try that.
> 
> Marcel, your fix is mode delicate compared to original and has same functionality. 
> 
> There's small problem with both yours & Rons - there's no error handling for case when other device will never ACK's unsniff - you pretty much DOS session if there's no reply. No real HS will do that but it may present security flaw. I do not know how severe is that
> 
> I attached original conversation when we 1st time seen the problem.

is there really a problem? The LMP will send an error via HCI. So we do
get the mode changed event, but just with an error. And since we are not
checking the error at all, we just go ahead with the SCO setup attempt
in that case.

Regards

Marcel

Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Matthew Wilson]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

Marcel,

On Jul 14, 2010, at 2:30 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> 
>> 
>> 
> 
> is there really a problem? The LMP will send an error via HCI. So we do
> get the mode changed event, but just with an error. And since we are not
> checking the error at all, we just go ahead with the SCO setup attempt
> in that case.

The attempt to start SCO after the related ACL is no longer present makes no sense.

Either after mode change event with no success or disconnection complete event the result is the same; there is no valid ACL to attempt SCO/eSCO.

Error code 0x02 "unknown connection identifier" may apply for the command status event for setup synchronous connection command but the spec does not require it.

-Matt

[-- Attachment #2: Type: text/html, Size: 2140 bytes --]

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Matt Wilson]

Marcel,

On Wed, 2010-07-14 at 16:30 -0300, Marcel Holtmann wrote:
> is there really a problem? The LMP will send an error via HCI. So we do
> get the mode changed event, but just with an error. And since we are not
> checking the error at all, we just go ahead with the SCO setup attempt
> in that case.
> 

The attempt to start SCO after the related ACL is no longer present
makes no sense. 

Either after mode change event with no success or disconnection complete
event the result is the same; there is no valid ACL to attempt
SCO/eSCO. 

Error code 0x02 "unknown connection identifier" may apply for the
command status event for setup synchronous connection command but the
spec does not require it.

-Matt

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Perelet, Oleg]

Marcel.

>> is there really a problem? The LMP will send an error via HCI. 
>
>The attempt to start SCO after the related ACL is no longer present
>makes no sense.
>
>Either after mode change event with no success or disconnection complete
>event the result is the same; there is no valid ACL to attempt
>SCO/eSCO.
>
>Error code 0x02 "unknown connection identifier" may apply for the
>command status event for setup synchronous connection command but the
>spec does not require it.

Security classification that this case fits is partial DOS for voice services, which is 
considered to be severe for emergency calls even for few seconds.

Depending on chip - It will take awhile (several seconds) before response will pop  
up  and then no matter what we'll still go over sco/esco setup at top of non existing ACL as Matt says.

Because whole thing is in kernel several seconds of audio may get lost before phone(userland) 
will realize that SCO failed and route audio to speaker, if ever:)

I'm not big fan of having all of this complication in kernel because of couple of screwed up headsets. 
There's no problem doing similar logic in userland with tight control over particular HS and timing. 

Oleg.

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Marcel Holtmann]

Hi Matt,

> > is there really a problem? The LMP will send an error via HCI. So we do
> > get the mode changed event, but just with an error. And since we are not
> > checking the error at all, we just go ahead with the SCO setup attempt
> > in that case.
> > 
> 
> The attempt to start SCO after the related ACL is no longer present
> makes no sense. 
> 
> Either after mode change event with no success or disconnection complete
> event the result is the same; there is no valid ACL to attempt
> SCO/eSCO. 
> 
> Error code 0x02 "unknown connection identifier" may apply for the
> command status event for setup synchronous connection command but the
> spec does not require it.

I see the point the the disconnect event and in that case we need to
check if we have an pending SCO setup and remove that.

However what is up with the mode change. If that fails, we are still in
sniff mode and can just proceed with the SCO setup attempt. With the
broken hardware it will fail for sure, but with good behaving LM on the
remote side it might just get the device out of sniff mode at that point
and SCO setup can succeed.

Why do you imply that a mode change event with a failure means that the
ACL is no longer present?

Regards

Marcel

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Marcel Holtmann]

Hi Oleg,

> >> is there really a problem? The LMP will send an error via HCI. 
> >
> >The attempt to start SCO after the related ACL is no longer present
> >makes no sense.
> >
> >Either after mode change event with no success or disconnection complete
> >event the result is the same; there is no valid ACL to attempt
> >SCO/eSCO.
> >
> >Error code 0x02 "unknown connection identifier" may apply for the
> >command status event for setup synchronous connection command but the
> >spec does not require it.
> 
> Security classification that this case fits is partial DOS for voice services, which is 
> considered to be severe for emergency calls even for few seconds.
> 
> Depending on chip - It will take awhile (several seconds) before response will pop  
> up  and then no matter what we'll still go over sco/esco setup at top of non existing ACL as Matt says.
> 
> Because whole thing is in kernel several seconds of audio may get lost before phone(userland) 
> will realize that SCO failed and route audio to speaker, if ever:)
> 
> I'm not big fan of having all of this complication in kernel because of couple of screwed up headsets. 
> There's no problem doing similar logic in userland with tight control over particular HS and timing. 

this might break for a broken headset, but that is than a problem of the
headset. Also until the SCO setup is completed the SCO socket should
never be connected. So userspace can properly detect if SCO setup is in
progress and route it to the speaker with a proper audio policy. So I
think the emergency call argument is void. And broken headset is broken
headset. If it wants to fail, it will fail eventually anyway. The patch
gives it a chance to et this working without breaking other headsets. So
disconnect event arriving on the ACL link and SCO setup pending is
something we need to fix. That is correct. However that is a different
issues actually. Since this can happen right now as well.

Regards

Marcel

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Perelet, Oleg]

>Also until the SCO setup is completed the SCO socket should
>never be connected. So userspace can properly detect if SCO setup is in
>progress and route it to the speaker with a proper audio policy. So I
>think the emergency call argument is void. 

Way it works (most of opensource phone systems) - when phone is "connected" to HS/CarKit with no active Ccall ACL + RFCOMM (for AT commnds) is up, but SCO is down for power savings because there's no active audio. Physically audio is routed to HS all the time while HS ACL is up.

When you enter call state phone will attempt to establish SCO over existing ACL with audio still routed to BT - this is when things get screwed up. 

several seconds later unsniff will fail, ACL will get messed up and disconnected and phone will switch to speaker.

>So userspace can properly detect if SCO setup is in
>progress and route it to the speaker with a proper audio policy.

So during "normal" SCO setup (which can take up to 2.5 secs) you suggest to route audio back to speaker for 2.5 secs and then put it back to BT?:)


Oleg.

Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Ron Shaffer]

Marcel:

On 7/12/2010 5:07 PM, Marcel Holtmann wrote:
> Hi Ron,
> 
>> Certain headsets such as the Motorola H350 will reject SCO and eSCO
>> connection requests while the ACL is transitioning from sniff mode
>> to active mode. Add synchronization so that SCO and eSCO connection
>> requests will wait until the ACL has fully transitioned to active mode.
> 
> I find your patch actually highly complicated. So I tried to capture
> your intend the in the attached patch (only compiled test) and it would
> be good if you can try that.
> 
> So in general it makes no difference which mode we are in. If a mode
> change is pending, then we have to delay the SCO setup. And once we are
> done we the mode change we just setup the SCO link. So in theory that
> should be enough. Not sure if that is true. I could have overlooked
> something since I don't really have tested the patch at all ;)
> 
> Regards
> 
> Marcel
> 

Would have been back with you sooner on this but I'm having some
hardware issues preventing me from testing your code. I'm fairly
confident it works as you've consolidated the pending check into
hci_conn which I like.

However, the complicated pieces to which I think you are referring are
the error handling pieces which we should want want to keep. These make
sure that we clean up appropriately if:

1. The HW rejects the exit sniff or the SCO/eSCO request for some reason
i.e. in the case the HW returns a command status with a failure
2. In the latter case, the user space process is also notified that its
SCO/eSCO request has failed.

Give the cleanup code some thought. I think you'll see we want this
there to make the system more reliable.

As soon as I've confirmed your patch I'll get back to you.

In the meantime, I'll talk with Oleg and Matt regarding their responses.

-- 
Ron Shaffer
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Marcel Holtmann]

Hi Ron,

> >> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> >> connection requests while the ACL is transitioning from sniff mode
> >> to active mode. Add synchronization so that SCO and eSCO connection
> >> requests will wait until the ACL has fully transitioned to active mode.
> > 
> > I find your patch actually highly complicated. So I tried to capture
> > your intend the in the attached patch (only compiled test) and it would
> > be good if you can try that.
> > 
> > So in general it makes no difference which mode we are in. If a mode
> > change is pending, then we have to delay the SCO setup. And once we are
> > done we the mode change we just setup the SCO link. So in theory that
> > should be enough. Not sure if that is true. I could have overlooked
> > something since I don't really have tested the patch at all ;)
>
> Would have been back with you sooner on this but I'm having some
> hardware issues preventing me from testing your code. I'm fairly
> confident it works as you've consolidated the pending check into
> hci_conn which I like.
> 
> However, the complicated pieces to which I think you are referring are
> the error handling pieces which we should want want to keep. These make
> sure that we clean up appropriately if:
> 
> 1. The HW rejects the exit sniff or the SCO/eSCO request for some reason
> i.e. in the case the HW returns a command status with a failure
> 2. In the latter case, the user space process is also notified that its
> SCO/eSCO request has failed.
> 
> Give the cleanup code some thought. I think you'll see we want this
> there to make the system more reliable.
> 
> As soon as I've confirmed your patch I'll get back to you.

I think the general discussion here is what we expect a normal behaving
system to do and what would be a good idea.

So we already have the case where just always unsniff the ACL link
before establishing any SCO link. If that assumption still holds then I
think it is more than fair to wait for the mode change event as well.

My point is just that when we receive the mode change event, we don't
care if the unsniff (or unpark/unhold) for that matter was successful.
We proceed with the SCO setup.

The change is like this "if a mode change is in progress, then wait
until it finished before trying to setup SCO". I think that is a proper
way of doing this anyway. It really doesn't change anything else. If the
link is active anyway, then nothing has been changed here in the first
place.

And yes, in case of a HCI disconnect event we should clear out the
pending SCO setup request. However that is done anyway, since this flag
is stored inside the ACL connection handle. Right now I really don't see
an issue with this patch.

For the potential security concern, yes that can happen, but lets be
fair, we can't prevent this from the host anyway. Same as someone could
just block the 2.4 GHz spectrum and will disrupt calls.

Regards

Marcel

RE: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state

[Matt Wilson]

Hi Marcel,

> However what is up with the mode change. If that fails, we are still in
> sniff mode and can just proceed with the SCO setup attempt. With the
> broken hardware it will fail for sure, but with good behaving LM on the
> remote side it might just get the device out of sniff mode at that point
> and SCO setup can succeed.
> 
> Why do you imply that a mode change event with a failure means that the
> ACL is no longer present?

I was thinking the Mode Change event would carry a LMP response timeout
or link supervision (LS) timeout status when the two devices lose
synchronization while in sniff just after sending exit sniff LMP PDU
(only baseband ACK). It is more clear when the LS timeout is greater
than the 30 second (immutable) LMP response timeout.

But reading the LMP spec (2.4.1) I conclude that the former cannot occur
in the exit sniff scenario and the Disconnection Complete event is the
only required event in the latter.

I agree to wait until we have a clear and real scenario and then discuss
further with those logs.

Best regards,

-Matt

Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.