* RE: [PATCH 1/2] drivers:staging:ti-st: move TI_ST from staging
From: Savoy, Pavan @ 2010-10-07 20:46 UTC (permalink / raw)
To: Alan Cox
Cc: Jiri Slaby, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org, linux-bluetooth@vger.kernel.org
In-Reply-To: <20101007215943.45365f45@lxorguk.ukuu.org.uk>
> -----Original Message-----
> From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk]
> Sent: Thursday, October 07, 2010 4:00 PM
> To: Savoy, Pavan
> Cc: Jiri Slaby; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@driverdev.osuosl.org; linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH 1/2] drivers:staging:ti-st: move TI_ST from staging
>
> > But, I want to attach my data not when ldsic is opened, but when ldisc is
> registered.
> > I want to begin accessing the data when ldisc is opened.
>
> How can you attach per tty data when the ldisc is registered - the
> relevant tty driver might not even have been loaded at that point. The
> user may not even have been to the shop and bought it even !
>
> What sort of data is this ?
Data related to requesting the user-space to open/install the ldisc.
Imagine a UEVENT structure or PID of the user-space process to which I need
to send a signal .. I currently use rfkill.
> > to be like a bunch of helpers (1 for FM, 1 for GPS, 1 for NFC, 1 for power-
> management), also the problem of who owns the /dev/tty begins to occur,
> Bluetooth has a utility called hciattach, I don't want my FM radio software to
> run hciattach when /dev/radio0 is opened and communicated via FM.
>
> I would have assumed the hotplug script would have run your own attach
> and daemon and the FM radio etc would talk to the ldisc via other kernel
> interfaces it presented.
>
> So whenever the hardware is detected it would load the hardware driver
> The hardware driver would create a tty instance for each physical port
> The hotplug user space would run 'ti-st-attach' as a helper which would
> load the ldisc and set up the bluetooth as well as providing exported
> methods for FM radio etc.
Yes, pretty similar to what I am doing now. I have this daemon which waits
for events from my ldisc driver, and on receiving the notification it
opens the uart, ioctl's the TIOCSETD and allows the tty to be accessed over
the ldisc.
> > In any case, the ti-st/ seems better now by look of things, I certainly
> welcome suggestions to improve it.
> > Also, is there any plan to re-write whole of TTY like a the i2C or the SPI
> bus structure?
> >
> > Here I can imagine, all TTY line disciplines being sort of protocol/client
> drivers, the TTY sub-system in itself would be like the algos driver and then
> > The uart drivers (like 8250.c) can be the adapter drivers.. What say?
>
> They already are, with the one oddity being that something needs to have
> it opened from user space and to attach the ldisc. Thats fixable but hard
> to fix and I'm not aware of any plan to do so - mostly because nobody
> needs it so far.
Yes, that oddity was the reason this notification had to be done.
I could as well have opened it up on boot and attached the ldisc, but I chose
to use it whenever other drivers wanted to - as in when hci0 is UP, or /dev/radio0 is opened.
> Alan
^ permalink raw reply
* Peculiar stuff in hci_ath3k/badness in hci_uart
From: Alan Cox @ 2010-10-07 20:41 UTC (permalink / raw)
To: linux-kernel, marcel, linux-bluetooth
Noticed this while looking at Savoy's stuff
ath_wakeup_ar3k:
int status = tty->driver->ops->tiocmget(tty, NULL);
Now if the tty driver it is bound to happens to use the file pointer or
worse still call into its file->ops badness is going to occur. Doubly fun
is this - a NULL tiocmget is perfectly acceptable and some drivers don't
have one. In the case of serial or usb core using code your backside is
saved by luck. For other hardware well the SELinux page 0 mapping stuff
will save your backside, and probably if enabled the sysctl bit on
current kernels, but if not - oh dear me.
It continues....
(kernel space)
struct termios settings;
n_tty_ioctl_helper(tty, NULL, TCGETS, (unsigned long)&settings);
settings.c_cflag &= ~CRTSCTS;
n_tty_ioctl_helper(tty, NULL, TCSETS, (unsigned long)&settings);
n_tty_ioctl_helper expects a user space pointer.
I've not reviewed it for security impacts. I'd imagine you can oops the
kernel happily with it and maybe more via the NULL pointers if suitable
obscure hardware happens to be present. My gut feeling is that for most
x86 boxes the worst will be an Oops or very bizarre behaviour because they
won't have any suitable serial ports you can hit or have permission to
access.
tiocmget needs a file pointer because some devices check for hangups
here. Now in fact it would actually make sense to move that into the core
code and see if we can kill the file pointer but right now it doesn't
seem safe.
The ioctl helper stuff can either use the ugly set_fs hackery or better
yet shove a helper in the kernel tree to get or set kernel termios which
just takes a struct ktermios and does the right mutex locks and ops calls
checks for NULL ops.
I can find no record of this code having been anywhere near
linux-kernel/linux-serial, which is a pity because after we'd mopped up
our spilled drinks the bug would have been reported.
Fortunately I looked further and the further I looked the more fun it gets
in hci_uart we have
len = tty->ops->write(tty, skb->data, skb->len);
but I can't find anywhere we check that the tty has a write op (a few
don't), and you should check this at open and refuse if the ops you need
don't exist (eg as SLIP does:
static int slip_open(struct tty_struct *tty)
{
struct slip *sl;
int err;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
if (tty->ops->write == NULL)
return -EOPNOTSUPP;
Fortunately almost no system will have a driver loaded which lacks a
write op, but this should be fixed.
Alan
^ permalink raw reply
* Re: [PATCH] Bluetooth: hci_uart: Fix typo in stats for sco tx
From: Gustavo F. Padovan @ 2010-10-07 20:35 UTC (permalink / raw)
To: Karl Beldan; +Cc: Jiri Kosina, linux-bluetooth, Marcel Holtmann
In-Reply-To: <1286481430-6761-1-git-send-email-karl.beldan@gmail.com>
Hi Karl,
* Karl Beldan <karl.beldan@gmail.com> [2010-10-07 21:57:10 +0200]:
> s/stat.cmd_tx++/stat.sco_tx++ for HCI_SCODATA_PKT
>
> Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> ---
> drivers/bluetooth/hci_ldisc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Good catch! Applied, Thanks.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 1/2] drivers:staging:ti-st: move TI_ST from staging
From: Alan Cox @ 2010-10-07 20:35 UTC (permalink / raw)
To: Savoy, Pavan
Cc: Jiri Slaby, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org, linux-bluetooth
In-Reply-To: <19F8576C6E063C45BE387C64729E739404AA21D23B@dbde02.ent.ti.com>
> ldisc is the ONLY way to do it, isn't it? Do I have any other option?
Userspace but even then it wouldn't solve your problem
> The situation was something similar here.
> What I was trying to get to is how we can have a per-device context if a driver is just a line discipline driver?
tty->driver_data
TTY private data
tty->disc_data
LDISC per instance private data
So when your ldisc is opened attach your data to the tty->disc_data, and
when it is closed free the data.
> I have 3 sub-devices if you will on a device which is interfaced over UART,
> One of them is Bluetooth which requires any UART Bluetooth device to have its
> Own line discipline - N_HCI.
The problem is that your chip by the sound of it does not talk the
bluetooth ldisc - it talks something more complex.
The obvious question then is
Does it talk
1. HCI with bits nailed on
2. Something rather different which contains muxed data some of
which is reformatted up to create HCI
In the first case it may be worth seeing if the existing N_HCI could
support forwarding unknown frame types to a helper. In the latter it's a
lot trickier. It is possible to create a mux tty layer (see n_gsm.c) but
that is almost certainly overkill for this.
I wonder what Marcel thinks in terms of re-using the bluetooth ldisc ?
^ permalink raw reply
* RE: [PATCH 1/2] drivers:staging:ti-st: move TI_ST from staging
From: Savoy, Pavan @ 2010-10-07 20:17 UTC (permalink / raw)
To: Alan Cox
Cc: Jiri Slaby, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org, linux-bluetooth@vger.kernel.org
In-Reply-To: <20101007213503.657bcab0@lxorguk.ukuu.org.uk>
> -----Original Message-----
> From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk]
> Sent: Thursday, October 07, 2010 3:35 PM
> To: Savoy, Pavan
> Cc: Jiri Slaby; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@driverdev.osuosl.org; linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH 1/2] drivers:staging:ti-st: move TI_ST from staging
>
> > ldisc is the ONLY way to do it, isn't it? Do I have any other option?
>
> Userspace but even then it wouldn't solve your problem
>
> > The situation was something similar here.
> > What I was trying to get to is how we can have a per-device context if a
> driver is just a line discipline driver?
>
> tty->driver_data
> TTY private data
> tty->disc_data
> LDISC per instance private data
>
> So when your ldisc is opened attach your data to the tty->disc_data, and
> when it is closed free the data.
But, I want to attach my data not when ldsic is opened, but when ldisc is registered.
I want to begin accessing the data when ldisc is opened.
> > I have 3 sub-devices if you will on a device which is interfaced over UART,
> > One of them is Bluetooth which requires any UART Bluetooth device to have
> its
> > Own line discipline - N_HCI.
>
> The problem is that your chip by the sound of it does not talk the
> bluetooth ldisc - it talks something more complex.
>
> The obvious question then is
>
> Does it talk
>
> 1. HCI with bits nailed on
> 2. Something rather different which contains muxed data some of
> which is reformatted up to create HCI
>
> In the first case it may be worth seeing if the existing N_HCI could
> support forwarding unknown frame types to a helper. In the latter it's a
> lot trickier. It is possible to create a mux tty layer (see n_gsm.c) but
> that is almost certainly overkill for this.
>
> I wonder what Marcel thinks in terms of re-using the bluetooth ldisc ?
Yes, Marcel did suggest extending N_HCI, But even then, there need
to be like a bunch of helpers (1 for FM, 1 for GPS, 1 for NFC, 1 for power-management), also the problem of who owns the /dev/tty begins to occur, Bluetooth has a utility called hciattach, I don't want my FM radio software to run hciattach when /dev/radio0 is opened and communicated via FM.
In any case, the ti-st/ seems better now by look of things, I certainly welcome suggestions to improve it.
Also, is there any plan to re-write whole of TTY like a the i2C or the SPI bus structure?
Here I can imagine, all TTY line disciplines being sort of protocol/client drivers, the TTY sub-system in itself would be like the algos driver and then
The uart drivers (like 8250.c) can be the adapter drivers.. What say?
With something like this all I had to do was to write a new client driver.
^ permalink raw reply
* [PATCH] Bluetooth: hci_uart: Fix typo in stats for sco tx
From: Karl Beldan @ 2010-10-07 19:57 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-bluetooth, Karl Beldan, Marcel Holtmann
s/stat.cmd_tx++/stat.sco_tx++ for HCI_SCODATA_PKT
Signed-off-by: Karl Beldan <karl.beldan@gmail.com>
Cc: Marcel Holtmann <marcel@holtmann.org>
---
drivers/bluetooth/hci_ldisc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 998833d..74cb6f3 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -101,7 +101,7 @@ static inline void hci_uart_tx_complete(struct hci_uart *hu, int pkt_type)
break;
case HCI_SCODATA_PKT:
- hdev->stat.cmd_tx++;
+ hdev->stat.sco_tx++;
break;
}
}
--
1.7.1.422.g049e9
^ permalink raw reply related
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Gustavo F. Padovan @ 2010-10-07 19:44 UTC (permalink / raw)
To: Greg KH
Cc: Marcel Holtmann, pavan-savoy, linux-bluetooth, johan.hedberg,
linux-kernel, Pavan Savoy
In-Reply-To: <20101007221737.GA23536@kroah.com>
* 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
^ permalink raw reply
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Gustavo F. Padovan @ 2010-10-07 19:09 UTC (permalink / raw)
To: Greg KH
Cc: Marcel Holtmann, pavan-savoy, linux-bluetooth, johan.hedberg,
linux-kernel, Pavan Savoy
In-Reply-To: <20101007213018.GA28043@kroah.com>
* 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
^ permalink raw reply
* Re: [PATCH 2/2] drivers:bluetooth: Kconfig & Makefile for TI BT
From: Gustavo F. Padovan @ 2010-10-07 18:47 UTC (permalink / raw)
To: pavan_savoy; +Cc: linux-bluetooth, marcel, greg, linux-kernel
In-Reply-To: <1286477237-7526-3-git-send-email-pavan_savoy@ti.com>
* pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-07 14:47:17 -0400]:
> 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 | 10 ++++++++++
> drivers/bluetooth/Makefile | 1 +
> 2 files changed, 11 insertions(+), 0 deletions(-)
Just merge this patch in the firt one. No need to keep this separated.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH 2/2] drivers:bluetooth: Kconfig & Makefile for TI BT
From: pavan_savoy @ 2010-10-07 18:47 UTC (permalink / raw)
To: linux-bluetooth, marcel; +Cc: greg, linux-kernel, Pavan Savoy
In-Reply-To: <1286477237-7526-2-git-send-email-pavan_savoy@ti.com>
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 | 10 ++++++++++
drivers/bluetooth/Makefile | 1 +
2 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 02deef4..936d8ce 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_TI
+ tristate "BlueZ bluetooth driver for TI ST"
+ 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..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
--
1.6.5
^ permalink raw reply related
* [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: pavan_savoy @ 2010-10-07 18:47 UTC (permalink / raw)
To: linux-bluetooth, marcel; +Cc: greg, linux-kernel, Pavan Savoy
In-Reply-To: <1286477237-7526-1-git-send-email-pavan_savoy@ti.com>
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
^ permalink raw reply related
* [PATCH 0/2] v2: Bluetooth driver for TI_ST
From: pavan_savoy @ 2010-10-07 18:47 UTC (permalink / raw)
To: linux-bluetooth, marcel; +Cc: greg, linux-kernel, 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
^ permalink raw reply
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Gustavo F. Padovan @ 2010-10-07 18:45 UTC (permalink / raw)
To: pavan_savoy; +Cc: linux-bluetooth, marcel, greg, linux-kernel
In-Reply-To: <1286477237-7526-2-git-send-email-pavan_savoy@ti.com>
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
^ permalink raw reply
* [PATCH] Add support for Attribute Write Request
From: Anderson Lizardo @ 2010-10-07 18:02 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Implement encoders/decoders for Write Request/Response and the handling
on attribute server. The attribute client still uses the Write Command
because currently SetProperty() has no means to wait for the server
response.
---
attrib/att.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
attrib/att.h | 4 ++++
attrib/gatt.c | 11 +++++++++++
attrib/gatt.h | 3 +++
src/attrib-server.c | 12 +++++++++++-
5 files changed, 79 insertions(+), 1 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index 2ffa8ce..fe41d0e 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -363,6 +363,56 @@ uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
return len;
}
+uint16_t enc_write_req(uint16_t handle, const uint8_t *value, int vlen,
+ uint8_t *pdu, int len)
+{
+ const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle);
+
+ if (pdu == NULL)
+ return 0;
+
+ if (len < min_len)
+ return 0;
+
+ if (vlen > len - min_len)
+ vlen = len - min_len;
+
+ pdu[0] = ATT_OP_WRITE_REQ;
+ att_put_u16(handle, &pdu[1]);
+
+ if (vlen > 0) {
+ memcpy(&pdu[3], value, vlen);
+ return min_len + vlen;
+ }
+
+ return min_len;
+}
+
+uint16_t dec_write_req(const uint8_t *pdu, int len, uint16_t *handle,
+ uint8_t *value, int *vlen)
+{
+ const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
+
+ if (pdu == NULL)
+ return 0;
+
+ if (value == NULL || vlen == NULL || handle == NULL)
+ return 0;
+
+ if (len < min_len)
+ return 0;
+
+ if (pdu[0] != ATT_OP_WRITE_REQ)
+ return 0;
+
+ *handle = att_get_u16(&pdu[1]);
+ *vlen = len - min_len;
+ if (*vlen > 0)
+ memcpy(value, pdu + min_len, *vlen);
+
+ return len;
+}
+
uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle);
diff --git a/attrib/att.h b/attrib/att.h
index 3913f47..ea49dc2 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -178,6 +178,10 @@ uint16_t enc_write_cmd(uint16_t handle, const uint8_t *value, int vlen,
uint16_t dec_write_cmd(const uint8_t *pdu, int len, uint16_t *handle,
uint8_t *value, int *vlen);
struct att_data_list *dec_read_by_type_resp(const uint8_t *pdu, int len);
+uint16_t enc_write_req(uint16_t handle, const uint8_t *value, int vlen,
+ uint8_t *pdu, int len);
+uint16_t dec_write_req(const uint8_t *pdu, int len, uint16_t *handle,
+ uint8_t *value, int *vlen);
uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
diff --git a/attrib/gatt.c b/attrib/gatt.c
index e8171a9..24ec990 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -76,6 +76,17 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
user_data, NULL);
}
+guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
+ int vlen, GAttribResultFunc func, gpointer user_data)
+{
+ uint8_t pdu[ATT_DEFAULT_MTU];
+ guint16 plen;
+
+ plen = enc_write_req(handle, value, vlen, pdu, sizeof(pdu));
+ return g_attrib_send(attrib, ATT_OP_WRITE_REQ, pdu, plen, func,
+ user_data, NULL);
+}
+
guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
GAttribResultFunc func, gpointer user_data)
{
diff --git a/attrib/gatt.h b/attrib/gatt.h
index c99b946..a357f58 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -31,6 +31,9 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
gpointer user_data);
+guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
+ int vlen, GAttribResultFunc func, gpointer user_data);
+
guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
GAttribResultFunc func, gpointer user_data);
diff --git a/src/attrib-server.c b/src/attrib-server.c
index b45f300..666b5fa 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -504,6 +504,17 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
length = find_info(start, end, opdu, channel->mtu);
break;
+ case ATT_OP_WRITE_REQ:
+ length = dec_write_req(ipdu, len, &start, value, &vlen);
+ if (length == 0) {
+ status = ATT_ECODE_INVALID_PDU;
+ goto done;
+ }
+
+ write_value(start, value, vlen);
+ opdu[0] = ATT_OP_WRITE_RESP;
+ length = sizeof(opdu[0]);
+ break;
case ATT_OP_WRITE_CMD:
length = dec_write_cmd(ipdu, len, &start, value, &vlen);
if (length > 0)
@@ -512,7 +523,6 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
case ATT_OP_FIND_BY_TYPE_REQ:
case ATT_OP_READ_BLOB_REQ:
case ATT_OP_READ_MULTI_REQ:
- case ATT_OP_WRITE_REQ:
case ATT_OP_PREP_WRITE_REQ:
case ATT_OP_EXEC_WRITE_REQ:
default:
--
1.7.0.4
^ permalink raw reply related
* RE: RFC: btusb firmware load help
From: Johannes Berg @ 2010-10-07 17:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, linux-bluetooth,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
Deepak Dhamdhere, Sree Durbha
In-Reply-To: <1286465072.6145.151.camel@aeonflux>
On Thu, 2010-10-07 at 17:24 +0200, Marcel Holtmann wrote:
> I am still trying to figure out if this is one stage firmware loading or
> a two stage firmware loading. This is all pretty unclear and nobody has
> answered this clearly so far.
afaict, it's just one stage -- either it has sflash and you load ath3k
firmware over it (the problematic 3002-before-loading case), or it
doesn't have any firmware (and comes up with 3000) and you load the same
ath3k firmware over it with a different mechanism, which currently
announces itself as 3002 but can be changed.
johannes
^ permalink raw reply
* Re: RFC: btusb firmware load help
From: Johannes Berg @ 2010-10-07 17:57 UTC (permalink / raw)
To: Bala Shanmugam
Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, Marcel Holtmann,
linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org, Deepak Dhamdhere, Sree Durbha
In-Reply-To: <4CADF634.1030203@atheros.com>
On Thu, 2010-10-07 at 22:02 +0530, Bala Shanmugam wrote:
> AR3011 when plugged-in uses PID 3000 and control goes to DFU driver [ath3k].
> ath3k downloads the firmware to the device changing its PID to 3002.
> Now btusb gets the control and attaches the device to bluetooth core.
>
> So blacklisting 3002 in btusb will create issues for AR3011 chipsets.
> In firmware if we change the PID from 3002 to 3003 as you suggested and
> blacklist 3002 in btusb we can make both devices work.
Yeah, that still seems like the best approach, or maybe 3004 instead of
3003 if 3003 is already in use by a different chip, or whatever.
johannes
^ permalink raw reply
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Gustavo F. Padovan @ 2010-10-07 17:57 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Greg KH, pavan-savoy, linux-bluetooth, johan.hedberg,
linux-kernel, Pavan Savoy
In-Reply-To: <1286464867.6145.148.camel@aeonflux>
* 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
^ permalink raw reply
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Gustavo F. Padovan @ 2010-10-07 17:51 UTC (permalink / raw)
To: Greg KH
Cc: Marcel Holtmann, pavan-savoy, linux-bluetooth, johan.hedberg,
linux-kernel, Pavan Savoy
In-Reply-To: <20101007143409.GB14913@kroah.com>
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
^ permalink raw reply
* Re: RFC: btusb firmware load help
From: Bala Shanmugam @ 2010-10-07 17:06 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, Johannes Berg,
linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org, Deepak Dhamdhere, Sree Durbha
In-Reply-To: <1286469741.6145.165.camel@aeonflux>
On 10/7/2010 10:12 PM, Marcel Holtmann wrote:
> Hi Bala,
>
>>>> Thanks Johannes. This would be better option to change PID in firmware
>>>> as blacklisting 3002 might create problems for 3011 chipsets.
>>>> Will try and let you people know.
>>> The misbehaving 3002 needs to be blacklisted in btusb.c anyway. However
>>> after loading the firmware to 3002 device, it should change its PID to
>>> something else.
>>>
>>> I am still trying to figure out if this is one stage firmware loading or
>>> a two stage firmware loading. This is all pretty unclear and nobody has
>>> answered this clearly so far.
>> eeprom based 3011 chips comes up with PID 3000 giving control to DFU
>> driver [ath3k]. ath3k downloads the
>> firmware changing PID to 3002. Now btusb gets control.
>>
>> In sflash based devices to reduce windows suspend/resume time we had a
>> small firmware in flash which
>> enables the device to get detected as Generic Bluetooth USB device with
>> PID 3002. So control reaches btusb when device is plugged in, leaving
>> no option for us to load the actual firmware.
>>
>> Solution would be to blacklist 3002 in btusb, enable ath3k to get
>> control for both the devices, download the firmware and change PID to
>> 3003 so that control with come to btusb.
> so here is the thing that needs to be done.
>
> a) Get a firmware for PID 3000 devices that change the firmware to some
> other PID. Since 3003 is already in use as well, using 3004 or later is
> better approach.
>
> b) Blacklist PID 3002 in btusb.c.
>
> c) Handle special firmware loading case for PID 3002 sflash devices. If
> firmware is loaded changed to 3005 or other.
>
> And as a general note, I prefer that the PID after loading firmware is
> different if you don't happen to actually load the same firmware.
>
> So please sort out your USB PID assignment for Bluetooth in general.
> This seems to be a mess that is not thought through properly.
>
> Regards
>
> Marcel
>
>
Thanks for your suggestion Marcel.
Can't we have same PID[3004 or later] for both the devices after loading
the firmware by ath3k?
We need two different firmware if we plan to have two different PIDs for
these two bluetooth devices.
Regards,
Bala.
^ permalink raw reply
* Re: RFC: btusb firmware load help
From: Marcel Holtmann @ 2010-10-07 16:42 UTC (permalink / raw)
To: Bala Shanmugam
Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, Johannes Berg,
linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org, Deepak Dhamdhere, Sree Durbha
In-Reply-To: <4CADF6BF.6070305@atheros.com>
Hi Bala,
> >> Thanks Johannes. This would be better option to change PID in firmware
> >> as blacklisting 3002 might create problems for 3011 chipsets.
> >> Will try and let you people know.
> > The misbehaving 3002 needs to be blacklisted in btusb.c anyway. However
> > after loading the firmware to 3002 device, it should change its PID to
> > something else.
> >
> > I am still trying to figure out if this is one stage firmware loading or
> > a two stage firmware loading. This is all pretty unclear and nobody has
> > answered this clearly so far.
>
> eeprom based 3011 chips comes up with PID 3000 giving control to DFU
> driver [ath3k]. ath3k downloads the
> firmware changing PID to 3002. Now btusb gets control.
>
> In sflash based devices to reduce windows suspend/resume time we had a
> small firmware in flash which
> enables the device to get detected as Generic Bluetooth USB device with
> PID 3002. So control reaches btusb when device is plugged in, leaving
> no option for us to load the actual firmware.
>
> Solution would be to blacklist 3002 in btusb, enable ath3k to get
> control for both the devices, download the firmware and change PID to
> 3003 so that control with come to btusb.
so here is the thing that needs to be done.
a) Get a firmware for PID 3000 devices that change the firmware to some
other PID. Since 3003 is already in use as well, using 3004 or later is
better approach.
b) Blacklist PID 3002 in btusb.c.
c) Handle special firmware loading case for PID 3002 sflash devices. If
firmware is loaded changed to 3005 or other.
And as a general note, I prefer that the PID after loading firmware is
different if you don't happen to actually load the same firmware.
So please sort out your USB PID assignment for Bluetooth in general.
This seems to be a mess that is not thought through properly.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/7] Bluetooth: Add LE connect support
From: Gustavo F. Padovan @ 2010-10-07 16:36 UTC (permalink / raw)
To: Ville Tervo; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <1286449089.6145.91.camel@aeonflux>
Hi Ville,
* Marcel Holtmann <marcel@holtmann.org> [2010-10-07 12:58:09 +0200]:
> Hi Ville,
>
> > Add logic to create LE connections.
Could you be more verbose on the commit message, that way people can
understand better what you are doing in the patch. ;)
> >
> > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> > ---
> > include/net/bluetooth/hci.h | 1 +
> > include/net/bluetooth/hci_core.h | 6 ++-
> > net/bluetooth/hci_conn.c | 38 ++++++++++++++-
> > net/bluetooth/hci_event.c | 100 +++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 141 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index b86aed5..b326240 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -162,6 +162,7 @@ enum {
> > #define SCO_LINK 0x00
> > #define ACL_LINK 0x01
> > #define ESCO_LINK 0x02
> > +#define LE_LINK 0x03
>
> this is not a value defined by the specification, while the others are.
> And some functions match these to HCI event. So if wanna do it like
> this, then using something like 0x80 is better.
A comment in the code saying that is also a good ideia.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: RFC: btusb firmware load help
From: Bala Shanmugam @ 2010-10-07 16:35 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, Johannes Berg,
linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org, Deepak Dhamdhere, Sree Durbha
In-Reply-To: <1286465072.6145.151.camel@aeonflux>
On 10/7/2010 8:54 PM, Marcel Holtmann wrote:
> Hi Bala,
>
>> Thanks Johannes. This would be better option to change PID in firmware
>> as blacklisting 3002 might create problems for 3011 chipsets.
>> Will try and let you people know.
> The misbehaving 3002 needs to be blacklisted in btusb.c anyway. However
> after loading the firmware to 3002 device, it should change its PID to
> something else.
>
> I am still trying to figure out if this is one stage firmware loading or
> a two stage firmware loading. This is all pretty unclear and nobody has
> answered this clearly so far.
>
> Regards
>
> Marcel
>
>
Marcel,
eeprom based 3011 chips comes up with PID 3000 giving control to DFU
driver [ath3k]. ath3k downloads the
firmware changing PID to 3002. Now btusb gets control.
In sflash based devices to reduce windows suspend/resume time we had a
small firmware in flash which
enables the device to get detected as Generic Bluetooth USB device with
PID 3002. So control reaches btusb when device is plugged in, leaving
no option for us to load the actual firmware.
Solution would be to blacklist 3002 in btusb, enable ath3k to get
control for both the devices, download the firmware and change PID to
3003 so that control with come to btusb.
Thanks for your time.
Regards,
Bala.
^ permalink raw reply
* Re: RFC: btusb firmware load help
From: Bala Shanmugam @ 2010-10-07 16:33 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, Johannes Berg,
linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org, Deepak Dhamdhere, Sree Durbha
In-Reply-To: <1286465072.6145.151.camel@aeonflux>
On 10/7/2010 8:54 PM, Marcel Holtmann wrote:
> Hi Bala,
>
>> Thanks Johannes. This would be better option to change PID in firmware
>> as blacklisting 3002 might create problems for 3011 chipsets.
>> Will try and let you people know.
> The misbehaving 3002 needs to be blacklisted in btusb.c anyway. However
> after loading the firmware to 3002 device, it should change its PID to
> something else.
>
> I am still trying to figure out if this is one stage firmware loading or
> a two stage firmware loading. This is all pretty unclear and nobody has
> answered this clearly so far.
>
> Regards
>
> Marcel
>
>
h
^ permalink raw reply
* Re: RFC: btusb firmware load help
From: Bala Shanmugam @ 2010-10-07 16:32 UTC (permalink / raw)
To: Johannes Berg
Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, Marcel Holtmann,
linux-bluetooth, linux-kernel@vger.kernel.org,
linux-wireless@vger.kernel.org, Deepak Dhamdhere, Sree Durbha
In-Reply-To: <1286464555.20974.3.camel@jlt3.sipsolutions.net>
On 10/7/2010 8:45 PM, Johannes Berg wrote:
>> Thanks Johannes. This would be better option to change PID in firmware
>> as blacklisting 3002 might create problems for 3011 chipsets.
> What would be the problem with 3011? Does it also use the 3002 ID, but
> not use firmware upload???
>
> johannes
>
AR3011 when plugged-in uses PID 3000 and control goes to DFU driver [ath3k].
ath3k downloads the firmware to the device changing its PID to 3002.
Now btusb gets the control and attaches the device to bluetooth core.
So blacklisting 3002 in btusb will create issues for AR3011 chipsets.
In firmware if we change the PID from 3002 to 3003 as you suggested and
blacklist 3002 in btusb
we can make both devices work.
Regards,
Bala.
^ permalink raw reply
* Re: [PATCH 1/7] Bluetooth: Add low energy commands and events
From: Gustavo F. Padovan @ 2010-10-07 16:31 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Ville Tervo, linux-bluetooth
In-Reply-To: <1286446105.6145.71.camel@aeonflux>
Hi Marcel,
* Marcel Holtmann <marcel@holtmann.org> [2010-10-07 12:08:25 +0200]:
> Hi Ville,
>
> > Add needed HCI command and event to create LE connections.
> >
> > Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
There is a comment in patch 3/7 from Anderson that may affect this
patch, so I'm going to wait Ville say if he actually want to remove that
piece of the code or not to merge this one in the tree.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox