sco: SCO socket option for voice_setting
Subsystems (2)
v7 cover sco: SCO socket option for voice_setting
Hi,
v7 changes defaults to BT_VOICE_CVSD
Remove mask parameter to sco_conn_defer_accept, it was always 0 check the bits for air codec instead of use constants.
Add S3, S2, S1, D1, D0 settings.
The controller default is now only used to initialize the controller or fill in the missing information in case of using the old Add_SCO command
v6 fixes style issues
v5 changes interface to SOL_BLUETOOTH, BT_VOICE.
Rework fallback mechanism.
This is the patch version 4 of the socket option for enabling transparent SCO. As requested by Marcel, this is now a 16-bit voice_setting. 0x0000 is the value corresponding to current behavior.0x0003 is the value to use for enabling transparent data.
It is easy to allow all possible values from Bluetooth core spec, but I guess results can be unexpected...
Should we filter allowed values in setsockopt ?
Let me know what you think.
Regards,
Fred
Frédéric Dalleau (8):
Bluetooth: Use hci_connect_sco directly
Bluetooth: Remove unused mask parameter in sco_conn_defer_accept
Bluetooth: Add bluetooth socket voice option
Bluetooth: Constants and macro declaration for SCO airmode
Bluetooth: Use voice setting in incoming SCO connection
Bluetooth: Parameters for outgoing SCO connections
Bluetooth: Fallback transparent SCO from T2 to T1
Bluetooth: Add support for fallback between CVSD recommended settings
include/net/bluetooth/bluetooth.h | 8 +++++ include/net/bluetooth/hci_core.h | 9 +++++ include/net/bluetooth/sco.h | 1 + net/bluetooth/hci_conn.c | 56 +++++++++++++++++++++++------ net/bluetooth/hci_event.c | 3 +- net/bluetooth/sco.c | 70 ++++++++++++++++++++++++++++++------- 6 files changed, 123 insertions(+), 24 deletions(-)
Signature · 1 line
1.7.9.5
PATCH v7 1/8 Bluetooth: Use hci_connect_sco directly
hci_connect is a super function for connecting hci protocols. But the voice_setting parameter is only needed by SCO and security requirements are not needed for SCO channels. Thus, it makes sense to have a separate function.
--- include/net/bluetooth/hci_core.h | 2 ++ net/bluetooth/hci_conn.c | 9 +++------ net/bluetooth/sco.c | 3 +-- 3 files changed, 6 insertions(+), 8 deletions(-)
include/net/bluetooth/hci_core.h +2 -0
@@ -584,6 +584,8 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle);struct hci_conn *hci_connect (struct hci_dev *hdev,int type,bdaddr_t *dst,__u8 dst_type,__u8 sec_level,__u8 auth_type); +struct hci_conn *hci_connect_sco (struct hci_dev *hdev,int type,bdaddr_t *dst, +__u16 setting);int hci_conn_check_link_mode (struct hci_conn *conn);int hci_conn_check_secure (struct hci_conn *conn,__u8 sec_level);int hci_conn_security (struct hci_conn *conn,__u8 sec_level,__u8 auth_type);
net/bluetooth/hci_conn.c +3 -6
@@ -560,13 +560,13 @@ static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,return acl; } -static struct hci_conn *hci_connect_sco (struct hci_dev *hdev,int type, -bdaddr_t *dst,u8 sec_level,u8 auth_type) +struct hci_conn *hci_connect_sco (struct hci_dev *hdev,int type,bdaddr_t *dst, +__u16 setting) {struct hci_conn *acl;struct hci_conn *sco; - acl =hci_connect_acl (hdev, dst, sec_level, auth_type); + acl =hci_connect_acl (hdev, dst,BT_SECURITY_LOW ,HCI_AT_NO_BONDING );if (IS_ERR (acl))return acl;
@@ -612,9 +612,6 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,return hci_connect_le (hdev, dst, dst_type, sec_level, auth_type);case ACL_LINK :return hci_connect_acl (hdev, dst, sec_level, auth_type); -case SCO_LINK : -case ESCO_LINK : -return hci_connect_sco (hdev, type, dst, sec_level, auth_type); }return ERR_PTR (-EINVAL );
net/bluetooth/sco.c +1 -2
@@ -176,8 +176,7 @@ static int sco_connect(struct sock *sk)else type =SCO_LINK ; - hcon =hci_connect (hdev, type, dst,BDADDR_BREDR ,BT_SECURITY_LOW , -HCI_AT_NO_BONDING ); + hcon =hci_connect_sco (hdev, type, dst,sco_pi (sk)->setting);if (IS_ERR (hcon)) { err =PTR_ERR (hcon); goto done;
Signature · 1 line
1.7.9.5
PATCH v7 2/8 Bluetooth: Remove unused mask parameter in sco_conn_defer_accept
In reply to Frédéric Dalleau--- net/bluetooth/sco.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
net/bluetooth/sco.c +3 -7
@@ -651,7 +651,7 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,return err; } -static void sco_conn_defer_accept (struct hci_conn *conn,int mask) +static void sco_conn_defer_accept (struct hci_conn *conn) {struct hci_dev *hdev = conn->hdev;
@@ -663,11 +663,7 @@ static void sco_conn_defer_accept(struct hci_conn *conn, int mask)struct hci_cp_accept_conn_req cp;bacpy (&cp.bdaddr, &conn->dst); - -if (lmp_rswitch_capable (hdev) && (mask &HCI_LM_MASTER )) - cp.role =0x00 ;/* Become master */ -else - cp.role =0x01 ;/* Remain slave */ + cp.role =0x01 ;/* Remain slave */ hci_send_cmd (hdev,HCI_OP_ACCEPT_CONN_REQ ,sizeof (cp), &cp); }else {
@@ -697,7 +693,7 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,if (sk->sk_state ==BT_CONNECT2 &&test_bit (BT_SK_DEFER_SETUP , &bt_sk (sk)->flags)) { -sco_conn_defer_accept (pi->conn->hcon,0 ); +sco_conn_defer_accept (pi->conn->hcon); sk->sk_state =BT_CONFIG ;release_sock (sk);
Signature · 1 line
1.7.9.5
Marcel Holtmann
maintainerRe: [PATCH v7 2/8] Bluetooth: Remove unused mask parameter in sco_conn_defer_accept
Hi Fred,
Quoted 1 line
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
PATCH v7 3/8 Bluetooth: Add bluetooth socket voice option
In reply to Frédéric DalleauThis patch extends the current bluetooth socket option to add BT_VOICE. This is intended to choose voice data type at runtime. It only applies to SCO sockets.
Incoming connections shall be setup during defered setup. Outgoing connections shall be setup before connect(). The desired setting is stored in the sco socket info.
This patch declares needed members, modifies getsockopt() and setsockopt().
--- include/net/bluetooth/bluetooth.h | 8 ++++++++ include/net/bluetooth/sco.h | 1 + net/bluetooth/sco.c | 38 ++++++++++++++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 1 deletion(-)
include/net/bluetooth/bluetooth.h +8 -0
@@ -107,6 +107,14 @@ struct bt_power {*/ #define BT_CHANNEL_POLICY_AMP_PREFERRED 2 +#define BT_VOICE 11 +struct bt_voice { +__u16 setting; +}; + +#define BT_VOICE_TRANSPARENT 0x0003 +#define BT_VOICE_CVSD 0x0060 + __printf(1, 2)int bt_info (const char *fmt, ...);__printf (1 ,2 )
include/net/bluetooth/sco.h +1 -0
@@ -73,6 +73,7 @@ struct sco_conn {struct sco_pinfo {struct bt_sock bt;__u32 flags; +__u16 setting;struct sco_conn *conn; };
net/bluetooth/sco.c +37 -1
@@ -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, charclear_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, charbreak ; +case BT_VOICE : + voice.setting =sco_pi (sk)->setting; + + len = min_t(unsigned int, len, sizeof(voice)); +if (copy_to_user (optval, (char *)&voice, len)) + err = -EFAULT ; + +break ; +default : err = -ENOPROTOOPT ;break ;
Signature · 1 line
1.7.9.5
Marcel Holtmann
maintainerRe: [PATCH v7 3/8] Bluetooth: Add bluetooth socket voice option
Hi Fred,
Quoted 20 lines
This patch extends the current bluetooth socket option to add
BT_VOICE. This is intended to choose voice data type at runtime. It only applies to SCO sockets.
Incoming connections shall be setup during defered setup. Outgoing connections shall be setup beforeconnect(). The desired setting is stored in the sco socket info.
This patch declares needed members, modifiesgetsockopt()andsetsockopt().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(-)include/net/bluetooth/bluetooth.h +0
-0
include/net/bluetooth/bluetooth.h struct bt_power { · line 116
Quoted 10 lines
include/net/bluetooth/bluetooth.h +7
-0@@ -107,6 +107,14 @@ struct bt_power {*/ #define BT_CHANNEL_POLICY_AMP_PREFERRED 2 +#define BT_VOICE 11 +struct bt_voice { +__u16 setting; +}; + +#define BT_VOICE_TRANSPARENT 0x0003 +#define BT_VOICE_CVSD 0x0060
call this BT_VOICE_CVSD_16BIT. You could run this with CVSD_8BIT as well if we ever care to get that fixed.
Quoted 40 lines
include/net/bluetooth/bluetooth.h +1
-0+ __printf(1, 2)int bt_info (const char *fmt, ...);__printf (1 ,2 )include/net/bluetooth/sco.h +1
-0@@ -73,6 +73,7 @@ struct sco_conn {struct sco_pinfo {struct bt_sock bt;__u32 flags; +__u16 setting;struct sco_conn *conn; };net/bluetooth/sco.c +4
-1@@ -416,6 +416,8 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock, int pro sk->sk_protocol = proto; sk->sk_state =BT_OPEN ; +sco_pi (sk)->setting =BT_VOICE_CVSD ; +setup_timer (&sk->sk_timer, sco_sock_timeout, (unsigned long )sk);bt_sock_link (&sco_sk_list, sk);@@ -708,7 +710,8 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,static int sco_sock_setsockopt (struct socket *sock,int level,int optname,char __user *optval,unsigned int optlen) {struct sock *sk = sock->sk; -int err =0 ; +int len, err =0 ; +struct bt_voice voice;u32 opt;BT_DBG ("sk %p" , sk);
net/bluetooth/sco.c static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char · line 746
Quoted 10 lines
net/bluetooth/sco.c +7
-0@@ -734,6 +737,29 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, charclear_bit (BT_SK_DEFER_SETUP , &bt_sk (sk)->flags);break ; +case BT_VOICE : +if (sk->sk_state !=BT_OPEN && sk->sk_state !=BT_BOUND && + sk->sk_state !=BT_CONNECT2 ) { + err = -EINVAL ; +break ; + } +
Actually you need to set defaults here first. Otherwise updated userspaces with more options might not work properly.
voice.setting = sco_pi(sk)->setting.
net/bluetooth/sco.c static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char · line 753
Quoted 7 lines
net/bluetooth/sco.c +7
-0+ len = min_t(unsigned int, sizeof(voice), optlen); +if (copy_from_user ((char *) &voice, optval, len)) { + err = -EFAULT ; +break ; + } + +
Double empty line here. I assume you wanted to add a comment that for now we are only accepting two settings at the moment.
Quoted 26 lines
net/bluetooth/sco.c +12
-0+if (voice.setting !=BT_VOICE_TRANSPARENT && + voice.setting !=BT_VOICE_CVSD ) { + err = -EINVAL ; +break ; + } + +sco_pi (sk)->setting = voice.setting; +break ; +default : err = -ENOPROTOOPT ;break ;@@ -803,6 +829,7 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char {struct sock *sk = sock->sk;int len, err =0 ; +struct bt_voice voice;BT_DBG ("sk %p" , sk);@@ -828,6 +855,15 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, charbreak ; +case BT_VOICE : + voice.setting =sco_pi (sk)->setting;
net/bluetooth/sco.c static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char · line 869
Quoted 10 lines
net/bluetooth/sco.c +7
-0+ + len = min_t(unsigned int, len, sizeof(voice)); +if (copy_to_user (optval, (char *)&voice, len)) + err = -EFAULT ; + +break ; +default : err = -ENOPROTOOPT ;break ;
Regards
Marcel
PATCH v7 4/8 Bluetooth: Constants and macro declaration for SCO airmode
In reply to Frédéric DalleauThis patchs define constants and macro for extracting SCO airmode from SCO voice setting. It refers to Bluetooth Core V4.0 specification, Part E, Chap 6.12 which describe SCO voice setting format.
--- include/net/bluetooth/hci_core.h | 5 +++++ 1 file changed, 5 insertions(+)
include/net/bluetooth/hci_core.h +5 -0
@@ -1214,4 +1214,9 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],u8 bdaddr_to_le (u8 bdaddr_type); +#define SCO_AIRMODE_MASK 0x0003 +#define SCO_AIRMODE_CVSD 0x0000 +#define SCO_AIRMODE_TRANSP 0x0003 +#define hci_sco_airmode (setting) ((setting) & SCO_AIRMODE_MASK) +#endif /* __HCI_CORE_H */
Signature · 1 line
1.7.9.5
Marcel Holtmann
maintainerRe: [PATCH v7 4/8] Bluetooth: Constants and macro declaration for SCO airmode
Quoted 14 lines · Frédéric Dalleau
This patchs define constants and macro for extracting SCO airmode from SCO voice setting. It refers to Bluetooth Core V4.0 specification, Part E, Chap 6.12 which describe SCO voice setting format.
Signed-off-by Frédéric Dalleau <[email protected]>---
include/net/bluetooth/hci_core.h| 5 +++++1 file changed, 5 insertions(+)include/net/bluetooth/hci_core.h +0
-0
include/net/bluetooth/hci_core.h void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8], · line 1220
Quoted 7 lines
include/net/bluetooth/hci_core.h +4
-0@@ -1214,4 +1214,9 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],u8 bdaddr_to_le (u8 bdaddr_type); +#define SCO_AIRMODE_MASK 0x0003 +#define SCO_AIRMODE_CVSD 0x0000 +#define SCO_AIRMODE_TRANSP 0x0003 +#define hci_sco_airmode (setting) ((setting) & SCO_AIRMODE_MASK)
the constants are fine, but I would not bother with the macro. Just use SCO_AIRMODE_MASK directly.
Regards
Marcel
PATCH v7 5/8 Bluetooth: Use voice setting in incoming SCO connection
In reply to Frédéric DalleauWhen an incoming SCO connection is requested, check the selected voice setting, and reply appropriately. Voice setting should have been negotiated previously. For example, in case of HFP, the codec is negotiated using AT commands on the RFCOMM channel. This patch only changes replies for socket with defered setup enabled.
--- net/bluetooth/sco.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
net/bluetooth/sco.c +18 -5
@@ -653,7 +653,7 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,return err; } -static void sco_conn_defer_accept (struct hci_conn *conn) +static void sco_conn_defer_accept (struct hci_conn *conn,int setting) {struct hci_dev *hdev = conn->hdev;
@@ -676,9 +676,22 @@ static void sco_conn_defer_accept(struct hci_conn *conn) cp.tx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); cp.rx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); - cp.max_latency =__constant_cpu_to_le16 (0xffff ); - cp.content_format =cpu_to_le16 (hdev->voice_setting); - cp.retrans_effort =0xff ; + +switch (hci_sco_airmode (setting)) { +case SCO_AIRMODE_TRANSP : +if (conn->pkt_type &ESCO_2EV3 ) + cp.max_latency =__constant_cpu_to_le16 (0x0008 ); +else + cp.max_latency =__constant_cpu_to_le16 (0x000D ); + cp.content_format =__constant_cpu_to_le16 (SCO_AIRMODE_TRANSP ); + cp.retrans_effort =0x02 ; +break ; +case SCO_AIRMODE_CVSD : + cp.max_latency =__constant_cpu_to_le16 (0xffff ); + cp.content_format =cpu_to_le16 (setting); + cp.retrans_effort =0xff ; +break ; + }hci_send_cmd (hdev,HCI_OP_ACCEPT_SYNC_CONN_REQ ,sizeof (cp), &cp);
@@ -695,7 +708,7 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,if (sk->sk_state ==BT_CONNECT2 &&test_bit (BT_SK_DEFER_SETUP , &bt_sk (sk)->flags)) { -sco_conn_defer_accept (pi->conn->hcon); +sco_conn_defer_accept (pi->conn->hcon, pi->setting); sk->sk_state =BT_CONFIG ;release_sock (sk);
Signature · 1 line
1.7.9.5
Marcel Holtmann
maintainerRe: [PATCH v7 5/8] Bluetooth: Use voice setting in incoming SCO connection
Hi Fred,
Quoted 25 lines
When an incoming SCO connection is requested, check the selected voice setting, and reply appropriately. Voice setting should have been negotiated previously. For example, in case of HFP, the codec is negotiated using AT commands on the RFCOMM channel. This patch only changes replies for socket with defered setup enabled.
Signed-off-by Frédéric Dalleau <[email protected]>---
net/bluetooth/sco.c| 23 ++++++++++++++++++-----1 file changed, 18 insertions(+), 5 deletions(-)net/bluetooth/sco.c +1
-1@@ -653,7 +653,7 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,return err; } -static void sco_conn_defer_accept (struct hci_conn *conn) +static void sco_conn_defer_accept (struct hci_conn *conn,int setting) {struct hci_dev *hdev = conn->hdev;
net/bluetooth/sco.c static void sco_conn_defer_accept(struct hci_conn *conn) · line 681
Quoted 6 lines
net/bluetooth/sco.c +0
-3@@ -676,9 +676,22 @@ static void sco_conn_defer_accept(struct hci_conn *conn) cp.tx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); cp.rx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); - cp.max_latency =__constant_cpu_to_le16 (0xffff ); - cp.content_format =cpu_to_le16 (hdev->voice_setting); - cp.retrans_effort =0xff ;
so we are now ignoring the hdev->voice_setting. This is fine for eSCO, but what are we doing in case we have a 1.1 controller that only supports the Add_SCO command?
Regards
Marcel
PATCH v7 6/8 Bluetooth: Parameters for outgoing SCO connections
In reply to Frédéric DalleauIn order to establish a transparent SCO connection, the correct settings must be specified in the Setup Synchronous Connection request. For that, voice_setting is added to ACL connection flags to set up the desired parameters. If this value is zero, a legacy SCO connection will be requested. This patch uses T2 settings.
--- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_conn.c | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-)
include/net/bluetooth/hci_core.h +1 -0
@@ -320,6 +320,7 @@ struct hci_conn {__u32 passkey_notify;__u8 passkey_entered;__u16 disc_timeout; +__u16 setting;unsigned long flags;__u8 remote_cap;
net/bluetooth/hci_conn.c +18 -4
@@ -185,13 +185,25 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle) conn->attempt++; cp.handle =cpu_to_le16 (handle); - cp.pkt_type =cpu_to_le16 (conn->pkt_type); cp.tx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); cp.rx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); - cp.max_latency =__constant_cpu_to_le16 (0xffff ); - cp.voice_setting =cpu_to_le16 (hdev->voice_setting); - cp.retrans_effort =0xff ; + +switch (hci_sco_airmode (conn->setting)) { +case SCO_AIRMODE_TRANSP : + cp.pkt_type =__constant_cpu_to_le16 (EDR_ESCO_MASK & + ~ESCO_2EV3 ); + cp.max_latency =__constant_cpu_to_le16 (0x000d ); + cp.voice_setting =__constant_cpu_to_le16 (SCO_AIRMODE_TRANSP ); + cp.retrans_effort =0x02 ; +break ; +case SCO_AIRMODE_CVSD : + cp.pkt_type =cpu_to_le16 (conn->pkt_type); + cp.max_latency =__constant_cpu_to_le16 (0xffff ); + cp.voice_setting =cpu_to_le16 (hdev->voice_setting); + cp.retrans_effort =0xff ; +break ; + }hci_send_cmd (hdev,HCI_OP_SETUP_SYNC_CONN ,sizeof (cp), &cp); }
@@ -584,6 +596,8 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,hci_conn_hold (sco); + sco->setting = setting; +if (acl->state ==BT_CONNECTED && (sco->state ==BT_OPEN || sco->state ==BT_CLOSED )) {set_bit (HCI_CONN_POWER_SAVE , &acl->flags);
Signature · 1 line
1.7.9.5
Marcel Holtmann
maintainerRe: [PATCH v7 6/8] Bluetooth: Parameters for outgoing SCO connections
Hi Fred,
Quoted 29 lines
In order to establish a transparent SCO connection, the correct settings must be specified in the Setup Synchronous Connection request. For that,
voice_settingis 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(-)include/net/bluetooth/hci_core.h +1
-0@@ -320,6 +320,7 @@ struct hci_conn {__u32 passkey_notify;__u8 passkey_entered;__u16 disc_timeout; +__u16 setting;unsigned long flags;__u8 remote_cap;net/bluetooth/hci_conn.c +0
-0
net/bluetooth/hci_conn.c void hci_setup_sync(struct hci_conn *conn, __u16 handle) · line 194
Quoted 10 lines
net/bluetooth/hci_conn.c +0
-4@@ -185,13 +185,25 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle) conn->attempt++; cp.handle =cpu_to_le16 (handle); - cp.pkt_type =cpu_to_le16 (conn->pkt_type); cp.tx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); cp.rx_bandwidth =__constant_cpu_to_le32 (0x00001f40 ); - cp.max_latency =__constant_cpu_to_le16 (0xffff ); - cp.voice_setting =cpu_to_le16 (hdev->voice_setting); - cp.retrans_effort =0xff ;
same here. What are we doing with Add_SCO fallback on older controllers.
We could just reject setting transparent air mode for controllers that do not support eSCO command procedures. And the only valid option there would be CVSD then. Which is default.
In case we use Add_SCO is is important that the voice setting programmed into the controller is actually 16-bit CVSD at that time. That needs to be taken care of as well.
Regards
Marcel
Re: [PATCH v7 6/8] Bluetooth: Parameters for outgoing SCO connections
Hi,
On Fri, May 17, 2013 at 5:55 AM, Marcel Holtmann <[email protected]> wrot= e:
Quoted 3 lines
Hi Fred,
Quoted 1 line
In order to establish a transparent SCO connection, the correct settings=
must
Quoted 3 lines
Quoted 3 lines
be specified in the Setup Synchronous Connection request. For that,
voice_settingis added to ACL connection flags to set up the desired parameters. If this value is zero, a legacy SCO connection will be reque=
sted.
Quoted 3 lines
Quoted 3 lines
This patch uses T2 settings.
Signed-off-by Fr=C3=A9d=C3=A9ric Dalleau <[email protected].=
com>
Quoted 6 lines
include/net/bluetooth/hc=
Quoted 6 lines
---
include/net/bluetooth/hci_core.h| 1 +net/bluetooth/hci_conn.c| 22 ++++++++++++++++++----2 files changed, 19 insertions(+), 4 deletions(-)include/net/bluetooth/hc= renamed from include/net/bluetooth/hci_core.h +0
-0
i_core.h
Quoted 10 lines
Quoted 10 lines
index a6fd544..b5996b3 100644
include/net/bluetooth/hci_core.h +1
-0@@ -320,6 +320,7 @@ struct hci_conn {__u32 passkey_notify;__u8 passkey_entered;__u16 disc_timeout; +__u16 setting;unsigned long flags;
Quoted 6 lines
net/bluetooth/hci_conn.c void hci_setup_sync(struct hci_conn *conn, __u16 h=
Quoted 6 lines
include/net/bluetooth/hci_core.h +0
-0__u8 remote_cap;net/bluetooth/hci_conn.c +0
-0@@ -185,13 +185,25 @@ void hci_setup_sync(struct hci_conn *conn, __u16 h=
andle)
Quoted 11 lines
net/bluetooth/hci_conn.c void hci_setup_sync(struct hci_conn *conn, __u16 h= · line 194
Quoted 10 lines
net/bluetooth/hci_conn.c +0
-4conn->attempt++; cp.handle =3D cpu_to_le16(handle); - cp.pkt_type =3D cpu_to_le16(conn->pkt_type); cp.tx_bandwidth =3D __constant_cpu_to_le32(0x00001f40); cp.rx_bandwidth =3D __constant_cpu_to_le32(0x00001f40); - cp.max_latency =3D __constant_cpu_to_le16(0xffff); - cp.voice_setting =3D cpu_to_le16(hdev->voice_setting); - cp.retrans_effort =3D 0xff;
Quoted 3 lines
same here. What are we doing with
Add_SCOfallback on older controllers.We could just reject setting transparent air mode for controllers that do=
not support eSCO command procedures. And the only valid option there would= be CVSD then. Which is default.
I have seem some controllers (IIRC CSR 2.0) that don't have eSCO but have support for transparent air mode, in these cases we want to use the Setup SCO/Accept SCO procedures.
Quoted 2 lines
In case we use
Add_SCOis is important that the voice setting programmed =
into the controller is actually 16-bit CVSD at that time. That needs to be = taken care of as well.
Quoted 7 lines
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
Quoted 2 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
PATCH v7 7/8 Bluetooth: Fallback transparent SCO from T2 to T1
In reply to Frédéric DalleauWhen initiating a transparent eSCO connection, make use of T2 settings at first try. T2 is the recommended settings from HFP 1.6 WideBand Speech. Upon connection failure, try T1 settings.
T2 failure is detected if Synchronous Connection Complete event fails with error 0x0d. This error code has been found experimentally by sending a T2 request to a T1 only SCO listener. It means "Connection Rejected due to Limited resource".
To know which of T2 or T1 should be used, conn->fallback is used. Bluez only attempt to reconnect twice. In the future, we will need more than 2 fallback possibilities. Thus, test conn->fallback as an alternative measure. We want to fallback only if conn->fallback is positive. Calling hci_setup_sync with conn->fallback == 0 is an initial connection attempt.
--- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_conn.c | 20 +++++++++++++++++--- net/bluetooth/hci_event.c | 3 ++- 3 files changed, 20 insertions(+), 4 deletions(-)
include/net/bluetooth/hci_core.h +1 -0
@@ -321,6 +321,7 @@ struct hci_conn {__u8 passkey_entered;__u16 disc_timeout;__u16 setting; +__u8 fallback;unsigned long flags;__u8 remote_cap;
net/bluetooth/hci_conn.c +17 -3
@@ -31,6 +31,17 @@#include <net/bluetooth/a2mp.h> #include <net/bluetooth/smp.h> +struct sco_param { +u16 pkt_type; +u16 max_latency; +u8 next; +}; + +static const struct sco_param sco_param_wideband[] = { + {EDR_ESCO_MASK & ~ESCO_2EV3 ,0x000d ,1 }, + {EDR_ESCO_MASK |ESCO_EV3 ,0x0008 ,0 }, +}; +static void hci_le_create_connection (struct hci_conn *conn) {struct hci_dev *hdev = conn->hdev;
@@ -176,6 +187,7 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle) {struct hci_dev *hdev = conn->hdev;struct hci_cp_setup_sync_conn cp; +const struct sco_param *param;BT_DBG ("hcon %p" , conn);
@@ -191,11 +203,13 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)switch (hci_sco_airmode (conn->setting)) {case SCO_AIRMODE_TRANSP : - cp.pkt_type =__constant_cpu_to_le16 (EDR_ESCO_MASK & - ~ESCO_2EV3 ); - cp.max_latency =__constant_cpu_to_le16 (0x000d ); + param = &sco_param_wideband[conn->fallback]; + cp.pkt_type =__constant_cpu_to_le16 (param->pkt_type); + cp.max_latency =__constant_cpu_to_le16 (param->max_latency); cp.voice_setting =__constant_cpu_to_le16 (SCO_AIRMODE_TRANSP ); cp.retrans_effort =0x02 ; + + conn->fallback = param->next;break ;case SCO_AIRMODE_CVSD : cp.pkt_type =cpu_to_le16 (conn->pkt_type);
net/bluetooth/hci_event.c +2 -1
@@ -2904,11 +2904,12 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,hci_conn_add_sysfs (conn);break ; +case 0x0d :/* No resource available */ case 0x11 :/* Unsupported Feature or Parameter Value */ case 0x1c :/* SCO interval rejected */ case 0x1a :/* Unsupported Remote Feature */ case 0x1f :/* Unspecified error */ -if (conn->out && conn->attempt <2 ) { +if (conn->out && (conn->attempt <2 || conn->fallback >0 )) { conn->pkt_type = (hdev->esco_type &SCO_ESCO_MASK ) | (hdev->esco_type &EDR_ESCO_MASK );hci_setup_sync (conn, conn->link->handle);
Signature · 1 line
1.7.9.5
Marcel Holtmann
maintainerRe: [PATCH v7 7/8] Bluetooth: Fallback transparent SCO from T2 to T1
Hi Fred,
Quoted 41 lines
When initiating a transparent eSCO connection, make use of T2 settings at first try. T2 is the recommended settings from HFP 1.6 WideBand Speech. Upon connection failure, try T1 settings.
T2 failure is detected if Synchronous Connection Complete event fails with error0x0d. 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->fallbackis used. Bluez only attempt to reconnect twice. In the future, we will need more than 2 fallback possibilities. Thus, testconn->fallbackas an alternative measure. We want to fallback only ifconn->fallbackis positive. Callinghci_setup_syncwithconn->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(-)include/net/bluetooth/hci_core.h +1
-0@@ -321,6 +321,7 @@ struct hci_conn {__u8 passkey_entered;__u16 disc_timeout;__u16 setting; +__u8 fallback;unsigned long flags;__u8 remote_cap;net/bluetooth/hci_conn.c +1
-0@@ -31,6 +31,17 @@#include <net/bluetooth/a2mp.h> #include <net/bluetooth/smp.h> +struct sco_param {
net/bluetooth/hci_conn.c line 44
Quoted 10 lines
net/bluetooth/hci_conn.c +10
-0+u16 pkt_type; +u16 max_latency; +u8 next; +}; + +static const struct sco_param sco_param_wideband[] = { + {EDR_ESCO_MASK & ~ESCO_2EV3 ,0x000d ,1 }, + {EDR_ESCO_MASK |ESCO_EV3 ,0x0008 ,0 }, +}; +
so I would combine patch 7 and 8 into one and have a detailed commit explaining both section.
Also comment the entries in the table properly to list which is which setting. So others have an idea on what is going on.
Regards
Marcel
PATCH v7 8/8 Bluetooth: Add support for fallback between CVSD recommended settings
In reply to Frédéric DalleauWhen CVSD is requested and eSCO is supported, try to establish eSCO connection using S3 settings. If it fails, fallback in sequence to S2, S1, D1, D0 settings. These settings are described in Bluetooth HFP 1.6 specification p. 101.
--- net/bluetooth/hci_conn.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
net/bluetooth/hci_conn.c +15 -4
@@ -42,6 +42,14 @@ static const struct sco_param sco_param_wideband[] = { { EDR_ESCO_MASK | ESCO_EV3, 0x0008, 0 }, }; +static const struct sco_param sco_param_cvsd[] = { + {EDR_ESCO_MASK & ~ESCO_2EV3 ,0x000a ,1 }, + {EDR_ESCO_MASK & ~ESCO_2EV3 ,0x0007 ,2 }, + {EDR_ESCO_MASK |ESCO_EV3 ,0x0007 ,3 }, + {EDR_ESCO_MASK |ESCO_HV3 ,0xffff ,4 }, + {EDR_ESCO_MASK |ESCO_HV1 ,0xffff ,0 }, +}; +static void hci_le_create_connection (struct hci_conn *conn) {struct hci_dev *hdev = conn->hdev;
@@ -212,10 +220,13 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle) conn->fallback = param->next;break ;case SCO_AIRMODE_CVSD : - cp.pkt_type =cpu_to_le16 (conn->pkt_type); - cp.max_latency =__constant_cpu_to_le16 (0xffff ); - cp.voice_setting =cpu_to_le16 (hdev->voice_setting); - cp.retrans_effort =0xff ; + param = &sco_param_cvsd[conn->fallback]; + cp.pkt_type =__constant_cpu_to_le16 (param->pkt_type); + cp.max_latency =__constant_cpu_to_le16 (param->max_latency); + cp.voice_setting =__constant_cpu_to_le16 (conn->setting); + cp.retrans_effort =0x01 ; + + conn->fallback = param->next;break ; }
Signature · 1 line
1.7.9.5