[PATCH 00/17] Bluetooth: Implement Three-wire UART HCI driver

[PATCH 00/17] Bluetooth: Implement Three-wire UART HCI driver

[Johan Hedberg]

Hi,

This set of patches adds basic support for the Three-wire UART HCI
protocol, aka H5, one of the standard HCI transports in the core spec.
If anyone has hardware that supports it the driver can be tested with
hciattach from BlueZ 4.100 or later: "hciattach /dev/ttyX 3wire"

The code is partly based on the existing hci_bcsp.c (since the BCSP
protocol is quite similar to Three-wire) as well as the (very
unpolished) patch-set from Mark Mendelsohn that was sent to lkml last
August (which was also largely based on hci_bcsp.c).

Johan

[PATCH 01/17] Bluetooth: Initial skeleton for Three-wire UART (H5) support

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds the initial skeleton for Three-wire UART (H5) support
and hooks it up to the HCI UART framework.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/Kconfig     |   12 ++++++
 drivers/bluetooth/Makefile    |    1 +
 drivers/bluetooth/hci_h5.c    |   88 +++++++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/hci_ldisc.c |    6 +++
 drivers/bluetooth/hci_uart.h  |    5 +++
 5 files changed, 112 insertions(+)
 create mode 100644 drivers/bluetooth/hci_h5.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 5ccf142..e9f203e 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -81,6 +81,18 @@ config BT_HCIUART_LL
 
 	  Say Y here to compile support for HCILL protocol.
 
+config BT_HCIUART_3WIRE
+	bool "Three-wire UART (H5) protocol support"
+	depends on BT_HCIUART
+	help
+	  The HCI Three-wire UART Transport Layer makes it possible to
+	  user the Bluetooth HCI over a serial port interface. The HCI
+	  Three-wire UART Transport Layer assumes that the UART
+	  communication may have bit errors, overrun errors or burst
+	  errors and thereby making CTS/RTS lines unnecessary.
+
+	  Say Y here to compile support for Three-wire UART protocol.
+
 config BT_HCIBCM203X
 	tristate "HCI BCM203x USB driver"
 	depends on USB
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index f4460f4..4afae20 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -28,4 +28,5 @@ hci_uart-$(CONFIG_BT_HCIUART_H4)	+= hci_h4.o
 hci_uart-$(CONFIG_BT_HCIUART_BCSP)	+= hci_bcsp.o
 hci_uart-$(CONFIG_BT_HCIUART_LL)	+= hci_ll.o
 hci_uart-$(CONFIG_BT_HCIUART_ATH3K)	+= hci_ath.o
+hci_uart-$(CONFIG_BT_HCIUART_3WIRE)	+= hci_h5.o
 hci_uart-objs				:= $(hci_uart-y)
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
new file mode 100644
index 0000000..6353d00
--- /dev/null
+++ b/drivers/bluetooth/hci_h5.c
@@ -0,0 +1,88 @@
+/*
+ *
+ *  Bluetooth HCI Three-wire UART driver
+ *
+ *  Copyright (C) 2012  Intel Corporation
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "hci_uart.h"
+
+static int h5_open(struct hci_uart *hu)
+{
+	return -ENOSYS;
+}
+
+static int h5_close(struct hci_uart *hu)
+{
+	return -ENOSYS;
+}
+
+static int h5_recv(struct hci_uart *hu, void *data, int count)
+{
+	return -ENOSYS;
+}
+
+static int h5_enqueue(struct hci_uart *hu, struct sk_buff *skb)
+{
+	return -ENOSYS;
+}
+
+static struct sk_buff *h5_dequeue(struct hci_uart *hu)
+{
+	return NULL;
+}
+
+static int h5_flush(struct hci_uart *hu)
+{
+	return -ENOSYS;
+}
+
+static struct hci_uart_proto h5p = {
+	.id		= HCI_UART_3WIRE,
+	.open		= h5_open,
+	.close		= h5_close,
+	.recv		= h5_recv,
+	.enqueue	= h5_enqueue,
+	.dequeue	= h5_dequeue,
+	.flush		= h5_flush,
+};
+
+int __init h5_init(void)
+{
+	int err = hci_uart_register_proto(&h5p);
+
+	if (!err)
+		BT_INFO("HCI Three-wire UART (H5) protocol initialized");
+	else
+		BT_ERR("HCI Three-wire UART (H5) protocol init failed");
+
+	return err;
+}
+
+int __exit h5_deinit(void)
+{
+	return hci_uart_unregister_proto(&h5p);
+}
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 2f9b796..142f49c 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -558,6 +558,9 @@ static int __init hci_uart_init(void)
 #ifdef CONFIG_BT_HCIUART_ATH3K
 	ath_init();
 #endif
+#ifdef CONFIG_BT_HCIUART_3WIRE
+	h5_init();
+#endif
 
 	return 0;
 }
@@ -578,6 +581,9 @@ static void __exit hci_uart_exit(void)
 #ifdef CONFIG_BT_HCIUART_ATH3K
 	ath_deinit();
 #endif
+#ifdef CONFIG_BT_HCIUART_3WIRE
+	h5_deinit();
+#endif
 
 	/* Release tty registration of line discipline */
 	if ((err = tty_unregister_ldisc(N_HCI)))
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 6cf6ab22..aaf9d7d 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -104,3 +104,8 @@ int ll_deinit(void);
 int ath_init(void);
 int ath_deinit(void);
 #endif
+
+#ifdef CONFIG_BT_HCIUART_3WIRE
+int h5_init(void);
+int h5_deinit(void);
+#endif
-- 
1.7.10.4

[PATCH 02/17] Bluetooth: Add basic state tracking to Three-wire UART driver

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds basic state tracking and socket buffer handling to the
Three-wire UART (H5) HCI driver.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   98 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 6353d00..c610ba3 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -30,14 +30,50 @@
 
 #include "hci_uart.h"
 
+#define H5_TXWINSIZE 4
+
+struct h5 {
+	struct sk_buff_head unack;	/* Unack'ed packets queue */
+	struct sk_buff_head rel;	/* Reliable packets queue */
+	struct sk_buff_head unrel;	/* Unreliable packets queue */
+
+	struct sk_buff *rx_skb;
+
+	bool txack_req;
+
+	u8 msgq_txseq;
+};
+
 static int h5_open(struct hci_uart *hu)
 {
-	return -ENOSYS;
+	struct h5 *h5;
+
+	BT_DBG("hu %p", hu);
+
+	h5 = kzalloc(sizeof(*h5), GFP_KERNEL);
+	if (!h5)
+		return -ENOMEM;
+
+	hu->priv = h5;
+
+	skb_queue_head_init(&h5->unack);
+	skb_queue_head_init(&h5->rel);
+	skb_queue_head_init(&h5->unrel);
+
+	return 0;
 }
 
 static int h5_close(struct hci_uart *hu)
 {
-	return -ENOSYS;
+	struct h5 *h5 = hu->priv;
+
+	skb_queue_purge(&h5->unack);
+	skb_queue_purge(&h5->rel);
+	skb_queue_purge(&h5->unrel);
+
+	kfree(h5);
+
+	return 0;
 }
 
 static int h5_recv(struct hci_uart *hu, void *data, int count)
@@ -47,17 +83,71 @@ static int h5_recv(struct hci_uart *hu, void *data, int count)
 
 static int h5_enqueue(struct hci_uart *hu, struct sk_buff *skb)
 {
-	return -ENOSYS;
+	struct h5 *h5 = hu->priv;
+
+	if (skb->len > 0xfff) {
+		BT_ERR("Packet too long (%u bytes)", skb->len);
+		kfree_skb(skb);
+		return 0;
+	}
+
+	switch (bt_cb(skb)->pkt_type) {
+	case HCI_ACLDATA_PKT:
+	case HCI_COMMAND_PKT:
+		skb_queue_tail(&h5->rel, skb);
+		break;
+
+	case HCI_SCODATA_PKT:
+		skb_queue_tail(&h5->unrel, skb);
+		break;
+
+	default:
+		BT_ERR("Unknown packet type %u", bt_cb(skb)->pkt_type);
+		kfree_skb(skb);
+		break;
+	}
+
+	return 0;
+}
+
+static struct sk_buff *h5_prepare_pkt(struct h5 *h5, struct sk_buff *skb)
+{
+	h5->txack_req = false;
+	return NULL;
+}
+
+static struct sk_buff *h5_prepare_ack(struct h5 *h5)
+{
+	h5->txack_req = false;
+	return NULL;
 }
 
 static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 {
+	struct h5 *h5 = hu->priv;
+	struct sk_buff *skb, *nskb;
+
+	if ((skb = skb_dequeue(&h5->unrel)) != NULL) {
+		nskb = h5_prepare_pkt(h5, skb);
+		if (nskb) {
+			kfree_skb(skb);
+			return nskb;
+		}
+
+		skb_queue_head(&h5->unrel, skb);
+		BT_ERR("Could not dequeue pkt because alloc_skb failed");
+	}
+
+	if (h5->txack_req)
+		return h5_prepare_ack(h5);
+
 	return NULL;
 }
 
 static int h5_flush(struct hci_uart *hu)
 {
-	return -ENOSYS;
+	BT_DBG("hu %p", hu);
+	return 0;
 }
 
 static struct hci_uart_proto h5p = {
-- 
1.7.10.4

[PATCH 03/17] Bluetooth: Add initial reliable packet support for Three-wire UART

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds initial support for reliable packets along with the
necessary retransmission timer for the Three-wire UART HCI driver.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   52 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index c610ba3..0f97b05 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -39,11 +39,34 @@ struct h5 {
 
 	struct sk_buff *rx_skb;
 
+	struct timer_list timer;	/* Retransmission timer */
+
 	bool txack_req;
 
 	u8 msgq_txseq;
 };
 
+static void h5_timed_event(unsigned long arg)
+{
+	struct hci_uart *hu = (struct hci_uart *) arg;
+	struct h5 *h5 = hu->priv;
+	struct sk_buff *skb;
+	unsigned long flags;
+
+	BT_DBG("hu %p retransmitting %u pkts", hu, h5->unack.qlen);
+
+	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
+
+	while ((skb = __skb_dequeue_tail(&h5->unack)) != NULL) {
+		h5->msgq_txseq = (h5->msgq_txseq - 1) & 0x07;
+		skb_queue_head(&h5->rel, skb);
+	}
+
+	spin_unlock_irqrestore(&h5->unack.lock, flags);
+
+	hci_uart_tx_wakeup(hu);
+}
+
 static int h5_open(struct hci_uart *hu)
 {
 	struct h5 *h5;
@@ -60,6 +83,10 @@ static int h5_open(struct hci_uart *hu)
 	skb_queue_head_init(&h5->rel);
 	skb_queue_head_init(&h5->unrel);
 
+	init_timer(&h5->timer);
+	h5->timer.function = h5_timed_event;
+	h5->timer.data = (unsigned long) hu;
+
 	return 0;
 }
 
@@ -71,6 +98,8 @@ static int h5_close(struct hci_uart *hu)
 	skb_queue_purge(&h5->rel);
 	skb_queue_purge(&h5->unrel);
 
+	del_timer(&h5->timer);
+
 	kfree(h5);
 
 	return 0;
@@ -125,6 +154,7 @@ static struct sk_buff *h5_prepare_ack(struct h5 *h5)
 static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 {
 	struct h5 *h5 = hu->priv;
+	unsigned long flags;
 	struct sk_buff *skb, *nskb;
 
 	if ((skb = skb_dequeue(&h5->unrel)) != NULL) {
@@ -138,6 +168,28 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 		BT_ERR("Could not dequeue pkt because alloc_skb failed");
 	}
 
+	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
+
+	if (h5->unack.qlen >= H5_TXWINSIZE)
+	       goto unlock;
+
+	if ((skb = skb_dequeue(&h5->rel)) != NULL) {
+		nskb = h5_prepare_pkt(h5, skb);
+
+		if (nskb) {
+			__skb_queue_tail(&h5->unack, skb);
+			mod_timer(&h5->timer, jiffies + HZ / 4);
+			spin_unlock_irqrestore(&h5->unack.lock, flags);
+			return nskb;
+		}
+
+		skb_queue_head(&h5->rel, skb);
+		BT_ERR("Could not dequeue pkt because alloc_skb failed");
+	}
+
+unlock:
+	spin_unlock_irqrestore(&h5->unack.lock, flags);
+
 	if (h5->txack_req)
 		return h5_prepare_ack(h5);
 
-- 
1.7.10.4

Re: [PATCH 03/17] Bluetooth: Add initial reliable packet support for Three-wire UART

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-06-27 14:25:53 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds initial support for reliable packets along with the
> necessary retransmission timer for the Three-wire UART HCI driver.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  drivers/bluetooth/hci_h5.c |   52 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index c610ba3..0f97b05 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -39,11 +39,34 @@ struct h5 {
>  
>  	struct sk_buff *rx_skb;
>  
> +	struct timer_list timer;	/* Retransmission timer */
> +
>  	bool txack_req;
>  
>  	u8 msgq_txseq;
>  };
>  
> +static void h5_timed_event(unsigned long arg)
> +{
> +	struct hci_uart *hu = (struct hci_uart *) arg;
> +	struct h5 *h5 = hu->priv;
> +	struct sk_buff *skb;
> +	unsigned long flags;
> +
> +	BT_DBG("hu %p retransmitting %u pkts", hu, h5->unack.qlen);
> +
> +	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
> +
> +	while ((skb = __skb_dequeue_tail(&h5->unack)) != NULL) {
> +		h5->msgq_txseq = (h5->msgq_txseq - 1) & 0x07;
> +		skb_queue_head(&h5->rel, skb);
> +	}
> +
> +	spin_unlock_irqrestore(&h5->unack.lock, flags);
> +
> +	hci_uart_tx_wakeup(hu);
> +}
> +
>  static int h5_open(struct hci_uart *hu)
>  {
>  	struct h5 *h5;
> @@ -60,6 +83,10 @@ static int h5_open(struct hci_uart *hu)
>  	skb_queue_head_init(&h5->rel);
>  	skb_queue_head_init(&h5->unrel);
>  
> +	init_timer(&h5->timer);
> +	h5->timer.function = h5_timed_event;
> +	h5->timer.data = (unsigned long) hu;
> +
>  	return 0;
>  }
>  
> @@ -71,6 +98,8 @@ static int h5_close(struct hci_uart *hu)
>  	skb_queue_purge(&h5->rel);
>  	skb_queue_purge(&h5->unrel);
>  
> +	del_timer(&h5->timer);
> +
>  	kfree(h5);
>  
>  	return 0;
> @@ -125,6 +154,7 @@ static struct sk_buff *h5_prepare_ack(struct h5 *h5)
>  static struct sk_buff *h5_dequeue(struct hci_uart *hu)
>  {
>  	struct h5 *h5 = hu->priv;
> +	unsigned long flags;
>  	struct sk_buff *skb, *nskb;
>  
>  	if ((skb = skb_dequeue(&h5->unrel)) != NULL) {
> @@ -138,6 +168,28 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
>  		BT_ERR("Could not dequeue pkt because alloc_skb failed");
>  	}
>  
> +	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
> +
> +	if (h5->unack.qlen >= H5_TXWINSIZE)
> +	       goto unlock;
> +
> +	if ((skb = skb_dequeue(&h5->rel)) != NULL) {
> +		nskb = h5_prepare_pkt(h5, skb);
> +
> +		if (nskb) {
> +			__skb_queue_tail(&h5->unack, skb);
> +			mod_timer(&h5->timer, jiffies + HZ / 4);

Better use a milliseconds value here, and then turn it in jiffies. If
CONFIG_HZ is not set to 1000 we set the wrong value for the timer.

	Gustavo

[PATCH 04/17] Bluetooth: Add basic packet parsing to Three-wire UART driver

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds basic packet parsing to the Three-wire UART HCI driver
for packets received from the controller.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |  230 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 222 insertions(+), 8 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 0f97b05..3b33f90 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -32,20 +32,37 @@
 
 #define H5_TXWINSIZE 4
 
+/*
+ * Maximum Three-wire packet:
+ *     4 byte header + max value for 12-bit length + 2 bytes for CRC
+ */
+#define H5_MAX_LEN (4 + 0xfff + 2)
+
+#define SLIP_DELIMITER	0xc0
+#define SLIP_ESC	0xdb
+#define SLIP_ESC_DELIM	0xdc
+#define SLIP_ESC_ESC	0xdd
+
 struct h5 {
-	struct sk_buff_head unack;	/* Unack'ed packets queue */
-	struct sk_buff_head rel;	/* Reliable packets queue */
-	struct sk_buff_head unrel;	/* Unreliable packets queue */
+	struct sk_buff_head	unack;		/* Unack'ed packets queue */
+	struct sk_buff_head	rel;		/* Reliable packets queue */
+	struct sk_buff_head	unrel;		/* Unreliable packets queue */
 
-	struct sk_buff *rx_skb;
+	struct sk_buff		*rx_skb;	/* Receive buffer */
+	size_t			rx_pending;	/* Expecting more bytes */
+	bool			rx_esc;		/* SLIP escape mode */
 
-	struct timer_list timer;	/* Retransmission timer */
+	int			(*rx_func) (struct hci_uart *hu, u8 c);
 
-	bool txack_req;
+	struct timer_list	timer;		/* Retransmission timer */
 
-	u8 msgq_txseq;
+	bool			txack_req;
+
+	u8			msgq_txseq;
 };
 
+static void h5_reset_rx(struct h5 *h5);
+
 static void h5_timed_event(unsigned long arg)
 {
 	struct hci_uart *hu = (struct hci_uart *) arg;
@@ -83,6 +100,8 @@ static int h5_open(struct hci_uart *hu)
 	skb_queue_head_init(&h5->rel);
 	skb_queue_head_init(&h5->unrel);
 
+	h5_reset_rx(h5);
+
 	init_timer(&h5->timer);
 	h5->timer.function = h5_timed_event;
 	h5->timer.data = (unsigned long) hu;
@@ -105,9 +124,204 @@ static int h5_close(struct hci_uart *hu)
 	return 0;
 }
 
+static void h5_handle_internal_rx(struct hci_uart *hu)
+{
+	BT_DBG("%s", hu->hdev->name);
+}
+
+static void h5_complete_rx_pkt(struct hci_uart *hu)
+{
+	struct h5 *h5 = hu->priv;
+	u8 pkt_type;
+
+	BT_DBG("%s", hu->hdev->name);
+
+	pkt_type = h5->rx_skb->data[1] & 0x0f;
+
+	switch (pkt_type) {
+	case HCI_EVENT_PKT:
+	case HCI_ACLDATA_PKT:
+	case HCI_SCODATA_PKT:
+		bt_cb(h5->rx_skb)->pkt_type = pkt_type;
+
+		/* Remove Three-wire header */
+		skb_pull(h5->rx_skb, 4);
+
+		hci_recv_frame(h5->rx_skb);
+		h5->rx_skb = NULL;
+
+		break;
+
+	default:
+		h5_handle_internal_rx(hu);
+		break;
+	}
+
+	h5_reset_rx(h5);
+}
+
+static int h5_rx_crc(struct hci_uart *hu, unsigned char c)
+{
+	struct h5 *h5 = hu->priv;
+
+	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
+
+	h5_complete_rx_pkt(hu);
+	h5_reset_rx(h5);
+
+	return 0;
+}
+
+static int h5_rx_payload(struct hci_uart *hu, unsigned char c)
+{
+	struct h5 *h5 = hu->priv;
+	const unsigned char *hdr = h5->rx_skb->data;
+
+	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
+
+	if ((hdr[0] >> 4) & 0x01) {
+		h5->rx_func = h5_rx_crc;
+		h5->rx_pending = 2;
+	} else {
+		h5_complete_rx_pkt(hu);
+		h5_reset_rx(h5);
+	}
+
+	return 0;
+}
+
+static int h5_rx_3wire_hdr(struct hci_uart *hu, unsigned char c)
+{
+	struct h5 *h5 = hu->priv;
+	const unsigned char *hdr = h5->rx_skb->data;
+
+	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
+
+	if (((hdr[0] + hdr[1] + hdr[2] + hdr[3]) & 0xff) != 0xff) {
+		BT_ERR("Invalid header checksum");
+		h5_reset_rx(h5);
+		return 0;
+	}
+
+	h5->rx_func = h5_rx_payload;
+	h5->rx_pending = ((hdr[1] >> 4) & 0xff) + (hdr[2] << 4);
+
+	return 0;
+}
+
+static int h5_rx_pkt_start(struct hci_uart *hu, unsigned char c)
+{
+	struct h5 *h5 = hu->priv;
+
+	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
+
+	if (c == SLIP_DELIMITER)
+		return 1;
+
+	h5->rx_func = h5_rx_3wire_hdr;
+	h5->rx_pending = 4;
+
+	h5->rx_skb = bt_skb_alloc(H5_MAX_LEN, GFP_ATOMIC);
+	if (!h5->rx_skb) {
+		BT_ERR("Can't allocate mem for new packet");
+		h5_reset_rx(h5);
+		return -ENOMEM;
+	}
+
+	h5->rx_skb->dev = (void *) hu->hdev;
+
+	return 0;
+}
+
+static int h5_rx_delimiter(struct hci_uart *hu, unsigned char c)
+{
+	struct h5 *h5 = hu->priv;
+
+	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
+
+	if (c == SLIP_DELIMITER)
+		h5->rx_func = h5_rx_pkt_start;
+
+	return 1;
+}
+
+static void h5_unslip_one_byte(struct h5 *h5, unsigned char c)
+{
+	const u8 delim = SLIP_DELIMITER, esc = SLIP_ESC;
+	const u8 *byte = &c;
+
+	if (!h5->rx_esc && c == SLIP_ESC) {
+		h5->rx_esc = true;
+		return;
+	}
+
+	if (h5->rx_esc) {
+		switch (c) {
+		case SLIP_ESC_DELIM:
+			byte = &delim;
+			break;
+		case SLIP_ESC_ESC:
+			byte = &esc;
+			break;
+		default:
+			BT_ERR("Invalid esc byte 0x%02hhx", c);
+			h5_reset_rx(h5);
+			return;
+		}
+
+		h5->rx_esc = false;
+	}
+
+	memcpy(skb_put(h5->rx_skb, 1), byte, 1);
+	h5->rx_pending--;
+
+	BT_DBG("unsliped 0x%02hhx", *byte);
+}
+
+static void h5_reset_rx(struct h5 *h5)
+{
+	if (h5->rx_skb) {
+		kfree_skb(h5->rx_skb);
+		h5->rx_skb = NULL;
+	}
+
+	h5->rx_func = h5_rx_delimiter;
+	h5->rx_pending = 0;
+	h5->rx_esc = false;
+}
+
 static int h5_recv(struct hci_uart *hu, void *data, int count)
 {
-	return -ENOSYS;
+	struct h5 *h5 = hu->priv;
+	unsigned char *ptr = data;
+
+	BT_DBG("%s count %d", hu->hdev->name, count);
+
+	while (count > 0) {
+		int processed;
+
+		if (h5->rx_pending > 0) {
+			if (*ptr == SLIP_DELIMITER) {
+				BT_ERR("Too short H5 packet");
+				h5_reset_rx(h5);
+				continue;
+			}
+
+			h5_unslip_one_byte(h5, *ptr);
+
+			ptr++; count--;
+			continue;
+		}
+
+		processed = h5->rx_func(hu, *ptr);
+		if (processed < 0)
+			return processed;
+
+		ptr += processed;
+		count -= processed;
+	}
+
+	return 0;
 }
 
 static int h5_enqueue(struct hci_uart *hu, struct sk_buff *skb)
-- 
1.7.10.4

Re: [PATCH 04/17] Bluetooth: Add basic packet parsing to Three-wire UART driver

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-06-27 14:25:54 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds basic packet parsing to the Three-wire UART HCI driver
> for packets received from the controller.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  drivers/bluetooth/hci_h5.c |  230 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 222 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index 0f97b05..3b33f90 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -32,20 +32,37 @@
>  
>  #define H5_TXWINSIZE 4
>  
> +/*
> + * Maximum Three-wire packet:
> + *     4 byte header + max value for 12-bit length + 2 bytes for CRC
> + */
> +#define H5_MAX_LEN (4 + 0xfff + 2)
> +
> +#define SLIP_DELIMITER	0xc0
> +#define SLIP_ESC	0xdb
> +#define SLIP_ESC_DELIM	0xdc
> +#define SLIP_ESC_ESC	0xdd
> +
>  struct h5 {
> -	struct sk_buff_head unack;	/* Unack'ed packets queue */
> -	struct sk_buff_head rel;	/* Reliable packets queue */
> -	struct sk_buff_head unrel;	/* Unreliable packets queue */
> +	struct sk_buff_head	unack;		/* Unack'ed packets queue */
> +	struct sk_buff_head	rel;		/* Reliable packets queue */
> +	struct sk_buff_head	unrel;		/* Unreliable packets queue */
>  
> -	struct sk_buff *rx_skb;
> +	struct sk_buff		*rx_skb;	/* Receive buffer */
> +	size_t			rx_pending;	/* Expecting more bytes */
> +	bool			rx_esc;		/* SLIP escape mode */
>  
> -	struct timer_list timer;	/* Retransmission timer */
> +	int			(*rx_func) (struct hci_uart *hu, u8 c);
>  
> -	bool txack_req;
> +	struct timer_list	timer;		/* Retransmission timer */
>  
> -	u8 msgq_txseq;
> +	bool			txack_req;

Can we merge those bool items in a flags field?

> +
> +	u8			msgq_txseq;
>  };
>  
> +static void h5_reset_rx(struct h5 *h5);
> +
>  static void h5_timed_event(unsigned long arg)
>  {
>  	struct hci_uart *hu = (struct hci_uart *) arg;
> @@ -83,6 +100,8 @@ static int h5_open(struct hci_uart *hu)
>  	skb_queue_head_init(&h5->rel);
>  	skb_queue_head_init(&h5->unrel);
>  
> +	h5_reset_rx(h5);
> +
>  	init_timer(&h5->timer);
>  	h5->timer.function = h5_timed_event;
>  	h5->timer.data = (unsigned long) hu;
> @@ -105,9 +124,204 @@ static int h5_close(struct hci_uart *hu)
>  	return 0;
>  }
>  
> +static void h5_handle_internal_rx(struct hci_uart *hu)
> +{
> +	BT_DBG("%s", hu->hdev->name);
> +}
> +
> +static void h5_complete_rx_pkt(struct hci_uart *hu)
> +{
> +	struct h5 *h5 = hu->priv;
> +	u8 pkt_type;
> +
> +	BT_DBG("%s", hu->hdev->name);
> +
> +	pkt_type = h5->rx_skb->data[1] & 0x0f;
> +
> +	switch (pkt_type) {
> +	case HCI_EVENT_PKT:
> +	case HCI_ACLDATA_PKT:
> +	case HCI_SCODATA_PKT:
> +		bt_cb(h5->rx_skb)->pkt_type = pkt_type;
> +
> +		/* Remove Three-wire header */
> +		skb_pull(h5->rx_skb, 4);
> +
> +		hci_recv_frame(h5->rx_skb);
> +		h5->rx_skb = NULL;
> +
> +		break;
> +
> +	default:
> +		h5_handle_internal_rx(hu);
> +		break;
> +	}
> +
> +	h5_reset_rx(h5);
> +}
> +
> +static int h5_rx_crc(struct hci_uart *hu, unsigned char c)
> +{
> +	struct h5 *h5 = hu->priv;
> +
> +	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
> +
> +	h5_complete_rx_pkt(hu);
> +	h5_reset_rx(h5);
> +
> +	return 0;

There is no point in returning int here.

> +}
> +
> +static int h5_rx_payload(struct hci_uart *hu, unsigned char c)
> +{
> +	struct h5 *h5 = hu->priv;
> +	const unsigned char *hdr = h5->rx_skb->data;
> +
> +	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
> +
> +	if ((hdr[0] >> 4) & 0x01) {
> +		h5->rx_func = h5_rx_crc;
> +		h5->rx_pending = 2;
> +	} else {
> +		h5_complete_rx_pkt(hu);
> +		h5_reset_rx(h5);
> +	}
> +
> +	return 0;
> +}
> +
> +static int h5_rx_3wire_hdr(struct hci_uart *hu, unsigned char c)
> +{
> +	struct h5 *h5 = hu->priv;
> +	const unsigned char *hdr = h5->rx_skb->data;
> +
> +	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
> +
> +	if (((hdr[0] + hdr[1] + hdr[2] + hdr[3]) & 0xff) != 0xff) {
> +		BT_ERR("Invalid header checksum");
> +		h5_reset_rx(h5);
> +		return 0;
> +	}
> +
> +	h5->rx_func = h5_rx_payload;
> +	h5->rx_pending = ((hdr[1] >> 4) & 0xff) + (hdr[2] << 4);
> +
> +	return 0;
> +}
> +
> +static int h5_rx_pkt_start(struct hci_uart *hu, unsigned char c)
> +{
> +	struct h5 *h5 = hu->priv;
> +
> +	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
> +
> +	if (c == SLIP_DELIMITER)
> +		return 1;
> +
> +	h5->rx_func = h5_rx_3wire_hdr;
> +	h5->rx_pending = 4;
> +
> +	h5->rx_skb = bt_skb_alloc(H5_MAX_LEN, GFP_ATOMIC);
> +	if (!h5->rx_skb) {
> +		BT_ERR("Can't allocate mem for new packet");
> +		h5_reset_rx(h5);
> +		return -ENOMEM;
> +	}
> +
> +	h5->rx_skb->dev = (void *) hu->hdev;
> +
> +	return 0;
> +}
> +
> +static int h5_rx_delimiter(struct hci_uart *hu, unsigned char c)
> +{
> +	struct h5 *h5 = hu->priv;
> +
> +	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
> +
> +	if (c == SLIP_DELIMITER)
> +		h5->rx_func = h5_rx_pkt_start;
> +
> +	return 1;

Same here.

	Gustavo

[PATCH 05/17] Bluetooth: Add initial packet sending support to Three-wire UART

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds initial packed encoding and sending support to the
Three-wire UART HCI transport driver.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |  105 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 98 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 3b33f90..6b02ccc 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -30,6 +30,9 @@
 
 #include "hci_uart.h"
 
+#define HCI_3WIRE_ACK_PKT	0
+#define HCI_3WIRE_LINK_PKT	15
+
 #define H5_TXWINSIZE 4
 
 /*
@@ -58,7 +61,8 @@ struct h5 {
 
 	bool			txack_req;
 
-	u8			msgq_txseq;
+	u8			next_ack;
+	u8			next_seq;
 };
 
 static void h5_reset_rx(struct h5 *h5);
@@ -75,7 +79,7 @@ static void h5_timed_event(unsigned long arg)
 	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
 
 	while ((skb = __skb_dequeue_tail(&h5->unack)) != NULL) {
-		h5->msgq_txseq = (h5->msgq_txseq - 1) & 0x07;
+		h5->next_seq = (h5->next_seq - 1) & 0x07;
 		skb_queue_head(&h5->rel, skb);
 	}
 
@@ -353,10 +357,96 @@ static int h5_enqueue(struct hci_uart *hu, struct sk_buff *skb)
 	return 0;
 }
 
-static struct sk_buff *h5_prepare_pkt(struct h5 *h5, struct sk_buff *skb)
+static void h5_slip_delim(struct sk_buff *skb)
+{
+	const char delim = SLIP_DELIMITER;
+
+	memcpy(skb_put(skb, 1), &delim, 1);
+}
+
+static void h5_slip_one_byte(struct sk_buff *skb, u8 c)
+{
+	const char esc_delim[2] = { SLIP_ESC, SLIP_ESC_DELIM };
+	const char esc_esc[2] = { SLIP_ESC, SLIP_ESC_ESC };
+
+	switch (c) {
+	case SLIP_DELIMITER:
+		memcpy(skb_put(skb, 2), &esc_delim, 2);
+		break;
+	case SLIP_ESC:
+		memcpy(skb_put(skb, 2), &esc_esc, 2);
+		break;
+	default:
+		memcpy(skb_put(skb, 1), &c, 1);
+	}
+}
+
+static struct sk_buff *h5_build_pkt(struct h5 *h5, bool rel, u8 pkt_type,
+				    const u8 *data, size_t len)
 {
+	struct sk_buff *nskb;
+	u8 hdr[4];
+	int i;
+
+	/*
+	 * Max len of packet: (original len + 4 (H5 hdr) + 2 (crc)) * 2
+	 * (because bytes 0xc0 and 0xdb are escaped, worst case is when
+	 * the packet is all made of 0xc0 and 0xdb) + 2 (0xc0
+	 * delimiters at start and end).
+	 */
+	nskb = alloc_skb((len + 6) * 2 + 2, GFP_ATOMIC);
+	if (!nskb)
+		return NULL;
+
+	bt_cb(nskb)->pkt_type = pkt_type;
+
+	h5_slip_delim(nskb);
+
+	hdr[0] = h5->next_ack << 3;
 	h5->txack_req = false;
-	return NULL;
+
+	if (rel) {
+		hdr[0] |= 1 << 7;
+		hdr[0] |= h5->next_seq;
+		h5->next_seq = (h5->next_seq + 1) % 8;
+	}
+
+	hdr[1] = pkt_type | ((len & 0x0f) << 4);
+	hdr[2] = len >> 4;
+	hdr[3] = ~((hdr[0] + hdr[1] + hdr[2]) & 0xff);
+
+	for (i = 0; i < 4; i++)
+		h5_slip_one_byte(nskb, hdr[i]);
+
+	for (i = 0; i < len; i++)
+		h5_slip_one_byte(nskb, data[i]);
+
+	h5_slip_delim(nskb);
+
+	return nskb;
+}
+
+static struct sk_buff *h5_prepare_pkt(struct h5 *h5, u8 pkt_type,
+				      const u8 *data, size_t len)
+{
+	bool rel;
+
+	switch (pkt_type) {
+	case HCI_ACLDATA_PKT:
+	case HCI_COMMAND_PKT:
+		rel = true;
+		break;
+	case HCI_SCODATA_PKT:
+	case HCI_3WIRE_LINK_PKT:
+	case HCI_3WIRE_ACK_PKT:
+		rel = false;
+		break;
+	default:
+		BT_ERR("Unknown packet type %u", pkt_type);
+		return NULL;
+	}
+
+	return h5_build_pkt(h5, rel, pkt_type, data, len);
 }
 
 static struct sk_buff *h5_prepare_ack(struct h5 *h5)
@@ -372,7 +462,8 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 	struct sk_buff *skb, *nskb;
 
 	if ((skb = skb_dequeue(&h5->unrel)) != NULL) {
-		nskb = h5_prepare_pkt(h5, skb);
+		nskb = h5_prepare_pkt(h5, bt_cb(skb)->pkt_type,
+				      skb->data, skb->len);
 		if (nskb) {
 			kfree_skb(skb);
 			return nskb;
@@ -388,8 +479,8 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 	       goto unlock;
 
 	if ((skb = skb_dequeue(&h5->rel)) != NULL) {
-		nskb = h5_prepare_pkt(h5, skb);
-
+		nskb = h5_prepare_pkt(h5, bt_cb(skb)->pkt_type,
+				      skb->data, skb->len);
 		if (nskb) {
 			__skb_queue_tail(&h5->unack, skb);
 			mod_timer(&h5->timer, jiffies + HZ / 4);
-- 
1.7.10.4

[PATCH 06/17] Bluetooth: Add Three-wire header value convenience macros

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds convenience macros for reading Three-wire header values.
This will help make the code more readable.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 6b02ccc..0d608f3 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -41,6 +41,14 @@
  */
 #define H5_MAX_LEN (4 + 0xfff + 2)
 
+/* Convenience macros for reading Three-wire header values */
+#define H5_HDR_SEQ(hdr)		((hdr)[0] & 0x07)
+#define H5_HDR_ACK(hdr)		(((hdr)[0] >> 3) & 0x07)
+#define H5_HDR_CRC(hdr)		(((hdr)[0] >> 6) & 0x01)
+#define H5_HDR_RELIABLE(hdr)	(((hdr)[0] >> 7) & 0x01)
+#define H5_HDR_PKT_TYPE(hdr)	((hdr)[1] & 0x0f)
+#define H5_HDR_LEN(hdr)		((((hdr)[1] >> 4) & 0xff) + ((hdr)[2] << 4))
+
 #define SLIP_DELIMITER	0xc0
 #define SLIP_ESC	0xdb
 #define SLIP_ESC_DELIM	0xdc
-- 
1.7.10.4

[PATCH 07/17] Bluetooth: Fix/implement Three-wire reliable packet sending

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch should complete the necessary code for sending reliable
Three-wire packets.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   87 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 17 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 0d608f3..e27dce7 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -62,15 +62,15 @@ struct h5 {
 	struct sk_buff		*rx_skb;	/* Receive buffer */
 	size_t			rx_pending;	/* Expecting more bytes */
 	bool			rx_esc;		/* SLIP escape mode */
+	u8			rx_ack;		/* Last ack number received */
+	u8			rx_seq;		/* Last seq number received */
 
 	int			(*rx_func) (struct hci_uart *hu, u8 c);
 
 	struct timer_list	timer;		/* Retransmission timer */
 
-	bool			txack_req;
-
-	u8			next_ack;
-	u8			next_seq;
+	bool			tx_ack_req;	/* Pending ack to send */
+	u8			tx_seq;		/* Next seq number to send */
 };
 
 static void h5_reset_rx(struct h5 *h5);
@@ -87,7 +87,7 @@ static void h5_timed_event(unsigned long arg)
 	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
 
 	while ((skb = __skb_dequeue_tail(&h5->unack)) != NULL) {
-		h5->next_seq = (h5->next_seq - 1) & 0x07;
+		h5->tx_seq = (h5->tx_seq - 1) & 0x07;
 		skb_queue_head(&h5->rel, skb);
 	}
 
@@ -136,6 +136,45 @@ static int h5_close(struct hci_uart *hu)
 	return 0;
 }
 
+static void h5_pkt_cull(struct h5 *h5)
+{
+	struct sk_buff *skb, *tmp;
+	unsigned long flags;
+	int i, to_remove;
+	u8 seq;
+
+	spin_lock_irqsave(&h5->unack.lock, flags);
+
+	to_remove = skb_queue_len(&h5->unack);
+
+	seq = h5->tx_seq;
+
+	while (to_remove > 0) {
+		if (h5->rx_ack == seq)
+			break;
+
+		to_remove--;
+		seq = (seq - 1) % 8;
+	}
+
+	if (seq != h5->rx_ack)
+		BT_ERR("Controller acked invalid packet");
+
+	i = 0;
+	skb_queue_walk_safe(&h5->unack, skb, tmp) {
+		if (i++ >= to_remove)
+			break;
+
+		__skb_unlink(skb, &h5->unack);
+		kfree_skb(skb);
+	}
+
+	if (skb_queue_empty(&h5->unack))
+		del_timer(&h5->timer);
+
+	spin_unlock_irqrestore(&h5->unack.lock, flags);
+}
+
 static void h5_handle_internal_rx(struct hci_uart *hu)
 {
 	BT_DBG("%s", hu->hdev->name);
@@ -144,17 +183,24 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 static void h5_complete_rx_pkt(struct hci_uart *hu)
 {
 	struct h5 *h5 = hu->priv;
-	u8 pkt_type;
+	const unsigned char *hdr = h5->rx_skb->data;
 
 	BT_DBG("%s", hu->hdev->name);
 
-	pkt_type = h5->rx_skb->data[1] & 0x0f;
+	if (H5_HDR_RELIABLE(hdr)) {
+		h5->tx_seq = (h5->tx_seq + 1) % 8;
+		h5->tx_ack_req = true;
+	}
 
-	switch (pkt_type) {
+	h5->rx_ack = H5_HDR_ACK(hdr);
+
+	h5_pkt_cull(h5);
+
+	switch (H5_HDR_PKT_TYPE(hdr)) {
 	case HCI_EVENT_PKT:
 	case HCI_ACLDATA_PKT:
 	case HCI_SCODATA_PKT:
-		bt_cb(h5->rx_skb)->pkt_type = pkt_type;
+		bt_cb(h5->rx_skb)->pkt_type = H5_HDR_PKT_TYPE(hdr);
 
 		/* Remove Three-wire header */
 		skb_pull(h5->rx_skb, 4);
@@ -191,7 +237,7 @@ static int h5_rx_payload(struct hci_uart *hu, unsigned char c)
 
 	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
 
-	if ((hdr[0] >> 4) & 0x01) {
+	if (H5_HDR_CRC(hdr)) {
 		h5->rx_func = h5_rx_crc;
 		h5->rx_pending = 2;
 	} else {
@@ -215,8 +261,15 @@ static int h5_rx_3wire_hdr(struct hci_uart *hu, unsigned char c)
 		return 0;
 	}
 
+	if (H5_HDR_RELIABLE(hdr) && H5_HDR_SEQ(hdr) != h5->tx_seq) {
+		BT_ERR("Out-of-order packet arrived (%u != %u)",
+		       H5_HDR_SEQ(hdr), h5->tx_ack);
+		h5_reset_rx(h5);
+		return 0;
+	}
+
 	h5->rx_func = h5_rx_payload;
-	h5->rx_pending = ((hdr[1] >> 4) & 0xff) + (hdr[2] << 4);
+	h5->rx_pending = H5_HDR_LEN(hdr);
 
 	return 0;
 }
@@ -410,13 +463,13 @@ static struct sk_buff *h5_build_pkt(struct h5 *h5, bool rel, u8 pkt_type,
 
 	h5_slip_delim(nskb);
 
-	hdr[0] = h5->next_ack << 3;
-	h5->txack_req = false;
+	hdr[0] = h5->rx_seq << 3;
+	h5->tx_ack_req = false;
 
 	if (rel) {
 		hdr[0] |= 1 << 7;
-		hdr[0] |= h5->next_seq;
-		h5->next_seq = (h5->next_seq + 1) % 8;
+		hdr[0] |= h5->tx_seq;
+		h5->tx_seq = (h5->tx_seq + 1) % 8;
 	}
 
 	hdr[1] = pkt_type | ((len & 0x0f) << 4);
@@ -459,7 +512,7 @@ static struct sk_buff *h5_prepare_pkt(struct h5 *h5, u8 pkt_type,
 
 static struct sk_buff *h5_prepare_ack(struct h5 *h5)
 {
-	h5->txack_req = false;
+	h5->tx_ack_req = false;
 	return NULL;
 }
 
@@ -503,7 +556,7 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 unlock:
 	spin_unlock_irqrestore(&h5->unack.lock, flags);
 
-	if (h5->txack_req)
+	if (h5->tx_ack_req)
 		return h5_prepare_ack(h5);
 
 	return NULL;
-- 
1.7.10.4

[PATCH 08/17] Bluetooth: Add support for Three-wire Link Control packets

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds basic support for parsing and sending Three-wire UART
Link Controll packets.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   93 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 77 insertions(+), 16 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index e27dce7..d602cc4 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -63,7 +63,6 @@ struct h5 {
 	size_t			rx_pending;	/* Expecting more bytes */
 	bool			rx_esc;		/* SLIP escape mode */
 	u8			rx_ack;		/* Last ack number received */
-	u8			rx_seq;		/* Last seq number received */
 
 	int			(*rx_func) (struct hci_uart *hu, u8 c);
 
@@ -71,6 +70,7 @@ struct h5 {
 
 	bool			tx_ack_req;	/* Pending ack to send */
 	u8			tx_seq;		/* Next seq number to send */
+	u8			tx_ack;		/* Next ack number to send */
 };
 
 static void h5_reset_rx(struct h5 *h5);
@@ -96,9 +96,26 @@ static void h5_timed_event(unsigned long arg)
 	hci_uart_tx_wakeup(hu);
 }
 
+static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
+{
+	struct h5 *h5 = hu->priv;
+	struct sk_buff *nskb;
+
+	nskb = alloc_skb(3, GFP_ATOMIC);
+	if (!nskb)
+		return;
+
+	bt_cb(nskb)->pkt_type = HCI_3WIRE_LINK_PKT;
+
+	memcpy(skb_put(nskb, len), data, len);
+
+	skb_queue_tail(&h5->unrel, nskb);
+}
+
 static int h5_open(struct hci_uart *hu)
 {
 	struct h5 *h5;
+	const unsigned char sync[] = { 0x01, 0x7e };
 
 	BT_DBG("hu %p", hu);
 
@@ -118,6 +135,10 @@ static int h5_open(struct hci_uart *hu)
 	h5->timer.function = h5_timed_event;
 	h5->timer.data = (unsigned long) hu;
 
+	/* Send initial sync request */
+	h5_link_control(hu, sync, sizeof(sync));
+	mod_timer(&h5->timer, jiffies + HZ / 10);
+
 	return 0;
 }
 
@@ -146,6 +167,8 @@ static void h5_pkt_cull(struct h5 *h5)
 	spin_lock_irqsave(&h5->unack.lock, flags);
 
 	to_remove = skb_queue_len(&h5->unack);
+	if (to_remove == 0)
+		goto unlock;
 
 	seq = h5->tx_seq;
 
@@ -172,12 +195,44 @@ static void h5_pkt_cull(struct h5 *h5)
 	if (skb_queue_empty(&h5->unack))
 		del_timer(&h5->timer);
 
+unlock:
 	spin_unlock_irqrestore(&h5->unack.lock, flags);
 }
 
 static void h5_handle_internal_rx(struct hci_uart *hu)
 {
+	struct h5 *h5 = hu->priv;
+	const unsigned char sync_req[] = { 0x01, 0x7e };
+	const unsigned char sync_rsp[] = { 0x02, 0x7d };
+	const unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
+	const unsigned char conf_rsp[] = { 0x04, 0x7b, 0x01 };
+	const unsigned char *hdr = h5->rx_skb->data;
+	const unsigned char *data = &h5->rx_skb->data[4];
+
 	BT_DBG("%s", hu->hdev->name);
+
+	if (H5_HDR_PKT_TYPE(hdr) != HCI_3WIRE_LINK_PKT)
+		return;
+
+	if (H5_HDR_LEN(hdr) < 2)
+		return;
+
+	if (memcmp(data, sync_req, 2) == 0)
+		h5_link_control(hu, sync_rsp, 2);
+	else if (memcmp(data, sync_rsp, 2) == 0)
+		h5_link_control(hu, conf_req, 3);
+	else if (memcmp(data, conf_req, 2) == 0) {
+		h5_link_control(hu, conf_rsp, 2);
+		h5_link_control(hu, conf_req, 3);
+	} else if (memcmp(data, conf_rsp, 2) == 0) {
+		BT_DBG("Three-wire init sequence complete");
+		return;
+	} else {
+		BT_DBG("Link Control: 0x%02hhx 0x%02hhx", data[0], data[1]);
+		return;
+	}
+
+	hci_uart_tx_wakeup(hu);
 }
 
 static void h5_complete_rx_pkt(struct hci_uart *hu)
@@ -188,8 +243,9 @@ static void h5_complete_rx_pkt(struct hci_uart *hu)
 	BT_DBG("%s", hu->hdev->name);
 
 	if (H5_HDR_RELIABLE(hdr)) {
-		h5->tx_seq = (h5->tx_seq + 1) % 8;
+		h5->tx_ack = (h5->tx_ack + 1) % 8;
 		h5->tx_ack_req = true;
+		hci_uart_tx_wakeup(hu);
 	}
 
 	h5->rx_ack = H5_HDR_ACK(hdr);
@@ -255,13 +311,18 @@ static int h5_rx_3wire_hdr(struct hci_uart *hu, unsigned char c)
 
 	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
 
+	BT_DBG("%s rx: seq %u ack %u crc %u rel %u type %u len %u",
+	       hu->hdev->name, H5_HDR_SEQ(hdr), H5_HDR_ACK(hdr),
+	       H5_HDR_CRC(hdr), H5_HDR_RELIABLE(hdr), H5_HDR_PKT_TYPE(hdr),
+	       H5_HDR_LEN(hdr));
+
 	if (((hdr[0] + hdr[1] + hdr[2] + hdr[3]) & 0xff) != 0xff) {
 		BT_ERR("Invalid header checksum");
 		h5_reset_rx(h5);
 		return 0;
 	}
 
-	if (H5_HDR_RELIABLE(hdr) && H5_HDR_SEQ(hdr) != h5->tx_seq) {
+	if (H5_HDR_RELIABLE(hdr) && H5_HDR_SEQ(hdr) != h5->tx_ack) {
 		BT_ERR("Out-of-order packet arrived (%u != %u)",
 		       H5_HDR_SEQ(hdr), h5->tx_ack);
 		h5_reset_rx(h5);
@@ -442,9 +503,10 @@ static void h5_slip_one_byte(struct sk_buff *skb, u8 c)
 	}
 }
 
-static struct sk_buff *h5_build_pkt(struct h5 *h5, bool rel, u8 pkt_type,
+static struct sk_buff *h5_build_pkt(struct hci_uart *hu, bool rel, u8 pkt_type,
 				    const u8 *data, size_t len)
 {
+	struct h5 *h5 = hu->priv;
 	struct sk_buff *nskb;
 	u8 hdr[4];
 	int i;
@@ -463,7 +525,7 @@ static struct sk_buff *h5_build_pkt(struct h5 *h5, bool rel, u8 pkt_type,
 
 	h5_slip_delim(nskb);
 
-	hdr[0] = h5->rx_seq << 3;
+	hdr[0] = h5->tx_ack << 3;
 	h5->tx_ack_req = false;
 
 	if (rel) {
@@ -476,6 +538,11 @@ static struct sk_buff *h5_build_pkt(struct h5 *h5, bool rel, u8 pkt_type,
 	hdr[2] = len >> 4;
 	hdr[3] = ~((hdr[0] + hdr[1] + hdr[2]) & 0xff);
 
+	BT_DBG("%s tx: seq %u ack %u crc %u rel %u type %u len %u",
+	       hu->hdev->name, H5_HDR_SEQ(hdr), H5_HDR_ACK(hdr),
+	       H5_HDR_CRC(hdr), H5_HDR_RELIABLE(hdr), H5_HDR_PKT_TYPE(hdr),
+	       H5_HDR_LEN(hdr));
+
 	for (i = 0; i < 4; i++)
 		h5_slip_one_byte(nskb, hdr[i]);
 
@@ -487,7 +554,7 @@ static struct sk_buff *h5_build_pkt(struct h5 *h5, bool rel, u8 pkt_type,
 	return nskb;
 }
 
-static struct sk_buff *h5_prepare_pkt(struct h5 *h5, u8 pkt_type,
+static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
 				      const u8 *data, size_t len)
 {
 	bool rel;
@@ -507,13 +574,7 @@ static struct sk_buff *h5_prepare_pkt(struct h5 *h5, u8 pkt_type,
 		return NULL;
 	}
 
-	return h5_build_pkt(h5, rel, pkt_type, data, len);
-}
-
-static struct sk_buff *h5_prepare_ack(struct h5 *h5)
-{
-	h5->tx_ack_req = false;
-	return NULL;
+	return h5_build_pkt(hu, rel, pkt_type, data, len);
 }
 
 static struct sk_buff *h5_dequeue(struct hci_uart *hu)
@@ -523,7 +584,7 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 	struct sk_buff *skb, *nskb;
 
 	if ((skb = skb_dequeue(&h5->unrel)) != NULL) {
-		nskb = h5_prepare_pkt(h5, bt_cb(skb)->pkt_type,
+		nskb = h5_prepare_pkt(hu, bt_cb(skb)->pkt_type,
 				      skb->data, skb->len);
 		if (nskb) {
 			kfree_skb(skb);
@@ -540,7 +601,7 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 	       goto unlock;
 
 	if ((skb = skb_dequeue(&h5->rel)) != NULL) {
-		nskb = h5_prepare_pkt(h5, bt_cb(skb)->pkt_type,
+		nskb = h5_prepare_pkt(hu, bt_cb(skb)->pkt_type,
 				      skb->data, skb->len);
 		if (nskb) {
 			__skb_queue_tail(&h5->unack, skb);
@@ -557,7 +618,7 @@ unlock:
 	spin_unlock_irqrestore(&h5->unack.lock, flags);
 
 	if (h5->tx_ack_req)
-		return h5_prepare_ack(h5);
+		return h5_prepare_pkt(hu, HCI_3WIRE_ACK_PKT, NULL, 0);
 
 	return NULL;
 }
-- 
1.7.10.4

Re: [PATCH 08/17] Bluetooth: Add support for Three-wire Link Control packets

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-06-27 14:25:58 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds basic support for parsing and sending Three-wire UART
> Link Controll packets.

typo here, it is Control.

> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  drivers/bluetooth/hci_h5.c |   93 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 77 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index e27dce7..d602cc4 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -63,7 +63,6 @@ struct h5 {
>  	size_t			rx_pending;	/* Expecting more bytes */
>  	bool			rx_esc;		/* SLIP escape mode */
>  	u8			rx_ack;		/* Last ack number received */
> -	u8			rx_seq;		/* Last seq number received */
>  
>  	int			(*rx_func) (struct hci_uart *hu, u8 c);
>  
> @@ -71,6 +70,7 @@ struct h5 {
>  
>  	bool			tx_ack_req;	/* Pending ack to send */
>  	u8			tx_seq;		/* Next seq number to send */
> +	u8			tx_ack;		/* Next ack number to send */
>  };
>  
>  static void h5_reset_rx(struct h5 *h5);
> @@ -96,9 +96,26 @@ static void h5_timed_event(unsigned long arg)
>  	hci_uart_tx_wakeup(hu);
>  }
>  
> +static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
> +{
> +	struct h5 *h5 = hu->priv;
> +	struct sk_buff *nskb;
> +
> +	nskb = alloc_skb(3, GFP_ATOMIC);
> +	if (!nskb)
> +		return;
> +
> +	bt_cb(nskb)->pkt_type = HCI_3WIRE_LINK_PKT;
> +
> +	memcpy(skb_put(nskb, len), data, len);
> +
> +	skb_queue_tail(&h5->unrel, nskb);
> +}
> +
>  static int h5_open(struct hci_uart *hu)
>  {
>  	struct h5 *h5;
> +	const unsigned char sync[] = { 0x01, 0x7e };
>  
>  	BT_DBG("hu %p", hu);
>  
> @@ -118,6 +135,10 @@ static int h5_open(struct hci_uart *hu)
>  	h5->timer.function = h5_timed_event;
>  	h5->timer.data = (unsigned long) hu;
>  
> +	/* Send initial sync request */
> +	h5_link_control(hu, sync, sizeof(sync));
> +	mod_timer(&h5->timer, jiffies + HZ / 10);

remove this HZ usage please.

> +
>  	return 0;
>  }
>  
> @@ -146,6 +167,8 @@ static void h5_pkt_cull(struct h5 *h5)
>  	spin_lock_irqsave(&h5->unack.lock, flags);
>  
>  	to_remove = skb_queue_len(&h5->unack);
> +	if (to_remove == 0)
> +		goto unlock;
>  
>  	seq = h5->tx_seq;
>  
> @@ -172,12 +195,44 @@ static void h5_pkt_cull(struct h5 *h5)
>  	if (skb_queue_empty(&h5->unack))
>  		del_timer(&h5->timer);
>  
> +unlock:
>  	spin_unlock_irqrestore(&h5->unack.lock, flags);
>  }
>  
>  static void h5_handle_internal_rx(struct hci_uart *hu)
>  {
> +	struct h5 *h5 = hu->priv;
> +	const unsigned char sync_req[] = { 0x01, 0x7e };
> +	const unsigned char sync_rsp[] = { 0x02, 0x7d };
> +	const unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
> +	const unsigned char conf_rsp[] = { 0x04, 0x7b, 0x01 };
> +	const unsigned char *hdr = h5->rx_skb->data;
> +	const unsigned char *data = &h5->rx_skb->data[4];
> +
>  	BT_DBG("%s", hu->hdev->name);
> +
> +	if (H5_HDR_PKT_TYPE(hdr) != HCI_3WIRE_LINK_PKT)
> +		return;
> +
> +	if (H5_HDR_LEN(hdr) < 2)
> +		return;
> +
> +	if (memcmp(data, sync_req, 2) == 0)
> +		h5_link_control(hu, sync_rsp, 2);
> +	else if (memcmp(data, sync_rsp, 2) == 0)
> +		h5_link_control(hu, conf_req, 3);
> +	else if (memcmp(data, conf_req, 2) == 0) {
> +		h5_link_control(hu, conf_rsp, 2);
> +		h5_link_control(hu, conf_req, 3);
> +	} else if (memcmp(data, conf_rsp, 2) == 0) {
> +		BT_DBG("Three-wire init sequence complete");
> +		return;
> +	} else {
> +		BT_DBG("Link Control: 0x%02hhx 0x%02hhx", data[0], data[1]);
> +		return;
> +	}


There are missing braces here, some of them have but some of them not.

	Gustavo

[PATCH 09/17] Bluetooth: Simplify hci_uart_tty_close logic

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch cleans up and reduces indentation in the hci_uart_tty_close
function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_ldisc.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 142f49c..b6d1f20 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -286,28 +286,29 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 static void hci_uart_tty_close(struct tty_struct *tty)
 {
 	struct hci_uart *hu = (void *)tty->disc_data;
+	struct hci_dev *hdev;
 
 	BT_DBG("tty %p", tty);
 
 	/* Detach from the tty */
 	tty->disc_data = NULL;
 
-	if (hu) {
-		struct hci_dev *hdev = hu->hdev;
+	if (!hu)
+		return;
 
-		if (hdev)
-			hci_uart_close(hdev);
+	hdev = hu->hdev;
+	if (hdev)
+		hci_uart_close(hdev);
 
-		if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
-			if (hdev) {
-				hci_unregister_dev(hdev);
-				hci_free_dev(hdev);
-			}
-			hu->proto->close(hu);
+	if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
+		if (hdev) {
+			hci_unregister_dev(hdev);
+			hci_free_dev(hdev);
 		}
-
-		kfree(hu);
+		hu->proto->close(hu);
 	}
+
+	kfree(hu);
 }
 
 /* hci_uart_tty_wakeup()
-- 
1.7.10.4

[PATCH 10/17] Bluetooth: Add delayed init sequence support for UART controllers

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch makes it possible to have UART drivers perform an internal
initialization before calling hci_reguister_dev. This allows moving a
lot of init code from user space (hciattach) to the kernel side, thereby
creating a more controlled/robust initialization process.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_ldisc.c |   39 ++++++++++++++++++++++++++++++++++++++-
 drivers/bluetooth/hci_uart.h  |    5 +++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index b6d1f20..74e0966 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -156,6 +156,35 @@ restart:
 	return 0;
 }
 
+static void hci_uart_init_work(struct work_struct *work)
+{
+	struct hci_uart *hu = container_of(work, struct hci_uart, init_ready);
+	int err;
+
+	if (!test_and_clear_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
+		return;
+
+	err = hci_register_dev(hu->hdev);
+	if (err < 0) {
+		BT_ERR("Can't register HCI device");
+		hci_free_dev(hu->hdev);
+		hu->hdev = NULL;
+		hu->proto->close(hu);
+	}
+
+	set_bit(HCI_UART_REGISTERED, &hu->flags);
+}
+
+int hci_uart_init_ready(struct hci_uart *hu)
+{
+	if (!test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
+		return -EALREADY;
+
+	schedule_work(&hu->init_ready);
+
+	return 0;
+}
+
 /* ------- Interface to HCI layer ------ */
 /* Initialize device */
 static int hci_uart_open(struct hci_dev *hdev)
@@ -264,6 +293,8 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	hu->tty = tty;
 	tty->receive_room = 65536;
 
+	INIT_WORK(&hu->init_ready, hci_uart_init_work);
+
 	spin_lock_init(&hu->rx_lock);
 
 	/* Flush any pending characters in the driver and line discipline. */
@@ -302,7 +333,8 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 
 	if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
 		if (hdev) {
-			hci_unregister_dev(hdev);
+			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
+				hci_unregister_dev(hdev);
 			hci_free_dev(hdev);
 		}
 		hu->proto->close(hu);
@@ -402,12 +434,17 @@ static int hci_uart_register_dev(struct hci_uart *hu)
 	else
 		hdev->dev_type = HCI_BREDR;
 
+	if (test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
+		return 0;
+
 	if (hci_register_dev(hdev) < 0) {
 		BT_ERR("Can't register HCI device");
 		hci_free_dev(hdev);
 		return -ENODEV;
 	}
 
+	set_bit(HCI_UART_REGISTERED, &hu->flags);
+
 	return 0;
 }
 
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index aaf9d7d..fffa61f 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -47,6 +47,7 @@
 #define HCI_UART_RAW_DEVICE	0
 #define HCI_UART_RESET_ON_INIT	1
 #define HCI_UART_CREATE_AMP	2
+#define HCI_UART_INIT_PENDING	3
 
 struct hci_uart;
 
@@ -66,6 +67,8 @@ struct hci_uart {
 	unsigned long		flags;
 	unsigned long		hdev_flags;
 
+	struct work_struct	init_ready;
+
 	struct hci_uart_proto	*proto;
 	void			*priv;
 
@@ -76,6 +79,7 @@ struct hci_uart {
 
 /* HCI_UART proto flag bits */
 #define HCI_UART_PROTO_SET	0
+#define HCI_UART_REGISTERED	1
 
 /* TX states  */
 #define HCI_UART_SENDING	1
@@ -84,6 +88,7 @@ struct hci_uart {
 int hci_uart_register_proto(struct hci_uart_proto *p);
 int hci_uart_unregister_proto(struct hci_uart_proto *p);
 int hci_uart_tx_wakeup(struct hci_uart *hu);
+int hci_uart_init_ready(struct hci_uart *hu);
 
 #ifdef CONFIG_BT_HCIUART_H4
 int h4_init(void);
-- 
1.7.10.4

Re: [PATCH 10/17] Bluetooth: Add delayed init sequence support for UART controllers

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-06-27 14:26:00 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch makes it possible to have UART drivers perform an internal
> initialization before calling hci_reguister_dev. This allows moving a

typo:  hci_register_dev

	Gustavo

[PATCH 11/17] Bluetooth: Use delayed init for Three-wire UART

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch takes into use the delayed initialization feature that the
Bluetooth UART framework provides.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index d602cc4..4f41d24 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -135,6 +135,8 @@ static int h5_open(struct hci_uart *hu)
 	h5->timer.function = h5_timed_event;
 	h5->timer.data = (unsigned long) hu;
 
+	set_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags);
+
 	/* Send initial sync request */
 	h5_link_control(hu, sync, sizeof(sync));
 	mod_timer(&h5->timer, jiffies + HZ / 10);
@@ -226,6 +228,7 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 		h5_link_control(hu, conf_req, 3);
 	} else if (memcmp(data, conf_rsp, 2) == 0) {
 		BT_DBG("Three-wire init sequence complete");
+		hci_uart_init_ready(hu);
 		return;
 	} else {
 		BT_DBG("Link Control: 0x%02hhx 0x%02hhx", data[0], data[1]);
-- 
1.7.10.4

[PATCH 12/17] Bluetooth: Improve rx debug logs for Three-wire UART

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

Remove unnecessary debug logs and add some to more centralized places.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 4f41d24..31b8d57 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -243,8 +243,6 @@ static void h5_complete_rx_pkt(struct hci_uart *hu)
 	struct h5 *h5 = hu->priv;
 	const unsigned char *hdr = h5->rx_skb->data;
 
-	BT_DBG("%s", hu->hdev->name);
-
 	if (H5_HDR_RELIABLE(hdr)) {
 		h5->tx_ack = (h5->tx_ack + 1) % 8;
 		h5->tx_ack_req = true;
@@ -281,8 +279,6 @@ static int h5_rx_crc(struct hci_uart *hu, unsigned char c)
 {
 	struct h5 *h5 = hu->priv;
 
-	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
-
 	h5_complete_rx_pkt(hu);
 	h5_reset_rx(h5);
 
@@ -294,8 +290,6 @@ static int h5_rx_payload(struct hci_uart *hu, unsigned char c)
 	struct h5 *h5 = hu->priv;
 	const unsigned char *hdr = h5->rx_skb->data;
 
-	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
-
 	if (H5_HDR_CRC(hdr)) {
 		h5->rx_func = h5_rx_crc;
 		h5->rx_pending = 2;
@@ -312,8 +306,6 @@ static int h5_rx_3wire_hdr(struct hci_uart *hu, unsigned char c)
 	struct h5 *h5 = hu->priv;
 	const unsigned char *hdr = h5->rx_skb->data;
 
-	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
-
 	BT_DBG("%s rx: seq %u ack %u crc %u rel %u type %u len %u",
 	       hu->hdev->name, H5_HDR_SEQ(hdr), H5_HDR_ACK(hdr),
 	       H5_HDR_CRC(hdr), H5_HDR_RELIABLE(hdr), H5_HDR_PKT_TYPE(hdr),
@@ -342,8 +334,6 @@ static int h5_rx_pkt_start(struct hci_uart *hu, unsigned char c)
 {
 	struct h5 *h5 = hu->priv;
 
-	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
-
 	if (c == SLIP_DELIMITER)
 		return 1;
 
@@ -366,8 +356,6 @@ static int h5_rx_delimiter(struct hci_uart *hu, unsigned char c)
 {
 	struct h5 *h5 = hu->priv;
 
-	BT_DBG("%s 0x%02hhx", hu->hdev->name, c);
-
 	if (c == SLIP_DELIMITER)
 		h5->rx_func = h5_rx_pkt_start;
 
@@ -404,7 +392,7 @@ static void h5_unslip_one_byte(struct h5 *h5, unsigned char c)
 	memcpy(skb_put(h5->rx_skb, 1), byte, 1);
 	h5->rx_pending--;
 
-	BT_DBG("unsliped 0x%02hhx", *byte);
+	BT_DBG("unsliped 0x%02hhx, rx_pending %zu", *byte, h5->rx_pending);
 }
 
 static void h5_reset_rx(struct h5 *h5)
@@ -424,7 +412,8 @@ static int h5_recv(struct hci_uart *hu, void *data, int count)
 	struct h5 *h5 = hu->priv;
 	unsigned char *ptr = data;
 
-	BT_DBG("%s count %d", hu->hdev->name, count);
+	BT_DBG("%s pending %zu count %d", hu->hdev->name, h5->rx_pending,
+	       count);
 
 	while (count > 0) {
 		int processed;
-- 
1.7.10.4

[PATCH 13/17] Bluetooth: Add initial sleep support to Three-wire UART

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds very basic support for the sleep related messages. The
only thing the code does right now is send a wakeup message as soon as
receiving a sleep one, essentially preventing the controller from going
to sleep.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 31b8d57..d26f801 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -71,6 +71,8 @@ struct h5 {
 	bool			tx_ack_req;	/* Pending ack to send */
 	u8			tx_seq;		/* Next seq number to send */
 	u8			tx_ack;		/* Next ack number to send */
+
+	bool			sleeping;
 };
 
 static void h5_reset_rx(struct h5 *h5);
@@ -208,6 +210,9 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 	const unsigned char sync_rsp[] = { 0x02, 0x7d };
 	const unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
 	const unsigned char conf_rsp[] = { 0x04, 0x7b, 0x01 };
+	const unsigned char wakeup_req[] = { 0x05, 0xfa };
+	const unsigned char woken_req[] = { 0x06, 0xf9 };
+	const unsigned char sleep_req[] = { 0x07, 0x78 };
 	const unsigned char *hdr = h5->rx_skb->data;
 	const unsigned char *data = &h5->rx_skb->data[4];
 
@@ -230,6 +235,14 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 		BT_DBG("Three-wire init sequence complete");
 		hci_uart_init_ready(hu);
 		return;
+	} else if (memcmp(data, sleep_req, 2) == 0) {
+		BT_DBG("Peer went to sleep");
+		h5->sleeping = true;
+		h5_link_control(hu, wakeup_req, 2);
+	} else if (memcmp(data, woken_req, 2) == 0) {
+		BT_DBG("Peer woke up");
+		h5->sleeping = false;
+		return;
 	} else {
 		BT_DBG("Link Control: 0x%02hhx 0x%02hhx", data[0], data[1]);
 		return;
-- 
1.7.10.4

Re: [PATCH 13/17] Bluetooth: Add initial sleep support to Three-wire UART

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-06-27 14:26:03 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds very basic support for the sleep related messages. The
> only thing the code does right now is send a wakeup message as soon as
> receiving a sleep one, essentially preventing the controller from going
> to sleep.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  drivers/bluetooth/hci_h5.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index 31b8d57..d26f801 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -71,6 +71,8 @@ struct h5 {
>  	bool			tx_ack_req;	/* Pending ack to send */
>  	u8			tx_seq;		/* Next seq number to send */
>  	u8			tx_ack;		/* Next ack number to send */
> +
> +	bool			sleeping;

Please fold this into the flags member I proposed in the other patch.

	Gustavo

[PATCH 14/17] Bluetooth: Add initialization tracking to HCI Three-wire driver

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds tracking for the uninitialized, initialized and active
states for Three-wire UART. This is needed so we can handle periodic
sending of the Link Establishment messages before reaching active state
and so that we do not try to do any higher level HCI data transmission
before reaching active state.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   70 ++++++++++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 18 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index d26f801..1e83118 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -72,18 +72,53 @@ struct h5 {
 	u8			tx_seq;		/* Next seq number to send */
 	u8			tx_ack;		/* Next ack number to send */
 
+	enum {
+		H5_UNINITIALIZED,
+		H5_INITIALIZED,
+		H5_ACTIVE,
+	} state;
+
 	bool			sleeping;
 };
 
 static void h5_reset_rx(struct h5 *h5);
 
+static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
+{
+	struct h5 *h5 = hu->priv;
+	struct sk_buff *nskb;
+
+	nskb = alloc_skb(3, GFP_ATOMIC);
+	if (!nskb)
+		return;
+
+	bt_cb(nskb)->pkt_type = HCI_3WIRE_LINK_PKT;
+
+	memcpy(skb_put(nskb, len), data, len);
+
+	skb_queue_tail(&h5->unrel, nskb);
+}
+
 static void h5_timed_event(unsigned long arg)
 {
+	const unsigned char sync_req[] = { 0x01, 0x7e };
+	const unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
 	struct hci_uart *hu = (struct hci_uart *) arg;
 	struct h5 *h5 = hu->priv;
 	struct sk_buff *skb;
 	unsigned long flags;
 
+	if (h5->state == H5_UNINITIALIZED)
+		h5_link_control(hu, sync_req, sizeof(sync_req));
+
+	if (h5->state == H5_INITIALIZED)
+		h5_link_control(hu, conf_req, sizeof(conf_req));
+
+	if (h5->state != H5_ACTIVE) {
+		mod_timer(&h5->timer, jiffies + HZ / 10);
+		goto wakeup;
+	}
+
 	BT_DBG("hu %p retransmitting %u pkts", hu, h5->unack.qlen);
 
 	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
@@ -95,25 +130,10 @@ static void h5_timed_event(unsigned long arg)
 
 	spin_unlock_irqrestore(&h5->unack.lock, flags);
 
+wakeup:
 	hci_uart_tx_wakeup(hu);
 }
 
-static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
-{
-	struct h5 *h5 = hu->priv;
-	struct sk_buff *nskb;
-
-	nskb = alloc_skb(3, GFP_ATOMIC);
-	if (!nskb)
-		return;
-
-	bt_cb(nskb)->pkt_type = HCI_3WIRE_LINK_PKT;
-
-	memcpy(skb_put(nskb, len), data, len);
-
-	skb_queue_tail(&h5->unrel, nskb);
-}
-
 static int h5_open(struct hci_uart *hu)
 {
 	struct h5 *h5;
@@ -226,13 +246,15 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 
 	if (memcmp(data, sync_req, 2) == 0)
 		h5_link_control(hu, sync_rsp, 2);
-	else if (memcmp(data, sync_rsp, 2) == 0)
+	else if (memcmp(data, sync_rsp, 2) == 0) {
+		h5->state = H5_INITIALIZED;
 		h5_link_control(hu, conf_req, 3);
-	else if (memcmp(data, conf_req, 2) == 0) {
+	} else if (memcmp(data, conf_req, 2) == 0) {
 		h5_link_control(hu, conf_rsp, 2);
 		h5_link_control(hu, conf_req, 3);
 	} else if (memcmp(data, conf_rsp, 2) == 0) {
 		BT_DBG("Three-wire init sequence complete");
+		h5->state = H5_ACTIVE;
 		hci_uart_init_ready(hu);
 		return;
 	} else if (memcmp(data, sleep_req, 2) == 0) {
@@ -337,6 +359,12 @@ static int h5_rx_3wire_hdr(struct hci_uart *hu, unsigned char c)
 		return 0;
 	}
 
+	if (h5->state != H5_ACTIVE &&
+	    H5_HDR_PKT_TYPE(hdr) != HCI_3WIRE_LINK_PKT) {
+		BT_ERR("Non-link packet received in non-active state");
+		h5_reset_rx(h5);
+	}
+
 	h5->rx_func = h5_rx_payload;
 	h5->rx_pending = H5_HDR_LEN(hdr);
 
@@ -465,6 +493,12 @@ static int h5_enqueue(struct hci_uart *hu, struct sk_buff *skb)
 		return 0;
 	}
 
+	if (h5->state != H5_ACTIVE) {
+		BT_ERR("Ignoring HCI data in non-active state");
+		kfree_skb(skb);
+		return 0;
+	}
+
 	switch (bt_cb(skb)->pkt_type) {
 	case HCI_ACLDATA_PKT:
 	case HCI_COMMAND_PKT:
-- 
1.7.10.4

Re: [PATCH 14/17] Bluetooth: Add initialization tracking to HCI Three-wire driver

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-06-27 14:26:04 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds tracking for the uninitialized, initialized and active
> states for Three-wire UART. This is needed so we can handle periodic
> sending of the Link Establishment messages before reaching active state
> and so that we do not try to do any higher level HCI data transmission
> before reaching active state.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  drivers/bluetooth/hci_h5.c |   70 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 52 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index d26f801..1e83118 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -72,18 +72,53 @@ struct h5 {
>  	u8			tx_seq;		/* Next seq number to send */
>  	u8			tx_ack;		/* Next ack number to send */
>  
> +	enum {
> +		H5_UNINITIALIZED,
> +		H5_INITIALIZED,
> +		H5_ACTIVE,
> +	} state;
> +
>  	bool			sleeping;
>  };
>  
>  static void h5_reset_rx(struct h5 *h5);
>  
> +static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
> +{
> +	struct h5 *h5 = hu->priv;
> +	struct sk_buff *nskb;
> +
> +	nskb = alloc_skb(3, GFP_ATOMIC);
> +	if (!nskb)
> +		return;
> +
> +	bt_cb(nskb)->pkt_type = HCI_3WIRE_LINK_PKT;
> +
> +	memcpy(skb_put(nskb, len), data, len);
> +
> +	skb_queue_tail(&h5->unrel, nskb);
> +}
> +
>  static void h5_timed_event(unsigned long arg)
>  {
> +	const unsigned char sync_req[] = { 0x01, 0x7e };
> +	const unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
>  	struct hci_uart *hu = (struct hci_uart *) arg;
>  	struct h5 *h5 = hu->priv;
>  	struct sk_buff *skb;
>  	unsigned long flags;
>  
> +	if (h5->state == H5_UNINITIALIZED)
> +		h5_link_control(hu, sync_req, sizeof(sync_req));
> +
> +	if (h5->state == H5_INITIALIZED)
> +		h5_link_control(hu, conf_req, sizeof(conf_req));
> +
> +	if (h5->state != H5_ACTIVE) {
> +		mod_timer(&h5->timer, jiffies + HZ / 10);
> +		goto wakeup;

Another HZ usage.

	Gustavo

[PATCH 15/17] Bluetooth: Implement proper low-power support for Three-wire UART

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds on-demand wakeup request sending (and re-sendind) when
we are in low-power state. When the controller enters this state it will
send a sleep message after which the host is not allowed to send any
other packets until a wakeup request has been sent and the woken message
received as a response to it. The wakeup requests are re-sent
periodically until a woken message is received.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 1e83118..630ed70 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -78,7 +78,11 @@ struct h5 {
 		H5_ACTIVE,
 	} state;
 
-	bool			sleeping;
+	enum {
+		H5_AWAKE,
+		H5_SLEEPING,
+		H5_WAKING_UP,
+	} sleep;
 };
 
 static void h5_reset_rx(struct h5 *h5);
@@ -108,6 +112,8 @@ static void h5_timed_event(unsigned long arg)
 	struct sk_buff *skb;
 	unsigned long flags;
 
+	BT_DBG("%s", hu->hdev->name);
+
 	if (h5->state == H5_UNINITIALIZED)
 		h5_link_control(hu, sync_req, sizeof(sync_req));
 
@@ -119,6 +125,11 @@ static void h5_timed_event(unsigned long arg)
 		goto wakeup;
 	}
 
+	if (h5->sleep != H5_AWAKE) {
+		h5->sleep = H5_SLEEPING;
+		goto wakeup;
+	}
+
 	BT_DBG("hu %p retransmitting %u pkts", hu, h5->unack.qlen);
 
 	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
@@ -259,12 +270,15 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 		return;
 	} else if (memcmp(data, sleep_req, 2) == 0) {
 		BT_DBG("Peer went to sleep");
-		h5->sleeping = true;
-		h5_link_control(hu, wakeup_req, 2);
+		h5->sleep = H5_SLEEPING;
+		return;
 	} else if (memcmp(data, woken_req, 2) == 0) {
 		BT_DBG("Peer woke up");
-		h5->sleeping = false;
-		return;
+		h5->sleep = H5_AWAKE;
+	} else if (memcmp(data, wakeup_req, 2) == 0) {
+		BT_DBG("Peer requested wakeup");
+		h5_link_control(hu, woken_req, 2);
+		h5->sleep = H5_AWAKE;
 	} else {
 		BT_DBG("Link Control: 0x%02hhx 0x%02hhx", data[0], data[1]);
 		return;
@@ -622,6 +636,19 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 	unsigned long flags;
 	struct sk_buff *skb, *nskb;
 
+	if (h5->sleep != H5_AWAKE) {
+		const unsigned char wakeup_req[] = { 0x05, 0xfa };
+
+		if (h5->sleep == H5_WAKING_UP)
+			return NULL;
+
+		h5->sleep = H5_WAKING_UP;
+		BT_DBG("Sending wakeup request");
+
+		mod_timer(&h5->timer, jiffies + HZ / 100);
+		return h5_prepare_pkt(hu, HCI_3WIRE_LINK_PKT, wakeup_req, 2);
+	}
+
 	if ((skb = skb_dequeue(&h5->unrel)) != NULL) {
 		nskb = h5_prepare_pkt(hu, bt_cb(skb)->pkt_type,
 				      skb->data, skb->len);
-- 
1.7.10.4

[PATCH 16/17] Bluetooth: Remove unnecessary h5_build_pkt function

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The implementation of h5_build_packet can be moved into
h5_prepare_pkt since all h5_prepare_pkt does is determine whether the
packet is reliable and then call h5_build_packet.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   49 +++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 630ed70..e036278 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -556,14 +556,33 @@ static void h5_slip_one_byte(struct sk_buff *skb, u8 c)
 	}
 }
 
-static struct sk_buff *h5_build_pkt(struct hci_uart *hu, bool rel, u8 pkt_type,
-				    const u8 *data, size_t len)
+static bool valid_packet_type(u8 type)
+{
+	switch (type) {
+	case HCI_ACLDATA_PKT:
+	case HCI_COMMAND_PKT:
+	case HCI_SCODATA_PKT:
+	case HCI_3WIRE_LINK_PKT:
+	case HCI_3WIRE_ACK_PKT:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
+				      const u8 *data, size_t len)
 {
 	struct h5 *h5 = hu->priv;
 	struct sk_buff *nskb;
 	u8 hdr[4];
 	int i;
 
+	if (!valid_packet_type(pkt_type)) {
+		BT_ERR("Unknown packet type %u", pkt_type);
+		return NULL;
+	}
+
 	/*
 	 * Max len of packet: (original len + 4 (H5 hdr) + 2 (crc)) * 2
 	 * (because bytes 0xc0 and 0xdb are escaped, worst case is when
@@ -581,7 +600,8 @@ static struct sk_buff *h5_build_pkt(struct hci_uart *hu, bool rel, u8 pkt_type,
 	hdr[0] = h5->tx_ack << 3;
 	h5->tx_ack_req = false;
 
-	if (rel) {
+	/* Reliable packet? */
+	if (pkt_type == HCI_ACLDATA_PKT || pkt_type == HCI_COMMAND_PKT) {
 		hdr[0] |= 1 << 7;
 		hdr[0] |= h5->tx_seq;
 		h5->tx_seq = (h5->tx_seq + 1) % 8;
@@ -607,29 +627,6 @@ static struct sk_buff *h5_build_pkt(struct hci_uart *hu, bool rel, u8 pkt_type,
 	return nskb;
 }
 
-static struct sk_buff *h5_prepare_pkt(struct hci_uart *hu, u8 pkt_type,
-				      const u8 *data, size_t len)
-{
-	bool rel;
-
-	switch (pkt_type) {
-	case HCI_ACLDATA_PKT:
-	case HCI_COMMAND_PKT:
-		rel = true;
-		break;
-	case HCI_SCODATA_PKT:
-	case HCI_3WIRE_LINK_PKT:
-	case HCI_3WIRE_ACK_PKT:
-		rel = false;
-		break;
-	default:
-		BT_ERR("Unknown packet type %u", pkt_type);
-		return NULL;
-	}
-
-	return h5_build_pkt(hu, rel, pkt_type, data, len);
-}
-
 static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 {
 	struct h5 *h5 = hu->priv;
-- 
1.7.10.4

[PATCH 17/17] Bluetooth: Improve Three-wire UART configuration handling

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The configuration request/response messages contain a configuration
field which contains the sliding window size (amount of unacked reliable
packets that can be pending). This patch makes sure that we configure
the correct size (minimum of local and remote values) and use it when
determining whether to send new packets or not.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 drivers/bluetooth/hci_h5.c |   34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index e036278..bc7bcb5 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -33,7 +33,8 @@
 #define HCI_3WIRE_ACK_PKT	0
 #define HCI_3WIRE_LINK_PKT	15
 
-#define H5_TXWINSIZE 4
+/* Sliding window size */
+#define H5_TX_WIN_MAX		4
 
 /*
  * Maximum Three-wire packet:
@@ -71,6 +72,7 @@ struct h5 {
 	bool			tx_ack_req;	/* Pending ack to send */
 	u8			tx_seq;		/* Next seq number to send */
 	u8			tx_ack;		/* Next ack number to send */
+	u8			tx_win;		/* Sliding window size */
 
 	enum {
 		H5_UNINITIALIZED,
@@ -103,10 +105,20 @@ static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
 	skb_queue_tail(&h5->unrel, nskb);
 }
 
+static u8 h5_cfg_field(struct h5 *h5)
+{
+	u8 field = 0;
+
+	/* Sliding window size (first 3 bits) */
+	field |= (h5->tx_win & 7);
+
+	return field;
+}
+
 static void h5_timed_event(unsigned long arg)
 {
 	const unsigned char sync_req[] = { 0x01, 0x7e };
-	const unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
+	unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
 	struct hci_uart *hu = (struct hci_uart *) arg;
 	struct h5 *h5 = hu->priv;
 	struct sk_buff *skb;
@@ -117,8 +129,10 @@ static void h5_timed_event(unsigned long arg)
 	if (h5->state == H5_UNINITIALIZED)
 		h5_link_control(hu, sync_req, sizeof(sync_req));
 
-	if (h5->state == H5_INITIALIZED)
+	if (h5->state == H5_INITIALIZED) {
+		conf_req[2] = h5_cfg_field(h5);
 		h5_link_control(hu, conf_req, sizeof(conf_req));
+	}
 
 	if (h5->state != H5_ACTIVE) {
 		mod_timer(&h5->timer, jiffies + HZ / 10);
@@ -168,6 +182,8 @@ static int h5_open(struct hci_uart *hu)
 	h5->timer.function = h5_timed_event;
 	h5->timer.data = (unsigned long) hu;
 
+	h5->tx_win = H5_TX_WIN_MAX;
+
 	set_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags);
 
 	/* Send initial sync request */
@@ -239,8 +255,8 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 	struct h5 *h5 = hu->priv;
 	const unsigned char sync_req[] = { 0x01, 0x7e };
 	const unsigned char sync_rsp[] = { 0x02, 0x7d };
-	const unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
-	const unsigned char conf_rsp[] = { 0x04, 0x7b, 0x01 };
+	unsigned char conf_req[] = { 0x03, 0xfc, 0x01 };
+	const unsigned char conf_rsp[] = { 0x04, 0x7b };
 	const unsigned char wakeup_req[] = { 0x05, 0xfa };
 	const unsigned char woken_req[] = { 0x06, 0xf9 };
 	const unsigned char sleep_req[] = { 0x07, 0x78 };
@@ -255,6 +271,8 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 	if (H5_HDR_LEN(hdr) < 2)
 		return;
 
+	conf_req[2] = h5_cfg_field(h5);
+
 	if (memcmp(data, sync_req, 2) == 0)
 		h5_link_control(hu, sync_rsp, 2);
 	else if (memcmp(data, sync_rsp, 2) == 0) {
@@ -264,7 +282,9 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
 		h5_link_control(hu, conf_rsp, 2);
 		h5_link_control(hu, conf_req, 3);
 	} else if (memcmp(data, conf_rsp, 2) == 0) {
-		BT_DBG("Three-wire init sequence complete");
+		if (H5_HDR_LEN(hdr) > 2)
+			h5->tx_win = (data[2] & 7);
+		BT_DBG("Three-wire init complete. tx_win %u", h5->tx_win);
 		h5->state = H5_ACTIVE;
 		hci_uart_init_ready(hu);
 		return;
@@ -660,7 +680,7 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
 
 	spin_lock_irqsave_nested(&h5->unack.lock, flags, SINGLE_DEPTH_NESTING);
 
-	if (h5->unack.qlen >= H5_TXWINSIZE)
+	if (h5->unack.qlen >= h5->tx_win)
 	       goto unlock;
 
 	if ((skb = skb_dequeue(&h5->rel)) != NULL) {
-- 
1.7.10.4