2010-10-19 22:06:15

by Pavan Savoy

[permalink] [raw]
Subject: [PATCH v4] Bluetooth: btwilink driver

From: Pavan Savoy <[email protected]>

v4 comments

module init now returns what platform_driver_register returns.
type casting of void* private data has been removed

v3 comments

Lizardo,
I have taken care of most of the comments you had.
Have re-wrote some of the code commenting you've mentioned.
Thanks for the comments,

Marcel, Gustavo, & list,
Please review this version of patch.

The other few like -EPERM for platform driver registration is to keep
it similar to other drivers, type casting is maintained just to feel safe
and have style similar to other drivers.
BT_WILINK in Kconfig is similar to BT_MRVL.
I hope those aren't too critical.

-- patch description --

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.

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

Signed-off-by: Pavan Savoy <[email protected]>
---
drivers/bluetooth/Kconfig | 10 +
drivers/bluetooth/Makefile | 1 +
drivers/bluetooth/btwilink.c | 411 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 422 insertions(+), 0 deletions(-)
create mode 100644 drivers/bluetooth/btwilink.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 02deef4..8e0de9a 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -219,4 +219,14 @@ 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_WILINK
+ tristate "Texas Instruments WiLink7 driver"
+ depends on 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..f4460f4 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) += btsdio.o
obj-$(CONFIG_BT_ATH3K) += ath3k.o
obj-$(CONFIG_BT_MRVL) += btmrvl.o
obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
+obj-$(CONFIG_BT_WILINK) += btwilink.o

btmrvl-y := btmrvl_main.o
btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
new file mode 100644
index 0000000..218efd6
--- /dev/null
+++ b/drivers/bluetooth/btwilink.c
@@ -0,0 +1,411 @@
+/*
+ * 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 <[email protected]>
+ * Pavan Savoy <[email protected]>
+ *
+ * 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"
+
+/* Number of seconds to wait for registration completion
+ * when ST returns PENDING status.
+ */
+#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
+
+/**
+ * struct ti_st - driver operation structure
+ * @hdev: hci device pointer which binds to bt driver
+ * @reg_status: ST registration callback status
+ * @st_write: write function provided by the ST driver
+ * to be used by the driver during send_frame.
+ * @wait_reg_completion - completion sync between ti_st_open
+ * and ti_st_registration_completion_cb.
+ */
+struct ti_st {
+ struct hci_dev *hdev;
+ char reg_status;
+ long (*st_write) (struct sk_buff *);
+ struct completion wait_reg_completion;
+};
+
+static int reset;
+
+/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
+static inline void ti_st_tx_complete(struct ti_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.sco_tx++;
+ break;
+ }
+}
+
+/* ------- Interfaces to Shared Transport ------ */
+
+/* Called by ST layer to indicate protocol registration completion
+ * status.ti_st_open() function will wait for signal from this
+ * API when st_register() function returns ST_PENDING.
+ */
+static void st_registration_completion_cb(void *priv_data, char data)
+{
+ struct ti_st *lhst = priv_data;
+
+ /* Save registration status for use in ti_st_open() */
+ lhst->reg_status = data;
+ /* complete the wait in ti_st_open() */
+ complete(&lhst->wait_reg_completion);
+}
+
+/* Called by Shared Transport layer when receive data is
+ * available */
+static long st_receive(void *priv_data, struct sk_buff *skb)
+{
+ int err;
+ struct ti_st *lhst = priv_data;
+
+ if (!skb)
+ return -EFAULT;
+
+ if (!lhst) {
+ kfree_skb(skb);
+ return -EFAULT;
+ }
+
+ 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)", err);
+ return err;
+ }
+
+ lhst->hdev->stat.byte_rx += skb->len;
+
+ return 0;
+}
+
+/* ------- Interfaces to HCI layer ------ */
+/* protocol structure registered with shared transport */
+static struct st_proto_s ti_st_proto = {
+ .type = ST_BT,
+ .recv = st_receive,
+ .reg_complete_cb = st_registration_completion_cb,
+ .priv_data = NULL,
+};
+
+/* Called from HCI core to initialize the device */
+static int ti_st_open(struct hci_dev *hdev)
+{
+ unsigned long timeleft;
+ struct ti_st *hst;
+ int err;
+
+ BT_DBG("%s %p", hdev->name, hdev);
+
+ /* provide contexts for callbacks from ST */
+ hst = hdev->driver_data;
+ ti_st_proto.priv_data = hst;
+
+ err = st_register(&ti_st_proto);
+ if (err == -EINPROGRESS) {
+ /* Prepare wait-for-completion handler data structures.
+ * Needed to synchronize this and
+ * st_registration_completion_cb() functions.
+ */
+ init_completion(&hst->wait_reg_completion);
+
+ /* Reset ST registration callback status flag , this value
+ * will be updated in ti_st_registration_completion_cb()
+ * function whenever it called from ST driver.
+ */
+ hst->reg_status = -EINPROGRESS;
+
+ /* ST is busy with either protocol registration or firmware
+ * download. Wait until the registration callback is called
+ */
+ BT_DBG(" waiting for registration completion signal from ST");
+
+ timeleft = wait_for_completion_timeout
+ (&hst->wait_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 status? */
+ if (hst->reg_status != 0) {
+ BT_ERR("ST registration completed with invalid "
+ "status %d", hst->reg_status);
+ return -EAGAIN;
+ }
+ err = 0;
+ } else if (err == -EPERM) {
+ BT_ERR("st_register failed %d", err);
+ return err;
+ }
+
+ hst->st_write = ti_st_proto.write;
+ if (!hst->st_write) {
+ BT_ERR("undefined ST write function");
+
+ /* Undo registration with ST */
+ err = st_unregister(ST_BT);
+ if (err)
+ BT_ERR("st_unregister() failed with error %d", err);
+
+ hst->st_write = NULL;
+ return err;
+ }
+
+ /* Registration with ST layer is successful,
+ * hardware is ready to accept commands from HCI core.
+ */
+ set_bit(HCI_RUNNING, &hdev->flags);
+
+ return err;
+}
+
+/* Close device */
+static int ti_st_close(struct hci_dev *hdev)
+{
+ int err;
+ struct ti_st *hst = hdev->driver_data;
+
+ /* continue to unregister from transport */
+ err = st_unregister(ST_BT);
+ if (err)
+ BT_ERR("st_unregister() failed with error %d", err);
+
+ hst->st_write = NULL;
+
+ return err;
+}
+
+static int ti_st_send_frame(struct sk_buff *skb)
+{
+ struct hci_dev *hdev;
+ struct ti_st *hst;
+ long len;
+
+ if (!skb)
+ return -ENOMEM;
+
+ hdev = (struct hci_dev *)skb->dev;
+ if (!hdev)
+ return -ENODEV;
+
+ if (!test_bit(HCI_RUNNING, &hdev->flags))
+ return -EBUSY;
+
+ hst = 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(" Could not write to ST (st_write is NULL)");
+ return -EAGAIN;
+ }
+
+ len = hst->st_write(skb);
+ if (len < 0) {
+ 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;
+ ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
+
+ return 0;
+}
+
+static void ti_st_destruct(struct hci_dev *hdev)
+{
+ if (!hdev)
+ return;
+
+ BT_DBG("%s", hdev->name);
+
+ /* free ti_st memory */
+ kfree(hdev->driver_data);
+
+ return;
+}
+
+/* Creates new HCI device */
+static int ti_st_register_dev(struct ti_st *hst)
+{
+ int err;
+ struct hci_dev *hdev;
+
+ /* Initialize and register HCI device */
+ hdev = hci_alloc_dev();
+ if (!hdev)
+ return -ENOMEM;
+
+ BT_DBG("hdev %p", hdev);
+
+ hst->hdev = hdev;
+ hdev->bus = HCI_UART;
+ hdev->driver_data = hst;
+ hdev->open = ti_st_open;
+ hdev->close = ti_st_close;
+ hdev->flush = NULL;
+ hdev->send = ti_st_send_frame;
+ hdev->destruct = ti_st_destruct;
+ hdev->owner = THIS_MODULE;
+
+ if (reset)
+ set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
+
+ err = hci_register_dev(hdev);
+ if (err < 0) {
+ BT_ERR("Can't register HCI device error %d", err);
+ hci_free_dev(hdev);
+ return err;
+ }
+
+ BT_DBG(" HCI device registered (hdev %p)", hdev);
+ return 0;
+}
+
+
+static int bt_ti_probe(struct platform_device *pdev)
+{
+ int err;
+ static struct ti_st *hst;
+
+ BT_DBG(" Bluetooth Driver Version %s", VERSION);
+
+ hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
+ if (!hst)
+ return -ENOMEM;
+
+ /* Expose "hciX" device to user space */
+ err = ti_st_register_dev(hst);
+ if (err) {
+ kfree(hst);
+ return err;
+ }
+
+ dev_set_drvdata(&pdev->dev, hst);
+ return err;
+}
+
+static int bt_ti_remove(struct platform_device *pdev)
+{
+ struct ti_st *hst;
+ struct hci_dev *hdev;
+
+ hst = dev_get_drvdata(&pdev->dev);
+
+ if (!hst)
+ return -EFAULT;
+
+ /* Deallocate local resource's memory */
+ hdev = hst->hdev;
+
+ if (!hdev) {
+ BT_ERR("Invalid hdev memory");
+ kfree(hst);
+ return -EFAULT;
+ }
+
+ ti_st_close(hdev);
+ hci_unregister_dev(hdev);
+ /* Free HCI device memory */
+ hci_free_dev(hdev);
+
+ return 0;
+}
+
+static struct platform_driver btwilink_driver = {
+ .probe = bt_ti_probe,
+ .remove = bt_ti_remove,
+ .driver = {
+ .name = "btwilink",
+ .owner = THIS_MODULE,
+ },
+};
+
+/* ------- Module Init/Exit interfaces ------ */
+static int __init bt_drv_init(void)
+{
+ long ret;
+
+ ret = platform_driver_register(&btwilink_driver);
+ if (ret != 0) {
+ BT_ERR("btwilink platform driver registration failed");
+ return ret;
+ }
+ return 0;
+}
+
+static void __exit bt_drv_exit(void)
+{
+ platform_driver_unregister(&btwilink_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 <[email protected]>");
+MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
--
1.6.5


2010-10-26 15:49:04

by Pavan Savoy

[permalink] [raw]
Subject: RE: [PATCH v4] Bluetooth: btwilink driver


> -----Original Message-----
> From: Pavan Savoy [mailto:[email protected]]
> Sent: Thursday, October 21, 2010 8:26 AM
> To: [email protected]; [email protected]
> Cc: [email protected]; [email protected]; Savoy,
> Pavan
> Subject: Re: [PATCH v4] Bluetooth: btwilink driver
>

Gustavo, Marcel,

Please review and provide comments.
If it is fine, please pull it in.

>
> On Tue, Oct 19, 2010 at 6:06 PM, <[email protected]> wrote:
> > From: Pavan Savoy <[email protected]>
> >
> > v4 comments
> >
> > module init now returns what platform_driver_register returns.
> > type casting of void* private data has been removed
>
> If there are no more critical comments, can it go in?
> Please review.
>
> Thanks,
> Pavan
>
> > v3 comments
> >
> > Lizardo,
> > I have taken care of most of the comments you had.
> > Have re-wrote some of the code commenting you've mentioned.
> > Thanks for the comments,
> >
> > Marcel, Gustavo, & list,
> > Please review this version of patch.
> >
> > The other few like -EPERM for platform driver registration is to keep
> > it similar to other drivers, type casting is maintained just to feel sa=
fe
> > and have style similar to other drivers.
> > BT_WILINK in Kconfig is similar to BT_MRVL.
> > I hope those aren't too critical.
> >
> > -- patch description --
> >
> > 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=
.
> >
> > Kconfig and Makefile modifications to enable the Bluetooth
> > driver for Texas Instrument's WiLink 7 chipset.
> >
> > Signed-off-by: Pavan Savoy <[email protected]>
> > ---
> > drivers/bluetooth/Kconfig | 10 +
> > drivers/bluetooth/Makefile | 1 +
> > drivers/bluetooth/btwilink.c | 411
> ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 422 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/bluetooth/btwilink.c
> >
> > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > index 02deef4..8e0de9a 100644
> > --- a/drivers/bluetooth/Kconfig
> > +++ b/drivers/bluetooth/Kconfig
> > @@ -219,4 +219,14 @@ 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_WILINK
> > + tristate "Texas Instruments WiLink7 driver"
> > + depends on TI_ST
> > + help
> > + This enables the Bluetooth driver for Texas Instrument's BT/F=
M/GPS
> > + combo devices. This makes use of shared transport line discip=
line
> > + 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..f4460f4 100644
> > --- a/drivers/bluetooth/Makefile
> > +++ b/drivers/bluetooth/Makefile
> > @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) +=3D btsdio.o
> > obj-$(CONFIG_BT_ATH3K) +=3D ath3k.o
> > obj-$(CONFIG_BT_MRVL) +=3D btmrvl.o
> > obj-$(CONFIG_BT_MRVL_SDIO) +=3D btmrvl_sdio.o
> > +obj-$(CONFIG_BT_WILINK) +=3D btwilink.o
> >
> > btmrvl-y :=3D btmrvl_main.o
> > btmrvl-$(CONFIG_DEBUG_FS) +=3D btmrvl_debugfs.o
> > diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.=
c
> > new file mode 100644
> > index 0000000..218efd6
> > --- /dev/null
> > +++ b/drivers/bluetooth/btwilink.c
> > @@ -0,0 +1,411 @@
> > +/*
> > + * 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 <[email protected]>
> > + * Pavan Savoy <[email protected]>
> > + *
> > + * 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"
> > +
> > +/* Number of seconds to wait for registration completion
> > + * when ST returns PENDING status.
> > + */
> > +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> > +
> > +/**
> > + * struct ti_st - driver operation structure
> > + * @hdev: hci device pointer which binds to bt driver
> > + * @reg_status: ST registration callback status
> > + * @st_write: write function provided by the ST driver
> > + * to be used by the driver during send_frame.
> > + * @wait_reg_completion - completion sync between ti_st_open
> > + * and ti_st_registration_completion_cb.
> > + */
> > +struct ti_st {
> > + struct hci_dev *hdev;
> > + char reg_status;
> > + long (*st_write) (struct sk_buff *);
> > + struct completion wait_reg_completion;
> > +};
> > +
> > +static int reset;
> > +
> > +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> > +static inline void ti_st_tx_complete(struct ti_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.sco_tx++;
> > + break;
> > + }
> > +}
> > +
> > +/* ------- Interfaces to Shared Transport ------ */
> > +
> > +/* Called by ST layer to indicate protocol registration completion
> > + * status.ti_st_open() function will wait for signal from this
> > + * API when st_register() function returns ST_PENDING.
> > + */
> > +static void st_registration_completion_cb(void *priv_data, char data)
> > +{
> > + struct ti_st *lhst =3D priv_data;
> > +
> > + /* Save registration status for use in ti_st_open() */
> > + lhst->reg_status =3D data;
> > + /* complete the wait in ti_st_open() */
> > + complete(&lhst->wait_reg_completion);
> > +}
> > +
> > +/* Called by Shared Transport layer when receive data is
> > + * available */
> > +static long st_receive(void *priv_data, struct sk_buff *skb)
> > +{
> > + int err;
> > + struct ti_st *lhst =3D priv_data;
> > +
> > + if (!skb)
> > + return -EFAULT;
> > +
> > + if (!lhst) {
> > + kfree_skb(skb);
> > + return -EFAULT;
> > + }
> > +
> > + 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)", err);
> > + return err;
> > + }
> > +
> > + lhst->hdev->stat.byte_rx +=3D skb->len;
> > +
> > + return 0;
> > +}
> > +
> > +/* ------- Interfaces to HCI layer ------ */
> > +/* protocol structure registered with shared transport */
> > +static struct st_proto_s ti_st_proto =3D {
> > + .type =3D ST_BT,
> > + .recv =3D st_receive,
> > + .reg_complete_cb =3D st_registration_completion_cb,
> > + .priv_data =3D NULL,
> > +};
> > +
> > +/* Called from HCI core to initialize the device */
> > +static int ti_st_open(struct hci_dev *hdev)
> > +{
> > + unsigned long timeleft;
> > + struct ti_st *hst;
> > + int err;
> > +
> > + BT_DBG("%s %p", hdev->name, hdev);
> > +
> > + /* provide contexts for callbacks from ST */
> > + hst =3D hdev->driver_data;
> > + ti_st_proto.priv_data =3D hst;
> > +
> > + err =3D st_register(&ti_st_proto);
> > + if (err =3D=3D -EINPROGRESS) {
> > + /* Prepare wait-for-completion handler data structures.
> > + * Needed to synchronize this and
> > + * st_registration_completion_cb() functions.
> > + */
> > + init_completion(&hst->wait_reg_completion);
> > +
> > + /* Reset ST registration callback status flag , this va=
lue
> > + * will be updated in ti_st_registration_completion_cb(=
)
> > + * function whenever it called from ST driver.
> > + */
> > + hst->reg_status =3D -EINPROGRESS;
> > +
> > + /* ST is busy with either protocol registration or firm=
ware
> > + * download. Wait until the registration callback is ca=
lled
> > + */
> > + BT_DBG(" waiting for registration completion signal fro=
m
> ST");
> > +
> > + timeleft =3D wait_for_completion_timeout
> > + (&hst->wait_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 status=
? */
> > + if (hst->reg_status !=3D 0) {
> > + BT_ERR("ST registration completed with invalid =
"
> > + "status %d", hst->reg_status);
> > + return -EAGAIN;
> > + }
> > + err =3D 0;
> > + } else if (err =3D=3D -EPERM) {
> > + BT_ERR("st_register failed %d", err);
> > + return err;
> > + }
> > +
> > + hst->st_write =3D ti_st_proto.write;
> > + if (!hst->st_write) {
> > + BT_ERR("undefined ST write function");
> > +
> > + /* Undo registration with ST */
> > + err =3D st_unregister(ST_BT);
> > + if (err)
> > + BT_ERR("st_unregister() failed with error %d", =
err);
> > +
> > + hst->st_write =3D NULL;
> > + return err;
> > + }
> > +
> > + /* Registration with ST layer is successful,
> > + * hardware is ready to accept commands from HCI core.
> > + */
> > + set_bit(HCI_RUNNING, &hdev->flags);
> > +
> > + return err;
> > +}
> > +
> > +/* Close device */
> > +static int ti_st_close(struct hci_dev *hdev)
> > +{
> > + int err;
> > + struct ti_st *hst =3D hdev->driver_data;
> > +
> > + /* continue to unregister from transport */
> > + err =3D st_unregister(ST_BT);
> > + if (err)
> > + BT_ERR("st_unregister() failed with error %d", err);
> > +
> > + hst->st_write =3D NULL;
> > +
> > + return err;
> > +}
> > +
> > +static int ti_st_send_frame(struct sk_buff *skb)
> > +{
> > + struct hci_dev *hdev;
> > + struct ti_st *hst;
> > + long len;
> > +
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + hdev =3D (struct hci_dev *)skb->dev;
> > + if (!hdev)
> > + return -ENODEV;
> > +
> > + if (!test_bit(HCI_RUNNING, &hdev->flags))
> > + return -EBUSY;
> > +
> > + hst =3D 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(" Could not write to ST (st_write is NULL)");
> > + return -EAGAIN;
> > + }
> > +
> > + len =3D hst->st_write(skb);
> > + if (len < 0) {
> > + 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;
> > + ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> > +
> > + return 0;
> > +}
> > +
> > +static void ti_st_destruct(struct hci_dev *hdev)
> > +{
> > + if (!hdev)
> > + return;
> > +
> > + BT_DBG("%s", hdev->name);
> > +
> > + /* free ti_st memory */
> > + kfree(hdev->driver_data);
> > +
> > + return;
> > +}
> > +
> > +/* Creates new HCI device */
> > +static int ti_st_register_dev(struct ti_st *hst)
> > +{
> > + int err;
> > + struct hci_dev *hdev;
> > +
> > + /* Initialize and register HCI device */
> > + hdev =3D hci_alloc_dev();
> > + if (!hdev)
> > + return -ENOMEM;
> > +
> > + BT_DBG("hdev %p", hdev);
> > +
> > + hst->hdev =3D hdev;
> > + hdev->bus =3D HCI_UART;
> > + hdev->driver_data =3D hst;
> > + hdev->open =3D ti_st_open;
> > + hdev->close =3D ti_st_close;
> > + hdev->flush =3D NULL;
> > + hdev->send =3D ti_st_send_frame;
> > + hdev->destruct =3D ti_st_destruct;
> > + hdev->owner =3D THIS_MODULE;
> > +
> > + if (reset)
> > + set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
> > +
> > + err =3D hci_register_dev(hdev);
> > + if (err < 0) {
> > + BT_ERR("Can't register HCI device error %d", err);
> > + hci_free_dev(hdev);
> > + return err;
> > + }
> > +
> > + BT_DBG(" HCI device registered (hdev %p)", hdev);
> > + return 0;
> > +}
> > +
> > +
> > +static int bt_ti_probe(struct platform_device *pdev)
> > +{
> > + int err;
> > + static struct ti_st *hst;
> > +
> > + BT_DBG(" Bluetooth Driver Version %s", VERSION);
> > +
> > + hst =3D kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> > + if (!hst)
> > + return -ENOMEM;
> > +
> > + /* Expose "hciX" device to user space */
> > + err =3D ti_st_register_dev(hst);
> > + if (err) {
> > + kfree(hst);
> > + return err;
> > + }
> > +
> > + dev_set_drvdata(&pdev->dev, hst);
> > + return err;
> > +}
> > +
> > +static int bt_ti_remove(struct platform_device *pdev)
> > +{
> > + struct ti_st *hst;
> > + struct hci_dev *hdev;
> > +
> > + hst =3D dev_get_drvdata(&pdev->dev);
> > +
> > + if (!hst)
> > + return -EFAULT;
> > +
> > + /* Deallocate local resource's memory */
> > + hdev =3D hst->hdev;
> > +
> > + if (!hdev) {
> > + BT_ERR("Invalid hdev memory");
> > + kfree(hst);
> > + return -EFAULT;
> > + }
> > +
> > + ti_st_close(hdev);
> > + hci_unregister_dev(hdev);
> > + /* Free HCI device memory */
> > + hci_free_dev(hdev);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver btwilink_driver =3D {
> > + .probe =3D bt_ti_probe,
> > + .remove =3D bt_ti_remove,
> > + .driver =3D {
> > + .name =3D "btwilink",
> > + .owner =3D THIS_MODULE,
> > + },
> > +};
> > +
> > +/* ------- Module Init/Exit interfaces ------ */
> > +static int __init bt_drv_init(void)
> > +{
> > + long ret;
> > +
> > + ret =3D platform_driver_register(&btwilink_driver);
> > + if (ret !=3D 0) {
> > + BT_ERR("btwilink platform driver registration failed");
> > + return ret;
> > + }
> > + return 0;
> > +}
> > +
> > +static void __exit bt_drv_exit(void)
> > +{
> > + platform_driver_unregister(&btwilink_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 <[email protected]>");
> > +MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION)=
;
> > +MODULE_VERSION(VERSION);
> > +MODULE_LICENSE("GPL");
> > --
> > 1.6.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetoo=
th"
> in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >

2010-10-21 13:26:09

by Pavan Savoy

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: btwilink driver

Gustavo, Marcel,

On Tue, Oct 19, 2010 at 6:06 PM, <[email protected]> wrote:
> From: Pavan Savoy <[email protected]>
>
> v4 comments
>
> module init now returns what platform_driver_register returns.
> type casting of void* private data has been removed

If there are no more critical comments, can it go in?
Please review.

Thanks,
Pavan

> v3 comments
>
> Lizardo,
> I have taken care of most of the comments you had.
> Have re-wrote some of the code commenting you've mentioned.
> Thanks for the comments,
>
> Marcel, Gustavo, & list,
> Please review this version of patch.
>
> The other few like -EPERM for platform driver registration is to keep
> it similar to other drivers, type casting is maintained just to feel safe
> and have style similar to other drivers.
> BT_WILINK in Kconfig is similar to BT_MRVL.
> I hope those aren't too critical.
>
> -- patch description --
>
> 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.
>
> Kconfig and Makefile modifications to enable the Bluetooth
> driver for Texas Instrument's WiLink 7 chipset.
>
> Signed-off-by: Pavan Savoy <[email protected]>
> ---
> =C2=A0drivers/bluetooth/Kconfig =C2=A0 =C2=A0| =C2=A0 10 +
> =C2=A0drivers/bluetooth/Makefile =C2=A0 | =C2=A0 =C2=A01 +
> =C2=A0drivers/bluetooth/btwilink.c | =C2=A0411 ++++++++++++++++++++++++++=
++++++++++++++++
> =C2=A03 files changed, 422 insertions(+), 0 deletions(-)
> =C2=A0create mode 100644 drivers/bluetooth/btwilink.c
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 02deef4..8e0de9a 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -219,4 +219,14 @@ config BT_ATH3K
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Say Y here to compile support for "Athe=
ros firmware download driver"
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0into the kernel or say M to compile it =
as module (ath3k).
>
> +config BT_WILINK
> + =C2=A0 =C2=A0 =C2=A0 tristate "Texas Instruments WiLink7 driver"
> + =C2=A0 =C2=A0 =C2=A0 depends on TI_ST
> + =C2=A0 =C2=A0 =C2=A0 help
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 This enables the Bluetooth driver for Texas=
Instrument's BT/FM/GPS
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 combo devices. This makes use of shared tra=
nsport line discipline
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 core driver to communicate with the BT core=
of the combo chip.
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 Say Y here to compile support for Texas Ins=
trument's WiLink7 driver
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 into the kernel or say M to compile it as m=
odule.
> =C2=A0endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 71bdf13..f4460f4 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) =C2=A0 =C2=A0+=3D btsdio.o
> =C2=A0obj-$(CONFIG_BT_ATH3K) =C2=A0 =C2=A0 =C2=A0 =C2=A0 +=3D ath3k.o
> =C2=A0obj-$(CONFIG_BT_MRVL) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+=3D btmrvl=
.o
> =C2=A0obj-$(CONFIG_BT_MRVL_SDIO) =C2=A0 =C2=A0 +=3D btmrvl_sdio.o
> +obj-$(CONFIG_BT_WILINK) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0+=3D btwilink.o
>
> =C2=A0btmrvl-y =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 :=3D btmrvl_main.o
> =C2=A0btmrvl-$(CONFIG_DEBUG_FS) =C2=A0 =C2=A0 =C2=A0+=3D btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
> new file mode 100644
> index 0000000..218efd6
> --- /dev/null
> +++ b/drivers/bluetooth/btwilink.c
> @@ -0,0 +1,411 @@
> +/*
> + * =C2=A0Texas Instrument's Bluetooth Driver For Shared Transport.
> + *
> + * =C2=A0Bluetooth Driver acts as interface between HCI core and
> + * =C2=A0TI Shared Transport Layer.
> + *
> + * =C2=A0Copyright (C) 2009-2010 Texas Instruments
> + * =C2=A0Author: Raja Mani <[email protected]>
> + * =C2=A0 =C2=A0 Pavan Savoy <[email protected]>
> + *
> + * =C2=A0This program is free software; you can redistribute it and/or m=
odify
> + * =C2=A0it under the terms of the GNU General Public License version 2 =
as
> + * =C2=A0published by the Free Software Foundation.
> + *
> + * =C2=A0This program is distributed in the hope that it will be useful,
> + * =C2=A0but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * =C2=A0MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. =C2=A0See =
the
> + * =C2=A0GNU General Public License for more details.
> + *
> + * =C2=A0You should have received a copy of the GNU General Public Licen=
se
> + * =C2=A0along with this program; if not, write to the Free Software
> + * =C2=A0Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA =C2=A0=
02111-1307 =C2=A0USA
> + *
> + */
> +
> +#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 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "1.0"
> +
> +/* Number of seconds to wait for registration completion
> + * when ST returns PENDING status.
> + */
> +#define BT_REGISTER_TIMEOUT =C2=A0 6000 =C2=A0 =C2=A0 /* 6 sec */
> +
> +/**
> + * struct ti_st - driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @reg_status: ST registration callback status
> + * @st_write: write function provided by the ST driver
> + * =C2=A0 =C2=A0 to be used by the driver during send_frame.
> + * @wait_reg_completion - completion sync between ti_st_open
> + * =C2=A0 =C2=A0 and ti_st_registration_completion_cb.
> + */
> +struct ti_st {
> + =C2=A0 =C2=A0 =C2=A0 struct hci_dev *hdev;
> + =C2=A0 =C2=A0 =C2=A0 char reg_status;
> + =C2=A0 =C2=A0 =C2=A0 long (*st_write) (struct sk_buff *);
> + =C2=A0 =C2=A0 =C2=A0 struct completion wait_reg_completion;
> +};
> +
> +static int reset;
> +
> +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> +static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_type)
> +{
> + =C2=A0 =C2=A0 =C2=A0 struct hci_dev *hdev;
> + =C2=A0 =C2=A0 =C2=A0 hdev =3D hst->hdev;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Update HCI stat counters */
> + =C2=A0 =C2=A0 =C2=A0 switch (pkt_type) {
> + =C2=A0 =C2=A0 =C2=A0 case HCI_COMMAND_PKT:
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hdev->stat.cmd_tx++;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +
> + =C2=A0 =C2=A0 =C2=A0 case HCI_ACLDATA_PKT:
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hdev->stat.acl_tx++;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +
> + =C2=A0 =C2=A0 =C2=A0 case HCI_SCODATA_PKT:
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hdev->stat.sco_tx++;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> + =C2=A0 =C2=A0 =C2=A0 }
> +}
> +
> +/* ------- Interfaces to Shared Transport ------ */
> +
> +/* Called by ST layer to indicate protocol registration completion
> + * status.ti_st_open() function will wait for signal from this
> + * API when st_register() function returns ST_PENDING.
> + */
> +static void st_registration_completion_cb(void *priv_data, char data)
> +{
> + =C2=A0 =C2=A0 =C2=A0 struct ti_st *lhst =3D priv_data;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Save registration status for use in ti_st_open(=
) */
> + =C2=A0 =C2=A0 =C2=A0 lhst->reg_status =3D data;
> + =C2=A0 =C2=A0 =C2=A0 /* complete the wait in ti_st_open() */
> + =C2=A0 =C2=A0 =C2=A0 complete(&lhst->wait_reg_completion);
> +}
> +
> +/* Called by Shared Transport layer when receive data is
> + * available */
> +static long st_receive(void *priv_data, struct sk_buff *skb)
> +{
> + =C2=A0 =C2=A0 =C2=A0 int err;
> + =C2=A0 =C2=A0 =C2=A0 struct ti_st *lhst =3D priv_data;
> +
> + =C2=A0 =C2=A0 =C2=A0 if (!skb)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EFAULT;
> +
> + =C2=A0 =C2=A0 =C2=A0 if (!lhst) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree_skb(skb);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EFAULT;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 skb->dev =3D (struct net_device *)lhst->hdev;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Forward skb to HCI core layer */
> + =C2=A0 =C2=A0 =C2=A0 err =3D hci_recv_frame(skb);
> + =C2=A0 =C2=A0 =C2=A0 if (err) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree_skb(skb);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("Unable to push=
skb to HCI core(%d)", err);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 lhst->hdev->stat.byte_rx +=3D skb->len;
> +
> + =C2=A0 =C2=A0 =C2=A0 return 0;
> +}
> +
> +/* ------- Interfaces to HCI layer ------ */
> +/* protocol structure registered with shared transport */
> +static struct st_proto_s ti_st_proto =3D {
> + =C2=A0 =C2=A0 =C2=A0 .type =3D ST_BT,
> + =C2=A0 =C2=A0 =C2=A0 .recv =3D st_receive,
> + =C2=A0 =C2=A0 =C2=A0 .reg_complete_cb =3D st_registration_completion_cb=
,
> + =C2=A0 =C2=A0 =C2=A0 .priv_data =3D NULL,
> +};
> +
> +/* Called from HCI core to initialize the device */
> +static int ti_st_open(struct hci_dev *hdev)
> +{
> + =C2=A0 =C2=A0 =C2=A0 unsigned long timeleft;
> + =C2=A0 =C2=A0 =C2=A0 struct ti_st *hst;
> + =C2=A0 =C2=A0 =C2=A0 int err;
> +
> + =C2=A0 =C2=A0 =C2=A0 BT_DBG("%s %p", hdev->name, hdev);
> +
> + =C2=A0 =C2=A0 =C2=A0 /* provide contexts for callbacks from ST */
> + =C2=A0 =C2=A0 =C2=A0 hst =3D hdev->driver_data;
> + =C2=A0 =C2=A0 =C2=A0 ti_st_proto.priv_data =3D hst;
> +
> + =C2=A0 =C2=A0 =C2=A0 err =3D st_register(&ti_st_proto);
> + =C2=A0 =C2=A0 =C2=A0 if (err =3D=3D -EINPROGRESS) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Prepare wait-for-co=
mpletion handler data structures.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* Needed to sync=
hronize this and
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* st_registratio=
n_completion_cb() functions.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 init_completion(&hst->=
wait_reg_completion);
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Reset ST registrati=
on callback status flag , this value
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* will be update=
d in ti_st_registration_completion_cb()
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* function whene=
ver it called from ST driver.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->reg_status =3D -E=
INPROGRESS;
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* ST is busy with eit=
her protocol registration or firmware
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* download. Wait=
until the registration callback is called
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_DBG(" waiting for r=
egistration completion signal from ST");
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timeleft =3D wait_for_=
completion_timeout
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 (&hst->wait_reg_completion,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!timeleft) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 BT_ERR("Timeout(%d sec),didn't get reg "
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "completion =
signal from ST",
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_REGISTER_=
TIMEOUT / 1000);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 return -ETIMEDOUT;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Is ST registration =
callback called with ERROR status? */
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (hst->reg_status !=
=3D 0) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 BT_ERR("ST registration completed with invalid "
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "status %d",=
hst->reg_status);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 return -EAGAIN;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D 0;
> + =C2=A0 =C2=A0 =C2=A0 } else if (err =3D=3D -EPERM) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("st_register fa=
iled %d", err);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D ti_st_proto.write;
> + =C2=A0 =C2=A0 =C2=A0 if (!hst->st_write) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("undefined ST w=
rite function");
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Undo registration w=
ith ST */
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D st_unregister(=
ST_BT);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 BT_ERR("st_unregister() failed with error %d", err);
> +
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D NULL=
;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Registration with ST layer is successful,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0* hardware is ready to accept commands from =
HCI core.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
> + =C2=A0 =C2=A0 =C2=A0 set_bit(HCI_RUNNING, &hdev->flags);
> +
> + =C2=A0 =C2=A0 =C2=A0 return err;
> +}
> +
> +/* Close device */
> +static int ti_st_close(struct hci_dev *hdev)
> +{
> + =C2=A0 =C2=A0 =C2=A0 int err;
> + =C2=A0 =C2=A0 =C2=A0 struct ti_st *hst =3D hdev->driver_data;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* continue to unregister from transport */
> + =C2=A0 =C2=A0 =C2=A0 err =3D st_unregister(ST_BT);
> + =C2=A0 =C2=A0 =C2=A0 if (err)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("st_unregister(=
) failed with error %d", err);
> +
> + =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D NULL;
> +
> + =C2=A0 =C2=A0 =C2=A0 return err;
> +}
> +
> +static int ti_st_send_frame(struct sk_buff *skb)
> +{
> + =C2=A0 =C2=A0 =C2=A0 struct hci_dev *hdev;
> + =C2=A0 =C2=A0 =C2=A0 struct ti_st *hst;
> + =C2=A0 =C2=A0 =C2=A0 long len;
> +
> + =C2=A0 =C2=A0 =C2=A0 if (!skb)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -ENOMEM;
> +
> + =C2=A0 =C2=A0 =C2=A0 hdev =3D (struct hci_dev *)skb->dev;
> + =C2=A0 =C2=A0 =C2=A0 if (!hdev)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -ENODEV;
> +
> + =C2=A0 =C2=A0 =C2=A0 if (!test_bit(HCI_RUNNING, &hdev->flags))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EBUSY;
> +
> + =C2=A0 =C2=A0 =C2=A0 hst =3D hdev->driver_data;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Prepend skb with frame type */
> + =C2=A0 =C2=A0 =C2=A0 memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1)=
;
> +
> + =C2=A0 =C2=A0 =C2=A0 BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(sk=
b)->pkt_type,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 skb->len);
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Insert skb to shared transport layer's transmit=
queue.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0* Freeing skb memory is taken care in shared=
transport layer,
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0* so don't free skb memory here.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
> + =C2=A0 =C2=A0 =C2=A0 if (!hst->st_write) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree_skb(skb);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR(" Could not wri=
te to ST (st_write is NULL)");
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EAGAIN;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 len =3D hst->st_write(skb);
> + =C2=A0 =C2=A0 =C2=A0 if (len < 0) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree_skb(skb);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR(" ST write fail=
ed (%ld)", len);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EAGAIN;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 /* ST accepted our skb. So, Go ahead and do rest *=
/
> + =C2=A0 =C2=A0 =C2=A0 hdev->stat.byte_tx +=3D len;
> + =C2=A0 =C2=A0 =C2=A0 ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> +
> + =C2=A0 =C2=A0 =C2=A0 return 0;
> +}
> +
> +static void ti_st_destruct(struct hci_dev *hdev)
> +{
> + =C2=A0 =C2=A0 =C2=A0 if (!hdev)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +
> + =C2=A0 =C2=A0 =C2=A0 BT_DBG("%s", hdev->name);
> +
> + =C2=A0 =C2=A0 =C2=A0 /* free ti_st memory */
> + =C2=A0 =C2=A0 =C2=A0 kfree(hdev->driver_data);
> +
> + =C2=A0 =C2=A0 =C2=A0 return;
> +}
> +
> +/* Creates new HCI device */
> +static int ti_st_register_dev(struct ti_st *hst)
> +{
> + =C2=A0 =C2=A0 =C2=A0 int err;
> + =C2=A0 =C2=A0 =C2=A0 struct hci_dev *hdev;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Initialize and register HCI device */
> + =C2=A0 =C2=A0 =C2=A0 hdev =3D hci_alloc_dev();
> + =C2=A0 =C2=A0 =C2=A0 if (!hdev)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -ENOMEM;
> +
> + =C2=A0 =C2=A0 =C2=A0 BT_DBG("hdev %p", hdev);
> +
> + =C2=A0 =C2=A0 =C2=A0 hst->hdev =3D hdev;
> + =C2=A0 =C2=A0 =C2=A0 hdev->bus =3D HCI_UART;
> + =C2=A0 =C2=A0 =C2=A0 hdev->driver_data =3D hst;
> + =C2=A0 =C2=A0 =C2=A0 hdev->open =3D ti_st_open;
> + =C2=A0 =C2=A0 =C2=A0 hdev->close =3D ti_st_close;
> + =C2=A0 =C2=A0 =C2=A0 hdev->flush =3D NULL;
> + =C2=A0 =C2=A0 =C2=A0 hdev->send =3D ti_st_send_frame;
> + =C2=A0 =C2=A0 =C2=A0 hdev->destruct =3D ti_st_destruct;
> + =C2=A0 =C2=A0 =C2=A0 hdev->owner =3D THIS_MODULE;
> +
> + =C2=A0 =C2=A0 =C2=A0 if (reset)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 set_bit(HCI_QUIRK_NO_R=
ESET, &hdev->quirks);
> +
> + =C2=A0 =C2=A0 =C2=A0 err =3D hci_register_dev(hdev);
> + =C2=A0 =C2=A0 =C2=A0 if (err < 0) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("Can't register=
HCI device error %d", err);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hci_free_dev(hdev);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 BT_DBG(" HCI device registered (hdev %p)", hdev);
> + =C2=A0 =C2=A0 =C2=A0 return 0;
> +}
> +
> +
> +static int bt_ti_probe(struct platform_device *pdev)
> +{
> + =C2=A0 =C2=A0 =C2=A0 int err;
> + =C2=A0 =C2=A0 =C2=A0 static struct ti_st *hst;
> +
> + =C2=A0 =C2=A0 =C2=A0 BT_DBG(" Bluetooth Driver Version %s", VERSION);
> +
> + =C2=A0 =C2=A0 =C2=A0 hst =3D kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> + =C2=A0 =C2=A0 =C2=A0 if (!hst)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -ENOMEM;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Expose "hciX" device to user space */
> + =C2=A0 =C2=A0 =C2=A0 err =3D ti_st_register_dev(hst);
> + =C2=A0 =C2=A0 =C2=A0 if (err) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree(hst);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 dev_set_drvdata(&pdev->dev, hst);
> + =C2=A0 =C2=A0 =C2=A0 return err;
> +}
> +
> +static int bt_ti_remove(struct platform_device *pdev)
> +{
> + =C2=A0 =C2=A0 =C2=A0 struct ti_st *hst;
> + =C2=A0 =C2=A0 =C2=A0 struct hci_dev *hdev;
> +
> + =C2=A0 =C2=A0 =C2=A0 hst =3D dev_get_drvdata(&pdev->dev);
> +
> + =C2=A0 =C2=A0 =C2=A0 if (!hst)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EFAULT;
> +
> + =C2=A0 =C2=A0 =C2=A0 /* Deallocate local resource's memory =C2=A0*/
> + =C2=A0 =C2=A0 =C2=A0 hdev =3D hst->hdev;
> +
> + =C2=A0 =C2=A0 =C2=A0 if (!hdev) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("Invalid hdev m=
emory");
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree(hst);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EFAULT;
> + =C2=A0 =C2=A0 =C2=A0 }
> +
> + =C2=A0 =C2=A0 =C2=A0 ti_st_close(hdev);
> + =C2=A0 =C2=A0 =C2=A0 hci_unregister_dev(hdev);
> + =C2=A0 =C2=A0 =C2=A0 /* Free HCI device memory */
> + =C2=A0 =C2=A0 =C2=A0 hci_free_dev(hdev);
> +
> + =C2=A0 =C2=A0 =C2=A0 return 0;
> +}
> +
> +static struct platform_driver btwilink_driver =3D {
> + =C2=A0 =C2=A0 =C2=A0 .probe =3D bt_ti_probe,
> + =C2=A0 =C2=A0 =C2=A0 .remove =3D bt_ti_remove,
> + =C2=A0 =C2=A0 =C2=A0 .driver =3D {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .name =3D "btwilink",
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .owner =3D THIS_MODULE=
,
> + =C2=A0 =C2=A0 =C2=A0 },
> +};
> +
> +/* ------- Module Init/Exit interfaces ------ */
> +static int __init bt_drv_init(void)
> +{
> + =C2=A0 =C2=A0 =C2=A0 long ret;
> +
> + =C2=A0 =C2=A0 =C2=A0 ret =3D platform_driver_register(&btwilink_driver)=
;
> + =C2=A0 =C2=A0 =C2=A0 if (ret !=3D 0) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("btwilink platf=
orm driver registration failed");
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return ret;
> + =C2=A0 =C2=A0 =C2=A0 }
> + =C2=A0 =C2=A0 =C2=A0 return 0;
> +}
> +
> +static void __exit bt_drv_exit(void)
> +{
> + =C2=A0 =C2=A0 =C2=A0 platform_driver_unregister(&btwilink_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 <[email protected]>");
> +MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
> +MODULE_VERSION(VERSION);
> +MODULE_LICENSE("GPL");
> --
> 1.6.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
>

2010-11-10 06:08:48

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: btwilink driver

Hi Pavan,

> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 02deef4..8e0de9a 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -219,4 +219,14 @@ 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_WILINK
> + tristate "Texas Instruments WiLink7 driver"
> + depends on 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..f4460f4 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) += btsdio.o
> obj-$(CONFIG_BT_ATH3K) += ath3k.o
> obj-$(CONFIG_BT_MRVL) += btmrvl.o
> obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
> +obj-$(CONFIG_BT_WILINK) += btwilink.o
>
> btmrvl-y := btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
> new file mode 100644
> index 0000000..218efd6
> --- /dev/null
> +++ b/drivers/bluetooth/btwilink.c
> @@ -0,0 +1,411 @@
> +/*
> + * 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 <[email protected]>
> + * Pavan Savoy <[email protected]>
> + *
> + * 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"
> +
> +/* Number of seconds to wait for registration completion
> + * when ST returns PENDING status.
> + */
> +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> +
> +/**
> + * struct ti_st - driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @reg_status: ST registration callback status
> + * @st_write: write function provided by the ST driver
> + * to be used by the driver during send_frame.
> + * @wait_reg_completion - completion sync between ti_st_open
> + * and ti_st_registration_completion_cb.
> + */
> +struct ti_st {
> + struct hci_dev *hdev;
> + char reg_status;
> + long (*st_write) (struct sk_buff *);
> + struct completion wait_reg_completion;
> +};
> +
> +static int reset;
> +
> +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> +static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_type)
> +{
> + struct hci_dev *hdev;
> + hdev = hst->hdev;

please do this properly. Just write it like this:

struct hci_dev *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.sco_tx++;
> + break;
> + }
> +}
> +
> +/* ------- Interfaces to Shared Transport ------ */
> +
> +/* Called by ST layer to indicate protocol registration completion
> + * status.ti_st_open() function will wait for signal from this
> + * API when st_register() function returns ST_PENDING.
> + */
> +static void st_registration_completion_cb(void *priv_data, char data)
> +{
> + struct ti_st *lhst = priv_data;
> +
> + /* Save registration status for use in ti_st_open() */
> + lhst->reg_status = data;
> + /* complete the wait in ti_st_open() */
> + complete(&lhst->wait_reg_completion);
> +}
> +
> +/* Called by Shared Transport layer when receive data is
> + * available */
> +static long st_receive(void *priv_data, struct sk_buff *skb)
> +{
> + int err;
> + struct ti_st *lhst = priv_data;

I really prefer if the variable with the assignment comes first.

> + if (!skb)
> + return -EFAULT;
> +
> + if (!lhst) {
> + kfree_skb(skb);
> + return -EFAULT;
> + }
> +
> + skb->dev = (struct net_device *)lhst->hdev;

Don't do this cast. See the other drivers where we just use (void *)
cast.

> + /* 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)", err);
> + return err;
> + }

So first of all, I prefer if you check like this:

if (err < 0) {

And then second, you are double freeing the SKB here. The hci_recv_frame
will free the SKB in an error case.

> +
> + lhst->hdev->stat.byte_rx += skb->len;
> +
> + return 0;
> +}
> +
> +/* ------- Interfaces to HCI layer ------ */
> +/* protocol structure registered with shared transport */
> +static struct st_proto_s ti_st_proto = {
> + .type = ST_BT,
> + .recv = st_receive,
> + .reg_complete_cb = st_registration_completion_cb,
> + .priv_data = NULL,
> +};

Please don't bother with NULL assignment. It should not be needed.

> +/* Called from HCI core to initialize the device */
> +static int ti_st_open(struct hci_dev *hdev)
> +{
> + unsigned long timeleft;
> + struct ti_st *hst;
> + int err;
> +
> + BT_DBG("%s %p", hdev->name, hdev);
> +
> + /* provide contexts for callbacks from ST */
> + hst = hdev->driver_data;
> + ti_st_proto.priv_data = hst;
> +
> + err = st_register(&ti_st_proto);
> + if (err == -EINPROGRESS) {
> + /* Prepare wait-for-completion handler data structures.
> + * Needed to synchronize this and
> + * st_registration_completion_cb() functions.
> + */
> + init_completion(&hst->wait_reg_completion);
> +
> + /* Reset ST registration callback status flag , this value
> + * will be updated in ti_st_registration_completion_cb()
> + * function whenever it called from ST driver.
> + */
> + hst->reg_status = -EINPROGRESS;
> +
> + /* ST is busy with either protocol registration or firmware
> + * download. Wait until the registration callback is called
> + */
> + BT_DBG(" waiting for registration completion signal from ST");
> +
> + timeleft = wait_for_completion_timeout
> + (&hst->wait_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 status? */
> + if (hst->reg_status != 0) {
> + BT_ERR("ST registration completed with invalid "
> + "status %d", hst->reg_status);
> + return -EAGAIN;
> + }
> + err = 0;
> + } else if (err == -EPERM) {
> + BT_ERR("st_register failed %d", err);
> + return err;
> + }
> +
> + hst->st_write = ti_st_proto.write;
> + if (!hst->st_write) {
> + BT_ERR("undefined ST write function");
> +
> + /* Undo registration with ST */
> + err = st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister() failed with error %d", err);
> +
> + hst->st_write = NULL;
> + return err;
> + }
> +
> + /* Registration with ST layer is successful,
> + * hardware is ready to accept commands from HCI core.
> + */
> + set_bit(HCI_RUNNING, &hdev->flags);
> +
> + return err;
> +}

I really don't like what you are doing here. So please use
test_and_set_bit and clear it in an error case.

Also you need to handle all error cases. Just not only two.

Where is the ti_st_proto.write coming from?

> +
> +/* Close device */
> +static int ti_st_close(struct hci_dev *hdev)
> +{
> + int err;
> + struct ti_st *hst = hdev->driver_data;
> +
> + /* continue to unregister from transport */
> + err = st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister() failed with error %d", err);
> +
> + hst->st_write = NULL;
> +
> + return err;
> +}

You need a test_and_clear_bit for HCI_RUNNING. There is a huge imbalance
here. Have you tested this with consecutive hciconfig hci0 up/down
executions actually?

> +static int ti_st_send_frame(struct sk_buff *skb)
> +{
> + struct hci_dev *hdev;
> + struct ti_st *hst;
> + long len;
> +
> + if (!skb)
> + return -ENOMEM;

Pointless check. The core will not call this function with a NULL
pointer SKB.

> +
> + hdev = (struct hci_dev *)skb->dev;
> + if (!hdev)
> + return -ENODEV;

Even this can't really happen. Have you seen such a case?

> + if (!test_bit(HCI_RUNNING, &hdev->flags))
> + return -EBUSY;
> +
> + hst = 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(" Could not write to ST (st_write is NULL)");
> + return -EAGAIN;
> + }

I don't like these crappy checks on every packet. That is just stupid.
You have checked for st_write when open happens and you set the hdev to
HCI_RUNNING. Are you saying this could change during the lifetime of the
hdev? If so then you have a serious problem here.

> + len = hst->st_write(skb);
> + if (len < 0) {
> + 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;
> + ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> +
> + return 0;
> +}

What is the reason for this deferred stats update. That code looks
pretty much hackish to me.

> +static void ti_st_destruct(struct hci_dev *hdev)
> +{
> + if (!hdev)
> + return;
> +
> + BT_DBG("%s", hdev->name);
> +
> + /* free ti_st memory */
> + kfree(hdev->driver_data);
> +
> + return;
> +}

What are you checking here for? Why do you think that hdev would not be
valid? This is what the btusb and btsdio drivers do:

static void btusb_destruct(struct hci_dev *hdev)
{
struct btusb_data *data = hdev->driver_data;

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

kfree(data);
}


> +/* Creates new HCI device */
> +static int ti_st_register_dev(struct ti_st *hst)
> +{
> + int err;
> + struct hci_dev *hdev;

I prefer if err is last in the variable list.

> +
> + /* Initialize and register HCI device */
> + hdev = hci_alloc_dev();
> + if (!hdev)
> + return -ENOMEM;
> +
> + BT_DBG("hdev %p", hdev);
> +
> + hst->hdev = hdev;
> + hdev->bus = HCI_UART;
> + hdev->driver_data = hst;
> + hdev->open = ti_st_open;
> + hdev->close = ti_st_close;
> + hdev->flush = NULL;

Please implement a flush callback.

> + hdev->send = ti_st_send_frame;
> + hdev->destruct = ti_st_destruct;
> + hdev->owner = THIS_MODULE;
> +
> + if (reset)
> + set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);

Why do you need this? This should only be crappy devices. Something like
Bluetooth 1.0b old devices.

> + err = hci_register_dev(hdev);
> + if (err < 0) {
> + BT_ERR("Can't register HCI device error %d", err);
> + hci_free_dev(hdev);
> + return err;
> + }
> +
> + BT_DBG(" HCI device registered (hdev %p)", hdev);
> + return 0;
> +}
> +
> +

No double empty lines please.

> +static int bt_ti_probe(struct platform_device *pdev)
> +{
> + int err;
> + static struct ti_st *hst;

See above.

> +
> + BT_DBG(" Bluetooth Driver Version %s", VERSION);

This should be in the module_init function. And should be a BT_INFO and
be precise what driver this actually this.

> +
> + hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> + if (!hst)
> + return -ENOMEM;
> +
> + /* Expose "hciX" device to user space */
> + err = ti_st_register_dev(hst);
> + if (err) {
> + kfree(hst);
> + return err;
> + }

Is this ti_st_register device use anywhere else. Then please just
include that code in here to make this clear. All other drivers do all
the work in their probe() callback.

> +
> + dev_set_drvdata(&pdev->dev, hst);
> + return err;
> +}
> +
> +static int bt_ti_remove(struct platform_device *pdev)
> +{
> + struct ti_st *hst;
> + struct hci_dev *hdev;
> +
> + hst = dev_get_drvdata(&pdev->dev);

Here I would prefer this:

struct ti_st *hst = dev_get_drvdata(&pdev->dev);

> +
> + if (!hst)
> + return -EFAULT;
> +
> + /* Deallocate local resource's memory */
> + hdev = hst->hdev;

That comment doesn't match what you are doing here.

> +
> + if (!hdev) {
> + BT_ERR("Invalid hdev memory");
> + kfree(hst);
> + return -EFAULT;
> + }

No need to check for hdev here. If probe fails, then remove should never
be called, right?

And just to be safe you might wanna add this:

dev_set_drvdata(&pdev->dev, NULL);

> +
> + ti_st_close(hdev);
> + hci_unregister_dev(hdev);
> + /* Free HCI device memory */
> + hci_free_dev(hdev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver btwilink_driver = {
> + .probe = bt_ti_probe,
> + .remove = bt_ti_remove,
> + .driver = {
> + .name = "btwilink",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +/* ------- Module Init/Exit interfaces ------ */
> +static int __init bt_drv_init(void)
> +{
> + long ret;
> +
> + ret = platform_driver_register(&btwilink_driver);
> + if (ret != 0) {
> + BT_ERR("btwilink platform driver registration failed");
> + return ret;
> + }
> + return 0;
> +}

please just do like we do with all other drivers;

BT_INFO(...)

return platform_driver_register(&btwilink_driver);

> +
> +static void __exit bt_drv_exit(void)
> +{
> + platform_driver_unregister(&btwilink_driver);
> +}
> +
> +module_init(bt_drv_init);
> +module_exit(bt_drv_exit);

And this should be btwilink_init and btwilink_exit. Please don't try to
grab some generic namespace.

> +
> +/* ------ Module Info ------ */
> +
> +module_param(reset, bool, 0644);
> +MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");

As mentioned above, that one seems wrong to me. You need to know what
your device supports. And by default it should allow sending HCI_Reset
at init. If not, then just that quirk. No need for module parameter
here.

> +MODULE_AUTHOR("Raja Mani <[email protected]>");
> +MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
> +MODULE_VERSION(VERSION);
> +MODULE_LICENSE("GPL");

Regards

Marcel