Since this function is only used by sco, move it from hci_event.c to
sco.c and rename to sco_conn_defer_accept.
Signed-off-by: Frédéric Dalleau <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 -
net/bluetooth/hci_event.c | 36 ------------------------------------
net/bluetooth/sco.c | 38 +++++++++++++++++++++++++++++++++++++-
3 files changed, 37 insertions(+), 38 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 90cf75a..f3be454 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -582,7 +582,6 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
int hci_conn_del(struct hci_conn *conn);
void hci_conn_hash_flush(struct hci_dev *hdev);
void hci_conn_check_pending(struct hci_dev *hdev);
-void hci_conn_accept(struct hci_conn *conn, int mask);
struct hci_chan *hci_chan_create(struct hci_conn *conn);
void hci_chan_del(struct hci_chan *chan);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d4fcba6..3fd0212 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2095,42 +2095,6 @@ unlock:
hci_conn_check_pending(hdev);
}
-void hci_conn_accept(struct hci_conn *conn, int mask)
-{
- struct hci_dev *hdev = conn->hdev;
-
- BT_DBG("conn %p", conn);
-
- conn->state = BT_CONFIG;
-
- if (!lmp_esco_capable(hdev)) {
- 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 */
-
- hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), &cp);
- } else /* lmp_esco_capable(hdev)) */ {
- struct hci_cp_accept_sync_conn_req cp;
-
- bacpy(&cp.bdaddr, &conn->dst);
- 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.content_format = cpu_to_le16(hdev->voice_setting);
- cp.retrans_effort = 0xff;
-
- hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
- sizeof(cp), &cp);
- }
-}
-
static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_conn_request *ev = (void *) skb->data;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index e435641..97177be6 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -654,6 +654,42 @@ 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)
+{
+ struct hci_dev *hdev = conn->hdev;
+
+ BT_DBG("conn %p", conn);
+
+ conn->state = BT_CONFIG;
+
+ if (!lmp_esco_capable(hdev)) {
+ 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 */
+
+ hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), &cp);
+ } else /* lmp_esco_capable(hdev)) */ {
+ struct hci_cp_accept_sync_conn_req cp;
+
+ bacpy(&cp.bdaddr, &conn->dst);
+ 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.content_format = cpu_to_le16(hdev->voice_setting);
+ cp.retrans_effort = 0xff;
+
+ hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
+ sizeof(cp), &cp);
+ }
+}
+
static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *msg, size_t len, int flags)
{
@@ -664,7 +700,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)) {
- hci_conn_accept(pi->conn->hcon, 0);
+ sco_conn_defer_accept(pi->conn->hcon, 0);
sk->sk_state = BT_CONFIG;
release_sock(sk);
--
1.7.9.5
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.
> To know which of T2 or T1 should be used, the connection attempt index is used.
> 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".
>
> Signed-off-by: Frédéric Dalleau <[email protected]>
> ---
> net/bluetooth/hci_conn.c | 11 ++++++++++-
> net/bluetooth/hci_event.c | 1 +
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 504367e..a3f4e29 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -179,12 +179,21 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
> cp.tx_bandwidth = __constant_cpu_to_le32(0x00001f40);
> cp.rx_bandwidth = __constant_cpu_to_le32(0x00001f40);
>
> - if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
> + if (conn->attempt == 1 &&
> + test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
> 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(0x0003);
> cp.retrans_effort = 0x02;
> + } else if (conn->attempt == 2 &&
> + test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
I do not really like abusing a simple counter as a state machine. We
should do something more explicit with tracking if T2 vs T1 sco_flags or
something.
> + cp.pkt_type = __constant_cpu_to_le16(EDR_ESCO_MASK |
> + ESCO_EV3);
> + cp.max_latency = __constant_cpu_to_le16(0x0007);
> + cp.voice_setting = __constant_cpu_to_le16(0x0003);
> + cp.retrans_effort = 0x02;
> + clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags);
If we clear this here, does it mean we end up creating a CVSD link. That
can not be correct. I think we need to fail the connection procedure in
that case. We can not magically turn a transparent link into a CVSD
link. That would confuse the hell out of HFP.
> } else {
> cp.pkt_type = cpu_to_le16(conn->pkt_type);
> cp.max_latency = __constant_cpu_to_le16(0xffff);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 3fd0212..6c1aec9 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3330,6 +3330,7 @@ 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 */
Regards
Marcel
Hi Fred,
> In order to establish a transparent SCO connection, the correct settings must
> be specified in the SetupSynchronousConnection request. For that, a bit is set
> in ACL connection flags to set up the desired parameters. If this bit is not
> set, 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 | 3 +++
> net/bluetooth/hci_conn.c | 29 +++++++++++++++++++----------
> net/bluetooth/sco.c | 3 +--
> 3 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index f3be454..4a216e5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -443,6 +443,7 @@ enum {
> HCI_CONN_SSP_ENABLED,
> HCI_CONN_POWER_SAVE,
> HCI_CONN_REMOTE_OOB,
> + HCI_CONN_SCO_TRANSPARENT,
why are we tracking it like this and not actually the voice setting
itself?
> };
>
> static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> @@ -590,6 +591,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,
> + u8 mode);
> 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 25bfce0..504367e 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -175,13 +175,22 @@ 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;
> +
> + if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
> + 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(0x0003);
> + cp.retrans_effort = 0x02;
> + } else {
> + 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;
> + }
>
> hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
> }
> @@ -551,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,
> + u8 mode)
> {
> 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);
I do not understand this change. Why are you doing this one?
> if (IS_ERR(acl))
> return acl;
>
> @@ -575,6 +584,9 @@ static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
>
> hci_conn_hold(sco);
>
> + if (mode)
> + set_bit(HCI_CONN_SCO_TRANSPARENT, &sco->flags);
> +
> if (acl->state == BT_CONNECTED &&
> (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
> set_bit(HCI_CONN_POWER_SAVE, &acl->flags);
> @@ -603,9 +615,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 7fbb4c4..1d79a2e 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)->mode);
> if (IS_ERR(hcon)) {
> err = PTR_ERR(hcon);
> goto done;
If we really split hci_connect into two entities, then we should have a
preparation patch and then end up with hci_connect_acl and
hci_connect_sco.
Regards
Marcel
Hi Fred,
> This patch extends the current SCO socket option to add a 'mode' field. This
> field is intended to choose data type at runtime. Current modes are CVSD and
> transparent SCO, but adding new modes could allow support for CSA2 and fine
> tuning a sco connection, for example latency, bandwith, voice setting. Incoming
> connections will be setup during defered setup. Outgoing connections have to
> be setup before connect(). The selected type is stored in the sco socket info.
> This patch declares needed members, modifies getsockopt() and implements
> setsockopt(). Setting the mtu is not supported.
>
> Signed-off-by: Frédéric Dalleau <[email protected]>
> ---
> include/net/bluetooth/sco.h | 7 +++++
> net/bluetooth/sco.c | 59 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+)
>
> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
> index 1e35c43..4de67ef 100644
> --- a/include/net/bluetooth/sco.h
> +++ b/include/net/bluetooth/sco.h
> @@ -41,8 +41,14 @@ struct sockaddr_sco {
>
> /* SCO socket options */
> #define SCO_OPTIONS 0x01
> +
> +#define SCO_MODE_CVSD 0x00
> +#define SCO_MODE_TRANSPARENT 0x01
> +#define SCO_MODE_MAX 0x01
> +
> struct sco_options {
> __u16 mtu;
> + __u8 mode;
> };
>
> #define SCO_CONNINFO 0x02
> @@ -73,6 +79,7 @@ struct sco_conn {
> struct sco_pinfo {
> struct bt_sock bt;
> __u32 flags;
> + __u8 mode;
> struct sco_conn *conn;
> };
so I have been reading the 4.0 core spec and CSA2 again. So with CSA2
and the enhanced eSCO setup they are splitting the PCM data format from
the the content format and make them independent in output and input.
While 4.0 core spec is essentially based around the 2-byte voice
setting.
I think we have two choices here, one is to just ignore CSA2 for now and
add a uint16 for voice setting value here, or go ahead with the full
blown crazy that is CSA2.
Splitting out voice setting into 5 byte content format and 1 byte PCM
data format seems straight forward, but then you have input and output
and also the routing path. And instead of a socket option, that looks
more like things for the socket address.
So I would actually propose to add a voice settings uint16 value as SCO
socket field. Using the full voice settings make sense since then we can
also retrieve the fact if we are doing 8-bit or 16-bit PCM. One detail
we kinda ignored since we always defaulted to 16-bit, but would be worth
while fixing while at it.
Regards
Marcel
Hi Fred,
> Since this function is only used by sco, move it from hci_event.c to
> sco.c and rename to sco_conn_defer_accept.
>
> Signed-off-by: Frédéric Dalleau <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 -
> net/bluetooth/hci_event.c | 36 ------------------------------------
> net/bluetooth/sco.c | 38 +++++++++++++++++++++++++++++++++++++-
> 3 files changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 90cf75a..f3be454 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -582,7 +582,6 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
> int hci_conn_del(struct hci_conn *conn);
> void hci_conn_hash_flush(struct hci_dev *hdev);
> void hci_conn_check_pending(struct hci_dev *hdev);
> -void hci_conn_accept(struct hci_conn *conn, int mask);
>
> struct hci_chan *hci_chan_create(struct hci_conn *conn);
> void hci_chan_del(struct hci_chan *chan);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d4fcba6..3fd0212 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2095,42 +2095,6 @@ unlock:
> hci_conn_check_pending(hdev);
> }
>
> -void hci_conn_accept(struct hci_conn *conn, int mask)
> -{
> - struct hci_dev *hdev = conn->hdev;
> -
> - BT_DBG("conn %p", conn);
> -
> - conn->state = BT_CONFIG;
> -
> - if (!lmp_esco_capable(hdev)) {
> - 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 */
> -
> - hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), &cp);
> - } else /* lmp_esco_capable(hdev)) */ {
> - struct hci_cp_accept_sync_conn_req cp;
> -
> - bacpy(&cp.bdaddr, &conn->dst);
> - 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.content_format = cpu_to_le16(hdev->voice_setting);
> - cp.retrans_effort = 0xff;
> -
> - hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
> - sizeof(cp), &cp);
> - }
> -}
> -
> static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct hci_ev_conn_request *ev = (void *) skb->data;
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index e435641..97177be6 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -654,6 +654,42 @@ 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)
> +{
> + struct hci_dev *hdev = conn->hdev;
> +
> + BT_DBG("conn %p", conn);
> +
> + conn->state = BT_CONFIG;
> +
> + if (!lmp_esco_capable(hdev)) {
> + 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 */
> +
> + hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp), &cp);
> + } else /* lmp_esco_capable(hdev)) */ {
I realize that you copied this, but just remove this pointless comment.
The if statement is less than 10 lines above, so no big deal.
> + struct hci_cp_accept_sync_conn_req cp;
> +
> + bacpy(&cp.bdaddr, &conn->dst);
> + 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.content_format = cpu_to_le16(hdev->voice_setting);
> + cp.retrans_effort = 0xff;
> +
> + hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
> + sizeof(cp), &cp);
> + }
> +}
> +
> static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct msghdr *msg, size_t len, int flags)
> {
> @@ -664,7 +700,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)) {
> - hci_conn_accept(pi->conn->hcon, 0);
> + sco_conn_defer_accept(pi->conn->hcon, 0);
> sk->sk_state = BT_CONFIG;
>
> release_sock(sk);
Regards
Marcel
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.
To know which of T2 or T1 should be used, the connection attempt index is used.
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".
Signed-off-by: Frédéric Dalleau <[email protected]>
---
net/bluetooth/hci_conn.c | 11 ++++++++++-
net/bluetooth/hci_event.c | 1 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 504367e..a3f4e29 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -179,12 +179,21 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
cp.tx_bandwidth = __constant_cpu_to_le32(0x00001f40);
cp.rx_bandwidth = __constant_cpu_to_le32(0x00001f40);
- if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+ if (conn->attempt == 1 &&
+ test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
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(0x0003);
cp.retrans_effort = 0x02;
+ } else if (conn->attempt == 2 &&
+ test_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+ cp.pkt_type = __constant_cpu_to_le16(EDR_ESCO_MASK |
+ ESCO_EV3);
+ cp.max_latency = __constant_cpu_to_le16(0x0007);
+ cp.voice_setting = __constant_cpu_to_le16(0x0003);
+ cp.retrans_effort = 0x02;
+ clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags);
} else {
cp.pkt_type = cpu_to_le16(conn->pkt_type);
cp.max_latency = __constant_cpu_to_le16(0xffff);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3fd0212..6c1aec9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3330,6 +3330,7 @@ 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 */
--
1.7.9.5
In order to establish a transparent SCO connection, the correct settings must
be specified in the SetupSynchronousConnection request. For that, a bit is set
in ACL connection flags to set up the desired parameters. If this bit is not
set, 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 | 3 +++
net/bluetooth/hci_conn.c | 29 +++++++++++++++++++----------
net/bluetooth/sco.c | 3 +--
3 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f3be454..4a216e5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -443,6 +443,7 @@ enum {
HCI_CONN_SSP_ENABLED,
HCI_CONN_POWER_SAVE,
HCI_CONN_REMOTE_OOB,
+ HCI_CONN_SCO_TRANSPARENT,
};
static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
@@ -590,6 +591,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,
+ u8 mode);
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 25bfce0..504367e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -175,13 +175,22 @@ 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;
+
+ if (test_and_clear_bit(HCI_CONN_SCO_TRANSPARENT, &conn->flags)) {
+ 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(0x0003);
+ cp.retrans_effort = 0x02;
+ } else {
+ 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;
+ }
hci_send_cmd(hdev, HCI_OP_SETUP_SYNC_CONN, sizeof(cp), &cp);
}
@@ -551,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,
+ u8 mode)
{
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;
@@ -575,6 +584,9 @@ static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type,
hci_conn_hold(sco);
+ if (mode)
+ set_bit(HCI_CONN_SCO_TRANSPARENT, &sco->flags);
+
if (acl->state == BT_CONNECTED &&
(sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
set_bit(HCI_CONN_POWER_SAVE, &acl->flags);
@@ -603,9 +615,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 7fbb4c4..1d79a2e 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)->mode);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
goto done;
--
1.7.9.5
When an incoming SCO connection is requested, check the selected mode, and
reply appropriately. Mode 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 7d57af5..7fbb4c4 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -656,7 +656,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, int mask, int mode)
{
struct hci_dev *hdev = conn->hdev;
@@ -683,9 +683,22 @@ static void sco_conn_defer_accept(struct hci_conn *conn, int mask)
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 (mode) {
+ case SCO_MODE_CVSD:
+ cp.max_latency = __constant_cpu_to_le16(0xffff);
+ cp.content_format = cpu_to_le16(hdev->voice_setting);
+ cp.retrans_effort = 0xff;
+ break;
+ case SCO_MODE_TRANSPARENT:
+ 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(0x0003);
+ cp.retrans_effort = 0x02;
+ break;
+ }
hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
sizeof(cp), &cp);
@@ -702,7 +715,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, 0, pi->mode);
sk->sk_state = BT_CONFIG;
release_sock(sk);
--
1.7.9.5
This patch extends the current SCO socket option to add a 'mode' field. This
field is intended to choose data type at runtime. Current modes are CVSD and
transparent SCO, but adding new modes could allow support for CSA2 and fine
tuning a sco connection, for example latency, bandwith, voice setting. Incoming
connections will be setup during defered setup. Outgoing connections have to
be setup before connect(). The selected type is stored in the sco socket info.
This patch declares needed members, modifies getsockopt() and implements
setsockopt(). Setting the mtu is not supported.
Signed-off-by: Frédéric Dalleau <[email protected]>
---
include/net/bluetooth/sco.h | 7 +++++
net/bluetooth/sco.c | 59 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 1e35c43..4de67ef 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -41,8 +41,14 @@ struct sockaddr_sco {
/* SCO socket options */
#define SCO_OPTIONS 0x01
+
+#define SCO_MODE_CVSD 0x00
+#define SCO_MODE_TRANSPARENT 0x01
+#define SCO_MODE_MAX 0x01
+
struct sco_options {
__u16 mtu;
+ __u8 mode;
};
#define SCO_CONNINFO 0x02
@@ -73,6 +79,7 @@ struct sco_conn {
struct sco_pinfo {
struct bt_sock bt;
__u32 flags;
+ __u8 mode;
struct sco_conn *conn;
};
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 97177be6..7d57af5 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -418,6 +418,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)->mode = SCO_MODE_CVSD;
+
setup_timer(&sk->sk_timer, sco_sock_timeout, (unsigned long)sk);
bt_sock_link(&sco_sk_list, sk);
@@ -712,6 +714,59 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
return bt_sock_recvmsg(iocb, sock, msg, len, flags);
}
+static int sco_sock_setsockopt_old(struct socket *sock, int optname,
+ char __user *optval, unsigned int optlen)
+{
+ struct sock *sk = sock->sk;
+ struct sco_options opts;
+ int len, err = 0;
+
+ BT_DBG("sk %p", sk);
+
+ lock_sock(sk);
+
+ switch (optname) {
+ case SCO_OPTIONS:
+ if (sk->sk_state != BT_OPEN &&
+ sk->sk_state != BT_BOUND &&
+ sk->sk_state != BT_CONNECT2) {
+ err = -EINVAL;
+ break;
+ }
+
+ opts.mtu = 0;
+ opts.mode = SCO_MODE_CVSD;
+
+ len = min_t(unsigned int, sizeof(opts), optlen);
+ if (copy_from_user((char *) &opts, optval, len)) {
+ err = -EFAULT;
+ break;
+ }
+
+ if (opts.mtu != 0) {
+ err = -EINVAL;
+ break;
+ }
+
+ BT_DBG("mode %d", opts.mode);
+
+ if (opts.mode > SCO_MODE_MAX) {
+ err = -EINVAL;
+ break;
+ }
+
+ sco_pi(sk)->mode = opts.mode;
+ break;
+
+ default:
+ err = -ENOPROTOOPT;
+ break;
+ }
+
+ release_sock(sk);
+ return err;
+}
+
static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
{
struct sock *sk = sock->sk;
@@ -720,6 +775,9 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, char
BT_DBG("sk %p", sk);
+ if (level == SOL_SCO)
+ return sco_sock_setsockopt_old(sock, optname, optval, optlen);
+
lock_sock(sk);
switch (optname) {
@@ -772,6 +830,7 @@ static int sco_sock_getsockopt_old(struct socket *sock, int optname, char __user
}
opts.mtu = sco_pi(sk)->conn->mtu;
+ opts.mode = sco_pi(sk)->mode;
BT_DBG("mtu %d", opts.mtu);
--
1.7.9.5
Marcel,
On Wed, Mar 13, 2013 at 5:26 PM, Marcel Holtmann <[email protected]> wrot=
e:
> maybe it is not worth supporting 8-bit SCO, but it is a feature that we s=
hould not ignore. And in case you want to do multiple SCO connections (some=
people want to actually), one way of getting less bandwidth usage is to sw=
itch to 8-bit PCM. So my current thinking is that we should support it.
There is still a problem : how would you enable specific synchronous
connection settings (max bandwith, latency, ...) ? And which one would
you choose.
Another way to reduce bandwith (at the price of some latency and some
computation) is to use mSBC, with less bandwith and a better quality
(msbc encode 240 bytes to 60).
I you really want 8 bit sco, then a suggestion in line with my patches
could be to have a dedicated mode: SCO_MODE_PCM8.
> Right now I am for working in the context of what we actually have. The C=
SA2 is not deployed at this moment and it is rather complex. So my thinking=
is to do voice setting right now and sort out CSA2 at some point later. Un=
less someone can come up with a good way of integrating CSA2 right now and =
allow a smooth backward compatibility to non CSA2 devices.
I agree that CSA2 is not top priority, but the "mode" options is
designed to make such a transition possible.
Fred
Hi Fred,
>> So I would actually propose to add a voice settings uint16 value as SCO
>> socket field. Using the full voice settings make sense since then we can
>> also retrieve the fact if we are doing 8-bit or 16-bit PCM. One detail
>> we kinda ignored since we always defaulted to 16-bit, but would be worth
>> while fixing while at it.
>
> I'm unconvinced about this. This mean the application wil be able to mess into
> what voice settings will be supported. And if csa2 has to be supported
> in the future,
> a new socket option will be required.
> Is 8-bit sco support really worth it ?
maybe it is not worth supporting 8-bit SCO, but it is a feature that we should not ignore. And in case you want to do multiple SCO connections (some people want to actually), one way of getting less bandwidth usage is to switch to 8-bit PCM. So my current thinking is that we should support it.
Right now I am for working in the context of what we actually have. The CSA2 is not deployed at this moment and it is rather complex. So my thinking is to do voice setting right now and sort out CSA2 at some point later. Unless someone can come up with a good way of integrating CSA2 right now and allow a smooth backward compatibility to non CSA2 devices.
That all said, we can always have a socket option later on that enables CSA2 features.
Regards
Marcel
Marcel,
> So I would actually propose to add a voice settings uint16 value as SCO
> socket field. Using the full voice settings make sense since then we can
> also retrieve the fact if we are doing 8-bit or 16-bit PCM. One detail
> we kinda ignored since we always defaulted to 16-bit, but would be worth
> while fixing while at it.
I'm unconvinced about this. This mean the application wil be able to mess into
what voice settings will be supported. And if csa2 has to be supported
in the future,
a new socket option will be required.
Is 8-bit sco support really worth it ?
Regards,
Fred