[PATCH 0/4] Add support for custom (e)SCO parameters

[PATCH 0/4] Add support for custom (e)SCO parameters

[Andrzej Kaczmarek]

Hi,

Following patches add possibility to specify custom parameters for
outgoing (e)SCO link. This is implemented as SCO socket options.

This change allows to use reference parameters as defined in HFP 1.5
specification as well as support for future enchancements as wideband
speech as defined in HFP 1.6 draft (uses different air coding format).

Additional socket option (patch 3) allows to disable automatic retry
in case of failure which fallbacks from eSCO to SCO connection in
current implementation. Such fallback should not happen when using
wideband speech which mandates using of eSCO transport.

Patches are on behalf of ST-Ericsson SA.


Andrzej Kaczmarek (4):
  Bluetooth: Change bit logic in pkt_type for SCO links
  Bluetooth: Add BT_SCO_PARAMETERS SCO socket option.
  Bluetooth: Add BT_NO_AUTORETRY SCO socket option.
  Bluetooth: Update mapping for eSCO related error codes

 include/net/bluetooth/bluetooth.h |   12 ++++++
 include/net/bluetooth/hci.h       |    2 +
 include/net/bluetooth/hci_core.h  |    6 ++-
 include/net/bluetooth/sco.h       |    2 +
 net/bluetooth/hci_conn.c          |   43 ++++++++++++++++-----
 net/bluetooth/hci_event.c         |    5 +-
 net/bluetooth/l2cap_core.c        |    4 +-
 net/bluetooth/lib.c               |    2 +
 net/bluetooth/mgmt.c              |    3 +-
 net/bluetooth/sco.c               |   75 ++++++++++++++++++++++++++++++++++++-
 10 files changed, 135 insertions(+), 19 deletions(-)

-- 
1.7.5.4

[PATCH 1/4] Bluetooth: Change bit logic in pkt_type for SCO links

[Andrzej Kaczmarek]

Change pkt_type to store packet type bitmask in standard on/off logic rather
than as specified in BT specification. This will make handling of pkt_type
easier. Conversion to BT logic is now done just prior to packing into HCI
command.

Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
---
 net/bluetooth/hci_conn.c  |    8 ++++----
 net/bluetooth/hci_event.c |    3 +--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2f5ae53..e203622 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -174,7 +174,8 @@ 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);
+	/* Bits for EDR packets have inverted logic in BT spec. */
+	cp.pkt_type = cpu_to_le16(conn->pkt_type ^ EDR_ESCO_MASK);
 
 	cp.tx_bandwidth   = cpu_to_le32(0x00001f40);
 	cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
@@ -362,13 +363,12 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
 		break;
 	case SCO_LINK:
 		if (lmp_esco_capable(hdev))
-			conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
-					(hdev->esco_type & EDR_ESCO_MASK);
+			conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK);
 		else
 			conn->pkt_type = hdev->pkt_type & SCO_PTYPE_MASK;
 		break;
 	case ESCO_LINK:
-		conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
+		conn->pkt_type = hdev->esco_type;
 		break;
 	}
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ac2c5e8..2f0d3ab 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2420,8 +2420,7 @@ static inline void hci_sync_conn_complete_evt(struct hci_dev *hdev, struct sk_bu
 	case 0x1a:	/* Unsupported Remote Feature */
 	case 0x1f:	/* Unspecified error */
 		if (conn->out && conn->attempt < 2) {
-			conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
-					(hdev->esco_type & EDR_ESCO_MASK);
+			conn->pkt_type = hdev->esco_type & SCO_ESCO_MASK;
 			hci_setup_sync(conn, conn->link->handle);
 			goto unlock;
 		}
-- 
1.7.5.4

[PATCH 2/4] Bluetooth: Add BT_SCO_PARAMETERS SCO socket option.

[Andrzej Kaczmarek]

Add BT_SCO_PARAMETERS socket option used to specify parameters for (e)SCO link.
This allows user-space to implement custom negotiation of link parameters i.e.
using reference parameters for eSCO from HFP.

For simplicity, packet type does not use logic defined in BT spec rather than
standard on/off logic and voice settings specifies only air coding format.
Conversion to proper values is done internally.

Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
---
 include/net/bluetooth/bluetooth.h |   10 +++++++
 include/net/bluetooth/hci.h       |    2 +
 include/net/bluetooth/hci_core.h  |    5 +++-
 include/net/bluetooth/sco.h       |    1 +
 net/bluetooth/hci_conn.c          |   39 ++++++++++++++++++++------
 net/bluetooth/l2cap_core.c        |    4 +-
 net/bluetooth/mgmt.c              |    3 +-
 net/bluetooth/sco.c               |   55 ++++++++++++++++++++++++++++++++++++-
 8 files changed, 105 insertions(+), 14 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 7bccaf9..795174b 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -76,6 +76,16 @@ struct bt_power {
 #define BT_POWER_FORCE_ACTIVE_OFF 0
 #define BT_POWER_FORCE_ACTIVE_ON  1
 
+#define BT_SCO_PARAMETERS	10
+struct bt_sco_parameters {
+	__u32 tx_bandwidth;
+	__u32 rx_bandwidth;
+	__u16 max_latency;
+	__u16 voice_setting;
+	__u8  retrans_effort;
+	__u16 pkt_type;
+} __packed;
+
 #define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
 #define BT_ERR(fmt, arg...)  printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
 #define BT_DBG(fmt, arg...)  pr_debug("%s: " fmt "\n" , __func__ , ## arg)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 65345cd..07c3076 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -159,6 +159,8 @@ enum {
 
 #define SCO_ESCO_MASK  (ESCO_HV1 | ESCO_HV2 | ESCO_HV3)
 #define EDR_ESCO_MASK  (ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5)
+#define ALL_ESCO_MASK  (SCO_ESCO_MASK | ESCO_EV3 | ESCO_EV4 | ESCO_EV5 | \
+			EDR_ESCO_MASK)
 
 /* ACL flags */
 #define ACL_START_NO_FLUSH	0x00
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fe05946..5048472 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -274,6 +274,8 @@ struct hci_conn {
 	void		*sco_data;
 	void		*priv;
 
+	struct bt_sco_parameters	*sco_parameters;
+
 	struct hci_conn	*link;
 
 	void (*connect_cfm_cb)	(struct hci_conn *conn, u8 status);
@@ -436,7 +438,8 @@ void hci_conn_hash_flush(struct hci_dev *hdev);
 void hci_conn_check_pending(struct hci_dev *hdev);
 
 struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
-						__u8 sec_level, __u8 auth_type);
+				__u8 sec_level, __u8 auth_type,
+				struct bt_sco_parameters *sco_parameters);
 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);
diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 1e35c43..4fa57bf 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -73,6 +73,7 @@ struct sco_conn {
 struct sco_pinfo {
 	struct bt_sock	bt;
 	__u32		flags;
+	struct bt_sco_parameters	param;
 	struct sco_conn	*conn;
 };
 
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e203622..b02e043 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -147,16 +147,21 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_add_sco cp;
+	struct bt_sco_parameters *p = conn->sco_parameters;
+	__u16 pkt_type;
 
 	BT_DBG("%p", conn);
 
+	/* HCI_Add_SCO_Connection uses shifted bitmask for packet type */
+	pkt_type = (p->pkt_type << 5) & conn->pkt_type;
+
 	conn->state = BT_CONNECT;
 	conn->out = 1;
 
 	conn->attempt++;
 
 	cp.handle   = cpu_to_le16(handle);
-	cp.pkt_type = cpu_to_le16(conn->pkt_type);
+	cp.pkt_type = cpu_to_le16(pkt_type);
 
 	hci_send_cmd(hdev, HCI_OP_ADD_SCO, sizeof(cp), &cp);
 }
@@ -165,23 +170,35 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_setup_sync_conn cp;
+	struct bt_sco_parameters *p = conn->sco_parameters;
+	__u16 voice_setting;
+	__u16 pkt_type;
 
 	BT_DBG("%p", conn);
 
+	/*
+	 * Combine voice setting using device parameters and air coding
+	 * format set by user.
+	 */
+	voice_setting = (hdev->voice_setting & 0xfffc) |
+					(p->voice_setting & 0x0003);
+
+	/* Bits for EDR packets have inverted logic in BT spec. */
+	pkt_type = (p->pkt_type & conn->pkt_type) ^ EDR_ESCO_MASK;
+
 	conn->state = BT_CONNECT;
 	conn->out = 1;
 
 	conn->attempt++;
 
 	cp.handle   = cpu_to_le16(handle);
-	/* Bits for EDR packets have inverted logic in BT spec. */
-	cp.pkt_type = cpu_to_le16(conn->pkt_type ^ EDR_ESCO_MASK);
 
-	cp.tx_bandwidth   = cpu_to_le32(0x00001f40);
-	cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
-	cp.max_latency    = cpu_to_le16(0xffff);
-	cp.voice_setting  = cpu_to_le16(hdev->voice_setting);
-	cp.retrans_effort = 0xff;
+	cp.tx_bandwidth   = cpu_to_le32(p->tx_bandwidth);
+	cp.rx_bandwidth   = cpu_to_le32(p->rx_bandwidth);
+	cp.max_latency    = cpu_to_le16(p->max_latency);
+	cp.voice_setting  = cpu_to_le16(voice_setting);
+	cp.retrans_effort = p->retrans_effort;
+	cp.pkt_type       = cpu_to_le16(pkt_type);
 
 	hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
 }
@@ -489,7 +506,9 @@ EXPORT_SYMBOL(hci_get_route);
 
 /* Create SCO, ACL or LE connection.
  * Device _must_ be locked */
-struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
+struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
+				__u8 sec_level, __u8 auth_type,
+				struct bt_sco_parameters *sco_parameters)
 {
 	struct hci_conn *acl;
 	struct hci_conn *sco;
@@ -554,6 +573,8 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 
 	hci_conn_hold(sco);
 
+	sco->sco_parameters = sco_parameters;
+
 	if (acl->state == BT_CONNECTED &&
 			(sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
 		acl->power_save = 1;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 584a423..21380f8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1105,10 +1105,10 @@ int l2cap_chan_connect(struct l2cap_chan *chan)
 
 	if (chan->dcid == L2CAP_CID_LE_DATA)
 		hcon = hci_connect(hdev, LE_LINK, dst,
-					chan->sec_level, auth_type);
+					chan->sec_level, auth_type, NULL);
 	else
 		hcon = hci_connect(hdev, ACL_LINK, dst,
-					chan->sec_level, auth_type);
+					chan->sec_level, auth_type, NULL);
 
 	if (IS_ERR(hcon)) {
 		err = PTR_ERR(hcon);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d192089..b0ffb8c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1353,7 +1353,8 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len)
 		auth_type = HCI_AT_DEDICATED_BONDING_MITM;
 	}
 
-	conn = hci_connect(hdev, ACL_LINK, &cp->bdaddr, sec_level, auth_type);
+	conn = hci_connect(hdev, ACL_LINK, &cp->bdaddr, sec_level,
+							auth_type, NULL);
 	if (IS_ERR(conn)) {
 		err = PTR_ERR(conn);
 		goto unlock;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index cb4fb78..c432428 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -177,6 +177,7 @@ static int sco_connect(struct sock *sk)
 {
 	bdaddr_t *src = &bt_sk(sk)->src;
 	bdaddr_t *dst = &bt_sk(sk)->dst;
+	struct bt_sco_parameters *param = &sco_pi(sk)->param;
 	struct sco_conn *conn;
 	struct hci_conn *hcon;
 	struct hci_dev  *hdev;
@@ -195,7 +196,8 @@ static int sco_connect(struct sock *sk)
 	else
 		type = SCO_LINK;
 
-	hcon = hci_connect(hdev, type, dst, BT_SECURITY_LOW, HCI_AT_NO_BONDING);
+	hcon = hci_connect(hdev, type, dst, BT_SECURITY_LOW, HCI_AT_NO_BONDING,
+									param);
 	if (IS_ERR(hcon)) {
 		err = PTR_ERR(hcon);
 		goto done;
@@ -401,10 +403,24 @@ static void sco_sock_close(struct sock *sk)
 
 static void sco_sock_init(struct sock *sk, struct sock *parent)
 {
+	struct sco_pinfo *pi = sco_pi(sk);
+
 	BT_DBG("sk %p", sk);
 
 	if (parent)
 		sk->sk_type = parent->sk_type;
+
+	pi->param.tx_bandwidth =   0x1f40; /* 8000 */
+	pi->param.rx_bandwidth =   0x1f40;
+	pi->param.max_latency =    0xffff; /* Don't care */
+
+	/* Only Air Coding Format matters here, other data will be
+	 * overriden by device settings during connection setup.
+	 */
+	pi->param.voice_setting  = 0x0000; /* CVSD */
+
+	pi->param.retrans_effort = 0xff;   /* Don't care */
+	pi->param.pkt_type       = ALL_ESCO_MASK;
 }
 
 static struct proto sco_proto = {
@@ -658,13 +674,37 @@ static int sco_sock_sendmsg(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 len;
 	int err = 0;
+	struct bt_sco_parameters *param;
 
 	BT_DBG("sk %p", sk);
 
+	if (level != SOL_BLUETOOTH)
+		return -ENOPROTOOPT;
+
 	lock_sock(sk);
 
 	switch (optname) {
+	case BT_SCO_PARAMETERS:
+		/* We do not support changing SCO parameters during
+		 * connection.
+		 */
+		if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) {
+			err = -EBUSY;
+			break;
+		}
+
+		param = &sco_pi(sk)->param;
+
+		len = min_t(unsigned int, sizeof(*param), optlen);
+		if (copy_from_user((char *) param, optval, len)) {
+			err = -EFAULT;
+			break;
+		}
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -734,18 +774,31 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 {
 	struct sock *sk = sock->sk;
 	int len, err = 0;
+	struct bt_sco_parameters *params;
 
 	BT_DBG("sk %p", sk);
 
 	if (level == SOL_SCO)
 		return sco_sock_getsockopt_old(sock, optname, optval, optlen);
 
+	if (level != SOL_BLUETOOTH)
+		return -ENOPROTOOPT;
+
 	if (get_user(len, optlen))
 		return -EFAULT;
 
 	lock_sock(sk);
 
 	switch (optname) {
+	case BT_SCO_PARAMETERS:
+		params = &sco_pi(sk)->param;
+
+		len = min_t(unsigned int, len, sizeof(*params));
+		if (copy_to_user(optval, (char *) params, len))
+			err = -EFAULT;
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
-- 
1.7.5.4

[PATCH 3/4] Bluetooth: Add BT_NO_AUTORETRY SCO socket option.

[Andrzej Kaczmarek]

Add BT_NO_AUTORETRY socket option used to disable automatic connection retry
in case of failure.

This is now implemented for SCO sockets to disable retrying connection using
SCO packets only when it failed using eSCO packets. Such fallback is unwanted
i.e. for HFP wideband speech it is mandatory to use eSCO transport.

Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
---
 include/net/bluetooth/bluetooth.h |    2 ++
 include/net/bluetooth/hci_core.h  |    1 +
 include/net/bluetooth/sco.h       |    1 +
 net/bluetooth/hci_event.c         |    2 +-
 net/bluetooth/sco.c               |   20 ++++++++++++++++++++
 5 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 795174b..bf7f1a5 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -86,6 +86,8 @@ struct bt_sco_parameters {
 	__u16 pkt_type;
 } __packed;
 
+#define BT_NO_AUTORETRY	11
+
 #define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
 #define BT_ERR(fmt, arg...)  printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
 #define BT_DBG(fmt, arg...)  pr_debug("%s: " fmt "\n" , __func__ , ## arg)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5048472..c1ba98f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -233,6 +233,7 @@ struct hci_conn {
 	__u8		type;
 	__u8		out;
 	__u8		attempt;
+	__u8		no_autoretry;
 	__u8		dev_class[3];
 	__u8		features[8];
 	__u8		ssp_mode;
diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 4fa57bf..c92a8b5 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -74,6 +74,7 @@ struct sco_pinfo {
 	struct bt_sock	bt;
 	__u32		flags;
 	struct bt_sco_parameters	param;
+	__u8	no_autoretry;
 	struct sco_conn	*conn;
 };
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 2f0d3ab..d30719e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2419,7 +2419,7 @@ static inline void hci_sync_conn_complete_evt(struct hci_dev *hdev, struct sk_bu
 	case 0x1c:	/* SCO interval rejected */
 	case 0x1a:	/* Unsupported Remote Feature */
 	case 0x1f:	/* Unspecified error */
-		if (conn->out && conn->attempt < 2) {
+		if (conn->out && !conn->no_autoretry && conn->attempt < 2) {
 			conn->pkt_type = hdev->esco_type & SCO_ESCO_MASK;
 			hci_setup_sync(conn, conn->link->handle);
 			goto unlock;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index c432428..d34228d 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -203,6 +203,11 @@ static int sco_connect(struct sock *sk)
 		goto done;
 	}
 
+	/* Make sure we won't retry connection automatically if user does
+	 * not want this. */
+	if (sco_pi(sk)->no_autoretry)
+		hcon->no_autoretry = 1;
+
 	conn = sco_conn_add(hcon, 0);
 	if (!conn) {
 		hci_conn_put(hcon);
@@ -677,6 +682,7 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char
 	int len;
 	int err = 0;
 	struct bt_sco_parameters *param;
+	u32 opt;
 
 	BT_DBG("sk %p", sk);
 
@@ -705,6 +711,15 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char
 
 		break;
 
+	case BT_NO_AUTORETRY:
+		if (get_user(opt, (u32 __user *) optval)) {
+			err = -EFAULT;
+			break;
+		}
+
+		sco_pi(sk)->no_autoretry = opt;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -799,6 +814,11 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 
 		break;
 
+	case BT_NO_AUTORETRY:
+		if (put_user(sco_pi(sk)->no_autoretry, (u32 __user *) optval))
+			err = -EFAULT;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
-- 
1.7.5.4

[PATCH 4/4] Bluetooth: Update mapping for eSCO related error codes

[Andrzej Kaczmarek]

Add mapping for SCO INTERVAL REJECTED (0X1C) and SCO AIR MODE REJECTED (0X1D)
error codes. It should map to ECONNREFUSED instead of ENOSYS.

Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
---
 net/bluetooth/lib.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/lib.c b/net/bluetooth/lib.c
index b826d1b..af615c9 100644
--- a/net/bluetooth/lib.c
+++ b/net/bluetooth/lib.c
@@ -136,6 +136,8 @@ int bt_err(__u16 code)
 		return EPROTONOSUPPORT;
 
 	case 0x1b:
+	case 0x1c:
+	case 0x1d:
 		return ECONNREFUSED;
 
 	case 0x19:
-- 
1.7.5.4

Re: [PATCH 0/4] Add support for custom (e)SCO parameters

[Marcel Holtmann]

Hi Andrzej,

> Following patches add possibility to specify custom parameters for
> outgoing (e)SCO link. This is implemented as SCO socket options.
> 
> This change allows to use reference parameters as defined in HFP 1.5
> specification as well as support for future enchancements as wideband
> speech as defined in HFP 1.6 draft (uses different air coding format).
> 
> Additional socket option (patch 3) allows to disable automatic retry
> in case of failure which fallbacks from eSCO to SCO connection in
> current implementation. Such fallback should not happen when using
> wideband speech which mandates using of eSCO transport.

I am not a big fan of just stupidly exposing all possible HCI value to
userspace and let userspace set them. Especially since in the end there
are no real variation here anyway. There are just a 2-3 use cases for
your SCO socket.

I would rather have simple mode to hand to the kernel and let it do the
right thing (including negotiation and downgrading to SCO) than having
userspace opening sockets over sockets again.

So what are the actual modes for HFP that need to be supported and what
is the range of possible SCO and eSCO options?

Regards

Marcel

Re: [PATCH 0/4] Add support for custom (e)SCO parameters

[Andrzej Kaczmarek]

Hi Marcel,

On 15.06.2011 13:08, Marcel Holtmann wrote:
> Hi Andrzej,
>
>> Following patches add possibility to specify custom parameters for
>> outgoing (e)SCO link. This is implemented as SCO socket options.
>>
>> This change allows to use reference parameters as defined in HFP 1.5
>> specification as well as support for future enchancements as wideband
>> speech as defined in HFP 1.6 draft (uses different air coding format).
>>
>> Additional socket option (patch 3) allows to disable automatic retry
>> in case of failure which fallbacks from eSCO to SCO connection in
>> current implementation. Such fallback should not happen when using
>> wideband speech which mandates using of eSCO transport.
>
> I am not a big fan of just stupidly exposing all possible HCI value to
> userspace and let userspace set them. Especially since in the end there
> are no real variation here anyway. There are just a 2-3 use cases for
> your SCO socket.

There's L2CAP_OPTIONS sockopt which allows to do pretty much the same 
for L2CAP sockets so I just followed it.

What do you mean by just 2-3 use cases for SCO socket? You consider only 
HFP as use case for SCO socket? I saw devices which use eSCO to stream 
music to mono headsets which do not support A2DP, this may require 
custom requirements which we cannot predict. This is only one example, 
but user can "invent" something else.

> I would rather have simple mode to hand to the kernel and let it do the
> right thing (including negotiation and downgrading to SCO) than having
> userspace opening sockets over sockets again.

By making such change you actually incorporate part of profiles into 
kernel. I don't think this is good idea. My understanding is that 
profiles should be in user-space.

And I don't recall any other profile than HFP that uses eSCO, so if 
we're going to implement "modes" using this spec, user won't have many 
options to choose from. See example above (music over eSCO).

> So what are the actual modes for HFP that need to be supported and what
> is the range of possible SCO and eSCO options?

At the moment only HFP 1.5 defines reference parameters for eSCO link.
In HFP 1.6 draft there are additional reference parameters for CVSD over 
SCO and mSBC over eSCO. And in future profiles you never know what BT 
SIG will invent :)

BR,
Andrzej

Re: [PATCH 0/4] Add support for custom (e)SCO parameters

[Marcel Holtmann]

Hi Andrzej,

> >> Following patches add possibility to specify custom parameters for
> >> outgoing (e)SCO link. This is implemented as SCO socket options.
> >>
> >> This change allows to use reference parameters as defined in HFP 1.5
> >> specification as well as support for future enchancements as wideband
> >> speech as defined in HFP 1.6 draft (uses different air coding format).
> >>
> >> Additional socket option (patch 3) allows to disable automatic retry
> >> in case of failure which fallbacks from eSCO to SCO connection in
> >> current implementation. Such fallback should not happen when using
> >> wideband speech which mandates using of eSCO transport.
> >
> > I am not a big fan of just stupidly exposing all possible HCI value to
> > userspace and let userspace set them. Especially since in the end there
> > are no real variation here anyway. There are just a 2-3 use cases for
> > your SCO socket.
> 
> There's L2CAP_OPTIONS sockopt which allows to do pretty much the same 
> for L2CAP sockets so I just followed it.
> 
> What do you mean by just 2-3 use cases for SCO socket? You consider only 
> HFP as use case for SCO socket? I saw devices which use eSCO to stream 
> music to mono headsets which do not support A2DP, this may require 
> custom requirements which we cannot predict. This is only one example, 
> but user can "invent" something else.
> 
> > I would rather have simple mode to hand to the kernel and let it do the
> > right thing (including negotiation and downgrading to SCO) than having
> > userspace opening sockets over sockets again.
> 
> By making such change you actually incorporate part of profiles into 
> kernel. I don't think this is good idea. My understanding is that 
> profiles should be in user-space.
> 
> And I don't recall any other profile than HFP that uses eSCO, so if 
> we're going to implement "modes" using this spec, user won't have many 
> options to choose from. See example above (music over eSCO).
> 
> > So what are the actual modes for HFP that need to be supported and what
> > is the range of possible SCO and eSCO options?
> 
> At the moment only HFP 1.5 defines reference parameters for eSCO link.
> In HFP 1.6 draft there are additional reference parameters for CVSD over 
> SCO and mSBC over eSCO. And in future profiles you never know what BT 
> SIG will invent :)

personally I would just go for PCM, CVSD and mSBC modes here actually.

And you can not compare L2CAP with SCO since they are different. And
also L2CAP does not expose everything that the spec. would offer.

My real question is how you expose negotiation capabilities. Since
re-creating the socket and its connection attempt is not going to work
out. That concept will fail and will also break qualification testing.

Regards

Marcel

Re: [PATCH 0/4] Add support for custom (e)SCO parameters

[Andrzej Kaczmarek]

Hi Marcel,

On 15.06.2011 16:25, Marcel Holtmann wrote:
> Hi Andrzej,
>
>>>> Following patches add possibility to specify custom parameters for
>>>> outgoing (e)SCO link. This is implemented as SCO socket options.
>>>>
>>>> This change allows to use reference parameters as defined in HFP 1.5
>>>> specification as well as support for future enchancements as wideband
>>>> speech as defined in HFP 1.6 draft (uses different air coding format).
>>>>
>>>> Additional socket option (patch 3) allows to disable automatic retry
>>>> in case of failure which fallbacks from eSCO to SCO connection in
>>>> current implementation. Such fallback should not happen when using
>>>> wideband speech which mandates using of eSCO transport.
>>>
>>> I am not a big fan of just stupidly exposing all possible HCI value to
>>> userspace and let userspace set them. Especially since in the end there
>>> are no real variation here anyway. There are just a 2-3 use cases for
>>> your SCO socket.
>>
>> There's L2CAP_OPTIONS sockopt which allows to do pretty much the same
>> for L2CAP sockets so I just followed it.
>>
>> What do you mean by just 2-3 use cases for SCO socket? You consider only
>> HFP as use case for SCO socket? I saw devices which use eSCO to stream
>> music to mono headsets which do not support A2DP, this may require
>> custom requirements which we cannot predict. This is only one example,
>> but user can "invent" something else.
>>
>>> I would rather have simple mode to hand to the kernel and let it do the
>>> right thing (including negotiation and downgrading to SCO) than having
>>> userspace opening sockets over sockets again.
>>
>> By making such change you actually incorporate part of profiles into
>> kernel. I don't think this is good idea. My understanding is that
>> profiles should be in user-space.
>>
>> And I don't recall any other profile than HFP that uses eSCO, so if
>> we're going to implement "modes" using this spec, user won't have many
>> options to choose from. See example above (music over eSCO).
>>
>>> So what are the actual modes for HFP that need to be supported and what
>>> is the range of possible SCO and eSCO options?
>>
>> At the moment only HFP 1.5 defines reference parameters for eSCO link.
>> In HFP 1.6 draft there are additional reference parameters for CVSD over
>> SCO and mSBC over eSCO. And in future profiles you never know what BT
>> SIG will invent :)
>
> personally I would just go for PCM, CVSD and mSBC modes here actually.

..and any other codec which can some soon? I'm simply not convinced that 
kernel should expose something defined in profiles which are 
application-level specs.

And what do you mean by PCM? Which spec uses eSCO to transport raw PCM data?

> And you can not compare L2CAP with SCO since they are different. And
> also L2CAP does not expose everything that the spec. would offer.

Sure, they are different. But this does not explain why L2CAP should 
expose low-level interface and SCO should not do the same but provide 
very high-level interface only.

In case of L2CAP was it decision not to expose some parameters for some 
reason other than "nobody uses them anyway"? For eSCO we could skip 
bandwidth parameter as I never seen anything other than 8000 rx/tx, but 
I can still imagine someone can use different value.

Actually, from what I see almost all L2CAP parameters supported by BlueZ 
are exposed.

> My real question is how you expose negotiation capabilities. Since
> re-creating the socket and its connection attempt is not going to work
> out. That concept will fail and will also break qualification testing.

Why do you think re-creating the socket is not going to work? How it is 
different than second attempt to connect SCO when eSCO failed, which is 
now handled inside kernel? The same could be done by user-space but in 
more sophisticated way which suits specific requirements. And this is 
how negotiation is solved.

And what qualification testing will it break?


Could you please elaborate a little bit more on the above questions so I 
can understand your point of view better.

BR,
Andrzej

Re: [PATCH 0/4] Add support for custom (e)SCO parameters

[Luiz Augusto von Dentz]

Hi,

On Thu, Jun 16, 2011 at 2:33 PM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> Why do you think re-creating the socket is not going to work? How it is
> different than second attempt to connect SCO when eSCO failed, which is n=
ow
> handled inside kernel? The same could be done by user-space but in more
> sophisticated way which suits specific requirements. And this is how
> negotiation is solved.

Well the socket API is not meant to do this type of negotiation while
connecting, you basically have connect/accept and in case you fail to
configure you have to give up this socket and create a new one, also
the error reported by the socket may not be enough to you to figure
out what went wrong the first time to retry.

> And what qualification testing will it break?
>
>
> Could you please elaborate a little bit more on the above questions so I =
can
> understand your point of view better.
>
> BR,
> Andrzej
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>



--=20
Luiz Augusto von Dentz

Re: [PATCH 0/4] Add support for custom (e)SCO parameters

[Marcel Holtmann]

Hi Andrzej,

> >>>> Following patches add possibility to specify custom parameters for
> >>>> outgoing (e)SCO link. This is implemented as SCO socket options.
> >>>>
> >>>> This change allows to use reference parameters as defined in HFP 1.5
> >>>> specification as well as support for future enchancements as wideband
> >>>> speech as defined in HFP 1.6 draft (uses different air coding format).
> >>>>
> >>>> Additional socket option (patch 3) allows to disable automatic retry
> >>>> in case of failure which fallbacks from eSCO to SCO connection in
> >>>> current implementation. Such fallback should not happen when using
> >>>> wideband speech which mandates using of eSCO transport.
> >>>
> >>> I am not a big fan of just stupidly exposing all possible HCI value to
> >>> userspace and let userspace set them. Especially since in the end there
> >>> are no real variation here anyway. There are just a 2-3 use cases for
> >>> your SCO socket.
> >>
> >> There's L2CAP_OPTIONS sockopt which allows to do pretty much the same
> >> for L2CAP sockets so I just followed it.
> >>
> >> What do you mean by just 2-3 use cases for SCO socket? You consider only
> >> HFP as use case for SCO socket? I saw devices which use eSCO to stream
> >> music to mono headsets which do not support A2DP, this may require
> >> custom requirements which we cannot predict. This is only one example,
> >> but user can "invent" something else.
> >>
> >>> I would rather have simple mode to hand to the kernel and let it do the
> >>> right thing (including negotiation and downgrading to SCO) than having
> >>> userspace opening sockets over sockets again.
> >>
> >> By making such change you actually incorporate part of profiles into
> >> kernel. I don't think this is good idea. My understanding is that
> >> profiles should be in user-space.
> >>
> >> And I don't recall any other profile than HFP that uses eSCO, so if
> >> we're going to implement "modes" using this spec, user won't have many
> >> options to choose from. See example above (music over eSCO).
> >>
> >>> So what are the actual modes for HFP that need to be supported and what
> >>> is the range of possible SCO and eSCO options?
> >>
> >> At the moment only HFP 1.5 defines reference parameters for eSCO link.
> >> In HFP 1.6 draft there are additional reference parameters for CVSD over
> >> SCO and mSBC over eSCO. And in future profiles you never know what BT
> >> SIG will invent :)
> >
> > personally I would just go for PCM, CVSD and mSBC modes here actually.
> 
> ..and any other codec which can some soon? I'm simply not convinced that 
> kernel should expose something defined in profiles which are 
> application-level specs.
> 
> And what do you mean by PCM? Which spec uses eSCO to transport raw PCM data?

the encoded data you are actually sending over the SCO socket. Or are
you telling me that for mSBC we have PCM input to the chip, and the chip
does mSBC then.

> > And you can not compare L2CAP with SCO since they are different. And
> > also L2CAP does not expose everything that the spec. would offer.
> 
> Sure, they are different. But this does not explain why L2CAP should 
> expose low-level interface and SCO should not do the same but provide 
> very high-level interface only.
> 
> In case of L2CAP was it decision not to expose some parameters for some 
> reason other than "nobody uses them anyway"? For eSCO we could skip 
> bandwidth parameter as I never seen anything other than 8000 rx/tx, but 
> I can still imagine someone can use different value.
> 
> Actually, from what I see almost all L2CAP parameters supported by BlueZ 
> are exposed.

For Basic Mode L2CAP, that is true, but for eL2CAP we clearly not expose
all possible settings. The options clearly grow over time, but that is
mainly because of eL2CAP and not because we wanted to.

However the settings are static here in that sense that we require them
for that specific connection to work. It is designed as minimum
requirement to establish the specific link.

> > My real question is how you expose negotiation capabilities. Since
> > re-creating the socket and its connection attempt is not going to work
> > out. That concept will fail and will also break qualification testing.
> 
> Why do you think re-creating the socket is not going to work? How it is 
> different than second attempt to connect SCO when eSCO failed, which is 
> now handled inside kernel? The same could be done by user-space but in 
> more sophisticated way which suits specific requirements. And this is 
> how negotiation is solved.

Because it actually is a timing issue and once a SCO socket gets closed,
the kernel will trigger various timers. I do not really like such an
approach.

If we wanna expose minimum required settings, then I am actually fine
with an SCO_OPTIONS socket option, but it needs to be better defined in
how that is suppose to work. Since clearly for HFP 1.5, we do not care
if it is a SCO or eSCO link in the end. So the kernel must be able to
negotiate this.

I do not want to implement heavy negotiation logic into every single
piece of code that wants a SCO/eSCO socket. In addition you have to
figure out on how the acceptor side should work. BlueZ can play both
roles of HFP. How do you deal with accepting SCO connections in either
SCO/eSCO or mSBC mode?

Regards

Marcel

Re: [PATCH 0/4] Add support for custom (e)SCO parameters

[Andrzej Kaczmarek]

Hi Marcel,

On 16.06.2011 16:41, Marcel Holtmann wrote:
>>>>> So what are the actual modes for HFP that need to be supported and what
>>>>> is the range of possible SCO and eSCO options?
>>>>
>>>> At the moment only HFP 1.5 defines reference parameters for eSCO link.
>>>> In HFP 1.6 draft there are additional reference parameters for CVSD over
>>>> SCO and mSBC over eSCO. And in future profiles you never know what BT
>>>> SIG will invent :)
>>>
>>> personally I would just go for PCM, CVSD and mSBC modes here actually.
>>
>> ..and any other codec which can some soon? I'm simply not convinced that
>> kernel should expose something defined in profiles which are
>> application-level specs.
>>
>> And what do you mean by PCM? Which spec uses eSCO to transport raw PCM data?
>
> the encoded data you are actually sending over the SCO socket. Or are
> you telling me that for mSBC we have PCM input to the chip, and the chip
> does mSBC then.

Yes, there are chips which have codecs embedded and can fill payload 
using data supplied over e.g. PCM input (and this applies not only to 
mSBC but also to CVSD). But what difference does it make here?

As I understand what you send over SCO socket depends (mostly) on what 
air coding format is used on link. For CVSD you send PCM data as I know 
(but PCM is input here so not sure why it should be included as mode). 
For transparent data it depends on application, in particular for HFP 
1.6 it depends on which codec was negotiated.

Looking through profiles you can actually have CVSD mode (let's say for 
"any" eSCO) and mSBC mode (for mSCB negotiated as per HFP 1.6).
But the question remains: what if application wants to send voice data 
(or just data) outside HFP spec? It's limited only to what BT SIG 
"invented"...

>>> My real question is how you expose negotiation capabilities. Since
>>> re-creating the socket and its connection attempt is not going to work
>>> out. That concept will fail and will also break qualification testing.
>>
>> Why do you think re-creating the socket is not going to work? How it is
>> different than second attempt to connect SCO when eSCO failed, which is
>> now handled inside kernel? The same could be done by user-space but in
>> more sophisticated way which suits specific requirements. And this is
>> how negotiation is solved.
>
> Because it actually is a timing issue and once a SCO socket gets closed,
> the kernel will trigger various timers. I do not really like such an
> approach.

Ok, so does it mean it's incorrect to connect one SCO after closing 
another one because there's issue in kernel? If yes then perhaps it 
should be fixed. If not then I don't understand your argument.

And eSCO negotiation implemented in user-space works just fine for me.

> If we wanna expose minimum required settings, then I am actually fine
> with an SCO_OPTIONS socket option, but it needs to be better defined in
> how that is suppose to work. Since clearly for HFP 1.5, we do not care
> if it is a SCO or eSCO link in the end. So the kernel must be able to
> negotiate this.

That's the problem. BT core does not define how this should work, 
because there's no eSCO negotiation on HCI level. Link with given 
parameters can be either established or not. If not, you can try another 
one. And such rules for negotiation are in best case defined on 
application level in profiles.

So that's why I think it's reasonable that application can define 
parameters it requires. Otherwise you will need to integrate new modes 
into SCO socket in case new profile have new requirements (for me it's 
strange that new profile would require new kernel as long as nothing new 
was introduced in core spec) or in worst case hack kernel so your 
application can use new custom mode.

> I do not want to implement heavy negotiation logic into every single
> piece of code that wants a SCO/eSCO socket. In addition you have to
> figure out on how the acceptor side should work. BlueZ can play both
> roles of HFP. How do you deal with accepting SCO connections in either
> SCO/eSCO or mSBC mode?

You don't need to implement any logic at all, nothing changes if you 
just want to connect any (e)SCO. This is only for those who want to have 
more control of link that is going to be established.

Socket options affect outgoing connections only at the moment, 
connections are accepted using default parameters. Problem with incoming 
connection is that on kernel level you don't know what kind of 
connection remote side requires (connection request event can only tell 
type of link). For HFP 1.6 headset should be ready to accept specified 
connection after codec was selected and this seems to be the only way to 
determine whether we should accept using CVSD or transparent data. I 
think this can be implemented in a way similar to BT_DEFER_SETUP so 
application can accept link, determine remote device address and setup 
accordingly before continuing. But patches I sent do not include this, 
they are only for outgoing connection.

BR,
Andrzej