Skip to contentSkip to conversation

sco: SCO socket option for voice_setting

PATCH Superseded Frédéric Dalleau 3 participants 16 messages May 16, 2013 – May 17, 2013

Subsystems (2)

v7 of 11 Superseded Latest is v11 →

lore
Share & tools
lore
#0 lore raw

v7 cover sco: SCO socket option for voice_setting

Hi,

v7 changes defaults to BT_VOICE_CVSD
Remove mask parameter to sco_conn_defer_accept, it was always 0 check the bits for air codec instead of use constants.
Add S3, S2, S1, D1, D0 settings.
The controller default is now only used to initialize the controller or fill in the missing information in case of using the old Add_SCO command

v6 fixes style issues

v5 changes interface to SOL_BLUETOOTH, BT_VOICE.
Rework fallback mechanism.

This is the patch version 4 of the socket option for enabling transparent SCO. As requested by Marcel, this is now a 16-bit voice_setting. 0x0000 is the value corresponding to current behavior.
0x0003 is the value to use for enabling transparent data.
It is easy to allow all possible values from Bluetooth core spec, but I guess results can be unexpected...
Should we filter allowed values in setsockopt ?

Let me know what you think.
Regards,
Fred

Frédéric Dalleau (8):
Bluetooth: Use hci_connect_sco directly
Bluetooth: Remove unused mask parameter in sco_conn_defer_accept
Bluetooth: Add bluetooth socket voice option
Bluetooth: Constants and macro declaration for SCO airmode
Bluetooth: Use voice setting in incoming SCO connection
Bluetooth: Parameters for outgoing SCO connections
Bluetooth: Fallback transparent SCO from T2 to T1
Bluetooth: Add support for fallback between CVSD recommended settings

 include/net/bluetooth/bluetooth.h |    8 +++++
 include/net/bluetooth/hci_core.h  |    9 +++++
 include/net/bluetooth/sco.h       |    1 +
 net/bluetooth/hci_conn.c          |   56 +++++++++++++++++++++++------
 net/bluetooth/hci_event.c         |    3 +-
 net/bluetooth/sco.c               |   70 ++++++++++++++++++++++++++++++-------
 6 files changed, 123 insertions(+), 24 deletions(-)
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH v7 1/8 Bluetooth: Use hci_connect_sco directly

hci_connect is a super function for connecting hci protocols. But the voice_setting parameter is only needed by SCO and security requirements are not needed for SCO channels. Thus, it makes sense to have a separate function.

---
 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_conn.c         |    9 +++------
 net/bluetooth/sco.c              |    3 +--
 3 files changed, 6 insertions(+), 8 deletions(-)
include/net/bluetooth/hci_core.h +2 -0
index 606615c..e9cf9fa 100644
@@ -584,6 +584,8 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle);
 
 struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
 			     __u8 dst_type, __u8 sec_level, __u8 auth_type);
+struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
+				 __u16 setting);
 int hci_conn_check_link_mode(struct hci_conn *conn);
 int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
 int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
net/bluetooth/hci_conn.c +3 -6
index 6c7f363..d1d9919 100644
@@ -560,13 +560,13 @@ static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
 	return acl;
 }
 
-static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
-				bdaddr_t *dst, u8 sec_level, u8 auth_type)
+struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
+				 __u16 setting)
 {
 	struct hci_conn *acl;
 	struct hci_conn *sco;
 
-	acl = hci_connect_acl(hdev, dst, sec_level, auth_type);
+	acl = hci_connect_acl(hdev, dst, BT_SECURITY_LOW, HCI_AT_NO_BONDING);
 	if (IS_ERR(acl))
 		return acl;
 
@@ -612,9 +612,6 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
 		return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
 	case ACL_LINK:
 		return hci_connect_acl(hdev, dst, sec_level, auth_type);
-	case SCO_LINK:
-	case ESCO_LINK:
-		return hci_connect_sco(hdev, type, dst, sec_level, auth_type);
 	}
 
 	return ERR_PTR(-EINVAL);
net/bluetooth/sco.c +1 -2
index 9e62102..a85182f 100644
@@ -176,8 +176,7 @@ static int sco_connect(struct sock *sk)
 	else
 		type = SCO_LINK;
 
-	hcon = hci_connect(hdev, type, dst, BDADDR_BREDR, BT_SECURITY_LOW,
-			   HCI_AT_NO_BONDING);
+	hcon = hci_connect_sco(hdev, type, dst, sco_pi(sk)->setting);
 	if (IS_ERR(hcon)) {
 		err = PTR_ERR(hcon);
 		goto done;
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH v7 2/8 Bluetooth: Remove unused mask parameter in sco_conn_defer_accept

In reply to Frédéric Dalleau
---
 net/bluetooth/sco.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
net/bluetooth/sco.c +3 -7
index a85182f..26c9d51 100644
@@ -651,7 +651,7 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	return err;
 }
 
-static void sco_conn_defer_accept(struct hci_conn *conn, int mask)
+static void sco_conn_defer_accept(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
 
@@ -663,11 +663,7 @@ static void sco_conn_defer_accept(struct hci_conn *conn, int mask)
 		struct hci_cp_accept_conn_req cp;
 
 		bacpy(&cp.bdaddr, &conn->dst);
-
-		if (lmp_rswitch_capable(hdev) && (mask & HCI_LM_MASTER))
-			cp.role = 0x00; /* Become master */
-		else
-			cp.role = 0x01; /* Remain slave */
+		cp.role = 0x01; /* Remain slave */
 
 		hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), &cp);
 	} else {
@@ -697,7 +693,7 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 
 	if (sk->sk_state == BT_CONNECT2 &&
 	    test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
-		sco_conn_defer_accept(pi->conn->hcon, 0);
+		sco_conn_defer_accept(pi->conn->hcon);
 		sk->sk_state = BT_CONFIG;
 
 		release_sock(sk);
Signature · 1 line
1.7.9.5
#4 lore raw

PATCH v7 3/8 Bluetooth: Add bluetooth socket voice option

In reply to Frédéric Dalleau

This patch extends the current bluetooth socket option to add BT_VOICE. This is intended to choose voice data type at runtime. It only applies to SCO sockets.
Incoming connections shall be setup during defered setup. Outgoing connections shall be setup before connect(). The desired setting is stored in the sco socket info.
This patch declares needed members, modifies getsockopt() and setsockopt().

---
 include/net/bluetooth/bluetooth.h |    8 ++++++++
 include/net/bluetooth/sco.h       |    1 +
 net/bluetooth/sco.c               |   38 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 46 insertions(+), 1 deletion(-)
include/net/bluetooth/bluetooth.h +8 -0
index 6912ef9..0239a49 100644
@@ -107,6 +107,14 @@ struct bt_power {
  */
 #define BT_CHANNEL_POLICY_AMP_PREFERRED		2
 
+#define BT_VOICE		11
+struct bt_voice {
+	__u16 setting;
+};
+
+#define BT_VOICE_TRANSPARENT			0x0003
+#define BT_VOICE_CVSD				0x0060
+
 __printf(1, 2)
 int bt_info(const char *fmt, ...);
 __printf(1, 2)
include/net/bluetooth/sco.h +1 -0
index 1e35c43..e252a31 100644
@@ -73,6 +73,7 @@ struct sco_conn {
 struct sco_pinfo {
 	struct bt_sock	bt;
 	__u32		flags;
+	__u16		setting;
 	struct sco_conn	*conn;
 };
 
net/bluetooth/sco.c +37 -1
index 26c9d51..7b4c3df 100644
@@ -416,6 +416,8 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock, int pro
 	sk->sk_protocol = proto;
 	sk->sk_state    = BT_OPEN;
 
+	sco_pi(sk)->setting = BT_VOICE_CVSD;
+
 	setup_timer(&sk->sk_timer, sco_sock_timeout, (unsigned long)sk);
 
 	bt_sock_link(&sco_sk_list, sk);
@@ -708,7 +710,8 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
 {
 	struct sock *sk = sock->sk;
-	int err = 0;
+	int len, err = 0;
+	struct bt_voice voice;
 	u32 opt;
 
 	BT_DBG("sk %p", sk);
@@ -734,6 +737,29 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char
 			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
 		break;
 
+	case BT_VOICE:
+		if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND &&
+		    sk->sk_state != BT_CONNECT2) {
+			err = -EINVAL;
+			break;
+		}
+
+		len = min_t(unsigned int, sizeof(voice), optlen);
+		if (copy_from_user((char *) &voice, optval, len)) {
+			err = -EFAULT;
+			break;
+		}
+
+
+		if (voice.setting != BT_VOICE_TRANSPARENT &&
+		    voice.setting != BT_VOICE_CVSD) {
+			err = -EINVAL;
+			break;
+		}
+
+		sco_pi(sk)->setting = voice.setting;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -803,6 +829,7 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 {
 	struct sock *sk = sock->sk;
 	int len, err = 0;
+	struct bt_voice voice;
 
 	BT_DBG("sk %p", sk);
 
@@ -828,6 +855,15 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 
 		break;
 
+	case BT_VOICE:
+		voice.setting = sco_pi(sk)->setting;
+
+		len = min_t(unsigned int, len, sizeof(voice));
+		if (copy_to_user(optval, (char *)&voice, len))
+			err = -EFAULT;
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
Signature · 1 line
1.7.9.5
maintainer
#5 lore raw

Re: [PATCH v7 3/8] Bluetooth: Add bluetooth socket voice option

Hi Fred,

Quoted 20 lines

This patch extends the current bluetooth socket option to add BT_VOICE. This is intended to choose voice data type at runtime. It only applies to SCO sockets.
Incoming connections shall be setup during defered setup. Outgoing connections shall be setup before connect(). The desired setting is stored in the sco socket info.
This patch declares needed members, modifies getsockopt() and setsockopt().

---

include/net/bluetooth/bluetooth.h |    8 ++++++++
include/net/bluetooth/sco.h       |    1 +
net/bluetooth/sco.c               |   38 ++++++++++++++++++++++++++++++++++++-
3 files changed, 46 insertions(+), 1 deletion(-)
include/net/bluetooth/bluetooth.h +0 -0
index 6912ef9..0239a49 100644

include/net/bluetooth/bluetooth.h struct bt_power { · line 116

Quoted 10 lines
include/net/bluetooth/bluetooth.h +7 -0
@@ -107,6 +107,14 @@ struct bt_power {
 */
 #define BT_CHANNEL_POLICY_AMP_PREFERRED		2
 
+#define BT_VOICE		11
+struct bt_voice {
+	__u16 setting;
+};
+
+#define BT_VOICE_TRANSPARENT			0x0003
+#define BT_VOICE_CVSD				0x0060

call this BT_VOICE_CVSD_16BIT. You could run this with CVSD_8BIT as well if we ever care to get that fixed.

Quoted 40 lines
include/net/bluetooth/bluetooth.h +1 -0
+
 __printf(1, 2)
 int bt_info(const char *fmt, ...);
 __printf(1, 2)
include/net/bluetooth/sco.h +1 -0
index 1e35c43..e252a31 100644
@@ -73,6 +73,7 @@ struct sco_conn {
 struct sco_pinfo {
 	struct bt_sock	bt;
 	__u32		flags;
+	__u16		setting;
 	struct sco_conn	*conn;
 };
 
net/bluetooth/sco.c +4 -1
index 26c9d51..7b4c3df 100644
@@ -416,6 +416,8 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock, int pro
 	sk->sk_protocol = proto;
 	sk->sk_state    = BT_OPEN;
 
+	sco_pi(sk)->setting = BT_VOICE_CVSD;
+
 	setup_timer(&sk->sk_timer, sco_sock_timeout, (unsigned long)sk);
 
 	bt_sock_link(&sco_sk_list, sk);
@@ -708,7 +710,8 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
 {
 	struct sock *sk = sock->sk;
-	int err = 0;
+	int len, err = 0;
+	struct bt_voice voice;
 	u32 opt;
 
 	BT_DBG("sk %p", sk);

net/bluetooth/sco.c static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char · line 746

Quoted 10 lines
net/bluetooth/sco.c +7 -0
@@ -734,6 +737,29 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char
 			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
 		break;
 
+	case BT_VOICE:
+		if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND &&
+		    sk->sk_state != BT_CONNECT2) {
+			err = -EINVAL;
+			break;
+		}
+

Actually you need to set defaults here first. Otherwise updated userspaces with more options might not work properly.

		voice.setting = sco_pi(sk)->setting.

net/bluetooth/sco.c static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char · line 753

Quoted 7 lines
net/bluetooth/sco.c +7 -0
+		len = min_t(unsigned int, sizeof(voice), optlen);
+		if (copy_from_user((char *) &voice, optval, len)) {
+			err = -EFAULT;
+			break;
+		}
+
+

Double empty line here. I assume you wanted to add a comment that for now we are only accepting two settings at the moment.

Quoted 26 lines
net/bluetooth/sco.c +12 -0
+		if (voice.setting != BT_VOICE_TRANSPARENT &&
+		    voice.setting != BT_VOICE_CVSD) {
+			err = -EINVAL;
+			break;
+		}
+
+		sco_pi(sk)->setting = voice.setting;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -803,6 +829,7 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 {
 	struct sock *sk = sock->sk;
 	int len, err = 0;
+	struct bt_voice voice;
 
 	BT_DBG("sk %p", sk);
 
@@ -828,6 +855,15 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 
 		break;
 
+	case BT_VOICE:
+		voice.setting = sco_pi(sk)->setting;

net/bluetooth/sco.c static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char · line 869

Quoted 10 lines
net/bluetooth/sco.c +7 -0
+
+		len = min_t(unsigned int, len, sizeof(voice));
+		if (copy_to_user(optval, (char *)&voice, len))
+			err = -EFAULT;
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;

Regards

Marcel

#6 lore raw

PATCH v7 4/8 Bluetooth: Constants and macro declaration for SCO airmode

In reply to Frédéric Dalleau

This patchs define constants and macro for extracting SCO airmode from SCO voice setting. It refers to Bluetooth Core V4.0 specification, Part E, Chap 6.12 which describe SCO voice setting format.

---
 include/net/bluetooth/hci_core.h |    5 +++++
 1 file changed, 5 insertions(+)
include/net/bluetooth/hci_core.h +5 -0
index e9cf9fa..a6fd544 100644
@@ -1214,4 +1214,9 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 
 u8 bdaddr_to_le(u8 bdaddr_type);
 
+#define SCO_AIRMODE_MASK       0x0003
+#define SCO_AIRMODE_CVSD       0x0000
+#define SCO_AIRMODE_TRANSP     0x0003
+#define hci_sco_airmode(setting) ((setting) & SCO_AIRMODE_MASK)
+
 #endif /* __HCI_CORE_H */
Signature · 1 line
1.7.9.5
maintainer
#7 lore raw

Re: [PATCH v7 4/8] Bluetooth: Constants and macro declaration for SCO airmode

Quoted 14 lines · Frédéric Dalleau

This patchs define constants and macro for extracting SCO airmode from SCO voice setting. It refers to Bluetooth Core V4.0 specification, Part E, Chap 6.12 which describe SCO voice setting format.

---
include/net/bluetooth/hci_core.h | 5 +++++

1 file changed, 5 insertions(+)
include/net/bluetooth/hci_core.h +0 -0
index e9cf9fa..a6fd544 100644

include/net/bluetooth/hci_core.h void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8], · line 1220

Quoted 7 lines
include/net/bluetooth/hci_core.h +4 -0
@@ -1214,4 +1214,9 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 
 u8 bdaddr_to_le(u8 bdaddr_type);
 
+#define SCO_AIRMODE_MASK       0x0003
+#define SCO_AIRMODE_CVSD       0x0000
+#define SCO_AIRMODE_TRANSP     0x0003
+#define hci_sco_airmode(setting) ((setting) & SCO_AIRMODE_MASK)

the constants are fine, but I would not bother with the macro. Just use SCO_AIRMODE_MASK directly.

Regards

Marcel

#8 lore raw

PATCH v7 5/8 Bluetooth: Use voice setting in incoming SCO connection

In reply to Frédéric Dalleau

When an incoming SCO connection is requested, check the selected voice setting, and reply appropriately. Voice setting should have been negotiated previously. For example, in case of HFP, the codec is negotiated using AT commands on the RFCOMM channel. This patch only changes replies for socket with defered setup enabled.

---
 net/bluetooth/sco.c |   23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)
net/bluetooth/sco.c +18 -5
index 7b4c3df..7881208 100644
@@ -653,7 +653,7 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	return err;
 }
 
-static void sco_conn_defer_accept(struct hci_conn *conn)
+static void sco_conn_defer_accept(struct hci_conn *conn, int setting)
 {
 	struct hci_dev *hdev = conn->hdev;
 
@@ -676,9 +676,22 @@ static void sco_conn_defer_accept(struct hci_conn *conn)
 
 		cp.tx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
 		cp.rx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
-		cp.max_latency    = __constant_cpu_to_le16(0xffff);
-		cp.content_format = cpu_to_le16(hdev->voice_setting);
-		cp.retrans_effort = 0xff;
+
+		switch (hci_sco_airmode(setting)) {
+		case SCO_AIRMODE_TRANSP:
+			if (conn->pkt_type & ESCO_2EV3)
+				cp.max_latency = __constant_cpu_to_le16(0x0008);
+			else
+				cp.max_latency = __constant_cpu_to_le16(0x000D);
+			cp.content_format = __constant_cpu_to_le16(SCO_AIRMODE_TRANSP);
+			cp.retrans_effort = 0x02;
+			break;
+		case SCO_AIRMODE_CVSD:
+			cp.max_latency    = __constant_cpu_to_le16(0xffff);
+			cp.content_format = cpu_to_le16(setting);
+			cp.retrans_effort = 0xff;
+			break;
+		}
 
 		hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
 			     sizeof(cp), &cp);
@@ -695,7 +708,7 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 
 	if (sk->sk_state == BT_CONNECT2 &&
 	    test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
-		sco_conn_defer_accept(pi->conn->hcon);
+		sco_conn_defer_accept(pi->conn->hcon, pi->setting);
 		sk->sk_state = BT_CONFIG;
 
 		release_sock(sk);
Signature · 1 line
1.7.9.5
maintainer
#9 lore raw

Re: [PATCH v7 5/8] Bluetooth: Use voice setting in incoming SCO connection

Hi Fred,

Quoted 25 lines

When an incoming SCO connection is requested, check the selected voice setting, and reply appropriately. Voice setting should have been negotiated previously. For example, in case of HFP, the codec is negotiated using AT commands on the RFCOMM channel. This patch only changes replies for socket with defered setup enabled.

---
net/bluetooth/sco.c | 23 ++++++++++++++++++-----

1 file changed, 18 insertions(+), 5 deletions(-)
net/bluetooth/sco.c +1 -1
index 7b4c3df..7881208 100644
@@ -653,7 +653,7 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	return err;
 }
 
-static void sco_conn_defer_accept(struct hci_conn *conn)
+static void sco_conn_defer_accept(struct hci_conn *conn, int setting)
 {
 	struct hci_dev *hdev = conn->hdev;
 

net/bluetooth/sco.c static void sco_conn_defer_accept(struct hci_conn *conn) · line 681

Quoted 6 lines
net/bluetooth/sco.c +0 -3
@@ -676,9 +676,22 @@ static void sco_conn_defer_accept(struct hci_conn *conn)
 
 		cp.tx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
 		cp.rx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
-		cp.max_latency    = __constant_cpu_to_le16(0xffff);
-		cp.content_format = cpu_to_le16(hdev->voice_setting);
-		cp.retrans_effort = 0xff;

so we are now ignoring the hdev->voice_setting. This is fine for eSCO, but what are we doing in case we have a 1.1 controller that only supports the Add_SCO command?

Regards

Marcel

#10 lore raw

PATCH v7 6/8 Bluetooth: Parameters for outgoing SCO connections

In reply to Frédéric Dalleau

In order to establish a transparent SCO connection, the correct settings must be specified in the Setup Synchronous Connection request. For that, voice_setting is added to ACL connection flags to set up the desired parameters. If this value is zero, a legacy SCO connection will be requested. This patch uses T2 settings.

---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_conn.c         |   22 ++++++++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)
include/net/bluetooth/hci_core.h +1 -0
index a6fd544..b5996b3 100644
@@ -320,6 +320,7 @@ struct hci_conn {
 	__u32		passkey_notify;
 	__u8		passkey_entered;
 	__u16		disc_timeout;
+	__u16		setting;
 	unsigned long	flags;
 
 	__u8		remote_cap;
net/bluetooth/hci_conn.c +18 -4
index d1d9919..cd7452e 100644
@@ -185,13 +185,25 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 	conn->attempt++;
 
 	cp.handle   = cpu_to_le16(handle);
-	cp.pkt_type = cpu_to_le16(conn->pkt_type);
 
 	cp.tx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
 	cp.rx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
-	cp.max_latency    = __constant_cpu_to_le16(0xffff);
-	cp.voice_setting  = cpu_to_le16(hdev->voice_setting);
-	cp.retrans_effort = 0xff;
+
+	switch (hci_sco_airmode(conn->setting)) {
+	case SCO_AIRMODE_TRANSP:
+		cp.pkt_type       = __constant_cpu_to_le16(EDR_ESCO_MASK &
+							   ~ESCO_2EV3);
+		cp.max_latency    = __constant_cpu_to_le16(0x000d);
+		cp.voice_setting  = __constant_cpu_to_le16(SCO_AIRMODE_TRANSP);
+		cp.retrans_effort = 0x02;
+		break;
+	case SCO_AIRMODE_CVSD:
+		cp.pkt_type       = cpu_to_le16(conn->pkt_type);
+		cp.max_latency    = __constant_cpu_to_le16(0xffff);
+		cp.voice_setting  = cpu_to_le16(hdev->voice_setting);
+		cp.retrans_effort = 0xff;
+		break;
+	}
 
 	hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
 }
@@ -584,6 +596,8 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
 
 	hci_conn_hold(sco);
 
+	sco->setting = setting;
+
 	if (acl->state == BT_CONNECTED &&
 	    (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
 		set_bit(HCI_CONN_POWER_SAVE, &acl->flags);
Signature · 1 line
1.7.9.5
maintainer
#11 lore raw

Re: [PATCH v7 6/8] Bluetooth: Parameters for outgoing SCO connections

Hi Fred,

Quoted 29 lines

In order to establish a transparent SCO connection, the correct settings must be specified in the Setup Synchronous Connection request. For that, voice_setting is added to ACL connection flags to set up the desired parameters. If this value is zero, a legacy SCO connection will be requested. This patch uses T2 settings.

---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 22 ++++++++++++++++++----

2 files changed, 19 insertions(+), 4 deletions(-)
include/net/bluetooth/hci_core.h +1 -0
index a6fd544..b5996b3 100644
@@ -320,6 +320,7 @@ struct hci_conn {
 	__u32		passkey_notify;
 	__u8		passkey_entered;
 	__u16		disc_timeout;
+	__u16		setting;
 	unsigned long	flags;
 
 	__u8		remote_cap;
net/bluetooth/hci_conn.c +0 -0
index d1d9919..cd7452e 100644

net/bluetooth/hci_conn.c void hci_setup_sync(struct hci_conn *conn, __u16 handle) · line 194

Quoted 10 lines
net/bluetooth/hci_conn.c +0 -4
@@ -185,13 +185,25 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 	conn->attempt++;
 
 	cp.handle   = cpu_to_le16(handle);
-	cp.pkt_type = cpu_to_le16(conn->pkt_type);
 
 	cp.tx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
 	cp.rx_bandwidth   = __constant_cpu_to_le32(0x00001f40);
-	cp.max_latency    = __constant_cpu_to_le16(0xffff);
-	cp.voice_setting  = cpu_to_le16(hdev->voice_setting);
-	cp.retrans_effort = 0xff;

same here. What are we doing with Add_SCO fallback on older controllers.

We could just reject setting transparent air mode for controllers that do not support eSCO command procedures. And the only valid option there would be CVSD then. Which is default.

In case we use Add_SCO is is important that the voice setting programmed into the controller is actually 16-bit CVSD at that time. That needs to be taken care of as well.

Regards

Marcel

#12 lore raw

Re: [PATCH v7 6/8] Bluetooth: Parameters for outgoing SCO connections

Hi,

On Fri, May 17, 2013 at 5:55 AM, Marcel Holtmann <[email protected]> wrot= e:

Quoted 3 lines

Hi Fred,

Quoted 1 line

In order to establish a transparent SCO connection, the correct settings=

must

Quoted 3 lines
Quoted 3 lines

be specified in the Setup Synchronous Connection request. For that, voice_setting is added to ACL connection flags to set up the desired parameters. If this value is zero, a legacy SCO connection will be reque=

sted.

Quoted 3 lines
Quoted 3 lines

This patch uses T2 settings.

com>

Quoted 6 lines

include/net/bluetooth/hc=

Quoted 6 lines

---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 22 ++++++++++++++++++----

2 files changed, 19 insertions(+), 4 deletions(-)
include/net/bluetooth/hc= renamed from include/net/bluetooth/hci_core.h +0 -0

i_core.h

Quoted 10 lines
Quoted 10 lines

index a6fd544..b5996b3 100644

include/net/bluetooth/hci_core.h +1 -0
@@ -320,6 +320,7 @@ struct hci_conn {
      __u32           passkey_notify;
      __u8            passkey_entered;
      __u16           disc_timeout;
+     __u16           setting;
      unsigned long   flags;
 
Quoted 6 lines

net/bluetooth/hci_conn.c void hci_setup_sync(struct hci_conn *conn, __u16 h=

Quoted 6 lines
include/net/bluetooth/hci_core.h +0 -0
      __u8            remote_cap;
net/bluetooth/hci_conn.c +0 -0
index d1d9919..cd7452e 100644
@@ -185,13 +185,25 @@ void hci_setup_sync(struct hci_conn *conn, __u16 h=

andle)

Quoted 11 lines

net/bluetooth/hci_conn.c void hci_setup_sync(struct hci_conn *conn, __u16 h= · line 194

Quoted 10 lines
net/bluetooth/hci_conn.c +0 -4
      conn->attempt++;
 
      cp.handle   =3D cpu_to_le16(handle);
-     cp.pkt_type =3D cpu_to_le16(conn->pkt_type);
 
      cp.tx_bandwidth   =3D __constant_cpu_to_le32(0x00001f40);
      cp.rx_bandwidth   =3D __constant_cpu_to_le32(0x00001f40);
-     cp.max_latency    =3D __constant_cpu_to_le16(0xffff);
-     cp.voice_setting  =3D cpu_to_le16(hdev->voice_setting);
-     cp.retrans_effort =3D 0xff;
Quoted 3 lines

same here. What are we doing with Add_SCO fallback on older controllers.

We could just reject setting transparent air mode for controllers that do=

not support eSCO command procedures. And the only valid option there would= be CVSD then. Which is default.

I have seem some controllers (IIRC CSR 2.0) that don't have eSCO but have support for transparent air mode, in these cases we want to use the Setup SCO/Accept SCO procedures.

Quoted 2 lines

In case we use Add_SCO is is important that the voice setting programmed =

into the controller is actually 16-bit CVSD at that time. That needs to be = taken care of as well.

Quoted 7 lines

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=

" in

Quoted 2 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Cheers,
--
Vinicius

#13 lore raw

PATCH v7 7/8 Bluetooth: Fallback transparent SCO from T2 to T1

In reply to Frédéric Dalleau

When initiating a transparent eSCO connection, make use of T2 settings at first try. T2 is the recommended settings from HFP 1.6 WideBand Speech. Upon connection failure, try T1 settings.
T2 failure is detected if Synchronous Connection Complete event fails with error 0x0d. This error code has been found experimentally by sending a T2 request to a T1 only SCO listener. It means "Connection Rejected due to Limited resource".
To know which of T2 or T1 should be used, conn->fallback is used. Bluez only attempt to reconnect twice. In the future, we will need more than 2 fallback possibilities. Thus, test conn->fallback as an alternative measure. We want to fallback only if conn->fallback is positive. Calling hci_setup_sync with conn->fallback == 0 is an initial connection attempt.

---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_conn.c         |   20 +++++++++++++++++---
 net/bluetooth/hci_event.c        |    3 ++-
 3 files changed, 20 insertions(+), 4 deletions(-)
include/net/bluetooth/hci_core.h +1 -0
index b5996b3..c8b91d2 100644
@@ -321,6 +321,7 @@ struct hci_conn {
 	__u8		passkey_entered;
 	__u16		disc_timeout;
 	__u16		setting;
+	__u8		fallback;
 	unsigned long	flags;
 
 	__u8		remote_cap;
net/bluetooth/hci_conn.c +17 -3
index cd7452e..8d71d73 100644
@@ -31,6 +31,17 @@
 #include <net/bluetooth/a2mp.h>
 #include <net/bluetooth/smp.h>
 
+struct sco_param {
+	u16 pkt_type;
+	u16 max_latency;
+	u8 next;
+};
+
+static const struct sco_param sco_param_wideband[] = {
+	{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x000d, 1 },
+	{ EDR_ESCO_MASK | ESCO_EV3,   0x0008, 0 },
+};
+
 static void hci_le_create_connection(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -176,6 +187,7 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_setup_sync_conn cp;
+	const struct sco_param *param;
 
 	BT_DBG("hcon %p", conn);
 
@@ -191,11 +203,13 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 
 	switch (hci_sco_airmode(conn->setting)) {
 	case SCO_AIRMODE_TRANSP:
-		cp.pkt_type       = __constant_cpu_to_le16(EDR_ESCO_MASK &
-							   ~ESCO_2EV3);
-		cp.max_latency    = __constant_cpu_to_le16(0x000d);
+		param = &sco_param_wideband[conn->fallback];
+		cp.pkt_type       = __constant_cpu_to_le16(param->pkt_type);
+		cp.max_latency    = __constant_cpu_to_le16(param->max_latency);
 		cp.voice_setting  = __constant_cpu_to_le16(SCO_AIRMODE_TRANSP);
 		cp.retrans_effort = 0x02;
+
+		conn->fallback    = param->next;
 		break;
 	case SCO_AIRMODE_CVSD:
 		cp.pkt_type       = cpu_to_le16(conn->pkt_type);
net/bluetooth/hci_event.c +2 -1
index db58e72..9be10be 100644
@@ -2904,11 +2904,12 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
 		hci_conn_add_sysfs(conn);
 		break;
 
+	case 0x0d:	/* No resource available */
 	case 0x11:	/* Unsupported Feature or Parameter Value */
 	case 0x1c:	/* SCO interval rejected */
 	case 0x1a:	/* Unsupported Remote Feature */
 	case 0x1f:	/* Unspecified error */
-		if (conn->out && conn->attempt < 2) {
+		if (conn->out && (conn->attempt < 2 || conn->fallback > 0)) {
 			conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
 					(hdev->esco_type & EDR_ESCO_MASK);
 			hci_setup_sync(conn, conn->link->handle);
Signature · 1 line
1.7.9.5
maintainer
#14 lore raw

Re: [PATCH v7 7/8] Bluetooth: Fallback transparent SCO from T2 to T1

Hi Fred,

Quoted 41 lines

When initiating a transparent eSCO connection, make use of T2 settings at first try. T2 is the recommended settings from HFP 1.6 WideBand Speech. Upon connection failure, try T1 settings.
T2 failure is detected if Synchronous Connection Complete event fails with error 0x0d. This error code has been found experimentally by sending a T2 request to a T1 only SCO listener. It means "Connection Rejected due to Limited resource".
To know which of T2 or T1 should be used, conn->fallback is used. Bluez only attempt to reconnect twice. In the future, we will need more than 2 fallback possibilities. Thus, test conn->fallback as an alternative measure. We want to fallback only if conn->fallback is positive. Calling hci_setup_sync with conn->fallback == 0 is an initial connection attempt.

---

include/net/bluetooth/hci_core.h |    1 +
net/bluetooth/hci_conn.c         |   20 +++++++++++++++++---
net/bluetooth/hci_event.c        |    3 ++-
3 files changed, 20 insertions(+), 4 deletions(-)
include/net/bluetooth/hci_core.h +1 -0
index b5996b3..c8b91d2 100644
@@ -321,6 +321,7 @@ struct hci_conn {
 	__u8		passkey_entered;
 	__u16		disc_timeout;
 	__u16		setting;
+	__u8		fallback;
 	unsigned long	flags;
 
 	__u8		remote_cap;
net/bluetooth/hci_conn.c +1 -0
index cd7452e..8d71d73 100644
@@ -31,6 +31,17 @@
 #include <net/bluetooth/a2mp.h>
 #include <net/bluetooth/smp.h>
 
+struct sco_param {

net/bluetooth/hci_conn.c line 44

Quoted 10 lines
net/bluetooth/hci_conn.c +10 -0
+	u16 pkt_type;
+	u16 max_latency;
+	u8 next;
+};
+
+static const struct sco_param sco_param_wideband[] = {
+	{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x000d, 1 },
+	{ EDR_ESCO_MASK | ESCO_EV3,   0x0008, 0 },
+};
+

so I would combine patch 7 and 8 into one and have a detailed commit explaining both section.

Also comment the entries in the table properly to list which is which setting. So others have an idea on what is going on.

Regards

Marcel

#15 lore raw

PATCH v7 8/8 Bluetooth: Add support for fallback between CVSD recommended settings

In reply to Frédéric Dalleau

When CVSD is requested and eSCO is supported, try to establish eSCO connection using S3 settings. If it fails, fallback in sequence to S2, S1, D1, D0 settings. These settings are described in Bluetooth HFP 1.6 specification p. 101.

---
 net/bluetooth/hci_conn.c |   19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)
net/bluetooth/hci_conn.c +15 -4
index 8d71d73..f4aa8b2 100644
@@ -42,6 +42,14 @@ static const struct sco_param sco_param_wideband[] = {
 	{ EDR_ESCO_MASK | ESCO_EV3,   0x0008, 0 },
 };
 
+static const struct sco_param sco_param_cvsd[] = {
+	{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x000a, 1 },
+	{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x0007, 2 },
+	{ EDR_ESCO_MASK | ESCO_EV3,   0x0007, 3 },
+	{ EDR_ESCO_MASK | ESCO_HV3,   0xffff, 4 },
+	{ EDR_ESCO_MASK | ESCO_HV1,   0xffff, 0 },
+};
+
 static void hci_le_create_connection(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -212,10 +220,13 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 		conn->fallback    = param->next;
 		break;
 	case SCO_AIRMODE_CVSD:
-		cp.pkt_type       = cpu_to_le16(conn->pkt_type);
-		cp.max_latency    = __constant_cpu_to_le16(0xffff);
-		cp.voice_setting  = cpu_to_le16(hdev->voice_setting);
-		cp.retrans_effort = 0xff;
+		param = &sco_param_cvsd[conn->fallback];
+		cp.pkt_type       = __constant_cpu_to_le16(param->pkt_type);
+		cp.max_latency    = __constant_cpu_to_le16(param->max_latency);
+		cp.voice_setting  = __constant_cpu_to_le16(conn->setting);
+		cp.retrans_effort = 0x01;
+
+		conn->fallback    = param->next;
 		break;
 	}
 
Signature · 1 line
1.7.9.5