* 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
* [PATCH] Sort TODO by priority
From: Luiz Augusto von Dentz @ 2010-10-07 16:21 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Items shall be sorted by priority and latter by complexity so that tasks
with higher priority and lower complexity always appear on top.
---
TODO | 58 +++++++++++++++++++++++++++++-----------------------------
1 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/TODO b/TODO
index 271fc45..3885c78 100644
--- a/TODO
+++ b/TODO
@@ -25,6 +25,29 @@ ATT/GATT
Complexity: C2
Owner: Claudio Takahasi <claudio.takahasi@openbossa.org>
+- Add ATT/GATT parsing to hcidump
+
+ Priority: Medium
+ Complexity: C2
+
+- GATT server: fix MTU exchange
+
+ Priority: Medium
+ Complexity: C2
+
+- GATT server: fix read by UUID (read by handle works)
+
+ Priority: Medium
+ Complexity: C2
+
+- gatttool: add an interactive command prompt mode. Many LE devices
+ expect the connection to stay up a long time and disable advertising
+ after a disconnection so it's inconvenient to use gatttool in the
+ current "single operation at a time" mode.
+
+ Priority: Medium
+ Complexity: C2
+
- gatttool should have the ability to wait for req responses before
quitting (some servers require a small sleep even with cmd's). Maybe a
--delay-exit or --timeout command line switch.
@@ -42,13 +65,6 @@ ATT/GATT
Priority: Low
Complexity: C1
-- Attribute server shall implement attribute permission verification,
- returning an error code if necessary. See Volume 3, Part F, 3.2.5
- for more information.
-
- Priority: Low
- Complexity: C2
-
- Attribute server should process queued GATT/ATT commands if the
client disconnects. The client can simply send a command and quit,
without wait for a response(ex: Write Command). For this scenario
@@ -58,11 +74,6 @@ ATT/GATT
Priority: Low
Complecity: C1
-- Long reads/writes don't work (consisting of multiple request packets)
-
- Priority: Low
- Complexity: C2
-
- Add sdp discovery support to gattool with BR (--sdp, default is 0x1f)
Priority: Low
@@ -74,25 +85,14 @@ ATT/GATT
Priority: Low
Complexity: C1
-- Add ATT/GATT parsing to hcidump
-
- Priority: Medium
- Complexity: C2
-
-- GATT server: fix MTU exchange
-
- Priority: Medium
- Complexity: C2
-
-- GATT server: fix read by UUID (read by handle works)
+- Long reads/writes don't work (consisting of multiple request packets)
- Priority: Medium
+ Priority: Low
Complexity: C2
-- gatttool: add an interactive command prompt mode. Many LE devices
- expect the connection to stay up a long time and disable advertising
- after a disconnection so it's inconvenient to use gatttool in the
- current "single operation at a time" mode.
+- Attribute server shall implement attribute permission verification,
+ returning an error code if necessary. See Volume 3, Part F, 3.2.5
+ for more information.
- Priority: Medium
+ Priority: Low
Complexity: C2
--
1.7.1
^ permalink raw reply related
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 15:52 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, johan.hedberg@gmail.com,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <1286466576.6145.161.camel@aeonflux>
> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 10:50 AM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> > > > > > > Registering the Bluetooth HCI driver in module_init/module_ex=
it is
> not
> > > > > > > acceptable. Turn your shared transport into a proper bus.
> > > > > >
> > > > > > Yes, you did comment on it before, I remember, I did prototype =
the
> > > driver as
> > > > > > a bus driver, However I didn't find any advantages by convertin=
g it
> to a
> > > bus
> > > > > > driver.
> > > > > > As in, currently the shared transport driver is a line discipli=
ne
> driver
> > > > > because
> > > > > > it is the only way it can communicate over TTY without being ti=
ghtly
> > > coupled
> > > > > with the UART driver.
> > > > > >
> > > > > > > We want to be able to have generic kernels where this module =
is
> > > enabled,
> > > > > > > but no Shared Transport is available.
> > > > > >
> > > > > > Oh if this is the reason I cannot have hci_register/_unregister=
in
> > > > > module_init/_exit, Can I do this module "depends" on TI_ST, Then =
it
> would
> > > not
> > > > > > even be visible to build if TI_ST is not selected.
> > > > >
> > > > > this is not helping either. Then TI_ST can not be selected and so=
you
> > > > > still end up with some weird platform specific kernels. We don't =
want
> > > > > that. We want generic kernels that can detect the hardware they a=
re
> > > > > running on.
> > > > >
> > > > > As I said, I will not accept this driver if it registers HCI devi=
ce in
> > > > > module_init. No other driver is doing this and it is in general a
> really
> > > > > really really bad idea.
> > > > >
> > > >
> > > > Ok, now I am beginning to get what you say, Let me check, may be wh=
at
> > > > I can do is, have something like a st_prepare() function called in =
the
> > > > module_init, and a _probe function of the bluetooth driver will be
> called,
> > > > _ONLY_ if the _probe of my platform driver has been called..
> > > > Do you think this would be a good idea?
> > > >
> > > > Note: the TI_ST driver is also a platform device driver, so that TI=
_ST's
> > > > Probe is not called, if a arch/xx/board-xx doesn't add it.
> > >
> > > that that should be your bus right there.
> >
> > I understand the perspective, but "bus" is not device-driver type of mo=
del
> right? I mean I need a device which will be added in some platform specif=
ic
> > board file, and the driver in my driver core file.
> >
> > > Let me repeat this. If you register the HCI device in module_init the=
n
> > > it will be registered on all platform this module is selected. Even i=
f
> > > the kernel runs on x86. And that is not acceptable. Registering devic=
es
> > > in module_init is a bad idea no matter what. That is why all other
> > > drivers just register a driver here and not a device.
> >
> > I did initially think about making each of the protocol drivers a
> > platform devices as well.
> > As in Bluetooth/FM/GPS TI_ST driver would also be a platform device and=
its
> _probe doing the HCI/v4L2/character device registration.
> >
> > So which one do you think makes more sense here?
> > 1. Do I EXPORT a new symbol called st_prepare? And allow hci registrati=
on
> there?
> >
> > 2. Or make Bluetooth device a platform device and this driver a platfor=
m
> driver
> > and add this Bluetooth device only when I add TI_ST platform device?
>=20
> then make them a platform device. Since you do need a proper parent for
> these devices anyway. Otherwise a lot of logic within sysfs will fail.
Hnm, OK, Thanks. This also sort of helps me out in future if I wanted to su=
pport
multiple TI_ST devices (very hypothetical)
Ok, I will send out a patch for this. I will also make sure the patch I sen=
d
would be for addition of driver as you suggested and not to move from stagi=
ng.
Thanks,
Pavan.
> Regards
>=20
> Marcel
>=20
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Marcel Holtmann @ 2010-10-07 15:49 UTC (permalink / raw)
To: Savoy, Pavan
Cc: linux-bluetooth@vger.kernel.org, johan.hedberg@gmail.com,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <19F8576C6E063C45BE387C64729E739404AA21D1BD@dbde02.ent.ti.com>
Hi Pavan,
> > > > > > Registering the Bluetooth HCI driver in module_init/module_exit is not
> > > > > > acceptable. Turn your shared transport into a proper bus.
> > > > >
> > > > > Yes, you did comment on it before, I remember, I did prototype the
> > driver as
> > > > > a bus driver, However I didn't find any advantages by converting it to a
> > bus
> > > > > driver.
> > > > > As in, currently the shared transport driver is a line discipline driver
> > > > because
> > > > > it is the only way it can communicate over TTY without being tightly
> > coupled
> > > > with the UART driver.
> > > > >
> > > > > > We want to be able to have generic kernels where this module is
> > enabled,
> > > > > > but no Shared Transport is available.
> > > > >
> > > > > Oh if this is the reason I cannot have hci_register/_unregister in
> > > > module_init/_exit, Can I do this module "depends" on TI_ST, Then it would
> > not
> > > > > even be visible to build if TI_ST is not selected.
> > > >
> > > > this is not helping either. Then TI_ST can not be selected and so you
> > > > still end up with some weird platform specific kernels. We don't want
> > > > that. We want generic kernels that can detect the hardware they are
> > > > running on.
> > > >
> > > > As I said, I will not accept this driver if it registers HCI device in
> > > > module_init. No other driver is doing this and it is in general a really
> > > > really really bad idea.
> > > >
> > >
> > > Ok, now I am beginning to get what you say, Let me check, may be what
> > > I can do is, have something like a st_prepare() function called in the
> > > module_init, and a _probe function of the bluetooth driver will be called,
> > > _ONLY_ if the _probe of my platform driver has been called..
> > > Do you think this would be a good idea?
> > >
> > > Note: the TI_ST driver is also a platform device driver, so that TI_ST's
> > > Probe is not called, if a arch/xx/board-xx doesn't add it.
> >
> > that that should be your bus right there.
>
> I understand the perspective, but "bus" is not device-driver type of model right? I mean I need a device which will be added in some platform specific
> board file, and the driver in my driver core file.
>
> > Let me repeat this. If you register the HCI device in module_init then
> > it will be registered on all platform this module is selected. Even if
> > the kernel runs on x86. And that is not acceptable. Registering devices
> > in module_init is a bad idea no matter what. That is why all other
> > drivers just register a driver here and not a device.
>
> I did initially think about making each of the protocol drivers a
> platform devices as well.
> As in Bluetooth/FM/GPS TI_ST driver would also be a platform device and its _probe doing the HCI/v4L2/character device registration.
>
> So which one do you think makes more sense here?
> 1. Do I EXPORT a new symbol called st_prepare? And allow hci registration there?
>
> 2. Or make Bluetooth device a platform device and this driver a platform driver
> and add this Bluetooth device only when I add TI_ST platform device?
then make them a platform device. Since you do need a proper parent for
these devices anyway. Otherwise a lot of logic within sysfs will fail.
Regards
Marcel
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 15:47 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, johan.hedberg@gmail.com,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <1286465843.6145.157.camel@aeonflux>
> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 10:37 AM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> > > > > Registering the Bluetooth HCI driver in module_init/module_exit i=
s not
> > > > > acceptable. Turn your shared transport into a proper bus.
> > > >
> > > > Yes, you did comment on it before, I remember, I did prototype the
> driver as
> > > > a bus driver, However I didn't find any advantages by converting it=
to a
> bus
> > > > driver.
> > > > As in, currently the shared transport driver is a line discipline d=
river
> > > because
> > > > it is the only way it can communicate over TTY without being tightl=
y
> coupled
> > > with the UART driver.
> > > >
> > > > > We want to be able to have generic kernels where this module is
> enabled,
> > > > > but no Shared Transport is available.
> > > >
> > > > Oh if this is the reason I cannot have hci_register/_unregister in
> > > module_init/_exit, Can I do this module "depends" on TI_ST, Then it w=
ould
> not
> > > > even be visible to build if TI_ST is not selected.
> > >
> > > this is not helping either. Then TI_ST can not be selected and so you
> > > still end up with some weird platform specific kernels. We don't want
> > > that. We want generic kernels that can detect the hardware they are
> > > running on.
> > >
> > > As I said, I will not accept this driver if it registers HCI device i=
n
> > > module_init. No other driver is doing this and it is in general a rea=
lly
> > > really really bad idea.
> > >
> >
> > Ok, now I am beginning to get what you say, Let me check, may be what
> > I can do is, have something like a st_prepare() function called in the
> > module_init, and a _probe function of the bluetooth driver will be call=
ed,
> > _ONLY_ if the _probe of my platform driver has been called..
> > Do you think this would be a good idea?
> >
> > Note: the TI_ST driver is also a platform device driver, so that TI_ST'=
s
> > Probe is not called, if a arch/xx/board-xx doesn't add it.
>=20
> that that should be your bus right there.
I understand the perspective, but "bus" is not device-driver type of model =
right? I mean I need a device which will be added in some platform specific
board file, and the driver in my driver core file.
> Let me repeat this. If you register the HCI device in module_init then
> it will be registered on all platform this module is selected. Even if
> the kernel runs on x86. And that is not acceptable. Registering devices
> in module_init is a bad idea no matter what. That is why all other
> drivers just register a driver here and not a device.
I did initially think about making each of the protocol drivers a=20
platform devices as well.
As in Bluetooth/FM/GPS TI_ST driver would also be a platform device and its=
_probe doing the HCI/v4L2/character device registration.
So which one do you think makes more sense here?
1. Do I EXPORT a new symbol called st_prepare? And allow hci registration t=
here?
2. Or make Bluetooth device a platform device and this driver a platform dr=
iver
and add this Bluetooth device only when I add TI_ST platform device?
> Regards
>=20
> Marcel
>=20
^ permalink raw reply
* RE: Sim Access profile server implementation
From: Marcel Holtmann @ 2010-10-07 15:42 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz
Cc: suraj, linux-bluetooth, Jothikumar.Mothilal, joakim.xj.ceder,
arunkr.singh
In-Reply-To: <99B09243E1A5DA4898CDD8B700111448097D01B814@EXMB04.eu.tieto.com>
Hi Waldemar,
> >>Can you share that code with us. And also hardware if you. We
> >are still
> >>having hard time to find proper hardware to test this on.
> >
> >Will send a patch soon. I use
> >http://www.stericsson.com/platforms/U8500.jsp hw, but need to
> >check if I can share one with you.
> >
>
> Here you are the patch. Just note that it is based on an old bluez 4.4x
> If you consider to accept the design I will update the patch upon the latest release.
in general I am fine if we do this similar to what we do with the
different telephony backend. So yes, go ahead with this.
However, please clean this up. The patch needs a lot of work before it
would be ready upstream. And of course it requires a dummy SAP plugin as
well. Same as we do have for telephony.
Regards
Marcel
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Marcel Holtmann @ 2010-10-07 15:37 UTC (permalink / raw)
To: Savoy, Pavan
Cc: linux-bluetooth@vger.kernel.org, johan.hedberg@gmail.com,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <19F8576C6E063C45BE387C64729E739404AA21D1B4@dbde02.ent.ti.com>
Hi Pavan,
> > > > Registering the Bluetooth HCI driver in module_init/module_exit is not
> > > > acceptable. Turn your shared transport into a proper bus.
> > >
> > > Yes, you did comment on it before, I remember, I did prototype the driver as
> > > a bus driver, However I didn't find any advantages by converting it to a bus
> > > driver.
> > > As in, currently the shared transport driver is a line discipline driver
> > because
> > > it is the only way it can communicate over TTY without being tightly coupled
> > with the UART driver.
> > >
> > > > We want to be able to have generic kernels where this module is enabled,
> > > > but no Shared Transport is available.
> > >
> > > Oh if this is the reason I cannot have hci_register/_unregister in
> > module_init/_exit, Can I do this module "depends" on TI_ST, Then it would not
> > > even be visible to build if TI_ST is not selected.
> >
> > this is not helping either. Then TI_ST can not be selected and so you
> > still end up with some weird platform specific kernels. We don't want
> > that. We want generic kernels that can detect the hardware they are
> > running on.
> >
> > As I said, I will not accept this driver if it registers HCI device in
> > module_init. No other driver is doing this and it is in general a really
> > really really bad idea.
> >
>
> Ok, now I am beginning to get what you say, Let me check, may be what
> I can do is, have something like a st_prepare() function called in the
> module_init, and a _probe function of the bluetooth driver will be called,
> _ONLY_ if the _probe of my platform driver has been called..
> Do you think this would be a good idea?
>
> Note: the TI_ST driver is also a platform device driver, so that TI_ST's
> Probe is not called, if a arch/xx/board-xx doesn't add it.
that that should be your bus right there.
Let me repeat this. If you register the HCI device in module_init then
it will be registered on all platform this module is selected. Even if
the kernel runs on x86. And that is not acceptable. Registering devices
in module_init is a bad idea no matter what. That is why all other
drivers just register a driver here and not a device.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Greg KH @ 2010-10-07 15:35 UTC (permalink / raw)
To: Marcel Holtmann
Cc: pavan-savoy, linux-bluetooth, johan.hedberg, linux-kernel,
Pavan Savoy
In-Reply-To: <1286464867.6145.148.camel@aeonflux>
On Thu, Oct 07, 2010 at 05:21:07PM +0200, Marcel Holtmann wrote:
> Hi Greg,
>
> > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > like BT, FM, GPS and WLAN onto a single chip.
> > > >
> > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > line discipline driver which also allows other drivers like
> > > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > > >
> > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > ---
> > > > drivers/bluetooth/bt_ti.c | 463 ++++++++++++++++++++++++++++++++++++
> > > > drivers/staging/ti-st/bt_drv.c | 509 ----------------------------------------
> > > > drivers/staging/ti-st/bt_drv.h | 61 -----
> > > > 3 files changed, 463 insertions(+), 570 deletions(-)
> > > > create mode 100644 drivers/bluetooth/bt_ti.c
> > > > delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > > delete mode 100644 drivers/staging/ti-st/bt_drv.h
> > >
> > > I don't care about staging at all. So you sort that out with Greg.
> > >
> > > Submit your driver for upstream inclusion. And once accepted you can pin
> > > Greg about removing it.
> >
> > The driver is already in staging, this is the request to move it out of
> > staging and into the "correct" place in the tree. The core of the ti-st
> > code is now in the drivers/misc/ directory in the linux-next tree, and
> > this patch is the request to move the bluetooth drive into the proper
> > drivers/bluetooth/ location.
>
> nice idea, but I don't want it that way. I am not dealing with staging
> at all. They can submit this driver for upstream inclusion and then
> delete it in a second step from staging. Or the other way around.
That's fine, then consider this a "please add the driver to your tree"
type of request.
> And as long as this driver registers a Bluetooth HCI device in its
> module_init routine it is not ready for upstream. This needs to be fixed
> first and I mentioned that already before.
That's also fine, it's an iterative process :)
thanks,
greg k-h
^ permalink raw reply
* RE: RFC: btusb firmware load help
From: Marcel Holtmann @ 2010-10-07 15:24 UTC (permalink / raw)
To: Shanmugamkamatchi Balashanmugam
Cc: Luis Rodriguez, Johannes Berg, linux-bluetooth,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
Deepak Dhamdhere, Sree Durbha
In-Reply-To: <44EE5C37ADC36343B0625A05DD408C4850DAD2CA31@CHEXMB-01.global.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.
Regards
Marcel
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 15:23 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, johan.hedberg@gmail.com,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <1286464659.6145.144.camel@aeonflux>
> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 10:18 AM
> To: Savoy, Pavan
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>=20
> Hi Pavan,
>=20
> > > Registering the Bluetooth HCI driver in module_init/module_exit is no=
t
> > > acceptable. Turn your shared transport into a proper bus.
> >
> > Yes, you did comment on it before, I remember, I did prototype the driv=
er as
> > a bus driver, However I didn't find any advantages by converting it to =
a bus
> > driver.
> > As in, currently the shared transport driver is a line discipline drive=
r
> because
> > it is the only way it can communicate over TTY without being tightly co=
upled
> with the UART driver.
> >
> > > We want to be able to have generic kernels where this module is enabl=
ed,
> > > but no Shared Transport is available.
> >
> > Oh if this is the reason I cannot have hci_register/_unregister in
> module_init/_exit, Can I do this module "depends" on TI_ST, Then it would=
not
> > even be visible to build if TI_ST is not selected.
>=20
> this is not helping either. Then TI_ST can not be selected and so you
> still end up with some weird platform specific kernels. We don't want
> that. We want generic kernels that can detect the hardware they are
> running on.
>=20
> As I said, I will not accept this driver if it registers HCI device in
> module_init. No other driver is doing this and it is in general a really
> really really bad idea.
>=20
Ok, now I am beginning to get what you say, Let me check, may be what
I can do is, have something like a st_prepare() function called in the
module_init, and a _probe function of the bluetooth driver will be called,
_ONLY_ if the _probe of my platform driver has been called..
Do you think this would be a good idea?
Note: the TI_ST driver is also a platform device driver, so that TI_ST's
Probe is not called, if a arch/xx/board-xx doesn't add it.
Please suggest.
>=20
> Marcel
>=20
^ permalink raw reply
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Marcel Holtmann @ 2010-10-07 15:21 UTC (permalink / raw)
To: Greg KH
Cc: pavan-savoy, linux-bluetooth, johan.hedberg, linux-kernel,
Pavan Savoy
In-Reply-To: <20101007143409.GB14913@kroah.com>
Hi Greg,
> > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > like BT, FM, GPS and WLAN onto a single chip.
> > >
> > > This Bluetooth driver works on top of the TI_ST shared transport
> > > line discipline driver which also allows other drivers like
> > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > >
> > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > ---
> > > drivers/bluetooth/bt_ti.c | 463 ++++++++++++++++++++++++++++++++++++
> > > drivers/staging/ti-st/bt_drv.c | 509 ----------------------------------------
> > > drivers/staging/ti-st/bt_drv.h | 61 -----
> > > 3 files changed, 463 insertions(+), 570 deletions(-)
> > > create mode 100644 drivers/bluetooth/bt_ti.c
> > > delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > > delete mode 100644 drivers/staging/ti-st/bt_drv.h
> >
> > I don't care about staging at all. So you sort that out with Greg.
> >
> > Submit your driver for upstream inclusion. And once accepted you can pin
> > Greg about removing it.
>
> The driver is already in staging, this is the request to move it out of
> staging and into the "correct" place in the tree. The core of the ti-st
> code is now in the drivers/misc/ directory in the linux-next tree, and
> this patch is the request to move the bluetooth drive into the proper
> drivers/bluetooth/ location.
nice idea, but I don't want it that way. I am not dealing with staging
at all. They can submit this driver for upstream inclusion and then
delete it in a second step from staging. Or the other way around.
And as long as this driver registers a Bluetooth HCI device in its
module_init routine it is not ready for upstream. This needs to be fixed
first and I mentioned that already before.
Regards
Marcel
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Marcel Holtmann @ 2010-10-07 15:17 UTC (permalink / raw)
To: Savoy, Pavan
Cc: linux-bluetooth@vger.kernel.org, johan.hedberg@gmail.com,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <19F8576C6E063C45BE387C64729E739404AA21D19E@dbde02.ent.ti.com>
Hi Pavan,
> > Registering the Bluetooth HCI driver in module_init/module_exit is not
> > acceptable. Turn your shared transport into a proper bus.
>
> Yes, you did comment on it before, I remember, I did prototype the driver as
> a bus driver, However I didn't find any advantages by converting it to a bus
> driver.
> As in, currently the shared transport driver is a line discipline driver because
> it is the only way it can communicate over TTY without being tightly coupled with the UART driver.
>
> > We want to be able to have generic kernels where this module is enabled,
> > but no Shared Transport is available.
>
> Oh if this is the reason I cannot have hci_register/_unregister in module_init/_exit, Can I do this module "depends" on TI_ST, Then it would not
> even be visible to build if TI_ST is not selected.
this is not helping either. Then TI_ST can not be selected and so you
still end up with some weird platform specific kernels. We don't want
that. We want generic kernels that can detect the hardware they are
running on.
As I said, I will not accept this driver if it registers HCI device in
module_init. No other driver is doing this and it is in general a really
really really bad idea.
Regards
Marcel
^ permalink raw reply
* RE: RFC: btusb firmware load help
From: Johannes Berg @ 2010-10-07 15:15 UTC (permalink / raw)
To: Shanmugamkamatchi Balashanmugam
Cc: Luis Rodriguez, Marcel Holtmann, linux-bluetooth,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
Deepak Dhamdhere, Sree Durbha
In-Reply-To: <44EE5C37ADC36343B0625A05DD408C4850DAD2CA31@CHEXMB-01.global.atheros.com>
> 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
^ permalink raw reply
* RE: RFC: btusb firmware load help
From: Shanmugamkamatchi Balashanmugam @ 2010-10-07 15:09 UTC (permalink / raw)
To: Luis Rodriguez, Johannes Berg
Cc: Luis Rodriguez, Marcel Holtmann, linux-bluetooth,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
Deepak Dhamdhere, Sree Durbha
In-Reply-To: <20101006183340.GI7070@tux>
________________________________________
From: linux-bluetooth-owner@vger.kernel.org [linux-bluetooth-owner@vger.kernel.org] On Behalf Of Luis R. Rodriguez [lrodriguez@atheros.com]
Sent: Thursday, October 07, 2010 12:03 AM
To: Johannes Berg
Cc: Luis Rodriguez; Marcel Holtmann; linux-bluetooth; linux-kernel@vger.kernel.org; linux-wireless@vger.kernel.org; Deepak Dhamdhere; Sree Durbha
Subject: Re: RFC: btusb firmware load help
On Wed, Oct 06, 2010 at 11:28:17AM -0700, Johannes Berg wrote:
> On Wed, 2010-10-06 at 11:26 -0700, Luis R. Rodriguez wrote:
>
> > > Good idea, I forgot about possible firmware changes :) Lets see if our
> > > team can do that. Thanks for all the feedback.
> >
> > Deepak a proof of concept test can involve simply hex-editing the
> > ath3k-1.fw and replacing 0x3002 to 0x3003, then the above patch might
> > work.
>
> $ hd ath3k-1.fw
> ...
> 00000670 00 00 00 00 00 00 00 00 00 00 00 00 f3 0c 02 30 |...............0|
> 00000680 12 01 10 01 e0 01 01 40 f3 0c 02 30 01 00 00 00 |.......@...0....|
> ...
>
> that looks a lot like the IDs right there, in little endian :-)
>Furthermore another idea by johannes is that if we cannot fix this
>in firmware by changing the exposed device ID, we could just check
>in btusb for some USB component that would come alive once the firmware
>does get loaded, like endpoints, or speed, or whatever. But that would
>be last resort.
> Luis
>--
>To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
Regards,
Bala.
^ permalink raw reply
* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 14:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth@vger.kernel.org, johan.hedberg@gmail.com,
greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <1286445948.6145.70.camel@aeonflux>
> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]
> Sent: Thursday, October 07, 2010 5:06 AM
> To: pavan-savoy@ti.com
> Cc: linux-bluetooth@vger.kernel.org; johan.hedberg@gmail.com; greg@kroah.=
com;
> linux-kernel@vger.kernel.org; Savoy, Pavan
> Subject: Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
>
> Hi Pavan,
>
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> >
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface=
.
> >
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> > drivers/bluetooth/bt_ti.c | 463 ++++++++++++++++++++++++++++++++=
++++
> > drivers/staging/ti-st/bt_drv.c | 509 --------------------------------=
-----
> ---
> > drivers/staging/ti-st/bt_drv.h | 61 -----
> > 3 files changed, 463 insertions(+), 570 deletions(-)
> > create mode 100644 drivers/bluetooth/bt_ti.c
> > delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > delete mode 100644 drivers/staging/ti-st/bt_drv.h
>
> I don't care about staging at all. So you sort that out with Greg.
>
> Submit your driver for upstream inclusion. And once accepted you can pin
> Greg about removing it.
>
> > diff --git a/drivers/bluetooth/bt_ti.c b/drivers/bluetooth/bt_ti.c
> > new file mode 100644
> > index 0000000..4f3d3aa
> > --- /dev/null
> > +++ b/drivers/bluetooth/bt_ti.c
> > @@ -0,0 +1,463 @@
> > +/*
> > + * Texas Instrument's Bluetooth Driver For Shared Transport.
> > + *
> > + * Bluetooth Driver acts as interface between HCI CORE and
> > + * TI Shared Transport Layer.
> > + *
> > + * Copyright (C) 2009-2010 Texas Instruments
> > + * Author: Raja Mani <raja_mani@ti.com>
> > + * Pavan Savoy <pavan_savoy@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modi=
fy
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-13=
07
> USA
> > + *
> > + */
> > +
> > +#define pr_fmt(fmt) "(tibt): " fmt
>
> Don't do this. Just use BT_DBG, BT_ERR, BT_INFO etc.
Yes, Will do it.
> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +
> > +#include <linux/ti_wilink_st.h>
> > +
> > +/* Bluetooth Driver Version */
> > +#define VERSION "1.0"
> > +
> > +/* Defines number of seconds to wait for reg completion
> > + * callback getting called from ST (in case,registration
> > + * with ST returns PENDING status)
> > + */
> > +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> > +
> > +/* BT driver's local status */
> > +#define BT_DRV_RUNNING 0
> > +#define BT_ST_REGISTERED 1
> > +
> > +/**
> > + * struct hci_st - BT driver operation structure
> > + * @hdev: hci device pointer which binds to bt driver
> > + * @flags: used locally,to maintain various BT driver status
> > + * @streg_cbdata: to hold ST registration callback status
> > + * @st_write: write function pointer of ST driver
> > + * @wait_for_btdrv_reg_completion - completion sync between hci_st_ope=
n
> > + * and hci_st_registration_completion_cb.
> > + */
> > +struct hci_st {
> > + struct hci_dev *hdev;
> > + unsigned long flags;
> > + char streg_cbdata;
> > + long (*st_write) (struct sk_buff *);
> > + struct completion wait_for_btdrv_reg_completion;
> > +};
> > +
> > +static struct hci_st *hst;
> > +static int reset;
> > +
> > +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> > +static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type=
)
> > +{
> > + struct hci_dev *hdev;
> > + hdev =3D hst->hdev;
> > +
> > + /* Update HCI stat counters */
> > + switch (pkt_type) {
> > + case HCI_COMMAND_PKT:
> > + hdev->stat.cmd_tx++;
> > + break;
> > +
> > + case HCI_ACLDATA_PKT:
> > + hdev->stat.acl_tx++;
> > + break;
> > +
> > + case HCI_SCODATA_PKT:
> > + hdev->stat.cmd_tx++;
> > + break;
> > + }
> > +}
> > +
> > +/* ------- Interfaces to Shared Transport ------ */
> > +
> > +/* Called by ST layer to indicate protocol registration completion
> > + * status.hci_st_open() function will wait for signal from this
> > + * API when st_register() function returns ST_PENDING.
> > + */
> > +static void hci_st_registration_completion_cb(void *priv_data, char da=
ta)
> > +{
> > + struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > + /* hci_st_open() function needs value of 'data' to know
> > + * the registration status(success/fail),So have a back
> > + * up of it.
> > + */
> > + lhst->streg_cbdata =3D data;
> > +
> > + /* Got a feedback from ST for BT driver registration
> > + * request.Wackup hci_st_open() function to continue
> > + * it's open operation.
> > + */
> > + complete(&lhst->wait_for_btdrv_reg_completion);
> > +}
> > +
> > +/* Called by Shared Transport layer when receive data is
> > + * available */
> > +static long hci_st_receive(void *priv_data, struct sk_buff *skb)
> > +{
> > + int err;
> > + int len;
> > + struct hci_st *lhst =3D (struct hci_st *)priv_data;
> > +
> > + err =3D 0;
> > + len =3D 0;
> > +
> > + if (skb =3D=3D NULL) {
> > + pr_err("Invalid SKB received from ST");
> > + return -EFAULT;
> > + }
> > + if (!lhst) {
> > + kfree_skb(skb);
> > + pr_err("Invalid hci_st memory,freeing SKB");
> > + return -EFAULT;
> > + }
> > + if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
> > + kfree_skb(skb);
> > + pr_err("Device is not running,freeing SKB");
> > + return -EINVAL;
> > + }
> > +
> > + len =3D skb->len;
> > + skb->dev =3D (struct net_device *)lhst->hdev;
> > +
> > + /* Forward skb to HCI CORE layer */
> > + err =3D hci_recv_frame(skb);
> > + if (err) {
> > + kfree_skb(skb);
> > + pr_err("Unable to push skb to HCI CORE(%d),freeing SKB",
> > + err);
> > + return err;
> > + }
> > + lhst->hdev->stat.byte_rx +=3D len;
> > +
> > + return 0;
> > +}
> > +
> > +/* ------- Interfaces to HCI layer ------ */
> > +
> > +/* Called from HCI core to initialize the device */
> > +static int hci_st_open(struct hci_dev *hdev)
> > +{
> > + static struct st_proto_s hci_st_proto;
> > + unsigned long timeleft;
> > + int err;
> > + err =3D 0;
> > +
> > + pr_debug("%s %p", hdev->name, hdev);
> > +
> > + /* Already registered with ST ? */
> > + if (test_bit(BT_ST_REGISTERED, &hst->flags)) {
> > + pr_err("Registered with ST already,open called again?");
> > + return 0;
> > + }
>
> Why are you testing against this. This should be not needed at all.
Oh yes, Agree, the HCI sock/core doesn't allow hci0 to be HCIDEVUP-ed twice=
.
> > + /* Populate BT driver info required by ST */
> > + memset(&hci_st_proto, 0, sizeof(hci_st_proto));
> > +
> > + /* BT driver ID */
> > + hci_st_proto.type =3D ST_BT;
> > +
> > + /* Receive function which called from ST */
> > + hci_st_proto.recv =3D hci_st_receive;
> > +
> > + /* Packet match function may used in future */
> > + hci_st_proto.match_packet =3D NULL;
> > +
> > + /* Callback to be called when registration is pending */
> > + hci_st_proto.reg_complete_cb =3D hci_st_registration_completion_cb;
> > +
> > + /* This is write function pointer of ST. BT driver will make use of
> this
> > + * for sending any packets to chip. ST will assign and give to us, =
so
> > + * make it as NULL */
> > + hci_st_proto.write =3D NULL;
> > +
> > + /* send in the hst to be received at registration complete callback
> > + * and during st's receive
> > + */
> > + hci_st_proto.priv_data =3D hst;
> > +
> > + /* Register with ST layer */
> > + err =3D st_register(&hci_st_proto);
>
> I am still against just claiming the st_ prefix where a company called
> ST is active in the kernel as well. Is the Shared Transport really a
> proper standard?
The driver is called TI_ST, I can rename it no problem, I'm not 100% convin=
ced with this either, any suggestions for names?
> > + if (err =3D=3D -EINPROGRESS) {
> > + /* Prepare wait-for-completion handler data structures.
> > + * Needed to syncronize this and st_registration_completion=
_cb()
> > + * functions.
> > + */
> > + init_completion(&hst->wait_for_btdrv_reg_completion);
> > +
> > + /* Reset ST registration callback status flag , this value
> > + * will be updated in hci_st_registration_completion_cb()
> > + * function whenever it called from ST driver.
> > + */
> > + hst->streg_cbdata =3D -EINPROGRESS;
> > +
> > + /* ST is busy with other protocol registration(may be busy =
with
> > + * firmware download).So,Wait till the registration callbac=
k
> > + * (passed as a argument to st_register() function) getting
> > + * called from ST.
> > + */
> > + pr_debug(" %s waiting for reg completion signal from ST",
> > + __func__);
> > +
> > + timeleft =3D
> > + wait_for_completion_timeout
> > + (&hst->wait_for_btdrv_reg_completion,
> > + msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> > + if (!timeleft) {
> > + pr_err("Timeout(%d sec),didn't get reg"
> > + "completion signal from ST",
> > + BT_REGISTER_TIMEOUT / 1000);
> > + return -ETIMEDOUT;
> > + }
> > +
> > + /* Is ST registration callback called with ERROR value? */
> > + if (hst->streg_cbdata !=3D 0) {
> > + pr_err("ST reg completion CB called with invalid"
> > + "status %d", hst->streg_cbdata);
> > + return -EAGAIN;
> > + }
> > + err =3D 0;
> > + } else if (err =3D=3D -1) {
> > + pr_err("st_register failed %d", err);
> > + return -EAGAIN;
> > + }
> > +
> > + /* Do we have proper ST write function? */
> > + if (hci_st_proto.write !=3D NULL) {
> > + /* We need this pointer for sending any Bluetooth pkts */
> > + hst->st_write =3D hci_st_proto.write;
> > + } else {
> > + pr_err("failed to get ST write func pointer");
> > +
> > + /* Undo registration with ST */
> > + err =3D st_unregister(ST_BT);
> > + if (err < 0)
> > + pr_err("st_unregister failed %d", err);
> > +
> > + hst->st_write =3D NULL;
> > + return -EAGAIN;
> > + }
> > +
> > + /* Registration with ST layer is completed successfully,
> > + * now chip is ready to accept commands from HCI CORE.
> > + * Mark HCI Device flag as RUNNING
> > + */
> > + set_bit(HCI_RUNNING, &hdev->flags);
> > +
> > + /* Registration with ST successful */
> > + set_bit(BT_ST_REGISTERED, &hst->flags);
> > +
> > + return err;
> > +}
> > +
> > +/* Close device */
> > +static int hci_st_close(struct hci_dev *hdev)
> > +{
> > + int err;
> > + err =3D 0;
> > +
> > + /* Unregister from ST layer */
> > + if (test_and_clear_bit(BT_ST_REGISTERED, &hst->flags)) {
> > + err =3D st_unregister(ST_BT);
> > + if (err !=3D 0) {
> > + pr_err("st_unregister failed %d", err);
> > + return -EBUSY;
> > + }
> > + }
> > +
> > + hst->st_write =3D NULL;
> > +
> > + /* ST layer would have moved chip to inactive state.
> > + * So,clear HCI device RUNNING flag.
> > + */
> > + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> > + return 0;
> > +
> > + return err;
> > +}
> > +
> > +/* Called from HCI CORE , Sends frames to Shared Transport */
> > +static int hci_st_send_frame(struct sk_buff *skb)
> > +{
> > + struct hci_dev *hdev;
> > + struct hci_st *hst;
> > + long len;
> > +
> > + if (skb =3D=3D NULL) {
> > + pr_err("Invalid skb received from HCI CORE");
> > + return -ENOMEM;
> > + }
> > + hdev =3D (struct hci_dev *)skb->dev;
> > + if (!hdev) {
> > + pr_err("SKB received for invalid HCI Device (hdev=3DNULL)")=
;
> > + return -ENODEV;
> > + }
> > + if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> > + pr_err("Device is not running");
> > + return -EBUSY;
> > + }
> > +
> > + hst =3D (struct hci_st *)hdev->driver_data;
> > +
> > + /* Prepend skb with frame type */
> > + memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> > +
> > + pr_debug(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> > + skb->len);
> > +
> > + /* Insert skb to shared transport layer's transmit queue.
> > + * Freeing skb memory is taken care in shared transport layer,
> > + * so don't free skb memory here.
> > + */
> > + if (!hst->st_write) {
> > + kfree_skb(skb);
> > + pr_err(" Can't write to ST, st_write null?");
> > + return -EAGAIN;
> > + }
> > + len =3D hst->st_write(skb);
> > + if (len < 0) {
> > + /* Something went wrong in st write , free skb memory */
> > + kfree_skb(skb);
> > + pr_err(" ST write failed (%ld)", len);
> > + return -EAGAIN;
> > + }
> > +
> > + /* ST accepted our skb. So, Go ahead and do rest */
> > + hdev->stat.byte_tx +=3D len;
> > + hci_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> > +
> > + return 0;
> > +}
> > +
> > +static void hci_st_destruct(struct hci_dev *hdev)
> > +{
> > + if (!hdev) {
> > + pr_err("Destruct called with invalid HCI Device"
> > + "(hdev=3DNULL)");
> > + return;
> > + }
> > +
> > + pr_debug("%s", hdev->name);
> > +
> > + /* free hci_st memory */
> > + if (hdev->driver_data !=3D NULL)
> > + kfree(hdev->driver_data);
> > +
> > + return;
> > +}
> > +
> > +/* Creates new HCI device */
> > +static int hci_st_register_dev(struct hci_st *hst)
> > +{
> > + struct hci_dev *hdev;
> > +
> > + /* Initialize and register HCI device */
> > + hdev =3D hci_alloc_dev();
> > + if (!hdev) {
> > + pr_err("Can't allocate HCI device");
> > + return -ENOMEM;
> > + }
> > + pr_debug(" HCI device allocated. hdev=3D %p", hdev);
> > +
> > + hst->hdev =3D hdev;
> > + hdev->bus =3D HCI_UART;
> > + hdev->driver_data =3D hst;
> > + hdev->open =3D hci_st_open;
> > + hdev->close =3D hci_st_close;
> > + hdev->flush =3D NULL;
> > + hdev->send =3D hci_st_send_frame;
> > + hdev->destruct =3D hci_st_destruct;
> > + hdev->owner =3D THIS_MODULE;
> > +
> > + if (reset)
> > + set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
> > +
> > + if (hci_register_dev(hdev) < 0) {
> > + pr_err("Can't register HCI device");
> > + hci_free_dev(hdev);
> > + return -ENODEV;
> > + }
> > +
> > + pr_debug(" HCI device registered. hdev=3D %p", hdev);
> > + return 0;
> > +}
> > +
> > +/* ------- Module Init interface ------ */
> > +
> > +static int __init bt_drv_init(void)
> > +{
> > + int err;
> > + err =3D 0;
> > +
> > + pr_debug(" Bluetooth Driver Version %s", VERSION);
> > +
> > + /* Allocate local resource memory */
> > + hst =3D kzalloc(sizeof(struct hci_st), GFP_KERNEL);
> > + if (!hst) {
> > + pr_err("Can't allocate control structure");
> > + return -ENFILE;
> > + }
> > +
> > + /* Expose "hciX" device to user space */
> > + err =3D hci_st_register_dev(hst);
> > + if (err) {
> > + /* Release local resource memory */
> > + kfree(hst);
> > +
> > + pr_err("Unable to expose hci0 device(%d)", err);
> > + return err;
> > + }
> > + set_bit(BT_DRV_RUNNING, &hst->flags);
> > + return err;
> > +}
> > +
> > +/* ------- Module Exit interface ------ */
> > +
> > +static void __exit bt_drv_exit(void)
> > +{
> > + /* Deallocate local resource's memory */
> > + if (hst) {
> > + struct hci_dev *hdev =3D hst->hdev;
> > + if (hdev =3D=3D NULL) {
> > + pr_err("Invalid hdev memory");
> > + kfree(hst);
> > + } else {
> > + hci_st_close(hdev);
> > + if (test_and_clear_bit(BT_DRV_RUNNING, &hst->flags)=
) {
> > + /* Remove HCI device (hciX) created
> > + * in module init.
> > + */
> > + hci_unregister_dev(hdev);
> > + /* Free HCI device memory */
> > + hci_free_dev(hdev);
> > + }
> > + }
> > + }
> > +}
>
> Registering the Bluetooth HCI driver in module_init/module_exit is not
> acceptable. Turn your shared transport into a proper bus.
Yes, you did comment on it before, I remember, I did prototype the driver a=
s
a bus driver, However I didn't find any advantages by converting it to a bu=
s
driver.
As in, currently the shared transport driver is a line discipline driver be=
cause
it is the only way it can communicate over TTY without being tightly couple=
d with the UART driver.
> We want to be able to have generic kernels where this module is enabled,
> but no Shared Transport is available.
Oh if this is the reason I cannot have hci_register/_unregister in module_i=
nit/_exit, Can I do this module "depends" on TI_ST, Then it would not
even be visible to build if TI_ST is not selected.
> Regards
>
> Marcel
>
^ permalink raw reply
* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Greg KH @ 2010-10-07 14:34 UTC (permalink / raw)
To: Marcel Holtmann
Cc: pavan-savoy, linux-bluetooth, johan.hedberg, linux-kernel,
Pavan Savoy
In-Reply-To: <1286445948.6145.70.camel@aeonflux>
On Thu, Oct 07, 2010 at 12:05:48PM +0200, Marcel Holtmann wrote:
> Hi Pavan,
>
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> >
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface.
> >
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> > drivers/bluetooth/bt_ti.c | 463 ++++++++++++++++++++++++++++++++++++
> > drivers/staging/ti-st/bt_drv.c | 509 ----------------------------------------
> > drivers/staging/ti-st/bt_drv.h | 61 -----
> > 3 files changed, 463 insertions(+), 570 deletions(-)
> > create mode 100644 drivers/bluetooth/bt_ti.c
> > delete mode 100644 drivers/staging/ti-st/bt_drv.c
> > delete mode 100644 drivers/staging/ti-st/bt_drv.h
>
> I don't care about staging at all. So you sort that out with Greg.
>
> Submit your driver for upstream inclusion. And once accepted you can pin
> Greg about removing it.
The driver is already in staging, this is the request to move it out of
staging and into the "correct" place in the tree. The core of the ti-st
code is now in the drivers/misc/ directory in the linux-next tree, and
this patch is the request to move the bluetooth drive into the proper
drivers/bluetooth/ location.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] TODO: Implement Server Characteristic Configuration
From: Claudio Takahasi @ 2010-10-07 14:12 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
---
TODO | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/TODO b/TODO
index 75a929c..d594592 100644
--- a/TODO
+++ b/TODO
@@ -18,6 +18,14 @@ Background
ATT/GATT
========
+- Implement Server characteristic Configuration support in the attribute
+ server to manage characteristic value broadcasting. There is a single
+ instance of the Server Characteristic Configuration for all clients.
+ See Volume 3, Part G, section 3.3.3.4 for more information.
+
+ Priority: Low
+ Complexity: C1
+
- Implement Client Characteristic Configuration support in the attribute
server to manage indications and notications. This is a per client attribute
to control how the client wants to receive reports of changes in a given
--
1.7.3.1
^ permalink raw reply related
* [PATCH 2/2] TODO: Implement Client Characteristic Configuration
From: Claudio Takahasi @ 2010-10-07 13:53 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1286459639-25194-1-git-send-email-claudio.takahasi@openbossa.org>
---
TODO | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/TODO b/TODO
index a9f1339..75a929c 100644
--- a/TODO
+++ b/TODO
@@ -18,6 +18,14 @@ Background
ATT/GATT
========
+- Implement Client Characteristic Configuration support in the attribute
+ server to manage indications and notications. This is a per client attribute
+ to control how the client wants to receive reports of changes in a given
+ characteristic value.
+
+ Priority: Low
+ Complexity: C2
+
- gatttool should have the ability to wait for req responses before
quitting (some servers require a small sleep even with cmd's). Maybe a
--delay-exit or --timeout command line switch.
--
1.7.3.1
^ permalink raw reply related
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