Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH BlueZ 1/2] TODO: Add entry for runtime selection of drivers
From: Johan Hedberg @ 2012-11-13 17:42 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1352821786-20230-1-git-send-email-anderson.lizardo@openbossa.org>

Hi Lizardo,

On Tue, Nov 13, 2012, Anderson Lizardo wrote:
> This will allow to drop usage of symlinks to C files, which require
> messing with CPPFLAGS so headers can be found by the preprocessor.
> 
> It also allows for easier testing, because it is not necessary to
> rebuild BlueZ to test different drivers.
> ---
>  TODO |    9 +++++++++
>  1 file changed, 9 insertions(+)

Both patches have been applied. Thanks.

Johan

^ permalink raw reply

* [RFC] sco: BT_DEFER_SETUP for SCO sockets
From: Frédéric Dalleau @ 2012-11-13 18:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau

Hi,

This is a first draft about what can be done to implement DEFER_SETUP on SCO
sockets.  I couldn't test it today I just pulled bluetooth-next and build
failed, so I plan to go further tomorrow.

hci layer get some changes since previous behavior was to accept all SCO
connections.

Some questions are open :
Should we be able to defer non eSCO capable devices? This would remove a few
lines of code.
hci_proto_defered is ugly: What about a flag similar to LM_ACCEPT called
LM_DEFER that would indicate that accepting is defered.

Let me know what you think.

Best regards,
Frédéric


Frédéric Dalleau (1):
  Bluetooth: Add BT_DEFER_SETUP option to sco socket

 include/net/bluetooth/hci_core.h |   15 +++++++
 net/bluetooth/hci_event.c        |   47 +++++++++++++++++++-
 net/bluetooth/sco.c              |   88 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 145 insertions(+), 5 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Frédéric Dalleau @ 2012-11-13 18:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1352830825-13987-1-git-send-email-frederic.dalleau@linux.intel.com>

In order to authenticate and later configure an incoming SCO connection, the
BT_DEFER_SETUP option is added.
When an connection is requested, the listening socket is unblocked but the
effective connection setup happens only on first recv.
Any send between accept and recv fails with -ENOTCONN.
---
 include/net/bluetooth/hci_core.h |   15 +++++++
 net/bluetooth/hci_event.c        |   47 +++++++++++++++++++-
 net/bluetooth/sco.c              |   88 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 145 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ef5b85d..2ee0ecb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -377,6 +377,7 @@ extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
 			      u16 flags);
 
 extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
+extern int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr);
 extern void sco_connect_cfm(struct hci_conn *hcon, __u8 status);
 extern void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason);
 extern int sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb);
@@ -577,6 +578,7 @@ 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);
@@ -796,6 +798,19 @@ static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
 	}
 }
 
+static inline int hci_proto_defered(struct hci_dev *hdev, bdaddr_t *bdaddr,
+								__u8 type)
+{
+	switch (type) {
+	case SCO_LINK:
+	case ESCO_LINK:
+		return sco_defer(hdev, bdaddr);
+
+	default:
+		return 0;
+	}
+}
+
 static inline void hci_proto_connect_cfm(struct hci_conn *conn, __u8 status)
 {
 	switch (conn->type) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9f5c5f2..167eca0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2047,15 +2047,54 @@ 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);
+
+	if (!lmp_esco_capable(hdev)) {
+		struct hci_cp_accept_conn_req cp;
+
+		conn->state = BT_CONNECT;
+		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;
+
+		conn->state = BT_CONNECT;
+		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;
 	int mask = hdev->link_mode;
+	int defered;
 
 	BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
 	       ev->link_type);
 
 	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
+	defered = hci_proto_defered(hdev, &ev->bdaddr, ev->link_type);
 
 	if ((mask & HCI_LM_ACCEPT) &&
 	    !hci_blacklist_lookup(hdev, &ev->bdaddr)) {
@@ -2085,7 +2124,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 		hci_dev_unlock(hdev);
 
-		if (ev->link_type == ACL_LINK || !lmp_esco_capable(hdev)) {
+		if (ev->link_type == ACL_LINK ||
+				(!defered && !lmp_esco_capable(hdev))) {
 			struct hci_cp_accept_conn_req cp;
 
 			bacpy(&cp.bdaddr, &ev->bdaddr);
@@ -2097,7 +2137,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 			hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
 				     &cp);
-		} else {
+		} else if (!defered) {
 			struct hci_cp_accept_sync_conn_req cp;
 
 			bacpy(&cp.bdaddr, &ev->bdaddr);
@@ -2111,6 +2151,9 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 			hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
 				     sizeof(cp), &cp);
+		} else {
+			hci_proto_connect_cfm(conn, 0);
+			hci_conn_put(conn);
 		}
 	} else {
 		/* Connection rejected */
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 450cdcd..416801a 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -662,16 +662,57 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	return err;
 }
 
+static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
+			      struct msghdr *msg, size_t len, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct sco_pinfo *pi = sco_pi(sk);
+
+	lock_sock(sk);
+
+	if (sk->sk_state == BT_CONNECT &&
+			test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
+		hci_conn_accept(pi->conn->hcon, 0);
+		sk->sk_state = BT_CONNECT2;
+
+		release_sock(sk);
+		return 0;
+	}
+
+	release_sock(sk);
+
+	return bt_sock_recvmsg(iocb, sock, msg, len, flags);
+}
+
 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;
+	u32 opt;
 
 	BT_DBG("sk %p", sk);
 
 	lock_sock(sk);
 
 	switch (optname) {
+
+	case BT_DEFER_SETUP:
+		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (get_user(opt, (u32 __user *) optval)) {
+			err = -EFAULT;
+			break;
+		}
+
+		if (opt)
+			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
+		else
+			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -753,6 +794,19 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
 	lock_sock(sk);
 
 	switch (optname) {
+
+	case BT_DEFER_SETUP:
+		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags),
+			     (u32 __user *) optval))
+			err = -EFAULT;
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -874,7 +928,10 @@ static void sco_conn_ready(struct sco_conn *conn)
 		hci_conn_hold(conn->hcon);
 		__sco_chan_add(conn, sk, parent);
 
-		sk->sk_state = BT_CONNECTED;
+		if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags))
+			sk->sk_state = BT_CONNECT;
+		else
+			sk->sk_state = BT_CONNECTED;
 
 		/* Wake up parent */
 		parent->sk_data_ready(parent, 1);
@@ -912,6 +969,32 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
 	return lm;
 }
 
+int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr)
+{
+	struct sock *sk;
+	struct hlist_node *node;
+	int defer = 0;
+
+	BT_DBG("hdev %s, bdaddr %pMR", hdev->name, bdaddr);
+
+	/* Find listening sockets */
+	read_lock(&sco_sk_list.lock);
+	sk_for_each(sk, node, &sco_sk_list.head) {
+		if (sk->sk_state != BT_LISTEN)
+			continue;
+
+		if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr) ||
+		    !bacmp(&bt_sk(sk)->src, BDADDR_ANY)) {
+			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
+				defer = 1;
+			break;
+		}
+	}
+	read_unlock(&sco_sk_list.lock);
+
+	return defer;
+}
+
 void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
 {
 	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
@@ -992,7 +1075,7 @@ static const struct proto_ops sco_sock_ops = {
 	.accept		= sco_sock_accept,
 	.getname	= sco_sock_getname,
 	.sendmsg	= sco_sock_sendmsg,
-	.recvmsg	= bt_sock_recvmsg,
+	.recvmsg	= sco_sock_recvmsg,
 	.poll		= bt_sock_poll,
 	.ioctl		= bt_sock_ioctl,
 	.mmap		= sock_no_mmap,
@@ -1036,7 +1119,6 @@ int __init sco_init(void)
 			BT_ERR("Failed to create SCO debug file");
 	}
 
-	BT_INFO("SCO socket layer initialized");
 
 	return 0;
 
-- 
1.7.9.5


^ permalink raw reply related

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Luiz Augusto von Dentz @ 2012-11-13 18:45 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1352830825-13987-2-git-send-email-frederic.dalleau@linux.intel.com>

Hi Frédéric,

On Tue, Nov 13, 2012 at 8:20 PM, Frédéric Dalleau
<frederic.dalleau@linux.intel.com> wrote:
> In order to authenticate and later configure an incoming SCO connection, the
> BT_DEFER_SETUP option is added.
> When an connection is requested, the listening socket is unblocked but the
> effective connection setup happens only on first recv.
> Any send between accept and recv fails with -ENOTCONN.

I think you should detail when exactly we do defer, we need to defer
before the audio settings negotiation takes place because latter once
we add support for passing the settings this happens during this stage
before accepting.

> ---
>  include/net/bluetooth/hci_core.h |   15 +++++++
>  net/bluetooth/hci_event.c        |   47 +++++++++++++++++++-
>  net/bluetooth/sco.c              |   88 ++++++++++++++++++++++++++++++++++++--
>  3 files changed, 145 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ef5b85d..2ee0ecb 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -377,6 +377,7 @@ extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
>                               u16 flags);
>
>  extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
> +extern int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr);
>  extern void sco_connect_cfm(struct hci_conn *hcon, __u8 status);
>  extern void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason);
>  extern int sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb);
> @@ -577,6 +578,7 @@ 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);
> @@ -796,6 +798,19 @@ static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
>         }
>  }
>
> +static inline int hci_proto_defered(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +                                                               __u8 type)
> +{
> +       switch (type) {
> +       case SCO_LINK:
> +       case ESCO_LINK:
> +               return sco_defer(hdev, bdaddr);
> +
> +       default:
> +               return 0;
> +       }
> +}
> +
>  static inline void hci_proto_connect_cfm(struct hci_conn *conn, __u8 status)
>  {
>         switch (conn->type) {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 9f5c5f2..167eca0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2047,15 +2047,54 @@ 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);
> +
> +       if (!lmp_esco_capable(hdev)) {
> +               struct hci_cp_accept_conn_req cp;
> +
> +               conn->state = BT_CONNECT;
> +               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;
> +
> +               conn->state = BT_CONNECT;
> +               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;
>         int mask = hdev->link_mode;
> +       int defered;
>
>         BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
>                ev->link_type);
>
>         mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
> +       defered = hci_proto_defered(hdev, &ev->bdaddr, ev->link_type);
>
>         if ((mask & HCI_LM_ACCEPT) &&
>             !hci_blacklist_lookup(hdev, &ev->bdaddr)) {
> @@ -2085,7 +2124,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>
>                 hci_dev_unlock(hdev);
>
> -               if (ev->link_type == ACL_LINK || !lmp_esco_capable(hdev)) {
> +               if (ev->link_type == ACL_LINK ||
> +                               (!defered && !lmp_esco_capable(hdev))) {
>                         struct hci_cp_accept_conn_req cp;
>
>                         bacpy(&cp.bdaddr, &ev->bdaddr);
> @@ -2097,7 +2137,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>
>                         hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
>                                      &cp);
> -               } else {
> +               } else if (!defered) {
>                         struct hci_cp_accept_sync_conn_req cp;
>
>                         bacpy(&cp.bdaddr, &ev->bdaddr);
> @@ -2111,6 +2151,9 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>
>                         hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
>                                      sizeof(cp), &cp);
> +               } else {
> +                       hci_proto_connect_cfm(conn, 0);
> +                       hci_conn_put(conn);
>                 }
>         } else {
>                 /* Connection rejected */
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 450cdcd..416801a 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -662,16 +662,57 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>         return err;
>  }
>
> +static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> +                             struct msghdr *msg, size_t len, int flags)
> +{
> +       struct sock *sk = sock->sk;
> +       struct sco_pinfo *pi = sco_pi(sk);
> +
> +       lock_sock(sk);
> +
> +       if (sk->sk_state == BT_CONNECT &&
> +                       test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> +               hci_conn_accept(pi->conn->hcon, 0);
> +               sk->sk_state = BT_CONNECT2;
> +
> +               release_sock(sk);
> +               return 0;
> +       }
> +
> +       release_sock(sk);
> +
> +       return bt_sock_recvmsg(iocb, sock, msg, len, flags);
> +}
> +
>  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;
> +       u32 opt;
>
>         BT_DBG("sk %p", sk);
>
>         lock_sock(sk);
>
>         switch (optname) {
> +
> +       case BT_DEFER_SETUP:
> +               if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +                       err = -EINVAL;
> +                       break;
> +               }
> +
> +               if (get_user(opt, (u32 __user *) optval)) {
> +                       err = -EFAULT;
> +                       break;
> +               }
> +
> +               if (opt)
> +                       set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +               else
> +                       clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +               break;
> +
>         default:
>                 err = -ENOPROTOOPT;
>                 break;
> @@ -753,6 +794,19 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
>         lock_sock(sk);
>
>         switch (optname) {
> +
> +       case BT_DEFER_SETUP:
> +               if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +                       err = -EINVAL;
> +                       break;
> +               }
> +
> +               if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags),
> +                            (u32 __user *) optval))
> +                       err = -EFAULT;
> +
> +               break;
> +
>         default:
>                 err = -ENOPROTOOPT;
>                 break;
> @@ -874,7 +928,10 @@ static void sco_conn_ready(struct sco_conn *conn)
>                 hci_conn_hold(conn->hcon);
>                 __sco_chan_add(conn, sk, parent);
>
> -               sk->sk_state = BT_CONNECTED;
> +               if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags))
> +                       sk->sk_state = BT_CONNECT;
> +               else
> +                       sk->sk_state = BT_CONNECTED;
>
>                 /* Wake up parent */
>                 parent->sk_data_ready(parent, 1);
> @@ -912,6 +969,32 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
>         return lm;
>  }
>
> +int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr)
> +{
> +       struct sock *sk;
> +       struct hlist_node *node;
> +       int defer = 0;
> +
> +       BT_DBG("hdev %s, bdaddr %pMR", hdev->name, bdaddr);
> +
> +       /* Find listening sockets */
> +       read_lock(&sco_sk_list.lock);
> +       sk_for_each(sk, node, &sco_sk_list.head) {
> +               if (sk->sk_state != BT_LISTEN)
> +                       continue;
> +
> +               if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr) ||
> +                   !bacmp(&bt_sk(sk)->src, BDADDR_ANY)) {
> +                       if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
> +                               defer = 1;
> +                       break;
> +               }
> +       }
> +       read_unlock(&sco_sk_list.lock);
> +
> +       return defer;
> +}
> +
>  void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
>  {
>         BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
> @@ -992,7 +1075,7 @@ static const struct proto_ops sco_sock_ops = {
>         .accept         = sco_sock_accept,
>         .getname        = sco_sock_getname,
>         .sendmsg        = sco_sock_sendmsg,
> -       .recvmsg        = bt_sock_recvmsg,
> +       .recvmsg        = sco_sock_recvmsg,
>         .poll           = bt_sock_poll,
>         .ioctl          = bt_sock_ioctl,
>         .mmap           = sock_no_mmap,
> @@ -1036,7 +1119,6 @@ int __init sco_init(void)
>                         BT_ERR("Failed to create SCO debug file");
>         }
>
> -       BT_INFO("SCO socket layer initialized");

There doesn't seems to be any need to remove the line above, could you
please fix that.

-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt
From: Marcel Holtmann @ 2012-11-13 22:49 UTC (permalink / raw)
  To: Mat Martineau; +Cc: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <alpine.DEB.2.02.1211130911200.27104@mathewm-linux>

Hi Mat,

> > ...
> >>>>> +void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> >>>>> +{
> >>>>> +	struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >>>>> +	struct amp_mgr *mgr = hs_hcon->amp_mgr;
> >>>>> +	struct l2cap_chan *bredr_chan;
> >>>>> +
> >>>>> +	BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> >>>>> +
> >>>>> +	if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> >>>>> +		return;
> >>>>> +
> >>>>> +	bredr_chan = mgr->bredr_chan;
> >>>>> +
> >>>>> +	set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> >>>>> +	bredr_chan->ctrl_id = hs_hcon->remote_id;
> >>>>> +	bredr_chan->hs_hcon = hs_hcon;
> >>>>> +	bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> >>>>> +	bredr_chan->fcs = L2CAP_FCS_NONE;
> >>
> >> Changing FCS requires L2CAP reconfiguration for the channel, and
> >> chan->fcs shouldn't be modified until reconfiguration happens.
> >> While it doesn't make much sense to do so, the remote device may
> >> want to keep FCS enabled.  The move may also fail and you don't want
> >> to forget the original FCS setting in that case.
> >
> > So we agree that FCS shall not be used for High Speed channels. I was
> > thinking more about the case where we start sending data right over HS
> > channel. The configuration should be just started.
> 
> No matter what the BlueZ implementation prefers, it must be able to 
> handle a remote device that requests FCS during L2CAP config.  If one 
> side requests FCS, then it must be used.
> 
> Do we want BlueZ to always ignore the L2CAP FCS socket option on AMP 
> controllers?  (Marcel?)  This checksum is always redundant with 802.11 
> AMP controllers.

if it is there, we should check it. Even on 802.11. Mainly just in case
someone tries to sneak packets in. But if by any means possible we
should try to disable FCS on AMP controllers.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH v2 11/20] cyclingspeed: Add stub to use SC Control Point
From: Lucas De Marchi @ 2012-11-14  0:31 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1352105705-13988-12-git-send-email-andrzej.kaczmarek@tieto.com>

On Mon, Nov 5, 2012 at 6:54 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> This patch implements common functions to use SC Control Point.
> Individual procedures will be implemented in subsequent patches.
> ---
>  profiles/cyclingspeed/cyclingspeed.c | 145 +++++++++++++++++++++++++++++++++--
>  1 file changed, 140 insertions(+), 5 deletions(-)
>
> diff --git a/profiles/cyclingspeed/cyclingspeed.c b/profiles/cyclingspeed/cyclingspeed.c
> index 5a65507..ffef6ae 100644
> --- a/profiles/cyclingspeed/cyclingspeed.c
> +++ b/profiles/cyclingspeed/cyclingspeed.c
> @@ -41,6 +41,11 @@
>  #include "log.h"
>  #include "cyclingspeed.h"
>
> +/* min length for ATT indication or notification: opcode (1b) + handle (2b) */
> +#define ATT_HDR_LEN 3
> +
> +#define ATT_TIMEOUT 30
> +
>  #define CYCLINGSPEED_INTERFACE         "org.bluez.CyclingSpeed"
>  #define CYCLINGSPEED_MANAGER_INTERFACE "org.bluez.CyclingSpeedManager"
>  #define CYCLINGSPEED_WATCHER_INTERFACE "org.bluez.CyclingSpeedWatcher"
> @@ -52,6 +57,25 @@
>  #define WHEEL_REV_PRESENT      0x01
>  #define CRANK_REV_PRESENT      0x02
>
> +#define SET_CUMULATIVE_VALUE           0x01
> +#define START_SENSOR_CALIBRATION       0x02
> +#define UPDATE_SENSOR_LOC              0x03
> +#define REQUEST_SUPPORTED_SENSOR_LOC   0x04
> +#define RESPONSE_CODE                  0x10
> +
> +#define RSP_SUCCESS            0x01
> +#define RSP_NOT_SUPPORTED      0x02
> +#define RSP_INVALID_PARAM      0x03
> +#define RSP_FAILED             0x04
> +
> +struct csc;
> +
> +struct controlpoint_req {
> +       struct csc              *csc;
> +       uint8_t                 opcode;
> +       guint                   timeout;
> +};
> +
>  struct csc_adapter {
>         struct btd_adapter      *adapter;
>         GSList                  *devices;       /* list of registered devices */
> @@ -66,6 +90,8 @@ struct csc {
>         guint                   attioid;
>         /* attio id for measurement characteristics value notifications */
>         guint                   attio_measurement_id;
> +       /* attio id for SC Control Point characteristics value indications */
> +       guint                   attio_controlpoint_id;
>
>         struct att_range        *svc_range;
>
> @@ -75,6 +101,8 @@ struct csc {
>         uint16_t                feature;
>         gboolean                has_location;
>         uint8_t                 location;
> +
> +       struct controlpoint_req *pending_req;
>  };
>
>  struct watcher {
> @@ -216,6 +244,7 @@ static void destroy_csc(gpointer user_data)
>
>         if (csc->attrib != NULL) {
>                 g_attrib_unregister(csc->attrib, csc->attio_measurement_id);
> +               g_attrib_unregister(csc->attrib, csc->attio_controlpoint_id);
>                 g_attrib_unref(csc->attrib);
>         }
>
> @@ -235,6 +264,35 @@ static void char_write_cb(guint8 status, const guint8 *pdu, guint16 len,
>         g_free(msg);
>  }
>
> +static gboolean controlpoint_timeout(gpointer user_data)
> +{
> +       struct controlpoint_req *req = user_data;
> +
> +       req->csc->pending_req = NULL;
> +       g_free(req);
> +
> +       return FALSE;
> +}
> +
> +__attribute__((unused)) /* TODO: remove once controlpoint ops are implemented */

This is a weird way to split the patches... but I don't care that much
for a bootstrap

Lucas De Marchi

^ permalink raw reply

* Re: [PATCH v2 00/20] CSCP plugin
From: Lucas De Marchi @ 2012-11-14  0:35 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1352105705-13988-1-git-send-email-andrzej.kaczmarek@tieto.com>

On Mon, Nov 5, 2012 at 6:54 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> Hi,
>
> Here are v2 patches for CSC profile implementation (cyclingspeed plugin).
>
> Changes since v1:
> - signal is emited when Location property is changed
> - supported locations CP procedure is done only once (see notes below)
> - test script can now calculate instantenous speed and cadence
> - few minor fixes found during testing and review
>
> It's now also tested with PTS - can pass all CSCP testcases but there are
> few things to remember if someone want to retest:
> - testcases in 4.5.3 are broken, look into erratas for updated ets file
> - TP/SPL/CO/BV-01-I shall be run after device is removed because Request
>   Supported Sensor Location procedure is done only once and then result
>   is cached - this is because running it every time device is connected
>   will make other testcases using SC Control Point fail due to unexpected
>   opcode written (and CSCP spec requires this value to be cached anyway)
> - as for now watcher shall be registered before running testcases using
>   SC Control Point to make them work - this is because PTS expects that
>   measurement notifications are enabled before control point indications,
>   otherwise it will wait for writing control point CCC even though it
>   was written before. Errata for this is already accepted and TS will be
>   changed so both CCC can be written in any order.
> - TP/SPS/CO/BV-02-I seems to be broken as it expected different value to
>   be written than what is displayed to tester - issue in PTS for this is
>   in progress
>
>
> Comments are welcome.

I reviewed mainly the gdbus calls, particularly the DBus.Properties
ones. It looks good for me.


Lucas De Marchi

^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Michael Knudsen @ 2012-11-14  8:26 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1352830825-13987-2-git-send-email-frederic.dalleau@linux.intel.com>

On 2012-11-13 19:20, Frédéric Dalleau wrote:
> In order to authenticate and later configure an incoming SCO connection, the
> BT_DEFER_SETUP option is added.
> When an connection is requested, the listening socket is unblocked but the
> effective connection setup happens only on first recv.
> Any send between accept and recv fails with -ENOTCONN.

I have a few comments, please see below.

> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ef5b85d..2ee0ecb 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -377,6 +377,7 @@ extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
>  			      u16 flags);
>  
>  extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
> +extern int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr);

I would prefer to make sco_defer() static and call it from sco_connect_ind()
instead of as a separate step.  More on this below.

> +static inline int hci_proto_defered(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +								__u8 type)
> +{
> +	switch (type) {
> +	case SCO_LINK:
> +	case ESCO_LINK:
> +		return sco_defer(hdev, bdaddr);
> +
> +	default:
> +		return 0;
> +	}
> +}

My idea is to define (I'm not sure if this is exactly the same as what you
suggested yourself) a bit, LM_DEFER, that e.g. sco_connect_ind() may return
and propagate this bit to hci_conn_request_evt().  This way all policing of
accepting SCO connections is handled from a single entry point into sco.c.

>  static inline void hci_proto_connect_cfm(struct hci_conn *conn, __u8 status)
>  {
>  	switch (conn->type) {

>  static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>  	struct hci_ev_conn_request *ev = (void *) skb->data;
>  	int mask = hdev->link_mode;
> +	int defered;
>  
>  	BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
>  	       ev->link_type);
>  
>  	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
> +	defered = hci_proto_defered(hdev, &ev->bdaddr, ev->link_type);

Given that these two take exactly the same parameters, I would much
rather see hci_proto_defered() folded into hci_proto_connect_ind()
and set (LM_ACCEPT | LM_DEFERRED) for deferred cases.  This would
also make sure we only do one iteration over the SCO socket list and
that the same SCO sockets are taken into account.  With your approach
there is a risk that one of the loops is changed and not the other.

-m.

^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Andrei Emeltchenko @ 2012-11-14  9:17 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1352830825-13987-2-git-send-email-frederic.dalleau@linux.intel.com>

Hi Frédéric,

On Tue, Nov 13, 2012 at 07:20:25PM +0100, Frédéric Dalleau wrote:
> In order to authenticate and later configure an incoming SCO connection, the
> BT_DEFER_SETUP option is added.
> When an connection is requested, the listening socket is unblocked but the
> effective connection setup happens only on first recv.
> Any send between accept and recv fails with -ENOTCONN.
> ---

Apart from other comments..

> +static inline int hci_proto_defered(struct hci_dev *hdev, bdaddr_t *bdaddr,

I think we do not use "inline" anymore.

> +								__u8 type)
> +{
> +	switch (type) {
> +	case SCO_LINK:
> +	case ESCO_LINK:
> +		return sco_defer(hdev, bdaddr);
> +
> +	default:
> +		return 0;
> +	}
> +}
> +
>  static inline void hci_proto_connect_cfm(struct hci_conn *conn, __u8 status)
>  {
>  	switch (conn->type) {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 9f5c5f2..167eca0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2047,15 +2047,54 @@ 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);
> +
> +	if (!lmp_esco_capable(hdev)) {
> +		struct hci_cp_accept_conn_req cp;
> +
> +		conn->state = BT_CONNECT;
> +		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;
> +
> +		conn->state = BT_CONNECT;
> +		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);

maybe some comments about those numbers

> +		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_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
>  {
>  	struct sock *sk = sock->sk;
>  	int err = 0;
> +	u32 opt;
>  
>  	BT_DBG("sk %p", sk);
>  
>  	lock_sock(sk);
>  
>  	switch (optname) {
> +
> +	case BT_DEFER_SETUP:
> +		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (get_user(opt, (u32 __user *) optval)) {
> +			err = -EFAULT;
> +			break;
> +		}
> +
> +		if (opt)
> +			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +		else
> +			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +		break;
> +
>  	default:
>  		err = -ENOPROTOOPT;
>  		break;
> @@ -753,6 +794,19 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
>  	lock_sock(sk);
>  
>  	switch (optname) {
> +
> +	case BT_DEFER_SETUP:
> +		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags),
> +			     (u32 __user *) optval))
> +			err = -EFAULT;
> +
> +		break;
> +

I think set/get sockopt might be implemented in a separate patch.
Maybe you can also break even into more logical chunks so this would be
easier to understand.

Best regards 
Andrei Emeltchenko 


^ permalink raw reply

* Re: [PATCH v4 00/16] mSBC tests
From: Frédéric Dalleau @ 2012-11-14 10:00 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1351589975-22640-1-git-send-email-frederic.dalleau@linux.intel.com>

Hi folks, Siarhei,

On 10/30/2012 10:39 AM, Frédéric Dalleau wrote:
> Hi,
>
> v4 integrate latest comments from Siarhei:
>   - remove state->pending field from sbc_encoder_state
>   - add armv6 and iwmmxt primitives
>   - use simd primitive instead of neon
>   - reorder patches so that the SBC_MSBC flag is exposed to users only when
> implementation is complete.

Any feedback ?

Thanks,
Frédéric

^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Arnaud Mouiche @ 2012-11-14 10:09 UTC (permalink / raw)
  To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1352830825-13987-2-git-send-email-frederic.dalleau@linux.intel.com>

Hello,

First thank you for this RFC. Defer (or something similar) is a must 
have for mSBC vs CVSD connection accept from multiple device management, 
at least for HFP case.
some comments / ideas:
- can't we take the opportunity to configure the whole set of eSCO 
parameters (bandwith / latency / retransmission effort) instead of only 
getting the possibility to change the voice settings
- can we introduce a better way to change the voice settings, instead of 
modifying the adapter config (hdev->voice_setting)
- any possibility to reject the eSCO creating (for 'connection rejected 
due to limited resources' reason)

I will try to find some time to try your patch.

regards,
arnaud



On 11/13/2012 07:20 PM, Frédéric Dalleau wrote:
> In order to authenticate and later configure an incoming SCO connection, the
> BT_DEFER_SETUP option is added.
> When an connection is requested, the listening socket is unblocked but the
> effective connection setup happens only on first recv.
> Any send between accept and recv fails with -ENOTCONN.
> ---
>   include/net/bluetooth/hci_core.h |   15 +++++++
>   net/bluetooth/hci_event.c        |   47 +++++++++++++++++++-
>   net/bluetooth/sco.c              |   88 ++++++++++++++++++++++++++++++++++++--
>   3 files changed, 145 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ef5b85d..2ee0ecb 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -377,6 +377,7 @@ extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
>   			      u16 flags);
>   
>   extern int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
> +extern int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr);
>   extern void sco_connect_cfm(struct hci_conn *hcon, __u8 status);
>   extern void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason);
>   extern int sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb);
> @@ -577,6 +578,7 @@ 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);
> @@ -796,6 +798,19 @@ static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
>   	}
>   }
>   
> +static inline int hci_proto_defered(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +								__u8 type)
> +{
> +	switch (type) {
> +	case SCO_LINK:
> +	case ESCO_LINK:
> +		return sco_defer(hdev, bdaddr);
> +
> +	default:
> +		return 0;
> +	}
> +}
> +
>   static inline void hci_proto_connect_cfm(struct hci_conn *conn, __u8 status)
>   {
>   	switch (conn->type) {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 9f5c5f2..167eca0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2047,15 +2047,54 @@ 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);
> +
> +	if (!lmp_esco_capable(hdev)) {
> +		struct hci_cp_accept_conn_req cp;
> +
> +		conn->state = BT_CONNECT;
> +		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;
> +
> +		conn->state = BT_CONNECT;
> +		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;
>   	int mask = hdev->link_mode;
> +	int defered;
>   
>   	BT_DBG("%s bdaddr %pMR type 0x%x", hdev->name, &ev->bdaddr,
>   	       ev->link_type);
>   
>   	mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
> +	defered = hci_proto_defered(hdev, &ev->bdaddr, ev->link_type);
>   
>   	if ((mask & HCI_LM_ACCEPT) &&
>   	    !hci_blacklist_lookup(hdev, &ev->bdaddr)) {
> @@ -2085,7 +2124,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   
>   		hci_dev_unlock(hdev);
>   
> -		if (ev->link_type == ACL_LINK || !lmp_esco_capable(hdev)) {
> +		if (ev->link_type == ACL_LINK ||
> +				(!defered && !lmp_esco_capable(hdev))) {
>   			struct hci_cp_accept_conn_req cp;
>   
>   			bacpy(&cp.bdaddr, &ev->bdaddr);
> @@ -2097,7 +2137,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   
>   			hci_send_cmd(hdev, HCI_OP_ACCEPT_CONN_REQ, sizeof(cp),
>   				     &cp);
> -		} else {
> +		} else if (!defered) {
>   			struct hci_cp_accept_sync_conn_req cp;
>   
>   			bacpy(&cp.bdaddr, &ev->bdaddr);
> @@ -2111,6 +2151,9 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
>   
>   			hci_send_cmd(hdev, HCI_OP_ACCEPT_SYNC_CONN_REQ,
>   				     sizeof(cp), &cp);
> +		} else {
> +			hci_proto_connect_cfm(conn, 0);
> +			hci_conn_put(conn);
>   		}
>   	} else {
>   		/* Connection rejected */
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 450cdcd..416801a 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -662,16 +662,57 @@ static int sco_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>   	return err;
>   }
>   
> +static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> +			      struct msghdr *msg, size_t len, int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct sco_pinfo *pi = sco_pi(sk);
> +
> +	lock_sock(sk);
> +
> +	if (sk->sk_state == BT_CONNECT &&
> +			test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> +		hci_conn_accept(pi->conn->hcon, 0);
> +		sk->sk_state = BT_CONNECT2;
> +
> +		release_sock(sk);
> +		return 0;
> +	}
> +
> +	release_sock(sk);
> +
> +	return bt_sock_recvmsg(iocb, sock, msg, len, flags);
> +}
> +
>   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;
> +	u32 opt;
>   
>   	BT_DBG("sk %p", sk);
>   
>   	lock_sock(sk);
>   
>   	switch (optname) {
> +
> +	case BT_DEFER_SETUP:
> +		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (get_user(opt, (u32 __user *) optval)) {
> +			err = -EFAULT;
> +			break;
> +		}
> +
> +		if (opt)
> +			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +		else
> +			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +		break;
> +
>   	default:
>   		err = -ENOPROTOOPT;
>   		break;
> @@ -753,6 +794,19 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, char
>   	lock_sock(sk);
>   
>   	switch (optname) {
> +
> +	case BT_DEFER_SETUP:
> +		if (sk->sk_state != BT_BOUND && sk->sk_state != BT_LISTEN) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (put_user(test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags),
> +			     (u32 __user *) optval))
> +			err = -EFAULT;
> +
> +		break;
> +
>   	default:
>   		err = -ENOPROTOOPT;
>   		break;
> @@ -874,7 +928,10 @@ static void sco_conn_ready(struct sco_conn *conn)
>   		hci_conn_hold(conn->hcon);
>   		__sco_chan_add(conn, sk, parent);
>   
> -		sk->sk_state = BT_CONNECTED;
> +		if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags))
> +			sk->sk_state = BT_CONNECT;
> +		else
> +			sk->sk_state = BT_CONNECTED;
>   
>   		/* Wake up parent */
>   		parent->sk_data_ready(parent, 1);
> @@ -912,6 +969,32 @@ int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
>   	return lm;
>   }
>   
> +int sco_defer(struct hci_dev *hdev, bdaddr_t *bdaddr)
> +{
> +	struct sock *sk;
> +	struct hlist_node *node;
> +	int defer = 0;
> +
> +	BT_DBG("hdev %s, bdaddr %pMR", hdev->name, bdaddr);
> +
> +	/* Find listening sockets */
> +	read_lock(&sco_sk_list.lock);
> +	sk_for_each(sk, node, &sco_sk_list.head) {
> +		if (sk->sk_state != BT_LISTEN)
> +			continue;
> +
> +		if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr) ||
> +		    !bacmp(&bt_sk(sk)->src, BDADDR_ANY)) {
> +			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
> +				defer = 1;
> +			break;
> +		}
> +	}
> +	read_unlock(&sco_sk_list.lock);
> +
> +	return defer;
> +}
> +
>   void sco_connect_cfm(struct hci_conn *hcon, __u8 status)
>   {
>   	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
> @@ -992,7 +1075,7 @@ static const struct proto_ops sco_sock_ops = {
>   	.accept		= sco_sock_accept,
>   	.getname	= sco_sock_getname,
>   	.sendmsg	= sco_sock_sendmsg,
> -	.recvmsg	= bt_sock_recvmsg,
> +	.recvmsg	= sco_sock_recvmsg,
>   	.poll		= bt_sock_poll,
>   	.ioctl		= bt_sock_ioctl,
>   	.mmap		= sock_no_mmap,
> @@ -1036,7 +1119,6 @@ int __init sco_init(void)
>   			BT_ERR("Failed to create SCO debug file");
>   	}
>   
> -	BT_INFO("SCO socket layer initialized");
>   
>   	return 0;
>   


^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Frédéric Dalleau @ 2012-11-14 10:10 UTC (permalink / raw)
  To: Michael Knudsen; +Cc: linux-bluetooth
In-Reply-To: <50A355CF.4030809@samsung.com>

Hi Michael,

On 11/14/2012 09:26 AM, Michael Knudsen wrote:

> My idea is to define (I'm not sure if this is exactly the same as what you
> suggested yourself) a bit, LM_DEFER, that e.g. sco_connect_ind() may return
> and propagate this bit to hci_conn_request_evt().  This way all policing of
> accepting SCO connections is handled from a single entry point into sco.c.

You detailed it much further than I did, but yes it is the same idea. I 
fully support it! If everybody agrees, then I can update the patch.

hci.h would receive the folowing line:

+ #define HCI_LM_DEFER   0x4000

Best regards,
Frédéric

^ permalink raw reply

* [PATCH 1/4] adapter: Convert storage aliases
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth

---
 src/adapter.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index f9eb3af..ea7b8ff 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -86,6 +86,7 @@
 
 #define SETTINGS_PATH STORAGEDIR "/%s/settings"
 #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
+#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"
 
 static GSList *adapter_drivers = NULL;
 
@@ -2533,6 +2534,68 @@ static void convert_names_entry(char *key, char *value, void *user_data)
 	store_cached_name(&local, &peer, value);
 }
 
+struct device_converter {
+	char *address;
+	void (*cb)(GKeyFile *key_file, void *value);
+};
+
+static void convert_aliases_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_string(key_file, "General", "Alias", value);
+}
+
+static void convert_entry(char *key, char *value, void *user_data)
+{
+	struct device_converter *converter = user_data;
+	char filename[PATH_MAX + 1];
+	GKeyFile *key_file;
+	char *data;
+	gsize length = 0;
+
+	if (key[17] == '#')
+		key[17] = '\0';
+
+	if (bachk(key) != 0)
+		return;
+
+	snprintf(filename, PATH_MAX, DEVICE_INFO_PATH, converter->address, key);
+	filename[PATH_MAX] = '\0';
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	converter->cb(key_file, value);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(filename, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+}
+
+static void convert_file(char *file, char *address,
+				void (*cb)(GKeyFile *key_file, void *value))
+{
+	char filename[PATH_MAX + 1];
+	struct device_converter converter;
+	char *str;
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s", address, file);
+	filename[PATH_MAX] = '\0';
+
+	str = textfile_get(filename, "converted");
+	if (str && strcmp(str, "yes") == 0) {
+		DBG("Legacy file %s already converted", filename);
+	} else {
+		converter.address = address;
+		converter.cb = cb;
+
+		textfile_foreach(filename, convert_entry, &converter);
+		textfile_put(filename, "converted", "yes");
+	}
+	free(str);
+}
+
 static void convert_device_storage(struct btd_adapter *adapter)
 {
 	char filename[PATH_MAX + 1];
@@ -2553,6 +2616,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 		textfile_put(filename, "converted", "yes");
 	}
 	free(str);
+
+	/* Convert aliases */
+	convert_file("aliases", address, convert_aliases_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 2/4] device: Use new storage for device alias
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

As set_alias() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
 src/device.c |   28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/src/device.c b/src/device.c
index 48970d2..c508d18 100644
--- a/src/device.c
+++ b/src/device.c
@@ -222,6 +222,10 @@ static gboolean store_device_info_cb(gpointer user_data)
 
 	g_key_file_set_string(key_file, "General", "Name", device->name);
 
+	if (device->alias != NULL)
+		g_key_file_set_string(key_file, "General", "Alias",
+					device->alias);
+
 	ba2str(adapter_get_address(device->adapter), adapter_addr);
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -432,28 +436,17 @@ static void set_alias(GDBusPendingPropertySet id, const char *alias,
 								void *data)
 {
 	struct btd_device *device = data;
-	struct btd_adapter *adapter = device->adapter;
-	char srcaddr[18], dstaddr[18];
-	int err;
 
 	/* No change */
 	if ((device->alias == NULL && g_str_equal(alias, "")) ||
 					g_strcmp0(device->alias, alias) == 0)
 		return g_dbus_pending_property_success(id);
 
-	ba2str(adapter_get_address(adapter), srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
-	/* Remove alias if empty string */
-	err = write_device_alias(srcaddr, dstaddr, device->bdaddr_type,
-					g_str_equal(alias, "") ? NULL : alias);
-	if (err < 0)
-		return g_dbus_pending_property_error(id,
-				ERROR_INTERFACE ".Failed", strerror(-err));
-
 	g_free(device->alias);
 	device->alias = g_str_equal(alias, "") ? NULL : g_strdup(alias);
 
+	store_device_info(device);
+
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 				device->path, DEVICE_INTERFACE, "Alias");
 
@@ -1746,6 +1739,10 @@ static void load_info(struct btd_device *device, const gchar *local,
 		g_free(str);
 	}
 
+	/* Load alias */
+	device->alias = g_key_file_get_string(key_file, "General", "Alias",
+						NULL);
+
 	if (store_needed)
 		store_device_info(device);
 
@@ -1759,7 +1756,7 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 	struct btd_device *device;
 	const gchar *adapter_path = adapter_get_path(adapter);
 	const bdaddr_t *src;
-	char srcaddr[18], alias[MAX_NAME_LENGTH + 1];
+	char srcaddr[18];
 	uint16_t vendor, product, version;
 
 	device = g_try_malloc0(sizeof(struct btd_device));
@@ -1790,9 +1787,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	if (read_device_alias(srcaddr, address, bdaddr_type, alias,
-							sizeof(alias)) == 0)
-		device->alias = g_strdup(alias);
 	device->trusted = read_trust(src, address, device->bdaddr_type);
 
 	if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 3/4] adapter: Convert storage trusts
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

All entry in trusts file are set to [all] (if a device is not trusted
it does not have entry in this file).
So, we do not need to check entry value and set device (entry key) as
trusted.
---
 src/adapter.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index ea7b8ff..99d58b5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2544,6 +2544,11 @@ static void convert_aliases_entry(GKeyFile *key_file, void *value)
 	g_key_file_set_string(key_file, "General", "Alias", value);
 }
 
+static void convert_trusts_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_boolean(key_file, "General", "Trusted", TRUE);
+}
+
 static void convert_entry(char *key, char *value, void *user_data)
 {
 	struct device_converter *converter = user_data;
@@ -2619,6 +2624,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 
 	/* Convert aliases */
 	convert_file("aliases", address, convert_aliases_entry);
+
+	/* Convert trusts */
+	convert_file("trusts", address, convert_trusts_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 4/4] device: Use new storage for device trust
From: Frédéric Danis @ 2012-11-14 10:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

As set_trust() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
 src/device.c |   22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/device.c b/src/device.c
index c508d18..792fe3d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -226,6 +226,9 @@ static gboolean store_device_info_cb(gpointer user_data)
 		g_key_file_set_string(key_file, "General", "Alias",
 					device->alias);
 
+	g_key_file_set_boolean(key_file, "General", "Trusted",
+				device->trusted);
+
 	ba2str(adapter_get_address(device->adapter), adapter_addr);
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -713,23 +716,14 @@ static gboolean dev_property_get_trusted(const GDBusPropertyTable *property,
 static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
 {
 	struct btd_device *device = data;
-	struct btd_adapter *adapter = device->adapter;
-	char srcaddr[18], dstaddr[18];
-	int err;
 
 	if (device->trusted == value)
 		return g_dbus_pending_property_success(id);
 
-	ba2str(adapter_get_address(adapter), srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
-	err = write_trust(srcaddr, dstaddr, device->bdaddr_type, value);
-	if (err < 0)
-		return g_dbus_pending_property_error(id,
-				ERROR_INTERFACE ".Failed", strerror(-err));
-
 	device->trusted = value;
 
+	store_device_info(device);
+
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 				device->path, DEVICE_INTERFACE, "Trusted");
 
@@ -1743,6 +1737,10 @@ static void load_info(struct btd_device *device, const gchar *local,
 	device->alias = g_key_file_get_string(key_file, "General", "Alias",
 						NULL);
 
+	/* Load trust */
+	device->trusted = g_key_file_get_boolean(key_file, "General",
+							"Trusted", NULL);
+
 	if (store_needed)
 		store_device_info(device);
 
@@ -1787,8 +1785,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	device->trusted = read_trust(src, address, device->bdaddr_type);
-
 	if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
 		device_block(device, FALSE);
 
-- 
1.7.9.5


^ permalink raw reply related

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Frédéric Dalleau @ 2012-11-14 10:18 UTC (permalink / raw)
  To: Arnaud Mouiche; +Cc: linux-bluetooth
In-Reply-To: <50A36DD1.1080507@invoxia.com>

Hi Arnaud,

On 11/14/2012 11:09 AM, Arnaud Mouiche wrote:
> Hello,
> I will try to find some time to try your patch.

It is a bit early since I haven't tested it myself, should be in much
better shape before end of the week.

Regards,
Frédéric

^ permalink raw reply

* RE: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Michael Knudsen @ 2012-11-14 10:21 UTC (permalink / raw)
  To: 'Arnaud Mouiche', 'Frédéric Dalleau'
  Cc: linux-bluetooth
In-Reply-To: <50A36DD1.1080507@invoxia.com>

> - can't we take the opportunity to configure the whole set
> of eSCO parameters (bandwith / latency / retransmission effort)
> instead of only getting the possibility to change the voice
> settings

I am currently looking into this as mentioned in my mail last
week:

	<509BB56A.80609@samsung.com>

If anyone has any input for this, I'm all ears.

-m.



^ permalink raw reply

* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Arnaud Mouiche @ 2012-11-14 10:33 UTC (permalink / raw)
  To: frederic.dalleau; +Cc: linux-bluetooth
In-Reply-To: <50A3700C.6090801@linux.intel.com>

What is your plan once the kernel will gain the defer capability.
- is it up to bluetoothd/audio to accept and configure ?
- is it the external HFP apps (for the HFP case) to do the job ?

arnaud

On 11/14/2012 11:18 AM, Frédéric Dalleau wrote:
> Hi Arnaud,
>
> On 11/14/2012 11:09 AM, Arnaud Mouiche wrote:
>> Hello,
>> I will try to find some time to try your patch.
>
> It is a bit early since I haven't tested it myself, should be in much
> better shape before end of the week.
>
> Regards,
> Frédéric


^ permalink raw reply

* Re: [PATCH 1/4] adapter: Convert storage aliases
From: Johan Hedberg @ 2012-11-14 10:45 UTC (permalink / raw)
  To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1352888166-7391-1-git-send-email-frederic.danis@linux.intel.com>

Hi Frederic,

On Wed, Nov 14, 2012, Frédéric Danis wrote:
>  #define SETTINGS_PATH STORAGEDIR "/%s/settings"
>  #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
> +#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"

Why do you have these separate defines? Do you need them in multiple
places? The define names are longer than the actual values and you're
just obfuscating the actual call that uses them (since you need to look
in two places to figure out of the format parameters are actually
matching the format string). So please don't use these.

Johan

^ permalink raw reply

* Re: [PATCH 1/4] adapter: Convert storage aliases
From: Frederic Danis @ 2012-11-14 11:04 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <20121114104553.GA6519@x220>

Hello Johan,

On 14/11/2012 11:45, Johan Hedberg wrote:
> Hi Frederic,
>
> On Wed, Nov 14, 2012, Frédéric Danis wrote:
>>   #define SETTINGS_PATH STORAGEDIR "/%s/settings"
>>   #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
>> +#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"
>
> Why do you have these separate defines? Do you need them in multiple
> places? The define names are longer than the actual values and you're
> just obfuscating the actual call that uses them (since you need to look
> in two places to figure out of the format parameters are actually
> matching the format string). So please don't use these.
>

CACHE_PATH and DEVICE_INFO_PATH are only used in one place, so they can 
be removed.

SETTINGS_PATH is used in both store_adapter_info() and load_config(), so 
I think it is better to keep it.

Fred


-- 
Frederic Danis                            Open Source Technology Center
frederic.danis@intel.com                              Intel Corporation


^ permalink raw reply

* Re: [PATCH 1/4] adapter: Convert storage aliases
From: Johan Hedberg @ 2012-11-14 11:11 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth
In-Reply-To: <50A37AC7.1090009@linux.intel.com>

Hi Frederic,

On Wed, Nov 14, 2012, Frederic Danis wrote:
> On 14/11/2012 11:45, Johan Hedberg wrote:
> >Hi Frederic,
> >
> >On Wed, Nov 14, 2012, Frédéric Danis wrote:
> >>  #define SETTINGS_PATH STORAGEDIR "/%s/settings"
> >>  #define CACHE_PATH STORAGEDIR "/%s/cache/%s"
> >>+#define DEVICE_INFO_PATH STORAGEDIR "/%s/%s/info"
> >
> >Why do you have these separate defines? Do you need them in multiple
> >places? The define names are longer than the actual values and you're
> >just obfuscating the actual call that uses them (since you need to look
> >in two places to figure out of the format parameters are actually
> >matching the format string). So please don't use these.
> >
> 
> CACHE_PATH and DEVICE_INFO_PATH are only used in one place, so they
> can be removed.
> 
> SETTINGS_PATH is used in both store_adapter_info() and
> load_config(), so I think it is better to keep it.

I'd still just duplicate it in both places for readability. These
locations will be static at least for the entire 5.x series and maybe
longer.

Johan

^ permalink raw reply

* [PATCH v2 1/5] adapter: Convert storage aliases
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth

---
 src/adapter.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index f9eb3af..7ba821e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2533,6 +2533,69 @@ static void convert_names_entry(char *key, char *value, void *user_data)
 	store_cached_name(&local, &peer, value);
 }
 
+struct device_converter {
+	char *address;
+	void (*cb)(GKeyFile *key_file, void *value);
+};
+
+static void convert_aliases_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_string(key_file, "General", "Alias", value);
+}
+
+static void convert_entry(char *key, char *value, void *user_data)
+{
+	struct device_converter *converter = user_data;
+	char filename[PATH_MAX + 1];
+	GKeyFile *key_file;
+	char *data;
+	gsize length = 0;
+
+	if (key[17] == '#')
+		key[17] = '\0';
+
+	if (bachk(key) != 0)
+		return;
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
+			converter->address, key);
+	filename[PATH_MAX] = '\0';
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	converter->cb(key_file, value);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(filename, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+}
+
+static void convert_file(char *file, char *address,
+				void (*cb)(GKeyFile *key_file, void *value))
+{
+	char filename[PATH_MAX + 1];
+	struct device_converter converter;
+	char *str;
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s", address, file);
+	filename[PATH_MAX] = '\0';
+
+	str = textfile_get(filename, "converted");
+	if (str && strcmp(str, "yes") == 0) {
+		DBG("Legacy file %s already converted", filename);
+	} else {
+		converter.address = address;
+		converter.cb = cb;
+
+		textfile_foreach(filename, convert_entry, &converter);
+		textfile_put(filename, "converted", "yes");
+	}
+	free(str);
+}
+
 static void convert_device_storage(struct btd_adapter *adapter)
 {
 	char filename[PATH_MAX + 1];
@@ -2553,6 +2616,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 		textfile_put(filename, "converted", "yes");
 	}
 	free(str);
+
+	/* Convert aliases */
+	convert_file("aliases", address, convert_aliases_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 2/5] device: Use new storage for device alias
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>

As set_alias() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
 src/device.c |   28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/src/device.c b/src/device.c
index 48970d2..c508d18 100644
--- a/src/device.c
+++ b/src/device.c
@@ -222,6 +222,10 @@ static gboolean store_device_info_cb(gpointer user_data)
 
 	g_key_file_set_string(key_file, "General", "Name", device->name);
 
+	if (device->alias != NULL)
+		g_key_file_set_string(key_file, "General", "Alias",
+					device->alias);
+
 	ba2str(adapter_get_address(device->adapter), adapter_addr);
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -432,28 +436,17 @@ static void set_alias(GDBusPendingPropertySet id, const char *alias,
 								void *data)
 {
 	struct btd_device *device = data;
-	struct btd_adapter *adapter = device->adapter;
-	char srcaddr[18], dstaddr[18];
-	int err;
 
 	/* No change */
 	if ((device->alias == NULL && g_str_equal(alias, "")) ||
 					g_strcmp0(device->alias, alias) == 0)
 		return g_dbus_pending_property_success(id);
 
-	ba2str(adapter_get_address(adapter), srcaddr);
-	ba2str(&device->bdaddr, dstaddr);
-
-	/* Remove alias if empty string */
-	err = write_device_alias(srcaddr, dstaddr, device->bdaddr_type,
-					g_str_equal(alias, "") ? NULL : alias);
-	if (err < 0)
-		return g_dbus_pending_property_error(id,
-				ERROR_INTERFACE ".Failed", strerror(-err));
-
 	g_free(device->alias);
 	device->alias = g_str_equal(alias, "") ? NULL : g_strdup(alias);
 
+	store_device_info(device);
+
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 				device->path, DEVICE_INTERFACE, "Alias");
 
@@ -1746,6 +1739,10 @@ static void load_info(struct btd_device *device, const gchar *local,
 		g_free(str);
 	}
 
+	/* Load alias */
+	device->alias = g_key_file_get_string(key_file, "General", "Alias",
+						NULL);
+
 	if (store_needed)
 		store_device_info(device);
 
@@ -1759,7 +1756,7 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 	struct btd_device *device;
 	const gchar *adapter_path = adapter_get_path(adapter);
 	const bdaddr_t *src;
-	char srcaddr[18], alias[MAX_NAME_LENGTH + 1];
+	char srcaddr[18];
 	uint16_t vendor, product, version;
 
 	device = g_try_malloc0(sizeof(struct btd_device));
@@ -1790,9 +1787,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 
 	load_info(device, srcaddr, address);
 
-	if (read_device_alias(srcaddr, address, bdaddr_type, alias,
-							sizeof(alias)) == 0)
-		device->alias = g_strdup(alias);
 	device->trusted = read_trust(src, address, device->bdaddr_type);
 
 	if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH v2 3/5] adapter: Convert storage trusts
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>

All entry in trusts file are set to [all] (if a device is not trusted
it does not have entry in this file).
So, we do not need to check entry value and set device (entry key) as
trusted.
---
 src/adapter.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 7ba821e..bdea378 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2543,6 +2543,11 @@ static void convert_aliases_entry(GKeyFile *key_file, void *value)
 	g_key_file_set_string(key_file, "General", "Alias", value);
 }
 
+static void convert_trusts_entry(GKeyFile *key_file, void *value)
+{
+	g_key_file_set_boolean(key_file, "General", "Trusted", TRUE);
+}
+
 static void convert_entry(char *key, char *value, void *user_data)
 {
 	struct device_converter *converter = user_data;
@@ -2619,6 +2624,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
 
 	/* Convert aliases */
 	convert_file("aliases", address, convert_aliases_entry);
+
+	/* Convert trusts */
+	convert_file("trusts", address, convert_trusts_entry);
 }
 
 static void convert_config(struct btd_adapter *adapter, const char *filename,
-- 
1.7.9.5


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox