[PATCH 0/2] Bluetooth driver for TI WiLink 7

[PATCH 0/2] Bluetooth driver for TI WiLink 7

[pavan-savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

Marcel/Johan and the linux-bluetooth list,

I have begun cleaning up the code for the Bluetooth drivers for
Texas Instrument's WiLink 7 chips.
This driver works on top the Texas Instrument's shared transport line
discipline driver.

So, apart from the transport part of it, the driver is like other regular
bluetooth drivers.
These patches is to move it out of staging/ into the drivers/bluetooth/

Please review and provide comments.

Pavan Savoy (2):
  drivers:bluetooth: TI_ST bluetooth driver
  drivers:bluetooth: Kconfig & Makefile for TI BT

 drivers/bluetooth/Kconfig      |   11 +
 drivers/bluetooth/Makefile     |    1 +
 drivers/bluetooth/bt_ti.c      |  464 ++++++++++++++++++++++++++++++++++++
 drivers/staging/ti-st/Kconfig  |   14 -
 drivers/staging/ti-st/Makefile |    5 -
 drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
 drivers/staging/ti-st/bt_drv.h |   61 -----
 7 files changed, 476 insertions(+), 589 deletions(-)
 create mode 100644 drivers/bluetooth/bt_ti.c
 delete mode 100644 drivers/staging/ti-st/Kconfig
 delete mode 100644 drivers/staging/ti-st/Makefile
 delete mode 100644 drivers/staging/ti-st/bt_drv.c
 delete mode 100644 drivers/staging/ti-st/bt_drv.h

[PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[pavan-savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

This is the bluetooth protocol driver for the TI WiLink7 chipsets.
Texas Instrument's WiLink chipsets combine wireless technologies
like BT, FM, GPS and WLAN onto a single chip.

This Bluetooth driver works on top of the TI_ST shared transport
line discipline driver which also allows other drivers like
FM V4L2 and GPS character driver to make use of the same UART interface.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
 drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
 drivers/staging/ti-st/bt_drv.h |   61 -----
 3 files changed, 463 insertions(+), 570 deletions(-)
 create mode 100644 drivers/bluetooth/bt_ti.c
 delete mode 100644 drivers/staging/ti-st/bt_drv.c
 delete mode 100644 drivers/staging/ti-st/bt_drv.h

diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
new file mode 100644
index 0000000..4f3d3aa
--- /dev/null
+++ b/drivers/bluetooth/bt_ti.c
@@ -0,0 +1,463 @@
+/*
+ *  Texas Instrument's Bluetooth Driver For Shared Transport.
+ *
+ *  Bluetooth Driver acts as interface between HCI CORE and
+ *  TI Shared Transport Layer.
+ *
+ *  Copyright (C) 2009-2010 Texas Instruments
+ *  Author: Raja Mani <raja_mani@ti.com>
+ *	Pavan Savoy <pavan_savoy@ti.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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
+ *
+ */
+
+#define pr_fmt(fmt)	"(tibt): " fmt
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include <linux/ti_wilink_st.h>
+
+/* Bluetooth Driver Version */
+#define VERSION               "1.0"
+
+/* Defines number of seconds to wait for reg completion
+ * callback getting called from ST (in case,registration
+ * with ST returns PENDING status)
+ */
+#define BT_REGISTER_TIMEOUT   6000	/* 6 sec */
+
+/* BT driver's local status */
+#define BT_DRV_RUNNING        0
+#define BT_ST_REGISTERED      1
+
+/**
+ * struct hci_st - BT driver operation structure
+ * @hdev: hci device pointer which binds to bt driver
+ * @flags: used locally,to maintain various BT driver status
+ * @streg_cbdata: to hold ST registration callback status
+ * @st_write: write function pointer of ST driver
+ * @wait_for_btdrv_reg_completion - completion sync between hci_st_open
+ *	and hci_st_registration_completion_cb.
+ */
+struct hci_st {
+	struct hci_dev *hdev;
+	unsigned long flags;
+	char streg_cbdata;
+	long (*st_write) (struct sk_buff *);
+	struct completion wait_for_btdrv_reg_completion;
+};
+
+static struct hci_st *hst;
+static int reset;
+
+/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
+static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type)
+{
+	struct hci_dev *hdev;
+	hdev = hst->hdev;
+
+	/* Update HCI stat counters */
+	switch (pkt_type) {
+	case HCI_COMMAND_PKT:
+		hdev->stat.cmd_tx++;
+		break;
+
+	case HCI_ACLDATA_PKT:
+		hdev->stat.acl_tx++;
+		break;
+
+	case HCI_SCODATA_PKT:
+		hdev->stat.cmd_tx++;
+		break;
+	}
+}
+
+/* ------- Interfaces to Shared Transport ------ */
+
+/* Called by ST layer to indicate protocol registration completion
+ * status.hci_st_open() function will wait for signal from this
+ * API when st_register() function returns ST_PENDING.
+ */
+static void hci_st_registration_completion_cb(void *priv_data, char data)
+{
+	struct hci_st *lhst = (struct hci_st *)priv_data;
+	/* hci_st_open() function needs value of 'data' to know
+	 * the registration status(success/fail),So have a back
+	 * up of it.
+	 */
+	lhst->streg_cbdata = data;
+
+	/* Got a feedback from ST for BT driver registration
+	 * request.Wackup hci_st_open() function to continue
+	 * it's open operation.
+	 */
+	complete(&lhst->wait_for_btdrv_reg_completion);
+}
+
+/* Called by Shared Transport layer when receive data is
+ * available */
+static long hci_st_receive(void *priv_data, struct sk_buff *skb)
+{
+	int err;
+	int len;
+	struct hci_st *lhst = (struct hci_st *)priv_data;
+
+	err = 0;
+	len = 0;
+
+	if (skb == NULL) {
+		pr_err("Invalid SKB received from ST");
+		return -EFAULT;
+	}
+	if (!lhst) {
+		kfree_skb(skb);
+		pr_err("Invalid hci_st memory,freeing SKB");
+		return -EFAULT;
+	}
+	if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
+		kfree_skb(skb);
+		pr_err("Device is not running,freeing SKB");
+		return -EINVAL;
+	}
+
+	len = skb->len;
+	skb->dev = (struct net_device *)lhst->hdev;
+
+	/* Forward skb to HCI CORE layer */
+	err = hci_recv_frame(skb);
+	if (err) {
+		kfree_skb(skb);
+		pr_err("Unable to push skb to HCI CORE(%d),freeing SKB",
+				err);
+		return err;
+	}
+	lhst->hdev->stat.byte_rx += len;
+
+	return 0;
+}
+
+/* ------- Interfaces to HCI layer ------ */
+
+/* Called from HCI core to initialize the device */
+static int hci_st_open(struct hci_dev *hdev)
+{
+	static struct st_proto_s hci_st_proto;
+	unsigned long timeleft;
+	int err;
+	err = 0;
+
+	pr_debug("%s %p", hdev->name, hdev);
+
+	/* Already registered with ST ? */
+	if (test_bit(BT_ST_REGISTERED, &hst->flags)) {
+		pr_err("Registered with ST already,open called again?");
+		return 0;
+	}
+
+	/* Populate BT driver info required by ST */
+	memset(&hci_st_proto, 0, sizeof(hci_st_proto));
+
+	/* BT driver ID */
+	hci_st_proto.type = ST_BT;
+
+	/* Receive function which called from ST */
+	hci_st_proto.recv = hci_st_receive;
+
+	/* Packet match function may used in future */
+	hci_st_proto.match_packet = NULL;
+
+	/* Callback to be called when registration is pending */
+	hci_st_proto.reg_complete_cb = hci_st_registration_completion_cb;
+
+	/* This is write function pointer of ST. BT driver will make use of this
+	 * for sending any packets to chip. ST will assign and give to us, so
+	 * make it as NULL */
+	hci_st_proto.write = NULL;
+
+	/* send in the hst to be received at registration complete callback
+	 * and during st's receive
+	 */
+	hci_st_proto.priv_data = hst;
+
+	/* Register with ST layer */
+	err = st_register(&hci_st_proto);
+	if (err == -EINPROGRESS) {
+		/* Prepare wait-for-completion handler data structures.
+		 * Needed to syncronize this and st_registration_completion_cb()
+		 * functions.
+		 */
+		init_completion(&hst->wait_for_btdrv_reg_completion);
+
+		/* Reset ST registration callback status flag , this value
+		 * will be updated in hci_st_registration_completion_cb()
+		 * function whenever it called from ST driver.
+		 */
+		hst->streg_cbdata = -EINPROGRESS;
+
+		/* ST is busy with other protocol registration(may be busy with
+		 * firmware download).So,Wait till the registration callback
+		 * (passed as a argument to st_register() function) getting
+		 * called from ST.
+		 */
+		pr_debug(" %s waiting for reg completion signal from ST",
+				__func__);
+
+		timeleft =
+			wait_for_completion_timeout
+			(&hst->wait_for_btdrv_reg_completion,
+			 msecs_to_jiffies(BT_REGISTER_TIMEOUT));
+		if (!timeleft) {
+			pr_err("Timeout(%d sec),didn't get reg"
+					"completion signal from ST",
+					BT_REGISTER_TIMEOUT / 1000);
+			return -ETIMEDOUT;
+		}
+
+		/* Is ST registration callback called with ERROR value? */
+		if (hst->streg_cbdata != 0) {
+			pr_err("ST reg completion CB called with invalid"
+					"status %d", hst->streg_cbdata);
+			return -EAGAIN;
+		}
+		err = 0;
+	} else if (err == -1) {
+		pr_err("st_register failed %d", err);
+		return -EAGAIN;
+	}
+
+	/* Do we have proper ST write function? */
+	if (hci_st_proto.write != NULL) {
+		/* We need this pointer for sending any Bluetooth pkts */
+		hst->st_write = hci_st_proto.write;
+	} else {
+		pr_err("failed to get ST write func pointer");
+
+		/* Undo registration with ST */
+		err = st_unregister(ST_BT);
+		if (err < 0)
+			pr_err("st_unregister failed %d", err);
+
+		hst->st_write = NULL;
+		return -EAGAIN;
+	}
+
+	/* Registration with ST layer is completed successfully,
+	 * now chip is ready to accept commands from HCI CORE.
+	 * Mark HCI Device flag as RUNNING
+	 */
+	set_bit(HCI_RUNNING, &hdev->flags);
+
+	/* Registration with ST successful */
+	set_bit(BT_ST_REGISTERED, &hst->flags);
+
+	return err;
+}
+
+/* Close device */
+static int hci_st_close(struct hci_dev *hdev)
+{
+	int err;
+	err = 0;
+
+	/* Unregister from ST layer */
+	if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
+		err = st_unregister(ST_BT);
+		if (err != 0) {
+			pr_err("st_unregister failed %d", err);
+			return -EBUSY;
+		}
+	}
+
+	hst->st_write = NULL;
+
+	/* ST layer would have moved chip to inactive state.
+	 * So,clear HCI device RUNNING flag.
+	 */
+	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
+		return 0;
+
+	return err;
+}
+
+/* Called from HCI CORE , Sends frames to Shared Transport */
+static int hci_st_send_frame(struct sk_buff *skb)
+{
+	struct hci_dev *hdev;
+	struct hci_st *hst;
+	long len;
+
+	if (skb == NULL) {
+		pr_err("Invalid skb received from HCI CORE");
+		return -ENOMEM;
+	}
+	hdev = (struct hci_dev *)skb->dev;
+	if (!hdev) {
+		pr_err("SKB received for invalid HCI Device (hdev=NULL)");
+		return -ENODEV;
+	}
+	if (!test_bit(HCI_RUNNING, &hdev->flags)) {
+		pr_err("Device is not running");
+		return -EBUSY;
+	}
+
+	hst = (struct hci_st *)hdev->driver_data;
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	pr_debug(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
+			skb->len);
+
+	/* Insert skb to shared transport layer's transmit queue.
+	 * Freeing skb memory is taken care in shared transport layer,
+	 * so don't free skb memory here.
+	 */
+	if (!hst->st_write) {
+		kfree_skb(skb);
+		pr_err(" Can't write to ST, st_write null?");
+		return -EAGAIN;
+	}
+	len = hst->st_write(skb);
+	if (len < 0) {
+		/* Something went wrong in st write , free skb memory */
+		kfree_skb(skb);
+		pr_err(" ST write failed (%ld)", len);
+		return -EAGAIN;
+	}
+
+	/* ST accepted our skb. So, Go ahead and do rest */
+	hdev->stat.byte_tx += len;
+	hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
+
+	return 0;
+}
+
+static void hci_st_destruct(struct hci_dev *hdev)
+{
+	if (!hdev) {
+		pr_err("Destruct called with invalid HCI Device"
+				"(hdev=NULL)");
+		return;
+	}
+
+	pr_debug("%s", hdev->name);
+
+	/* free hci_st memory */
+	if (hdev->driver_data != NULL)
+		kfree(hdev->driver_data);
+
+	return;
+}
+
+/* Creates new HCI device */
+static int hci_st_register_dev(struct hci_st *hst)
+{
+	struct hci_dev *hdev;
+
+	/* Initialize and register HCI device */
+	hdev = hci_alloc_dev();
+	if (!hdev) {
+		pr_err("Can't allocate HCI device");
+		return -ENOMEM;
+	}
+	pr_debug(" HCI device allocated. hdev= %p", hdev);
+
+	hst->hdev = hdev;
+	hdev->bus = HCI_UART;
+	hdev->driver_data = hst;
+	hdev->open = hci_st_open;
+	hdev->close = hci_st_close;
+	hdev->flush = NULL;
+	hdev->send = hci_st_send_frame;
+	hdev->destruct = hci_st_destruct;
+	hdev->owner = THIS_MODULE;
+
+	if (reset)
+		set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
+
+	if (hci_register_dev(hdev) < 0) {
+		pr_err("Can't register HCI device");
+		hci_free_dev(hdev);
+		return -ENODEV;
+	}
+
+	pr_debug(" HCI device registered. hdev= %p", hdev);
+	return 0;
+}
+
+/* ------- Module Init interface ------ */
+
+static int __init bt_drv_init(void)
+{
+	int err;
+	err = 0;
+
+	pr_debug(" Bluetooth Driver Version %s", VERSION);
+
+	/* Allocate local resource memory */
+	hst = kzalloc(sizeof(struct hci_st), GFP_KERNEL);
+	if (!hst) {
+		pr_err("Can't allocate control structure");
+		return -ENFILE;
+	}
+
+	/* Expose "hciX" device to user space */
+	err = hci_st_register_dev(hst);
+	if (err) {
+		/* Release local resource memory */
+		kfree(hst);
+
+		pr_err("Unable to expose hci0 device(%d)", err);
+		return err;
+	}
+	set_bit(BT_DRV_RUNNING, &hst->flags);
+	return err;
+}
+
+/* ------- Module Exit interface ------ */
+
+static void __exit bt_drv_exit(void)
+{
+	/* Deallocate local resource's memory  */
+	if (hst) {
+		struct hci_dev *hdev = hst->hdev;
+		if (hdev == NULL) {
+			pr_err("Invalid hdev memory");
+			kfree(hst);
+		} else {
+			hci_st_close(hdev);
+			if (test_and_clear_bit(BT_DRV_RUNNING, &hst->flags)) {
+				/* Remove HCI device (hciX) created
+				 * in module init.
+				 */
+				hci_unregister_dev(hdev);
+				/* Free HCI device memory */
+				hci_free_dev(hdev);
+			}
+		}
+	}
+}
+
+module_init(bt_drv_init);
+module_exit(bt_drv_exit);
+
+/* ------ Module Info ------ */
+
+module_param(reset, bool, 0644);
+MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
+MODULE_AUTHOR("Raja Mani <raja_mani@ti.com>");
+MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/ti-st/bt_drv.c b/drivers/staging/ti-st/bt_drv.c
deleted file mode 100644
index 75065bf..0000000
--- a/drivers/staging/ti-st/bt_drv.c
+++ /dev/null
@@ -1,509 +0,0 @@
-/*
- *  Texas Instrument's Bluetooth Driver For Shared Transport.
- *
- *  Bluetooth Driver acts as interface between HCI CORE and
- *  TI Shared Transport Layer.
- *
- *  Copyright (C) 2009 Texas Instruments
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License version 2 as
- *  published by the Free Software Foundation.
- *
- *  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 <net/bluetooth/bluetooth.h>
-#include <net/bluetooth/hci_core.h>
-
-#include <linux/ti_wilink_st.h>
-#include "bt_drv.h"
-
-/* Define this macro to get debug msg */
-#undef DEBUG
-
-#ifdef DEBUG
-#define BT_DRV_DBG(fmt, arg...)  printk(KERN_INFO "(btdrv):"fmt"\n" , ## arg)
-#define BTDRV_API_START()        printk(KERN_INFO "(btdrv): %s Start\n", \
-	__func__)
-#define BTDRV_API_EXIT(errno)    printk(KERN_INFO "(btdrv): %s Exit(%d)\n", \
-	__func__, errno)
-#else
-#define BT_DRV_DBG(fmt, arg...)
-#define BTDRV_API_START()
-#define BTDRV_API_EXIT(errno)
-#endif
-
-#define BT_DRV_ERR(fmt, arg...)  printk(KERN_ERR "(btdrv):"fmt"\n" , ## arg)
-
-static int reset;
-static struct hci_st *hst;
-
-/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
-static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type)
-{
-	struct hci_dev *hdev;
-
-	BTDRV_API_START();
-
-	hdev = hst->hdev;
-
-	/* Update HCI stat counters */
-	switch (pkt_type) {
-	case HCI_COMMAND_PKT:
-		hdev->stat.cmd_tx++;
-		break;
-
-	case HCI_ACLDATA_PKT:
-		hdev->stat.acl_tx++;
-		break;
-
-	case HCI_SCODATA_PKT:
-		hdev->stat.cmd_tx++;
-		break;
-	}
-
-	BTDRV_API_EXIT(0);
-}
-
-/* ------- Interfaces to Shared Transport ------ */
-
-/* Called by ST layer to indicate protocol registration completion
- * status.hci_st_open() function will wait for signal from this
- * API when st_register() function returns ST_PENDING.
- */
-static void hci_st_registration_completion_cb(void *priv_data, char data)
-{
-	struct hci_st *lhst = (struct hci_st *)priv_data;
-	BTDRV_API_START();
-
-	/* hci_st_open() function needs value of 'data' to know
-	 * the registration status(success/fail),So have a back
-	 * up of it.
-	 */
-	lhst->streg_cbdata = data;
-
-	/* Got a feedback from ST for BT driver registration
-	 * request.Wackup hci_st_open() function to continue
-	 * it's open operation.
-	 */
-	complete(&lhst->wait_for_btdrv_reg_completion);
-
-	BTDRV_API_EXIT(0);
-}
-
-/* Called by Shared Transport layer when receive data is
- * available */
-static long hci_st_receive(void *priv_data, struct sk_buff *skb)
-{
-	int err;
-	int len;
-	struct hci_st *lhst = (struct hci_st *)priv_data;
-
-	BTDRV_API_START();
-
-	err = 0;
-	len = 0;
-
-	if (skb == NULL) {
-		BT_DRV_ERR("Invalid SKB received from ST");
-		BTDRV_API_EXIT(-EFAULT);
-		return -EFAULT;
-	}
-	if (!lhst) {
-		kfree_skb(skb);
-		BT_DRV_ERR("Invalid hci_st memory,freeing SKB");
-		BTDRV_API_EXIT(-EFAULT);
-		return -EFAULT;
-	}
-	if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
-		kfree_skb(skb);
-		BT_DRV_ERR("Device is not running,freeing SKB");
-		BTDRV_API_EXIT(-EINVAL);
-		return -EINVAL;
-	}
-
-	len = skb->len;
-	skb->dev = (struct net_device *)lhst->hdev;
-
-	/* Forward skb to HCI CORE layer */
-	err = hci_recv_frame(skb);
-	if (err) {
-		kfree_skb(skb);
-		BT_DRV_ERR("Unable to push skb to HCI CORE(%d),freeing SKB",
-			   err);
-		BTDRV_API_EXIT(err);
-		return err;
-	}
-	lhst->hdev->stat.byte_rx += len;
-
-	BTDRV_API_EXIT(0);
-	return 0;
-}
-
-/* ------- Interfaces to HCI layer ------ */
-
-/* Called from HCI core to initialize the device */
-static int hci_st_open(struct hci_dev *hdev)
-{
-	static struct st_proto_s hci_st_proto;
-	unsigned long timeleft;
-	int err;
-
-	BTDRV_API_START();
-
-	err = 0;
-
-	BT_DRV_DBG("%s %p", hdev->name, hdev);
-
-	/* Already registered with ST ? */
-	if (test_bit(BT_ST_REGISTERED, &hst->flags)) {
-		BT_DRV_ERR("Registered with ST already,open called again?");
-		BTDRV_API_EXIT(0);
-		return 0;
-	}
-
-	/* Populate BT driver info required by ST */
-	memset(&hci_st_proto, 0, sizeof(hci_st_proto));
-
-	/* BT driver ID */
-	hci_st_proto.type = ST_BT;
-
-	/* Receive function which called from ST */
-	hci_st_proto.recv = hci_st_receive;
-
-	/* Packet match function may used in future */
-	hci_st_proto.match_packet = NULL;
-
-	/* Callback to be called when registration is pending */
-	hci_st_proto.reg_complete_cb = hci_st_registration_completion_cb;
-
-	/* This is write function pointer of ST. BT driver will make use of this
-	 * for sending any packets to chip. ST will assign and give to us, so
-	 * make it as NULL */
-	hci_st_proto.write = NULL;
-
-	/* send in the hst to be received at registration complete callback
-	 * and during st's receive
-	 */
-	hci_st_proto.priv_data = hst;
-
-	/* Register with ST layer */
-	err = st_register(&hci_st_proto);
-	if (err == -EINPROGRESS) {
-		/* Prepare wait-for-completion handler data structures.
-		 * Needed to syncronize this and st_registration_completion_cb()
-		 * functions.
-		 */
-		init_completion(&hst->wait_for_btdrv_reg_completion);
-
-		/* Reset ST registration callback status flag , this value
-		 * will be updated in hci_st_registration_completion_cb()
-		 * function whenever it called from ST driver.
-		 */
-		hst->streg_cbdata = -EINPROGRESS;
-
-		/* ST is busy with other protocol registration(may be busy with
-		 * firmware download).So,Wait till the registration callback
-		 * (passed as a argument to st_register() function) getting
-		 * called from ST.
-		 */
-		BT_DRV_DBG(" %s waiting for reg completion signal from ST",
-			   __func__);
-
-		timeleft =
-		    wait_for_completion_timeout
-		    (&hst->wait_for_btdrv_reg_completion,
-		     msecs_to_jiffies(BT_REGISTER_TIMEOUT));
-		if (!timeleft) {
-			BT_DRV_ERR("Timeout(%ld sec),didn't get reg"
-				   "completion signal from ST",
-				   BT_REGISTER_TIMEOUT / 1000);
-			BTDRV_API_EXIT(-ETIMEDOUT);
-			return -ETIMEDOUT;
-		}
-
-		/* Is ST registration callback called with ERROR value? */
-		if (hst->streg_cbdata != 0) {
-			BT_DRV_ERR("ST reg completion CB called with invalid"
-				   "status %d", hst->streg_cbdata);
-			BTDRV_API_EXIT(-EAGAIN);
-			return -EAGAIN;
-		}
-		err = 0;
-	} else if (err == -1) {
-		BT_DRV_ERR("st_register failed %d", err);
-		BTDRV_API_EXIT(-EAGAIN);
-		return -EAGAIN;
-	}
-
-	/* Do we have proper ST write function? */
-	if (hci_st_proto.write != NULL) {
-		/* We need this pointer for sending any Bluetooth pkts */
-		hst->st_write = hci_st_proto.write;
-	} else {
-		BT_DRV_ERR("failed to get ST write func pointer");
-
-		/* Undo registration with ST */
-		err = st_unregister(ST_BT);
-		if (err < 0)
-			BT_DRV_ERR("st_unregister failed %d", err);
-
-		hst->st_write = NULL;
-		BTDRV_API_EXIT(-EAGAIN);
-		return -EAGAIN;
-	}
-
-	/* Registration with ST layer is completed successfully,
-	 * now chip is ready to accept commands from HCI CORE.
-	 * Mark HCI Device flag as RUNNING
-	 */
-	set_bit(HCI_RUNNING, &hdev->flags);
-
-	/* Registration with ST successful */
-	set_bit(BT_ST_REGISTERED, &hst->flags);
-
-	BTDRV_API_EXIT(err);
-	return err;
-}
-
-/* Close device */
-static int hci_st_close(struct hci_dev *hdev)
-{
-	int err;
-
-	BTDRV_API_START();
-
-	err = 0;
-
-	/* Unregister from ST layer */
-	if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
-		err = st_unregister(ST_BT);
-		if (err != 0) {
-			BT_DRV_ERR("st_unregister failed %d", err);
-			BTDRV_API_EXIT(-EBUSY);
-			return -EBUSY;
-		}
-	}
-
-	hst->st_write = NULL;
-
-	/* ST layer would have moved chip to inactive state.
-	 * So,clear HCI device RUNNING flag.
-	 */
-	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags)) {
-		BTDRV_API_EXIT(0);
-		return 0;
-	}
-
-	BTDRV_API_EXIT(err);
-	return err;
-}
-
-/* Called from HCI CORE , Sends frames to Shared Transport */
-static int hci_st_send_frame(struct sk_buff *skb)
-{
-	struct hci_dev *hdev;
-	struct hci_st *hst;
-	long len;
-
-	BTDRV_API_START();
-
-	if (skb == NULL) {
-		BT_DRV_ERR("Invalid skb received from HCI CORE");
-		BTDRV_API_EXIT(-ENOMEM);
-		return -ENOMEM;
-	}
-	hdev = (struct hci_dev *)skb->dev;
-	if (!hdev) {
-		BT_DRV_ERR("SKB received for invalid HCI Device (hdev=NULL)");
-		BTDRV_API_EXIT(-ENODEV);
-		return -ENODEV;
-	}
-	if (!test_bit(HCI_RUNNING, &hdev->flags)) {
-		BT_DRV_ERR("Device is not running");
-		BTDRV_API_EXIT(-EBUSY);
-		return -EBUSY;
-	}
-
-	hst = (struct hci_st *)hdev->driver_data;
-
-	/* Prepend skb with frame type */
-	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
-
-	BT_DRV_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
-		   skb->len);
-
-	/* Insert skb to shared transport layer's transmit queue.
-	 * Freeing skb memory is taken care in shared transport layer,
-	 * so don't free skb memory here.
-	 */
-	if (!hst->st_write) {
-		kfree_skb(skb);
-		BT_DRV_ERR(" Can't write to ST, st_write null?");
-		BTDRV_API_EXIT(-EAGAIN);
-		return -EAGAIN;
-	}
-	len = hst->st_write(skb);
-	if (len < 0) {
-		/* Something went wrong in st write , free skb memory */
-		kfree_skb(skb);
-		BT_DRV_ERR(" ST write failed (%ld)", len);
-		BTDRV_API_EXIT(-EAGAIN);
-		return -EAGAIN;
-	}
-
-	/* ST accepted our skb. So, Go ahead and do rest */
-	hdev->stat.byte_tx += len;
-	hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
-
-	BTDRV_API_EXIT(0);
-	return 0;
-}
-
-static void hci_st_destruct(struct hci_dev *hdev)
-{
-	BTDRV_API_START();
-
-	if (!hdev) {
-		BT_DRV_ERR("Destruct called with invalid HCI Device"
-			   "(hdev=NULL)");
-		BTDRV_API_EXIT(0);
-		return;
-	}
-
-	BT_DRV_DBG("%s", hdev->name);
-
-	/* free hci_st memory */
-	if (hdev->driver_data != NULL)
-		kfree(hdev->driver_data);
-
-	BTDRV_API_EXIT(0);
-	return;
-}
-
-/* Creates new HCI device */
-static int hci_st_register_dev(struct hci_st *hst)
-{
-	struct hci_dev *hdev;
-
-	BTDRV_API_START();
-
-	/* Initialize and register HCI device */
-	hdev = hci_alloc_dev();
-	if (!hdev) {
-		BT_DRV_ERR("Can't allocate HCI device");
-		BTDRV_API_EXIT(-ENOMEM);
-		return -ENOMEM;
-	}
-	BT_DRV_DBG(" HCI device allocated. hdev= %p", hdev);
-
-	hst->hdev = hdev;
-	hdev->bus = HCI_UART;
-	hdev->driver_data = hst;
-	hdev->open = hci_st_open;
-	hdev->close = hci_st_close;
-	hdev->flush = NULL;
-	hdev->send = hci_st_send_frame;
-	hdev->destruct = hci_st_destruct;
-	hdev->owner = THIS_MODULE;
-
-	if (reset)
-		set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
-
-	if (hci_register_dev(hdev) < 0) {
-		BT_DRV_ERR("Can't register HCI device");
-		hci_free_dev(hdev);
-		BTDRV_API_EXIT(-ENODEV);
-		return -ENODEV;
-	}
-
-	BT_DRV_DBG(" HCI device registered. hdev= %p", hdev);
-	BTDRV_API_EXIT(0);
-	return 0;
-}
-
-/* ------- Module Init interface ------ */
-
-static int __init bt_drv_init(void)
-{
-	int err;
-
-	BTDRV_API_START();
-
-	err = 0;
-
-	BT_DRV_DBG(" Bluetooth Driver Version %s", VERSION);
-
-	/* Allocate local resource memory */
-	hst = kzalloc(sizeof(struct hci_st), GFP_KERNEL);
-	if (!hst) {
-		BT_DRV_ERR("Can't allocate control structure");
-		BTDRV_API_EXIT(-ENFILE);
-		return -ENFILE;
-	}
-
-	/* Expose "hciX" device to user space */
-	err = hci_st_register_dev(hst);
-	if (err) {
-		/* Release local resource memory */
-		kfree(hst);
-
-		BT_DRV_ERR("Unable to expose hci0 device(%d)", err);
-		BTDRV_API_EXIT(err);
-		return err;
-	}
-	set_bit(BT_DRV_RUNNING, &hst->flags);
-
-	BTDRV_API_EXIT(err);
-	return err;
-}
-
-/* ------- Module Exit interface ------ */
-
-static void __exit bt_drv_exit(void)
-{
-	BTDRV_API_START();
-
-	/* Deallocate local resource's memory  */
-	if (hst) {
-		struct hci_dev *hdev = hst->hdev;
-
-		if (hdev == NULL) {
-			BT_DRV_ERR("Invalid hdev memory");
-			kfree(hst);
-		} else {
-			hci_st_close(hdev);
-			if (test_and_clear_bit(BT_DRV_RUNNING, &hst->flags)) {
-				/* Remove HCI device (hciX) created
-				 * in module init.
-				 */
-				hci_unregister_dev(hdev);
-
-				/* Free HCI device memory */
-				hci_free_dev(hdev);
-			}
-		}
-	}
-	BTDRV_API_EXIT(0);
-}
-
-module_init(bt_drv_init);
-module_exit(bt_drv_exit);
-
-/* ------ Module Info ------ */
-
-module_param(reset, bool, 0644);
-MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
-MODULE_AUTHOR("Raja Mani <raja_mani@ti.com>");
-MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
-MODULE_VERSION(VERSION);
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/ti-st/bt_drv.h b/drivers/staging/ti-st/bt_drv.h
deleted file mode 100644
index a0beebe..0000000
--- a/drivers/staging/ti-st/bt_drv.h
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- *  Texas Instrument's Bluetooth Driver For Shared Transport.
- *
- *  Bluetooth Driver acts as interface between HCI CORE and
- *  TI Shared Transport Layer.
- *
- *  Copyright (C) 2009 Texas Instruments
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License version 2 as
- *  published by the Free Software Foundation.
- *
- *  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
- *
- */
-
-#ifndef _BT_DRV_H
-#define _BT_DRV_H
-
-/* Bluetooth Driver Version */
-#define VERSION               "1.0"
-
-/* Defines number of seconds to wait for reg completion
- * callback getting called from ST (in case,registration
- * with ST returns PENDING status)
- */
-#define BT_REGISTER_TIMEOUT   msecs_to_jiffies(6000)	/* 6 sec */
-
-/* BT driver's local status */
-#define BT_DRV_RUNNING        0
-#define BT_ST_REGISTERED      1
-
-/* BT driver operation structure */
-struct hci_st {
-
-	/* hci device pointer which binds to bt driver */
-	struct hci_dev *hdev;
-
-	/* used locally,to maintain various BT driver status */
-	unsigned long flags;
-
-	/* to hold ST registration callback  status */
-	char streg_cbdata;
-
-	/* write function pointer of ST driver */
-	long (*st_write) (struct sk_buff *);
-
-	/* Wait on comepletion handler needed to synchronize
-	 * hci_st_open() and hci_st_registration_completion_cb()
-	 * functions.*/
-	struct completion wait_for_btdrv_reg_completion;
-};
-
-#endif
-- 
1.6.5

[PATCH 2/2] drivers:bluetooth: Kconfig & Makefile for TI BT

[pavan-savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

Kconfig and Makefile modifications to enable the Bluetooth
driver for Texas Instrument's WiLink 7 chipset.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/bluetooth/Kconfig      |   11 +++++++++++
 drivers/bluetooth/Makefile     |    1 +
 drivers/staging/ti-st/Kconfig  |   14 --------------
 drivers/staging/ti-st/Makefile |    5 -----
 4 files changed, 12 insertions(+), 19 deletions(-)
 delete mode 100644 drivers/staging/ti-st/Kconfig
 delete mode 100644 drivers/staging/ti-st/Makefile

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 02deef4..91a18b3 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -219,4 +219,15 @@ config BT_ATH3K
 	  Say Y here to compile support for "Atheros firmware download driver"
 	  into the kernel or say M to compile it as module (ath3k).
 
+config BT_TI
+	tristate "BlueZ bluetooth driver for TI ST"
+	depends on BT && RFKILL
+	select TI_ST
+	help
+	  This enables the Bluetooth driver for Texas Instrument's BT/FM/GPS
+	  combo devices. This makes use of shared transport line discipline
+	  core driver to communicate with the BT core of the combo chip.
+
+	  Say Y here to compile support for Texas Instrument's WiLink7 driver
+	  into the kernel or say M to compile it as module.
 endmenu
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 71bdf13..63156da 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -28,3 +28,4 @@ hci_uart-$(CONFIG_BT_HCIUART_BCSP)	+= hci_bcsp.o
 hci_uart-$(CONFIG_BT_HCIUART_LL)	+= hci_ll.o
 hci_uart-$(CONFIG_BT_HCIUART_ATH3K)	+= hci_ath.o
 hci_uart-objs				:= $(hci_uart-y)
+obj-$(CONFIG_BT_TI)			:= bt_ti.o
diff --git a/drivers/staging/ti-st/Kconfig b/drivers/staging/ti-st/Kconfig
deleted file mode 100644
index 074b8e8..0000000
--- a/drivers/staging/ti-st/Kconfig
+++ /dev/null
@@ -1,14 +0,0 @@
-#
-# TI's shared transport line discipline and the protocol
-# drivers (BT, FM and GPS)
-#
-menu "Texas Instruments shared transport line discipline"
-config ST_BT
-	tristate "BlueZ bluetooth driver for ST"
-	depends on BT && RFKILL
-	select TI_ST
-	help
-	  This enables the Bluetooth driver for TI BT/FM/GPS combo devices.
-	  This makes use of shared transport line discipline core driver to
-	  communicate with the BT core of the combo chip.
-endmenu
diff --git a/drivers/staging/ti-st/Makefile b/drivers/staging/ti-st/Makefile
deleted file mode 100644
index 5f11b82..0000000
--- a/drivers/staging/ti-st/Makefile
+++ /dev/null
@@ -1,5 +0,0 @@
-#
-# Makefile for TI's shared transport line discipline
-# and its protocol drivers (BT, FM, GPS)
-#
-obj-$(CONFIG_ST_BT) 		+= bt_drv.o
-- 
1.6.5

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Marcel Holtmann]

Hi Pavan,

> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> Texas Instrument's WiLink chipsets combine wireless technologies
> like BT, FM, GPS and WLAN onto a single chip.
> 
> This Bluetooth driver works on top of the TI_ST shared transport
> line discipline driver which also allows other drivers like
> FM V4L2 and GPS character driver to make use of the same UART interface.
> 
> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> ---
>  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
>  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
>  drivers/staging/ti-st/bt_drv.h |   61 -----
>  3 files changed, 463 insertions(+), 570 deletions(-)
>  create mode 100644 drivers/bluetooth/bt_ti.c
>  delete mode 100644 drivers/staging/ti-st/bt_drv.c
>  delete mode 100644 drivers/staging/ti-st/bt_drv.h

I don't care about staging at all. So you sort that out with Greg.

Submit your driver for upstream inclusion. And once accepted you can pin
Greg about removing it.

> diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
> new file mode 100644
> index 0000000..4f3d3aa
> --- /dev/null
> +++ b/drivers/bluetooth/bt_ti.c
> @@ -0,0 +1,463 @@
> +/*
> + *  Texas Instrument's Bluetooth Driver For Shared Transport.
> + *
> + *  Bluetooth Driver acts as interface between HCI CORE and
> + *  TI Shared Transport Layer.
> + *
> + *  Copyright (C) 2009-2010 Texas Instruments
> + *  Author: Raja Mani <raja_mani@ti.com>
> + *	Pavan Savoy <pavan_savoy@ti.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  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
> + *
> + */
> +
> +#define pr_fmt(fmt)	"(tibt): " fmt

Don't do this. Just use BT_DBG, BT_ERR, BT_INFO etc.

> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include <linux/ti_wilink_st.h>
> +
> +/* Bluetooth Driver Version */
> +#define VERSION               "1.0"
> +
> +/* Defines number of seconds to wait for reg completion
> + * callback getting called from ST (in case,registration
> + * with ST returns PENDING status)
> + */
> +#define BT_REGISTER_TIMEOUT   6000	/* 6 sec */
> +
> +/* BT driver's local status */
> +#define BT_DRV_RUNNING        0
> +#define BT_ST_REGISTERED      1
> +
> +/**
> + * struct hci_st - BT driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @flags: used locally,to maintain various BT driver status
> + * @streg_cbdata: to hold ST registration callback status
> + * @st_write: write function pointer of ST driver
> + * @wait_for_btdrv_reg_completion - completion sync between hci_st_open
> + *	and hci_st_registration_completion_cb.
> + */
> +struct hci_st {
> +	struct hci_dev *hdev;
> +	unsigned long flags;
> +	char streg_cbdata;
> +	long (*st_write) (struct sk_buff *);
> +	struct completion wait_for_btdrv_reg_completion;
> +};
> +
> +static struct hci_st *hst;
> +static int reset;
> +
> +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> +static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type)
> +{
> +	struct hci_dev *hdev;
> +	hdev = hst->hdev;
> +
> +	/* Update HCI stat counters */
> +	switch (pkt_type) {
> +	case HCI_COMMAND_PKT:
> +		hdev->stat.cmd_tx++;
> +		break;
> +
> +	case HCI_ACLDATA_PKT:
> +		hdev->stat.acl_tx++;
> +		break;
> +
> +	case HCI_SCODATA_PKT:
> +		hdev->stat.cmd_tx++;
> +		break;
> +	}
> +}
> +
> +/* ------- Interfaces to Shared Transport ------ */
> +
> +/* Called by ST layer to indicate protocol registration completion
> + * status.hci_st_open() function will wait for signal from this
> + * API when st_register() function returns ST_PENDING.
> + */
> +static void hci_st_registration_completion_cb(void *priv_data, char data)
> +{
> +	struct hci_st *lhst = (struct hci_st *)priv_data;
> +	/* hci_st_open() function needs value of 'data' to know
> +	 * the registration status(success/fail),So have a back
> +	 * up of it.
> +	 */
> +	lhst->streg_cbdata = data;
> +
> +	/* Got a feedback from ST for BT driver registration
> +	 * request.Wackup hci_st_open() function to continue
> +	 * it's open operation.
> +	 */
> +	complete(&lhst->wait_for_btdrv_reg_completion);
> +}
> +
> +/* Called by Shared Transport layer when receive data is
> + * available */
> +static long hci_st_receive(void *priv_data, struct sk_buff *skb)
> +{
> +	int err;
> +	int len;
> +	struct hci_st *lhst = (struct hci_st *)priv_data;
> +
> +	err = 0;
> +	len = 0;
> +
> +	if (skb == NULL) {
> +		pr_err("Invalid SKB received from ST");
> +		return -EFAULT;
> +	}
> +	if (!lhst) {
> +		kfree_skb(skb);
> +		pr_err("Invalid hci_st memory,freeing SKB");
> +		return -EFAULT;
> +	}
> +	if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
> +		kfree_skb(skb);
> +		pr_err("Device is not running,freeing SKB");
> +		return -EINVAL;
> +	}
> +
> +	len = skb->len;
> +	skb->dev = (struct net_device *)lhst->hdev;
> +
> +	/* Forward skb to HCI CORE layer */
> +	err = hci_recv_frame(skb);
> +	if (err) {
> +		kfree_skb(skb);
> +		pr_err("Unable to push skb to HCI CORE(%d),freeing SKB",
> +				err);
> +		return err;
> +	}
> +	lhst->hdev->stat.byte_rx += len;
> +
> +	return 0;
> +}
> +
> +/* ------- Interfaces to HCI layer ------ */
> +
> +/* Called from HCI core to initialize the device */
> +static int hci_st_open(struct hci_dev *hdev)
> +{
> +	static struct st_proto_s hci_st_proto;
> +	unsigned long timeleft;
> +	int err;
> +	err = 0;
> +
> +	pr_debug("%s %p", hdev->name, hdev);
> +
> +	/* Already registered with ST ? */
> +	if (test_bit(BT_ST_REGISTERED, &hst->flags)) {
> +		pr_err("Registered with ST already,open called again?");
> +		return 0;
> +	}

Why are you testing against this. This should be not needed at all.

> +	/* Populate BT driver info required by ST */
> +	memset(&hci_st_proto, 0, sizeof(hci_st_proto));
> +
> +	/* BT driver ID */
> +	hci_st_proto.type = ST_BT;
> +
> +	/* Receive function which called from ST */
> +	hci_st_proto.recv = hci_st_receive;
> +
> +	/* Packet match function may used in future */
> +	hci_st_proto.match_packet = NULL;
> +
> +	/* Callback to be called when registration is pending */
> +	hci_st_proto.reg_complete_cb = hci_st_registration_completion_cb;
> +
> +	/* This is write function pointer of ST. BT driver will make use of this
> +	 * for sending any packets to chip. ST will assign and give to us, so
> +	 * make it as NULL */
> +	hci_st_proto.write = NULL;
> +
> +	/* send in the hst to be received at registration complete callback
> +	 * and during st's receive
> +	 */
> +	hci_st_proto.priv_data = hst;
> +
> +	/* Register with ST layer */
> +	err = st_register(&hci_st_proto);

I am still against just claiming the st_ prefix where a company called
ST is active in the kernel as well. Is the Shared Transport really a
proper standard?

> +	if (err == -EINPROGRESS) {
> +		/* Prepare wait-for-completion handler data structures.
> +		 * Needed to syncronize this and st_registration_completion_cb()
> +		 * functions.
> +		 */
> +		init_completion(&hst->wait_for_btdrv_reg_completion);
> +
> +		/* Reset ST registration callback status flag , this value
> +		 * will be updated in hci_st_registration_completion_cb()
> +		 * function whenever it called from ST driver.
> +		 */
> +		hst->streg_cbdata = -EINPROGRESS;
> +
> +		/* ST is busy with other protocol registration(may be busy with
> +		 * firmware download).So,Wait till the registration callback
> +		 * (passed as a argument to st_register() function) getting
> +		 * called from ST.
> +		 */
> +		pr_debug(" %s waiting for reg completion signal from ST",
> +				__func__);
> +
> +		timeleft =
> +			wait_for_completion_timeout
> +			(&hst->wait_for_btdrv_reg_completion,
> +			 msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> +		if (!timeleft) {
> +			pr_err("Timeout(%d sec),didn't get reg"
> +					"completion signal from ST",
> +					BT_REGISTER_TIMEOUT / 1000);
> +			return -ETIMEDOUT;
> +		}
> +
> +		/* Is ST registration callback called with ERROR value? */
> +		if (hst->streg_cbdata != 0) {
> +			pr_err("ST reg completion CB called with invalid"
> +					"status %d", hst->streg_cbdata);
> +			return -EAGAIN;
> +		}
> +		err = 0;
> +	} else if (err == -1) {
> +		pr_err("st_register failed %d", err);
> +		return -EAGAIN;
> +	}
> +
> +	/* Do we have proper ST write function? */
> +	if (hci_st_proto.write != NULL) {
> +		/* We need this pointer for sending any Bluetooth pkts */
> +		hst->st_write = hci_st_proto.write;
> +	} else {
> +		pr_err("failed to get ST write func pointer");
> +
> +		/* Undo registration with ST */
> +		err = st_unregister(ST_BT);
> +		if (err < 0)
> +			pr_err("st_unregister failed %d", err);
> +
> +		hst->st_write = NULL;
> +		return -EAGAIN;
> +	}
> +
> +	/* Registration with ST layer is completed successfully,
> +	 * now chip is ready to accept commands from HCI CORE.
> +	 * Mark HCI Device flag as RUNNING
> +	 */
> +	set_bit(HCI_RUNNING, &hdev->flags);
> +
> +	/* Registration with ST successful */
> +	set_bit(BT_ST_REGISTERED, &hst->flags);
> +
> +	return err;
> +}
> +
> +/* Close device */
> +static int hci_st_close(struct hci_dev *hdev)
> +{
> +	int err;
> +	err = 0;
> +
> +	/* Unregister from ST layer */
> +	if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
> +		err = st_unregister(ST_BT);
> +		if (err != 0) {
> +			pr_err("st_unregister failed %d", err);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	hst->st_write = NULL;
> +
> +	/* ST layer would have moved chip to inactive state.
> +	 * So,clear HCI device RUNNING flag.
> +	 */
> +	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> +		return 0;
> +
> +	return err;
> +}
> +
> +/* Called from HCI CORE , Sends frames to Shared Transport */
> +static int hci_st_send_frame(struct sk_buff *skb)
> +{
> +	struct hci_dev *hdev;
> +	struct hci_st *hst;
> +	long len;
> +
> +	if (skb == NULL) {
> +		pr_err("Invalid skb received from HCI CORE");
> +		return -ENOMEM;
> +	}
> +	hdev = (struct hci_dev *)skb->dev;
> +	if (!hdev) {
> +		pr_err("SKB received for invalid HCI Device (hdev=NULL)");
> +		return -ENODEV;
> +	}
> +	if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> +		pr_err("Device is not running");
> +		return -EBUSY;
> +	}
> +
> +	hst = (struct hci_st *)hdev->driver_data;
> +
> +	/* Prepend skb with frame type */
> +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> +	pr_debug(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> +			skb->len);
> +
> +	/* Insert skb to shared transport layer's transmit queue.
> +	 * Freeing skb memory is taken care in shared transport layer,
> +	 * so don't free skb memory here.
> +	 */
> +	if (!hst->st_write) {
> +		kfree_skb(skb);
> +		pr_err(" Can't write to ST, st_write null?");
> +		return -EAGAIN;
> +	}
> +	len = hst->st_write(skb);
> +	if (len < 0) {
> +		/* Something went wrong in st write , free skb memory */
> +		kfree_skb(skb);
> +		pr_err(" ST write failed (%ld)", len);
> +		return -EAGAIN;
> +	}
> +
> +	/* ST accepted our skb. So, Go ahead and do rest */
> +	hdev->stat.byte_tx += len;
> +	hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> +
> +	return 0;
> +}
> +
> +static void hci_st_destruct(struct hci_dev *hdev)
> +{
> +	if (!hdev) {
> +		pr_err("Destruct called with invalid HCI Device"
> +				"(hdev=NULL)");
> +		return;
> +	}
> +
> +	pr_debug("%s", hdev->name);
> +
> +	/* free hci_st memory */
> +	if (hdev->driver_data != NULL)
> +		kfree(hdev->driver_data);
> +
> +	return;
> +}
> +
> +/* Creates new HCI device */
> +static int hci_st_register_dev(struct hci_st *hst)
> +{
> +	struct hci_dev *hdev;
> +
> +	/* Initialize and register HCI device */
> +	hdev = hci_alloc_dev();
> +	if (!hdev) {
> +		pr_err("Can't allocate HCI device");
> +		return -ENOMEM;
> +	}
> +	pr_debug(" HCI device allocated. hdev= %p", hdev);
> +
> +	hst->hdev = hdev;
> +	hdev->bus = HCI_UART;
> +	hdev->driver_data = hst;
> +	hdev->open = hci_st_open;
> +	hdev->close = hci_st_close;
> +	hdev->flush = NULL;
> +	hdev->send = hci_st_send_frame;
> +	hdev->destruct = hci_st_destruct;
> +	hdev->owner = THIS_MODULE;
> +
> +	if (reset)
> +		set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
> +
> +	if (hci_register_dev(hdev) < 0) {
> +		pr_err("Can't register HCI device");
> +		hci_free_dev(hdev);
> +		return -ENODEV;
> +	}
> +
> +	pr_debug(" HCI device registered. hdev= %p", hdev);
> +	return 0;
> +}
> +
> +/* ------- Module Init interface ------ */
> +
> +static int __init bt_drv_init(void)
> +{
> +	int err;
> +	err = 0;
> +
> +	pr_debug(" Bluetooth Driver Version %s", VERSION);
> +
> +	/* Allocate local resource memory */
> +	hst = kzalloc(sizeof(struct hci_st), GFP_KERNEL);
> +	if (!hst) {
> +		pr_err("Can't allocate control structure");
> +		return -ENFILE;
> +	}
> +
> +	/* Expose "hciX" device to user space */
> +	err = hci_st_register_dev(hst);
> +	if (err) {
> +		/* Release local resource memory */
> +		kfree(hst);
> +
> +		pr_err("Unable to expose hci0 device(%d)", err);
> +		return err;
> +	}
> +	set_bit(BT_DRV_RUNNING, &hst->flags);
> +	return err;
> +}
> +
> +/* ------- Module Exit interface ------ */
> +
> +static void __exit bt_drv_exit(void)
> +{
> +	/* Deallocate local resource's memory  */
> +	if (hst) {
> +		struct hci_dev *hdev = hst->hdev;
> +		if (hdev == NULL) {
> +			pr_err("Invalid hdev memory");
> +			kfree(hst);
> +		} else {
> +			hci_st_close(hdev);
> +			if (test_and_clear_bit(BT_DRV_RUNNING, &hst->flags)) {
> +				/* Remove HCI device (hciX) created
> +				 * in module init.
> +				 */
> +				hci_unregister_dev(hdev);
> +				/* Free HCI device memory */
> +				hci_free_dev(hdev);
> +			}
> +		}
> +	}
> +}

Registering the Bluetooth HCI driver in module_init/module_exit is not
acceptable. Turn your shared transport into a proper bus.

We want to be able to have generic kernels where this module is enabled,
but no Shared Transport is available.

Regards

Marcel

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Greg KH]

On Thu, Oct 07, 2010 at 12:05:48PM +0200, Marcel Holtmann wrote:
> Hi Pavan,
> 
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> > 
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface.
> > 
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> >  drivers/staging/ti-st/bt_drv.h |   61 -----
> >  3 files changed, 463 insertions(+), 570 deletions(-)
> >  create mode 100644 drivers/bluetooth/bt_ti.c
> >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> 
> I don't care about staging at all. So you sort that out with Greg.
> 
> Submit your driver for upstream inclusion. And once accepted you can pin
> Greg about removing it.

The driver is already in staging, this is the request to move it out of
staging and into the "correct" place in the tree.  The core of the ti-st
code is now in the drivers/misc/ directory in the linux-next tree, and
this patch is the request to move the bluetooth drive into the proper
drivers/bluetooth/ location.

thanks,

greg k-h

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Marcel Holtmann]

Hi Greg,

> > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > like BT, FM, GPS and WLAN onto a single chip.
> > > 
> > > This Bluetooth driver works on top of the TI_ST shared transport
> > > line discipline driver which also allows other drivers like
> > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > 
> > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > ---
> > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > 
> > I don't care about staging at all. So you sort that out with Greg.
> > 
> > Submit your driver for upstream inclusion. And once accepted you can pin
> > Greg about removing it.
> 
> The driver is already in staging, this is the request to move it out of
> staging and into the "correct" place in the tree.  The core of the ti-st
> code is now in the drivers/misc/ directory in the linux-next tree, and
> this patch is the request to move the bluetooth drive into the proper
> drivers/bluetooth/ location.

nice idea, but I don't want it that way. I am not dealing with staging
at all. They can submit this driver for upstream inclusion and then
delete it in a second step from staging. Or the other way around.

And as long as this driver registers a Bluetooth HCI device in its
module_init routine it is not ready for upstream. This needs to be fixed
first and I mentioned that already before.

Regards

Marcel

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Greg KH]

On Thu, Oct 07, 2010 at 05:21:07PM +0200, Marcel Holtmann wrote:
> Hi Greg,
> 
> > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > like BT, FM, GPS and WLAN onto a single chip.
> > > > 
> > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > line discipline driver which also allows other drivers like
> > > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > > 
> > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > ---
> > > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > > 
> > > I don't care about staging at all. So you sort that out with Greg.
> > > 
> > > Submit your driver for upstream inclusion. And once accepted you can pin
> > > Greg about removing it.
> > 
> > The driver is already in staging, this is the request to move it out of
> > staging and into the "correct" place in the tree.  The core of the ti-st
> > code is now in the drivers/misc/ directory in the linux-next tree, and
> > this patch is the request to move the bluetooth drive into the proper
> > drivers/bluetooth/ location.
> 
> nice idea, but I don't want it that way. I am not dealing with staging
> at all. They can submit this driver for upstream inclusion and then
> delete it in a second step from staging. Or the other way around.

That's fine, then consider this a "please add the driver to your tree"
type of request.

> And as long as this driver registers a Bluetooth HCI device in its
> module_init routine it is not ready for upstream. This needs to be fixed
> first and I mentioned that already before.

That's also fine, it's an iterative process :)

thanks,

greg k-h

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Gustavo F. Padovan]

* Marcel Holtmann <marcel@holtmann.org> [2010-10-07 17:21:07 +0200]:

> Hi Greg,
> 
> > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > like BT, FM, GPS and WLAN onto a single chip.
> > > > 
> > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > line discipline driver which also allows other drivers like
> > > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > > 
> > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > ---
> > > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > > 
> > > I don't care about staging at all. So you sort that out with Greg.
> > > 
> > > Submit your driver for upstream inclusion. And once accepted you can pin
> > > Greg about removing it.
> > 
> > The driver is already in staging, this is the request to move it out of
> > staging and into the "correct" place in the tree.  The core of the ti-st
> > code is now in the drivers/misc/ directory in the linux-next tree, and
> > this patch is the request to move the bluetooth drive into the proper
> > drivers/bluetooth/ location.
> 
> nice idea, but I don't want it that way. I am not dealing with staging
> at all. They can submit this driver for upstream inclusion and then
> delete it in a second step from staging. Or the other way around.

We just have to be sure to do both steps in the same release cycle,
otherwise we could ship the driver twice in the kernel (considering we
will delete it after merge in drivers/bluetooth/)

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

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Savoy, Pavan]

Marcel, Gustavo,

> -----Original Message-----
> From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gustavo=
 F.
> Padovan
> Sent: Thursday, October 07, 2010 12:57 PM
> To: Marcel Holtmann
> Cc: Greg KH; pavan-savoy@ti.com; linux-bluetooth@vger.kernel.org;
> johan.hedberg@gmail.com; linux-kernel@vger.kernel.org; Savoy, Pavan
> Subject: Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> * Marcel Holtmann <marcel@holtmann.org> [2010-10-07 17:21:07 +0200]:
>=20
> > Hi Greg,
> >
> > > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets=
.
> > > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > > like BT, FM, GPS and WLAN onto a single chip.
> > > > >
> > > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > > line discipline driver which also allows other drivers like
> > > > > FM V4L2 and GPS character driver to make use of the same UART
> interface.
> > > > >
> > > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > > ---
> > > > >  drivers/bluetooth/bt_ti.c      |  463
> ++++++++++++++++++++++++++++++++++++
> > > > >  drivers/staging/ti-st/bt_drv.c |  509 --------------------------=
-----
> ---------
> > > > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > > > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > > > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > > >
> > > > I don't care about staging at all. So you sort that out with Greg.
> > > >
> > > > Submit your driver for upstream inclusion. And once accepted you ca=
n pin
> > > > Greg about removing it.
> > >
> > > The driver is already in staging, this is the request to move it out =
of
> > > staging and into the "correct" place in the tree.  The core of the ti=
-st
> > > code is now in the drivers/misc/ directory in the linux-next tree, an=
d
> > > this patch is the request to move the bluetooth drive into the proper
> > > drivers/bluetooth/ location.
> >
> > nice idea, but I don't want it that way. I am not dealing with staging
> > at all. They can submit this driver for upstream inclusion and then
> > delete it in a second step from staging. Or the other way around.
>=20
> We just have to be sure to do both steps in the same release cycle,
> otherwise we could ship the driver twice in the kernel (considering we
> will delete it after merge in drivers/bluetooth/)

Yes, once this is reviewed, I will send across patches to delete it from st=
aging.
Also note, I have sent across updates patches, So please review.

The shared transport design can be found @ http://omappedia.org/wiki/Wilink=
_ST


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

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Gustavo F. Padovan]

Hi Greg,

* Greg KH <greg@kroah.com> [2010-10-07 07:34:09 -0700]:

> On Thu, Oct 07, 2010 at 12:05:48PM +0200, Marcel Holtmann wrote:
> > Hi Pavan,
> > 
> > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > like BT, FM, GPS and WLAN onto a single chip.
> > > 
> > > This Bluetooth driver works on top of the TI_ST shared transport
> > > line discipline driver which also allows other drivers like
> > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > 
> > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > ---
> > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > 
> > I don't care about staging at all. So you sort that out with Greg.
> > 
> > Submit your driver for upstream inclusion. And once accepted you can pin
> > Greg about removing it.
> 
> The driver is already in staging, this is the request to move it out of
> staging and into the "correct" place in the tree.  The core of the ti-st
> code is now in the drivers/misc/ directory in the linux-next tree, and
> this patch is the request to move the bluetooth drive into the proper
> drivers/bluetooth/ location.

I'm wondering why this driver never touched linux-bluetooth before. It
is on staging because it is not ready for a proper merge and while it is
not ready it needs the comments from the bluetooth developers here to
get it ready for merge in drivers/bluetooth. So why this never arrived 
here before?

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

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Greg KH]

On Thu, Oct 07, 2010 at 02:51:48PM -0300, Gustavo F. Padovan wrote:
> Hi Greg,
> 
> * Greg KH <greg@kroah.com> [2010-10-07 07:34:09 -0700]:
> 
> > On Thu, Oct 07, 2010 at 12:05:48PM +0200, Marcel Holtmann wrote:
> > > Hi Pavan,
> > > 
> > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > like BT, FM, GPS and WLAN onto a single chip.
> > > > 
> > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > line discipline driver which also allows other drivers like
> > > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > > 
> > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > ---
> > > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > > 
> > > I don't care about staging at all. So you sort that out with Greg.
> > > 
> > > Submit your driver for upstream inclusion. And once accepted you can pin
> > > Greg about removing it.
> > 
> > The driver is already in staging, this is the request to move it out of
> > staging and into the "correct" place in the tree.  The core of the ti-st
> > code is now in the drivers/misc/ directory in the linux-next tree, and
> > this patch is the request to move the bluetooth drive into the proper
> > drivers/bluetooth/ location.
> 
> I'm wondering why this driver never touched linux-bluetooth before. It
> is on staging because it is not ready for a proper merge and while it is
> not ready it needs the comments from the bluetooth developers here to
> get it ready for merge in drivers/bluetooth. So why this never arrived 
> here before?

This is the exact reason _why_ it is being sent here now.  To get the
review of the bluetooth developers for any changes that are needed to
get it merged into the proper place in the tree.

thanks,

greg k-h

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Gustavo F. Padovan]

* Greg KH <greg@kroah.com> [2010-10-07 14:30:18 -0700]:

> On Thu, Oct 07, 2010 at 02:51:48PM -0300, Gustavo F. Padovan wrote:
> > Hi Greg,
> > 
> > * Greg KH <greg@kroah.com> [2010-10-07 07:34:09 -0700]:
> > 
> > > On Thu, Oct 07, 2010 at 12:05:48PM +0200, Marcel Holtmann wrote:
> > > > Hi Pavan,
> > > > 
> > > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > > like BT, FM, GPS and WLAN onto a single chip.
> > > > > 
> > > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > > line discipline driver which also allows other drivers like
> > > > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > > > 
> > > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > > ---
> > > > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > > > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > > > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > > > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > > > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > > > 
> > > > I don't care about staging at all. So you sort that out with Greg.
> > > > 
> > > > Submit your driver for upstream inclusion. And once accepted you can pin
> > > > Greg about removing it.
> > > 
> > > The driver is already in staging, this is the request to move it out of
> > > staging and into the "correct" place in the tree.  The core of the ti-st
> > > code is now in the drivers/misc/ directory in the linux-next tree, and
> > > this patch is the request to move the bluetooth drive into the proper
> > > drivers/bluetooth/ location.
> > 
> > I'm wondering why this driver never touched linux-bluetooth before. It
> > is on staging because it is not ready for a proper merge and while it is
> > not ready it needs the comments from the bluetooth developers here to
> > get it ready for merge in drivers/bluetooth. So why this never arrived 
> > here before?
> 
> This is the exact reason _why_ it is being sent here now.  To get the
> review of the bluetooth developers for any changes that are needed to
> get it merged into the proper place in the tree.

Yes, but IMHO it took to long, from what I looked this drivers was merged
in stage about May and the patches arrived in linux-bluetooth only in
October. Is there a reason for such delay?  That's is something we need
to fix for the next bluetooth drivers we want to merge, so the developers
can get feedback earlier.

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

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Greg KH]

On Thu, Oct 07, 2010 at 04:09:06PM -0300, Gustavo F. Padovan wrote:
> * Greg KH <greg@kroah.com> [2010-10-07 14:30:18 -0700]:
> 
> > On Thu, Oct 07, 2010 at 02:51:48PM -0300, Gustavo F. Padovan wrote:
> > > Hi Greg,
> > > 
> > > * Greg KH <greg@kroah.com> [2010-10-07 07:34:09 -0700]:
> > > 
> > > > On Thu, Oct 07, 2010 at 12:05:48PM +0200, Marcel Holtmann wrote:
> > > > > Hi Pavan,
> > > > > 
> > > > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > > > like BT, FM, GPS and WLAN onto a single chip.
> > > > > > 
> > > > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > > > line discipline driver which also allows other drivers like
> > > > > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > > > > 
> > > > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > > > ---
> > > > > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > > > > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > > > > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > > > > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > > > > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > > > > 
> > > > > I don't care about staging at all. So you sort that out with Greg.
> > > > > 
> > > > > Submit your driver for upstream inclusion. And once accepted you can pin
> > > > > Greg about removing it.
> > > > 
> > > > The driver is already in staging, this is the request to move it out of
> > > > staging and into the "correct" place in the tree.  The core of the ti-st
> > > > code is now in the drivers/misc/ directory in the linux-next tree, and
> > > > this patch is the request to move the bluetooth drive into the proper
> > > > drivers/bluetooth/ location.
> > > 
> > > I'm wondering why this driver never touched linux-bluetooth before. It
> > > is on staging because it is not ready for a proper merge and while it is
> > > not ready it needs the comments from the bluetooth developers here to
> > > get it ready for merge in drivers/bluetooth. So why this never arrived 
> > > here before?
> > 
> > This is the exact reason _why_ it is being sent here now.  To get the
> > review of the bluetooth developers for any changes that are needed to
> > get it merged into the proper place in the tree.
> 
> Yes, but IMHO it took to long, from what I looked this drivers was merged
> in stage about May and the patches arrived in linux-bluetooth only in
> October. Is there a reason for such delay?

It took that long to get the "obvious" things fixed up.

> That's is something we need to fix for the next bluetooth drivers we
> want to merge, so the developers can get feedback earlier.

If you want to be notified of staging drivers that affect the bluetooth
stuff earlier, fine, I'll let you know.  Otherwise, no other subsystem
has asked for such notification, preferring to let the stable stuff stay
alone in the directory until it at least achieves a basic "clean" level.

thanks,

greg k-h

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Gustavo F. Padovan]

* Greg KH <greg@kroah.com> [2010-10-07 15:17:37 -0700]:

> On Thu, Oct 07, 2010 at 04:09:06PM -0300, Gustavo F. Padovan wrote:
> > * Greg KH <greg@kroah.com> [2010-10-07 14:30:18 -0700]:
> > 
> > > On Thu, Oct 07, 2010 at 02:51:48PM -0300, Gustavo F. Padovan wrote:
> > > > Hi Greg,
> > > > 
> > > > * Greg KH <greg@kroah.com> [2010-10-07 07:34:09 -0700]:
> > > > 
> > > > > On Thu, Oct 07, 2010 at 12:05:48PM +0200, Marcel Holtmann wrote:
> > > > > > Hi Pavan,
> > > > > > 
> > > > > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > > > > like BT, FM, GPS and WLAN onto a single chip.
> > > > > > > 
> > > > > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > > > > line discipline driver which also allows other drivers like
> > > > > > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > > > > > 
> > > > > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > > > > ---
> > > > > > >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++++++
> > > > > > >  drivers/staging/ti-st/bt_drv.c |  509 ----------------------------------------
> > > > > > >  drivers/staging/ti-st/bt_drv.h |   61 -----
> > > > > > >  3 files changed, 463 insertions(+), 570 deletions(-)
> > > > > > >  create mode 100644 drivers/bluetooth/bt_ti.c
> > > > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > > > > >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > > > > > 
> > > > > > I don't care about staging at all. So you sort that out with Greg.
> > > > > > 
> > > > > > Submit your driver for upstream inclusion. And once accepted you can pin
> > > > > > Greg about removing it.
> > > > > 
> > > > > The driver is already in staging, this is the request to move it out of
> > > > > staging and into the "correct" place in the tree.  The core of the ti-st
> > > > > code is now in the drivers/misc/ directory in the linux-next tree, and
> > > > > this patch is the request to move the bluetooth drive into the proper
> > > > > drivers/bluetooth/ location.
> > > > 
> > > > I'm wondering why this driver never touched linux-bluetooth before. It
> > > > is on staging because it is not ready for a proper merge and while it is
> > > > not ready it needs the comments from the bluetooth developers here to
> > > > get it ready for merge in drivers/bluetooth. So why this never arrived 
> > > > here before?
> > > 
> > > This is the exact reason _why_ it is being sent here now.  To get the
> > > review of the bluetooth developers for any changes that are needed to
> > > get it merged into the proper place in the tree.
> > 
> > Yes, but IMHO it took to long, from what I looked this drivers was merged
> > in stage about May and the patches arrived in linux-bluetooth only in
> > October. Is there a reason for such delay?
> 
> It took that long to get the "obvious" things fixed up.

Ok, I got your point. So let's keep things as is (unless the submitter
also wants the feedback from the subsystem). Thanks for the explanations. ;)

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

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Savoy, Pavan]

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 5:06 AM
> To: pavan-savoy@ti.com
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org; Savoy, Pavan
> Subject: Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>
> Hi Pavan,
>
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> >
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface=
.
> >
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> >  drivers/bluetooth/bt_ti.c      |  463 ++++++++++++++++++++++++++++++++=
++++
> >  drivers/staging/ti-st/bt_drv.c |  509 --------------------------------=
-----
> ---
> >  drivers/staging/ti-st/bt_drv.h |   61 -----
> >  3 files changed, 463 insertions(+), 570 deletions(-)
> >  create mode 100644 drivers/bluetooth/bt_ti.c
> >  delete mode 100644 drivers/staging/ti-st/bt_drv.c
> >  delete mode 100644 drivers/staging/ti-st/bt_drv.h
>
> I don't care about staging at all. So you sort that out with Greg.
>
> Submit your driver for upstream inclusion. And once accepted you can pin
> Greg about removing it.
>
> > diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
> > new file mode 100644
> > index 0000000..4f3d3aa
> > --- /dev/null
> > +++ b/drivers/bluetooth/bt_ti.c
> > @@ -0,0 +1,463 @@
> > +/*
> > + *  Texas Instrument's Bluetooth Driver For Shared Transport.
> > + *
> > + *  Bluetooth Driver acts as interface between HCI CORE and
> > + *  TI Shared Transport Layer.
> > + *
> > + *  Copyright (C) 2009-2010 Texas Instruments
> > + *  Author: Raja Mani <raja_mani@ti.com>
> > + * Pavan Savoy <pavan_savoy@ti.com>
> > + *
> > + *  This program is free software; you can redistribute it and/or modi=
fy
> > + *  it under the terms of the GNU General Public License version 2 as
> > + *  published by the Free Software Foundation.
> > + *
> > + *  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-13=
07
> USA
> > + *
> > + */
> > +
> > +#define pr_fmt(fmt)        "(tibt): " fmt
>
> Don't do this. Just use BT_DBG, BT_ERR, BT_INFO etc.

Yes, Will do it.

> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +
> > +#include <linux/ti_wilink_st.h>
> > +
> > +/* Bluetooth Driver Version */
> > +#define VERSION               "1.0"
> > +
> > +/* Defines number of seconds to wait for reg completion
> > + * callback getting called from ST (in case,registration
> > + * with ST returns PENDING status)
> > + */
> > +#define BT_REGISTER_TIMEOUT   6000 /* 6 sec */
> > +
> > +/* BT driver's local status */
> > +#define BT_DRV_RUNNING        0
> > +#define BT_ST_REGISTERED      1
> > +
> > +/**
> > + * struct hci_st - BT driver operation structure
> > + * @hdev: hci device pointer which binds to bt driver
> > + * @flags: used locally,to maintain various BT driver status
> > + * @streg_cbdata: to hold ST registration callback status
> > + * @st_write: write function pointer of ST driver
> > + * @wait_for_btdrv_reg_completion - completion sync between hci_st_ope=
n
> > + * and hci_st_registration_completion_cb.
> > + */
> > +struct hci_st {
> > +   struct hci_dev *hdev;
> > +   unsigned long flags;
> > +   char streg_cbdata;
> > +   long (*st_write) (struct sk_buff *);
> > +   struct completion wait_for_btdrv_reg_completion;
> > +};
> > +
> > +static struct hci_st *hst;
> > +static int reset;
> > +
> > +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> > +static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type=
)
> > +{
> > +   struct hci_dev *hdev;
> > +   hdev =3D hst->hdev;
> > +
> > +   /* Update HCI stat counters */
> > +   switch (pkt_type) {
> > +   case HCI_COMMAND_PKT:
> > +           hdev->stat.cmd_tx++;
> > +           break;
> > +
> > +   case HCI_ACLDATA_PKT:
> > +           hdev->stat.acl_tx++;
> > +           break;
> > +
> > +   case HCI_SCODATA_PKT:
> > +           hdev->stat.cmd_tx++;
> > +           break;
> > +   }
> > +}
> > +
> > +/* ------- Interfaces to Shared Transport ------ */
> > +
> > +/* Called by ST layer to indicate protocol registration completion
> > + * status.hci_st_open() function will wait for signal from this
> > + * API when st_register() function returns ST_PENDING.
> > + */
> > +static void hci_st_registration_completion_cb(void *priv_data, char da=
ta)
> > +{
> > +   struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > +   /* hci_st_open() function needs value of 'data' to know
> > +    * the registration status(success/fail),So have a back
> > +    * up of it.
> > +    */
> > +   lhst->streg_cbdata =3D data;
> > +
> > +   /* Got a feedback from ST for BT driver registration
> > +    * request.Wackup hci_st_open() function to continue
> > +    * it's open operation.
> > +    */
> > +   complete(&lhst->wait_for_btdrv_reg_completion);
> > +}
> > +
> > +/* Called by Shared Transport layer when receive data is
> > + * available */
> > +static long hci_st_receive(void *priv_data, struct sk_buff *skb)
> > +{
> > +   int err;
> > +   int len;
> > +   struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > +
> > +   err =3D 0;
> > +   len =3D 0;
> > +
> > +   if (skb =3D=3D NULL) {
> > +           pr_err("Invalid SKB received from ST");
> > +           return -EFAULT;
> > +   }
> > +   if (!lhst) {
> > +           kfree_skb(skb);
> > +           pr_err("Invalid hci_st memory,freeing SKB");
> > +           return -EFAULT;
> > +   }
> > +   if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
> > +           kfree_skb(skb);
> > +           pr_err("Device is not running,freeing SKB");
> > +           return -EINVAL;
> > +   }
> > +
> > +   len =3D skb->len;
> > +   skb->dev =3D (struct net_device *)lhst->hdev;
> > +
> > +   /* Forward skb to HCI CORE layer */
> > +   err =3D hci_recv_frame(skb);
> > +   if (err) {
> > +           kfree_skb(skb);
> > +           pr_err("Unable to push skb to HCI CORE(%d),freeing SKB",
> > +                           err);
> > +           return err;
> > +   }
> > +   lhst->hdev->stat.byte_rx +=3D len;
> > +
> > +   return 0;
> > +}
> > +
> > +/* ------- Interfaces to HCI layer ------ */
> > +
> > +/* Called from HCI core to initialize the device */
> > +static int hci_st_open(struct hci_dev *hdev)
> > +{
> > +   static struct st_proto_s hci_st_proto;
> > +   unsigned long timeleft;
> > +   int err;
> > +   err =3D 0;
> > +
> > +   pr_debug("%s %p", hdev->name, hdev);
> > +
> > +   /* Already registered with ST ? */
> > +   if (test_bit(BT_ST_REGISTERED, &hst->flags)) {
> > +           pr_err("Registered with ST already,open called again?");
> > +           return 0;
> > +   }
>
> Why are you testing against this. This should be not needed at all.

Oh yes, Agree, the HCI sock/core doesn't allow hci0 to be HCIDEVUP-ed twice=
.

> > +   /* Populate BT driver info required by ST */
> > +   memset(&hci_st_proto, 0, sizeof(hci_st_proto));
> > +
> > +   /* BT driver ID */
> > +   hci_st_proto.type =3D ST_BT;
> > +
> > +   /* Receive function which called from ST */
> > +   hci_st_proto.recv =3D hci_st_receive;
> > +
> > +   /* Packet match function may used in future */
> > +   hci_st_proto.match_packet =3D NULL;
> > +
> > +   /* Callback to be called when registration is pending */
> > +   hci_st_proto.reg_complete_cb =3D hci_st_registration_completion_cb;
> > +
> > +   /* This is write function pointer of ST. BT driver will make use of
> this
> > +    * for sending any packets to chip. ST will assign and give to us, =
so
> > +    * make it as NULL */
> > +   hci_st_proto.write =3D NULL;
> > +
> > +   /* send in the hst to be received at registration complete callback
> > +    * and during st's receive
> > +    */
> > +   hci_st_proto.priv_data =3D hst;
> > +
> > +   /* Register with ST layer */
> > +   err =3D st_register(&hci_st_proto);
>
> I am still against just claiming the st_ prefix where a company called
> ST is active in the kernel as well. Is the Shared Transport really a
> proper standard?

The driver is called TI_ST, I can rename it no problem, I'm not 100% convin=
ced with this either, any suggestions for names?

> > +   if (err =3D=3D -EINPROGRESS) {
> > +           /* Prepare wait-for-completion handler data structures.
> > +            * Needed to syncronize this and st_registration_completion=
_cb()
> > +            * functions.
> > +            */
> > +           init_completion(&hst->wait_for_btdrv_reg_completion);
> > +
> > +           /* Reset ST registration callback status flag , this value
> > +            * will be updated in hci_st_registration_completion_cb()
> > +            * function whenever it called from ST driver.
> > +            */
> > +           hst->streg_cbdata =3D -EINPROGRESS;
> > +
> > +           /* ST is busy with other protocol registration(may be busy =
with
> > +            * firmware download).So,Wait till the registration callbac=
k
> > +            * (passed as a argument to st_register() function) getting
> > +            * called from ST.
> > +            */
> > +           pr_debug(" %s waiting for reg completion signal from ST",
> > +                           __func__);
> > +
> > +           timeleft =3D
> > +                   wait_for_completion_timeout
> > +                   (&hst->wait_for_btdrv_reg_completion,
> > +                    msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> > +           if (!timeleft) {
> > +                   pr_err("Timeout(%d sec),didn't get reg"
> > +                                   "completion signal from ST",
> > +                                   BT_REGISTER_TIMEOUT / 1000);
> > +                   return -ETIMEDOUT;
> > +           }
> > +
> > +           /* Is ST registration callback called with ERROR value? */
> > +           if (hst->streg_cbdata !=3D 0) {
> > +                   pr_err("ST reg completion CB called with invalid"
> > +                                   "status %d", hst->streg_cbdata);
> > +                   return -EAGAIN;
> > +           }
> > +           err =3D 0;
> > +   } else if (err =3D=3D -1) {
> > +           pr_err("st_register failed %d", err);
> > +           return -EAGAIN;
> > +   }
> > +
> > +   /* Do we have proper ST write function? */
> > +   if (hci_st_proto.write !=3D NULL) {
> > +           /* We need this pointer for sending any Bluetooth pkts */
> > +           hst->st_write =3D hci_st_proto.write;
> > +   } else {
> > +           pr_err("failed to get ST write func pointer");
> > +
> > +           /* Undo registration with ST */
> > +           err =3D st_unregister(ST_BT);
> > +           if (err < 0)
> > +                   pr_err("st_unregister failed %d", err);
> > +
> > +           hst->st_write =3D NULL;
> > +           return -EAGAIN;
> > +   }
> > +
> > +   /* Registration with ST layer is completed successfully,
> > +    * now chip is ready to accept commands from HCI CORE.
> > +    * Mark HCI Device flag as RUNNING
> > +    */
> > +   set_bit(HCI_RUNNING, &hdev->flags);
> > +
> > +   /* Registration with ST successful */
> > +   set_bit(BT_ST_REGISTERED, &hst->flags);
> > +
> > +   return err;
> > +}
> > +
> > +/* Close device */
> > +static int hci_st_close(struct hci_dev *hdev)
> > +{
> > +   int err;
> > +   err =3D 0;
> > +
> > +   /* Unregister from ST layer */
> > +   if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
> > +           err =3D st_unregister(ST_BT);
> > +           if (err !=3D 0) {
> > +                   pr_err("st_unregister failed %d", err);
> > +                   return -EBUSY;
> > +           }
> > +   }
> > +
> > +   hst->st_write =3D NULL;
> > +
> > +   /* ST layer would have moved chip to inactive state.
> > +    * So,clear HCI device RUNNING flag.
> > +    */
> > +   if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> > +           return 0;
> > +
> > +   return err;
> > +}
> > +
> > +/* Called from HCI CORE , Sends frames to Shared Transport */
> > +static int hci_st_send_frame(struct sk_buff *skb)
> > +{
> > +   struct hci_dev *hdev;
> > +   struct hci_st *hst;
> > +   long len;
> > +
> > +   if (skb =3D=3D NULL) {
> > +           pr_err("Invalid skb received from HCI CORE");
> > +           return -ENOMEM;
> > +   }
> > +   hdev =3D (struct hci_dev *)skb->dev;
> > +   if (!hdev) {
> > +           pr_err("SKB received for invalid HCI Device (hdev=3DNULL)")=
;
> > +           return -ENODEV;
> > +   }
> > +   if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> > +           pr_err("Device is not running");
> > +           return -EBUSY;
> > +   }
> > +
> > +   hst =3D (struct hci_st *)hdev->driver_data;
> > +
> > +   /* Prepend skb with frame type */
> > +   memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> > +
> > +   pr_debug(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> > +                   skb->len);
> > +
> > +   /* Insert skb to shared transport layer's transmit queue.
> > +    * Freeing skb memory is taken care in shared transport layer,
> > +    * so don't free skb memory here.
> > +    */
> > +   if (!hst->st_write) {
> > +           kfree_skb(skb);
> > +           pr_err(" Can't write to ST, st_write null?");
> > +           return -EAGAIN;
> > +   }
> > +   len =3D hst->st_write(skb);
> > +   if (len < 0) {
> > +           /* Something went wrong in st write , free skb memory */
> > +           kfree_skb(skb);
> > +           pr_err(" ST write failed (%ld)", len);
> > +           return -EAGAIN;
> > +   }
> > +
> > +   /* ST accepted our skb. So, Go ahead and do rest */
> > +   hdev->stat.byte_tx +=3D len;
> > +   hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> > +
> > +   return 0;
> > +}
> > +
> > +static void hci_st_destruct(struct hci_dev *hdev)
> > +{
> > +   if (!hdev) {
> > +           pr_err("Destruct called with invalid HCI Device"
> > +                           "(hdev=3DNULL)");
> > +           return;
> > +   }
> > +
> > +   pr_debug("%s", hdev->name);
> > +
> > +   /* free hci_st memory */
> > +   if (hdev->driver_data !=3D NULL)
> > +           kfree(hdev->driver_data);
> > +
> > +   return;
> > +}
> > +
> > +/* Creates new HCI device */
> > +static int hci_st_register_dev(struct hci_st *hst)
> > +{
> > +   struct hci_dev *hdev;
> > +
> > +   /* Initialize and register HCI device */
> > +   hdev =3D hci_alloc_dev();
> > +   if (!hdev) {
> > +           pr_err("Can't allocate HCI device");
> > +           return -ENOMEM;
> > +   }
> > +   pr_debug(" HCI device allocated. hdev=3D %p", hdev);
> > +
> > +   hst->hdev =3D hdev;
> > +   hdev->bus =3D HCI_UART;
> > +   hdev->driver_data =3D hst;
> > +   hdev->open =3D hci_st_open;
> > +   hdev->close =3D hci_st_close;
> > +   hdev->flush =3D NULL;
> > +   hdev->send =3D hci_st_send_frame;
> > +   hdev->destruct =3D hci_st_destruct;
> > +   hdev->owner =3D THIS_MODULE;
> > +
> > +   if (reset)
> > +           set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
> > +
> > +   if (hci_register_dev(hdev) < 0) {
> > +           pr_err("Can't register HCI device");
> > +           hci_free_dev(hdev);
> > +           return -ENODEV;
> > +   }
> > +
> > +   pr_debug(" HCI device registered. hdev=3D %p", hdev);
> > +   return 0;
> > +}
> > +
> > +/* ------- Module Init interface ------ */
> > +
> > +static int __init bt_drv_init(void)
> > +{
> > +   int err;
> > +   err =3D 0;
> > +
> > +   pr_debug(" Bluetooth Driver Version %s", VERSION);
> > +
> > +   /* Allocate local resource memory */
> > +   hst =3D kzalloc(sizeof(struct hci_st), GFP_KERNEL);
> > +   if (!hst) {
> > +           pr_err("Can't allocate control structure");
> > +           return -ENFILE;
> > +   }
> > +
> > +   /* Expose "hciX" device to user space */
> > +   err =3D hci_st_register_dev(hst);
> > +   if (err) {
> > +           /* Release local resource memory */
> > +           kfree(hst);
> > +
> > +           pr_err("Unable to expose hci0 device(%d)", err);
> > +           return err;
> > +   }
> > +   set_bit(BT_DRV_RUNNING, &hst->flags);
> > +   return err;
> > +}
> > +
> > +/* ------- Module Exit interface ------ */
> > +
> > +static void __exit bt_drv_exit(void)
> > +{
> > +   /* Deallocate local resource's memory  */
> > +   if (hst) {
> > +           struct hci_dev *hdev =3D hst->hdev;
> > +           if (hdev =3D=3D NULL) {
> > +                   pr_err("Invalid hdev memory");
> > +                   kfree(hst);
> > +           } else {
> > +                   hci_st_close(hdev);
> > +                   if (test_and_clear_bit(BT_DRV_RUNNING, &hst->flags)=
) {
> > +                           /* Remove HCI device (hciX) created
> > +                            * in module init.
> > +                            */
> > +                           hci_unregister_dev(hdev);
> > +                           /* Free HCI device memory */
> > +                           hci_free_dev(hdev);
> > +                   }
> > +           }
> > +   }
> > +}
>
> Registering the Bluetooth HCI driver in module_init/module_exit is not
> acceptable. Turn your shared transport into a proper bus.

Yes, you did comment on it before, I remember, I did prototype the driver a=
s
a bus driver, However I didn't find any advantages by converting it to a bu=
s
driver.
As in, currently the shared transport driver is a line discipline driver be=
cause
it is the only way it can communicate over TTY without being tightly couple=
d with the UART driver.

> We want to be able to have generic kernels where this module is enabled,
> but no Shared Transport is available.

Oh if this is the reason I cannot have hci_register/_unregister in module_i=
nit/_exit, Can I do this module "depends" on TI_ST, Then it would not
even be visible to build if TI_ST is not selected.

> Regards
>
> Marcel
>

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Marcel Holtmann]

Hi Pavan,

> > Registering the Bluetooth HCI driver in module_init/module_exit is not
> > acceptable. Turn your shared transport into a proper bus.
> 
> Yes, you did comment on it before, I remember, I did prototype the driver as
> a bus driver, However I didn't find any advantages by converting it to a bus
> driver.
> As in, currently the shared transport driver is a line discipline driver because
> it is the only way it can communicate over TTY without being tightly coupled with the UART driver.
> 
> > We want to be able to have generic kernels where this module is enabled,
> > but no Shared Transport is available.
> 
> Oh if this is the reason I cannot have hci_register/_unregister in module_init/_exit, Can I do this module "depends" on TI_ST, Then it would not
> even be visible to build if TI_ST is not selected.

this is not helping either. Then TI_ST can not be selected and so you
still end up with some weird platform specific kernels. We don't want
that. We want generic kernels that can detect the hardware they are
running on.

As I said, I will not accept this driver if it registers HCI device in
module_init. No other driver is doing this and it is in general a really
really really bad idea.

Regards

Marcel

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Savoy, Pavan]

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 10:18 AM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> > > Registering the Bluetooth HCI driver in module_init/module_exit is no=
t
> > > acceptable. Turn your shared transport into a proper bus.
> >
> > Yes, you did comment on it before, I remember, I did prototype the driv=
er as
> > a bus driver, However I didn't find any advantages by converting it to =
a bus
> > driver.
> > As in, currently the shared transport driver is a line discipline drive=
r
> because
> > it is the only way it can communicate over TTY without being tightly co=
upled
> with the UART driver.
> >
> > > We want to be able to have generic kernels where this module is enabl=
ed,
> > > but no Shared Transport is available.
> >
> > Oh if this is the reason I cannot have hci_register/_unregister in
> module_init/_exit, Can I do this module "depends" on TI_ST, Then it would=
 not
> > even be visible to build if TI_ST is not selected.
>=20
> this is not helping either. Then TI_ST can not be selected and so you
> still end up with some weird platform specific kernels. We don't want
> that. We want generic kernels that can detect the hardware they are
> running on.
>=20
> As I said, I will not accept this driver if it registers HCI device in
> module_init. No other driver is doing this and it is in general a really
> really really bad idea.
>=20

Ok, now I am beginning to get what you say, Let me check, may be what
I can do is, have something like a st_prepare() function called in the
module_init, and a _probe function of the bluetooth driver will be called,
_ONLY_ if the _probe of my platform driver has been called..
Do you think this would be a good idea?

Note: the TI_ST driver is also a platform device driver, so that TI_ST's
Probe is not called, if a arch/xx/board-xx doesn't add it.

Please suggest.


>=20
> Marcel
>=20

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Marcel Holtmann]

Hi Pavan,

> > > > Registering the Bluetooth HCI driver in module_init/module_exit is not
> > > > acceptable. Turn your shared transport into a proper bus.
> > >
> > > Yes, you did comment on it before, I remember, I did prototype the driver as
> > > a bus driver, However I didn't find any advantages by converting it to a bus
> > > driver.
> > > As in, currently the shared transport driver is a line discipline driver
> > because
> > > it is the only way it can communicate over TTY without being tightly coupled
> > with the UART driver.
> > >
> > > > We want to be able to have generic kernels where this module is enabled,
> > > > but no Shared Transport is available.
> > >
> > > Oh if this is the reason I cannot have hci_register/_unregister in
> > module_init/_exit, Can I do this module "depends" on TI_ST, Then it would not
> > > even be visible to build if TI_ST is not selected.
> > 
> > this is not helping either. Then TI_ST can not be selected and so you
> > still end up with some weird platform specific kernels. We don't want
> > that. We want generic kernels that can detect the hardware they are
> > running on.
> > 
> > As I said, I will not accept this driver if it registers HCI device in
> > module_init. No other driver is doing this and it is in general a really
> > really really bad idea.
> > 
> 
> Ok, now I am beginning to get what you say, Let me check, may be what
> I can do is, have something like a st_prepare() function called in the
> module_init, and a _probe function of the bluetooth driver will be called,
> _ONLY_ if the _probe of my platform driver has been called..
> Do you think this would be a good idea?
> 
> Note: the TI_ST driver is also a platform device driver, so that TI_ST's
> Probe is not called, if a arch/xx/board-xx doesn't add it.

that that should be your bus right there.

Let me repeat this. If you register the HCI device in module_init then
it will be registered on all platform this module is selected. Even if
the kernel runs on x86. And that is not acceptable. Registering devices
in module_init is a bad idea no matter what. That is why all other
drivers just register a driver here and not a device.

Regards

Marcel

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Savoy, Pavan]

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 10:37 AM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> > > > > Registering the Bluetooth HCI driver in module_init/module_exit i=
s not
> > > > > acceptable. Turn your shared transport into a proper bus.
> > > >
> > > > Yes, you did comment on it before, I remember, I did prototype the
> driver as
> > > > a bus driver, However I didn't find any advantages by converting it=
 to a
> bus
> > > > driver.
> > > > As in, currently the shared transport driver is a line discipline d=
river
> > > because
> > > > it is the only way it can communicate over TTY without being tightl=
y
> coupled
> > > with the UART driver.
> > > >
> > > > > We want to be able to have generic kernels where this module is
> enabled,
> > > > > but no Shared Transport is available.
> > > >
> > > > Oh if this is the reason I cannot have hci_register/_unregister in
> > > module_init/_exit, Can I do this module "depends" on TI_ST, Then it w=
ould
> not
> > > > even be visible to build if TI_ST is not selected.
> > >
> > > this is not helping either. Then TI_ST can not be selected and so you
> > > still end up with some weird platform specific kernels. We don't want
> > > that. We want generic kernels that can detect the hardware they are
> > > running on.
> > >
> > > As I said, I will not accept this driver if it registers HCI device i=
n
> > > module_init. No other driver is doing this and it is in general a rea=
lly
> > > really really bad idea.
> > >
> >
> > Ok, now I am beginning to get what you say, Let me check, may be what
> > I can do is, have something like a st_prepare() function called in the
> > module_init, and a _probe function of the bluetooth driver will be call=
ed,
> > _ONLY_ if the _probe of my platform driver has been called..
> > Do you think this would be a good idea?
> >
> > Note: the TI_ST driver is also a platform device driver, so that TI_ST'=
s
> > Probe is not called, if a arch/xx/board-xx doesn't add it.
>=20
> that that should be your bus right there.

I understand the perspective, but "bus" is not device-driver type of model =
right? I mean I need a device which will be added in some platform specific
board file, and the driver in my driver core file.

> Let me repeat this. If you register the HCI device in module_init then
> it will be registered on all platform this module is selected. Even if
> the kernel runs on x86. And that is not acceptable. Registering devices
> in module_init is a bad idea no matter what. That is why all other
> drivers just register a driver here and not a device.

I did initially think about making each of the protocol drivers a=20
platform devices as well.
As in Bluetooth/FM/GPS TI_ST driver would also be a platform device and its=
 _probe doing the HCI/v4L2/character device registration.

So which one do you think makes more sense here?
1. Do I EXPORT a new symbol called st_prepare? And allow hci registration t=
here?

2. Or make Bluetooth device a platform device and this driver a platform dr=
iver
and add this Bluetooth device only when I add TI_ST platform device?

> Regards
>=20
> Marcel
>=20

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Marcel Holtmann]

Hi Pavan,

> > > > > > Registering the Bluetooth HCI driver in module_init/module_exit is not
> > > > > > acceptable. Turn your shared transport into a proper bus.
> > > > >
> > > > > Yes, you did comment on it before, I remember, I did prototype the
> > driver as
> > > > > a bus driver, However I didn't find any advantages by converting it to a
> > bus
> > > > > driver.
> > > > > As in, currently the shared transport driver is a line discipline driver
> > > > because
> > > > > it is the only way it can communicate over TTY without being tightly
> > coupled
> > > > with the UART driver.
> > > > >
> > > > > > We want to be able to have generic kernels where this module is
> > enabled,
> > > > > > but no Shared Transport is available.
> > > > >
> > > > > Oh if this is the reason I cannot have hci_register/_unregister in
> > > > module_init/_exit, Can I do this module "depends" on TI_ST, Then it would
> > not
> > > > > even be visible to build if TI_ST is not selected.
> > > >
> > > > this is not helping either. Then TI_ST can not be selected and so you
> > > > still end up with some weird platform specific kernels. We don't want
> > > > that. We want generic kernels that can detect the hardware they are
> > > > running on.
> > > >
> > > > As I said, I will not accept this driver if it registers HCI device in
> > > > module_init. No other driver is doing this and it is in general a really
> > > > really really bad idea.
> > > >
> > >
> > > Ok, now I am beginning to get what you say, Let me check, may be what
> > > I can do is, have something like a st_prepare() function called in the
> > > module_init, and a _probe function of the bluetooth driver will be called,
> > > _ONLY_ if the _probe of my platform driver has been called..
> > > Do you think this would be a good idea?
> > >
> > > Note: the TI_ST driver is also a platform device driver, so that TI_ST's
> > > Probe is not called, if a arch/xx/board-xx doesn't add it.
> > 
> > that that should be your bus right there.
> 
> I understand the perspective, but "bus" is not device-driver type of model right? I mean I need a device which will be added in some platform specific
> board file, and the driver in my driver core file.
> 
> > Let me repeat this. If you register the HCI device in module_init then
> > it will be registered on all platform this module is selected. Even if
> > the kernel runs on x86. And that is not acceptable. Registering devices
> > in module_init is a bad idea no matter what. That is why all other
> > drivers just register a driver here and not a device.
> 
> I did initially think about making each of the protocol drivers a 
> platform devices as well.
> As in Bluetooth/FM/GPS TI_ST driver would also be a platform device and its _probe doing the HCI/v4L2/character device registration.
> 
> So which one do you think makes more sense here?
> 1. Do I EXPORT a new symbol called st_prepare? And allow hci registration there?
> 
> 2. Or make Bluetooth device a platform device and this driver a platform driver
> and add this Bluetooth device only when I add TI_ST platform device?

then make them a platform device. Since you do need a proper parent for
these devices anyway. Otherwise a lot of logic within sysfs will fail.

Regards

Marcel

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Savoy, Pavan]

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 10:50 AM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> > > > > > > Registering the Bluetooth HCI driver in module_init/module_ex=
it is
> not
> > > > > > > acceptable. Turn your shared transport into a proper bus.
> > > > > >
> > > > > > Yes, you did comment on it before, I remember, I did prototype =
the
> > > driver as
> > > > > > a bus driver, However I didn't find any advantages by convertin=
g it
> to a
> > > bus
> > > > > > driver.
> > > > > > As in, currently the shared transport driver is a line discipli=
ne
> driver
> > > > > because
> > > > > > it is the only way it can communicate over TTY without being ti=
ghtly
> > > coupled
> > > > > with the UART driver.
> > > > > >
> > > > > > > We want to be able to have generic kernels where this module =
is
> > > enabled,
> > > > > > > but no Shared Transport is available.
> > > > > >
> > > > > > Oh if this is the reason I cannot have hci_register/_unregister=
 in
> > > > > module_init/_exit, Can I do this module "depends" on TI_ST, Then =
it
> would
> > > not
> > > > > > even be visible to build if TI_ST is not selected.
> > > > >
> > > > > this is not helping either. Then TI_ST can not be selected and so=
 you
> > > > > still end up with some weird platform specific kernels. We don't =
want
> > > > > that. We want generic kernels that can detect the hardware they a=
re
> > > > > running on.
> > > > >
> > > > > As I said, I will not accept this driver if it registers HCI devi=
ce in
> > > > > module_init. No other driver is doing this and it is in general a
> really
> > > > > really really bad idea.
> > > > >
> > > >
> > > > Ok, now I am beginning to get what you say, Let me check, may be wh=
at
> > > > I can do is, have something like a st_prepare() function called in =
the
> > > > module_init, and a _probe function of the bluetooth driver will be
> called,
> > > > _ONLY_ if the _probe of my platform driver has been called..
> > > > Do you think this would be a good idea?
> > > >
> > > > Note: the TI_ST driver is also a platform device driver, so that TI=
_ST's
> > > > Probe is not called, if a arch/xx/board-xx doesn't add it.
> > >
> > > that that should be your bus right there.
> >
> > I understand the perspective, but "bus" is not device-driver type of mo=
del
> right? I mean I need a device which will be added in some platform specif=
ic
> > board file, and the driver in my driver core file.
> >
> > > Let me repeat this. If you register the HCI device in module_init the=
n
> > > it will be registered on all platform this module is selected. Even i=
f
> > > the kernel runs on x86. And that is not acceptable. Registering devic=
es
> > > in module_init is a bad idea no matter what. That is why all other
> > > drivers just register a driver here and not a device.
> >
> > I did initially think about making each of the protocol drivers a
> > platform devices as well.
> > As in Bluetooth/FM/GPS TI_ST driver would also be a platform device and=
 its
> _probe doing the HCI/v4L2/character device registration.
> >
> > So which one do you think makes more sense here?
> > 1. Do I EXPORT a new symbol called st_prepare? And allow hci registrati=
on
> there?
> >
> > 2. Or make Bluetooth device a platform device and this driver a platfor=
m
> driver
> > and add this Bluetooth device only when I add TI_ST platform device?
>=20
> then make them a platform device. Since you do need a proper parent for
> these devices anyway. Otherwise a lot of logic within sysfs will fail.

Hnm, OK, Thanks. This also sort of helps me out in future if I wanted to su=
pport
multiple TI_ST devices (very hypothetical)

Ok, I will send out a patch for this. I will also make sure the patch I sen=
d
would be for addition of driver as you suggested and not to move from stagi=
ng.

Thanks,
Pavan.

> Regards
>=20
> Marcel
>=20

[PATCH 0/2] v2: Bluetooth driver for TI_ST

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

Marcel,

I have taken care of the changes you've mentioned, Now the
bluetooth driver is also a platform driver and it requires a
platform device and this platform driver's probe will do the
registration to HCI.

other changes include the usage of BT_DBG instead of pr_*
and removal of piece of code which was redundant.

Please review and comment...

Pavan Savoy (2):
  drivers:bluetooth: TI_ST bluetooth driver
  drivers:bluetooth: Kconfig & Makefile for TI BT

 drivers/bluetooth/Kconfig  |   10 +
 drivers/bluetooth/Makefile |    1 +
 drivers/bluetooth/bt_ti.c  |  489 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 500 insertions(+), 0 deletions(-)
 create mode 100644 drivers/bluetooth/bt_ti.c

[PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

This is the bluetooth protocol driver for the TI WiLink7 chipsets.
Texas Instrument's WiLink chipsets combine wireless technologies
like BT, FM, GPS and WLAN onto a single chip.

This Bluetooth driver works on top of the TI_ST shared transport
line discipline driver which also allows other drivers like
FM V4L2 and GPS character driver to make use of the same UART interface.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/bluetooth/bt_ti.c |  489 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 489 insertions(+), 0 deletions(-)
 create mode 100644 drivers/bluetooth/bt_ti.c

diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
new file mode 100644
index 0000000..dffbb56
--- /dev/null
+++ b/drivers/bluetooth/bt_ti.c
@@ -0,0 +1,489 @@
+/*
+ *  Texas Instrument's Bluetooth Driver For Shared Transport.
+ *
+ *  Bluetooth Driver acts as interface between HCI CORE and
+ *  TI Shared Transport Layer.
+ *
+ *  Copyright (C) 2009-2010 Texas Instruments
+ *  Author: Raja Mani <raja_mani@ti.com>
+ *	Pavan Savoy <pavan_savoy@ti.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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/platform_device.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include <linux/ti_wilink_st.h>
+
+/* Bluetooth Driver Version */
+#define VERSION               "1.0"
+
+/* Defines number of seconds to wait for reg completion
+ * callback getting called from ST (in case,registration
+ * with ST returns PENDING status)
+ */
+#define BT_REGISTER_TIMEOUT   6000	/* 6 sec */
+
+/* BT driver's local status */
+#define BT_DRV_RUNNING        0
+#define BT_ST_REGISTERED      1
+
+/**
+ * struct hci_st - BT driver operation structure
+ * @hdev: hci device pointer which binds to bt driver
+ * @flags: used locally,to maintain various BT driver status
+ * @streg_cbdata: to hold ST registration callback status
+ * @st_write: write function pointer of ST driver
+ * @wait_for_btdrv_reg_completion - completion sync between hci_st_open
+ *	and hci_st_registration_completion_cb.
+ */
+struct hci_st {
+	struct hci_dev *hdev;
+	unsigned long flags;
+	char streg_cbdata;
+	long (*st_write) (struct sk_buff *);
+	struct completion wait_for_btdrv_reg_completion;
+};
+
+static int reset;
+
+/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
+static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type)
+{
+	struct hci_dev *hdev;
+	hdev = hst->hdev;
+
+	/* Update HCI stat counters */
+	switch (pkt_type) {
+	case HCI_COMMAND_PKT:
+		hdev->stat.cmd_tx++;
+		break;
+
+	case HCI_ACLDATA_PKT:
+		hdev->stat.acl_tx++;
+		break;
+
+	case HCI_SCODATA_PKT:
+		hdev->stat.cmd_tx++;
+		break;
+	}
+}
+
+/* ------- Interfaces to Shared Transport ------ */
+
+/* Called by ST layer to indicate protocol registration completion
+ * status.hci_st_open() function will wait for signal from this
+ * API when st_register() function returns ST_PENDING.
+ */
+static void hci_st_registration_completion_cb(void *priv_data, char data)
+{
+	struct hci_st *lhst = (struct hci_st *)priv_data;
+	/* hci_st_open() function needs value of 'data' to know
+	 * the registration status(success/fail),So have a back
+	 * up of it.
+	 */
+	lhst->streg_cbdata = data;
+
+	/* Got a feedback from ST for BT driver registration
+	 * request.Wackup hci_st_open() function to continue
+	 * it's open operation.
+	 */
+	complete(&lhst->wait_for_btdrv_reg_completion);
+}
+
+/* Called by Shared Transport layer when receive data is
+ * available */
+static long hci_st_receive(void *priv_data, struct sk_buff *skb)
+{
+	int err;
+	int len;
+	struct hci_st *lhst = (struct hci_st *)priv_data;
+
+	err = 0;
+	len = 0;
+
+	if (skb == NULL) {
+		BT_ERR("Invalid SKB received from ST");
+		return -EFAULT;
+	}
+	if (!lhst) {
+		kfree_skb(skb);
+		BT_ERR("Invalid hci_st memory,freeing SKB");
+		return -EFAULT;
+	}
+	if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
+		kfree_skb(skb);
+		BT_ERR("Device is not running,freeing SKB");
+		return -EINVAL;
+	}
+
+	len = skb->len;
+	skb->dev = (struct net_device *)lhst->hdev;
+
+	/* Forward skb to HCI CORE layer */
+	err = hci_recv_frame(skb);
+	if (err) {
+		kfree_skb(skb);
+		BT_ERR("Unable to push skb to HCI CORE(%d),freeing SKB",
+				err);
+		return err;
+	}
+	lhst->hdev->stat.byte_rx += len;
+
+	return 0;
+}
+
+/* ------- Interfaces to HCI layer ------ */
+
+/* Called from HCI core to initialize the device */
+static int hci_st_open(struct hci_dev *hdev)
+{
+	static struct st_proto_s hci_st_proto;
+	unsigned long timeleft;
+	struct hci_st *hst;
+	int err;
+	err = 0;
+
+	BT_DBG("%s %p", hdev->name, hdev);
+	hst = hdev->driver_data;
+
+	/* Populate BT driver info required by ST */
+	memset(&hci_st_proto, 0, sizeof(hci_st_proto));
+
+	/* BT driver ID */
+	hci_st_proto.type = ST_BT;
+
+	/* Receive function which called from ST */
+	hci_st_proto.recv = hci_st_receive;
+
+	/* Packet match function may used in future */
+	hci_st_proto.match_packet = NULL;
+
+	/* Callback to be called when registration is pending */
+	hci_st_proto.reg_complete_cb = hci_st_registration_completion_cb;
+
+	/* This is write function pointer of ST. BT driver will make use of this
+	 * for sending any packets to chip. ST will assign and give to us, so
+	 * make it as NULL */
+	hci_st_proto.write = NULL;
+
+	/* send in the hst to be received at registration complete callback
+	 * and during st's receive
+	 */
+	hci_st_proto.priv_data = hst;
+
+	/* Register with ST layer */
+	err = st_register(&hci_st_proto);
+	if (err == -EINPROGRESS) {
+		/* Prepare wait-for-completion handler data structures.
+		 * Needed to syncronize this and st_registration_completion_cb()
+		 * functions.
+		 */
+		init_completion(&hst->wait_for_btdrv_reg_completion);
+
+		/* Reset ST registration callback status flag , this value
+		 * will be updated in hci_st_registration_completion_cb()
+		 * function whenever it called from ST driver.
+		 */
+		hst->streg_cbdata = -EINPROGRESS;
+
+		/* ST is busy with other protocol registration(may be busy with
+		 * firmware download).So,Wait till the registration callback
+		 * (passed as a argument to st_register() function) getting
+		 * called from ST.
+		 */
+		BT_DBG(" %s waiting for reg completion signal from ST",
+				__func__);
+
+		timeleft =
+			wait_for_completion_timeout
+			(&hst->wait_for_btdrv_reg_completion,
+			 msecs_to_jiffies(BT_REGISTER_TIMEOUT));
+		if (!timeleft) {
+			BT_ERR("Timeout(%d sec),didn't get reg"
+					"completion signal from ST",
+					BT_REGISTER_TIMEOUT / 1000);
+			return -ETIMEDOUT;
+		}
+
+		/* Is ST registration callback called with ERROR value? */
+		if (hst->streg_cbdata != 0) {
+			BT_ERR("ST reg completion CB called with invalid"
+					"status %d", hst->streg_cbdata);
+			return -EAGAIN;
+		}
+		err = 0;
+	} else if (err == -1) {
+		BT_ERR("st_register failed %d", err);
+		return -EAGAIN;
+	}
+
+	/* Do we have proper ST write function? */
+	if (hci_st_proto.write != NULL) {
+		/* We need this pointer for sending any Bluetooth pkts */
+		hst->st_write = hci_st_proto.write;
+	} else {
+		BT_ERR("failed to get ST write func pointer");
+
+		/* Undo registration with ST */
+		err = st_unregister(ST_BT);
+		if (err < 0)
+			BT_ERR("st_unregister failed %d", err);
+
+		hst->st_write = NULL;
+		return -EAGAIN;
+	}
+
+	/* Registration with ST layer is completed successfully,
+	 * now chip is ready to accept commands from HCI CORE.
+	 * Mark HCI Device flag as RUNNING
+	 */
+	set_bit(HCI_RUNNING, &hdev->flags);
+
+	/* Registration with ST successful */
+	set_bit(BT_ST_REGISTERED, &hst->flags);
+
+	return err;
+}
+
+/* Close device */
+static int hci_st_close(struct hci_dev *hdev)
+{
+	int err;
+	struct hci_st *hst;
+	err = 0;
+
+	hst = hdev->driver_data;
+	/* Unregister from ST layer */
+	if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
+		err = st_unregister(ST_BT);
+		if (err != 0) {
+			BT_ERR("st_unregister failed %d", err);
+			return -EBUSY;
+		}
+	}
+
+	hst->st_write = NULL;
+
+	/* ST layer would have moved chip to inactive state.
+	 * So,clear HCI device RUNNING flag.
+	 */
+	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
+		return 0;
+
+	return err;
+}
+
+/* Called from HCI CORE , Sends frames to Shared Transport */
+static int hci_st_send_frame(struct sk_buff *skb)
+{
+	struct hci_dev *hdev;
+	struct hci_st *hst;
+	long len;
+
+	if (skb == NULL) {
+		BT_ERR("Invalid skb received from HCI CORE");
+		return -ENOMEM;
+	}
+	hdev = (struct hci_dev *)skb->dev;
+	if (!hdev) {
+		BT_ERR("SKB received for invalid HCI Device (hdev=NULL)");
+		return -ENODEV;
+	}
+	if (!test_bit(HCI_RUNNING, &hdev->flags)) {
+		BT_ERR("Device is not running");
+		return -EBUSY;
+	}
+
+	hst = (struct hci_st *)hdev->driver_data;
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
+			skb->len);
+
+	/* Insert skb to shared transport layer's transmit queue.
+	 * Freeing skb memory is taken care in shared transport layer,
+	 * so don't free skb memory here.
+	 */
+	if (!hst->st_write) {
+		kfree_skb(skb);
+		BT_ERR(" Can't write to ST, st_write null?");
+		return -EAGAIN;
+	}
+	len = hst->st_write(skb);
+	if (len < 0) {
+		/* Something went wrong in st write , free skb memory */
+		kfree_skb(skb);
+		BT_ERR(" ST write failed (%ld)", len);
+		return -EAGAIN;
+	}
+
+	/* ST accepted our skb. So, Go ahead and do rest */
+	hdev->stat.byte_tx += len;
+	hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
+
+	return 0;
+}
+
+static void hci_st_destruct(struct hci_dev *hdev)
+{
+	if (!hdev) {
+		BT_ERR("Destruct called with invalid HCI Device"
+				"(hdev=NULL)");
+		return;
+	}
+
+	BT_DBG("%s", hdev->name);
+
+	/* free hci_st memory */
+	if (hdev->driver_data != NULL)
+		kfree(hdev->driver_data);
+
+	return;
+}
+
+/* Creates new HCI device */
+static int hci_st_register_dev(struct hci_st *hst)
+{
+	struct hci_dev *hdev;
+
+	/* Initialize and register HCI device */
+	hdev = hci_alloc_dev();
+	if (!hdev) {
+		BT_ERR("Can't allocate HCI device");
+		return -ENOMEM;
+	}
+	BT_DBG(" HCI device allocated. hdev= %p", hdev);
+
+	hst->hdev = hdev;
+	hdev->bus = HCI_UART;
+	hdev->driver_data = hst;
+	hdev->open = hci_st_open;
+	hdev->close = hci_st_close;
+	hdev->flush = NULL;
+	hdev->send = hci_st_send_frame;
+	hdev->destruct = hci_st_destruct;
+	hdev->owner = THIS_MODULE;
+
+	if (reset)
+		set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
+
+	if (hci_register_dev(hdev) < 0) {
+		BT_ERR("Can't register HCI device");
+		hci_free_dev(hdev);
+		return -ENODEV;
+	}
+
+	BT_DBG(" HCI device registered. hdev= %p", hdev);
+	return 0;
+}
+
+
+static int bt_ti_probe(struct platform_device *pdev)
+{
+	int err;
+	static struct hci_st *hst;
+	err = 0;
+
+	BT_DBG(" Bluetooth Driver Version %s", VERSION);
+
+	/* Allocate local resource memory */
+	hst = kzalloc(sizeof(struct hci_st), GFP_KERNEL);
+	if (!hst) {
+		BT_ERR("Can't allocate control structure");
+		return -ENFILE;
+	}
+
+	/* Expose "hciX" device to user space */
+	err = hci_st_register_dev(hst);
+	if (err) {
+		/* Release local resource memory */
+		kfree(hst);
+
+		BT_ERR("Unable to expose hci0 device(%d)", err);
+		return err;
+	}
+	set_bit(BT_DRV_RUNNING, &hst->flags);
+	dev_set_drvdata(&pdev->dev, hst);
+	return err;
+}
+
+static int bt_ti_remove(struct platform_device *pdev)
+{
+	struct hci_st *hst;
+
+	hst = dev_get_drvdata(&pdev->dev);
+	/* Deallocate local resource's memory  */
+	if (hst) {
+		struct hci_dev *hdev = hst->hdev;
+		if (hdev == NULL) {
+			BT_ERR("Invalid hdev memory");
+			kfree(hst);
+		} else {
+			hci_st_close(hdev);
+			if (test_and_clear_bit(BT_DRV_RUNNING, &hst->flags)) {
+				/* Remove HCI device (hciX) created
+				 * in module init.
+				 */
+				hci_unregister_dev(hdev);
+				/* Free HCI device memory */
+				hci_free_dev(hdev);
+			}
+		}
+	}
+	return 0;
+}
+
+static struct platform_driver bt_ti_driver = {
+	.probe = bt_ti_probe,
+	.remove = bt_ti_remove,
+	.driver = {
+		.name = "ti_st_bluetooth",
+		.owner = THIS_MODULE,
+	},
+};
+
+/* ------- Module Init/Exit interfaces ------ */
+static int __init bt_drv_init(void)
+{
+	long ret = 0;
+	ret = platform_driver_register(&bt_ti_driver);
+	if (ret != 0) {
+		BT_ERR("bt_ti platform drv registration failed");
+		return -1;
+	}
+	return 0;
+}
+
+static void __exit bt_drv_exit(void)
+{
+	platform_driver_unregister(&bt_ti_driver);
+}
+
+module_init(bt_drv_init);
+module_exit(bt_drv_exit);
+
+/* ------ Module Info ------ */
+
+module_param(reset, bool, 0644);
+MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
+MODULE_AUTHOR("Raja Mani <raja_mani@ti.com>");
+MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
-- 
1.6.5

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Gustavo F. Padovan]

Hi Pavan,

Change the commit subject to "Bluetooth: TI_ST bluetooth driver" 

* pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-07 14:47:16 -0400]:

> From: Pavan Savoy <pavan_savoy@ti.com>
> 
> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> Texas Instrument's WiLink chipsets combine wireless technologies
> like BT, FM, GPS and WLAN onto a single chip.
> 
> This Bluetooth driver works on top of the TI_ST shared transport
> line discipline driver which also allows other drivers like
> FM V4L2 and GPS character driver to make use of the same UART interface.
> 
> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> ---
>  drivers/bluetooth/bt_ti.c |  489 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 489 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/bluetooth/bt_ti.c

We don't have filename with bt_.. in drivers/bluetooth/. Maybe ti_st.c
should be a better name, or something like that.

> 
> diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
> new file mode 100644
> index 0000000..dffbb56
> --- /dev/null
> +++ b/drivers/bluetooth/bt_ti.c
> @@ -0,0 +1,489 @@
> +/*
> + *  Texas Instrument's Bluetooth Driver For Shared Transport.
> + *
> + *  Bluetooth Driver acts as interface between HCI CORE and
> + *  TI Shared Transport Layer.
> + *
> + *  Copyright (C) 2009-2010 Texas Instruments
> + *  Author: Raja Mani <raja_mani@ti.com>
> + *	Pavan Savoy <pavan_savoy@ti.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  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/platform_device.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include <linux/ti_wilink_st.h>
> +
> +/* Bluetooth Driver Version */
> +#define VERSION               "1.0"
> +
> +/* Defines number of seconds to wait for reg completion
> + * callback getting called from ST (in case,registration
> + * with ST returns PENDING status)
> + */
> +#define BT_REGISTER_TIMEOUT   6000	/* 6 sec */
> +
> +/* BT driver's local status */
> +#define BT_DRV_RUNNING        0
> +#define BT_ST_REGISTERED      1
> +
> +/**
> + * struct hci_st - BT driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @flags: used locally,to maintain various BT driver status
> + * @streg_cbdata: to hold ST registration callback status
> + * @st_write: write function pointer of ST driver
> + * @wait_for_btdrv_reg_completion - completion sync between hci_st_open
> + *	and hci_st_registration_completion_cb.
> + */
> +struct hci_st {
> +	struct hci_dev *hdev;
> +	unsigned long flags;
> +	char streg_cbdata;
> +	long (*st_write) (struct sk_buff *);
> +	struct completion wait_for_btdrv_reg_completion;
> +};
> +
> +static int reset;
> +
> +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> +static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type)
> +{
> +	struct hci_dev *hdev;
> +	hdev = hst->hdev;
> +
> +	/* Update HCI stat counters */
> +	switch (pkt_type) {
> +	case HCI_COMMAND_PKT:
> +		hdev->stat.cmd_tx++;
> +		break;
> +
> +	case HCI_ACLDATA_PKT:
> +		hdev->stat.acl_tx++;
> +		break;
> +
> +	case HCI_SCODATA_PKT:
> +		hdev->stat.cmd_tx++;

it should be sco_tx here.

> +		break;
> +	}
> +}
> +
> +/* ------- Interfaces to Shared Transport ------ */
> +
> +/* Called by ST layer to indicate protocol registration completion
> + * status.hci_st_open() function will wait for signal from this
> + * API when st_register() function returns ST_PENDING.
> + */
> +static void hci_st_registration_completion_cb(void *priv_data, char data)

That is not the hci layer, so rename this function (and the others) to
something that reflect where they are really doing.

> +{
> +	struct hci_st *lhst = (struct hci_st *)priv_data;
> +	/* hci_st_open() function needs value of 'data' to know
> +	 * the registration status(success/fail),So have a back
> +	 * up of it.
> +	 */
> +	lhst->streg_cbdata = data;
> +
> +	/* Got a feedback from ST for BT driver registration
> +	 * request.Wackup hci_st_open() function to continue
> +	 * it's open operation.
> +	 */
> +	complete(&lhst->wait_for_btdrv_reg_completion);
> +}
> +
> +/* Called by Shared Transport layer when receive data is
> + * available */
> +static long hci_st_receive(void *priv_data, struct sk_buff *skb)
> +{
> +	int err;
> +	int len;

you can put err and len in the same line.

> +	struct hci_st *lhst = (struct hci_st *)priv_data;
> +
> +	err = 0;
> +	len = 0;

and no need to set them to 0 here.

> +
> +	if (skb == NULL) {
> +		BT_ERR("Invalid SKB received from ST");
> +		return -EFAULT;
> +	}

We need a empty line here.

> +	if (!lhst) {
> +		kfree_skb(skb);
> +		BT_ERR("Invalid hci_st memory,freeing SKB");
> +		return -EFAULT;
> +	}

And also here. Check the rest of the code for similar issues.

> +	if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
> +		kfree_skb(skb);
> +		BT_ERR("Device is not running,freeing SKB");
> +		return -EINVAL;
> +	}

If you are here, your device is running, right? Or am I missing
something?

> +
> +	len = skb->len;
> +	skb->dev = (struct net_device *)lhst->hdev;
> +
> +	/* Forward skb to HCI CORE layer */
> +	err = hci_recv_frame(skb);
> +	if (err) {
> +		kfree_skb(skb);
> +		BT_ERR("Unable to push skb to HCI CORE(%d),freeing SKB",
> +				err);
> +		return err;
> +	}
> +	lhst->hdev->stat.byte_rx += len;

actually you even don't need len, just use skb->len

> +
> +	return 0;
> +}
> +
> +/* ------- Interfaces to HCI layer ------ */
> +
> +/* Called from HCI core to initialize the device */
> +static int hci_st_open(struct hci_dev *hdev)
> +{
> +	static struct st_proto_s hci_st_proto;
> +	unsigned long timeleft;
> +	struct hci_st *hst;
> +	int err;
> +	err = 0;
> +
> +	BT_DBG("%s %p", hdev->name, hdev);
> +	hst = hdev->driver_data;
> +
> +	/* Populate BT driver info required by ST */
> +	memset(&hci_st_proto, 0, sizeof(hci_st_proto));
> +
> +	/* BT driver ID */
> +	hci_st_proto.type = ST_BT;
> +
> +	/* Receive function which called from ST */
> +	hci_st_proto.recv = hci_st_receive;
> +
> +	/* Packet match function may used in future */
> +	hci_st_proto.match_packet = NULL;

It is already NULL, you dua a memset.

> +
> +	/* Callback to be called when registration is pending */
> +	hci_st_proto.reg_complete_cb = hci_st_registration_completion_cb;
> +
> +	/* This is write function pointer of ST. BT driver will make use of this
> +	 * for sending any packets to chip. ST will assign and give to us, so
> +	 * make it as NULL */
> +	hci_st_proto.write = NULL;

Same here.

> +
> +	/* send in the hst to be received at registration complete callback
> +	 * and during st's receive
> +	 */
> +	hci_st_proto.priv_data = hst;
> +
> +	/* Register with ST layer */
> +	err = st_register(&hci_st_proto);
> +	if (err == -EINPROGRESS) {
> +		/* Prepare wait-for-completion handler data structures.
> +		 * Needed to syncronize this and st_registration_completion_cb()
> +		 * functions.
> +		 */
> +		init_completion(&hst->wait_for_btdrv_reg_completion);

I'm not liking that, but I'll leave for Marcel and others comment.

> +
> +		/* Reset ST registration callback status flag , this value
> +		 * will be updated in hci_st_registration_completion_cb()
> +		 * function whenever it called from ST driver.
> +		 */
> +		hst->streg_cbdata = -EINPROGRESS;
> +
> +		/* ST is busy with other protocol registration(may be busy with
> +		 * firmware download).So,Wait till the registration callback
> +		 * (passed as a argument to st_register() function) getting
> +		 * called from ST.
> +		 */
> +		BT_DBG(" %s waiting for reg completion signal from ST",
> +				__func__);
> +
> +		timeleft =
> +			wait_for_completion_timeout
> +			(&hst->wait_for_btdrv_reg_completion,
> +			 msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> +		if (!timeleft) {
> +			BT_ERR("Timeout(%d sec),didn't get reg"
> +					"completion signal from ST",
> +					BT_REGISTER_TIMEOUT / 1000);
> +			return -ETIMEDOUT;
> +		}
> +
> +		/* Is ST registration callback called with ERROR value? */
> +		if (hst->streg_cbdata != 0) {
> +			BT_ERR("ST reg completion CB called with invalid"
> +					"status %d", hst->streg_cbdata);
> +			return -EAGAIN;
> +		}
> +		err = 0;
> +	} else if (err == -1) {

Use the proper error macro instead "-1" 

> +		BT_ERR("st_register failed %d", err);
> +		return -EAGAIN;
> +	}
> +
> +	/* Do we have proper ST write function? */
> +	if (hci_st_proto.write != NULL) {
> +		/* We need this pointer for sending any Bluetooth pkts */
> +		hst->st_write = hci_st_proto.write;
> +	} else {
> +		BT_ERR("failed to get ST write func pointer");
> +
> +		/* Undo registration with ST */
> +		err = st_unregister(ST_BT);
> +		if (err < 0)
> +			BT_ERR("st_unregister failed %d", err);
> +
> +		hst->st_write = NULL;
> +		return -EAGAIN;
> +	}
> +
> +	/* Registration with ST layer is completed successfully,
> +	 * now chip is ready to accept commands from HCI CORE.
> +	 * Mark HCI Device flag as RUNNING
> +	 */
> +	set_bit(HCI_RUNNING, &hdev->flags);
> +
> +	/* Registration with ST successful */
> +	set_bit(BT_ST_REGISTERED, &hst->flags);
> +
> +	return err;
> +}
> +
> +/* Close device */
> +static int hci_st_close(struct hci_dev *hdev)
> +{
> +	int err;
> +	struct hci_st *hst;

Skip a line after declarations.

> +	err = 0;

you can set err to 0 in the declaration if you really need that.

> +
> +	hst = hdev->driver_data;
> +	/* Unregister from ST layer */
> +	if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
> +		err = st_unregister(ST_BT);
> +		if (err != 0) {
> +			BT_ERR("st_unregister failed %d", err);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	hst->st_write = NULL;
> +
> +	/* ST layer would have moved chip to inactive state.
> +	 * So,clear HCI device RUNNING flag.
> +	 */
> +	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> +		return 0;

Looks you are screwing up the flags here, if it fails on st_unregister()
and returns HCI_RUNNING should keep set?

> +
> +	return err;

Rethink how you are doing error handling here, it should no be
complicated like that.

> +}
> +
> +/* Called from HCI CORE , Sends frames to Shared Transport */
> +static int hci_st_send_frame(struct sk_buff *skb)
> +{
> +	struct hci_dev *hdev;
> +	struct hci_st *hst;
> +	long len;
> +
> +	if (skb == NULL) {
> +		BT_ERR("Invalid skb received from HCI CORE");
> +		return -ENOMEM;
> +	}
> +	hdev = (struct hci_dev *)skb->dev;
> +	if (!hdev) {
> +		BT_ERR("SKB received for invalid HCI Device (hdev=NULL)");
> +		return -ENODEV;
> +	}
> +	if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> +		BT_ERR("Device is not running");
> +		return -EBUSY;
> +	}
> +
> +	hst = (struct hci_st *)hdev->driver_data;
> +
> +	/* Prepend skb with frame type */
> +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> +	BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> +			skb->len);
> +
> +	/* Insert skb to shared transport layer's transmit queue.
> +	 * Freeing skb memory is taken care in shared transport layer,
> +	 * so don't free skb memory here.
> +	 */
> +	if (!hst->st_write) {
> +		kfree_skb(skb);
> +		BT_ERR(" Can't write to ST, st_write null?");
> +		return -EAGAIN;
> +	}
> +	len = hst->st_write(skb);
> +	if (len < 0) {
> +		/* Something went wrong in st write , free skb memory */

IMHO we don't need comments like that, clearly we now that something
went wrong.

> +		kfree_skb(skb);
> +		BT_ERR(" ST write failed (%ld)", len);
> +		return -EAGAIN;
> +	}
> +
> +	/* ST accepted our skb. So, Go ahead and do rest */
> +	hdev->stat.byte_tx += len;
> +	hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> +
> +	return 0;

goto might be better to handle error here.


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

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Savoy, Pavan]

Gustavo / Marcel,
=20

> -----Original Message-----
> From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gustavo=
 F.
> Padovan
> Sent: Thursday, October 07, 2010 1:46 PM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org; greg@kroah.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> Change the commit subject to "Bluetooth: TI_ST bluetooth driver"
>=20
> * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-07 14:47:16 -0400]:
>=20
> > From: Pavan Savoy <pavan_savoy@ti.com>
> >
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> >
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface=
.
> >
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> >  drivers/bluetooth/bt_ti.c |  489
> +++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 489 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/bluetooth/bt_ti.c
>=20
> We don't have filename with bt_.. in drivers/bluetooth/. Maybe ti_st.c
> should be a better name, or something like that.
>=20
> >
> > diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
> > new file mode 100644
> > index 0000000..dffbb56
> > --- /dev/null
> > +++ b/drivers/bluetooth/bt_ti.c
> > @@ -0,0 +1,489 @@
> > +/*
> > + *  Texas Instrument's Bluetooth Driver For Shared Transport.
> > + *
> > + *  Bluetooth Driver acts as interface between HCI CORE and
> > + *  TI Shared Transport Layer.
> > + *
> > + *  Copyright (C) 2009-2010 Texas Instruments
> > + *  Author: Raja Mani <raja_mani@ti.com>
> > + *	Pavan Savoy <pavan_savoy@ti.com>
> > + *
> > + *  This program is free software; you can redistribute it and/or modi=
fy
> > + *  it under the terms of the GNU General Public License version 2 as
> > + *  published by the Free Software Foundation.
> > + *
> > + *  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-13=
07
> USA
> > + *
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +
> > +#include <linux/ti_wilink_st.h>
> > +
> > +/* Bluetooth Driver Version */
> > +#define VERSION               "1.0"
> > +
> > +/* Defines number of seconds to wait for reg completion
> > + * callback getting called from ST (in case,registration
> > + * with ST returns PENDING status)
> > + */
> > +#define BT_REGISTER_TIMEOUT   6000	/* 6 sec */
> > +
> > +/* BT driver's local status */
> > +#define BT_DRV_RUNNING        0
> > +#define BT_ST_REGISTERED      1
> > +
> > +/**
> > + * struct hci_st - BT driver operation structure
> > + * @hdev: hci device pointer which binds to bt driver
> > + * @flags: used locally,to maintain various BT driver status
> > + * @streg_cbdata: to hold ST registration callback status
> > + * @st_write: write function pointer of ST driver
> > + * @wait_for_btdrv_reg_completion - completion sync between hci_st_ope=
n
> > + *	and hci_st_registration_completion_cb.
> > + */
> > +struct hci_st {
> > +	struct hci_dev *hdev;
> > +	unsigned long flags;
> > +	char streg_cbdata;
> > +	long (*st_write) (struct sk_buff *);
> > +	struct completion wait_for_btdrv_reg_completion;
> > +};
> > +
> > +static int reset;
> > +
> > +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> > +static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type=
)
> > +{
> > +	struct hci_dev *hdev;
> > +	hdev =3D hst->hdev;
> > +
> > +	/* Update HCI stat counters */
> > +	switch (pkt_type) {
> > +	case HCI_COMMAND_PKT:
> > +		hdev->stat.cmd_tx++;
> > +		break;
> > +
> > +	case HCI_ACLDATA_PKT:
> > +		hdev->stat.acl_tx++;
> > +		break;
> > +
> > +	case HCI_SCODATA_PKT:
> > +		hdev->stat.cmd_tx++;
>=20
> it should be sco_tx here.
>=20
> > +		break;
> > +	}
> > +}
> > +
> > +/* ------- Interfaces to Shared Transport ------ */
> > +
> > +/* Called by ST layer to indicate protocol registration completion
> > + * status.hci_st_open() function will wait for signal from this
> > + * API when st_register() function returns ST_PENDING.
> > + */
> > +static void hci_st_registration_completion_cb(void *priv_data, char da=
ta)
>=20
> That is not the hci layer, so rename this function (and the others) to
> something that reflect where they are really doing.
>=20
> > +{
> > +	struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > +	/* hci_st_open() function needs value of 'data' to know
> > +	 * the registration status(success/fail),So have a back
> > +	 * up of it.
> > +	 */
> > +	lhst->streg_cbdata =3D data;
> > +
> > +	/* Got a feedback from ST for BT driver registration
> > +	 * request.Wackup hci_st_open() function to continue
> > +	 * it's open operation.
> > +	 */
> > +	complete(&lhst->wait_for_btdrv_reg_completion);
> > +}
> > +
> > +/* Called by Shared Transport layer when receive data is
> > + * available */
> > +static long hci_st_receive(void *priv_data, struct sk_buff *skb)
> > +{
> > +	int err;
> > +	int len;
>=20
> you can put err and len in the same line.
>=20
> > +	struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > +
> > +	err =3D 0;
> > +	len =3D 0;
>=20
> and no need to set them to 0 here.
>=20
> > +
> > +	if (skb =3D=3D NULL) {
> > +		BT_ERR("Invalid SKB received from ST");
> > +		return -EFAULT;
> > +	}
>=20
> We need a empty line here.
>=20
> > +	if (!lhst) {
> > +		kfree_skb(skb);
> > +		BT_ERR("Invalid hci_st memory,freeing SKB");
> > +		return -EFAULT;
> > +	}
>=20
> And also here. Check the rest of the code for similar issues.
>=20
> > +	if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
> > +		kfree_skb(skb);
> > +		BT_ERR("Device is not running,freeing SKB");
> > +		return -EINVAL;
> > +	}
>=20
> If you are here, your device is running, right? Or am I missing
> something?
>=20
> > +
> > +	len =3D skb->len;
> > +	skb->dev =3D (struct net_device *)lhst->hdev;
> > +
> > +	/* Forward skb to HCI CORE layer */
> > +	err =3D hci_recv_frame(skb);
> > +	if (err) {
> > +		kfree_skb(skb);
> > +		BT_ERR("Unable to push skb to HCI CORE(%d),freeing SKB",
> > +				err);
> > +		return err;
> > +	}
> > +	lhst->hdev->stat.byte_rx +=3D len;
>=20
> actually you even don't need len, just use skb->len
>=20
> > +
> > +	return 0;
> > +}
> > +
> > +/* ------- Interfaces to HCI layer ------ */
> > +
> > +/* Called from HCI core to initialize the device */
> > +static int hci_st_open(struct hci_dev *hdev)
> > +{
> > +	static struct st_proto_s hci_st_proto;
> > +	unsigned long timeleft;
> > +	struct hci_st *hst;
> > +	int err;
> > +	err =3D 0;
> > +
> > +	BT_DBG("%s %p", hdev->name, hdev);
> > +	hst =3D hdev->driver_data;
> > +
> > +	/* Populate BT driver info required by ST */
> > +	memset(&hci_st_proto, 0, sizeof(hci_st_proto));
> > +
> > +	/* BT driver ID */
> > +	hci_st_proto.type =3D ST_BT;
> > +
> > +	/* Receive function which called from ST */
> > +	hci_st_proto.recv =3D hci_st_receive;
> > +
> > +	/* Packet match function may used in future */
> > +	hci_st_proto.match_packet =3D NULL;
>=20
> It is already NULL, you dua a memset.
>=20
> > +
> > +	/* Callback to be called when registration is pending */
> > +	hci_st_proto.reg_complete_cb =3D hci_st_registration_completion_cb;
> > +
> > +	/* This is write function pointer of ST. BT driver will make use of
> this
> > +	 * for sending any packets to chip. ST will assign and give to us, so
> > +	 * make it as NULL */
> > +	hci_st_proto.write =3D NULL;
>=20
> Same here.
>=20
> > +
> > +	/* send in the hst to be received at registration complete callback
> > +	 * and during st's receive
> > +	 */
> > +	hci_st_proto.priv_data =3D hst;
> > +
> > +	/* Register with ST layer */
> > +	err =3D st_register(&hci_st_proto);
> > +	if (err =3D=3D -EINPROGRESS) {
> > +		/* Prepare wait-for-completion handler data structures.
> > +		 * Needed to syncronize this and st_registration_completion_cb()
> > +		 * functions.
> > +		 */
> > +		init_completion(&hst->wait_for_btdrv_reg_completion);
>=20
> I'm not liking that, but I'll leave for Marcel and others comment.

Thanks for the comments, I will take care of them all, but I want to
tell why I do this here.
"st_register" function takes time. It takes about 2 seconds for us
to download the firmware. (or longer based on UART baud-rate).

This is intended to happen in the process context. Say hciconfig hci0 up
takes a while longer than usual or if someone doing an ioctl(HCIDEVUP)
and this is also the reason I have a callback, as and when firmware downloa=
d
completes, I call the Bluetooth driver's callback.

and I await marcel's comment on this.

> > +
> > +		/* Reset ST registration callback status flag , this value
> > +		 * will be updated in hci_st_registration_completion_cb()
> > +		 * function whenever it called from ST driver.
> > +		 */
> > +		hst->streg_cbdata =3D -EINPROGRESS;
> > +
> > +		/* ST is busy with other protocol registration(may be busy with
> > +		 * firmware download).So,Wait till the registration callback
> > +		 * (passed as a argument to st_register() function) getting
> > +		 * called from ST.
> > +		 */
> > +		BT_DBG(" %s waiting for reg completion signal from ST",
> > +				__func__);
> > +
> > +		timeleft =3D
> > +			wait_for_completion_timeout
> > +			(&hst->wait_for_btdrv_reg_completion,
> > +			 msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> > +		if (!timeleft) {
> > +			BT_ERR("Timeout(%d sec),didn't get reg"
> > +					"completion signal from ST",
> > +					BT_REGISTER_TIMEOUT / 1000);
> > +			return -ETIMEDOUT;
> > +		}
> > +
> > +		/* Is ST registration callback called with ERROR value? */
> > +		if (hst->streg_cbdata !=3D 0) {
> > +			BT_ERR("ST reg completion CB called with invalid"
> > +					"status %d", hst->streg_cbdata);
> > +			return -EAGAIN;
> > +		}
> > +		err =3D 0;
> > +	} else if (err =3D=3D -1) {
>=20
> Use the proper error macro instead "-1"
>=20
> > +		BT_ERR("st_register failed %d", err);
> > +		return -EAGAIN;
> > +	}
> > +
> > +	/* Do we have proper ST write function? */
> > +	if (hci_st_proto.write !=3D NULL) {
> > +		/* We need this pointer for sending any Bluetooth pkts */
> > +		hst->st_write =3D hci_st_proto.write;
> > +	} else {
> > +		BT_ERR("failed to get ST write func pointer");
> > +
> > +		/* Undo registration with ST */
> > +		err =3D st_unregister(ST_BT);
> > +		if (err < 0)
> > +			BT_ERR("st_unregister failed %d", err);
> > +
> > +		hst->st_write =3D NULL;
> > +		return -EAGAIN;
> > +	}
> > +
> > +	/* Registration with ST layer is completed successfully,
> > +	 * now chip is ready to accept commands from HCI CORE.
> > +	 * Mark HCI Device flag as RUNNING
> > +	 */
> > +	set_bit(HCI_RUNNING, &hdev->flags);
> > +
> > +	/* Registration with ST successful */
> > +	set_bit(BT_ST_REGISTERED, &hst->flags);
> > +
> > +	return err;
> > +}
> > +
> > +/* Close device */
> > +static int hci_st_close(struct hci_dev *hdev)
> > +{
> > +	int err;
> > +	struct hci_st *hst;
>=20
> Skip a line after declarations.
>=20
> > +	err =3D 0;
>=20
> you can set err to 0 in the declaration if you really need that.
>=20
> > +
> > +	hst =3D hdev->driver_data;
> > +	/* Unregister from ST layer */
> > +	if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
> > +		err =3D st_unregister(ST_BT);
> > +		if (err !=3D 0) {
> > +			BT_ERR("st_unregister failed %d", err);
> > +			return -EBUSY;
> > +		}
> > +	}
> > +
> > +	hst->st_write =3D NULL;
> > +
> > +	/* ST layer would have moved chip to inactive state.
> > +	 * So,clear HCI device RUNNING flag.
> > +	 */
> > +	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> > +		return 0;
>=20
> Looks you are screwing up the flags here, if it fails on st_unregister()
> and returns HCI_RUNNING should keep set?
>=20
> > +
> > +	return err;
>=20
> Rethink how you are doing error handling here, it should no be
> complicated like that.
>=20
> > +}
> > +
> > +/* Called from HCI CORE , Sends frames to Shared Transport */
> > +static int hci_st_send_frame(struct sk_buff *skb)
> > +{
> > +	struct hci_dev *hdev;
> > +	struct hci_st *hst;
> > +	long len;
> > +
> > +	if (skb =3D=3D NULL) {
> > +		BT_ERR("Invalid skb received from HCI CORE");
> > +		return -ENOMEM;
> > +	}
> > +	hdev =3D (struct hci_dev *)skb->dev;
> > +	if (!hdev) {
> > +		BT_ERR("SKB received for invalid HCI Device (hdev=3DNULL)");
> > +		return -ENODEV;
> > +	}
> > +	if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> > +		BT_ERR("Device is not running");
> > +		return -EBUSY;
> > +	}
> > +
> > +	hst =3D (struct hci_st *)hdev->driver_data;
> > +
> > +	/* Prepend skb with frame type */
> > +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> > +
> > +	BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> > +			skb->len);
> > +
> > +	/* Insert skb to shared transport layer's transmit queue.
> > +	 * Freeing skb memory is taken care in shared transport layer,
> > +	 * so don't free skb memory here.
> > +	 */
> > +	if (!hst->st_write) {
> > +		kfree_skb(skb);
> > +		BT_ERR(" Can't write to ST, st_write null?");
> > +		return -EAGAIN;
> > +	}
> > +	len =3D hst->st_write(skb);
> > +	if (len < 0) {
> > +		/* Something went wrong in st write , free skb memory */
>=20
> IMHO we don't need comments like that, clearly we now that something
> went wrong.
>=20
> > +		kfree_skb(skb);
> > +		BT_ERR(" ST write failed (%ld)", len);
> > +		return -EAGAIN;
> > +	}
> > +
> > +	/* ST accepted our skb. So, Go ahead and do rest */
> > +	hdev->stat.byte_tx +=3D len;
> > +	hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> > +
> > +	return 0;
>=20
> goto might be better to handle error here.
>=20
>=20
> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Marcel Holtmann]

Hi Gustavo,

> Change the commit subject to "Bluetooth: TI_ST bluetooth driver" 
> 
> * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-07 14:47:16 -0400]:
> 
> > From: Pavan Savoy <pavan_savoy@ti.com>
> > 
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> > 
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface.
> > 
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> >  drivers/bluetooth/bt_ti.c |  489 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 489 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/bluetooth/bt_ti.c
> 
> We don't have filename with bt_.. in drivers/bluetooth/. Maybe ti_st.c
> should be a better name, or something like that.

actually we have the bt prefix for company generic ones where they
didn't wanna use the hardware name.

So I prefer to use the hardware for a driver since it is much more clear
that way. You acronym naming here is bad. It is confusing like hell.

What about just calling this btwilink.c or something. I just spinning
ideas here.

Regards

Marcel

RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver

[Savoy, Pavan]

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Friday, October 08, 2010 3:38 AM
> To: Gustavo F. Padovan
> Cc: Savoy, Pavan; linux-bluetooth@vger.kernel.org; greg@kroah.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Gustavo,
>=20
> > Change the commit subject to "Bluetooth: TI_ST bluetooth driver"
> >
> > * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-07 14:47:16 -0400]:
> >
> > > From: Pavan Savoy <pavan_savoy@ti.com>
> > >
> > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > like BT, FM, GPS and WLAN onto a single chip.
> > >
> > > This Bluetooth driver works on top of the TI_ST shared transport
> > > line discipline driver which also allows other drivers like
> > > FM V4L2 and GPS character driver to make use of the same UART interfa=
ce.
> > >
> > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > ---
> > >  drivers/bluetooth/bt_ti.c |  489
> +++++++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 489 insertions(+), 0 deletions(-)
> > >  create mode 100644 drivers/bluetooth/bt_ti.c
> >
> > We don't have filename with bt_.. in drivers/bluetooth/. Maybe ti_st.c
> > should be a better name, or something like that.
>=20
> actually we have the bt prefix for company generic ones where they
> didn't wanna use the hardware name.
>=20
> So I prefer to use the hardware for a driver since it is much more clear
> that way. You acronym naming here is bad. It is confusing like hell.
>=20
> What about just calling this btwilink.c or something. I just spinning
> ideas here.

Ok, I like btwilink.c too. Does bt_wilink.c sound OK?
I have sent a patch with ti_st.c as of now.

Please review, and provide comments. I will incorporate comments and change
Name in the next version of the patch.

Thanks,
Pavan

> Regards
>=20
> Marcel
>=20