[RFC] Basic Bluetooth LE support

[RFC] Basic Bluetooth LE support

[Ville Tervo]

Hi,

This pathcset introduses basic Bluetooth LE support. Please review.

[PATCH 1/7] Bluetooth: Add low energy commands and events

[Ville Tervo]

Add needed HCI command and event to create LE connections.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/hci.h |   52 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 740e5de..b86aed5 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -597,6 +597,39 @@ struct hci_rp_read_bd_addr {
 } __packed;
 
 /* --- HCI LE Commands --- */
+#define HCI_OP_LE_SET_EVENT_MASK	0x2001
+struct hci_cp_le_set_event_mask {
+	__u8     mask[8];
+} __packed;
+
+#define HCI_OP_LE_READ_BUFFER_SIZE	0x2002
+struct hci_rp_le_read_buffer_size {
+	__u8     status;
+	__le16   le_mtu;
+	__u8     le_max_pkt;
+} __packed;
+
+#define HCI_OP_LE_CREATE_CONN		0x200d
+struct hci_cp_le_create_conn {
+	__le16   scan_interval;
+	__le16   scan_window;
+	__u8     filter_policy;
+	__u8     peer_addr_type;
+	bdaddr_t peer_addr;
+	__u8     own_address_type;
+	__le16   conn_interval_min;
+	__le16   conn_interval_max;
+	__le16   conn_latency;
+	__le16   supervision_timeout;
+	__le16   min_ce_len;
+	__le16   max_ce_len;
+} __packed;
+
+#define HCI_OP_LE_CREATE_CONN_CANCEL	0x200e
+struct hci_cp_le_create_conn_cancel {
+	__u8     status;
+} __packed;
+
 #define HCI_OP_LE_SET_ADVERTISE_ENABLE	0x200a
 	#define LE_ADVERTISE_ENABLED	0x01
 	#define LE_ADVERTISE_DISABLED	0x00
@@ -857,6 +890,25 @@ struct hci_ev_remote_host_features {
 	__u8     features[8];
 } __packed;
 
+#define HCI_EV_LE_META			0x3e
+struct hci_ev_le_meta {
+	__u8     subevent;
+} __packed;
+
+/* Low energy meta events */
+#define HCI_EV_LE_CONN_COMPLETE		0x01
+struct hci_ev_le_conn_complete {
+	__u8     status;
+	__le16   handle;
+	__u8     role;
+	__u8     bdaddr_type;
+	bdaddr_t bdaddr;
+	__le16   interval;
+	__le16   latency;
+	__le16   supervision_timeout;
+	__u8     clk_accurancy;
+} __packed;
+
 /* Internal events generated by Bluetooth stack */
 #define HCI_EV_STACK_INTERNAL	0xfd
 struct hci_ev_stack_internal {
-- 
1.7.0.1

Re: [PATCH 1/7] Bluetooth: Add low energy commands and events

[Marcel Holtmann]

Hi Ville,

> Add needed HCI command and event to create LE connections.
> 
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 1/7] Bluetooth: Add low energy commands and events

[Gustavo F. Padovan]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2010-10-07 12:08:25 +0200]:

> Hi Ville,
> 
> > Add needed HCI command and event to create LE connections.
> > 
> > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>

There is a comment in patch 3/7 from Anderson that may affect this
patch, so I'm going to wait Ville say if he actually want to remove that
piece of the code or not to merge this one in the tree.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH 1/7] Bluetooth: Add low energy commands and events

[Ville Tervo]

Hi,

On Thu, Oct 07, 2010 at 06:31:11PM +0200, ext Gustavo F. Padovan wrote:
> Hi Marcel,
> 
> * Marcel Holtmann <marcel@holtmann.org> [2010-10-07 12:08:25 +0200]:
> 
> > Hi Ville,
> > 
> > > Add needed HCI command and event to create LE connections.
> > > 
> > > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> > 
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> There is a comment in patch 3/7 from Anderson that may affect this
> patch, so I'm going to wait Ville say if he actually want to remove that
> piece of the code or not to merge this one in the tree.

Yes I already have fixed version in my tree. I'll fix go through Marcel's other
suggestions also and send V2.

-- 
Ville

[PATCH 2/7] Bluetooth: Add LE connect support

[Ville Tervo]

Add logic to create LE connections.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/hci.h      |    1 +
 include/net/bluetooth/hci_core.h |    6 ++-
 net/bluetooth/hci_conn.c         |   38 ++++++++++++++-
 net/bluetooth/hci_event.c        |  100 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 141 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b86aed5..b326240 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -162,6 +162,7 @@ enum {
 #define SCO_LINK	0x00
 #define ACL_LINK	0x01
 #define ESCO_LINK	0x02
+#define LE_LINK		0x03
 
 /* LMP features */
 #define LMP_3SLOT	0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebec8c9..89f4b10 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -170,6 +170,7 @@ struct hci_conn {
 	bdaddr_t	 dst;
 	__u16		 handle;
 	__u16		 state;
+	__u16		 le_state;
 	__u8             mode;
 	__u8		 type;
 	__u8		 out;
@@ -203,6 +204,7 @@ struct hci_conn {
 	struct hci_dev	*hdev;
 	void		*l2cap_data;
 	void		*sco_data;
+	void		*le_data;
 	void		*priv;
 
 	struct hci_conn	*link;
@@ -272,7 +274,7 @@ static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c)
 {
 	struct hci_conn_hash *h = &hdev->conn_hash;
 	list_add(&c->list, &h->list);
-	if (c->type == ACL_LINK)
+	if (c->type == ACL_LINK || c->type == LE_LINK)
 		h->acl_num++;
 	else
 		h->sco_num++;
@@ -282,7 +284,7 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
 {
 	struct hci_conn_hash *h = &hdev->conn_hash;
 	list_del(&c->list);
-	if (c->type == ACL_LINK)
+	if (c->type == ACL_LINK || c->type == LE_LINK)
 		h->acl_num--;
 	else
 		h->sco_num--;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 145993f..cb41d64 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -45,6 +45,27 @@
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
 
+void hci_le_connect(struct hci_conn *conn)
+{
+	struct hci_dev *hdev = conn->hdev;
+	struct hci_cp_le_create_conn cp;
+
+	conn->le_state = BT_CONNECT;
+	conn->out = 1;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.scan_interval = cpu_to_le16(0x0004);
+	cp.scan_window = cpu_to_le16(0x0004);
+	bacpy(&cp.peer_addr, &conn->dst);
+	cp.conn_interval_min = cpu_to_le16(0x0008);
+	cp.conn_interval_max = cpu_to_le16(0x0100);
+	cp.supervision_timeout = cpu_to_le16(0x0064);
+	cp.min_ce_len = cpu_to_le16(0x0001);
+	cp.max_ce_len = cpu_to_le16(0xffff);
+
+	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
+}
+
 void hci_acl_connect(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -365,15 +386,30 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
 }
 EXPORT_SYMBOL(hci_get_route);
 
-/* Create SCO or ACL connection.
+/* Create SCO, ACL or LE connection.
  * Device _must_ be locked */
 struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
 {
 	struct hci_conn *acl;
 	struct hci_conn *sco;
+	struct hci_conn *le;
 
 	BT_DBG("%s dst %s", hdev->name, batostr(dst));
 
+	if (type == LE_LINK) {
+		le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
+
+		if (!le)
+			le = hci_conn_add(hdev, LE_LINK, dst);
+
+		if (!le)
+			return NULL;
+
+		hci_le_connect(le);
+
+		return le;
+	}
+
 	if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
 		if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
 			return NULL;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d3c68de..0b979ae 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -868,6 +868,44 @@ static void hci_cc_le_set_scan(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_req_complete(hdev, status);
 }
 
+static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
+{
+	struct hci_cp_le_create_conn *cp;
+	struct hci_conn *conn;
+
+	BT_DBG("%s status 0x%x", hdev->name, status);
+
+	cp = hci_sent_cmd_data(hdev, HCI_OP_LE_CREATE_CONN);
+	if (!cp)
+		return;
+
+	hci_dev_lock(hdev);
+
+	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
+
+	BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->peer_addr),
+		conn);
+
+	if (status) {
+		if (conn && conn->le_state == BT_CONNECT) {
+			conn->le_state = BT_CLOSED;
+			hci_proto_connect_cfm(conn, status);
+			hci_conn_del(conn);
+		}
+	} else {
+		if (!conn) {
+			conn = hci_conn_add(hdev, LE_LINK, &cp->peer_addr);
+			if (conn) {
+				conn->out = 1;
+				conn->link_mode |= HCI_LM_MASTER;
+			} else
+				BT_ERR("No memory for new connection");
+		}
+	}
+
+	hci_dev_unlock(hdev);
+}
+
 static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	__u8 status = *((__u8 *) skb->data);
@@ -1069,7 +1107,10 @@ static inline void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff
 
 	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
 	if (conn) {
-		conn->state = BT_CLOSED;
+		if (conn->type == LE_LINK)
+			conn->le_state = BT_CLOSED;
+		else
+			conn->state = BT_CLOSED;
 
 		hci_proto_disconn_cfm(conn, ev->reason);
 		hci_conn_del(conn);
@@ -1430,6 +1471,10 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cs_exit_sniff_mode(hdev, ev->status);
 		break;
 
+	case HCI_OP_LE_CREATE_CONN:
+		hci_cs_le_create_conn(hdev, ev->status);
+		break;
+
 	default:
 		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
 		break;
@@ -1875,6 +1920,55 @@ static inline void hci_remote_host_features_evt(struct hci_dev *hdev, struct sk_
 	hci_dev_unlock(hdev);
 }
 
+static inline void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_ev_le_conn_complete *ev = (void *) skb->data;
+	struct hci_conn *conn;
+
+	BT_DBG("%s status %d", hdev->name, ev->status);
+
+	hci_dev_lock(hdev);
+
+	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
+
+	if (!conn)
+		goto unlock;
+
+	if (ev->status) {
+		hci_proto_connect_cfm(conn, ev->status);
+		conn->le_state = BT_CLOSED;
+		hci_conn_del(conn);
+		goto unlock;
+	}
+
+	conn->handle = __le16_to_cpu(ev->handle);
+	conn->le_state = BT_CONNECTED;
+
+	hci_conn_hold_device(conn);
+	hci_conn_add_sysfs(conn);
+
+	hci_proto_connect_cfm(conn, ev->status);
+unlock:
+	hci_dev_unlock(hdev);
+}
+
+static inline void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_ev_le_meta *le_ev = (void *) skb->data;
+	__u8 event = le_ev->subevent;
+
+	skb_pull(skb, sizeof(*le_ev));
+
+	switch (event) {
+	case HCI_EV_LE_CONN_COMPLETE:
+		hci_le_conn_complete_evt(hdev, skb);
+		break;
+
+	default:
+		break;
+	}
+}
+
 void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_event_hdr *hdr = (void *) skb->data;
@@ -2011,6 +2105,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_remote_host_features_evt(hdev, skb);
 		break;
 
+	case HCI_EV_LE_META:
+		hci_le_meta_evt(hdev, skb);
+		break;
+
 	default:
 		BT_DBG("%s event 0x%x", hdev->name, event);
 		break;
-- 
1.7.0.1

Re: [PATCH 2/7] Bluetooth: Add LE connect support

[Marcel Holtmann]

Hi Ville,

> Add logic to create LE connections.
> 
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    1 +
>  include/net/bluetooth/hci_core.h |    6 ++-
>  net/bluetooth/hci_conn.c         |   38 ++++++++++++++-
>  net/bluetooth/hci_event.c        |  100 +++++++++++++++++++++++++++++++++++++-
>  4 files changed, 141 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index b86aed5..b326240 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -162,6 +162,7 @@ enum {
>  #define SCO_LINK	0x00
>  #define ACL_LINK	0x01
>  #define ESCO_LINK	0x02
> +#define LE_LINK		0x03

this is not a value defined by the specification, while the others are.
And some functions match these to HCI event. So if wanna do it like
this, then using something like 0x80 is better.
 
>  /* LMP features */
>  #define LMP_3SLOT	0x01
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ebec8c9..89f4b10 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -170,6 +170,7 @@ struct hci_conn {
>  	bdaddr_t	 dst;
>  	__u16		 handle;
>  	__u16		 state;
> +	__u16		 le_state;

I don't see the need for a separate state here. The LE link is different
from an ACL link and also from a SCO link. We will create a new hci_conn
for each type of link.

>  	__u8             mode;
>  	__u8		 type;
>  	__u8		 out;
> @@ -203,6 +204,7 @@ struct hci_conn {
>  	struct hci_dev	*hdev;
>  	void		*l2cap_data;
>  	void		*sco_data;
> +	void		*le_data;
>  	void		*priv;

What is the use of le_data here?
 
>  	struct hci_conn	*link;
> @@ -272,7 +274,7 @@ static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c)
>  {
>  	struct hci_conn_hash *h = &hdev->conn_hash;
>  	list_add(&c->list, &h->list);
> -	if (c->type == ACL_LINK)
> +	if (c->type == ACL_LINK || c->type == LE_LINK)
>  		h->acl_num++;
>  	else
>  		h->sco_num++;
> @@ -282,7 +284,7 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
>  {
>  	struct hci_conn_hash *h = &hdev->conn_hash;
>  	list_del(&c->list);
> -	if (c->type == ACL_LINK)
> +	if (c->type == ACL_LINK || c->type == LE_LINK)
>  		h->acl_num--;
>  	else
>  		h->sco_num--;

I would assume that counting LE connection separately would be way
better. We could have ACL link to one device and LE link to another.

> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 145993f..cb41d64 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -45,6 +45,27 @@
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
>  
> +void hci_le_connect(struct hci_conn *conn)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +	struct hci_cp_le_create_conn cp;
> +
> +	conn->le_state = BT_CONNECT;
> +	conn->out = 1;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.scan_interval = cpu_to_le16(0x0004);
> +	cp.scan_window = cpu_to_le16(0x0004);
> +	bacpy(&cp.peer_addr, &conn->dst);
> +	cp.conn_interval_min = cpu_to_le16(0x0008);
> +	cp.conn_interval_max = cpu_to_le16(0x0100);
> +	cp.supervision_timeout = cpu_to_le16(0x0064);
> +	cp.min_ce_len = cpu_to_le16(0x0001);
> +	cp.max_ce_len = cpu_to_le16(0xffff);
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> +}
> +
>  void hci_acl_connect(struct hci_conn *conn)
>  {
>  	struct hci_dev *hdev = conn->hdev;
> @@ -365,15 +386,30 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
>  }
>  EXPORT_SYMBOL(hci_get_route);
>  
> -/* Create SCO or ACL connection.
> +/* Create SCO, ACL or LE connection.
>   * Device _must_ be locked */
>  struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
>  {
>  	struct hci_conn *acl;
>  	struct hci_conn *sco;
> +	struct hci_conn *le;
>  
>  	BT_DBG("%s dst %s", hdev->name, batostr(dst));
>  
> +	if (type == LE_LINK) {
> +		le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
> +
> +		if (!le)
> +			le = hci_conn_add(hdev, LE_LINK, dst);
> +
> +		if (!le)
> +			return NULL;
> +
> +		hci_le_connect(le);
> +
> +		return le;
> +	}
> +
>  	if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
>  		if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
>  			return NULL;

No liking it this much. We might have to re-think on how to do things
here.

> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d3c68de..0b979ae 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -868,6 +868,44 @@ static void hci_cc_le_set_scan(struct hci_dev *hdev, struct sk_buff *skb)
>  	hci_req_complete(hdev, status);
>  }
>  
> +static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
> +{
> +	struct hci_cp_le_create_conn *cp;
> +	struct hci_conn *conn;
> +
> +	BT_DBG("%s status 0x%x", hdev->name, status);
> +
> +	cp = hci_sent_cmd_data(hdev, HCI_OP_LE_CREATE_CONN);
> +	if (!cp)
> +		return;
> +
> +	hci_dev_lock(hdev);
> +
> +	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
> +
> +	BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->peer_addr),
> +		conn);
> +
> +	if (status) {
> +		if (conn && conn->le_state == BT_CONNECT) {
> +			conn->le_state = BT_CLOSED;
> +			hci_proto_connect_cfm(conn, status);
> +			hci_conn_del(conn);
> +		}
> +	} else {
> +		if (!conn) {
> +			conn = hci_conn_add(hdev, LE_LINK, &cp->peer_addr);
> +			if (conn) {
> +				conn->out = 1;
> +				conn->link_mode |= HCI_LM_MASTER;
> +			} else
> +				BT_ERR("No memory for new connection");
> +		}
> +	}
> +
> +	hci_dev_unlock(hdev);
> +}

Do we have the master/slave association with LE?

>  static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>  	__u8 status = *((__u8 *) skb->data);
> @@ -1069,7 +1107,10 @@ static inline void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff
>  
>  	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
>  	if (conn) {
> -		conn->state = BT_CLOSED;
> +		if (conn->type == LE_LINK)
> +			conn->le_state = BT_CLOSED;
> +		else
> +			conn->state = BT_CLOSED;

If we would just use conn->state then this should not be needed.
 
>  		hci_proto_disconn_cfm(conn, ev->reason);
>  		hci_conn_del(conn);
> @@ -1430,6 +1471,10 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  		hci_cs_exit_sniff_mode(hdev, ev->status);
>  		break;
>  
> +	case HCI_OP_LE_CREATE_CONN:
> +		hci_cs_le_create_conn(hdev, ev->status);
> +		break;
> +
>  	default:
>  		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
>  		break;
> @@ -1875,6 +1920,55 @@ static inline void hci_remote_host_features_evt(struct hci_dev *hdev, struct sk_
>  	hci_dev_unlock(hdev);
>  }
>  
> +static inline void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	struct hci_ev_le_conn_complete *ev = (void *) skb->data;
> +	struct hci_conn *conn;
> +
> +	BT_DBG("%s status %d", hdev->name, ev->status);
> +
> +	hci_dev_lock(hdev);
> +
> +	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
> +
> +	if (!conn)
> +		goto unlock;

The empty line between conn assignment and check should be removed.

> +
> +	if (ev->status) {
> +		hci_proto_connect_cfm(conn, ev->status);
> +		conn->le_state = BT_CLOSED;
> +		hci_conn_del(conn);
> +		goto unlock;
> +	}
> +
> +	conn->handle = __le16_to_cpu(ev->handle);
> +	conn->le_state = BT_CONNECTED;
> +
> +	hci_conn_hold_device(conn);
> +	hci_conn_add_sysfs(conn);
> +
> +	hci_proto_connect_cfm(conn, ev->status);
> +unlock:
> +	hci_dev_unlock(hdev);

And here you should have an empty line between the label and the last
statement before the label.

> +}
> +
> +static inline void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	struct hci_ev_le_meta *le_ev = (void *) skb->data;
> +	__u8 event = le_ev->subevent;

Why?

> +
> +	skb_pull(skb, sizeof(*le_ev));
> +
> +	switch (event) {

Using le_ev->subevent would be just fine here.

> +	case HCI_EV_LE_CONN_COMPLETE:
> +		hci_le_conn_complete_evt(hdev, skb);
> +		break;
> +
> +	default:
> +		break;
> +	}
> +}
> +
>  void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>  	struct hci_event_hdr *hdr = (void *) skb->data;
> @@ -2011,6 +2105,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
>  		hci_remote_host_features_evt(hdev, skb);
>  		break;
>  
> +	case HCI_EV_LE_META:
> +		hci_le_meta_evt(hdev, skb);
> +		break;
> +
>  	default:
>  		BT_DBG("%s event 0x%x", hdev->name, event);
>  		break;

Regards

Marcel

Re: [PATCH 2/7] Bluetooth: Add LE connect support

[Gustavo F. Padovan]

Hi Ville,

* Marcel Holtmann <marcel@holtmann.org> [2010-10-07 12:58:09 +0200]:

> Hi Ville,
> 
> > Add logic to create LE connections.


Could you be more verbose on the commit message, that way people can
understand better what you are doing in the patch. ;) 

> > 
> > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> > ---
> >  include/net/bluetooth/hci.h      |    1 +
> >  include/net/bluetooth/hci_core.h |    6 ++-
> >  net/bluetooth/hci_conn.c         |   38 ++++++++++++++-
> >  net/bluetooth/hci_event.c        |  100 +++++++++++++++++++++++++++++++++++++-
> >  4 files changed, 141 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index b86aed5..b326240 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -162,6 +162,7 @@ enum {
> >  #define SCO_LINK	0x00
> >  #define ACL_LINK	0x01
> >  #define ESCO_LINK	0x02
> > +#define LE_LINK		0x03
> 
> this is not a value defined by the specification, while the others are.
> And some functions match these to HCI event. So if wanna do it like
> this, then using something like 0x80 is better.

A comment in the code saying that is also a good ideia.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH 2/7] Bluetooth: Add LE connect support

[Ville Tervo]

On Thu, Oct 07, 2010 at 06:36:20PM +0200, ext Gustavo F. Padovan wrote:
> Hi Ville,
> 
> * Marcel Holtmann <marcel@holtmann.org> [2010-10-07 12:58:09 +0200]:
> 
> > Hi Ville,
> > 
> > > Add logic to create LE connections.
> 
> 
> Could you be more verbose on the commit message, that way people can
> understand better what you are doing in the patch. ;) 

Sure. 

> 
> > > 
> > > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> > > ---
> > >  include/net/bluetooth/hci.h      |    1 +
> > >  include/net/bluetooth/hci_core.h |    6 ++-
> > >  net/bluetooth/hci_conn.c         |   38 ++++++++++++++-
> > >  net/bluetooth/hci_event.c        |  100 +++++++++++++++++++++++++++++++++++++-
> > >  4 files changed, 141 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > > index b86aed5..b326240 100644
> > > --- a/include/net/bluetooth/hci.h
> > > +++ b/include/net/bluetooth/hci.h
> > > @@ -162,6 +162,7 @@ enum {
> > >  #define SCO_LINK	0x00
> > >  #define ACL_LINK	0x01
> > >  #define ESCO_LINK	0x02
> > > +#define LE_LINK		0x03
> > 
> > this is not a value defined by the specification, while the others are.
> > And some functions match these to HCI event. So if wanna do it like
> > this, then using something like 0x80 is better.
> 
> A comment in the code saying that is also a good ideia.

Done

-- 
VIlle

Re: [PATCH 2/7] Bluetooth: Add LE connect support

[Ville Tervo]

Hi Marcel,

Thanks for the review. Could you  comments on hci_connect?

On Thu, Oct 07, 2010 at 12:58:09PM +0200, ext Marcel Holtmann wrote:
> Hi Ville,
> 
> > Add logic to create LE connections.
> > 
> > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> > ---
> >  include/net/bluetooth/hci.h      |    1 +
> >  include/net/bluetooth/hci_core.h |    6 ++-
> >  net/bluetooth/hci_conn.c         |   38 ++++++++++++++-
> >  net/bluetooth/hci_event.c        |  100 +++++++++++++++++++++++++++++++++++++-
> >  4 files changed, 141 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index b86aed5..b326240 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -162,6 +162,7 @@ enum {
> >  #define SCO_LINK	0x00
> >  #define ACL_LINK	0x01
> >  #define ESCO_LINK	0x02
> > +#define LE_LINK		0x03
> 
> this is not a value defined by the specification, while the others are.
> And some functions match these to HCI event. So if wanna do it like
> this, then using something like 0x80 is better.

Done with some explanation why 0x80 is used.

> >  /* LMP features */
> >  #define LMP_3SLOT	0x01
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index ebec8c9..89f4b10 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -170,6 +170,7 @@ struct hci_conn {
> >  	bdaddr_t	 dst;
> >  	__u16		 handle;
> >  	__u16		 state;
> > +	__u16		 le_state;
> 
> I don't see the need for a separate state here. The LE link is different
> from an ACL link and also from a SCO link. We will create a new hci_conn
> for each type of link.

I removed le_state completely.

> >  	__u8             mode;
> >  	__u8		 type;
> >  	__u8		 out;
> > @@ -203,6 +204,7 @@ struct hci_conn {
> >  	struct hci_dev	*hdev;
> >  	void		*l2cap_data;
> >  	void		*sco_data;
> > +	void		*le_data;
> >  	void		*priv;
> 
> What is the use of le_data here?

No use. Some left over earlier version.

> >  	struct hci_conn	*link;
> > @@ -272,7 +274,7 @@ static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c)
> >  {
> >  	struct hci_conn_hash *h = &hdev->conn_hash;
> >  	list_add(&c->list, &h->list);
> > -	if (c->type == ACL_LINK)
> > +	if (c->type == ACL_LINK || c->type == LE_LINK)
> >  		h->acl_num++;
> >  	else
> >  		h->sco_num++;
> > @@ -282,7 +284,7 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
> >  {
> >  	struct hci_conn_hash *h = &hdev->conn_hash;
> >  	list_del(&c->list);
> > -	if (c->type == ACL_LINK)
> > +	if (c->type == ACL_LINK || c->type == LE_LINK)
> >  		h->acl_num--;
> >  	else
> >  		h->sco_num--;
> 
> I would assume that counting LE connection separately would be way
> better. We could have ACL link to one device and LE link to another.

Done

> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 145993f..cb41d64 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -45,6 +45,27 @@
> >  #include <net/bluetooth/bluetooth.h>
> >  #include <net/bluetooth/hci_core.h>
> >  
> > +void hci_le_connect(struct hci_conn *conn)
> > +{
> > +	struct hci_dev *hdev = conn->hdev;
> > +	struct hci_cp_le_create_conn cp;
> > +
> > +	conn->le_state = BT_CONNECT;
> > +	conn->out = 1;
> > +
> > +	memset(&cp, 0, sizeof(cp));
> > +	cp.scan_interval = cpu_to_le16(0x0004);
> > +	cp.scan_window = cpu_to_le16(0x0004);
> > +	bacpy(&cp.peer_addr, &conn->dst);
> > +	cp.conn_interval_min = cpu_to_le16(0x0008);
> > +	cp.conn_interval_max = cpu_to_le16(0x0100);
> > +	cp.supervision_timeout = cpu_to_le16(0x0064);
> > +	cp.min_ce_len = cpu_to_le16(0x0001);
> > +	cp.max_ce_len = cpu_to_le16(0xffff);
> > +
> > +	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> > +}
> > +
> >  void hci_acl_connect(struct hci_conn *conn)
> >  {
> >  	struct hci_dev *hdev = conn->hdev;
> > @@ -365,15 +386,30 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
> >  }
> >  EXPORT_SYMBOL(hci_get_route);
> >  
> > -/* Create SCO or ACL connection.
> > +/* Create SCO, ACL or LE connection.
> >   * Device _must_ be locked */
> >  struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
> >  {
> >  	struct hci_conn *acl;
> >  	struct hci_conn *sco;
> > +	struct hci_conn *le;
> >  
> >  	BT_DBG("%s dst %s", hdev->name, batostr(dst));
> >  
> > +	if (type == LE_LINK) {
> > +		le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
> > +
> > +		if (!le)
> > +			le = hci_conn_add(hdev, LE_LINK, dst);
> > +
> > +		if (!le)
> > +			return NULL;
> > +
> > +		hci_le_connect(le);
> > +
> > +		return le;
> > +	}
> > +
> >  	if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
> >  		if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
> >  			return NULL;
> 
> No liking it this much. We might have to re-think on how to do things
> here.

Could you eloborate which part you dislike? I'm leaving this currently like it was.

> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index d3c68de..0b979ae 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -868,6 +868,44 @@ static void hci_cc_le_set_scan(struct hci_dev *hdev, struct sk_buff *skb)
> >  	hci_req_complete(hdev, status);
> >  }
> >  
> > +static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
> > +{
> > +	struct hci_cp_le_create_conn *cp;
> > +	struct hci_conn *conn;
> > +
> > +	BT_DBG("%s status 0x%x", hdev->name, status);
> > +
> > +	cp = hci_sent_cmd_data(hdev, HCI_OP_LE_CREATE_CONN);
> > +	if (!cp)
> > +		return;
> > +
> > +	hci_dev_lock(hdev);
> > +
> > +	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
> > +
> > +	BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->peer_addr),
> > +		conn);
> > +
> > +	if (status) {
> > +		if (conn && conn->le_state == BT_CONNECT) {
> > +			conn->le_state = BT_CLOSED;
> > +			hci_proto_connect_cfm(conn, status);
> > +			hci_conn_del(conn);
> > +		}
> > +	} else {
> > +		if (!conn) {
> > +			conn = hci_conn_add(hdev, LE_LINK, &cp->peer_addr);
> > +			if (conn) {
> > +				conn->out = 1;
> > +				conn->link_mode |= HCI_LM_MASTER;
> > +			} else
> > +				BT_ERR("No memory for new connection");
> > +		}
> > +	}
> > +
> > +	hci_dev_unlock(hdev);
> > +}
> 
> Do we have the master/slave association with LE?

Yes. But it cannot be changed like in legacy bluetooth connection. conn->out
and conn->link_mode |= HCI_LM_MASTER are redutant information. I removed
conn->link_mode setting.

> >  static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >  {
> >  	__u8 status = *((__u8 *) skb->data);
> > @@ -1069,7 +1107,10 @@ static inline void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff
> >  
> >  	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
> >  	if (conn) {
> > -		conn->state = BT_CLOSED;
> > +		if (conn->type == LE_LINK)
> > +			conn->le_state = BT_CLOSED;
> > +		else
> > +			conn->state = BT_CLOSED;
> 
> If we would just use conn->state then this should not be needed.
>  
> >  		hci_proto_disconn_cfm(conn, ev->reason);
> >  		hci_conn_del(conn);
> > @@ -1430,6 +1471,10 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >  		hci_cs_exit_sniff_mode(hdev, ev->status);
> >  		break;
> >  
> > +	case HCI_OP_LE_CREATE_CONN:
> > +		hci_cs_le_create_conn(hdev, ev->status);
> > +		break;
> > +
> >  	default:
> >  		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
> >  		break;
> > @@ -1875,6 +1920,55 @@ static inline void hci_remote_host_features_evt(struct hci_dev *hdev, struct sk_
> >  	hci_dev_unlock(hdev);
> >  }
> >  
> > +static inline void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > +	struct hci_ev_le_conn_complete *ev = (void *) skb->data;
> > +	struct hci_conn *conn;
> > +
> > +	BT_DBG("%s status %d", hdev->name, ev->status);
> > +
> > +	hci_dev_lock(hdev);
> > +
> > +	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
> > +
> > +	if (!conn)
> > +		goto unlock;
> 
> The empty line between conn assignment and check should be removed.

Done

> 
> > +
> > +	if (ev->status) {
> > +		hci_proto_connect_cfm(conn, ev->status);
> > +		conn->le_state = BT_CLOSED;
> > +		hci_conn_del(conn);
> > +		goto unlock;
> > +	}
> > +
> > +	conn->handle = __le16_to_cpu(ev->handle);
> > +	conn->le_state = BT_CONNECTED;
> > +
> > +	hci_conn_hold_device(conn);
> > +	hci_conn_add_sysfs(conn);
> > +
> > +	hci_proto_connect_cfm(conn, ev->status);
> > +unlock:
> > +	hci_dev_unlock(hdev);
> 
> And here you should have an empty line between the label and the last
> statement before the label.

Done

> > +}
> > +
> > +static inline void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > +	struct hci_ev_le_meta *le_ev = (void *) skb->data;
> > +	__u8 event = le_ev->subevent;
> 
> Why?
> 
> > +
> > +	skb_pull(skb, sizeof(*le_ev));
> > +
> > +	switch (event) {
> 
> Using le_ev->subevent would be just fine here.

Done

> > +	case HCI_EV_LE_CONN_COMPLETE:
> > +		hci_le_conn_complete_evt(hdev, skb);
> > +		break;
> > +
> > +	default:
> > +		break;
> > +	}
> > +}
> > +
> >  void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
> >  {
> >  	struct hci_event_hdr *hdr = (void *) skb->data;
> > @@ -2011,6 +2105,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
> >  		hci_remote_host_features_evt(hdev, skb);
> >  		break;
> >  
> > +	case HCI_EV_LE_META:
> > +		hci_le_meta_evt(hdev, skb);
> > +		break;
> > +
> >  	default:
> >  		BT_DBG("%s event 0x%x", hdev->name, event);
> >  		break;
> 

-- 
Ville

[PATCH 3/7] Bluetooth: LE disconnection and connect cancel support

[Ville Tervo]

Add supprt to cancel and disconnet connections.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/hci.h      |    5 ++---
 include/net/bluetooth/hci_core.h |    3 +++
 net/bluetooth/hci_conn.c         |   30 ++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b326240..d04ecea 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -191,6 +191,8 @@ enum {
 
 #define LMP_EV4		0x01
 #define LMP_EV5		0x02
+#define LMP_NO_BR	0x20
+#define LMP_LE		0x40
 
 #define LMP_SNIFF_SUBR	0x02
 #define LMP_EDR_ESCO_2M	0x20
@@ -627,9 +629,6 @@ struct hci_cp_le_create_conn {
 } __packed;
 
 #define HCI_OP_LE_CREATE_CONN_CANCEL	0x200e
-struct hci_cp_le_create_conn_cancel {
-	__u8     status;
-} __packed;
 
 #define HCI_OP_LE_SET_ADVERTISE_ENABLE	0x200a
 	#define LE_ADVERTISE_ENABLED	0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 89f4b10..a430a57 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -455,10 +455,13 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define lmp_rswitch_capable(dev)   ((dev)->features[0] & LMP_RSWITCH)
 #define lmp_encrypt_capable(dev)   ((dev)->features[0] & LMP_ENCRYPT)
 #define lmp_sniff_capable(dev)     ((dev)->features[0] & LMP_SNIFF)
+#define lmp_br_capable(dev)        (!((dev)->features[4] & LMP_NO_BR))
+#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
 #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
 #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
 #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
 
+
 /* ----- HCI protocols ----- */
 struct hci_proto {
 	char		*name;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index cb41d64..50f8973 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -66,6 +66,31 @@ void hci_le_connect(struct hci_conn *conn)
 	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
 }
 
+static void hci_le_connect_cancel(struct hci_conn *conn)
+{
+	struct hci_dev *hdev = conn->hdev;
+
+	BT_DBG("%p", conn);
+
+	if (!lmp_le_capable(hdev))
+		return;
+
+	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
+}
+
+void hci_le_disconn(struct hci_conn *conn, __u8 reason)
+{
+	struct hci_cp_disconnect cp;
+
+	BT_DBG("%p", conn);
+
+	conn->le_state = BT_DISCONN;
+
+	cp.handle = cpu_to_le16(conn->handle);
+	cp.reason = reason;
+	hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp);
+}
+
 void hci_acl_connect(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -221,6 +246,8 @@ static void hci_conn_timeout(unsigned long arg)
 	case BT_CONNECT2:
 		if (conn->type == ACL_LINK && conn->out)
 			hci_acl_connect_cancel(conn);
+		if (conn->type == LE_LINK && conn->out)
+			hci_le_connect_cancel(conn);
 		break;
 	case BT_CONFIG:
 	case BT_CONNECTED:
@@ -397,6 +424,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 	BT_DBG("%s dst %s", hdev->name, batostr(dst));
 
 	if (type == LE_LINK) {
+		if (!lmp_le_capable(hdev))
+			return NULL;
+
 		le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
 
 		if (!le)
-- 
1.7.0.1

Re: [PATCH 3/7] Bluetooth: LE disconnection and connect cancel support

[Anderson Lizardo]

Hi Ville,

On Wed, Oct 6, 2010 at 2:42 PM, Ville Tervo <ville.tervo@nokia.com> wrote:
> @@ -627,9 +629,6 @@ struct hci_cp_le_create_conn {
>  } __packed;
>
>  #define HCI_OP_LE_CREATE_CONN_CANCEL   0x200e
> -struct hci_cp_le_create_conn_cancel {
> -       __u8     status;
> -} __packed;
>
>  #define HCI_OP_LE_SET_ADVERTISE_ENABLE 0x200a
>        #define LE_ADVERTISE_ENABLED    0x01

The struct above is added in 1/7 then removed on this one, so you
could drop it from patch 1/7 instead?

> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 89f4b10..a430a57 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -455,10 +455,13 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>  #define lmp_rswitch_capable(dev)   ((dev)->features[0] & LMP_RSWITCH)
>  #define lmp_encrypt_capable(dev)   ((dev)->features[0] & LMP_ENCRYPT)
>  #define lmp_sniff_capable(dev)     ((dev)->features[0] & LMP_SNIFF)
> +#define lmp_br_capable(dev)        (!((dev)->features[4] & LMP_NO_BR))
> +#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
>  #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
>  #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
>  #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
>
> +
>  /* ----- HCI protocols ----- */

Unrelated extra line added above.

>  struct hci_proto {
>        char            *name;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index cb41d64..50f8973 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -66,6 +66,31 @@ void hci_le_connect(struct hci_conn *conn)
>        hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
>  }
>
> +static void hci_le_connect_cancel(struct hci_conn *conn)
> +{
> +       struct hci_dev *hdev = conn->hdev;
> +
> +       BT_DBG("%p", conn);

This debug message could be made more readable, e.g.:

BT_DBG("%s conn %p", hdev->name, conn);

(or simply dropped if not that useful).

> +
> +       if (!lmp_le_capable(hdev))
> +               return;
> +
> +       hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
> +}
> +
> +void hci_le_disconn(struct hci_conn *conn, __u8 reason)
> +{
> +       struct hci_cp_disconnect cp;
> +
> +       BT_DBG("%p", conn);

Same here.

Regards,
-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

Re: [PATCH 3/7] Bluetooth: LE disconnection and connect cancel support

[Marcel Holtmann]

Hi Ville,

> Add supprt to cancel and disconnet connections.
> 
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    5 ++---
>  include/net/bluetooth/hci_core.h |    3 +++
>  net/bluetooth/hci_conn.c         |   30 ++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index b326240..d04ecea 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -191,6 +191,8 @@ enum {
>  
>  #define LMP_EV4		0x01
>  #define LMP_EV5		0x02
> +#define LMP_NO_BR	0x20
> +#define LMP_LE		0x40

Keep these in sync with the constants we use in userspace.
 
>  #define LMP_SNIFF_SUBR	0x02
>  #define LMP_EDR_ESCO_2M	0x20
> @@ -627,9 +629,6 @@ struct hci_cp_le_create_conn {
>  } __packed;
>  
>  #define HCI_OP_LE_CREATE_CONN_CANCEL	0x200e
> -struct hci_cp_le_create_conn_cancel {
> -	__u8     status;
> -} __packed;
>  
>  #define HCI_OP_LE_SET_ADVERTISE_ENABLE	0x200a
>  	#define LE_ADVERTISE_ENABLED	0x01
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 89f4b10..a430a57 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -455,10 +455,13 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>  #define lmp_rswitch_capable(dev)   ((dev)->features[0] & LMP_RSWITCH)
>  #define lmp_encrypt_capable(dev)   ((dev)->features[0] & LMP_ENCRYPT)
>  #define lmp_sniff_capable(dev)     ((dev)->features[0] & LMP_SNIFF)
> +#define lmp_br_capable(dev)        (!((dev)->features[4] & LMP_NO_BR))

This makes no sense to me. And leave this out for now. This is more
complicated when running on LE only.

> +#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)

I would just add this at the end of the list and not intermix it with
sniff and sniffsubr defines.

>  #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
>  #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
>  #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
>  
> +
>  /* ----- HCI protocols ----- */
>  struct hci_proto {
>  	char		*name;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index cb41d64..50f8973 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -66,6 +66,31 @@ void hci_le_connect(struct hci_conn *conn)
>  	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
>  }
>  
> +static void hci_le_connect_cancel(struct hci_conn *conn)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +
> +	BT_DBG("%p", conn);
> +
> +	if (!lmp_le_capable(hdev))
> +		return;

This should not be needed. We should not have tried to establish a LE
link if we don't support LE in the first place.

> +
> +	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
> +}
> +
> +void hci_le_disconn(struct hci_conn *conn, __u8 reason)
> +{
> +	struct hci_cp_disconnect cp;
> +
> +	BT_DBG("%p", conn);
> +
> +	conn->le_state = BT_DISCONN;
> +
> +	cp.handle = cpu_to_le16(conn->handle);
> +	cp.reason = reason;
> +	hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp);
> +}

When just using conn->state, then this becomes obsolete and we can use
the generic one.

> +
>  void hci_acl_connect(struct hci_conn *conn)
>  {
>  	struct hci_dev *hdev = conn->hdev;
> @@ -221,6 +246,8 @@ static void hci_conn_timeout(unsigned long arg)
>  	case BT_CONNECT2:
>  		if (conn->type == ACL_LINK && conn->out)
>  			hci_acl_connect_cancel(conn);
> +		if (conn->type == LE_LINK && conn->out)
> +			hci_le_connect_cancel(conn);

This should be redone with as this:

	if (conn->out) {
		if (ACL_LINK)
			...
		else if (LE_LINK)
			...
	}

>  		break;
>  	case BT_CONFIG:
>  	case BT_CONNECTED:
> @@ -397,6 +424,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
>  	BT_DBG("%s dst %s", hdev->name, batostr(dst));
>  
>  	if (type == LE_LINK) {
> +		if (!lmp_le_capable(hdev))
> +			return NULL;
> +
>  		le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
>  
>  		if (!le)

We might need to move that lmp_le_capable check into L2CAP. Since
otherwise we can not give a proper return value if someone tries to use
LE on a BR/EDR only controller.

Regards

Marcel

Re: [PATCH 3/7] Bluetooth: LE disconnection and connect cancel support

[Ville Tervo]

Hi Marcel,

On Thu, Oct 07, 2010 at 01:04:27PM +0200, ext Marcel Holtmann wrote:
> Hi Ville,
> 
> > Add supprt to cancel and disconnet connections.
> > 
> > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> > ---
> >  include/net/bluetooth/hci.h      |    5 ++---
> >  include/net/bluetooth/hci_core.h |    3 +++
> >  net/bluetooth/hci_conn.c         |   30 ++++++++++++++++++++++++++++++
> >  3 files changed, 35 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index b326240..d04ecea 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -191,6 +191,8 @@ enum {
> >  
> >  #define LMP_EV4		0x01
> >  #define LMP_EV5		0x02
> > +#define LMP_NO_BR	0x20
> > +#define LMP_LE		0x40
> 
> Keep these in sync with the constants we use in userspace.

I changed these to match userspace ones.

> >  #define LMP_SNIFF_SUBR	0x02
> >  #define LMP_EDR_ESCO_2M	0x20
> > @@ -627,9 +629,6 @@ struct hci_cp_le_create_conn {
> >  } __packed;
> >  
> >  #define HCI_OP_LE_CREATE_CONN_CANCEL	0x200e
> > -struct hci_cp_le_create_conn_cancel {
> > -	__u8     status;
> > -} __packed;
> >  
> >  #define HCI_OP_LE_SET_ADVERTISE_ENABLE	0x200a
> >  	#define LE_ADVERTISE_ENABLED	0x01
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 89f4b10..a430a57 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -455,10 +455,13 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> >  #define lmp_rswitch_capable(dev)   ((dev)->features[0] & LMP_RSWITCH)
> >  #define lmp_encrypt_capable(dev)   ((dev)->features[0] & LMP_ENCRYPT)
> >  #define lmp_sniff_capable(dev)     ((dev)->features[0] & LMP_SNIFF)
> > +#define lmp_br_capable(dev)        (!((dev)->features[4] & LMP_NO_BR))
> 
> This makes no sense to me. And leave this out for now. This is more
> complicated when running on LE only.

ok

> > +#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
> 
> I would just add this at the end of the list and not intermix it with
> sniff and sniffsubr defines.
> 
> >  #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
> >  #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
> >  #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
> >  
> > +
> >  /* ----- HCI protocols ----- */
> >  struct hci_proto {
> >  	char		*name;
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index cb41d64..50f8973 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -66,6 +66,31 @@ void hci_le_connect(struct hci_conn *conn)
> >  	hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> >  }
> >  
> > +static void hci_le_connect_cancel(struct hci_conn *conn)
> > +{
> > +	struct hci_dev *hdev = conn->hdev;
> > +
> > +	BT_DBG("%p", conn);
> > +
> > +	if (!lmp_le_capable(hdev))
> > +		return;
> 
> This should not be needed. We should not have tried to establish a LE
> link if we don't support LE in the first place.

ok

> > +
> > +	hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
> > +}
> > +
> > +void hci_le_disconn(struct hci_conn *conn, __u8 reason)
> > +{
> > +	struct hci_cp_disconnect cp;
> > +
> > +	BT_DBG("%p", conn);
> > +
> > +	conn->le_state = BT_DISCONN;
> > +
> > +	cp.handle = cpu_to_le16(conn->handle);
> > +	cp.reason = reason;
> > +	hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp);
> > +}
> 
> When just using conn->state, then this becomes obsolete and we can use
> the generic one.

I removed whole function.

> > +
> >  void hci_acl_connect(struct hci_conn *conn)
> >  {
> >  	struct hci_dev *hdev = conn->hdev;
> > @@ -221,6 +246,8 @@ static void hci_conn_timeout(unsigned long arg)
> >  	case BT_CONNECT2:
> >  		if (conn->type == ACL_LINK && conn->out)
> >  			hci_acl_connect_cancel(conn);
> > +		if (conn->type == LE_LINK && conn->out)
> > +			hci_le_connect_cancel(conn);
> 
> This should be redone with as this:
> 
> 	if (conn->out) {
> 		if (ACL_LINK)
> 			...
> 		else if (LE_LINK)
> 			...
> 	}

Done

> >  		break;
> >  	case BT_CONFIG:
> >  	case BT_CONNECTED:
> > @@ -397,6 +424,9 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
> >  	BT_DBG("%s dst %s", hdev->name, batostr(dst));
> >  
> >  	if (type == LE_LINK) {
> > +		if (!lmp_le_capable(hdev))
> > +			return NULL;
> > +
> >  		le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
> >  
> >  		if (!le)
> 
> We might need to move that lmp_le_capable check into L2CAP. Since
> otherwise we can not give a proper return value if someone tries to use
> LE on a BR/EDR only controller.

Check removed from from here. However I left lmp_le_capable() to code so it can
be used in l2cap patches.


-- 
Ville

[PATCH 4/7] Bluetooth: Use LE buffers for LE traffic

[Ville Tervo]

BLuetooth chips may have separate buffers for
LE traffic. This patch add support to use LE
buffers provided by the chip.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/hci_core.h |    6 +++
 net/bluetooth/hci_conn.c         |   11 +++++-
 net/bluetooth/hci_core.c         |   77 +++++++++++++++++++++++++++++++++++--
 net/bluetooth/hci_event.c        |   34 ++++++++++++++++-
 4 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a430a57..2eb314f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -102,15 +102,19 @@ struct hci_dev {
 	atomic_t	cmd_cnt;
 	unsigned int	acl_cnt;
 	unsigned int	sco_cnt;
+	unsigned int	le_cnt;
 
 	unsigned int	acl_mtu;
 	unsigned int	sco_mtu;
+	unsigned int	le_mtu;
 	unsigned int	acl_pkts;
 	unsigned int	sco_pkts;
+	unsigned int	le_pkts;
 
 	unsigned long	cmd_last_tx;
 	unsigned long	acl_last_tx;
 	unsigned long	sco_last_tx;
+	unsigned long	le_last_tx;
 
 	struct workqueue_struct	*workqueue;
 
@@ -358,6 +362,8 @@ void hci_conn_enter_sniff_mode(struct hci_conn *conn);
 void hci_conn_hold_device(struct hci_conn *conn);
 void hci_conn_put_device(struct hci_conn *conn);
 
+void hci_le_disconn(struct hci_conn *conn, __u8 reason);
+
 static inline void hci_conn_hold(struct hci_conn *conn)
 {
 	atomic_inc(&conn->refcnt);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 50f8973..3443065 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -232,6 +232,7 @@ static void hci_conn_timeout(unsigned long arg)
 {
 	struct hci_conn *conn = (void *) arg;
 	struct hci_dev *hdev = conn->hdev;
+	__u16 state;
 	__u8 reason;
 
 	BT_DBG("conn %p state %d", conn, conn->state);
@@ -241,7 +242,12 @@ static void hci_conn_timeout(unsigned long arg)
 
 	hci_dev_lock(hdev);
 
-	switch (conn->state) {
+	if (conn->type == LE_LINK)
+		state = conn->le_state;
+	else
+		state = conn->state;
+
+	switch (state) {
 	case BT_CONNECT:
 	case BT_CONNECT2:
 		if (conn->type == ACL_LINK && conn->out)
@@ -348,6 +354,8 @@ int hci_conn_del(struct hci_conn *conn)
 
 		/* Unacked frames */
 		hdev->acl_cnt += conn->sent;
+	} else if (conn->type == LE_LINK) {
+		hdev->le_cnt += conn->sent;
 	} else {
 		struct hci_conn *acl = conn->link;
 		if (acl) {
@@ -436,6 +444,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 			return NULL;
 
 		hci_le_connect(le);
+		hci_conn_hold(le);
 
 		return le;
 	}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bc2a052..b320798 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -254,6 +254,14 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
 	hci_send_cmd(hdev, HCI_OP_WRITE_CA_TIMEOUT, 2, &param);
 }
 
+static void hci_le_init_req(struct hci_dev *hdev, unsigned long opt)
+{
+	BT_DBG("%s", hdev->name);
+
+	/* Read LE buffer size */
+	hci_send_cmd(hdev, HCI_OP_LE_READ_BUFFER_SIZE, 0, NULL);
+}
+
 static void hci_scan_req(struct hci_dev *hdev, unsigned long opt)
 {
 	__u8 scan = opt;
@@ -509,6 +517,10 @@ int hci_dev_open(__u16 dev)
 		ret = __hci_request(hdev, hci_init_req, 0,
 					msecs_to_jiffies(HCI_INIT_TIMEOUT));
 
+		if (lmp_le_capable(hdev))
+			ret = __hci_request(hdev, hci_le_init_req, 0,
+					msecs_to_jiffies(HCI_INIT_TIMEOUT));
+
 		clear_bit(HCI_INIT, &hdev->flags);
 	}
 
@@ -645,7 +657,7 @@ int hci_dev_reset(__u16 dev)
 		hdev->flush(hdev);
 
 	atomic_set(&hdev->cmd_cnt, 1);
-	hdev->acl_cnt = 0; hdev->sco_cnt = 0;
+	hdev->acl_cnt = 0; hdev->sco_cnt = 0; hdev->le_cnt = 0;
 
 	if (!test_bit(HCI_RAW, &hdev->flags))
 		ret = __hci_request(hdev, hci_reset_req, 0,
@@ -1444,7 +1456,8 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int
 		if (c->type != type || skb_queue_empty(&c->data_q))
 			continue;
 
-		if (c->state != BT_CONNECTED && c->state != BT_CONFIG)
+		if ((c->state != BT_CONNECTED && c->state != BT_CONFIG) &&
+		    (c->le_state != BT_CONNECTED && c->le_state != BT_CONFIG))
 			continue;
 
 		num++;
@@ -1456,8 +1469,25 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int
 	}
 
 	if (conn) {
-		int cnt = (type == ACL_LINK ? hdev->acl_cnt : hdev->sco_cnt);
-		int q = cnt / num;
+		int cnt, q;
+
+		switch (conn->type) {
+		case ACL_LINK:
+			cnt = hdev->acl_cnt;
+			break;
+		case SCO_LINK:
+		case ESCO_LINK:
+			cnt = hdev->sco_cnt;
+			break;
+		case LE_LINK:
+			cnt = hdev->le_mtu ? hdev->le_cnt : hdev->acl_cnt;
+			break;
+		default:
+			cnt = 0;
+			BT_ERR("Unknown link type");
+		}
+
+		q = cnt / num;
 		*quote = q ? q : 1;
 	} else
 		*quote = 0;
@@ -1482,6 +1512,11 @@ static inline void hci_acl_tx_to(struct hci_dev *hdev)
 				hdev->name, batostr(&c->dst));
 			hci_acl_disconn(c, 0x13);
 		}
+		if (c->type == LE_LINK && c->sent) {
+			BT_ERR("%s killing stalled LE connection %s",
+				hdev->name, batostr(&c->dst));
+			hci_le_disconn(c, 0x13);
+		}
 	}
 }
 
@@ -1556,6 +1591,35 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
 	}
 }
 
+static inline void hci_sched_le(struct hci_dev *hdev)
+{
+	struct hci_conn *conn;
+	struct sk_buff *skb;
+	int quote;
+
+	BT_DBG("%s", hdev->name);
+
+	if (!test_bit(HCI_RAW, &hdev->flags)) {
+		/* ACL tx timeout must be longer than maximum
+		 * link supervision timeout (40.9 seconds) */
+		if (!hdev->le_cnt &&
+		    time_after(jiffies, hdev->le_last_tx + HZ * 45))
+			hci_acl_tx_to(hdev);
+	}
+
+	while (hdev->le_cnt && (conn = hci_low_sent(hdev, LE_LINK, &quote))) {
+		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
+			BT_DBG("skb %p len %d", skb, skb->len);
+
+			hci_send_frame(skb);
+			hdev->le_last_tx = jiffies;
+
+			hdev->le_cnt--;
+			conn->sent++;
+		}
+	}
+}
+
 static void hci_tx_task(unsigned long arg)
 {
 	struct hci_dev *hdev = (struct hci_dev *) arg;
@@ -1563,7 +1627,8 @@ static void hci_tx_task(unsigned long arg)
 
 	read_lock(&hci_task_lock);
 
-	BT_DBG("%s acl %d sco %d", hdev->name, hdev->acl_cnt, hdev->sco_cnt);
+	BT_DBG("%s acl %d sco %d le %d", hdev->name, hdev->acl_cnt,
+		hdev->sco_cnt, hdev->le_cnt);
 
 	/* Schedule queues and send stuff to HCI driver */
 
@@ -1573,6 +1638,8 @@ static void hci_tx_task(unsigned long arg)
 
 	hci_sched_esco(hdev);
 
+	hci_sched_le(hdev);
+
 	/* Send next queued raw (unknown type) packet */
 	while ((skb = skb_dequeue(&hdev->raw_q)))
 		hci_send_frame(skb);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0b979ae..a914314 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -539,6 +539,26 @@ static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_req_complete(hdev, rp->status);
 }
 
+static void hci_cc_le_read_buffer_size(struct hci_dev *hdev,
+				       struct sk_buff *skb)
+{
+	struct hci_rp_le_read_buffer_size *rp = (void *) skb->data;
+
+	BT_DBG("%s status 0x%x", hdev->name, rp->status);
+
+	if (rp->status)
+		return;
+
+	hdev->le_mtu = __le16_to_cpu(rp->le_mtu);
+	hdev->le_pkts = rp->le_max_pkt;
+
+	hdev->le_cnt = hdev->le_pkts;
+
+	BT_DBG("%s le mtu %d:%d", hdev->name, hdev->le_mtu, hdev->le_pkts);
+
+	hci_req_complete(hdev, rp->status);
+}
+
 static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 {
 	BT_DBG("%s status 0x%x", hdev->name, status);
@@ -1397,6 +1417,10 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
 		hci_cc_read_bd_addr(hdev, skb);
 		break;
 
+	case HCI_OP_LE_READ_BUFFER_SIZE:
+		hci_cc_le_read_buffer_size(hdev, skb);
+		break;
+
 	case HCI_OP_LE_SET_ADVERTISE_ENABLE:
 		hci_cc_le_set_advertise(hdev, skb);
 		break;
@@ -1542,10 +1566,16 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
 			conn->sent -= count;
 
 			if (conn->type == ACL_LINK) {
-				if ((hdev->acl_cnt += count) > hdev->acl_pkts)
+				hdev->acl_cnt += count;
+				if (hdev->acl_cnt > hdev->acl_pkts)
 					hdev->acl_cnt = hdev->acl_pkts;
+			} else if (conn->type == LE_LINK) {
+				hdev->le_cnt += count;
+				if (hdev->le_cnt > hdev->le_pkts)
+					hdev->le_cnt = hdev->le_pkts;
 			} else {
-				if ((hdev->sco_cnt += count) > hdev->sco_pkts)
+				hdev->sco_cnt += count;
+				if (hdev->sco_cnt > hdev->sco_pkts)
 					hdev->sco_cnt = hdev->sco_pkts;
 			}
 		}
-- 
1.7.0.1

Re: [PATCH 4/7] Bluetooth: Use LE buffers for LE traffic

[Marcel Holtmann]

Hi Ville,

> BLuetooth chips may have separate buffers for
> LE traffic. This patch add support to use LE
> buffers provided by the chip.
> 
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
>  include/net/bluetooth/hci_core.h |    6 +++
>  net/bluetooth/hci_conn.c         |   11 +++++-
>  net/bluetooth/hci_core.c         |   77 +++++++++++++++++++++++++++++++++++--
>  net/bluetooth/hci_event.c        |   34 ++++++++++++++++-
>  4 files changed, 120 insertions(+), 8 deletions(-)

this whole patch needs some re-thinking and proper LE init phase for
combo chips where BR/EDR and LE can share the buffers.

Regards

Marcel

[PATCH 5/7] Bluetooth: Add LE connection support to L2CAP

[Ville Tervo]

Add basic LE connection support to L2CAP

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/l2cap.h |    4 ++++
 net/bluetooth/l2cap_core.c    |   32 ++++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index df599dc..41374ad 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -160,6 +160,9 @@ struct l2cap_conn_rsp {
 /* channel indentifier */
 #define L2CAP_CID_SIGNALING	0x0001
 #define L2CAP_CID_CONN_LESS	0x0002
+#define L2CAP_CID_LE_DATA	0x0004
+#define L2CAP_CID_LE_SIGNALING	0x0005
+#define L2CAP_CID_SMP		0x0006
 #define L2CAP_CID_DYN_START	0x0040
 #define L2CAP_CID_DYN_END	0xffff
 
@@ -269,6 +272,7 @@ struct l2cap_conn {
 	bdaddr_t	*src;
 
 	unsigned int	mtu;
+	unsigned int	le_mtu;
 
 	__u32		feat_mask;
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 07b55c1..1d43280 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -588,6 +588,12 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 	for (sk = l->head; sk; sk = l2cap_pi(sk)->next_c) {
 		bh_lock_sock(sk);
 
+		if (conn->hcon->type == LE_LINK) {
+			l2cap_sock_clear_timer(sk);
+			sk->sk_state = BT_CONNECTED;
+			sk->sk_state_change(sk);
+		}
+
 		if (sk->sk_type != SOCK_SEQPACKET &&
 				sk->sk_type != SOCK_STREAM) {
 			l2cap_sock_clear_timer(sk);
@@ -646,7 +652,11 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)
 
 	BT_DBG("hcon %p conn %p", hcon, conn);
 
-	conn->mtu = hcon->hdev->acl_mtu;
+	if (hcon->hdev->le_mtu && hcon->type == LE_LINK)
+		conn->mtu = hcon->hdev->le_mtu;
+	else
+		conn->mtu = hcon->hdev->acl_mtu;
+
 	conn->src = &hcon->hdev->bdaddr;
 	conn->dst = &hcon->dst;
 
@@ -962,8 +972,13 @@ static int l2cap_do_connect(struct sock *sk)
 		}
 	}
 
-	hcon = hci_connect(hdev, ACL_LINK, dst,
+	if (l2cap_pi(sk)->dcid == L2CAP_CID_LE_DATA)
+		hcon = hci_connect(hdev, LE_LINK, dst,
+					l2cap_pi(sk)->sec_level, auth_type);
+	else
+		hcon = hci_connect(hdev, ACL_LINK, dst,
 					l2cap_pi(sk)->sec_level, auth_type);
+
 	if (!hcon)
 		goto done;
 
@@ -1014,13 +1029,13 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
 	len = min_t(unsigned int, sizeof(la), alen);
 	memcpy(&la, addr, len);
 
-	if (la.l2_cid)
++	if (la.l2_cid && la.l2_psm)
 		return -EINVAL;
 
 	lock_sock(sk);
 
 	if ((sk->sk_type == SOCK_SEQPACKET || sk->sk_type == SOCK_STREAM)
-			&& !la.l2_psm) {
+			&& !(la.l2_psm || la.la_cid)) {
 		err = -EINVAL;
 		goto done;
 	}
@@ -1062,14 +1077,15 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
 
 	/* PSM must be odd and lsb of upper byte must be 0 */
 	if ((__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001 &&
-		sk->sk_type != SOCK_RAW) {
+		sk->sk_type != SOCK_RAW && !la.l2_cid) {
 		err = -EINVAL;
 		goto done;
 	}
 
-	/* Set destination address and psm */
+	/* Set destination address and psm or cid */
 	bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr);
 	l2cap_pi(sk)->psm = la.l2_psm;
++	l2cap_pi(sk)->dcid = la.l2_cid;
 
 	err = l2cap_do_connect(sk);
 	if (err)
@@ -4380,7 +4396,7 @@ static int l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 
 	BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
 
-	if (hcon->type != ACL_LINK)
++	if (!(hcon->type == ACL_LINK || hcon->type == LE_LINK))
 		return -EINVAL;
 
 	if (!status) {
@@ -4409,7 +4425,7 @@ static int l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason)
 {
 	BT_DBG("hcon %p reason %d", hcon, reason);
 
-	if (hcon->type != ACL_LINK)
++	if (!(hcon->type == ACL_LINK || hcon->type == LE_LINK))
 		return -EINVAL;
 
 	l2cap_conn_del(hcon, bt_err(reason));
-- 
1.7.0.1

Re: [PATCH 5/7] Bluetooth: Add LE connection support to L2CAP

[Anderson Lizardo]

On Wed, Oct 6, 2010 at 2:42 PM, Ville Tervo <ville.tervo@nokia.com> wrote:
> @@ -1014,13 +1029,13 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
>        len = min_t(unsigned int, sizeof(la), alen);
>        memcpy(&la, addr, len);
>
> -       if (la.l2_cid)
> ++      if (la.l2_cid && la.l2_psm)
>                return -EINVAL;
>
>        lock_sock(sk);
>
>        if ((sk->sk_type == SOCK_SEQPACKET || sk->sk_type == SOCK_STREAM)
> -                       && !la.l2_psm) {
> +                       && !(la.l2_psm || la.la_cid)) {
>                err = -EINVAL;
>                goto done;
>        }

This snippet looks strange for two reasons:

1) the "++" marker looks weird. Corrupted patch?
2) there is a typo here : la.la_cid -> la.l2_cid . Didn't the compiler
catch this?

Note that there are some other occurrences of "++" in this patch.

Regards,
-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

[PATCH 6/7] Bluetooth: Add server socket support for LE connection

[Ville Tervo]

Add support for LE server sockets.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/hci_event.c     |   10 +++-
 net/bluetooth/l2cap_core.c    |  117 ++++++++++++++++++++++++++++++++++++-----
 3 files changed, 113 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 41374ad..e655b31 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -38,6 +38,7 @@
 #define L2CAP_DEFAULT_MAX_PDU_SIZE	1009    /* Sized for 3-DH5 packet */
 #define L2CAP_DEFAULT_ACK_TO		200
 #define L2CAP_LOCAL_BUSY_TRIES		12
+#define L2CAP_LE_DEFAULT_MTU		23
 
 #define L2CAP_CONN_TIMEOUT	(40000) /* 40 seconds */
 #define L2CAP_INFO_TIMEOUT	(4000)  /*  4 seconds */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a914314..fa945e9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1961,8 +1961,14 @@ static inline void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff
 
 	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
 
-	if (!conn)
-		goto unlock;
+	if (!conn) {
+		conn = hci_conn_add(hdev, LE_LINK, &ev->bdaddr);
+		if (!conn) {
+			BT_ERR("No memory for new connection");
+			hci_dev_unlock(hdev);
+			return;
+		}
+	}
 
 	if (ev->status) {
 		hci_proto_connect_cfm(conn, ev->status);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 1d43280..20be2f9 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -78,6 +78,8 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
 
 static int l2cap_ertm_data_rcv(struct sock *sk, struct sk_buff *skb);
 
+static void l2cap_le_conn_ready(struct l2cap_conn *conn);
+
 /* ---- L2CAP timers ---- */
 static void l2cap_sock_set_timer(struct sock *sk, long timeout)
 {
@@ -199,8 +201,16 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, struct so
 	l2cap_pi(sk)->conn = conn;
 
 	if (sk->sk_type == SOCK_SEQPACKET || sk->sk_type == SOCK_STREAM) {
-		/* Alloc CID for connection-oriented socket */
-		l2cap_pi(sk)->scid = l2cap_alloc_cid(l);
+		if (conn->hcon->type == LE_LINK) {
+			/* LE connection */
+			l2cap_pi(sk)->omtu = L2CAP_LE_DEFAULT_MTU;
+			l2cap_pi(sk)->scid = L2CAP_CID_LE_DATA;
+			l2cap_pi(sk)->dcid = L2CAP_CID_LE_DATA;
+		} else {
+			/* Alloc CID for connection-oriented socket */
+			l2cap_pi(sk)->scid = l2cap_alloc_cid(l);
+			l2cap_pi(sk)->omtu = L2CAP_DEFAULT_MTU;
+		}
 	} else if (sk->sk_type == SOCK_DGRAM) {
 		/* Connectionless socket */
 		l2cap_pi(sk)->scid = L2CAP_CID_CONN_LESS;
@@ -583,15 +593,18 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 
 	BT_DBG("conn %p", conn);
 
+	if (!conn->hcon->out && conn->hcon->type == LE_LINK)
+		l2cap_le_conn_ready(conn);
+
 	read_lock(&l->lock);
 
 	for (sk = l->head; sk; sk = l2cap_pi(sk)->next_c) {
 		bh_lock_sock(sk);
 
 		if (conn->hcon->type == LE_LINK) {
-			l2cap_sock_clear_timer(sk);
-			sk->sk_state = BT_CONNECTED;
-			sk->sk_state_change(sk);
+				l2cap_sock_clear_timer(sk);
+				sk->sk_state = BT_CONNECTED;
+				sk->sk_state_change(sk);
 		}
 
 		if (sk->sk_type != SOCK_SEQPACKET &&
@@ -665,7 +678,8 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)
 	spin_lock_init(&conn->lock);
 	rwlock_init(&conn->chan_list.lock);
 
-	setup_timer(&conn->info_timer, l2cap_info_timeout,
+	if (hcon->type != LE_LINK)
+		setup_timer(&conn->info_timer, l2cap_info_timeout,
 						(unsigned long) conn);
 
 	conn->disc_reason = 0x13;
@@ -759,6 +773,37 @@ static inline struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t
 	return s;
 }
 
+static inline struct sock *l2cap_get_sock_by_cid(int state, __le16 cid, bdaddr_t *src)
+{
+	struct sock *s;
+	struct sock *sk = NULL, *sk1 = NULL;
+	struct hlist_node *node;
+
+	read_lock(&l2cap_sk_list.lock);
+	sk_for_each(sk, node, &l2cap_sk_list.head) {
+		if (state && sk->sk_state != state)
+			continue;
+
+		if (l2cap_pi(sk)->dcid == cid) {
+			/* Exact match. */
+			if (!bacmp(&bt_sk(sk)->src, src))
+				break;
+
+			/* Closest match */
+			if (!bacmp(&bt_sk(sk)->src, BDADDR_ANY))
+				sk1 = sk;
+		}
+	}
+
+	s = node ? sk : sk1;
+
+	if (s)
+		bh_lock_sock(s);
+	read_unlock(&l2cap_sk_list.lock);
+
+	return s;
+}
+
 static void l2cap_sock_cleanup_listen(struct sock *parent)
 {
 	struct sock *sk;
@@ -868,7 +913,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
 	len = min_t(unsigned int, sizeof(la), alen);
 	memcpy(&la, addr, len);
 
-	if (la.l2_cid)
+	if (la.l2_cid && la.l2_psm)
 		return -EINVAL;
 
 	lock_sock(sk);
@@ -910,6 +955,9 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
 			l2cap_pi(sk)->sec_level = BT_SECURITY_SDP;
 	}
 
+	if (la.l2_cid)
+		l2cap_pi(sk)->dcid = la.l2_cid;
+
 	write_unlock_bh(&l2cap_sk_list.lock);
 
 done:
@@ -1029,13 +1077,13 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
 	len = min_t(unsigned int, sizeof(la), alen);
 	memcpy(&la, addr, len);
 
-+	if (la.l2_cid && la.l2_psm)
+	if (la.l2_cid && la.l2_psm)
 		return -EINVAL;
 
 	lock_sock(sk);
 
 	if ((sk->sk_type == SOCK_SEQPACKET || sk->sk_type == SOCK_STREAM)
-			&& !(la.l2_psm || la.la_cid)) {
+			&& !(la.l2_psm || la.l2_cid)) {
 		err = -EINVAL;
 		goto done;
 	}
@@ -1085,7 +1133,7 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
 	/* Set destination address and psm or cid */
 	bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr);
 	l2cap_pi(sk)->psm = la.l2_psm;
-+	l2cap_pi(sk)->dcid = la.l2_cid;
+	l2cap_pi(sk)->dcid = la.l2_cid;
 
 	err = l2cap_do_connect(sk);
 	if (err)
@@ -1127,7 +1175,7 @@ static int l2cap_sock_listen(struct socket *sock, int backlog)
 		goto done;
 	}
 
-	if (!l2cap_pi(sk)->psm) {
+	if (!l2cap_pi(sk)->psm && !l2cap_pi(sk)->dcid) {
 		bdaddr_t *src = &bt_sk(sk)->src;
 		u16 psm;
 
@@ -1237,6 +1285,49 @@ static int l2cap_sock_getname(struct socket *sock, struct sockaddr *addr, int *l
 	return 0;
 }
 
+static void l2cap_le_conn_ready(struct l2cap_conn *conn)
+{
+	struct l2cap_chan_list *list = &conn->chan_list;
+	struct sock *parent, *uninitialized_var(sk);
+
+	BT_DBG("");
+
+	/* Check if we have socket listening on cid */
+	parent = l2cap_get_sock_by_cid(BT_LISTEN, 0x04, conn->src);
+	if (!parent)
+		goto clean;
+
+	/* Check for backlog size */
+	if (sk_acceptq_is_full(parent)) {
+		BT_DBG("backlog full %d", parent->sk_ack_backlog);
+		goto clean;
+	}
+
+	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, GFP_ATOMIC);
+	if (!sk)
+		goto clean;
+
+	write_lock_bh(&list->lock);
+
+	hci_conn_hold(conn->hcon);
+
+	l2cap_sock_init(sk, parent);
+	bacpy(&bt_sk(sk)->src, conn->src);
+	bacpy(&bt_sk(sk)->dst, conn->dst);
+
+	__l2cap_chan_add(conn, sk, parent);
+
+	l2cap_sock_set_timer(sk, sk->sk_sndtimeo);
+
+	sk->sk_state = BT_CONNECTED;
+	parent->sk_data_ready(parent, 0);
+
+	write_unlock_bh(&list->lock);
+
+clean:
+	bh_unlock_sock(parent);
+}
+
 static int __l2cap_wait_ack(struct sock *sk)
 {
 	DECLARE_WAITQUEUE(wait, current);
@@ -4396,7 +4487,7 @@ static int l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 
 	BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
 
-+	if (!(hcon->type == ACL_LINK || hcon->type == LE_LINK))
+	if (!(hcon->type == ACL_LINK || hcon->type == LE_LINK))
 		return -EINVAL;
 
 	if (!status) {
@@ -4425,7 +4516,7 @@ static int l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason)
 {
 	BT_DBG("hcon %p reason %d", hcon, reason);
 
-+	if (!(hcon->type == ACL_LINK || hcon->type == LE_LINK))
+	if (!(hcon->type == ACL_LINK || hcon->type == LE_LINK))
 		return -EINVAL;
 
 	l2cap_conn_del(hcon, bt_err(reason));
-- 
1.7.0.1

Re: [PATCH 6/7] Bluetooth: Add server socket support for LE connection

[Anderson Lizardo]

On Wed, Oct 6, 2010 at 2:42 PM, Ville Tervo <ville.tervo@nokia.com> wrote:
>                if (conn->hcon->type == LE_LINK) {
> -                       l2cap_sock_clear_timer(sk);
> -                       sk->sk_state = BT_CONNECTED;
> -                       sk->sk_state_change(sk);
> +                               l2cap_sock_clear_timer(sk);
> +                               sk->sk_state = BT_CONNECTED;
> +                               sk->sk_state_change(sk);
>                }
>
>                if (sk->sk_type != SOCK_SEQPACKET &&

Unrelated indentation above.

> @@ -1029,13 +1077,13 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
>        len = min_t(unsigned int, sizeof(la), alen);
>        memcpy(&la, addr, len);
>
> -+      if (la.l2_cid && la.l2_psm)
> +       if (la.l2_cid && la.l2_psm)
>                return -EINVAL;
>
>        lock_sock(sk);
>
>        if ((sk->sk_type == SOCK_SEQPACKET || sk->sk_type == SOCK_STREAM)
> -                       && !(la.l2_psm || la.la_cid)) {
> +                       && !(la.l2_psm || la.l2_cid)) {
>                err = -EINVAL;
>                goto done;
>        }

These typos were introduced in a previous patch. You should fix there instead.

-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

[PATCH 7/7] Bluetooth: Do not send disconn comand over LE links

[Ville Tervo]

l2cap over LE links can be disconnected without sending
disconnect command first.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 net/bluetooth/l2cap_core.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 20be2f9..53860f6 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -836,6 +836,8 @@ void l2cap_sock_kill(struct sock *sk)
 
 void __l2cap_sock_close(struct sock *sk, int reason)
 {
+	struct l2cap_conn *conn;
+
 	BT_DBG("sk %p state %d socket %p", sk, sk->sk_state, sk->sk_socket);
 
 	switch (sk->sk_state) {
@@ -845,8 +847,10 @@ void __l2cap_sock_close(struct sock *sk, int reason)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sk->sk_type == SOCK_SEQPACKET ||
-				sk->sk_type == SOCK_STREAM) {
+		conn = l2cap_pi(sk)->conn;
+		if ((sk->sk_type == SOCK_SEQPACKET ||
+				sk->sk_type == SOCK_STREAM) &&
+				conn->hcon->type != LE_LINK) {
 			struct l2cap_conn *conn = l2cap_pi(sk)->conn;
 
 			l2cap_sock_set_timer(sk, sk->sk_sndtimeo);
@@ -856,9 +860,10 @@ void __l2cap_sock_close(struct sock *sk, int reason)
 		break;
 
 	case BT_CONNECT2:
-		if (sk->sk_type == SOCK_SEQPACKET ||
-				sk->sk_type == SOCK_STREAM) {
-			struct l2cap_conn *conn = l2cap_pi(sk)->conn;
+		conn = l2cap_pi(sk)->conn;
+		if ((sk->sk_type == SOCK_SEQPACKET ||
+				sk->sk_type == SOCK_STREAM) &&
+				conn->hcon->type != LE_LINK) {
 			struct l2cap_conn_rsp rsp;
 			__u16 result;
 
-- 
1.7.0.1