Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] Add sysfs speed attribute for wacom bluetooth tablet
From: Jiri Kosina @ 2010-04-23  0:16 UTC (permalink / raw)
  To: Przemo Firszt
  Cc: Bastien Nocera, linux-bluetooth, marcel, Peter Hutterer, Ping
In-Reply-To: <1271965880.3540.16.camel@pldmachine>

On Thu, 22 Apr 2010, Przemo Firszt wrote:

> > The patch looks fine to me, so I have applied it to my tree.
> > 
> > Could you also please send a followup patch which would add 
> > Documentation/ABI entry for the speed attribute?
> Hi Jiri,
> Sorry for the delayed reply. See attached patch for ABI entry.

Applied.

> Is Kernel Version: 2.6.35 OK?

Yup.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

^ permalink raw reply

* Re: [PATCH 2/2] Add sysfs speed attribute for wacom bluetooth tablet
From: Przemo Firszt @ 2010-04-22 19:51 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Bastien Nocera, linux-bluetooth, marcel, Peter Hutterer, Ping
In-Reply-To: <alpine.LNX.2.00.1003220947310.18642@pobox.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Dnia 2010-03-22, pon o godzinie 09:48 +0100, Jiri Kosina pisze:
[..]
> The patch looks fine to me, so I have applied it to my tree.
> 
> Could you also please send a followup patch which would add 
> Documentation/ABI entry for the speed attribute?
Hi Jiri,
Sorry for the delayed reply. See attached patch for ABI entry.
Is Kernel Version: 2.6.35 OK?

Thanks,
Przemo

[-- Attachment #2: 0001-Add-Documentation-ABI-entry-for-wacom-reporting-spee.patch --]
[-- Type: text/x-patch, Size: 1088 bytes --]

>From f118300739d2592646ddb5d58b47df59db002873 Mon Sep 17 00:00:00 2001
From: Przemo Firszt <przemo@firszt.eu>
Date: Wed, 21 Apr 2010 22:16:25 +0100
Subject: [PATCH] Add Documentation/ABI entry for wacom reporting speed attribute.

Signed-off-by: Przemo Firszt <przemo@firszt.eu>
---
 Documentation/ABI/testing/sysfs-wacom |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-wacom

diff --git a/Documentation/ABI/testing/sysfs-wacom b/Documentation/ABI/testing/sysfs-wacom
new file mode 100644
index 0000000..1517976
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-wacom
@@ -0,0 +1,10 @@
+What:		/sys/class/hidraw/hidraw*/device/speed
+Date:		April 2010
+Kernel Version:	2.6.35
+Contact:	linux-bluetooth@vger.kernel.org
+Description:
+		The /sys/class/hidraw/hidraw*/device/speed file controls
+		reporting speed of wacom bluetooth tablet. Reading from
+		this file returns 1 if tablet reports in high speed mode
+		or 0 otherwise. Writing to this file one of these values
+		switches reporting speed.
-- 
1.7.0.2


^ permalink raw reply related

* Re: [patch] bluetooth: handle l2cap_create_connless_pdu() errors
From: Gustavo F. Padovan @ 2010-04-22 12:33 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Marcel Holtmann, David S. Miller, Andrei Emeltchenko,
	linux-bluetooth, netdev, kernel-janitors
In-Reply-To: <20100422095201.GO29647@bicker>

Hi Dan,

* Dan Carpenter <error27@gmail.com> [2010-04-22 11:52:01 +0200]:

> l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or
> ERR_PTR(-EFAULT).
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 99d68c3..9753b69 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1626,7 +1626,10 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
>  	/* Connectionless channel */
>  	if (sk->sk_type == SOCK_DGRAM) {
>  		skb = l2cap_create_connless_pdu(sk, msg, len);
> -		err = l2cap_do_send(sk, skb);
> +		if (IS_ERR(skb))
> +			err = PTR_ERR(skb);
> +		else
> +			err = l2cap_do_send(sk, skb);
>  		goto done;
>  	}

Your fix looks ok, but we have changed l2cap_do_send() to void, so you
have to modify your patch.

That change is not in the bluetooth-testingtree yet, so remote add my git
tree and use the branch ertm.

git://git.profusion.mobi/users/padovan/bluetooth-testing.git

Regards,

-- 
Gustavo F. Padovan
http://padovan.org

^ permalink raw reply

* [patch] bluetooth: handle l2cap_create_connless_pdu() errors
From: Dan Carpenter @ 2010-04-22  9:52 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: David S. Miller, Gustavo F. Padovan, Andrei Emeltchenko,
	linux-bluetooth, netdev, kernel-janitors

l2cap_create_connless_pdu() can sometimes return ERR_PTR(-ENOMEM) or
ERR_PTR(-EFAULT).

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 99d68c3..9753b69 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1626,7 +1626,10 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 	/* Connectionless channel */
 	if (sk->sk_type == SOCK_DGRAM) {
 		skb = l2cap_create_connless_pdu(sk, msg, len);
-		err = l2cap_do_send(sk, skb);
+		if (IS_ERR(skb))
+			err = PTR_ERR(skb);
+		else
+			err = l2cap_do_send(sk, skb);
 		goto done;
 	}
 

^ permalink raw reply related

* [PATCH v5] Add support for the Atheros AR300x Bluetooth Chip
From: suraj @ 2010-04-22  9:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal, gfpadovan
In-Reply-To: <1271845337.15010.1.camel@atheros013-desktop>

This implements the Atheros Bluetooth serial protocol to
support the AR300x Bluetooth chipsets.
The serial protocol implements enhanced power management
features for the AR300x chipsets. 

Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Reviewed-by: Gustavo F. Padovan <gustavo@padovan.org>
Signed-off-by: Suraj <suraj@atheros.com>

---
 drivers/bluetooth/Kconfig     |   14 ++
 drivers/bluetooth/Makefile    |    1 +
 drivers/bluetooth/hci_ath.c   |  378 +++++++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/hci_ldisc.c |    6 +
 drivers/bluetooth/hci_uart.h  |    8 +-
 5 files changed, 406 insertions(+), 1 deletions(-)
 create mode 100755 drivers/bluetooth/hci_ath.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 058fbcc..5546142 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -58,6 +58,20 @@ config BT_HCIUART_BCSP
 
 	  Say Y here to compile support for HCI BCSP protocol.
 
+config BT_HCIUART_ATH
+	bool "Atheros AR300x serial Bluetooth support"
+	depends on BT_HCIUART
+	help
+	  HCIATH (HCI Atheros) is a serial protocol for communication
+	  between the host and Atheros AR300x Bluetooth devices. The
+	  protocol implements enhaned power management features for the
+	  the AR300x chipsets. it lets the controller chip go to sleep
+	  mode if there is no Bluetooth activity for some time and wakes
+	  up the chip in case of a Bluetooth activity. Enable this option
+	  if you have an UART Atheros AR300x serial device.
+
+	  Say Y here to compile support for HCIATH protocol.
+
 config BT_HCIUART_LL
 	bool "HCILL protocol support"
 	depends on BT_HCIUART
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 7e5aed5..1481faa 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -26,4 +26,5 @@ hci_uart-y				:= hci_ldisc.o
 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_ATH)	+= hci_ath.o
 hci_uart-objs				:= $(hci_uart-y)
diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
new file mode 100755
index 0000000..152a1f6
--- /dev/null
+++ b/drivers/bluetooth/hci_ath.c
@@ -0,0 +1,378 @@
+/*
+ * Copyright (c) 2009-2010 Atheros Communications Inc.
+ *
+ *  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/module.h>
+#include <linux/kernel.h>
+
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/errno.h>
+#include <linux/ioctl.h>
+#include <linux/skbuff.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "hci_uart.h"
+
+/* HCIATH receiver States */
+#define HCIATH_W4_PACKET_TYPE			0
+#define HCIATH_W4_EVENT_HDR			1
+#define HCIATH_W4_ACL_HDR			2
+#define HCIATH_W4_SCO_HDR			3
+#define HCIATH_W4_DATA				4
+
+struct ath_struct {
+	struct hci_uart *hu;
+	unsigned int rx_state;
+	unsigned int rx_count;
+	unsigned int cur_sleep;
+
+	spinlock_t hciath_lock;
+	struct sk_buff *rx_skb;
+	struct sk_buff_head txq;
+	wait_queue_head_t wqevt;
+	struct work_struct ctxtsw;
+};
+
+static int ath_wakeup_ar3001(struct tty_struct *tty)
+{
+	struct termios settings;
+	int status = tty->driver->ops->tiocmget(tty, NULL);
+
+	if (status & TIOCM_CTS)
+		return status;
+
+	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
+
+	/* Disable Automatic RTSCTS */
+	settings.c_cflag &= ~CRTSCTS;
+	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	/* Clear RTS first */
+	tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
+	mdelay(20);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	/* Set RTS, wake up board */
+	tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
+	mdelay(20);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
+
+	settings.c_cflag |= CRTSCTS;
+	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
+
+	return status;
+}
+
+static void ath_hci_uart_work(struct work_struct *work)
+{
+	int status;
+	struct ath_struct *ath;
+	struct hci_uart *hu;
+	struct tty_struct *tty;
+
+	ath = container_of(work, struct ath_struct, ctxtsw);
+
+	hu = ath->hu;
+	tty = hu->tty;
+
+	/* verify and wake up controller */
+	if (ath->cur_sleep) {
+		status = ath_wakeup_ar3001(tty);
+
+		if (!(status & TIOCM_CTS))
+			return;
+	}
+
+	/* Ready to send Data */
+	clear_bit(HCI_UART_SENDING, &hu->tx_state);
+	hci_uart_tx_wakeup(hu);
+}
+
+/* Initialize protocol */
+static int ath_open(struct hci_uart *hu)
+{
+	struct ath_struct *ath;
+
+	BT_DBG("hu %p", hu);
+
+	ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
+	if (!ath)
+		return -ENOMEM;
+
+	skb_queue_head_init(&ath->txq);
+	spin_lock_init(&ath->hciath_lock);
+
+	hu->priv = ath;
+	ath->hu = hu;
+
+	init_waitqueue_head(&ath->wqevt);
+	INIT_WORK(&ath->ctxtsw, ath_hci_uart_work);
+
+	return 0;
+}
+
+/* Flush protocol data */
+static int ath_flush(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+
+	BT_DBG("hu %p", hu);
+
+	skb_queue_purge(&ath->txq);
+
+	return 0;
+}
+
+/* Close protocol */
+static int ath_close(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+
+	BT_DBG("hu %p", hu);
+
+	skb_queue_purge(&ath->txq);
+
+	kfree_skb(ath->rx_skb);
+
+	cancel_work_sync(&ath->ctxtsw);
+
+	hu->priv = NULL;
+	kfree(ath);
+
+	return 0;
+}
+
+/* Enqueue frame for transmittion */
+static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
+{
+	struct ath_struct *ath = hu->priv;
+
+	if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
+
+		/* Discard SCO packet. AR300x does not support SCO over HCI */
+		BT_DBG("SCO Packet over HCI received Dropping");
+
+		kfree(skb);
+
+		return 0;
+	}
+
+	BT_DBG("hu %p skb %p", hu, skb);
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	skb_queue_tail(&ath->txq, skb);
+	set_bit(HCI_UART_SENDING, &hu->tx_state);
+
+	schedule_work(&ath->ctxtsw);
+
+	return 0;
+}
+
+static struct sk_buff *ath_dequeue(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+	struct sk_buff *skbuf;
+
+	skbuf = skb_dequeue(&ath->txq);
+
+	if (!skbuf)
+		return NULL;
+
+	/*
+	 * Check if the HCI command is  HCI sleep enable and
+	 * update the sleep enable flag with command parameter.
+	 *
+	 * Value of sleep enable flag will be used later
+	 * to verify if controller has to be woken up before
+	 * sending any packet.
+	 */
+	if (skbuf->data[0] == 0x01 &&
+	    skbuf->data[1] == 0x04 &&
+	    skbuf->data[2] == 0xFC)
+		ath->cur_sleep = skbuf->data[4];
+
+	return skbuf;
+}
+
+static void ath_check_data_len(struct ath_struct *ath, int len)
+{
+	int room = skb_tailroom(ath->rx_skb);
+
+	BT_DBG("len %d room %d", len, room);
+
+	if (len > room) {
+		BT_ERR("Data length is too large");
+		kfree_skb(ath->rx_skb);
+		ath->rx_state = HCIATH_W4_PACKET_TYPE;
+		ath->rx_skb = NULL;
+		ath->rx_count = 0;
+	} else {
+		ath->rx_state = HCIATH_W4_DATA;
+		ath->rx_count = len;
+	}
+}
+
+/* Recv data */
+static int ath_recv(struct hci_uart *hu, void *data, int count)
+{
+	struct ath_struct *ath = hu->priv;
+	char *ptr = data;
+	struct hci_event_hdr *eh;
+	struct hci_acl_hdr *ah;
+	struct hci_sco_hdr *sh;
+	int len, type, dlen;
+
+	BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
+	       ath->rx_state, ath->rx_count);
+
+	while (count) {
+		if (ath->rx_count) {
+
+			len = min_t(unsigned int, ath->rx_count, count);
+			memcpy(skb_put(ath->rx_skb, len), ptr, len);
+			ath->rx_count -= len;
+			count -= len;
+			ptr += len;
+
+			if (ath->rx_count)
+				continue;
+			switch (ath->rx_state) {
+			case HCIATH_W4_DATA:
+				hci_recv_frame(ath->rx_skb);
+				ath->rx_state = HCIATH_W4_PACKET_TYPE;
+				ath->rx_skb = NULL;
+				ath->rx_count = 0;
+				continue;
+
+			case HCIATH_W4_EVENT_HDR:
+				eh = hci_event_hdr(ath->rx_skb);
+
+				BT_DBG("Event header: evt 0x%2.2x plen %d",
+				       eh->evt, eh->plen);
+
+				ath_check_data_len(ath, eh->plen);
+				continue;
+
+			case HCIATH_W4_ACL_HDR:
+				ah = hci_acl_hdr(ath->rx_skb);
+				dlen = __le16_to_cpu(ah->dlen);
+
+				BT_DBG("ACL header: dlen %d", dlen);
+
+				ath_check_data_len(ath, dlen);
+				continue;
+
+			case HCIATH_W4_SCO_HDR:
+				sh = hci_sco_hdr(ath->rx_skb);
+
+				BT_DBG("SCO header: dlen %d", sh->dlen);
+
+				ath_check_data_len(ath, sh->dlen);
+				continue;
+
+			}
+		}
+
+		/* HCIATH_W4_PACKET_TYPE */
+		switch (*ptr) {
+		case HCI_EVENT_PKT:
+			BT_DBG("Event packet");
+			ath->rx_state = HCIATH_W4_EVENT_HDR;
+			ath->rx_count = HCI_EVENT_HDR_SIZE;
+			type = HCI_EVENT_PKT;
+			break;
+
+		case HCI_ACLDATA_PKT:
+			BT_DBG("ACL packet");
+			ath->rx_state = HCIATH_W4_ACL_HDR;
+			ath->rx_count = HCI_ACL_HDR_SIZE;
+			type = HCI_ACLDATA_PKT;
+			break;
+
+		case HCI_SCODATA_PKT:
+			BT_DBG("SCO packet");
+			ath->rx_state = HCIATH_W4_SCO_HDR;
+			ath->rx_count = HCI_SCO_HDR_SIZE;
+			type = HCI_SCODATA_PKT;
+			break;
+
+		default:
+			BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
+			hu->hdev->stat.err_rx++;
+			ptr++;
+			count--;
+			continue;
+
+		};
+		ptr++;
+		count--;
+
+		/* Allocate packet */
+		ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
+		if (!ath->rx_skb) {
+			BT_ERR("Can't allocate mem for new packet");
+			ath->rx_state = HCIATH_W4_PACKET_TYPE;
+			ath->rx_count = 0;
+
+			return -ENOMEM;
+		}
+		ath->rx_skb->dev = (void *)hu->hdev;
+		bt_cb(ath->rx_skb)->pkt_type = type;
+	}
+
+	return count;
+}
+
+static struct hci_uart_proto athp = {
+	.id = HCI_UART_ATH,
+	.open = ath_open,
+	.close = ath_close,
+	.recv = ath_recv,
+	.enqueue = ath_enqueue,
+	.dequeue = ath_dequeue,
+	.flush = ath_flush,
+};
+
+int ath_init(void)
+{
+	int err = hci_uart_register_proto(&athp);
+
+	if (!err)
+		BT_INFO("HCIATH protocol initialized");
+	else
+		BT_ERR("HCIATH protocol registration failed");
+
+	return err;
+}
+
+int ath_deinit(void)
+{
+	return hci_uart_unregister_proto(&athp);
+}
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 76a1abb..7dd76d1 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
 #ifdef CONFIG_BT_HCIUART_LL
 	ll_init();
 #endif
+#ifdef CONFIG_BT_HCIUART_ATH
+	ath_init();
+#endif
 
 	return 0;
 }
@@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
 #ifdef CONFIG_BT_HCIUART_LL
 	ll_deinit();
 #endif
+#ifdef CONFIG_BT_HCIUART_ATH
+	ath_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 50113db..385537f 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -33,13 +33,14 @@
 #define HCIUARTGETDEVICE	_IOR('U', 202, int)
 
 /* UART protocols */
-#define HCI_UART_MAX_PROTO	5
+#define HCI_UART_MAX_PROTO	6
 
 #define HCI_UART_H4	0
 #define HCI_UART_BCSP	1
 #define HCI_UART_3WIRE	2
 #define HCI_UART_H4DS	3
 #define HCI_UART_LL	4
+#define HCI_UART_ATH	5
 
 struct hci_uart;
 
@@ -91,3 +92,8 @@ int bcsp_deinit(void);
 int ll_init(void);
 int ll_deinit(void);
 #endif
+
+#ifdef CONFIG_BT_HCIUART_ATH
+int ath_init(void);
+int ath_deinit(void);
+#endif
-- 
1.7.0

^ permalink raw reply related

* Re: [PATCH v4] Add support for the Atheros AR300x Bluetooth Chip
From: Gustavo F. Padovan @ 2010-04-22  8:59 UTC (permalink / raw)
  To: Suraj Sumangala
  Cc: Suraj Sumangala, linux-bluetooth@vger.kernel.org,
	marcel@holtmann.org, Luis Rodriguez, Jothikumar Mothilal
In-Reply-To: <4BCFF28C.8000606@atheros.com>

Hi Suraj,

* Suraj Sumangala <suraj@atheros.com> [2010-04-22 12:24:04 +0530]:

> 
> Hi Gustavo,
> 
> Gustavo F. Padovan wrote:
> >* suraj <suraj@atheros.com> [2010-04-21 15:52:17 +0530]:
> >
> >>This implements the Atheros Bluetooth serial protocol to
> >>support the AR300x Bluetooth chipsets.
> >>The serial protocol implements enhanced power management
> >>features for the AR300x chipsets.
> >>
> >>Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> >>Signed-off-by: Suraj <suraj@atheros.com>
> >>
> >>---
> >> drivers/bluetooth/Kconfig     |   14 ++
> >> drivers/bluetooth/Makefile    |    1 +
> >> drivers/bluetooth/hci_ath.c   |  378 +++++++++++++++++++++++++++++++++++++++++
> >> drivers/bluetooth/hci_ldisc.c |    6 +
> >> drivers/bluetooth/hci_uart.h  |    8 +-
> >> 5 files changed, 406 insertions(+), 1 deletions(-)
> >> create mode 100755 drivers/bluetooth/hci_ath.c
> >>

..snip..

> >>+
> >>+static void ath_check_data_len(struct ath_struct *ath, int len)
> >>+{
> >>+     int room = skb_tailroom(ath->rx_skb);
> >>+
> >>+     BT_DBG("len %d room %d", len, room);
> >>+
> >>+     if (len > room) {
> >>+             BT_ERR("Data length is too large");
> >>+             kfree_skb(ath->rx_skb);
> >>+             ath->rx_state = HCIATH_W4_PACKET_TYPE;
> >>+             ath->rx_skb = NULL;
> >>+             ath->rx_count = 0;
> >>+     } else {
> >>+             ath->rx_state = HCIATH_W4_DATA;
> >>+             ath->rx_count = len;
> >>+     }
> >>+}
> >>+
> >>+/* Recv data */
> >>+static int ath_recv(struct hci_uart *hu, void *data, int count)
> >>+{
> >>+     struct ath_struct *ath = hu->priv;
> >>+     char *ptr = data;
> >>+     struct hci_event_hdr *eh;
> >>+     struct hci_acl_hdr *ah;
> >>+     struct hci_sco_hdr *sh;
> >>+     int len, type, dlen;
> >>+
> >>+     BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
> >>+            ath->rx_state, ath->rx_count);
> >>+
> >>+     while (count) {
> >>+             if (ath->rx_count) {
> >>+
> >>+                     len = min_t(unsigned int, ath->rx_count, count);
> >>+                     memcpy(skb_put(ath->rx_skb, len), ptr, len);
> >>+                     ath->rx_count -= len;
> >>+                     count -= len;
> >>+                     ptr += len;
> >>+
> >>+                     if (ath->rx_count)
> >>+                             continue;
> >>+                     switch (ath->rx_state) {
> >>+                     case HCIATH_W4_DATA:
> >>+                             hci_recv_frame(ath->rx_skb);
> >>+                             ath->rx_state = HCIATH_W4_PACKET_TYPE;
> >>+                             ath->rx_skb = NULL;
> >>+                             ath->rx_count = 0;
> >>+                             continue;
> >>+
> >>+                     case HCIATH_W4_EVENT_HDR:
> >>+                             eh = (struct hci_event_hdr *)ath->rx_skb->data;
> >
> >Use hci_event_hdr() here like hci_h4.c does.
> >
> >>+
> >>+                             BT_DBG("Event header: evt 0x%2.2x plen %d",
> >>+                                    eh->evt, eh->plen);
> >>+
> >>+                             ath_check_data_len(ath, eh->plen);
> >>+                             continue;
> >>+
> >>+                     case HCIATH_W4_ACL_HDR:
> >>+                             ah = (struct hci_acl_hdr *)ath->rx_skb->data;
> >
> >And hci_acl_hdr() here.
> >
> >>+                             dlen = __le16_to_cpu(ah->dlen);
> >>+
> >>+                             BT_DBG("ACL header: dlen %d", dlen);
> >>+
> >>+                             ath_check_data_len(ath, dlen);
> >>+                             continue;
> >>+
> >>+                     case HCIATH_W4_SCO_HDR:
> >>+                             sh = (struct hci_sco_hdr *)ath->rx_skb->data;
> >
> >hci_sco_hdr() here.
> >
> >>+
> >>+                             BT_DBG("SCO header: dlen %d", sh->dlen);
> >>+
> >>+                             ath_check_data_len(ath, sh->dlen);
> >>+                             continue;
> >>+
> >>+                     }
> >>+             }
> >>+
> >>+             /* HCIATH_W4_PACKET_TYPE */
> >>+             switch (*ptr) {
> >>+             case HCI_EVENT_PKT:
> >>+                     BT_DBG("Event packet");
> >>+                     ath->rx_state = HCIATH_W4_EVENT_HDR;
> >>+                     ath->rx_count = HCI_EVENT_HDR_SIZE;
> >>+                     type = HCI_EVENT_PKT;
> >>+                     break;
> >>+
> >>+             case HCI_ACLDATA_PKT:
> >>+                     BT_DBG("ACL packet");
> >>+                     ath->rx_state = HCIATH_W4_ACL_HDR;
> >>+                     ath->rx_count = HCI_ACL_HDR_SIZE;
> >>+                     type = HCI_ACLDATA_PKT;
> >>+                     break;
> >>+
> >>+             case HCI_SCODATA_PKT:
> >>+                     BT_DBG("SCO packet");
> >>+                     ath->rx_state = HCIATH_W4_SCO_HDR;
> >>+                     ath->rx_count = HCI_SCO_HDR_SIZE;
> >>+                     type = HCI_SCODATA_PKT;
> >>+                     break;
> >>+
> >>+             default:
> >>+                     BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
> >>+                     hu->hdev->stat.err_rx++;
> >>+                     ptr++;
> >>+                     count--;
> >>+                     continue;
> >>+
> >>+             };
> >>+             ptr++;
> >>+             count--;
> >>+
> >>+             /* Allocate packet */
> >>+             ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> >>+             if (!ath->rx_skb) {
> >>+                     BT_ERR("Can't allocate mem for new packet");
> >>+                     ath->rx_state = HCIATH_W4_PACKET_TYPE;
> >>+                     ath->rx_count = 0;
> >>+
> >>+                     return -ENOMEM;
> >>+             }
> >>+             ath->rx_skb->dev = (void *)hu->hdev;
> >>+             bt_cb(ath->rx_skb)->pkt_type = type;
> >>+     }
> >>+
> >>+     return count;
> >>+}
> >
> >Just out of curiosity. I never worked in the driver world, but is it ok
> >to duplicate lots of code when working with drivers? hci_h4.c and this
> >patch share lots of similar code. ath_recv() and h4_recv() are exactly
> >the same except for one line, the ath->rx_count = 0; at case HCIATH_W4_DATA.
> >Does this line makes real difference? recv() is not the only function
> >duplicating code here.
> 
> Let me try to anwer you here. I am also new to the driver world, so
> correct me if I am wrong.
> 
> Both drivers do the same job, expect that the ATH driver does
> something more on the data transmit path. So, the receive path does
> the same thing. That is the reason why both looks same. This could
> possibly change later as new feature will be added to the ATH
> protocol.

Ok. I'm thinking if it is not possible create a separated 'lib' with the
common code. That's why I asked about the code duplication. I don't know
how worth it is, since we have only 3 drivers using similar code.

> 
> 
> The function "hci_recv_fragment()" could potentially replace most of
> the code in the ath_recv() function. But, unfortunately this
> function require the caller to provide the HCI Packet type as
> parameter.
> 
> This defeats all the advantage of "hci_recv_fragment()" in a UART
> HCI transport driver as all types of packets are received through
> the same callback. So the caller will have to write the same messy
> code in ath_recv() to find out the packet type negating all the
> advantages of "hci_recv_fragment()".
> 
> 
> >
> >>+
> >>+static struct hci_uart_proto athp = {
> >>+     .id = HCI_UART_ATH,
> >>+     .open = ath_open,
> >>+     .close = ath_close,
> >>+     .recv = ath_recv,
> >>+     .enqueue = ath_enqueue,
> >>+     .dequeue = ath_dequeue,
> >>+     .flush = ath_flush,
> >>+};
> >>+
> >>+int ath_init(void)
> >>+{
> >>+     int err = hci_uart_register_proto(&athp);
> >>+
> >>+     if (!err)
> >>+             BT_INFO("HCIATH protocol initialized");
> >>+     else
> >>+             BT_ERR("HCIATH protocol registration failed");
> >>+
> >>+     return err;
> >>+}
> >
> >BTW, we never check this return value on hci_ldisc.c, why?
> 
> you, are correct. Just thought of keeping the same signature used by
> other protocol. moreover hci_ldisc.c being a common file used by all
> protocol it is possible that somebody want to check the return value
> at later point of time. So, kept it that way so that we do not have
> a problem then :-D
> 
> Your thoughts?

Yes, your code is right, the return value should be kept.  The problem
is that hci_uart_init() doesn't check any other protocol initialization
(h4, bcsp and ll). My thought is: we really need to check?

Marcel, can you answer that?

> >
> >>+
> >>+int ath_deinit(void)
> >>+{
> >>+     return hci_uart_unregister_proto(&athp);
> >>+}
> >>diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> >>index 76a1abb..7dd76d1 100644
> >>--- a/drivers/bluetooth/hci_ldisc.c
> >>+++ b/drivers/bluetooth/hci_ldisc.c
> >>@@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
> >> #ifdef CONFIG_BT_HCIUART_LL
> >>      ll_init();
> >> #endif
> >>+#ifdef CONFIG_BT_HCIUART_ATH
> >>+     ath_init();
> >>+#endif
> >>
> >>      return 0;
> >> }
> >>@@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
> >> #ifdef CONFIG_BT_HCIUART_LL
> >>      ll_deinit();
> >> #endif
> >>+#ifdef CONFIG_BT_HCIUART_ATH
> >>+     ath_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 50113db..385537f 100644
> >>--- a/drivers/bluetooth/hci_uart.h
> >>+++ b/drivers/bluetooth/hci_uart.h
> >>@@ -33,13 +33,14 @@
> >> #define HCIUARTGETDEVICE     _IOR('U', 202, int)
> >>
> >> /* UART protocols */
> >>-#define HCI_UART_MAX_PROTO   5
> >>+#define HCI_UART_MAX_PROTO   6
> >>
> >> #define HCI_UART_H4  0
> >> #define HCI_UART_BCSP        1
> >> #define HCI_UART_3WIRE       2
> >> #define HCI_UART_H4DS        3
> >> #define HCI_UART_LL  4
> >>+#define HCI_UART_ATH 5
> >>
> >> struct hci_uart;
> >>
> >>@@ -91,3 +92,8 @@ int bcsp_deinit(void);
> >> int ll_init(void);
> >> int ll_deinit(void);
> >> #endif
> >>+
> >>+#ifdef CONFIG_BT_HCIUART_ATH
> >>+int ath_init(void);
> >>+int ath_deinit(void);
> >>+#endif
> >>--
> >>1.7.0
> >>
> >>
> >>
> >>
> >>
> >
> >--
> >Gustavo F. Padovan
> >http://padovan.org
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Gustavo F. Padovan
http://padovan.org

^ permalink raw reply

* Re: [PATCH v4] Add support for the Atheros AR300x Bluetooth Chip
From: Suraj Sumangala @ 2010-04-22  6:54 UTC (permalink / raw)
  To: Gustavo F. Padovan
  Cc: Suraj Sumangala, linux-bluetooth@vger.kernel.org,
	marcel@holtmann.org, Luis Rodriguez, Jothikumar Mothilal
In-Reply-To: <20100422061056.GE22959@vigoh>


Hi Gustavo,

Gustavo F. Padovan wrote:
> * suraj <suraj@atheros.com> [2010-04-21 15:52:17 +0530]:
> 
>> This implements the Atheros Bluetooth serial protocol to
>> support the AR300x Bluetooth chipsets.
>> The serial protocol implements enhanced power management
>> features for the AR300x chipsets.
>>
>> Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>> Signed-off-by: Suraj <suraj@atheros.com>
>>
>> ---
>>  drivers/bluetooth/Kconfig     |   14 ++
>>  drivers/bluetooth/Makefile    |    1 +
>>  drivers/bluetooth/hci_ath.c   |  378 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/bluetooth/hci_ldisc.c |    6 +
>>  drivers/bluetooth/hci_uart.h  |    8 +-
>>  5 files changed, 406 insertions(+), 1 deletions(-)
>>  create mode 100755 drivers/bluetooth/hci_ath.c
>>
>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>> index 058fbcc..5546142 100644
>> --- a/drivers/bluetooth/Kconfig
>> +++ b/drivers/bluetooth/Kconfig
>> @@ -58,6 +58,20 @@ config BT_HCIUART_BCSP
>>
>>         Say Y here to compile support for HCI BCSP protocol.
>>
>> +config BT_HCIUART_ATH
>> +     bool "Atheros AR300x serial Bluetooth support"
>> +     depends on BT_HCIUART
>> +     help
>> +       HCIATH (HCI Atheros) is a serial protocol for communication
>> +       between the host and Atheros AR300x Bluetooth devices. The
>> +       protocol implements enhaned power management features for the
>> +       the AR300x chipsets. it lets the controller chip go to sleep
>> +       mode if there is no Bluetooth activity for some time and wakes
>> +       up the chip in case of a Bluetooth activity. Enable this option
>> +       if you have an UART Atheros AR300x serial device.
>> +
>> +       Say Y here to compile support for HCIATH protocol.
>> +
>>  config BT_HCIUART_LL
>>       bool "HCILL protocol support"
>>       depends on BT_HCIUART
>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>> index 7e5aed5..1481faa 100644
>> --- a/drivers/bluetooth/Makefile
>> +++ b/drivers/bluetooth/Makefile
>> @@ -26,4 +26,5 @@ hci_uart-y                          := hci_ldisc.o
>>  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_ATH)    += hci_ath.o
>>  hci_uart-objs                                := $(hci_uart-y)
>> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
>> new file mode 100755
>> index 0000000..7e99559
>> --- /dev/null
>> +++ b/drivers/bluetooth/hci_ath.c
>> @@ -0,0 +1,378 @@
>> +/*
>> + * Copyright (c) 2009-2010 Atheros Communications Inc.
>> + *
>> + *  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/module.h>
>> +#include <linux/kernel.h>
>> +
>> +#include <linux/init.h>
>> +#include <linux/slab.h>
>> +#include <linux/tty.h>
>> +#include <linux/errno.h>
>> +#include <linux/ioctl.h>
>> +#include <linux/skbuff.h>
>> +
>> +#include <net/bluetooth/bluetooth.h>
>> +#include <net/bluetooth/hci_core.h>
>> +
>> +#include "hci_uart.h"
>> +
>> +/* HCIATH receiver States */
>> +#define HCIATH_W4_PACKET_TYPE                        0
>> +#define HCIATH_W4_EVENT_HDR                  1
>> +#define HCIATH_W4_ACL_HDR                    2
>> +#define HCIATH_W4_SCO_HDR                    3
>> +#define HCIATH_W4_DATA                               4
>> +
>> +struct ath_struct {
>> +     struct hci_uart *hu;
>> +     unsigned int rx_state;
>> +     unsigned int rx_count;
>> +     unsigned int cur_sleep;
>> +
>> +     spinlock_t hciath_lock;
>> +     struct sk_buff *rx_skb;
>> +     struct sk_buff_head txq;
>> +     wait_queue_head_t wqevt;
>> +     struct work_struct ctxtsw;
>> +};
>> +
>> +static int ath_wakeup_ar3001(struct tty_struct *tty)
>> +{
>> +     struct termios settings;
>> +     int status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     if (status & TIOCM_CTS)
>> +             return status;
>> +
>> +     n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
>> +
>> +     /* Disable Automatic RTSCTS */
>> +     settings.c_cflag &= ~CRTSCTS;
>> +     n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
>> +
>> +     status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     /* Clear RTS first */
>> +     tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
>> +     mdelay(20);
>> +
>> +     status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     /* Set RTS, wake up board */
>> +     tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
>> +     mdelay(20);
>> +
>> +     status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
>> +
>> +     settings.c_cflag |= CRTSCTS;
>> +     n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
>> +
>> +     return status;
>> +}
>> +
>> +static void ath_hci_uart_work(struct work_struct *work)
>> +{
>> +     int status;
>> +     struct ath_struct *ath;
>> +     struct hci_uart *hu;
>> +     struct tty_struct *tty;
>> +
>> +     ath = container_of(work, struct ath_struct, ctxtsw);
>> +
>> +     hu = ath->hu;
>> +     tty = hu->tty;
>> +
>> +     /* verify and wake up controller */
>> +     if (ath->cur_sleep) {
>> +             status = ath_wakeup_ar3001(tty);
>> +
>> +             if (!(status & TIOCM_CTS))
>> +                     return;
>> +     }
>> +
>> +     /* Ready to send Data */
>> +     clear_bit(HCI_UART_SENDING, &hu->tx_state);
>> +     hci_uart_tx_wakeup(hu);
>> +}
>> +
>> +/* Initialize protocol */
>> +static int ath_open(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath;
>> +
>> +     BT_DBG("hu %p", hu);
>> +
>> +     ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
>> +     if (!ath)
>> +             return -ENOMEM;
>> +
>> +     skb_queue_head_init(&ath->txq);
>> +     spin_lock_init(&ath->hciath_lock);
>> +
>> +     hu->priv = ath;
>> +     ath->hu = hu;
>> +
>> +     init_waitqueue_head(&ath->wqevt);
>> +     INIT_WORK(&ath->ctxtsw, ath_hci_uart_work);
>> +
>> +     return 0;
>> +}
>> +
>> +/* Flush protocol data */
>> +static int ath_flush(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +
>> +     BT_DBG("hu %p", hu);
>> +
>> +     skb_queue_purge(&ath->txq);
>> +
>> +     return 0;
>> +}
>> +
>> +/* Close protocol */
>> +static int ath_close(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +
>> +     BT_DBG("hu %p", hu);
>> +
>> +     skb_queue_purge(&ath->txq);
>> +
>> +     kfree_skb(ath->rx_skb);
>> +
>> +     cancel_work_sync(&ath->ctxtsw);
>> +
>> +     hu->priv = NULL;
>> +     kfree(ath);
>> +
>> +     return 0;
>> +}
>> +
>> +/* Enqueue frame for transmittion */
>> +static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +
>> +     if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
>> +
>> +             /* Discard SCO packet. AR3001 does not support SCO over HCI */
>> +             BT_DBG("SCO Packet over HCI received Dropping");
>> +
>> +             kfree(skb);
>> +
>> +             return 0;
>> +     }
>> +
>> +     BT_DBG("hu %p skb %p", hu, skb);
>> +
>> +     /* Prepend skb with frame type */
>> +     memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
>> +
>> +     skb_queue_tail(&ath->txq, skb);
>> +     set_bit(HCI_UART_SENDING, &hu->tx_state);
>> +
>> +     schedule_work(&ath->ctxtsw);
>> +
>> +     return 0;
>> +}
>> +
>> +static struct sk_buff *ath_dequeue(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +     struct sk_buff *skbuf;
>> +
>> +     skbuf = skb_dequeue(&ath->txq);
>> +
>> +     if (!skbuf)
>> +             return NULL;
>> +
>> +     /*
>> +      * Check if the HCI command is  HCI sleep enable and
>> +      * update the sleep enable flag with command parameter.
>> +      *
>> +      * Value of sleep enable flag will be used later
>> +      * to verify if controller has to be woken up before
>> +      * sending any packet.
>> +      */
>> +     if (skbuf->data[0] == 0x01 &&
>> +         skbuf->data[1] == 0x04 &&
>> +         skbuf->data[2] == 0xFC)
>> +             ath->cur_sleep = skbuf->data[4];
>> +
>> +     return skbuf;
>> +}
>> +
>> +static void ath_check_data_len(struct ath_struct *ath, int len)
>> +{
>> +     int room = skb_tailroom(ath->rx_skb);
>> +
>> +     BT_DBG("len %d room %d", len, room);
>> +
>> +     if (len > room) {
>> +             BT_ERR("Data length is too large");
>> +             kfree_skb(ath->rx_skb);
>> +             ath->rx_state = HCIATH_W4_PACKET_TYPE;
>> +             ath->rx_skb = NULL;
>> +             ath->rx_count = 0;
>> +     } else {
>> +             ath->rx_state = HCIATH_W4_DATA;
>> +             ath->rx_count = len;
>> +     }
>> +}
>> +
>> +/* Recv data */
>> +static int ath_recv(struct hci_uart *hu, void *data, int count)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +     char *ptr = data;
>> +     struct hci_event_hdr *eh;
>> +     struct hci_acl_hdr *ah;
>> +     struct hci_sco_hdr *sh;
>> +     int len, type, dlen;
>> +
>> +     BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
>> +            ath->rx_state, ath->rx_count);
>> +
>> +     while (count) {
>> +             if (ath->rx_count) {
>> +
>> +                     len = min_t(unsigned int, ath->rx_count, count);
>> +                     memcpy(skb_put(ath->rx_skb, len), ptr, len);
>> +                     ath->rx_count -= len;
>> +                     count -= len;
>> +                     ptr += len;
>> +
>> +                     if (ath->rx_count)
>> +                             continue;
>> +                     switch (ath->rx_state) {
>> +                     case HCIATH_W4_DATA:
>> +                             hci_recv_frame(ath->rx_skb);
>> +                             ath->rx_state = HCIATH_W4_PACKET_TYPE;
>> +                             ath->rx_skb = NULL;
>> +                             ath->rx_count = 0;
>> +                             continue;
>> +
>> +                     case HCIATH_W4_EVENT_HDR:
>> +                             eh = (struct hci_event_hdr *)ath->rx_skb->data;
> 
> Use hci_event_hdr() here like hci_h4.c does.
> 
>> +
>> +                             BT_DBG("Event header: evt 0x%2.2x plen %d",
>> +                                    eh->evt, eh->plen);
>> +
>> +                             ath_check_data_len(ath, eh->plen);
>> +                             continue;
>> +
>> +                     case HCIATH_W4_ACL_HDR:
>> +                             ah = (struct hci_acl_hdr *)ath->rx_skb->data;
> 
> And hci_acl_hdr() here.
> 
>> +                             dlen = __le16_to_cpu(ah->dlen);
>> +
>> +                             BT_DBG("ACL header: dlen %d", dlen);
>> +
>> +                             ath_check_data_len(ath, dlen);
>> +                             continue;
>> +
>> +                     case HCIATH_W4_SCO_HDR:
>> +                             sh = (struct hci_sco_hdr *)ath->rx_skb->data;
> 
> hci_sco_hdr() here.
> 
>> +
>> +                             BT_DBG("SCO header: dlen %d", sh->dlen);
>> +
>> +                             ath_check_data_len(ath, sh->dlen);
>> +                             continue;
>> +
>> +                     }
>> +             }
>> +
>> +             /* HCIATH_W4_PACKET_TYPE */
>> +             switch (*ptr) {
>> +             case HCI_EVENT_PKT:
>> +                     BT_DBG("Event packet");
>> +                     ath->rx_state = HCIATH_W4_EVENT_HDR;
>> +                     ath->rx_count = HCI_EVENT_HDR_SIZE;
>> +                     type = HCI_EVENT_PKT;
>> +                     break;
>> +
>> +             case HCI_ACLDATA_PKT:
>> +                     BT_DBG("ACL packet");
>> +                     ath->rx_state = HCIATH_W4_ACL_HDR;
>> +                     ath->rx_count = HCI_ACL_HDR_SIZE;
>> +                     type = HCI_ACLDATA_PKT;
>> +                     break;
>> +
>> +             case HCI_SCODATA_PKT:
>> +                     BT_DBG("SCO packet");
>> +                     ath->rx_state = HCIATH_W4_SCO_HDR;
>> +                     ath->rx_count = HCI_SCO_HDR_SIZE;
>> +                     type = HCI_SCODATA_PKT;
>> +                     break;
>> +
>> +             default:
>> +                     BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
>> +                     hu->hdev->stat.err_rx++;
>> +                     ptr++;
>> +                     count--;
>> +                     continue;
>> +
>> +             };
>> +             ptr++;
>> +             count--;
>> +
>> +             /* Allocate packet */
>> +             ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
>> +             if (!ath->rx_skb) {
>> +                     BT_ERR("Can't allocate mem for new packet");
>> +                     ath->rx_state = HCIATH_W4_PACKET_TYPE;
>> +                     ath->rx_count = 0;
>> +
>> +                     return -ENOMEM;
>> +             }
>> +             ath->rx_skb->dev = (void *)hu->hdev;
>> +             bt_cb(ath->rx_skb)->pkt_type = type;
>> +     }
>> +
>> +     return count;
>> +}
> 
> Just out of curiosity. I never worked in the driver world, but is it ok
> to duplicate lots of code when working with drivers? hci_h4.c and this
> patch share lots of similar code. ath_recv() and h4_recv() are exactly
> the same except for one line, the ath->rx_count = 0; at case HCIATH_W4_DATA.
> Does this line makes real difference? recv() is not the only function
> duplicating code here.

Let me try to anwer you here. I am also new to the driver world, so 
correct me if I am wrong.

Both drivers do the same job, expect that the ATH driver does something 
more on the data transmit path. So, the receive path does the same 
thing. That is the reason why both looks same. This could possibly 
change later as new feature will be added to the ATH protocol.


The function "hci_recv_fragment()" could potentially replace most of the 
code in the ath_recv() function. But, unfortunately this function 
require the caller to provide the HCI Packet type as parameter.

This defeats all the advantage of "hci_recv_fragment()" in a UART HCI 
transport driver as all types of packets are received through the same 
callback. So the caller will have to write the same messy code in 
ath_recv() to find out the packet type negating all the advantages of 
"hci_recv_fragment()".


> 
>> +
>> +static struct hci_uart_proto athp = {
>> +     .id = HCI_UART_ATH,
>> +     .open = ath_open,
>> +     .close = ath_close,
>> +     .recv = ath_recv,
>> +     .enqueue = ath_enqueue,
>> +     .dequeue = ath_dequeue,
>> +     .flush = ath_flush,
>> +};
>> +
>> +int ath_init(void)
>> +{
>> +     int err = hci_uart_register_proto(&athp);
>> +
>> +     if (!err)
>> +             BT_INFO("HCIATH protocol initialized");
>> +     else
>> +             BT_ERR("HCIATH protocol registration failed");
>> +
>> +     return err;
>> +}
> 
> BTW, we never check this return value on hci_ldisc.c, why?

you, are correct. Just thought of keeping the same signature used by 
other protocol. moreover hci_ldisc.c being a common file used by all 
protocol it is possible that somebody want to check the return value at 
later point of time. So, kept it that way so that we do not have a 
problem then :-D

Your thoughts?
> 
>> +
>> +int ath_deinit(void)
>> +{
>> +     return hci_uart_unregister_proto(&athp);
>> +}
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index 76a1abb..7dd76d1 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
>>  #ifdef CONFIG_BT_HCIUART_LL
>>       ll_init();
>>  #endif
>> +#ifdef CONFIG_BT_HCIUART_ATH
>> +     ath_init();
>> +#endif
>>
>>       return 0;
>>  }
>> @@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
>>  #ifdef CONFIG_BT_HCIUART_LL
>>       ll_deinit();
>>  #endif
>> +#ifdef CONFIG_BT_HCIUART_ATH
>> +     ath_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 50113db..385537f 100644
>> --- a/drivers/bluetooth/hci_uart.h
>> +++ b/drivers/bluetooth/hci_uart.h
>> @@ -33,13 +33,14 @@
>>  #define HCIUARTGETDEVICE     _IOR('U', 202, int)
>>
>>  /* UART protocols */
>> -#define HCI_UART_MAX_PROTO   5
>> +#define HCI_UART_MAX_PROTO   6
>>
>>  #define HCI_UART_H4  0
>>  #define HCI_UART_BCSP        1
>>  #define HCI_UART_3WIRE       2
>>  #define HCI_UART_H4DS        3
>>  #define HCI_UART_LL  4
>> +#define HCI_UART_ATH 5
>>
>>  struct hci_uart;
>>
>> @@ -91,3 +92,8 @@ int bcsp_deinit(void);
>>  int ll_init(void);
>>  int ll_deinit(void);
>>  #endif
>> +
>> +#ifdef CONFIG_BT_HCIUART_ATH
>> +int ath_init(void);
>> +int ath_deinit(void);
>> +#endif
>> --
>> 1.7.0
>>
>>
>>
>>
>>
> 
> --
> Gustavo F. Padovan
> http://padovan.org

^ permalink raw reply

* Re: [PATCH] Bluetooth: Fix return value when bt_skb_alloc fails
From: Gustavo F. Padovan @ 2010-04-22  6:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel
In-Reply-To: <1271917230-30439-1-git-send-email-gustavo@padovan.org>

Hi Marcel,

* Gustavo F. Padovan <gustavo@padovan.org> [2010-04-22 03:20:30 -0300]:

> From: Gustavo F. Padovan <padovan@profusion.mobi>
> 
> Set the proper error(ENOMEM), instead of just return 0.
> 
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
>  drivers/bluetooth/hci_h4.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
> index c0ce813..3f038f5 100644
> --- a/drivers/bluetooth/hci_h4.c
> +++ b/drivers/bluetooth/hci_h4.c
> @@ -246,7 +246,7 @@ static int h4_recv(struct hci_uart *hu, void *data, int count)
>  			BT_ERR("Can't allocate mem for new packet");
>  			h4->rx_state = H4_W4_PACKET_TYPE;
>  			h4->rx_count = 0;
> -			return 0;
> +			return -ENOMEM;
>  		}
>  
>  		h4->rx_skb->dev = (void *) hu->hdev;
> -- 
> 1.7.0.4
> 

Ignore this one, I have found a similar issue in other file, just sent
the new fix to the list.

-- 
Gustavo F. Padovan
http://padovan.org

^ permalink raw reply

* [PATCH 3/3] Bluetooth: Fix wrong packet type count increment
From: Gustavo F. Padovan @ 2010-04-22  6:51 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1271919099-22107-2-git-send-email-gustavo@padovan.org>

From: Gustavo F. Padovan <padovan@profusion.mobi>

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 drivers/bluetooth/hci_vhci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index bb0aefd..3aa7b2a 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -157,7 +157,7 @@ static inline ssize_t vhci_put_user(struct vhci_data *data,
 		break;
 
 	case HCI_SCODATA_PKT:
-		data->hdev->stat.cmd_tx++;
+		data->hdev->stat.sco_tx++;
 		break;
 	};
 
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 2/3] Bluetooth: Use the proper function cast to get hdr data
From: Gustavo F. Padovan @ 2010-04-22  6:51 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1271919099-22107-1-git-send-email-gustavo@padovan.org>

From: Gustavo F. Padovan <padovan@profusion.mobi>

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 drivers/bluetooth/hci_ll.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 91e44df..fb8445c 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -402,7 +402,7 @@ static int ll_recv(struct hci_uart *hu, void *data, int count)
 				continue;
 
 			case HCILL_W4_EVENT_HDR:
-				eh = (struct hci_event_hdr *) ll->rx_skb->data;
+				eh = hci_event_hdr(ll->rx_skb);
 
 				BT_DBG("Event header: evt 0x%2.2x plen %d", eh->evt, eh->plen);
 
@@ -410,7 +410,7 @@ static int ll_recv(struct hci_uart *hu, void *data, int count)
 				continue;
 
 			case HCILL_W4_ACL_HDR:
-				ah = (struct hci_acl_hdr *) ll->rx_skb->data;
+				ah = hci_acl_hdr(ll->rx_skb);
 				dlen = __le16_to_cpu(ah->dlen);
 
 				BT_DBG("ACL header: dlen %d", dlen);
@@ -419,7 +419,7 @@ static int ll_recv(struct hci_uart *hu, void *data, int count)
 				continue;
 
 			case HCILL_W4_SCO_HDR:
-				sh = (struct hci_sco_hdr *) ll->rx_skb->data;
+				sh = hci_sco_hdr(ll->rx_skb);
 
 				BT_DBG("SCO header: dlen %d", sh->dlen);
 
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 1/3] Bluetooth: Fix return value when bt_skb_alloc fails
From: Gustavo F. Padovan @ 2010-04-22  6:51 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

Set the proper error(ENOMEM), instead of just return 0.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 drivers/bluetooth/hci_h4.c |    2 +-
 drivers/bluetooth/hci_ll.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
index c0ce813..3f038f5 100644
--- a/drivers/bluetooth/hci_h4.c
+++ b/drivers/bluetooth/hci_h4.c
@@ -246,7 +246,7 @@ static int h4_recv(struct hci_uart *hu, void *data, int count)
 			BT_ERR("Can't allocate mem for new packet");
 			h4->rx_state = H4_W4_PACKET_TYPE;
 			h4->rx_count = 0;
-			return 0;
+			return -ENOMEM;
 		}
 
 		h4->rx_skb->dev = (void *) hu->hdev;
diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 5c65014..91e44df 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -491,7 +491,7 @@ static int ll_recv(struct hci_uart *hu, void *data, int count)
 			BT_ERR("Can't allocate mem for new packet");
 			ll->rx_state = HCILL_W4_PACKET_TYPE;
 			ll->rx_count = 0;
-			return 0;
+			return -ENOMEM;
 		}
 
 		ll->rx_skb->dev = (void *) hu->hdev;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH] Bluetooth: Fix return value when bt_skb_alloc fails
From: Gustavo F. Padovan @ 2010-04-22  6:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

Set the proper error(ENOMEM), instead of just return 0.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 drivers/bluetooth/hci_h4.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
index c0ce813..3f038f5 100644
--- a/drivers/bluetooth/hci_h4.c
+++ b/drivers/bluetooth/hci_h4.c
@@ -246,7 +246,7 @@ static int h4_recv(struct hci_uart *hu, void *data, int count)
 			BT_ERR("Can't allocate mem for new packet");
 			h4->rx_state = H4_W4_PACKET_TYPE;
 			h4->rx_count = 0;
-			return 0;
+			return -ENOMEM;
 		}
 
 		h4->rx_skb->dev = (void *) hu->hdev;
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH v4] Add support for the Atheros AR300x Bluetooth Chip
From: Gustavo F. Padovan @ 2010-04-22  6:10 UTC (permalink / raw)
  To: suraj; +Cc: linux-bluetooth, marcel, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1271845337.15010.1.camel@atheros013-desktop>

* suraj <suraj@atheros.com> [2010-04-21 15:52:17 +0530]:

> This implements the Atheros Bluetooth serial protocol to
> support the AR300x Bluetooth chipsets.
> The serial protocol implements enhanced power management
> features for the AR300x chipsets.
> 
> Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> Signed-off-by: Suraj <suraj@atheros.com>
> 
> ---
>  drivers/bluetooth/Kconfig     |   14 ++
>  drivers/bluetooth/Makefile    |    1 +
>  drivers/bluetooth/hci_ath.c   |  378 +++++++++++++++++++++++++++++++++++++++++
>  drivers/bluetooth/hci_ldisc.c |    6 +
>  drivers/bluetooth/hci_uart.h  |    8 +-
>  5 files changed, 406 insertions(+), 1 deletions(-)
>  create mode 100755 drivers/bluetooth/hci_ath.c
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 058fbcc..5546142 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -58,6 +58,20 @@ config BT_HCIUART_BCSP
>  
>  	  Say Y here to compile support for HCI BCSP protocol.
>  
> +config BT_HCIUART_ATH
> +	bool "Atheros AR300x serial Bluetooth support"
> +	depends on BT_HCIUART
> +	help
> +	  HCIATH (HCI Atheros) is a serial protocol for communication
> +	  between the host and Atheros AR300x Bluetooth devices. The
> +	  protocol implements enhaned power management features for the
> +	  the AR300x chipsets. it lets the controller chip go to sleep
> +	  mode if there is no Bluetooth activity for some time and wakes
> +	  up the chip in case of a Bluetooth activity. Enable this option
> +	  if you have an UART Atheros AR300x serial device.
> +
> +	  Say Y here to compile support for HCIATH protocol.
> +
>  config BT_HCIUART_LL
>  	bool "HCILL protocol support"
>  	depends on BT_HCIUART
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 7e5aed5..1481faa 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -26,4 +26,5 @@ hci_uart-y				:= hci_ldisc.o
>  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_ATH)	+= hci_ath.o
>  hci_uart-objs				:= $(hci_uart-y)
> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
> new file mode 100755
> index 0000000..7e99559
> --- /dev/null
> +++ b/drivers/bluetooth/hci_ath.c
> @@ -0,0 +1,378 @@
> +/*
> + * Copyright (c) 2009-2010 Atheros Communications Inc.
> + *
> + *  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/module.h>
> +#include <linux/kernel.h>
> +
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/errno.h>
> +#include <linux/ioctl.h>
> +#include <linux/skbuff.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include "hci_uart.h"
> +
> +/* HCIATH receiver States */
> +#define HCIATH_W4_PACKET_TYPE			0
> +#define HCIATH_W4_EVENT_HDR			1
> +#define HCIATH_W4_ACL_HDR			2
> +#define HCIATH_W4_SCO_HDR			3
> +#define HCIATH_W4_DATA				4
> +
> +struct ath_struct {
> +	struct hci_uart *hu;
> +	unsigned int rx_state;
> +	unsigned int rx_count;
> +	unsigned int cur_sleep;
> +
> +	spinlock_t hciath_lock;
> +	struct sk_buff *rx_skb;
> +	struct sk_buff_head txq;
> +	wait_queue_head_t wqevt;
> +	struct work_struct ctxtsw;
> +};
> +
> +static int ath_wakeup_ar3001(struct tty_struct *tty)
> +{
> +	struct termios settings;
> +	int status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	if (status & TIOCM_CTS)
> +		return status;
> +
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	/* Disable Automatic RTSCTS */
> +	settings.c_cflag &= ~CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	/* Clear RTS first */
> +	tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	/* Set RTS, wake up board */
> +	tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	settings.c_cflag |= CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +
> +	return status;
> +}
> +
> +static void ath_hci_uart_work(struct work_struct *work)
> +{
> +	int status;
> +	struct ath_struct *ath;
> +	struct hci_uart *hu;
> +	struct tty_struct *tty;
> +
> +	ath = container_of(work, struct ath_struct, ctxtsw);
> +
> +	hu = ath->hu;
> +	tty = hu->tty;
> +
> +	/* verify and wake up controller */
> +	if (ath->cur_sleep) {
> +		status = ath_wakeup_ar3001(tty);
> +
> +		if (!(status & TIOCM_CTS))
> +			return;
> +	}
> +
> +	/* Ready to send Data */
> +	clear_bit(HCI_UART_SENDING, &hu->tx_state);
> +	hci_uart_tx_wakeup(hu);
> +}
> +
> +/* Initialize protocol */
> +static int ath_open(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
> +	if (!ath)
> +		return -ENOMEM;
> +
> +	skb_queue_head_init(&ath->txq);
> +	spin_lock_init(&ath->hciath_lock);
> +
> +	hu->priv = ath;
> +	ath->hu = hu;
> +
> +	init_waitqueue_head(&ath->wqevt);
> +	INIT_WORK(&ath->ctxtsw, ath_hci_uart_work);
> +
> +	return 0;
> +}
> +
> +/* Flush protocol data */
> +static int ath_flush(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	skb_queue_purge(&ath->txq);
> +
> +	return 0;
> +}
> +
> +/* Close protocol */
> +static int ath_close(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	skb_queue_purge(&ath->txq);
> +
> +	kfree_skb(ath->rx_skb);
> +
> +	cancel_work_sync(&ath->ctxtsw);
> +
> +	hu->priv = NULL;
> +	kfree(ath);
> +
> +	return 0;
> +}
> +
> +/* Enqueue frame for transmittion */
> +static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
> +
> +		/* Discard SCO packet. AR3001 does not support SCO over HCI */
> +		BT_DBG("SCO Packet over HCI received Dropping");
> +
> +		kfree(skb);
> +
> +		return 0;
> +	}
> +
> +	BT_DBG("hu %p skb %p", hu, skb);
> +
> +	/* Prepend skb with frame type */
> +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> +	skb_queue_tail(&ath->txq, skb);
> +	set_bit(HCI_UART_SENDING, &hu->tx_state);
> +
> +	schedule_work(&ath->ctxtsw);
> +
> +	return 0;
> +}
> +
> +static struct sk_buff *ath_dequeue(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	struct sk_buff *skbuf;
> +
> +	skbuf = skb_dequeue(&ath->txq);
> +
> +	if (!skbuf)
> +		return NULL;
> +
> +	/*
> +	 * Check if the HCI command is  HCI sleep enable and
> +	 * update the sleep enable flag with command parameter.
> +	 *
> +	 * Value of sleep enable flag will be used later
> +	 * to verify if controller has to be woken up before
> +	 * sending any packet.
> +	 */
> +	if (skbuf->data[0] == 0x01 &&
> +	    skbuf->data[1] == 0x04 &&
> +	    skbuf->data[2] == 0xFC)
> +		ath->cur_sleep = skbuf->data[4];
> +
> +	return skbuf;
> +}
> +
> +static void ath_check_data_len(struct ath_struct *ath, int len)
> +{
> +	int room = skb_tailroom(ath->rx_skb);
> +
> +	BT_DBG("len %d room %d", len, room);
> +
> +	if (len > room) {
> +		BT_ERR("Data length is too large");
> +		kfree_skb(ath->rx_skb);
> +		ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +		ath->rx_skb = NULL;
> +		ath->rx_count = 0;
> +	} else {
> +		ath->rx_state = HCIATH_W4_DATA;
> +		ath->rx_count = len;
> +	}
> +}
> +
> +/* Recv data */
> +static int ath_recv(struct hci_uart *hu, void *data, int count)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	char *ptr = data;
> +	struct hci_event_hdr *eh;
> +	struct hci_acl_hdr *ah;
> +	struct hci_sco_hdr *sh;
> +	int len, type, dlen;
> +
> +	BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
> +	       ath->rx_state, ath->rx_count);
> +
> +	while (count) {
> +		if (ath->rx_count) {
> +
> +			len = min_t(unsigned int, ath->rx_count, count);
> +			memcpy(skb_put(ath->rx_skb, len), ptr, len);
> +			ath->rx_count -= len;
> +			count -= len;
> +			ptr += len;
> +
> +			if (ath->rx_count)
> +				continue;
> +			switch (ath->rx_state) {
> +			case HCIATH_W4_DATA:
> +				hci_recv_frame(ath->rx_skb);
> +				ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +				ath->rx_skb = NULL;
> +				ath->rx_count = 0;
> +				continue;
> +
> +			case HCIATH_W4_EVENT_HDR:
> +				eh = (struct hci_event_hdr *)ath->rx_skb->data;

Use hci_event_hdr() here like hci_h4.c does.

> +
> +				BT_DBG("Event header: evt 0x%2.2x plen %d",
> +				       eh->evt, eh->plen);
> +
> +				ath_check_data_len(ath, eh->plen);
> +				continue;
> +
> +			case HCIATH_W4_ACL_HDR:
> +				ah = (struct hci_acl_hdr *)ath->rx_skb->data;

And hci_acl_hdr() here.

> +				dlen = __le16_to_cpu(ah->dlen);
> +
> +				BT_DBG("ACL header: dlen %d", dlen);
> +
> +				ath_check_data_len(ath, dlen);
> +				continue;
> +
> +			case HCIATH_W4_SCO_HDR:
> +				sh = (struct hci_sco_hdr *)ath->rx_skb->data;

hci_sco_hdr() here.

> +
> +				BT_DBG("SCO header: dlen %d", sh->dlen);
> +
> +				ath_check_data_len(ath, sh->dlen);
> +				continue;
> +
> +			}
> +		}
> +
> +		/* HCIATH_W4_PACKET_TYPE */
> +		switch (*ptr) {
> +		case HCI_EVENT_PKT:
> +			BT_DBG("Event packet");
> +			ath->rx_state = HCIATH_W4_EVENT_HDR;
> +			ath->rx_count = HCI_EVENT_HDR_SIZE;
> +			type = HCI_EVENT_PKT;
> +			break;
> +
> +		case HCI_ACLDATA_PKT:
> +			BT_DBG("ACL packet");
> +			ath->rx_state = HCIATH_W4_ACL_HDR;
> +			ath->rx_count = HCI_ACL_HDR_SIZE;
> +			type = HCI_ACLDATA_PKT;
> +			break;
> +
> +		case HCI_SCODATA_PKT:
> +			BT_DBG("SCO packet");
> +			ath->rx_state = HCIATH_W4_SCO_HDR;
> +			ath->rx_count = HCI_SCO_HDR_SIZE;
> +			type = HCI_SCODATA_PKT;
> +			break;
> +
> +		default:
> +			BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
> +			hu->hdev->stat.err_rx++;
> +			ptr++;
> +			count--;
> +			continue;
> +
> +		};
> +		ptr++;
> +		count--;
> +
> +		/* Allocate packet */
> +		ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> +		if (!ath->rx_skb) {
> +			BT_ERR("Can't allocate mem for new packet");
> +			ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +			ath->rx_count = 0;
> +
> +			return -ENOMEM;
> +		}
> +		ath->rx_skb->dev = (void *)hu->hdev;
> +		bt_cb(ath->rx_skb)->pkt_type = type;
> +	}
> +
> +	return count;
> +}

Just out of curiosity. I never worked in the driver world, but is it ok
to duplicate lots of code when working with drivers? hci_h4.c and this
patch share lots of similar code. ath_recv() and h4_recv() are exactly
the same except for one line, the ath->rx_count = 0; at case HCIATH_W4_DATA.
Does this line makes real difference? recv() is not the only function
duplicating code here.

> +
> +static struct hci_uart_proto athp = {
> +	.id = HCI_UART_ATH,
> +	.open = ath_open,
> +	.close = ath_close,
> +	.recv = ath_recv,
> +	.enqueue = ath_enqueue,
> +	.dequeue = ath_dequeue,
> +	.flush = ath_flush,
> +};
> +
> +int ath_init(void)
> +{
> +	int err = hci_uart_register_proto(&athp);
> +
> +	if (!err)
> +		BT_INFO("HCIATH protocol initialized");
> +	else
> +		BT_ERR("HCIATH protocol registration failed");
> +
> +	return err;
> +}

BTW, we never check this return value on hci_ldisc.c, why?

> +
> +int ath_deinit(void)
> +{
> +	return hci_uart_unregister_proto(&athp);
> +}
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 76a1abb..7dd76d1 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_init();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_init();
> +#endif
>  
>  	return 0;
>  }
> @@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_deinit();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_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 50113db..385537f 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -33,13 +33,14 @@
>  #define HCIUARTGETDEVICE	_IOR('U', 202, int)
>  
>  /* UART protocols */
> -#define HCI_UART_MAX_PROTO	5
> +#define HCI_UART_MAX_PROTO	6
>  
>  #define HCI_UART_H4	0
>  #define HCI_UART_BCSP	1
>  #define HCI_UART_3WIRE	2
>  #define HCI_UART_H4DS	3
>  #define HCI_UART_LL	4
> +#define HCI_UART_ATH	5
>  
>  struct hci_uart;
>  
> @@ -91,3 +92,8 @@ int bcsp_deinit(void);
>  int ll_init(void);
>  int ll_deinit(void);
>  #endif
> +
> +#ifdef CONFIG_BT_HCIUART_ATH
> +int ath_init(void);
> +int ath_deinit(void);
> +#endif
> -- 
> 1.7.0
> 
> 
> 
> 
> 

-- 
Gustavo F. Padovan
http://padovan.org

^ permalink raw reply

* Re: [PATCH v4] Add support for the Atheros AR300x Bluetooth Chip
From: Luis R. Rodriguez @ 2010-04-21 17:30 UTC (permalink / raw)
  To: Suraj Sumangala
  Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
	Luis Rodriguez, Jothikumar Mothilal, gfpadovan@gmail.com
In-Reply-To: <1271845337.15010.1.camel@atheros013-desktop>

On Wed, Apr 21, 2010 at 03:22:17AM -0700, Suraj Sumangala wrote:
> This implements the Atheros Bluetooth serial protocol to
> support the AR300x Bluetooth chipsets.
> The serial protocol implements enhanced power management
> features for the AR300x chipsets.
> 
> Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> Signed-off-by: Suraj <suraj@atheros.com>

Looks good to me now, thanks for your patience and the resends. Marcel?

  Luis

^ permalink raw reply

* Race Condition between Read_Remote_Extended_Supported_Features and Remote_Name_Request causing BC6 crash
From: Stephen Coe @ 2010-04-21 16:32 UTC (permalink / raw)
  To: linux-bluetooth

I am experiencing a race condition between
Read_Remote_Extended_Supported_Features and Remote_Name_Request which
is crashing our BC6 module.  It seems that Bluez (in
security.c:conn_complete) is requesting the remote name at the same
time that the kernel is requesting the extended features.  The
responses are coming out of order which is crashing the BC6 module
(given some time).  After contacting CSR support, they have said this
is a known issue and that to fix I need that ensure that the response
comes back prior to sending another command.

"Input from the firmware team points to it being an issue with the
remote_name_request. Can you please verify that this is responded to
before sending the read_remote_supported_features request? Also check
that any pending requests are responded to and that the
remote_name_request is responded to before continuing."

Does anyone have any suggestions on the best way resolve the issue?  I
have proven it out with a small delay but that is a really bad hack.

Please see dump below:

2007-09-20 12:01:24.263113 > HCI Event: Connect Complete (0x03) plen 11
    status 0x00 handle 37 bdaddr 00:24:9F:F0:F8:DE type ACL encrypt 0x00
2007-09-20 12:01:24.263222 < HCI Command: Read Remote Supported
Features (0x01|0x001b) plen 2
    handle 37
2007-09-20 12:01:24.263671 > HCI Event: Page Scan Repetition Mode
Change (0x20) plen 7
    bdaddr 00:24:9F:F0:F8:DE mode 1
2007-09-20 12:01:24.265568 > HCI Event: Command Status (0x0f) plen 4
    Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 0
2007-09-20 12:01:24.268835 > HCI Event: Max Slots Change (0x1b) plen 3
    handle 37 slots 5
2007-09-20 12:01:24.275646 > HCI Event: Command Status (0x0f) plen 4
    Unknown (0x00|0x0000) status 0x00 ncmd 1
2007-09-20 12:01:24.280022 < HCI Command: Remote Name Request
(0x01|0x0019) plen 10
    bdaddr 00:24:9F:F0:F8:DE mode 2 clkoffset 0x0000
2007-09-20 12:01:24.283066 > HCI Event: Command Status (0x0f) plen 4
    Remote Name Request (0x01|0x0019) status 0x00 ncmd 1
2007-09-20 12:01:24.285635 > HCI Event: Read Remote Supported Features
(0x0b) plen 11
    status 0x00 handle 37
    Features: 0xbf 0xfe 0x8f 0xfe 0x98 0x19 0x59 0x83
2007-09-20 12:01:24.285733 < HCI Command: Read Remote Extended
Features (0x01|0x001c) plen 3
    handle 37 page 1
2007-09-20 12:01:24.288261 > HCI Event: Command Status (0x0f) plen 4
    Read Remote Extended Features (0x01|0x001c) status 0x00 ncmd 1
2007-09-20 12:01:24.295597 > HCI Event: Read Remote Extended Features
(0x23) plen 13
    status 0x00 handle 37 page 1 max 1
    Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00
2007-09-20 12:01:24.304385 > HCI Event: Remote Name Req Complete (0x07) plen 255
    status 0x00 bdaddr 00:24:9F:F0:F8:DE name 'BlackBerry 9530 scoe'

^ permalink raw reply

* [PATCH v4] Add support for the Atheros AR300x Bluetooth Chip
From: suraj @ 2010-04-21 10:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal, gfpadovan
In-Reply-To: <1271758832.6585.33.camel@atheros013-desktop>

This implements the Atheros Bluetooth serial protocol to
support the AR300x Bluetooth chipsets.
The serial protocol implements enhanced power management
features for the AR300x chipsets.

Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: Suraj <suraj@atheros.com>

---
 drivers/bluetooth/Kconfig     |   14 ++
 drivers/bluetooth/Makefile    |    1 +
 drivers/bluetooth/hci_ath.c   |  378 +++++++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/hci_ldisc.c |    6 +
 drivers/bluetooth/hci_uart.h  |    8 +-
 5 files changed, 406 insertions(+), 1 deletions(-)
 create mode 100755 drivers/bluetooth/hci_ath.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 058fbcc..5546142 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -58,6 +58,20 @@ config BT_HCIUART_BCSP
 
 	  Say Y here to compile support for HCI BCSP protocol.
 
+config BT_HCIUART_ATH
+	bool "Atheros AR300x serial Bluetooth support"
+	depends on BT_HCIUART
+	help
+	  HCIATH (HCI Atheros) is a serial protocol for communication
+	  between the host and Atheros AR300x Bluetooth devices. The
+	  protocol implements enhaned power management features for the
+	  the AR300x chipsets. it lets the controller chip go to sleep
+	  mode if there is no Bluetooth activity for some time and wakes
+	  up the chip in case of a Bluetooth activity. Enable this option
+	  if you have an UART Atheros AR300x serial device.
+
+	  Say Y here to compile support for HCIATH protocol.
+
 config BT_HCIUART_LL
 	bool "HCILL protocol support"
 	depends on BT_HCIUART
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 7e5aed5..1481faa 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -26,4 +26,5 @@ hci_uart-y				:= hci_ldisc.o
 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_ATH)	+= hci_ath.o
 hci_uart-objs				:= $(hci_uart-y)
diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
new file mode 100755
index 0000000..7e99559
--- /dev/null
+++ b/drivers/bluetooth/hci_ath.c
@@ -0,0 +1,378 @@
+/*
+ * Copyright (c) 2009-2010 Atheros Communications Inc.
+ *
+ *  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/module.h>
+#include <linux/kernel.h>
+
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/errno.h>
+#include <linux/ioctl.h>
+#include <linux/skbuff.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "hci_uart.h"
+
+/* HCIATH receiver States */
+#define HCIATH_W4_PACKET_TYPE			0
+#define HCIATH_W4_EVENT_HDR			1
+#define HCIATH_W4_ACL_HDR			2
+#define HCIATH_W4_SCO_HDR			3
+#define HCIATH_W4_DATA				4
+
+struct ath_struct {
+	struct hci_uart *hu;
+	unsigned int rx_state;
+	unsigned int rx_count;
+	unsigned int cur_sleep;
+
+	spinlock_t hciath_lock;
+	struct sk_buff *rx_skb;
+	struct sk_buff_head txq;
+	wait_queue_head_t wqevt;
+	struct work_struct ctxtsw;
+};
+
+static int ath_wakeup_ar3001(struct tty_struct *tty)
+{
+	struct termios settings;
+	int status = tty->driver->ops->tiocmget(tty, NULL);
+
+	if (status & TIOCM_CTS)
+		return status;
+
+	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
+
+	/* Disable Automatic RTSCTS */
+	settings.c_cflag &= ~CRTSCTS;
+	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	/* Clear RTS first */
+	tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
+	mdelay(20);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	/* Set RTS, wake up board */
+	tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
+	mdelay(20);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
+
+	settings.c_cflag |= CRTSCTS;
+	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
+
+	return status;
+}
+
+static void ath_hci_uart_work(struct work_struct *work)
+{
+	int status;
+	struct ath_struct *ath;
+	struct hci_uart *hu;
+	struct tty_struct *tty;
+
+	ath = container_of(work, struct ath_struct, ctxtsw);
+
+	hu = ath->hu;
+	tty = hu->tty;
+
+	/* verify and wake up controller */
+	if (ath->cur_sleep) {
+		status = ath_wakeup_ar3001(tty);
+
+		if (!(status & TIOCM_CTS))
+			return;
+	}
+
+	/* Ready to send Data */
+	clear_bit(HCI_UART_SENDING, &hu->tx_state);
+	hci_uart_tx_wakeup(hu);
+}
+
+/* Initialize protocol */
+static int ath_open(struct hci_uart *hu)
+{
+	struct ath_struct *ath;
+
+	BT_DBG("hu %p", hu);
+
+	ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
+	if (!ath)
+		return -ENOMEM;
+
+	skb_queue_head_init(&ath->txq);
+	spin_lock_init(&ath->hciath_lock);
+
+	hu->priv = ath;
+	ath->hu = hu;
+
+	init_waitqueue_head(&ath->wqevt);
+	INIT_WORK(&ath->ctxtsw, ath_hci_uart_work);
+
+	return 0;
+}
+
+/* Flush protocol data */
+static int ath_flush(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+
+	BT_DBG("hu %p", hu);
+
+	skb_queue_purge(&ath->txq);
+
+	return 0;
+}
+
+/* Close protocol */
+static int ath_close(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+
+	BT_DBG("hu %p", hu);
+
+	skb_queue_purge(&ath->txq);
+
+	kfree_skb(ath->rx_skb);
+
+	cancel_work_sync(&ath->ctxtsw);
+
+	hu->priv = NULL;
+	kfree(ath);
+
+	return 0;
+}
+
+/* Enqueue frame for transmittion */
+static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
+{
+	struct ath_struct *ath = hu->priv;
+
+	if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
+
+		/* Discard SCO packet. AR3001 does not support SCO over HCI */
+		BT_DBG("SCO Packet over HCI received Dropping");
+
+		kfree(skb);
+
+		return 0;
+	}
+
+	BT_DBG("hu %p skb %p", hu, skb);
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	skb_queue_tail(&ath->txq, skb);
+	set_bit(HCI_UART_SENDING, &hu->tx_state);
+
+	schedule_work(&ath->ctxtsw);
+
+	return 0;
+}
+
+static struct sk_buff *ath_dequeue(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+	struct sk_buff *skbuf;
+
+	skbuf = skb_dequeue(&ath->txq);
+
+	if (!skbuf)
+		return NULL;
+
+	/*
+	 * Check if the HCI command is  HCI sleep enable and
+	 * update the sleep enable flag with command parameter.
+	 *
+	 * Value of sleep enable flag will be used later
+	 * to verify if controller has to be woken up before
+	 * sending any packet.
+	 */
+	if (skbuf->data[0] == 0x01 &&
+	    skbuf->data[1] == 0x04 &&
+	    skbuf->data[2] == 0xFC)
+		ath->cur_sleep = skbuf->data[4];
+
+	return skbuf;
+}
+
+static void ath_check_data_len(struct ath_struct *ath, int len)
+{
+	int room = skb_tailroom(ath->rx_skb);
+
+	BT_DBG("len %d room %d", len, room);
+
+	if (len > room) {
+		BT_ERR("Data length is too large");
+		kfree_skb(ath->rx_skb);
+		ath->rx_state = HCIATH_W4_PACKET_TYPE;
+		ath->rx_skb = NULL;
+		ath->rx_count = 0;
+	} else {
+		ath->rx_state = HCIATH_W4_DATA;
+		ath->rx_count = len;
+	}
+}
+
+/* Recv data */
+static int ath_recv(struct hci_uart *hu, void *data, int count)
+{
+	struct ath_struct *ath = hu->priv;
+	char *ptr = data;
+	struct hci_event_hdr *eh;
+	struct hci_acl_hdr *ah;
+	struct hci_sco_hdr *sh;
+	int len, type, dlen;
+
+	BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
+	       ath->rx_state, ath->rx_count);
+
+	while (count) {
+		if (ath->rx_count) {
+
+			len = min_t(unsigned int, ath->rx_count, count);
+			memcpy(skb_put(ath->rx_skb, len), ptr, len);
+			ath->rx_count -= len;
+			count -= len;
+			ptr += len;
+
+			if (ath->rx_count)
+				continue;
+			switch (ath->rx_state) {
+			case HCIATH_W4_DATA:
+				hci_recv_frame(ath->rx_skb);
+				ath->rx_state = HCIATH_W4_PACKET_TYPE;
+				ath->rx_skb = NULL;
+				ath->rx_count = 0;
+				continue;
+
+			case HCIATH_W4_EVENT_HDR:
+				eh = (struct hci_event_hdr *)ath->rx_skb->data;
+
+				BT_DBG("Event header: evt 0x%2.2x plen %d",
+				       eh->evt, eh->plen);
+
+				ath_check_data_len(ath, eh->plen);
+				continue;
+
+			case HCIATH_W4_ACL_HDR:
+				ah = (struct hci_acl_hdr *)ath->rx_skb->data;
+				dlen = __le16_to_cpu(ah->dlen);
+
+				BT_DBG("ACL header: dlen %d", dlen);
+
+				ath_check_data_len(ath, dlen);
+				continue;
+
+			case HCIATH_W4_SCO_HDR:
+				sh = (struct hci_sco_hdr *)ath->rx_skb->data;
+
+				BT_DBG("SCO header: dlen %d", sh->dlen);
+
+				ath_check_data_len(ath, sh->dlen);
+				continue;
+
+			}
+		}
+
+		/* HCIATH_W4_PACKET_TYPE */
+		switch (*ptr) {
+		case HCI_EVENT_PKT:
+			BT_DBG("Event packet");
+			ath->rx_state = HCIATH_W4_EVENT_HDR;
+			ath->rx_count = HCI_EVENT_HDR_SIZE;
+			type = HCI_EVENT_PKT;
+			break;
+
+		case HCI_ACLDATA_PKT:
+			BT_DBG("ACL packet");
+			ath->rx_state = HCIATH_W4_ACL_HDR;
+			ath->rx_count = HCI_ACL_HDR_SIZE;
+			type = HCI_ACLDATA_PKT;
+			break;
+
+		case HCI_SCODATA_PKT:
+			BT_DBG("SCO packet");
+			ath->rx_state = HCIATH_W4_SCO_HDR;
+			ath->rx_count = HCI_SCO_HDR_SIZE;
+			type = HCI_SCODATA_PKT;
+			break;
+
+		default:
+			BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
+			hu->hdev->stat.err_rx++;
+			ptr++;
+			count--;
+			continue;
+
+		};
+		ptr++;
+		count--;
+
+		/* Allocate packet */
+		ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
+		if (!ath->rx_skb) {
+			BT_ERR("Can't allocate mem for new packet");
+			ath->rx_state = HCIATH_W4_PACKET_TYPE;
+			ath->rx_count = 0;
+
+			return -ENOMEM;
+		}
+		ath->rx_skb->dev = (void *)hu->hdev;
+		bt_cb(ath->rx_skb)->pkt_type = type;
+	}
+
+	return count;
+}
+
+static struct hci_uart_proto athp = {
+	.id = HCI_UART_ATH,
+	.open = ath_open,
+	.close = ath_close,
+	.recv = ath_recv,
+	.enqueue = ath_enqueue,
+	.dequeue = ath_dequeue,
+	.flush = ath_flush,
+};
+
+int ath_init(void)
+{
+	int err = hci_uart_register_proto(&athp);
+
+	if (!err)
+		BT_INFO("HCIATH protocol initialized");
+	else
+		BT_ERR("HCIATH protocol registration failed");
+
+	return err;
+}
+
+int ath_deinit(void)
+{
+	return hci_uart_unregister_proto(&athp);
+}
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 76a1abb..7dd76d1 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
 #ifdef CONFIG_BT_HCIUART_LL
 	ll_init();
 #endif
+#ifdef CONFIG_BT_HCIUART_ATH
+	ath_init();
+#endif
 
 	return 0;
 }
@@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
 #ifdef CONFIG_BT_HCIUART_LL
 	ll_deinit();
 #endif
+#ifdef CONFIG_BT_HCIUART_ATH
+	ath_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 50113db..385537f 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -33,13 +33,14 @@
 #define HCIUARTGETDEVICE	_IOR('U', 202, int)
 
 /* UART protocols */
-#define HCI_UART_MAX_PROTO	5
+#define HCI_UART_MAX_PROTO	6
 
 #define HCI_UART_H4	0
 #define HCI_UART_BCSP	1
 #define HCI_UART_3WIRE	2
 #define HCI_UART_H4DS	3
 #define HCI_UART_LL	4
+#define HCI_UART_ATH	5
 
 struct hci_uart;
 
@@ -91,3 +92,8 @@ int bcsp_deinit(void);
 int ll_init(void);
 int ll_deinit(void);
 #endif
+
+#ifdef CONFIG_BT_HCIUART_ATH
+int ath_init(void);
+int ath_deinit(void);
+#endif
-- 
1.7.0

^ permalink raw reply related

* Re: [PATCH v3] Added support for Atheros AR300x Bluetooth Chip
From: Suraj Sumangala @ 2010-04-21  4:21 UTC (permalink / raw)
  To: Luis Rodriguez
  Cc: Suraj Sumangala, linux-bluetooth@vger.kernel.org,
	marcel@holtmann.org, Jothikumar Mothilal, gfpadovan@gmail.com
In-Reply-To: <20100420173428.GA2559@tux>

Hi Luis,

Luis Rodriguez wrote:
> On Tue, Apr 20, 2010 at 03:20:32AM -0700, Suraj Sumangala wrote:
>> This protocol implements support for power management feature provided by AR300x chip.
>> This lets the controller chip go to sleep mode if there is no Bluetooth
>> activity for some time.
>> It then wakes up the chip in case of a Bluetooth activity.
> 
> The above commit log needs some more work. Try to keep the
> commit log entry subject to about 50 characters, the context should
> not pass around 75 characters. Your commit also indicates this is a
> "protocol" ? This is driver, and you do have some hacks for enhancing
> power saving, but that is not that relevant to the commit log.
> 
> How about:
> 
> ---
> Add support for the Atheros AR300x Bluetooth Chip
> 
> This adds support for the Atheros Bluetooth serial protocol to
> support the AR300x chipsets. The serial protocol implements
> enhanced power management features for the AR300x chipsets.
> 
> Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> Signed-off-by: Suraj <suraj@atheros.com>
> ---
> 
> Of course you will need to adjust the subject and prepend it with
> PATCH v4, just as you did with a v3 for this patch.
> 
> Then, this stuff:
> 
>> * Third version
>>
>> ** Updated with extra spacing and indentation
> 
> Do you use checkpatch.pl for your patches? If not please
> add check your patches after committing them with:
> 
> git show ./scripts/checkpatch.pl -
> vi drivers/bluetooth/hci_ath.c
> git commit -a --amend
> 
> And repeat until checkpatch.pl is git happy :)

Yes, I had done a checkpatch round with the patch. it looked happy. Did 
you see any issue that could should have been caught by it?

> 
>> ** made function definitions static
>> ** Removed inline and register keyword usage.
>> ** Removed unused return calls.
>> ** Incorporated code comments by Luis and Gustavo
>>
>> Thanks Luis and Gustavo for your comments
> 
> 
> Remove it from the commit log, if you do want to add some extra text
> to the patch youc an put it below the three dashes ("-") where the
> diff stat goes:
> 
>>
>> Signed-off-by: Suraj <suraj@atheros.com>
>>
>> ---
>   ^^^
> 
> These are the three lines, anything below is ignored by git am
> when the maintainer applies the patch. So you can add anything you
> want ignored by git am here.
> 
>>  drivers/bluetooth/Kconfig     |   11 ++
>>  drivers/bluetooth/Makefile    |    1 +
>>  drivers/bluetooth/hci_ath.c   |  384 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/bluetooth/hci_ldisc.c |    6 +
>>  drivers/bluetooth/hci_uart.h  |    8 +-
>>  5 files changed, 409 insertions(+), 1 deletions(-)
>>  create mode 100755 drivers/bluetooth/hci_ath.c
>>
>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>> index 058fbcc..81abeff 100644
>> --- a/drivers/bluetooth/Kconfig
>> +++ b/drivers/bluetooth/Kconfig
>> @@ -58,6 +58,17 @@ config BT_HCIUART_BCSP
>>
>>         Say Y here to compile support for HCI BCSP protocol.
>>
>> +config BT_HCIUART_ATH
>> +     bool "Atheros AR300x Board support"
>> +     depends on BT_HCIUART
>> +     help
>> +       HCIATH (HCI Atheros) is a serial protocol for communication
>> +       between Bluetooth device and host with support for Atheros AR300x
>> +       power management feature. This protocol is required for
>> +       serial Bluetooth devices that are based on Atheros AR300x chips.
> 
> Please adjust the description as well. How about:
> 
> +         HCIATH (HCI Atheros) is a serial protocol for communication
> +         between the host and Atheros AR300x Bluetooth devices. The
> +         protocol implements enhaned power management features for the
> +         the AR300x chipsets, it lets the controller chip go to sleep
> +         mode if there is no Bluetooth activity for some time and wakes
> +         up the chip in case of a Bluetooth activity. Enabling this
> +         option will build HCI Atheros support into the hci_uart driver.
> +         Enable this option if you have an UART Atheros AR300x serial
> +         device.
> 
> 
>> +
>> +       Say Y here to compile support for HCIATH protocol.
>> +
>>  config BT_HCIUART_LL
>>       bool "HCILL protocol support"
>>       depends on BT_HCIUART
>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>> index 7e5aed5..1481faa 100644
>> --- a/drivers/bluetooth/Makefile
>> +++ b/drivers/bluetooth/Makefile
>> @@ -26,4 +26,5 @@ hci_uart-y                          := hci_ldisc.o
>>  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_ATH)    += hci_ath.o
>>  hci_uart-objs                                := $(hci_uart-y)
>> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
>> new file mode 100755
>> index 0000000..2f91954
>> --- /dev/null
>> +++ b/drivers/bluetooth/hci_ath.c
>> @@ -0,0 +1,384 @@
>> +/*
>> + * Copyright (c) 2009-2010 Atheros Communications Inc.
>> + *
>> + *  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/module.h>
>> +#include <linux/kernel.h>
>> +
>> +#include <linux/init.h>
>> +#include <linux/slab.h>
>> +#include <linux/tty.h>
>> +#include <linux/errno.h>
>> +#include <linux/ioctl.h>
>> +#include <linux/skbuff.h>
>> +
>> +#include <net/bluetooth/bluetooth.h>
>> +#include <net/bluetooth/hci_core.h>
>> +
>> +#include "hci_uart.h"
>> +
>> +
>> +/* HCIATH receiver States */
>> +#define HCIATH_W4_PACKET_TYPE                        0
>> +#define HCIATH_W4_EVENT_HDR                  1
>> +#define HCIATH_W4_ACL_HDR                    2
>> +#define HCIATH_W4_SCO_HDR                    3
>> +#define HCIATH_W4_DATA                               4
>> +
>> +struct ath_struct {
>> +     struct hci_uart *hu;
>> +     unsigned int rx_state;
>> +     unsigned int rx_count;
>> +     unsigned int cur_sleep;
>> +
>> +     spinlock_t hciath_lock;
>> +     struct sk_buff *rx_skb;
>> +     struct sk_buff_head txq;
>> +     wait_queue_head_t wqevt;
>> +     struct work_struct ctxtsw;
>> +};
>> +
>> +static int ath_wakeup_ar3001(struct tty_struct *tty)
>> +{
>> +     struct termios settings;
>> +     int status = 0x00;
>> +
>> +     status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     if ((status & TIOCM_CTS))
>> +             return status;
> 
> No need for double () parens here. This should be fine:
> 
> +       if (status & TIOCM_CTS)
> +               return status;
> 
> You would use double parens if you are doing a check against
> another flag as well.
> 
>> +
>> +     n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
>> +
>> +     /* Disable Automatic RTSCTS */
>> +     settings.c_cflag &= ~CRTSCTS;
>> +     n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
>> +
>> +     status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     /* Clear RTS first */
>> +     tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
>> +     mdelay(20);
>> +
>> +     status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     /* Set RTS, wake up board */
>> +     tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
>> +     mdelay(20);
>> +
>> +     status = tty->driver->ops->tiocmget(tty, NULL);
>> +
>> +     n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
>> +
>> +     settings.c_cflag |= CRTSCTS;
>> +     n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
>> +
>> +     return status;
>> +}
>> +
>> +static void ath_context_switch(struct work_struct *work)
> 
> Can you please rename this to ath_hci_uart_work()
> 
>> +{
>> +     int status;
>> +     struct ath_struct *ath;
>> +     struct hci_uart *hu;
>> +     struct tty_struct *tty;
>> +
>> +     ath = container_of(work, struct ath_struct, ctxtsw);
>> +
>> +     hu = ath->hu;
>> +     tty = hu->tty;
>> +
>> +     /* verify and wake up controller */
>> +     if (ath->cur_sleep) {
>> +
>> +             status = ath_wakeup_ar3001(tty);
>> +
>> +             if (!(status & TIOCM_CTS))
>> +                     return;
>> +     }
>> +
>> +     /* Ready to send Data */
>> +     clear_bit(HCI_UART_SENDING, &hu->tx_state);
>> +     hci_uart_tx_wakeup(hu);
>> +}
>> +
>> +/* Initialize protocol */
>> +static int ath_open(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath;
>> +
>> +     BT_DBG("hu %p", hu);
>> +
>> +     ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
>> +     if (!ath)
>> +             return -ENOMEM;
>> +
>> +     skb_queue_head_init(&ath->txq);
>> +     spin_lock_init(&ath->hciath_lock);
>> +
>> +     hu->priv = ath;
>> +     ath->hu = hu;
>> +
>> +     init_waitqueue_head(&ath->wqevt);
>> +     INIT_WORK(&ath->ctxtsw, ath_context_switch);
>> +
>> +     return 0;
>> +}
>> +
>> +/* Flush protocol data */
>> +static int ath_flush(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +
>> +     BT_DBG("hu %p", hu);
>> +
>> +     skb_queue_purge(&ath->txq);
>> +
>> +     return 0;
>> +}
>> +
>> +/* Close protocol */
>> +static int ath_close(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +
>> +     BT_DBG("hu %p", hu);
>> +
>> +     skb_queue_purge(&ath->txq);
>> +
>> +     if (ath->rx_skb)
>> +             kfree_skb(ath->rx_skb);
> 
> No need for the check, kfree_skb() does that for you
> and it has it optimized for the case where the skb you
> pass is NULL. You can just do:
> 
> +       kfree_skb(ath->rx_skb);
> 
>> +
>> +     cancel_work_sync(&ath->ctxtsw);
>> +
>> +     hu->priv = NULL;
>> +     kfree(ath);
>> +
>> +     return 0;
>> +}
> 
> Did you give this new patch a spin by looping bringing up,
> scanning, bringing the interface down? Or have a loop doing
> a scan while in another window you bring the interface up
> and down?
> 
Yep, Have been testing it for sometime. Haven't seen any issue yet (Hope 
I dont see any :-) )
>> +
>> +/* Enqueue frame for transmittion */
>> +static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +
>> +     if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
>> +
>> +             /* Discard SCO packet.AR3001 does not support SCO over HCI */
> 
> Add a space after packet. Why does it not support SCO over HCI BTW?
> Just curious. I'm new to BT :) Is this common? If this is common can't
> the BT stack be informed of these things so that they don't pass the skbs
> to the driver?
> 
 From my experience, it is common for smaller BT devices (headsets) and 
devices with memory limitations.

I guess, there is no way to tell SCO routing using any HCI cmd/Event.

In the Linux Ubuntu distro that I use, it is controlled using the 
"SCORouting" flag in "/etc/bluetooth/audio.conf". Since it is a user 
configuration, driver can not assume that the flag will be set.

So, this is like a precautionary check.



> This could be done in a separate patch though if this is the case
> though.
> 
>> +             BT_DBG("SCO Packet over HCI received Dropping");
>> +
>> +             kfree(skb);
>> +
>> +             return 0;
>> +     }
>> +
>> +     BT_DBG("hu %p skb %p", hu, skb);
>> +
>> +     /* Prepend skb with frame type */
>> +     memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
>> +
>> +     skb_queue_tail(&ath->txq, skb);
>> +     set_bit(HCI_UART_SENDING, &hu->tx_state);
>> +
>> +     schedule_work(&ath->ctxtsw);
>> +
>> +     return 0;
>> +}
>> +
>> +static struct sk_buff *ath_dequeue(struct hci_uart *hu)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +     struct sk_buff *skbuf;
>> +
>> +     skbuf = skb_dequeue(&ath->txq);
>> +
>> +     if (!skbuf)
>> +             return NULL;
>> +
>> +
>> +     /*
>> +      * Check if the HCI command is  HCI sleep enable and
>> +      * update the sleep enable flag with command parameter.
>> +      *
>> +      * Value of sleep enable flag will be used later
>> +      * to verify if controller has to be woken up before
>> +      * sending any packet.
>> +      */
>> +     if (skbuf->data[0] == 0x01 && skbuf->data[1] == 0x04 &&
>> +         skbuf->data[2] == 0xFC)
>> +             ath->cur_sleep = skbuf->data[4];
> 
> Might as well just do this:
> 
> +       if (skbuf->data[0] == 0x01 &&
> +           skbuf->data[1] == 0x04 &&
> +           skbuf->data[2] == 0xFC)
> +               ath->cur_sleep = skbuf->data[4];
> 
> Is this sort of check done in any other drivers/protocols? If so a
> helper could be added to hci_uart.h if this is the case, but likely
> better though a separate patch.

No, this check is done only for this specific driver/protocol.

This is the reason why we had to go for board specific driver change. 
Otherways, we could have used the default HCI driver implementation.

> 
>> +
>> +     return skbuf;
>> +}
>> +
>> +static void ath_check_data_len(struct ath_struct *ath, int len)
>> +{
>> +     int room = skb_tailroom(ath->rx_skb);
>> +
>> +     BT_DBG("len %d room %d", len, room);
>> +
>> +     if (len > room) {
>> +             BT_ERR("Data length is too large");
>> +             kfree_skb(ath->rx_skb);
>> +             ath->rx_state = HCIATH_W4_PACKET_TYPE;
>> +             ath->rx_skb = NULL;
>> +             ath->rx_count = 0;
>> +     } else {
>> +             ath->rx_state = HCIATH_W4_DATA;
>> +             ath->rx_count = len;
>> +     }
>> +}
>> +
>> +/* Recv data */
>> +static int ath_recv(struct hci_uart *hu, void *data, int count)
>> +{
>> +     struct ath_struct *ath = hu->priv;
>> +     char *ptr = data;
>> +     struct hci_event_hdr *eh;
>> +     struct hci_acl_hdr *ah;
>> +     struct hci_sco_hdr *sh;
>> +     int len, type, dlen;
>> +
>> +
>> +     BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
>> +            ath->rx_state, ath->rx_count);
>> +
>> +     while (count) {
>> +             if (ath->rx_count) {
>> +
>> +                     len = min_t(unsigned int, ath->rx_count, count);
>> +                     memcpy(skb_put(ath->rx_skb, len), ptr, len);
>> +                     ath->rx_count -= len;
>> +                     count -= len;
>> +                     ptr += len;
>> +
>> +                     if (ath->rx_count)
>> +                             continue;
>> +                     switch (ath->rx_state) {
>> +                     case HCIATH_W4_DATA:
>> +                             hci_recv_frame(ath->rx_skb);
>> +                             ath->rx_state = HCIATH_W4_PACKET_TYPE;
>> +                             ath->rx_skb = NULL;
>> +                             ath->rx_count = 0;
>> +                             continue;
>> +
>> +                     case HCIATH_W4_EVENT_HDR:
>> +                             eh = (struct hci_event_hdr *)ath->rx_skb->data;
>> +
>> +                             BT_DBG("Event header: evt 0x%2.2x plen %d",
>> +                                    eh->evt, eh->plen);
>> +
>> +                             ath_check_data_len(ath, eh->plen);
>> +                             continue;
>> +
>> +                     case HCIATH_W4_ACL_HDR:
>> +                             ah = (struct hci_acl_hdr *)ath->rx_skb->data;
>> +                             dlen = __le16_to_cpu(ah->dlen);
>> +
>> +                             BT_DBG("ACL header: dlen %d", dlen);
>> +
>> +                             ath_check_data_len(ath, dlen);
>> +                             continue;
>> +
>> +                     case HCIATH_W4_SCO_HDR:
>> +                             sh = (struct hci_sco_hdr *)ath->rx_skb->data;
>> +
>> +                             BT_DBG("SCO header: dlen %d", sh->dlen);
>> +
>> +                             ath_check_data_len(ath, sh->dlen);
>> +                             continue;
>> +
>> +                     }
>> +             }
>> +
>> +             /* HCIATH_W4_PACKET_TYPE */
>> +             switch (*ptr) {
>> +             case HCI_EVENT_PKT:
>> +                     BT_DBG("Event packet");
>> +                     ath->rx_state = HCIATH_W4_EVENT_HDR;
>> +                     ath->rx_count = HCI_EVENT_HDR_SIZE;
>> +                     type = HCI_EVENT_PKT;
>> +                     break;
>> +
>> +             case HCI_ACLDATA_PKT:
>> +                     BT_DBG("ACL packet");
>> +                     ath->rx_state = HCIATH_W4_ACL_HDR;
>> +                     ath->rx_count = HCI_ACL_HDR_SIZE;
>> +                     type = HCI_ACLDATA_PKT;
>> +                     break;
>> +
>> +             case HCI_SCODATA_PKT:
>> +                     BT_DBG("SCO packet");
>> +                     ath->rx_state = HCIATH_W4_SCO_HDR;
>> +                     ath->rx_count = HCI_SCO_HDR_SIZE;
>> +                     type = HCI_SCODATA_PKT;
>> +                     break;
>> +
>> +             default:
>> +                     BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
>> +                     hu->hdev->stat.err_rx++;
>> +                     ptr++;
>> +                     count--;
>> +                     continue;
>> +
>> +             };
>> +             ptr++;
>> +             count--;
>> +
>> +             /* Allocate packet */
>> +             ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
>> +             if (!ath->rx_skb) {
>> +                     BT_ERR("Can't allocate mem for new packet");
>> +                     ath->rx_state = HCIATH_W4_PACKET_TYPE;
>> +                     ath->rx_count = 0;
>> +
>> +                     return -ENOMEM;
>> +             }
>> +             ath->rx_skb->dev = (void *)hu->hdev;
>> +             bt_cb(ath->rx_skb)->pkt_type = type;
>> +     }
>> +
>> +     return count;
>> +}
>> +
>> +static struct hci_uart_proto athp = {
>> +     .id = HCI_UART_ATH,
>> +     .open = ath_open,
>> +     .close = ath_close,
>> +     .recv = ath_recv,
>> +     .enqueue = ath_enqueue,
>> +     .dequeue = ath_dequeue,
>> +     .flush = ath_flush,
>> +};
>> +
>> +int ath_init(void)
>> +{
>> +     int err = hci_uart_register_proto(&athp);
>> +
>> +     if (!err)
>> +             BT_INFO("HCIATH protocol initialized");
>> +     else
>> +             BT_ERR("HCIATH protocol registration failed");
>> +
>> +     return err;
>> +}
>> +
>> +int ath_deinit(void)
>> +{
>> +     return hci_uart_unregister_proto(&athp);
>> +}
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index 76a1abb..7dd76d1 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
>>  #ifdef CONFIG_BT_HCIUART_LL
>>       ll_init();
>>  #endif
>> +#ifdef CONFIG_BT_HCIUART_ATH
>> +     ath_init();
>> +#endif
>>
>>       return 0;
>>  }
>> @@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
>>  #ifdef CONFIG_BT_HCIUART_LL
>>       ll_deinit();
>>  #endif
>> +#ifdef CONFIG_BT_HCIUART_ATH
>> +     ath_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 50113db..385537f 100644
>> --- a/drivers/bluetooth/hci_uart.h
>> +++ b/drivers/bluetooth/hci_uart.h
>> @@ -33,13 +33,14 @@
>>  #define HCIUARTGETDEVICE     _IOR('U', 202, int)
>>
>>  /* UART protocols */
>> -#define HCI_UART_MAX_PROTO   5
>> +#define HCI_UART_MAX_PROTO   6
>>
>>  #define HCI_UART_H4  0
>>  #define HCI_UART_BCSP        1
>>  #define HCI_UART_3WIRE       2
>>  #define HCI_UART_H4DS        3
>>  #define HCI_UART_LL  4
>> +#define HCI_UART_ATH 5
>>
>>  struct hci_uart;
>>
>> @@ -91,3 +92,8 @@ int bcsp_deinit(void);
>>  int ll_init(void);
>>  int ll_deinit(void);
>>  #endif
>> +
>> +#ifdef CONFIG_BT_HCIUART_ATH
>> +int ath_init(void);
>> +int ath_deinit(void);
>> +#endif
>> --
>> 1.7.0
>>
>>
>>

Thanks for the comments.

Regards,
Suraj

^ permalink raw reply

* eL2CAP git tree
From: Gustavo F. Padovan @ 2010-04-21  0:34 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

Since my code is still not merged in bluetooth's kernel tree and might
be some people interested in help with the eL2CAP development, I'm
posting my tree here. eL2CAP means the Enhanced Retransmission and
Streaming Modes, Segmentation and Reassembly, and Frame Check
Sequence(FCS).

My git tree is hosted at:

http://git.profusion.mobi/cgit.cgi/padovan/bluetooth-testing/

and you can clone it from here:

git://git.profusion.mobi/users/padovan/bluetooth-testing.git 

The code is on the branch 'ertm'.

If you wanna help with testing there is a patch that adds debug output
on the top of the branch 'test'.

Comments, suggestion, patches and bug reports are all welcome ;)

Regards,

-- 
Gustavo F. Padovan
http://padovan.org

^ permalink raw reply

* [PATCH] Bluetooth: Implement Local Busy Condition handling
From: Gustavo F. Padovan @ 2010-04-21  0:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, jprvita
In-Reply-To: <1271376703-25040-1-git-send-email-padovan@profusion.mobi>

Supports Local Busy condition handling through a waitqueue that wake ups
each 200ms and try to push the packets to the upper layer. If it can
push all the queue then it leaves the Local Busy state.

The patch modifies the behaviour of l2cap_ertm_reassembly_sdu() to
support retry of the push operation.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 include/net/bluetooth/l2cap.h |    6 ++
 net/bluetooth/l2cap.c         |  185 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 171 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 15f6e55..fd0e4e4 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -35,6 +35,7 @@
 #define L2CAP_DEFAULT_MONITOR_TO	12000   /* 12 seconds */
 #define L2CAP_DEFAULT_MAX_PDU_SIZE	672
 #define L2CAP_DEFAULT_ACK_TO		200
+#define L2CAP_LOCAL_BUSY_TRIES		12
 
 #define L2CAP_CONN_TIMEOUT	(40000) /* 40 seconds */
 #define L2CAP_INFO_TIMEOUT	(4000)  /*  4 seconds */
@@ -294,6 +295,7 @@ struct l2cap_conn {
 #define l2cap_pi(sk) ((struct l2cap_pinfo *) sk)
 #define TX_QUEUE(sk) (&l2cap_pi(sk)->tx_queue)
 #define SREJ_QUEUE(sk) (&l2cap_pi(sk)->srej_queue)
+#define BUSY_QUEUE(sk) (&l2cap_pi(sk)->busy_queue)
 #define SREJ_LIST(sk) (&l2cap_pi(sk)->srej_l.list)
 
 struct srej_list {
@@ -356,6 +358,8 @@ struct l2cap_pinfo {
 	struct timer_list	ack_timer;
 	struct sk_buff_head	tx_queue;
 	struct sk_buff_head	srej_queue;
+	struct sk_buff_head	busy_queue;
+	struct work_struct	busy_work;
 	struct srej_list	srej_l;
 	struct l2cap_conn	*conn;
 	struct sock		*next_c;
@@ -383,6 +387,8 @@ struct l2cap_pinfo {
 #define L2CAP_CONN_LOCAL_BUSY      0x0040
 #define L2CAP_CONN_REJ_ACT         0x0080
 #define L2CAP_CONN_SEND_FBIT       0x0100
+#define L2CAP_CONN_RNR_SENT        0x0200
+#define L2CAP_CONN_SAR_RETRY       0x0400
 
 #define __mod_retrans_timer() mod_timer(&l2cap_pi(sk)->retrans_timer, \
 		jiffies +  msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index f3a3692..43d375b 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -68,10 +68,14 @@ static u8 l2cap_fixed_chan[8] = { 0x02, };
 
 static const struct proto_ops l2cap_sock_ops;
 
+static struct workqueue_struct *_busy_wq;
+
 static struct bt_sock_list l2cap_sk_list = {
 	.lock = __RW_LOCK_UNLOCKED(l2cap_sk_list.lock)
 };
 
+static void l2cap_busy_work(struct work_struct *work);
+
 static void __l2cap_sock_close(struct sock *sk, int reason);
 static void l2cap_sock_close(struct sock *sk);
 static void l2cap_sock_kill(struct sock *sk);
@@ -386,8 +390,10 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
 
 static inline void l2cap_send_rr_or_rnr(struct l2cap_pinfo *pi, u16 control)
 {
-	if (pi->conn_state & L2CAP_CONN_LOCAL_BUSY)
+	if (pi->conn_state & L2CAP_CONN_LOCAL_BUSY) {
 		control |= L2CAP_SUPER_RCV_NOT_READY;
+		pi->conn_state |= L2CAP_CONN_RNR_SENT;
+	}
 	else
 		control |= L2CAP_SUPER_RCV_READY;
 
@@ -816,6 +822,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 	pi->flush_to = L2CAP_DEFAULT_FLUSH_TO;
 	skb_queue_head_init(TX_QUEUE(sk));
 	skb_queue_head_init(SREJ_QUEUE(sk));
+	skb_queue_head_init(BUSY_QUEUE(sk));
 	INIT_LIST_HEAD(SREJ_LIST(sk));
 }
 
@@ -1441,6 +1448,7 @@ static void l2cap_send_ack(struct l2cap_pinfo *pi)
 
 	if (pi->conn_state & L2CAP_CONN_LOCAL_BUSY) {
 		control |= L2CAP_SUPER_RCV_NOT_READY;
+		pi->conn_state |= L2CAP_CONN_RNR_SENT;
 		l2cap_send_sframe(pi, control);
 		return;
 	} else if (l2cap_ertm_send(sk) == 0) {
@@ -2267,6 +2275,9 @@ static inline void l2cap_ertm_init(struct sock *sk)
 			l2cap_ack_timeout, (unsigned long) sk);
 
 	__skb_queue_head_init(SREJ_QUEUE(sk));
+	__skb_queue_head_init(BUSY_QUEUE(sk));
+
+	INIT_WORK(&l2cap_pi(sk)->busy_work, l2cap_busy_work);
 }
 
 static int l2cap_mode_supported(__u8 mode, __u32 feat_mask)
@@ -3034,6 +3045,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 
 	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
 		skb_queue_purge(SREJ_QUEUE(sk));
+		skb_queue_purge(BUSY_QUEUE(sk));
 		del_timer(&l2cap_pi(sk)->retrans_timer);
 		del_timer(&l2cap_pi(sk)->monitor_timer);
 		del_timer(&l2cap_pi(sk)->ack_timer);
@@ -3065,6 +3077,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 
 	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
 		skb_queue_purge(SREJ_QUEUE(sk));
+		skb_queue_purge(BUSY_QUEUE(sk));
 		del_timer(&l2cap_pi(sk)->retrans_timer);
 		del_timer(&l2cap_pi(sk)->monitor_timer);
 		del_timer(&l2cap_pi(sk)->ack_timer);
@@ -3275,6 +3288,7 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
 	if (pi->conn_state & L2CAP_CONN_LOCAL_BUSY) {
 		control |= L2CAP_SUPER_RCV_NOT_READY | L2CAP_CTRL_FINAL;
 		l2cap_send_sframe(pi, control);
+		pi->conn_state |= L2CAP_CONN_RNR_SENT;
 		pi->conn_state &= ~L2CAP_CONN_SEND_FBIT;
 
 	}
@@ -3322,7 +3336,7 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
 {
 	struct l2cap_pinfo *pi = l2cap_pi(sk);
 	struct sk_buff *_skb;
-	int err = 0;
+	int err;
 
 	switch (control & L2CAP_CTRL_SAR) {
 	case L2CAP_SDU_UNSEGMENTED:
@@ -3340,16 +3354,18 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
 			goto drop;
 
 		pi->sdu_len = get_unaligned_le16(skb->data);
-		skb_pull(skb, 2);
 
 		if (pi->sdu_len > pi->imtu)
 			goto disconnect;
 
 		pi->sdu = bt_skb_alloc(pi->sdu_len, GFP_ATOMIC);
-		if (!pi->sdu) {
-			err = -ENOMEM;
-			break;
-		}
+		if (!pi->sdu)
+			return -ENOMEM;
+
+		/* pull sdu_len bytes only after alloc, because of Local Busy
+		 * condition we have to be sure that this will be executed
+		 * only once, i.e., when alloc does not fail */
+		skb_pull(skb, 2);
 
 		memcpy(skb_put(pi->sdu, skb->len), skb->data, skb->len);
 
@@ -3379,28 +3395,40 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
 		if (!pi->sdu)
 			goto disconnect;
 
-		memcpy(skb_put(pi->sdu, skb->len), skb->data, skb->len);
+		if (!(pi->conn_state & L2CAP_CONN_SAR_RETRY)) {
+			memcpy(skb_put(pi->sdu, skb->len), skb->data, skb->len);
 
-		pi->conn_state &= ~L2CAP_CONN_SAR_SDU;
-		pi->partial_sdu_len += skb->len;
+			pi->partial_sdu_len += skb->len;
 
-		if (pi->partial_sdu_len > pi->imtu)
-			goto drop;
+			if (pi->partial_sdu_len > pi->imtu)
+				goto drop;
 
-		if (pi->partial_sdu_len != pi->sdu_len)
-			goto drop;
+			if (pi->partial_sdu_len != pi->sdu_len)
+				goto drop;
+		}
 
 		_skb = skb_clone(pi->sdu, GFP_ATOMIC);
+		if (!_skb) {
+			pi->conn_state |= L2CAP_CONN_SAR_RETRY;
+			return -ENOMEM;
+		}
+
 		err = sock_queue_rcv_skb(sk, _skb);
-		if (err < 0)
+		if (err < 0) {
 			kfree_skb(_skb);
+			pi->conn_state |= L2CAP_CONN_SAR_RETRY;
+			return err;
+		}
+
+		pi->conn_state &= ~L2CAP_CONN_SAR_RETRY;
+		pi->conn_state &= ~L2CAP_CONN_SAR_SDU;
 
 		kfree_skb(pi->sdu);
 		break;
 	}
 
 	kfree_skb(skb);
-	return err;
+	return 0;
 
 drop:
 	kfree_skb(pi->sdu);
@@ -3412,6 +3440,112 @@ disconnect:
 	return 0;
 }
 
+static void l2cap_busy_work(struct work_struct *work)
+{
+	DECLARE_WAITQUEUE(wait, current);
+	struct l2cap_pinfo *pi =
+		container_of(work, struct l2cap_pinfo, busy_work);
+	struct sock *sk = (struct sock *)pi;
+	int n_tries = 0, timeo = HZ/5, err;
+	struct sk_buff *skb;
+	u16 control;
+
+	lock_sock(sk);
+
+	add_wait_queue(sk->sk_sleep, &wait);
+	while ((skb = skb_peek(BUSY_QUEUE(sk)))) {
+		set_current_state(TASK_INTERRUPTIBLE);
+
+		if (n_tries++ > L2CAP_LOCAL_BUSY_TRIES) {
+			err = -EBUSY;
+			l2cap_send_disconn_req(pi->conn, sk);
+			goto done;
+		}
+
+		if (!timeo)
+			timeo = HZ/5;
+
+		if (signal_pending(current)) {
+			err = sock_intr_errno(timeo);
+			goto done;
+		}
+
+		release_sock(sk);
+		timeo = schedule_timeout(timeo);
+		lock_sock(sk);
+
+		err = sock_error(sk);
+		if (err)
+			goto done;
+
+		while((skb = skb_peek(BUSY_QUEUE(sk)))) {
+			control = bt_cb(skb)->sar << L2CAP_CTRL_SAR_SHIFT;
+			err = l2cap_ertm_reassembly_sdu(sk, skb, control);
+			if (err < 0)
+				break;
+
+			skb_dequeue(BUSY_QUEUE(sk));
+
+			pi->buffer_seq = (pi->buffer_seq + 1) % 64;
+		}
+
+		if (!skb)
+			break;
+	}
+
+	if (!(pi->conn_state & L2CAP_CONN_RNR_SENT))
+		goto done;
+
+	control = pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT;
+	control |= L2CAP_SUPER_RCV_READY | L2CAP_CTRL_POLL;
+	l2cap_send_sframe(pi, control);
+
+	del_timer(&pi->retrans_timer);
+	__mod_monitor_timer();
+
+done:
+	pi->conn_state &= ~L2CAP_CONN_LOCAL_BUSY;
+	pi->conn_state &= ~L2CAP_CONN_RNR_SENT;
+
+	set_current_state(TASK_RUNNING);
+	remove_wait_queue(sk->sk_sleep, &wait);
+
+	release_sock(sk);
+}
+
+static int l2cap_push_rx_skb(struct sock *sk, struct sk_buff *skb, u16 control)
+{
+	struct l2cap_pinfo *pi = l2cap_pi(sk);
+	int sctrl, err;
+
+	if (pi->conn_state & L2CAP_CONN_LOCAL_BUSY) {
+		bt_cb(skb)->sar = control >> L2CAP_CTRL_SAR_SHIFT;
+		__skb_queue_tail(BUSY_QUEUE(sk), skb);
+		return -EBUSY;
+	}
+
+	err = l2cap_ertm_reassembly_sdu(sk, skb, control);
+	if (err >= 0) {
+		pi->buffer_seq = (pi->buffer_seq + 1) % 64;
+		return err;
+	}
+
+	/* Busy Condition */
+	pi->conn_state |= L2CAP_CONN_LOCAL_BUSY;
+	bt_cb(skb)->sar = control >> L2CAP_CTRL_SAR_SHIFT;
+	__skb_queue_tail(BUSY_QUEUE(sk), skb);
+
+	sctrl = pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT;
+	sctrl |= L2CAP_SUPER_RCV_NOT_READY;
+	l2cap_send_sframe(pi, sctrl);
+
+	pi->conn_state |= L2CAP_CONN_RNR_SENT;
+
+	queue_work(_busy_wq, &pi->busy_work);
+
+	return err;
+}
+
 static int l2cap_streaming_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 control)
 {
 	struct l2cap_pinfo *pi = l2cap_pi(sk);
@@ -3586,6 +3720,11 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
 	if (tx_seq == pi->expected_tx_seq)
 		goto expected;
 
+	if (pi->conn_state == L2CAP_CONN_LOCAL_BUSY) {
+		kfree_skb(skb);
+		return 0;
+	}
+
 	/* Check for duplicated tx_seq. This actually
 	 * subtracts 1 from expected_tx_seq */
 	if (tx_seq == (pi->expected_tx_seq + 63) % 64) {
@@ -3629,6 +3768,7 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
 		pi->buffer_seq_srej = pi->buffer_seq;
 
 		__skb_queue_head_init(SREJ_QUEUE(sk));
+		__skb_queue_head_init(BUSY_QUEUE(sk));
 		l2cap_add_to_srej_queue(sk, skb, tx_seq, sar);
 
 		pi->conn_state |= L2CAP_CONN_SEND_PBIT;
@@ -3656,11 +3796,9 @@ expected:
 		}
 	}
 
-	pi->buffer_seq = (pi->buffer_seq + 1) % 64;
-
-	err = l2cap_ertm_reassembly_sdu(sk, skb, rx_control);
+	err = l2cap_push_rx_skb(sk, skb, rx_control);
 	if (err < 0)
-		return err;
+		return 0;
 
 	__mod_ack_timer();
 
@@ -4355,6 +4493,10 @@ static int __init l2cap_init(void)
 	if (err < 0)
 		return err;
 
+	_busy_wq = create_singlethread_workqueue("l2cap");
+	if (!_busy_wq)
+		goto error;
+
 	err = bt_sock_register(BTPROTO_L2CAP, &l2cap_sock_family_ops);
 	if (err < 0) {
 		BT_ERR("L2CAP socket registration failed");
@@ -4389,6 +4531,9 @@ static void __exit l2cap_exit(void)
 {
 	debugfs_remove(l2cap_debugfs);
 
+	flush_workqueue(_busy_wq);
+	destroy_workqueue(_busy_wq);
+
 	if (bt_sock_unregister(BTPROTO_L2CAP) < 0)
 		BT_ERR("L2CAP socket unregistration failed");
 
-- 
1.6.4.4

^ permalink raw reply related

* Re: [PATCH v3] Added support for Atheros AR300x Bluetooth Chip
From: Luis R. Rodriguez @ 2010-04-20 17:34 UTC (permalink / raw)
  To: Suraj Sumangala
  Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
	Luis Rodriguez, Jothikumar Mothilal, gfpadovan@gmail.com
In-Reply-To: <1271758832.6585.33.camel@atheros013-desktop>

On Tue, Apr 20, 2010 at 03:20:32AM -0700, Suraj Sumangala wrote:
> This protocol implements support for power management feature provided by AR300x chip.
> This lets the controller chip go to sleep mode if there is no Bluetooth
> activity for some time.
> It then wakes up the chip in case of a Bluetooth activity.

The above commit log needs some more work. Try to keep the
commit log entry subject to about 50 characters, the context should
not pass around 75 characters. Your commit also indicates this is a
"protocol" ? This is driver, and you do have some hacks for enhancing
power saving, but that is not that relevant to the commit log.

How about:

---
Add support for the Atheros AR300x Bluetooth Chip

This adds support for the Atheros Bluetooth serial protocol to
support the AR300x chipsets. The serial protocol implements
enhanced power management features for the AR300x chipsets.

Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: Suraj <suraj@atheros.com>
---

Of course you will need to adjust the subject and prepend it with
PATCH v4, just as you did with a v3 for this patch.

Then, this stuff:

> * Third version
> 
> ** Updated with extra spacing and indentation

Do you use checkpatch.pl for your patches? If not please
add check your patches after committing them with:

git show ./scripts/checkpatch.pl -
vi drivers/bluetooth/hci_ath.c
git commit -a --amend

And repeat until checkpatch.pl is git happy :)

> ** made function definitions static
> ** Removed inline and register keyword usage.
> ** Removed unused return calls.
> ** Incorporated code comments by Luis and Gustavo
> 
> Thanks Luis and Gustavo for your comments


Remove it from the commit log, if you do want to add some extra text
to the patch youc an put it below the three dashes ("-") where the
diff stat goes:

> 
> 
> Signed-off-by: Suraj <suraj@atheros.com>
> 
> ---
  ^^^

These are the three lines, anything below is ignored by git am
when the maintainer applies the patch. So you can add anything you
want ignored by git am here.

>  drivers/bluetooth/Kconfig     |   11 ++
>  drivers/bluetooth/Makefile    |    1 +
>  drivers/bluetooth/hci_ath.c   |  384 +++++++++++++++++++++++++++++++++++++++++
>  drivers/bluetooth/hci_ldisc.c |    6 +
>  drivers/bluetooth/hci_uart.h  |    8 +-
>  5 files changed, 409 insertions(+), 1 deletions(-)
>  create mode 100755 drivers/bluetooth/hci_ath.c
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 058fbcc..81abeff 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -58,6 +58,17 @@ config BT_HCIUART_BCSP
>  
>  	  Say Y here to compile support for HCI BCSP protocol.
>  
> +config BT_HCIUART_ATH
> +	bool "Atheros AR300x Board support"
> +	depends on BT_HCIUART
> +	help
> +	  HCIATH (HCI Atheros) is a serial protocol for communication
> +	  between Bluetooth device and host with support for Atheros AR300x
> +	  power management feature. This protocol is required for
> +	  serial Bluetooth devices that are based on Atheros AR300x chips.

Please adjust the description as well. How about:

+	  HCIATH (HCI Atheros) is a serial protocol for communication
+	  between the host and Atheros AR300x Bluetooth devices. The
+	  protocol implements enhaned power management features for the
+	  the AR300x chipsets, it lets the controller chip go to sleep
+	  mode if there is no Bluetooth activity for some time and wakes
+	  up the chip in case of a Bluetooth activity. Enabling this
+	  option will build HCI Atheros support into the hci_uart driver.
+	  Enable this option if you have an UART Atheros AR300x serial
+	  device.


> +
> +	  Say Y here to compile support for HCIATH protocol.
> +
>  config BT_HCIUART_LL
>  	bool "HCILL protocol support"
>  	depends on BT_HCIUART
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 7e5aed5..1481faa 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -26,4 +26,5 @@ hci_uart-y				:= hci_ldisc.o
>  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_ATH)	+= hci_ath.o
>  hci_uart-objs				:= $(hci_uart-y)
> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
> new file mode 100755
> index 0000000..2f91954
> --- /dev/null
> +++ b/drivers/bluetooth/hci_ath.c
> @@ -0,0 +1,384 @@
> +/*
> + * Copyright (c) 2009-2010 Atheros Communications Inc.
> + *
> + *  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/module.h>
> +#include <linux/kernel.h>
> +
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/errno.h>
> +#include <linux/ioctl.h>
> +#include <linux/skbuff.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include "hci_uart.h"
> +
> +
> +/* HCIATH receiver States */
> +#define HCIATH_W4_PACKET_TYPE			0
> +#define HCIATH_W4_EVENT_HDR			1
> +#define HCIATH_W4_ACL_HDR			2
> +#define HCIATH_W4_SCO_HDR			3
> +#define HCIATH_W4_DATA				4
> +
> +struct ath_struct {
> +	struct hci_uart *hu;
> +	unsigned int rx_state;
> +	unsigned int rx_count;
> +	unsigned int cur_sleep;
> +
> +	spinlock_t hciath_lock;
> +	struct sk_buff *rx_skb;
> +	struct sk_buff_head txq;
> +	wait_queue_head_t wqevt;
> +	struct work_struct ctxtsw;
> +};
> +
> +static int ath_wakeup_ar3001(struct tty_struct *tty)
> +{
> +	struct termios settings;
> +	int status = 0x00;
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	if ((status & TIOCM_CTS))
> +		return status;

No need for double () parens here. This should be fine:

+	if (status & TIOCM_CTS)
+		return status;

You would use double parens if you are doing a check against
another flag as well.

> +
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	/* Disable Automatic RTSCTS */
> +	settings.c_cflag &= ~CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	/* Clear RTS first */
> +	tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	/* Set RTS, wake up board */
> +	tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	settings.c_cflag |= CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +
> +	return status;
> +}
> +
> +static void ath_context_switch(struct work_struct *work)

Can you please rename this to ath_hci_uart_work()

> +{
> +	int status;
> +	struct ath_struct *ath;
> +	struct hci_uart *hu;
> +	struct tty_struct *tty;
> +
> +	ath = container_of(work, struct ath_struct, ctxtsw);
> +
> +	hu = ath->hu;
> +	tty = hu->tty;
> +
> +	/* verify and wake up controller */
> +	if (ath->cur_sleep) {
> +
> +		status = ath_wakeup_ar3001(tty);
> +
> +		if (!(status & TIOCM_CTS))
> +			return;
> +	}
> +
> +	/* Ready to send Data */
> +	clear_bit(HCI_UART_SENDING, &hu->tx_state);
> +	hci_uart_tx_wakeup(hu);
> +}
> +
> +/* Initialize protocol */
> +static int ath_open(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
> +	if (!ath)
> +		return -ENOMEM;
> +
> +	skb_queue_head_init(&ath->txq);
> +	spin_lock_init(&ath->hciath_lock);
> +
> +	hu->priv = ath;
> +	ath->hu = hu;
> +
> +	init_waitqueue_head(&ath->wqevt);
> +	INIT_WORK(&ath->ctxtsw, ath_context_switch);
> +
> +	return 0;
> +}
> +
> +/* Flush protocol data */
> +static int ath_flush(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	skb_queue_purge(&ath->txq);
> +
> +	return 0;
> +}
> +
> +/* Close protocol */
> +static int ath_close(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	skb_queue_purge(&ath->txq);
> +
> +	if (ath->rx_skb)
> +		kfree_skb(ath->rx_skb);

No need for the check, kfree_skb() does that for you
and it has it optimized for the case where the skb you
pass is NULL. You can just do:

+	kfree_skb(ath->rx_skb);

> +
> +	cancel_work_sync(&ath->ctxtsw);
> +
> +	hu->priv = NULL;
> +	kfree(ath);
> +
> +	return 0;
> +}

Did you give this new patch a spin by looping bringing up,
scanning, bringing the interface down? Or have a loop doing
a scan while in another window you bring the interface up
and down?

> +
> +/* Enqueue frame for transmittion */
> +static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
> +
> +		/* Discard SCO packet.AR3001 does not support SCO over HCI */

Add a space after packet. Why does it not support SCO over HCI BTW?
Just curious. I'm new to BT :) Is this common? If this is common can't
the BT stack be informed of these things so that they don't pass the skbs
to the driver?

This could be done in a separate patch though if this is the case
though.

> +		BT_DBG("SCO Packet over HCI received Dropping");
> +
> +		kfree(skb);
> +
> +		return 0;
> +	}
> +
> +	BT_DBG("hu %p skb %p", hu, skb);
> +
> +	/* Prepend skb with frame type */
> +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> +	skb_queue_tail(&ath->txq, skb);
> +	set_bit(HCI_UART_SENDING, &hu->tx_state);
> +
> +	schedule_work(&ath->ctxtsw);
> +
> +	return 0;
> +}
> +
> +static struct sk_buff *ath_dequeue(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	struct sk_buff *skbuf;
> +
> +	skbuf = skb_dequeue(&ath->txq);
> +
> +	if (!skbuf)
> +		return NULL;
> +
> +
> +	/*
> +	 * Check if the HCI command is  HCI sleep enable and
> +	 * update the sleep enable flag with command parameter.
> +	 *
> +	 * Value of sleep enable flag will be used later
> +	 * to verify if controller has to be woken up before
> +	 * sending any packet.
> +	 */
> +	if (skbuf->data[0] == 0x01 && skbuf->data[1] == 0x04 &&
> +	    skbuf->data[2] == 0xFC)
> +		ath->cur_sleep = skbuf->data[4];

Might as well just do this:

+	if (skbuf->data[0] == 0x01 &&
+	    skbuf->data[1] == 0x04 &&
+	    skbuf->data[2] == 0xFC)
+		ath->cur_sleep = skbuf->data[4];

Is this sort of check done in any other drivers/protocols? If so a
helper could be added to hci_uart.h if this is the case, but likely
better though a separate patch.

> +
> +	return skbuf;
> +}
> +
> +static void ath_check_data_len(struct ath_struct *ath, int len)
> +{
> +	int room = skb_tailroom(ath->rx_skb);
> +
> +	BT_DBG("len %d room %d", len, room);
> +
> +	if (len > room) {
> +		BT_ERR("Data length is too large");
> +		kfree_skb(ath->rx_skb);
> +		ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +		ath->rx_skb = NULL;
> +		ath->rx_count = 0;
> +	} else {
> +		ath->rx_state = HCIATH_W4_DATA;
> +		ath->rx_count = len;
> +	}
> +}
> +
> +/* Recv data */
> +static int ath_recv(struct hci_uart *hu, void *data, int count)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	char *ptr = data;
> +	struct hci_event_hdr *eh;
> +	struct hci_acl_hdr *ah;
> +	struct hci_sco_hdr *sh;
> +	int len, type, dlen;
> +
> +
> +	BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
> +	       ath->rx_state, ath->rx_count);
> +
> +	while (count) {
> +		if (ath->rx_count) {
> +
> +			len = min_t(unsigned int, ath->rx_count, count);
> +			memcpy(skb_put(ath->rx_skb, len), ptr, len);
> +			ath->rx_count -= len;
> +			count -= len;
> +			ptr += len;
> +
> +			if (ath->rx_count)
> +				continue;
> +			switch (ath->rx_state) {
> +			case HCIATH_W4_DATA:
> +				hci_recv_frame(ath->rx_skb);
> +				ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +				ath->rx_skb = NULL;
> +				ath->rx_count = 0;
> +				continue;
> +
> +			case HCIATH_W4_EVENT_HDR:
> +				eh = (struct hci_event_hdr *)ath->rx_skb->data;
> +
> +				BT_DBG("Event header: evt 0x%2.2x plen %d",
> +				       eh->evt, eh->plen);
> +
> +				ath_check_data_len(ath, eh->plen);
> +				continue;
> +
> +			case HCIATH_W4_ACL_HDR:
> +				ah = (struct hci_acl_hdr *)ath->rx_skb->data;
> +				dlen = __le16_to_cpu(ah->dlen);
> +
> +				BT_DBG("ACL header: dlen %d", dlen);
> +
> +				ath_check_data_len(ath, dlen);
> +				continue;
> +
> +			case HCIATH_W4_SCO_HDR:
> +				sh = (struct hci_sco_hdr *)ath->rx_skb->data;
> +
> +				BT_DBG("SCO header: dlen %d", sh->dlen);
> +
> +				ath_check_data_len(ath, sh->dlen);
> +				continue;
> +
> +			}
> +		}
> +
> +		/* HCIATH_W4_PACKET_TYPE */
> +		switch (*ptr) {
> +		case HCI_EVENT_PKT:
> +			BT_DBG("Event packet");
> +			ath->rx_state = HCIATH_W4_EVENT_HDR;
> +			ath->rx_count = HCI_EVENT_HDR_SIZE;
> +			type = HCI_EVENT_PKT;
> +			break;
> +
> +		case HCI_ACLDATA_PKT:
> +			BT_DBG("ACL packet");
> +			ath->rx_state = HCIATH_W4_ACL_HDR;
> +			ath->rx_count = HCI_ACL_HDR_SIZE;
> +			type = HCI_ACLDATA_PKT;
> +			break;
> +
> +		case HCI_SCODATA_PKT:
> +			BT_DBG("SCO packet");
> +			ath->rx_state = HCIATH_W4_SCO_HDR;
> +			ath->rx_count = HCI_SCO_HDR_SIZE;
> +			type = HCI_SCODATA_PKT;
> +			break;
> +
> +		default:
> +			BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
> +			hu->hdev->stat.err_rx++;
> +			ptr++;
> +			count--;
> +			continue;
> +
> +		};
> +		ptr++;
> +		count--;
> +
> +		/* Allocate packet */
> +		ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> +		if (!ath->rx_skb) {
> +			BT_ERR("Can't allocate mem for new packet");
> +			ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +			ath->rx_count = 0;
> +
> +			return -ENOMEM;
> +		}
> +		ath->rx_skb->dev = (void *)hu->hdev;
> +		bt_cb(ath->rx_skb)->pkt_type = type;
> +	}
> +
> +	return count;
> +}
> +
> +static struct hci_uart_proto athp = {
> +	.id = HCI_UART_ATH,
> +	.open = ath_open,
> +	.close = ath_close,
> +	.recv = ath_recv,
> +	.enqueue = ath_enqueue,
> +	.dequeue = ath_dequeue,
> +	.flush = ath_flush,
> +};
> +
> +int ath_init(void)
> +{
> +	int err = hci_uart_register_proto(&athp);
> +
> +	if (!err)
> +		BT_INFO("HCIATH protocol initialized");
> +	else
> +		BT_ERR("HCIATH protocol registration failed");
> +
> +	return err;
> +}
> +
> +int ath_deinit(void)
> +{
> +	return hci_uart_unregister_proto(&athp);
> +}
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 76a1abb..7dd76d1 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_init();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_init();
> +#endif
>  
>  	return 0;
>  }
> @@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_deinit();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_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 50113db..385537f 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -33,13 +33,14 @@
>  #define HCIUARTGETDEVICE	_IOR('U', 202, int)
>  
>  /* UART protocols */
> -#define HCI_UART_MAX_PROTO	5
> +#define HCI_UART_MAX_PROTO	6
>  
>  #define HCI_UART_H4	0
>  #define HCI_UART_BCSP	1
>  #define HCI_UART_3WIRE	2
>  #define HCI_UART_H4DS	3
>  #define HCI_UART_LL	4
> +#define HCI_UART_ATH	5
>  
>  struct hci_uart;
>  
> @@ -91,3 +92,8 @@ int bcsp_deinit(void);
>  int ll_init(void);
>  int ll_deinit(void);
>  #endif
> +
> +#ifdef CONFIG_BT_HCIUART_ATH
> +int ath_init(void);
> +int ath_deinit(void);
> +#endif
> -- 
> 1.7.0
> 
> 
> 

^ permalink raw reply

* Re: [PATCH] Added support for deleting all MDLS in MCAP
From: Elvis Pfützenreuter @ 2010-04-20 16:44 UTC (permalink / raw)
  To: jcaden; +Cc: Santiago Carot Nemesio, linux-bluetooth
In-Reply-To: <201004091344.41686.jcaden@libresoft.es>

Taking a look on these.

^ permalink raw reply

* Re: [PATCH v3] Added support for Atheros AR300x Bluetooth Chip
From: Gustavo F. Padovan @ 2010-04-20 15:36 UTC (permalink / raw)
  To: suraj; +Cc: linux-bluetooth, marcel, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1271758832.6585.33.camel@atheros013-desktop>

Hi Suraj,

* suraj <suraj@atheros.com> [2010-04-20 15:50:32 +0530]:

> This protocol implements support for power management feature provided by AR300x chip.
> This lets the controller chip go to sleep mode if there is no Bluetooth
> activity for some time.
> It then wakes up the chip in case of a Bluetooth activity.
> 
> * Third version
> 
> ** Updated with extra spacing and indentation
> ** made function definitions static
> ** Removed inline and register keyword usage.
> ** Removed unused return calls.
> ** Incorporated code comments by Luis and Gustavo
> 
> Thanks Luis and Gustavo for your comments
> 
> 
> Signed-off-by: Suraj <suraj@atheros.com>
> 
> ---
>  drivers/bluetooth/Kconfig     |   11 ++
>  drivers/bluetooth/Makefile    |    1 +
>  drivers/bluetooth/hci_ath.c   |  384 +++++++++++++++++++++++++++++++++++++++++
>  drivers/bluetooth/hci_ldisc.c |    6 +
>  drivers/bluetooth/hci_uart.h  |    8 +-
>  5 files changed, 409 insertions(+), 1 deletions(-)
>  create mode 100755 drivers/bluetooth/hci_ath.c
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 058fbcc..81abeff 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -58,6 +58,17 @@ config BT_HCIUART_BCSP
>  
>  	  Say Y here to compile support for HCI BCSP protocol.
>  
> +config BT_HCIUART_ATH
> +	bool "Atheros AR300x Board support"
> +	depends on BT_HCIUART
> +	help
> +	  HCIATH (HCI Atheros) is a serial protocol for communication
> +	  between Bluetooth device and host with support for Atheros AR300x
> +	  power management feature. This protocol is required for
> +	  serial Bluetooth devices that are based on Atheros AR300x chips.
> +
> +	  Say Y here to compile support for HCIATH protocol.
> +
>  config BT_HCIUART_LL
>  	bool "HCILL protocol support"
>  	depends on BT_HCIUART
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 7e5aed5..1481faa 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -26,4 +26,5 @@ hci_uart-y				:= hci_ldisc.o
>  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_ATH)	+= hci_ath.o
>  hci_uart-objs				:= $(hci_uart-y)
> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
> new file mode 100755
> index 0000000..2f91954
> --- /dev/null
> +++ b/drivers/bluetooth/hci_ath.c
> @@ -0,0 +1,384 @@
> +/*
> + * Copyright (c) 2009-2010 Atheros Communications Inc.
> + *
> + *  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/module.h>
> +#include <linux/kernel.h>
> +
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/errno.h>
> +#include <linux/ioctl.h>
> +#include <linux/skbuff.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include "hci_uart.h"
> +
> +
> +/* HCIATH receiver States */
> +#define HCIATH_W4_PACKET_TYPE			0
> +#define HCIATH_W4_EVENT_HDR			1
> +#define HCIATH_W4_ACL_HDR			2
> +#define HCIATH_W4_SCO_HDR			3
> +#define HCIATH_W4_DATA				4
> +
> +struct ath_struct {
> +	struct hci_uart *hu;
> +	unsigned int rx_state;
> +	unsigned int rx_count;
> +	unsigned int cur_sleep;
> +
> +	spinlock_t hciath_lock;
> +	struct sk_buff *rx_skb;
> +	struct sk_buff_head txq;
> +	wait_queue_head_t wqevt;
> +	struct work_struct ctxtsw;
> +};
> +
> +static int ath_wakeup_ar3001(struct tty_struct *tty)
> +{
> +	struct termios settings;
> +	int status = 0x00;
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	if ((status & TIOCM_CTS))
> +		return status;
> +
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	/* Disable Automatic RTSCTS */
> +	settings.c_cflag &= ~CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	/* Clear RTS first */
> +	tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	/* Set RTS, wake up board */
> +	tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	settings.c_cflag |= CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +
> +	return status;
> +}
> +
> +static void ath_context_switch(struct work_struct *work)
> +{
> +	int status;
> +	struct ath_struct *ath;
> +	struct hci_uart *hu;
> +	struct tty_struct *tty;
> +
> +	ath = container_of(work, struct ath_struct, ctxtsw);
> +
> +	hu = ath->hu;
> +	tty = hu->tty;
> +
> +	/* verify and wake up controller */
> +	if (ath->cur_sleep) {
> +

Extra blank line here.

> +		status = ath_wakeup_ar3001(tty);
> +
> +		if (!(status & TIOCM_CTS))
> +			return;
> +	}
> +
> +	/* Ready to send Data */
> +	clear_bit(HCI_UART_SENDING, &hu->tx_state);
> +	hci_uart_tx_wakeup(hu);
> +}
> +
> +/* Initialize protocol */
> +static int ath_open(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
> +	if (!ath)
> +		return -ENOMEM;
> +
> +	skb_queue_head_init(&ath->txq);
> +	spin_lock_init(&ath->hciath_lock);
> +
> +	hu->priv = ath;
> +	ath->hu = hu;
> +
> +	init_waitqueue_head(&ath->wqevt);
> +	INIT_WORK(&ath->ctxtsw, ath_context_switch);
> +
> +	return 0;
> +}
> +
> +/* Flush protocol data */
> +static int ath_flush(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	skb_queue_purge(&ath->txq);
> +
> +	return 0;
> +}
> +
> +/* Close protocol */
> +static int ath_close(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	BT_DBG("hu %p", hu);
> +
> +	skb_queue_purge(&ath->txq);
> +
> +	if (ath->rx_skb)
> +		kfree_skb(ath->rx_skb);
> +
> +	cancel_work_sync(&ath->ctxtsw);
> +
> +	hu->priv = NULL;
> +	kfree(ath);
> +
> +	return 0;
> +}
> +
> +/* Enqueue frame for transmittion */
> +static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
> +{
> +	struct ath_struct *ath = hu->priv;
> +
> +	if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
> +
> +		/* Discard SCO packet.AR3001 does not support SCO over HCI */
> +		BT_DBG("SCO Packet over HCI received Dropping");
> +
> +		kfree(skb);
> +
> +		return 0;
> +	}
> +
> +	BT_DBG("hu %p skb %p", hu, skb);
> +
> +	/* Prepend skb with frame type */
> +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> +	skb_queue_tail(&ath->txq, skb);
> +	set_bit(HCI_UART_SENDING, &hu->tx_state);
> +
> +	schedule_work(&ath->ctxtsw);
> +
> +	return 0;
> +}
> +
> +static struct sk_buff *ath_dequeue(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	struct sk_buff *skbuf;
> +
> +	skbuf = skb_dequeue(&ath->txq);
> +
> +	if (!skbuf)
> +		return NULL;
> +
> +

Another extra blank line here.

> +	/*
> +	 * Check if the HCI command is  HCI sleep enable and
> +	 * update the sleep enable flag with command parameter.
> +	 *
> +	 * Value of sleep enable flag will be used later
> +	 * to verify if controller has to be woken up before
> +	 * sending any packet.
> +	 */
> +	if (skbuf->data[0] == 0x01 && skbuf->data[1] == 0x04 &&
> +	    skbuf->data[2] == 0xFC)
> +		ath->cur_sleep = skbuf->data[4];
> +
> +	return skbuf;
> +}
> +
> +static void ath_check_data_len(struct ath_struct *ath, int len)
> +{
> +	int room = skb_tailroom(ath->rx_skb);
> +
> +	BT_DBG("len %d room %d", len, room);
> +
> +	if (len > room) {
> +		BT_ERR("Data length is too large");
> +		kfree_skb(ath->rx_skb);
> +		ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +		ath->rx_skb = NULL;
> +		ath->rx_count = 0;
> +	} else {
> +		ath->rx_state = HCIATH_W4_DATA;
> +		ath->rx_count = len;
> +	}
> +}
> +
> +/* Recv data */
> +static int ath_recv(struct hci_uart *hu, void *data, int count)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	char *ptr = data;
> +	struct hci_event_hdr *eh;
> +	struct hci_acl_hdr *ah;
> +	struct hci_sco_hdr *sh;
> +	int len, type, dlen;
> +
> +

Extra blank line here too.

> +	BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
> +	       ath->rx_state, ath->rx_count);
> +
> +	while (count) {
> +		if (ath->rx_count) {
> +
> +			len = min_t(unsigned int, ath->rx_count, count);
> +			memcpy(skb_put(ath->rx_skb, len), ptr, len);
> +			ath->rx_count -= len;
> +			count -= len;
> +			ptr += len;
> +
> +			if (ath->rx_count)
> +				continue;
> +			switch (ath->rx_state) {
> +			case HCIATH_W4_DATA:
> +				hci_recv_frame(ath->rx_skb);
> +				ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +				ath->rx_skb = NULL;
> +				ath->rx_count = 0;
> +				continue;
> +
> +			case HCIATH_W4_EVENT_HDR:
> +				eh = (struct hci_event_hdr *)ath->rx_skb->data;
> +
> +				BT_DBG("Event header: evt 0x%2.2x plen %d",
> +				       eh->evt, eh->plen);
> +
> +				ath_check_data_len(ath, eh->plen);
> +				continue;
> +
> +			case HCIATH_W4_ACL_HDR:
> +				ah = (struct hci_acl_hdr *)ath->rx_skb->data;
> +				dlen = __le16_to_cpu(ah->dlen);
> +
> +				BT_DBG("ACL header: dlen %d", dlen);
> +
> +				ath_check_data_len(ath, dlen);
> +				continue;
> +
> +			case HCIATH_W4_SCO_HDR:
> +				sh = (struct hci_sco_hdr *)ath->rx_skb->data;
> +
> +				BT_DBG("SCO header: dlen %d", sh->dlen);
> +
> +				ath_check_data_len(ath, sh->dlen);
> +				continue;
> +
> +			}
> +		}
> +
> +		/* HCIATH_W4_PACKET_TYPE */
> +		switch (*ptr) {
> +		case HCI_EVENT_PKT:
> +			BT_DBG("Event packet");
> +			ath->rx_state = HCIATH_W4_EVENT_HDR;
> +			ath->rx_count = HCI_EVENT_HDR_SIZE;
> +			type = HCI_EVENT_PKT;
> +			break;
> +
> +		case HCI_ACLDATA_PKT:
> +			BT_DBG("ACL packet");
> +			ath->rx_state = HCIATH_W4_ACL_HDR;
> +			ath->rx_count = HCI_ACL_HDR_SIZE;
> +			type = HCI_ACLDATA_PKT;
> +			break;
> +
> +		case HCI_SCODATA_PKT:
> +			BT_DBG("SCO packet");
> +			ath->rx_state = HCIATH_W4_SCO_HDR;
> +			ath->rx_count = HCI_SCO_HDR_SIZE;
> +			type = HCI_SCODATA_PKT;
> +			break;
> +
> +		default:
> +			BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
> +			hu->hdev->stat.err_rx++;
> +			ptr++;
> +			count--;
> +			continue;
> +
> +		};
> +		ptr++;
> +		count--;
> +
> +		/* Allocate packet */
> +		ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> +		if (!ath->rx_skb) {
> +			BT_ERR("Can't allocate mem for new packet");
> +			ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +			ath->rx_count = 0;
> +
> +			return -ENOMEM;
> +		}
> +		ath->rx_skb->dev = (void *)hu->hdev;
> +		bt_cb(ath->rx_skb)->pkt_type = type;
> +	}
> +
> +	return count;
> +}
> +
> +static struct hci_uart_proto athp = {
> +	.id = HCI_UART_ATH,
> +	.open = ath_open,
> +	.close = ath_close,
> +	.recv = ath_recv,
> +	.enqueue = ath_enqueue,
> +	.dequeue = ath_dequeue,
> +	.flush = ath_flush,
> +};
> +
> +int ath_init(void)
> +{
> +	int err = hci_uart_register_proto(&athp);
> +
> +	if (!err)
> +		BT_INFO("HCIATH protocol initialized");
> +	else
> +		BT_ERR("HCIATH protocol registration failed");
> +
> +	return err;
> +}
> +
> +int ath_deinit(void)
> +{
> +	return hci_uart_unregister_proto(&athp);
> +}
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 76a1abb..7dd76d1 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_init();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_init();
> +#endif
>  
>  	return 0;
>  }
> @@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_deinit();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_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 50113db..385537f 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -33,13 +33,14 @@
>  #define HCIUARTGETDEVICE	_IOR('U', 202, int)
>  
>  /* UART protocols */
> -#define HCI_UART_MAX_PROTO	5
> +#define HCI_UART_MAX_PROTO	6
>  
>  #define HCI_UART_H4	0
>  #define HCI_UART_BCSP	1
>  #define HCI_UART_3WIRE	2
>  #define HCI_UART_H4DS	3
>  #define HCI_UART_LL	4
> +#define HCI_UART_ATH	5
>  
>  struct hci_uart;
>  
> @@ -91,3 +92,8 @@ int bcsp_deinit(void);
>  int ll_init(void);
>  int ll_deinit(void);
>  #endif
> +
> +#ifdef CONFIG_BT_HCIUART_ATH
> +int ath_init(void);
> +int ath_deinit(void);
> +#endif
> -- 
> 1.7.0
> 
> 
> 

-- 
Gustavo F. Padovan
http://padovan.org

^ permalink raw reply

* [PATCH v3] Added support for Atheros AR300x Bluetooth Chip
From: suraj @ 2010-04-20 10:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal, gfpadovan
In-Reply-To: <1268629296.21425.23.camel@atheros013-desktop>

This protocol implements support for power management feature provided by AR300x chip.
This lets the controller chip go to sleep mode if there is no Bluetooth
activity for some time.
It then wakes up the chip in case of a Bluetooth activity.

* Third version

** Updated with extra spacing and indentation
** made function definitions static
** Removed inline and register keyword usage.
** Removed unused return calls.
** Incorporated code comments by Luis and Gustavo

Thanks Luis and Gustavo for your comments


Signed-off-by: Suraj <suraj@atheros.com>

---
 drivers/bluetooth/Kconfig     |   11 ++
 drivers/bluetooth/Makefile    |    1 +
 drivers/bluetooth/hci_ath.c   |  384 +++++++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/hci_ldisc.c |    6 +
 drivers/bluetooth/hci_uart.h  |    8 +-
 5 files changed, 409 insertions(+), 1 deletions(-)
 create mode 100755 drivers/bluetooth/hci_ath.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 058fbcc..81abeff 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -58,6 +58,17 @@ config BT_HCIUART_BCSP
 
 	  Say Y here to compile support for HCI BCSP protocol.
 
+config BT_HCIUART_ATH
+	bool "Atheros AR300x Board support"
+	depends on BT_HCIUART
+	help
+	  HCIATH (HCI Atheros) is a serial protocol for communication
+	  between Bluetooth device and host with support for Atheros AR300x
+	  power management feature. This protocol is required for
+	  serial Bluetooth devices that are based on Atheros AR300x chips.
+
+	  Say Y here to compile support for HCIATH protocol.
+
 config BT_HCIUART_LL
 	bool "HCILL protocol support"
 	depends on BT_HCIUART
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 7e5aed5..1481faa 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -26,4 +26,5 @@ hci_uart-y				:= hci_ldisc.o
 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_ATH)	+= hci_ath.o
 hci_uart-objs				:= $(hci_uart-y)
diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
new file mode 100755
index 0000000..2f91954
--- /dev/null
+++ b/drivers/bluetooth/hci_ath.c
@@ -0,0 +1,384 @@
+/*
+ * Copyright (c) 2009-2010 Atheros Communications Inc.
+ *
+ *  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/module.h>
+#include <linux/kernel.h>
+
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/errno.h>
+#include <linux/ioctl.h>
+#include <linux/skbuff.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "hci_uart.h"
+
+
+/* HCIATH receiver States */
+#define HCIATH_W4_PACKET_TYPE			0
+#define HCIATH_W4_EVENT_HDR			1
+#define HCIATH_W4_ACL_HDR			2
+#define HCIATH_W4_SCO_HDR			3
+#define HCIATH_W4_DATA				4
+
+struct ath_struct {
+	struct hci_uart *hu;
+	unsigned int rx_state;
+	unsigned int rx_count;
+	unsigned int cur_sleep;
+
+	spinlock_t hciath_lock;
+	struct sk_buff *rx_skb;
+	struct sk_buff_head txq;
+	wait_queue_head_t wqevt;
+	struct work_struct ctxtsw;
+};
+
+static int ath_wakeup_ar3001(struct tty_struct *tty)
+{
+	struct termios settings;
+	int status = 0x00;
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	if ((status & TIOCM_CTS))
+		return status;
+
+	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
+
+	/* Disable Automatic RTSCTS */
+	settings.c_cflag &= ~CRTSCTS;
+	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	/* Clear RTS first */
+	tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
+	mdelay(20);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	/* Set RTS, wake up board */
+	tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
+	mdelay(20);
+
+	status = tty->driver->ops->tiocmget(tty, NULL);
+
+	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
+
+	settings.c_cflag |= CRTSCTS;
+	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
+
+	return status;
+}
+
+static void ath_context_switch(struct work_struct *work)
+{
+	int status;
+	struct ath_struct *ath;
+	struct hci_uart *hu;
+	struct tty_struct *tty;
+
+	ath = container_of(work, struct ath_struct, ctxtsw);
+
+	hu = ath->hu;
+	tty = hu->tty;
+
+	/* verify and wake up controller */
+	if (ath->cur_sleep) {
+
+		status = ath_wakeup_ar3001(tty);
+
+		if (!(status & TIOCM_CTS))
+			return;
+	}
+
+	/* Ready to send Data */
+	clear_bit(HCI_UART_SENDING, &hu->tx_state);
+	hci_uart_tx_wakeup(hu);
+}
+
+/* Initialize protocol */
+static int ath_open(struct hci_uart *hu)
+{
+	struct ath_struct *ath;
+
+	BT_DBG("hu %p", hu);
+
+	ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
+	if (!ath)
+		return -ENOMEM;
+
+	skb_queue_head_init(&ath->txq);
+	spin_lock_init(&ath->hciath_lock);
+
+	hu->priv = ath;
+	ath->hu = hu;
+
+	init_waitqueue_head(&ath->wqevt);
+	INIT_WORK(&ath->ctxtsw, ath_context_switch);
+
+	return 0;
+}
+
+/* Flush protocol data */
+static int ath_flush(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+
+	BT_DBG("hu %p", hu);
+
+	skb_queue_purge(&ath->txq);
+
+	return 0;
+}
+
+/* Close protocol */
+static int ath_close(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+
+	BT_DBG("hu %p", hu);
+
+	skb_queue_purge(&ath->txq);
+
+	if (ath->rx_skb)
+		kfree_skb(ath->rx_skb);
+
+	cancel_work_sync(&ath->ctxtsw);
+
+	hu->priv = NULL;
+	kfree(ath);
+
+	return 0;
+}
+
+/* Enqueue frame for transmittion */
+static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
+{
+	struct ath_struct *ath = hu->priv;
+
+	if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
+
+		/* Discard SCO packet.AR3001 does not support SCO over HCI */
+		BT_DBG("SCO Packet over HCI received Dropping");
+
+		kfree(skb);
+
+		return 0;
+	}
+
+	BT_DBG("hu %p skb %p", hu, skb);
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	skb_queue_tail(&ath->txq, skb);
+	set_bit(HCI_UART_SENDING, &hu->tx_state);
+
+	schedule_work(&ath->ctxtsw);
+
+	return 0;
+}
+
+static struct sk_buff *ath_dequeue(struct hci_uart *hu)
+{
+	struct ath_struct *ath = hu->priv;
+	struct sk_buff *skbuf;
+
+	skbuf = skb_dequeue(&ath->txq);
+
+	if (!skbuf)
+		return NULL;
+
+
+	/*
+	 * Check if the HCI command is  HCI sleep enable and
+	 * update the sleep enable flag with command parameter.
+	 *
+	 * Value of sleep enable flag will be used later
+	 * to verify if controller has to be woken up before
+	 * sending any packet.
+	 */
+	if (skbuf->data[0] == 0x01 && skbuf->data[1] == 0x04 &&
+	    skbuf->data[2] == 0xFC)
+		ath->cur_sleep = skbuf->data[4];
+
+	return skbuf;
+}
+
+static void ath_check_data_len(struct ath_struct *ath, int len)
+{
+	int room = skb_tailroom(ath->rx_skb);
+
+	BT_DBG("len %d room %d", len, room);
+
+	if (len > room) {
+		BT_ERR("Data length is too large");
+		kfree_skb(ath->rx_skb);
+		ath->rx_state = HCIATH_W4_PACKET_TYPE;
+		ath->rx_skb = NULL;
+		ath->rx_count = 0;
+	} else {
+		ath->rx_state = HCIATH_W4_DATA;
+		ath->rx_count = len;
+	}
+}
+
+/* Recv data */
+static int ath_recv(struct hci_uart *hu, void *data, int count)
+{
+	struct ath_struct *ath = hu->priv;
+	char *ptr = data;
+	struct hci_event_hdr *eh;
+	struct hci_acl_hdr *ah;
+	struct hci_sco_hdr *sh;
+	int len, type, dlen;
+
+
+	BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
+	       ath->rx_state, ath->rx_count);
+
+	while (count) {
+		if (ath->rx_count) {
+
+			len = min_t(unsigned int, ath->rx_count, count);
+			memcpy(skb_put(ath->rx_skb, len), ptr, len);
+			ath->rx_count -= len;
+			count -= len;
+			ptr += len;
+
+			if (ath->rx_count)
+				continue;
+			switch (ath->rx_state) {
+			case HCIATH_W4_DATA:
+				hci_recv_frame(ath->rx_skb);
+				ath->rx_state = HCIATH_W4_PACKET_TYPE;
+				ath->rx_skb = NULL;
+				ath->rx_count = 0;
+				continue;
+
+			case HCIATH_W4_EVENT_HDR:
+				eh = (struct hci_event_hdr *)ath->rx_skb->data;
+
+				BT_DBG("Event header: evt 0x%2.2x plen %d",
+				       eh->evt, eh->plen);
+
+				ath_check_data_len(ath, eh->plen);
+				continue;
+
+			case HCIATH_W4_ACL_HDR:
+				ah = (struct hci_acl_hdr *)ath->rx_skb->data;
+				dlen = __le16_to_cpu(ah->dlen);
+
+				BT_DBG("ACL header: dlen %d", dlen);
+
+				ath_check_data_len(ath, dlen);
+				continue;
+
+			case HCIATH_W4_SCO_HDR:
+				sh = (struct hci_sco_hdr *)ath->rx_skb->data;
+
+				BT_DBG("SCO header: dlen %d", sh->dlen);
+
+				ath_check_data_len(ath, sh->dlen);
+				continue;
+
+			}
+		}
+
+		/* HCIATH_W4_PACKET_TYPE */
+		switch (*ptr) {
+		case HCI_EVENT_PKT:
+			BT_DBG("Event packet");
+			ath->rx_state = HCIATH_W4_EVENT_HDR;
+			ath->rx_count = HCI_EVENT_HDR_SIZE;
+			type = HCI_EVENT_PKT;
+			break;
+
+		case HCI_ACLDATA_PKT:
+			BT_DBG("ACL packet");
+			ath->rx_state = HCIATH_W4_ACL_HDR;
+			ath->rx_count = HCI_ACL_HDR_SIZE;
+			type = HCI_ACLDATA_PKT;
+			break;
+
+		case HCI_SCODATA_PKT:
+			BT_DBG("SCO packet");
+			ath->rx_state = HCIATH_W4_SCO_HDR;
+			ath->rx_count = HCI_SCO_HDR_SIZE;
+			type = HCI_SCODATA_PKT;
+			break;
+
+		default:
+			BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
+			hu->hdev->stat.err_rx++;
+			ptr++;
+			count--;
+			continue;
+
+		};
+		ptr++;
+		count--;
+
+		/* Allocate packet */
+		ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
+		if (!ath->rx_skb) {
+			BT_ERR("Can't allocate mem for new packet");
+			ath->rx_state = HCIATH_W4_PACKET_TYPE;
+			ath->rx_count = 0;
+
+			return -ENOMEM;
+		}
+		ath->rx_skb->dev = (void *)hu->hdev;
+		bt_cb(ath->rx_skb)->pkt_type = type;
+	}
+
+	return count;
+}
+
+static struct hci_uart_proto athp = {
+	.id = HCI_UART_ATH,
+	.open = ath_open,
+	.close = ath_close,
+	.recv = ath_recv,
+	.enqueue = ath_enqueue,
+	.dequeue = ath_dequeue,
+	.flush = ath_flush,
+};
+
+int ath_init(void)
+{
+	int err = hci_uart_register_proto(&athp);
+
+	if (!err)
+		BT_INFO("HCIATH protocol initialized");
+	else
+		BT_ERR("HCIATH protocol registration failed");
+
+	return err;
+}
+
+int ath_deinit(void)
+{
+	return hci_uart_unregister_proto(&athp);
+}
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 76a1abb..7dd76d1 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
 #ifdef CONFIG_BT_HCIUART_LL
 	ll_init();
 #endif
+#ifdef CONFIG_BT_HCIUART_ATH
+	ath_init();
+#endif
 
 	return 0;
 }
@@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
 #ifdef CONFIG_BT_HCIUART_LL
 	ll_deinit();
 #endif
+#ifdef CONFIG_BT_HCIUART_ATH
+	ath_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 50113db..385537f 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -33,13 +33,14 @@
 #define HCIUARTGETDEVICE	_IOR('U', 202, int)
 
 /* UART protocols */
-#define HCI_UART_MAX_PROTO	5
+#define HCI_UART_MAX_PROTO	6
 
 #define HCI_UART_H4	0
 #define HCI_UART_BCSP	1
 #define HCI_UART_3WIRE	2
 #define HCI_UART_H4DS	3
 #define HCI_UART_LL	4
+#define HCI_UART_ATH	5
 
 struct hci_uart;
 
@@ -91,3 +92,8 @@ int bcsp_deinit(void);
 int ll_init(void);
 int ll_deinit(void);
 #endif
+
+#ifdef CONFIG_BT_HCIUART_ATH
+int ath_init(void);
+int ath_deinit(void);
+#endif
-- 
1.7.0

^ permalink raw reply related

* Re: [PATCH] Added support for Atheros AR300x Bluetooth Chip
From: Gustavo F. Padovan @ 2010-04-19 23:53 UTC (permalink / raw)
  To: suraj; +Cc: linux-bluetooth, marcel, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1268629296.21425.23.camel@atheros013-desktop>

Hi Suraj,

* suraj <suraj@atheros.com> [2010-03-15 10:31:36 +0530]:

> 
> This protocol implements support for power management feature provided by AR300x chip.
> This lets the controller chip go to sleep mode if there is no Bluetooth activity for some time.
> It then wakes up the chip in case of a Bluetooth activity.
> 
> 
>  Signed-off-by: Suraj <suraj@atheros.com>
> 
> ---
>  drivers/bluetooth/Kconfig     |   11 ++
>  drivers/bluetooth/Makefile    |    1 +
>  drivers/bluetooth/hci_ath.c   |  353 +++++++++++++++++++++++++++++++++++++++++
>  drivers/bluetooth/hci_ldisc.c |    6 +
>  drivers/bluetooth/hci_uart.h  |    8 +-
>  5 files changed, 378 insertions(+), 1 deletions(-)
>  create mode 100755 drivers/bluetooth/hci_ath.c
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 058fbcc..81abeff 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -58,6 +58,17 @@ config BT_HCIUART_BCSP
>  
>  	  Say Y here to compile support for HCI BCSP protocol.
>  
> +config BT_HCIUART_ATH
> +	bool "Atheros AR300x Board support"
> +	depends on BT_HCIUART
> +	help
> +	  HCIATH (HCI Atheros) is a serial protocol for communication
> +	  between Bluetooth device and host with support for Atheros AR300x
> +	  power management feature. This protocol is required for
> +	  serial Bluetooth devices that are based on Atheros AR300x chips.
> +
> +	  Say Y here to compile support for HCIATH protocol.
> +
>  config BT_HCIUART_LL
>  	bool "HCILL protocol support"
>  	depends on BT_HCIUART
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 7e5aed5..1481faa 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -26,4 +26,5 @@ hci_uart-y				:= hci_ldisc.o
>  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_ATH)	+= hci_ath.o
>  hci_uart-objs				:= $(hci_uart-y)
> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
> new file mode 100755
> index 0000000..13e4404
> --- /dev/null
> +++ b/drivers/bluetooth/hci_ath.c
> @@ -0,0 +1,353 @@
> +/*
> + * Copyright (c) 2009-2010 Atheros Communications Inc.
> + *
> + *  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/module.h>
> +#include <linux/kernel.h>
> +
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/errno.h>
> +#include <linux/ioctl.h>
> +#include <linux/skbuff.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include "hci_uart.h"
> +
> +
> +/* HCIATH receiver States */
> +#define HCIATH_W4_PACKET_TYPE			0
> +#define HCIATH_W4_EVENT_HDR			1
> +#define HCIATH_W4_ACL_HDR			2
> +#define HCIATH_W4_SCO_HDR			3
> +#define HCIATH_W4_DATA				4
> +
> +struct ath_struct {
> +	struct hci_uart *hu;
> +	unsigned int rx_state;
> +	unsigned int rx_count;
> +	unsigned int cur_sleep;
> +
> +	spinlock_t hciath_lock;
> +	struct sk_buff *rx_skb;
> +	struct sk_buff_head txq;
> +	wait_queue_head_t wqevt;
> +	struct work_struct ctxtsw;
> +};
> +
> +int ath_wakeup_ar3001(struct tty_struct *tty)
> +{
> +	struct termios settings;
> +	int status = 0x00;
> +	mm_segment_t oldfs;
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	if ((status & TIOCM_CTS))
> +		return status;
> +
> +	oldfs = get_fs();
> +	set_fs(KERNEL_DS);
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	settings.c_cflag &= ~CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +	set_fs(oldfs);
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	/* Wake up board */
> +	tty->driver->ops->tiocmset(tty, NULL, 0x00, TIOCM_RTS);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +
> +	tty->driver->ops->tiocmset(tty, NULL, TIOCM_RTS, 0x00);
> +	mdelay(20);
> +
> +	status = tty->driver->ops->tiocmget(tty, NULL);
> +	oldfs = get_fs();
> +	set_fs(KERNEL_DS);
> +	n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
> +
> +	settings.c_cflag |= CRTSCTS;
> +	n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
> +	set_fs(oldfs);
> +	return status;
> +}
> +
> +static void ath_context_switch(struct work_struct *work)
> +{
> +	int status;
> +	struct ath_struct *ath;
> +	struct hci_uart *hu;
> +	struct tty_struct *tty;
> +
> +	ath = container_of(work, struct ath_struct, ctxtsw);
> +
> +	hu = ath->hu;
> +	tty = hu->tty;
> +
> +	/* verify and wake up controller */
> +	if (ath->cur_sleep) {
> +

Extra new line here.

> +		status = ath_wakeup_ar3001(tty);
> +		if (!(status & TIOCM_CTS))
> +			return;
> +	}
> +
> +	/* Ready to send Data */
> +	clear_bit(HCI_UART_SENDING, &hu->tx_state);
> +	hci_uart_tx_wakeup(hu);
> +}
> +
> +int ath_check_sleep_cmd(struct ath_struct *ath, unsigned char *packet)
> +{
> +	if (packet[0] == 0x04 && packet[1] == 0xFC)
> +		ath->cur_sleep = packet[3];
> +
> +	return 0;
> +}
> +
> +
> +/* Initialize protocol */
> +static int ath_open(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath;
> +	BT_DBG("hu %p", hu);
> +
> +	ath = kzalloc(sizeof(*ath), GFP_ATOMIC);
> +	if (!ath)
> +		return -ENOMEM;
> +
> +	skb_queue_head_init(&ath->txq);
> +	spin_lock_init(&ath->hciath_lock);
> +
> +	ath->cur_sleep = 0;

You've used kzalloc, so cur_sleep is already 0

> +	hu->priv = ath;
> +	ath->hu = hu;
> +
> +	init_waitqueue_head(&ath->wqevt);
> +	INIT_WORK(&ath->ctxtsw, ath_context_switch);
> +	return 0;
> +}
> +
> +/* Flush protocol data */
> +static int ath_flush(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	BT_DBG("hu %p", hu);
> +	skb_queue_purge(&ath->txq);
> +
> +	return 0;
> +}
> +
> +/* Close protocol */
> +static int ath_close(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;

Add a extra line before the BT_DBG()

> +	BT_DBG("hu %p", hu);
> +
> +	skb_queue_purge(&ath->txq);
> +
> +	if (ath->rx_skb)
> +		kfree_skb(ath->rx_skb);
> +
> +	wake_up_interruptible(&ath->wqevt);
> +	hu->priv = NULL;
> +	kfree(ath);
> +	return 0;
> +}
> +
> +/* Enqueue frame for transmittion */
> +static int ath_enqueue(struct hci_uart *hu, struct sk_buff *skb)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	if (bt_cb(skb)->pkt_type == HCI_SCODATA_PKT) {
> +
> +		/* Discard SCO packet.AR3001 does not support SCO over HCI */
> +		BT_DBG("SCO Packet over HCI received Dropping\n");
> +		kfree(skb);
> +		return 0;
> +	}
> +	BT_DBG("hu %p skb %p", hu, skb);

Add a extra line before the BT_DBG()

> +
> +	/* Prepend skb with frame type */
> +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> +	skb_queue_tail(&ath->txq, skb);
> +	set_bit(HCI_UART_SENDING, &hu->tx_state);
> +
> +	schedule_work(&ath->ctxtsw);
> +	return 0;
> +}
> +
> +static struct sk_buff *ath_dequeue(struct hci_uart *hu)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	struct sk_buff *skbuf;
> +
> +	skbuf = skb_dequeue(&ath->txq);
> +	if (skbuf != NULL)
> +		ath_check_sleep_cmd(ath, &skbuf->data[1]);
> +
> +	return skbuf;
> +}
> +
> +static inline int ath_check_data_len(struct ath_struct *ath, int len)
> +{
> +	register int room = skb_tailroom(ath->rx_skb);
> +	BT_DBG("len %d room %d", len, room);
> +
> +	if (len > room) {
> +		BT_ERR("Data length is too large");
> +		kfree_skb(ath->rx_skb);
> +		ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +		ath->rx_skb = NULL;
> +		ath->rx_count = 0;
> +	} else {
> +		ath->rx_state = HCIATH_W4_DATA;
> +		ath->rx_count = len;
> +		return len;
> +	}
> +
> +	return 0;

You can return ath->rx_count here and remove the return from the else.
Or ad the return 0 on the if (len > room).

> +}
> +
> +/* Recv data */
> +static int ath_recv(struct hci_uart *hu, void *data, int count)
> +{
> +	struct ath_struct *ath = hu->priv;
> +	register char *ptr;
> +	struct hci_event_hdr *eh;
> +	struct hci_acl_hdr *ah;
> +	struct hci_sco_hdr *sh;
> +	struct sk_buff *skbuf;
> +	register int len, type, dlen;

Wouldn't be better let the compiler choose which vars it will put on the
registers?

> +
> +	skbuf = NULL;

You never use skbuf after here.

> +	BT_DBG("hu %p count %d rx_state %d rx_count %d", hu, count,
> +	       ath->rx_state, ath->rx_count);
> +	ptr = data;

You can set ptr to data when you declare it.

> +	while (count) {
> +		if (ath->rx_count) {
> +
> +			len = min_t(unsigned int, ath->rx_count, count);
> +			memcpy(skb_put(ath->rx_skb, len), ptr, len);
> +			ath->rx_count -= len;
> +			count -= len;
> +			ptr += len;
> +
> +			if (ath->rx_count)
> +				continue;
> +			switch (ath->rx_state) {
> +			case HCIATH_W4_DATA:
> +				hci_recv_frame(ath->rx_skb);
> +				ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +				ath->rx_skb = NULL;
> +				ath->rx_count = 0;
> +				continue;
> +			case HCIATH_W4_EVENT_HDR:
> +				eh = (struct hci_event_hdr *)ath->rx_skb->data;
> +				BT_DBG("Event header: evt 0x%2.2x plen %d",
> +				       eh->evt, eh->plen);
> +				ath_check_data_len(ath, eh->plen);
> +				continue;
> +			case HCIATH_W4_ACL_HDR:
> +				ah = (struct hci_acl_hdr *)ath->rx_skb->data;
> +				dlen = __le16_to_cpu(ah->dlen);
> +				BT_DBG("ACL header: dlen %d", dlen);
> +				ath_check_data_len(ath, dlen);
> +				continue;
> +			case HCIATH_W4_SCO_HDR:
> +				sh = (struct hci_sco_hdr *)ath->rx_skb->data;
> +				BT_DBG("SCO header: dlen %d", sh->dlen);
> +				ath_check_data_len(ath, sh->dlen);
> +				continue;
> +			}
> +		}
> +
> +		/* HCIATH_W4_PACKET_TYPE */
> +		switch (*ptr) {
> +		case HCI_EVENT_PKT:
> +			BT_DBG("Event packet");
> +			ath->rx_state = HCIATH_W4_EVENT_HDR;
> +			ath->rx_count = HCI_EVENT_HDR_SIZE;
> +			type = HCI_EVENT_PKT;
> +			break;
> +		case HCI_ACLDATA_PKT:
> +			BT_DBG("ACL packet");
> +			ath->rx_state = HCIATH_W4_ACL_HDR;
> +			ath->rx_count = HCI_ACL_HDR_SIZE;
> +			type = HCI_ACLDATA_PKT;
> +			break;
> +		case HCI_SCODATA_PKT:
> +			BT_DBG("SCO packet");
> +			ath->rx_state = HCIATH_W4_SCO_HDR;
> +			ath->rx_count = HCI_SCO_HDR_SIZE;
> +			type = HCI_SCODATA_PKT;
> +			break;
> +		default:
> +			BT_ERR("Unknown HCI packet type %2.2x", (__u8) *ptr);
> +			hu->hdev->stat.err_rx++;
> +			ptr++;
> +			count--;
> +			continue;
> +		};
> +		ptr++;
> +		count--;
> +
> +		/* Allocate packet */
> +		ath->rx_skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> +		if (!ath->rx_skb) {
> +			BT_ERR("Can't allocate mem for new packet");
> +			ath->rx_state = HCIATH_W4_PACKET_TYPE;
> +			ath->rx_count = 0;
> +			return -ENOMEM;
> +		}
> +		ath->rx_skb->dev = (void *)hu->hdev;
> +		bt_cb(ath->rx_skb)->pkt_type = type;
> +	} return count;
> +}
> +
> +static struct hci_uart_proto athp = {
> +	.id = HCI_UART_ATH,
> +	.open = ath_open,
> +	.close = ath_close,
> +	.recv = ath_recv,
> +	.enqueue = ath_enqueue,
> +	.dequeue = ath_dequeue,
> +	.flush = ath_flush,
> +};
> +
> +int ath_init(void)
> +{
> +	int err = hci_uart_register_proto(&athp);
> +	if (!err)
> +		BT_INFO("HCIATH protocol initialized");
> +
> +	else
> +		BT_ERR("HCIATH protocol registration failed");

Add a new line here.

> +	return err;
> +}
> +
> +int ath_deinit(void)
> +{
> +	return hci_uart_unregister_proto(&athp);
> +}
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 76a1abb..7dd76d1 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -542,6 +542,9 @@ static int __init hci_uart_init(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_init();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_init();
> +#endif
>  
>  	return 0;
>  }
> @@ -559,6 +562,9 @@ static void __exit hci_uart_exit(void)
>  #ifdef CONFIG_BT_HCIUART_LL
>  	ll_deinit();
>  #endif
> +#ifdef CONFIG_BT_HCIUART_ATH
> +	ath_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 50113db..385537f 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -33,13 +33,14 @@
>  #define HCIUARTGETDEVICE	_IOR('U', 202, int)
>  
>  /* UART protocols */
> -#define HCI_UART_MAX_PROTO	5
> +#define HCI_UART_MAX_PROTO	6
>  
>  #define HCI_UART_H4	0
>  #define HCI_UART_BCSP	1
>  #define HCI_UART_3WIRE	2
>  #define HCI_UART_H4DS	3
>  #define HCI_UART_LL	4
> +#define HCI_UART_ATH	5
>  
>  struct hci_uart;
>  
> @@ -91,3 +92,8 @@ int bcsp_deinit(void);
>  int ll_init(void);
>  int ll_deinit(void);
>  #endif
> +
> +#ifdef CONFIG_BT_HCIUART_ATH
> +int ath_init(void);
> +int ath_deinit(void);
> +#endif
> -- 
> 1.6.3.3
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Gustavo F. Padovan
http://padovan.org

^ permalink raw reply

* RE: A strange compatible problem for eSCO audio with CSR USB Bluetooth dongle
From: Cooper Xu @ 2010-04-19 22:03 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1268707083.2700.14.camel@localhost.localdomain>


Hi,

I was written to this list one month ago about a strange compatible problem
between Bluetooth dongle of CSR chipset with HCI Rev: 0xc5c and Bluez. The
phenomena were that the eSCO connection can be made without any problem, but
I received all zero for voice data. 

After some tests with several USB Dongles of CSR chipset, I found that if I
turned off eSCO, I am able to get real voice data. 

The strange thing is that I used a host with Bluez and CSR dongle to test
the eSCO connection with Blackberry 8310 curve, which Blackberry has the CSR
chipset and HCI Rev: 0xc5c, the voice data worked without problem. However
if I use two hosts with CSR dongles and Bluez at the both ends, the eSCO
connection keeps receiving zero for voice data. This problem exists with all
CSR USB dongles I tried including a Bluetooth 2.1 dongle. If I turn off eSCO
mode, this problem will go away.

Does any one have clue why the eSCO connection will receive all zero if the
both side of eSCO connection are using Bluez and CSR chipset? How can eSCO
connection not compatible between USB dongles with the same CSR chipset and
firmware?


Thanks,
Cooper


^ permalink raw reply


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