2013-07-12 16:05:34

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 0/9] sco: SCO socket option for voice_setting

Hi,

v9 is mostly cosmetics. Introduces LMP_TRANSPARENT bit in patch 8/8.


v8 declares BT_VOICE_CVSD_16BIT
Merge T*, S* and D* patches in one
The last patch returns -ECONNABORTED when trying to setup a transparent data
connection if eSCO is not supported.

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 (9):
Bluetooth: Use hci_connect_sco directly
Bluetooth: Remove unused mask parameter in sco_conn_defer_accept
Bluetooth: Add bluetooth socket voice option
Bluetooth: Constants declaration for SCO airmode
Bluetooth: Use voice setting in defered SCO connection request
Bluetooth: Parameters for outgoing SCO connections
Bluetooth: Constants and macro declaration for transparent data
Bluetooth: Prevent transparent SCO on older devices
Bluetooth: SCO connection fallback

include/net/bluetooth/bluetooth.h | 8 ++++
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/hci_core.h | 9 +++++
include/net/bluetooth/sco.h | 1 +
net/bluetooth/hci_conn.c | 55 +++++++++++++++++++++-----
net/bluetooth/hci_event.c | 3 +-
net/bluetooth/sco.c | 77 ++++++++++++++++++++++++++++++-------
7 files changed, 130 insertions(+), 24 deletions(-)

--
1.7.9.5



2013-07-18 01:22:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v9 6/9] Bluetooth: Parameters for outgoing SCO connections

Hi Fred,

> In order to establish a transparent SCO connection, the correct settings must
> be specified in the Setup Synchronous Connection request. For that,
> a setting field is added to ACL connection data to set up the desired
> parameters.
> Remove usage of hdev->voice_setting in CVSD connection.
> Make use of T2 parameters for transparent data.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 21 +++++++++++++++++----
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 69bdb67..61ca2ce 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -320,6 +320,7 @@ struct hci_conn {
> __u32 passkey_notify;
> __u8 passkey_entered;
> __u16 disc_timeout;
> + __u16 setting;
> unsigned long flags;
>
> __u8 remote_cap;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index d1d9919..c4d6163 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -185,13 +185,24 @@ 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;
> + cp.voice_setting = cpu_to_le16(conn->setting);
> +
> + switch (conn->setting & SCO_AIRMODE_MASK) {
> + case SCO_AIRMODE_TRANSP:
> + cp.pkt_type = __constant_cpu_to_le16(EDR_ESCO_MASK &
> + ~ESCO_2EV3);
> + cp.max_latency = __constant_cpu_to_le16(0x000d);

please do not try to align = here.

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

And here as well. No need for that.

> + cp.retrans_effort = 0xff;
> + break;
> + }
>
> hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
> }
> @@ -584,6 +595,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);

Regards

Marcel


2013-07-18 01:20:49

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v9 9/9] Bluetooth: SCO connection fallback

Hi Fred,

> 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.
>
> 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.
>
> 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 setting should be used, conn->fallback is used. Bluez only
> attempt to reconnect twice. Since, more than 2 fallback are required,
> conn->fallback is tested as an alternative measure. We want to fallback only if
> conn->fallback is positive. Calling hci_setup_sync with conn->fallback == 0
> triggers initial connection attempt.
>
> These setting and the fallback order are described in Bluetooth HFP 1.6
> specification p. 101.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 37 +++++++++++++++++++++++++++++++------
> net/bluetooth/hci_event.c | 3 ++-
> 3 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b2bfab8..26560c6 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -321,6 +321,7 @@ struct hci_conn {
> __u8 passkey_entered;
> __u16 disc_timeout;
> __u16 setting;
> + __u8 fallback;
> unsigned long flags;
>
> __u8 remote_cap;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index c4d6163..d6b09fa 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -31,6 +31,25 @@
> #include <net/bluetooth/a2mp.h>
> #include <net/bluetooth/smp.h>
>
> +struct sco_param {
> + u16 pkt_type;
> + u16 max_latency;
> + u8 next;

this next entry seems to be useless since we are trying them in order anyway. So why not use ARRAY_SIZE here and start counting at 0.

> +};
> +
> +static const struct sco_param sco_param_cvsd[] = {
> + { EDR_ESCO_MASK & ~ESCO_2EV3, 0x000a, 1 }, /* S3 */
> + { EDR_ESCO_MASK & ~ESCO_2EV3, 0x0007, 2 }, /* S2 */
> + { EDR_ESCO_MASK | ESCO_EV3, 0x0007, 3 }, /* S1 */
> + { EDR_ESCO_MASK | ESCO_HV3, 0xffff, 4 }, /* D1 */
> + { EDR_ESCO_MASK | ESCO_HV1, 0xffff, 0 }, /* D0 */
> +};
> +
> +static const struct sco_param sco_param_wideband[] = {
> + { EDR_ESCO_MASK & ~ESCO_2EV3, 0x000d, 1 }, /* T2 */
> + { EDR_ESCO_MASK | ESCO_EV3, 0x0008, 0 }, /* T1 */
> +};
> +
> static void hci_le_create_connection(struct hci_conn *conn)
> {
> struct hci_dev *hdev = conn->hdev;
> @@ -176,6 +195,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);
>
> @@ -192,15 +212,20 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
>
> switch (conn->setting & SCO_AIRMODE_MASK) {
> 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.retrans_effort = 0x02;
> +
> + 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.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);

Now this is duplicated code here. Please select the array and then apply it in a generic section.

In addition you can not use __constant_cpu_to_le16 anymore since these values are now coming from a struct and not from a define.

> + cp.retrans_effort = 0x01;
> +
> + conn->fallback = param->next;
> break;
> }
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 0437200..6659af8 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -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 */

This should be a separate patch with its own commit message explaining why we handle this error.

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

I wonder if you can not just use conn->attempt and how it counts up here. It should give you the correct position in the parameter array. And conn->fallback becomes obsolete.

> conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
> (hdev->esco_type & EDR_ESCO_MASK);
> hci_setup_sync(conn, conn->link->handle);

Regards

Marcel


2013-07-18 01:15:09

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v9 8/9] Bluetooth: Prevent transparent SCO on older devices

Hi Fred,

> Older Bluetooth devices may not support Setup Synchronous Connection or SCO
> transparent data. This is indicated by the corresponding LMP feature bits.
> It is not possible to know if the adapter support these features before setting
> BT_VOICE option since the socket is not bound to an adapter. An adapter can
> also be added after the socket is created. The socket can be bound to an
> address before adapter is plugged in.
>
> Thus, on a such adapters, if user request BT_VOICE_TRANSPARENT, outgoing
> connections fail on connect() and returns -EOPNOTSUPP. Incoming connections
> do not fail. However, they should only be allowed depending on what was
> specified in Write_Voice_Settings command.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> net/bluetooth/sco.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 934002c..190765c 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -176,6 +176,12 @@ static int sco_connect(struct sock *sk)
> else
> type = SCO_LINK;
>
> + if (sco_pi(sk)->setting == BT_VOICE_TRANSPARENT &&
> + (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) {
> + err = -ECONNABORTED;

the error message does not match the commit description. It seems you want to return EOPNOTSUPP here.

> + goto done;
> + }
> +
> hcon = hci_connect_sco(hdev, type, dst, sco_pi(sk)->setting);
> if (IS_ERR(hcon)) {
> err = PTR_ERR(hcon);

Regards

Marcel


2013-07-18 01:13:25

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v9 7/9] Bluetooth: Constants and macro declaration for transparent data

Hi Fred,

> This patchs define constants and macro for transparent data LMP features. It
> refers to Bluetooth Core V4.0 specification, Part C, Chap 3.3 which defines
> LMP feature mask.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/hci_core.h | 1 +
> 2 files changed, 2 insertions(+)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel


2013-07-18 01:11:33

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v9 6/9] Bluetooth: Parameters for outgoing SCO connections

Hi Fred,

> In order to establish a transparent SCO connection, the correct settings must
> be specified in the Setup Synchronous Connection request. For that,
> a setting field is added to ACL connection data to set up the desired
> parameters.
> Remove usage of hdev->voice_setting in CVSD connection.
> Make use of T2 parameters for transparent data.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 21 +++++++++++++++++----
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 69bdb67..61ca2ce 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -320,6 +320,7 @@ struct hci_conn {
> __u32 passkey_notify;
> __u8 passkey_entered;
> __u16 disc_timeout;
> + __u16 setting;
> unsigned long flags;
>
> __u8 remote_cap;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index d1d9919..c4d6163 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -185,13 +185,24 @@ 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;
> + cp.voice_setting = cpu_to_le16(conn->setting);
> +
> + switch (conn->setting & SCO_AIRMODE_MASK) {
> + case SCO_AIRMODE_TRANSP:
> + cp.pkt_type = __constant_cpu_to_le16(EDR_ESCO_MASK &
> + ~ESCO_2EV3);
> + cp.max_latency = __constant_cpu_to_le16(0x000d);

please do not try to align = here.

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

And here as well. No need for that.

> + cp.retrans_effort = 0xff;
> + break;
> + }
>
> hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
> }
> @@ -584,6 +595,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);

Regards

Marcel


2013-07-18 01:08:49

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v9 5/9] Bluetooth: Use voice setting in defered SCO connection request

Hi Fred,

> When an incoming eSCO 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.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> net/bluetooth/sco.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index ce8690f..934002c 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -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)

since we are using the setting as input to cpu_to_le16, please make this __u16.

> {
> struct hci_dev *hdev = conn->hdev;
>
> @@ -676,9 +676,21 @@ 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;
> + cp.content_format = cpu_to_le16(setting);
> +
> + switch (setting & SCO_AIRMODE_MASK) {
> + 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.retrans_effort = 0x02;
> + break;
> + case SCO_AIRMODE_CVSD:
> + cp.max_latency = __constant_cpu_to_le16(0xffff);

No need to align = here.

> + cp.retrans_effort = 0xff;
> + break;
> + }
>
> hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
> sizeof(cp), &cp);
> @@ -695,7 +707,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);

Please check that pi->setting is not int type. It needs to be __u16.

> sk->sk_state = BT_CONFIG;
> msg->msg_namelen = 0;

Regards

Marcel


2013-07-12 16:05:37

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 3/9] Bluetooth: Add bluetooth socket voice option

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

Signed-off-by: Frédéric Dalleau <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/bluetooth.h | 8 ++++++++
include/net/bluetooth/sco.h | 1 +
net/bluetooth/sco.c | 40 ++++++++++++++++++++++++++++++++++++-
3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 10eb9b3..10d43d8 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -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_16BIT 0x0060
+
__printf(1, 2)
int bt_info(const char *fmt, ...);
__printf(1, 2)
diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 1e35c43..e252a31 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;
+ __u16 setting;
struct sco_conn *conn;
};

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 0c15a0c..ce8690f 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -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_16BIT;
+
setup_timer(&sk->sk_timer, sco_sock_timeout, (unsigned long)sk);

bt_sock_link(&sco_sk_list, sk);
@@ -709,7 +711,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);
@@ -735,6 +738,31 @@ 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;
+ }
+
+ voice.setting = sco_pi(sk)->setting;
+
+ len = min_t(unsigned int, sizeof(voice), optlen);
+ if (copy_from_user((char *) &voice, optval, len)) {
+ err = -EFAULT;
+ break;
+ }
+
+ /* Explicitly check for these values */
+ if (voice.setting != BT_VOICE_TRANSPARENT &&
+ voice.setting != BT_VOICE_CVSD_16BIT) {
+ err = -EINVAL;
+ break;
+ }
+
+ sco_pi(sk)->setting = voice.setting;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -804,6 +832,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);

@@ -829,6 +858,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;
--
1.7.9.5


2013-07-12 16:05:43

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 9/9] Bluetooth: SCO connection fallback

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.

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.

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 setting should be used, conn->fallback is used. Bluez only
attempt to reconnect twice. Since, more than 2 fallback are required,
conn->fallback is tested as an alternative measure. We want to fallback only if
conn->fallback is positive. Calling hci_setup_sync with conn->fallback == 0
triggers initial connection attempt.

These setting and the fallback order are described in Bluetooth HFP 1.6
specification p. 101.

Signed-off-by: Frédéric Dalleau <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 37 +++++++++++++++++++++++++++++++------
net/bluetooth/hci_event.c | 3 ++-
3 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b2bfab8..26560c6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -321,6 +321,7 @@ struct hci_conn {
__u8 passkey_entered;
__u16 disc_timeout;
__u16 setting;
+ __u8 fallback;
unsigned long flags;

__u8 remote_cap;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index c4d6163..d6b09fa 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -31,6 +31,25 @@
#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_cvsd[] = {
+ { EDR_ESCO_MASK & ~ESCO_2EV3, 0x000a, 1 }, /* S3 */
+ { EDR_ESCO_MASK & ~ESCO_2EV3, 0x0007, 2 }, /* S2 */
+ { EDR_ESCO_MASK | ESCO_EV3, 0x0007, 3 }, /* S1 */
+ { EDR_ESCO_MASK | ESCO_HV3, 0xffff, 4 }, /* D1 */
+ { EDR_ESCO_MASK | ESCO_HV1, 0xffff, 0 }, /* D0 */
+};
+
+static const struct sco_param sco_param_wideband[] = {
+ { EDR_ESCO_MASK & ~ESCO_2EV3, 0x000d, 1 }, /* T2 */
+ { EDR_ESCO_MASK | ESCO_EV3, 0x0008, 0 }, /* T1 */
+};
+
static void hci_le_create_connection(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
@@ -176,6 +195,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);

@@ -192,15 +212,20 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)

switch (conn->setting & SCO_AIRMODE_MASK) {
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.retrans_effort = 0x02;
+
+ 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.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.retrans_effort = 0x01;
+
+ conn->fallback = param->next;
break;
}

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0437200..6659af8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -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);
--
1.7.9.5


2013-07-12 16:05:41

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 7/9] Bluetooth: Constants and macro declaration for transparent data

This patchs define constants and macro for transparent data LMP features. It
refers to Bluetooth Core V4.0 specification, Part C, Chap 3.3 which defines
LMP feature mask.

Signed-off-by: Frédéric Dalleau <[email protected]>
---
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/hci_core.h | 1 +
2 files changed, 2 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 3c592cf..90d3b22 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -238,6 +238,7 @@ enum {
#define LMP_CVSD 0x01
#define LMP_PSCHEME 0x02
#define LMP_PCONTROL 0x04
+#define LMP_TRANSPARENT 0x08

#define LMP_RSSI_INQ 0x40
#define LMP_ESCO 0x80
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 61ca2ce..b2bfab8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -800,6 +800,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define lmp_lsto_capable(dev) ((dev)->features[0][7] & LMP_LSTO)
#define lmp_inq_tx_pwr_capable(dev) ((dev)->features[0][7] & LMP_INQ_TX_PWR)
#define lmp_ext_feat_capable(dev) ((dev)->features[0][7] & LMP_EXTFEATURES)
+#define lmp_transp_capable(dev) ((dev)->features[0][2] & LMP_TRANSPARENT)

/* ----- Extended LMP capabilities ----- */
#define lmp_host_ssp_capable(dev) ((dev)->features[1][0] & LMP_HOST_SSP)
--
1.7.9.5


2013-07-12 16:05:42

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 8/9] Bluetooth: Prevent transparent SCO on older devices

Older Bluetooth devices may not support Setup Synchronous Connection or SCO
transparent data. This is indicated by the corresponding LMP feature bits.
It is not possible to know if the adapter support these features before setting
BT_VOICE option since the socket is not bound to an adapter. An adapter can
also be added after the socket is created. The socket can be bound to an
address before adapter is plugged in.

Thus, on a such adapters, if user request BT_VOICE_TRANSPARENT, outgoing
connections fail on connect() and returns -EOPNOTSUPP. Incoming connections
do not fail. However, they should only be allowed depending on what was
specified in Write_Voice_Settings command.

Signed-off-by: Frédéric Dalleau <[email protected]>
---
net/bluetooth/sco.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 934002c..190765c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -176,6 +176,12 @@ static int sco_connect(struct sock *sk)
else
type = SCO_LINK;

+ if (sco_pi(sk)->setting == BT_VOICE_TRANSPARENT &&
+ (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) {
+ err = -ECONNABORTED;
+ goto done;
+ }
+
hcon = hci_connect_sco(hdev, type, dst, sco_pi(sk)->setting);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
--
1.7.9.5


2013-07-12 16:05:36

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 2/9] Bluetooth: Remove unused mask parameter in sco_conn_defer_accept

>From Bluetooth Core v4.0 specification, 7.1.8 Accept Connection Request Command
"When accepting synchronous connection request, the Role parameter is not used
and will be ignored by the BR/EDR Controller."

Signed-off-by: Frédéric Dalleau <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/sco.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 7114984..0c15a0c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -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 = 0x00; /* Ignored */

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;
msg->msg_namelen = 0;

--
1.7.9.5


2013-07-12 16:05:40

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 6/9] Bluetooth: Parameters for outgoing SCO connections

In order to establish a transparent SCO connection, the correct settings must
be specified in the Setup Synchronous Connection request. For that,
a setting field is added to ACL connection data to set up the desired
parameters.
Remove usage of hdev->voice_setting in CVSD connection.
Make use of T2 parameters for transparent data.

Signed-off-by: Frédéric Dalleau <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 21 +++++++++++++++++----
2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 69bdb67..61ca2ce 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -320,6 +320,7 @@ struct hci_conn {
__u32 passkey_notify;
__u8 passkey_entered;
__u16 disc_timeout;
+ __u16 setting;
unsigned long flags;

__u8 remote_cap;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index d1d9919..c4d6163 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -185,13 +185,24 @@ 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;
+ cp.voice_setting = cpu_to_le16(conn->setting);
+
+ switch (conn->setting & SCO_AIRMODE_MASK) {
+ 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.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.retrans_effort = 0xff;
+ break;
+ }

hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
}
@@ -584,6 +595,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);
--
1.7.9.5


2013-07-12 16:05:38

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 4/9] Bluetooth: Constants declaration for SCO airmode

This patchs define constants for 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.

Signed-off-by: Frédéric Dalleau <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci_core.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 679b07f..69bdb67 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1215,4 +1215,8 @@ 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
+
#endif /* __HCI_CORE_H */
--
1.7.9.5


2013-07-12 16:05:35

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 1/9] 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.

Signed-off-by: Frédéric Dalleau <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
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(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f77885e..679b07f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -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);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 6c7f363..d1d9919 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -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);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index e7bd4ee..7114984 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -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;
--
1.7.9.5


2013-07-12 16:05:39

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v9 5/9] Bluetooth: Use voice setting in defered SCO connection request

When an incoming eSCO 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.

Signed-off-by: Frédéric Dalleau <[email protected]>
---
net/bluetooth/sco.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index ce8690f..934002c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -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,21 @@ 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;
+ cp.content_format = cpu_to_le16(setting);
+
+ switch (setting & SCO_AIRMODE_MASK) {
+ 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.retrans_effort = 0x02;
+ break;
+ case SCO_AIRMODE_CVSD:
+ cp.max_latency = __constant_cpu_to_le16(0xffff);
+ cp.retrans_effort = 0xff;
+ break;
+ }

hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
sizeof(cp), &cp);
@@ -695,7 +707,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;
msg->msg_namelen = 0;

--
1.7.9.5