2013-05-16 17:33:57

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v7 0/8] 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(-)

--
1.7.9.5



2013-05-17 13:10:15

by Vinicius Costa Gomes

[permalink] [raw]
Subject: 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:
> Hi Fred,
>
>> 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 reque=
sted.
>> This patch uses T2 settings.
>>
>> Signed-off-by: Fr=C3=A9d=C3=A9ric Dalleau <[email protected].=
com>
>> ---
>> include/net/bluetooth/hci_core.h | 1 +
>> net/bluetooth/hci_conn.c | 22 ++++++++++++++++++----
>> 2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index a6fd544..b5996b3 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..cd7452e 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -185,13 +185,25 @@ void hci_setup_sync(struct hci_conn *conn, __u16 h=
andle)
>> 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;
>
> 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.

>
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Cheers,
--
Vinicius

2013-05-17 09:00:42

by Marcel Holtmann

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

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.
> 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.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> 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(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b5996b3..c8b91d2 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 cd7452e..8d71d73 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -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 },
> +};
> +

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


2013-05-17 08:55:29

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v7 6/8] 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,
> 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.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 22 ++++++++++++++++++----
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index a6fd544..b5996b3 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..cd7452e 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -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


2013-05-17 08:51:58

by Marcel Holtmann

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

Hi Fred,

> 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.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> net/bluetooth/sco.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 7b4c3df..7881208 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,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


2013-05-17 08:49:58

by Marcel Holtmann

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


On May 16, 2013, at 7:34 PM, Fr?d?ric Dalleau <[email protected]> wrote:

> 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.
>
> Signed-off-by: Fr?d?ric Dalleau <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e9cf9fa..a6fd544 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -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


2013-05-17 08:48:02

by Marcel Holtmann

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

Hi Fred,

> 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]>
> ---
> include/net/bluetooth/bluetooth.h | 8 ++++++++
> include/net/bluetooth/sco.h | 1 +
> net/bluetooth/sco.c | 38 ++++++++++++++++++++++++++++++++++++-
> 3 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 6912ef9..0239a49 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 0x0060

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

> +
> __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 26c9d51..7b4c3df 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;
> +
> 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;
> + }
> +

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

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

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

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

Regards

Marcel


2013-05-17 08:40:25

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] Bluetooth: Remove unused mask parameter in sco_conn_defer_accept

Hi Fred,

> Signed-off-by: Fr?d?ric Dalleau <[email protected]>

please add a commit message on why it is unused and okay to remove.

Regards

Marcel


2013-05-16 17:33:58

by Frédéric DALLEAU

[permalink] [raw]
Subject: [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.

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 606615c..e9cf9fa 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 9e62102..a85182f 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-05-16 17:34:01

by Frédéric DALLEAU

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

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.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e9cf9fa..a6fd544 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -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 */
--
1.7.9.5


2013-05-16 17:34:04

by Frédéric DALLEAU

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

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.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b5996b3..c8b91d2 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 cd7452e..8d71d73 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -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);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index db58e72..9be10be 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-05-16 17:34:02

by Frédéric DALLEAU

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

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.

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

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 7b4c3df..7881208 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,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);
--
1.7.9.5


2013-05-16 17:34:05

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v7 8/8] Bluetooth: Add support for fallback between CVSD recommended 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.
These settings are described in Bluetooth HFP 1.6 specification p. 101.

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

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 8d71d73..f4aa8b2 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -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;
}

--
1.7.9.5


2013-05-16 17:34:03

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v7 6/8] 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,
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.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a6fd544..b5996b3 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..cd7452e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -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);
--
1.7.9.5


2013-05-16 17:33:59

by Frédéric DALLEAU

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

Signed-off-by: Frédéric Dalleau <[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 a85182f..26c9d51 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 = 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);
--
1.7.9.5


2013-05-16 17:34:00

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v7 3/8] 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]>
---
include/net/bluetooth/bluetooth.h | 8 ++++++++
include/net/bluetooth/sco.h | 1 +
net/bluetooth/sco.c | 38 ++++++++++++++++++++++++++++++++++++-
3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 6912ef9..0239a49 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 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 26c9d51..7b4c3df 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;
+
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;
--
1.7.9.5