Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] Bluetooth: update MAINTAINERS for Bluetooth subsys
From: Marcel Holtmann @ 2010-10-09  8:07 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1286540658-2890-1-git-send-email-padovan@profusion.mobi>

Hi Gustavo,

> Add myself to MAINTAINERS and update the git trees.
> 
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Marcel Holtmann @ 2010-10-09  8:06 UTC (permalink / raw)
  To: Sven-Haegar Koch
  Cc: Luis R. Rodriguez, Suraj Sumangala, Luis Rodriguez,
	David Woodhouse, linux-bluetooth, linux-kernel@vger.kernel.org,
	linux-wireless
In-Reply-To: <alpine.DEB.2.02.1010090040240.19472@aurora.sdinet.de>

Hi Sven-Haeger,

> > > > I last tried to document a thread we had over this here:
> > > >
> > > > http://wireless.kernel.org/en/developers/Documentation/firmware-versioning
> > > >
> > 
> > Thanks, I've updated that link above to document bug fixing does not require
> > a filename change.
> 
> I would summarize it as:
> 
> If a new firmware version also works with an old driver, keep the filename.

correct.

> If a new firmware version also requires a new driver, change the name.
> 
> If a new driver requires a new firmware, change the name.

These two depend. The exposed API stays the same. The firmware file
itself is the same. Just the loading procedure is different. So no need
to change the firmware name.

Let me repeat this. If the API of the firmware exposed after loading it,
breaks or is incompatible, then you need a new name.

If you have generic commands to detect features in the firmware, then
you should never be needed to change your firmware name. So you could
extend the API as much as you like with keeping the same name.

The different firmware names are for the driver to be able to detect the
API of the firmware. And only if that is only possible via the filename
you should use different filenames. Otherwise don't bother and use the
generic feature detection of the firmware itself.

And for Bluetooth in specific that is HCI. So any company needed
different firmware filenames for Bluetooth have done something really
really wrong in their development cycles.

Regards

Marcel



^ permalink raw reply

* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Marcel Holtmann @ 2010-10-09  8:01 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Suraj Sumangala, David Woodhouse, linux-bluetooth, linux-kernel,
	linux-wireless
In-Reply-To: <20101008170258.GJ10149@tux>

Hi Luis,

> What is the difference between ath3k-2.fw and ath3k-1.fw ?
> 
> Won't the API change now that you are addressing the sflash
> configuration fix? Would it not help to identify the two
> different firmwares then?
> 
> David, Marcel, what are your preferences for a firmware upgrade
> where the firmware does not change API (lets just pretend it does
> not for a moment) ? Do we keep the same filename?

that is what most companies do and that is what iwlwifi has done so far.
Only if the API breaks a different suffix is used.

With Bluetooth this should be never needed at all. The reason is that
you need to expose Bluetooth HCI. And that has generic version, support
commands and supported features commands.

We are not even using the version information for anything useful these
days since the firmware has to identify its features and its supported
commands with standard HCI commands. So it is pretty simple to detect
what Bluetooth subsystem needs to do.

> In this particular case I would assume our new sflash configuration
> fix that might be being worked on might change the re-enumerated
> USB device IDs so it seems to me a good idea to use a new filename.
> I should note ath3k-2.fw already made it to linux-firmware.git...

No it does not. The changed PID is not a breakage. It will just keep
working. So please fix this in linux-firmware.git right away and remove
the new firmware file.

And here is something that is wrong with your process as well. Don't
submit firmware files upstream before the upstream maintainers accepted
your driver or patch.

I know it is nice to have the firmware available quickly, but if your
driver gets rejected for the reason we have stated in this thread, you
have dangling firmware somewhere.

> I last tried to document a thread we had over this here:
> 
> http://wireless.kernel.org/en/developers/Documentation/firmware-versioning
> 
> Does this sound sane? If so then the sflash configuration fix
> would seem to me like it would require a new filename. Now, while
> we're at it, how about bug fixes?

If your firmware files are identical and the exposed API is identical
(in this case Bluetooth HCI), then you do NO need a new filename.

Regards

Marcel



^ permalink raw reply

* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Sven-Haegar Koch @ 2010-10-08 22:47 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Suraj Sumangala, Luis Rodriguez, David Woodhouse, Marcel Holtmann,
	linux-bluetooth, linux-kernel@vger.kernel.org, linux-wireless
In-Reply-To: <20101008181508.GM10149@tux>

On Fri, 8 Oct 2010, Luis R. Rodriguez wrote:

> > > I last tried to document a thread we had over this here:
> > >
> > > http://wireless.kernel.org/en/developers/Documentation/firmware-versioning
> > >
> 
> Thanks, I've updated that link above to document bug fixing does not require
> a filename change.

I would summarize it as:

If a new firmware version also works with an old driver, keep the filename.

If a new firmware version also requires a new driver, change the name.

If a new driver requires a new firmware, change the name.

c'ya
sven-haegar

-- 
Three may keep a secret, if two of them are dead.
- Ben F.

^ permalink raw reply

* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Luis R. Rodriguez @ 2010-10-08 18:15 UTC (permalink / raw)
  To: Suraj Sumangala
  Cc: Luis Rodriguez, David Woodhouse, Marcel Holtmann, linux-bluetooth,
	linux-kernel@vger.kernel.org, linux-wireless
In-Reply-To: <4CAF5488.3030706@Atheros.com>

On Fri, Oct 08, 2010 at 10:27:36AM -0700, Suraj Sumangala wrote:
> Hi Luis,
> 
> On 10/8/2010 10:32 PM, Luis Rodriguez wrote:
> > Suraj,
> >
> > What is the difference between ath3k-2.fw and ath3k-1.fw ?
> 
> This is the same question for which I have been trying to get an answer.
> The only information that I got was it fixes some critical bug and 
> support shared antenna.
> 
> If ath3k-2.fw is an upgrade of ath3k-1.fw why do we need to name it 
> differently?

Sure, agreed, but what about the sflash configuration fix?

> > Won't the API change now that you are addressing the sflash
> > configuration fix? Would it not help to identify the two
> > different firmwares then?
> >
> > David, Marcel, what are your preferences for a firmware upgrade
> > where the firmware does not change API (lets just pretend it does
> > not for a moment) ? Do we keep the same filename?
> 
> Marcel had answered me before. It makes sense to have same file name.
> Other ways we end up changing the driver whenever there is a firmware 
> change.

> > I last tried to document a thread we had over this here:
> >
> > http://wireless.kernel.org/en/developers/Documentation/firmware-versioning
> >

Thanks, I've updated that link above to document bug fixing does not require
a filename change.

 Luis

^ permalink raw reply

* Re: Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Suraj Sumangala @ 2010-10-08 17:27 UTC (permalink / raw)
  To: Luis Rodriguez
  Cc: Suraj Sumangala, David Woodhouse, Marcel Holtmann,
	linux-bluetooth, linux-kernel@vger.kernel.org, linux-wireless
In-Reply-To: <20101008170258.GJ10149@tux>

Hi Luis,

On 10/8/2010 10:32 PM, Luis Rodriguez wrote:
> Suraj,
>
> What is the difference between ath3k-2.fw and ath3k-1.fw ?

This is the same question for which I have been trying to get an answer.
The only information that I got was it fixes some critical bug and 
support shared antenna.

If ath3k-2.fw is an upgrade of ath3k-1.fw why do we need to name it 
differently?

>
> Won't the API change now that you are addressing the sflash
> configuration fix? Would it not help to identify the two
> different firmwares then?
>
> David, Marcel, what are your preferences for a firmware upgrade
> where the firmware does not change API (lets just pretend it does
> not for a moment) ? Do we keep the same filename?

Marcel had answered me before. It makes sense to have same file name.
Other ways we end up changing the driver whenever there is a firmware 
change.

>
> In this particular case I would assume our new sflash configuration
> fix that might be being worked on might change the re-enumerated
> USB device IDs so it seems to me a good idea to use a new filename.
> I should note ath3k-2.fw already made it to linux-firmware.git...
>
> I last tried to document a thread we had over this here:
>
> http://wireless.kernel.org/en/developers/Documentation/firmware-versioning
>
> Does this sound sane? If so then the sflash configuration fix
> would seem to me like it would require a new filename. Now, while
> we're at it, how about bug fixes?
>
> Suraj -- keep these discussions public please....
>
>    Luis

Regards
Suraj

^ permalink raw reply

* Firmware versioning best practices: ath3k-2.fw rename or replace ath3k-1.fw ?
From: Luis R. Rodriguez @ 2010-10-08 17:02 UTC (permalink / raw)
  To: Suraj Sumangala, David Woodhouse, Marcel Holtmann
  Cc: linux-bluetooth, linux-kernel, linux-wireless

Suraj,

What is the difference between ath3k-2.fw and ath3k-1.fw ?

Won't the API change now that you are addressing the sflash
configuration fix? Would it not help to identify the two
different firmwares then?

David, Marcel, what are your preferences for a firmware upgrade
where the firmware does not change API (lets just pretend it does
not for a moment) ? Do we keep the same filename?

In this particular case I would assume our new sflash configuration
fix that might be being worked on might change the re-enumerated
USB device IDs so it seems to me a good idea to use a new filename.
I should note ath3k-2.fw already made it to linux-firmware.git...

I last tried to document a thread we had over this here:

http://wireless.kernel.org/en/developers/Documentation/firmware-versioning

Does this sound sane? If so then the sflash configuration fix
would seem to me like it would require a new filename. Now, while
we're at it, how about bug fixes?

Suraj -- keep these discussions public please....

  Luis

^ permalink raw reply

* [PATCH] Use a valid PSM as the default in l2test.
From: Mat Martineau @ 2010-10-08 15:16 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, johan.hedberg, rshaffer, gustavo



---------- Forwarded message ----------
Date: Wed, 4 Aug 2010 15:31:13 -0700
From: Mat Martineau <mathewm@codeaurora.org>
To: linux-bluetooth@vger.kernel.org
Cc: marcel@holtmann.org, johan.hedberg@gmail.com, rshaffer@codeaurora.org,
     Mat Martineau <mathewm@codeaurora.org>
Subject: [PATCH] Use a valid PSM as the default in l2test.

The Bluetooth spec requires PSMs to be odd numbers, with the least
significant bit of the most significant byte set to 0.  This
change is a prerequisite for PSM validation in the kernel.
---
  test/l2test.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/l2test.c b/test/l2test.c
index 0bb46f3..32cd27a 100644
--- a/test/l2test.c
+++ b/test/l2test.c
@@ -87,7 +87,7 @@ static long buffer_size = 2048;

  /* Default addr and psm */
  static bdaddr_t bdaddr;
-static unsigned short psm = 10;
+static unsigned short psm = 0x1001;

  /* Default number of frames to send (-1 = infinite) */
  static int num_frames = -1;
-- 
1.7.1


Ping.  Marcel requested this patch to go with the "Validate PSM values 
in calls to connect() and bind()" change to the kernel code.


--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply related

* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-08 14:54 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo F. Padovan
  Cc: linux-bluetooth@vger.kernel.org, greg@kroah.com,
	linux-kernel@vger.kernel.org
In-Reply-To: <1286527062.6145.182.camel@aeonflux>



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

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

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

Thanks,
Pavan

> Regards
>=20
> Marcel
>=20

^ permalink raw reply

* RE: Sim Access profile server implementation
From: Waldemar.Rymarkiewicz @ 2010-10-08 14:27 UTC (permalink / raw)
  To: marcel
  Cc: suraj, linux-bluetooth, Jothikumar.Mothilal, joakim.xj.ceder,
	arunkr.singh
In-Reply-To: <1286466136.6145.159.camel@aeonflux>

Hi Marcel,=20

>in general I am fine if we do this similar to what we do with=20
>the different telephony backend. So yes, go ahead with this.
>
>However, please clean this up. The patch needs a lot of work=20
>before it would be ready upstream. And of course it requires a=20
>dummy SAP plugin as well. Same as we do have for telephony.
>

Then, I will prepare it for upstream.=20

Regards,
/Waldek

^ permalink raw reply

* [PATCH] Reduction memory leaks in phonebook module
From: Rafał Michalski @ 2010-10-08 14:00 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Rafał Michalski

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



[-- Attachment #2: 0001-Reduction-memory-leaks-in-phonebook-module.patch --]
[-- Type: text/x-patch, Size: 965 bytes --]

From 40399b62ec62569213948f7eb77fd39d1ff42ea1 Mon Sep 17 00:00:00 2001
From: Rafal Michalski <michalski.raf@gmail.com>
Date: Fri, 8 Oct 2010 15:16:50 +0200
Subject: [PATCH] Reduction memory leaks in phonebook module

Memory, pointed by pointers home_addr and work_addr, is allocated
temporarily and not freed. Only duplicates of this memory (made after
invoking add_address function) are freed later.
---
 plugins/phonebook-tracker.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index c10491d..d0cea2e 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1007,6 +1007,9 @@ add_numbers:
 	add_address(contact, home_addr, ADDR_TYPE_HOME);
 	add_address(contact, work_addr, ADDR_TYPE_WORK);
 
+	g_free(home_addr);
+	g_free(work_addr);
+
 	DBG("contact %p", contact);
 
 	/* Adding contacts data to wrapper struct - this data will be used to
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH] Bluetooth: update MAINTAINERS for Bluetooth subsys
From: Gustavo F. Padovan @ 2010-10-08 12:24 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel

Add myself to MAINTAINERS and update the git trees.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 MAINTAINERS |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ddb5ac..07ca793 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1351,16 +1351,19 @@ F:	drivers/mtd/devices/block2mtd.c
 
 BLUETOOTH DRIVERS
 M:	Marcel Holtmann <marcel@holtmann.org>
+M:	Gustavo F. Padovan <padovan@profusion.mobi>
 L:	linux-bluetooth@vger.kernel.org
 W:	http://www.bluez.org/
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git
 S:	Maintained
 F:	drivers/bluetooth/
 
 BLUETOOTH SUBSYSTEM
 M:	Marcel Holtmann <marcel@holtmann.org>
+M:	Gustavo F. Padovan <padovan@profusion.mobi>
 L:	linux-bluetooth@vger.kernel.org
 W:	http://www.bluez.org/
-T:	git git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git
 S:	Maintained
 F:	net/bluetooth/
 F:	include/net/bluetooth/
-- 
1.7.3.1

^ permalink raw reply related

* Re: Peculiar stuff in hci_ath3k/badness in hci_uart
From: Alan Cox @ 2010-10-08 10:48 UTC (permalink / raw)
  To: Suraj Sumangala
  Cc: linux-kernel@vger.kernel.org, marcel@holtmann.org,
	linux-bluetooth@vger.kernel.org
In-Reply-To: <4CAEE1DB.1070202@Atheros.com>

On Fri, 8 Oct 2010 14:48:19 +0530
Suraj Sumangala <suraj@Atheros.com> wrote:

> Hi Alan,
> 
> On 10/8/2010 2:11 AM, Alan Cox wrote:
> > Noticed this while looking at Savoy's stuff
> >
> >
> > ath_wakeup_ar3k:
> >
> >         int status = tty->driver->ops->tiocmget(tty, NULL);
> >
> I agree, it will be a problem if the underlying driver try to access the 
> "struct file". Is there any other way I can get the MCR status here 
> without providing user space pointer?

Currently no - that should get fixed in the tty layer and I will look
into it end of next week as a priority.

Alan

^ permalink raw reply

* Re: Peculiar stuff in hci_ath3k/badness in hci_uart
From: Suraj Sumangala @ 2010-10-08  9:18 UTC (permalink / raw)
  To: Alan Cox
  Cc: linux-kernel@vger.kernel.org, marcel@holtmann.org,
	linux-bluetooth@vger.kernel.org
In-Reply-To: <20101007214134.6bcd2e8f@lxorguk.ukuu.org.uk>

Hi Alan,

On 10/8/2010 2:11 AM, Alan Cox wrote:
> Noticed this while looking at Savoy's stuff
>
>
> ath_wakeup_ar3k:
>
>         int status = tty->driver->ops->tiocmget(tty, NULL);
>
I agree, it will be a problem if the underlying driver try to access the 
"struct file". Is there any other way I can get the MCR status here 
without providing user space pointer?

>
> 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);
>
Regards
Suraj

^ permalink raw reply

* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Marcel Holtmann @ 2010-10-08  8:37 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: pavan_savoy, linux-bluetooth, greg, linux-kernel
In-Reply-To: <20101007184534.GB13602@vigoh>

Hi Gustavo,

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

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

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

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

Regards

Marcel



^ permalink raw reply

* Re: [PATCH] Fix problem with invalid read from array
From: Lukasz Pawlik @ 2010-10-08  8:35 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <AANLkTim4WEh7ZLTiWbhiWcv1VXV+jpb_V8NqJjeq=OzH@mail.gmail.com>

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

Attaching patch without change in src/adapter.c file.

Lukasz Pawlik

2010/10/6 Lukasz Pawlik <lucas.pawlik@gmail.com>:
> Hi,
>
> Sorry. My bad. It was never my intention to change src/adapter.c. I've
> prepared two patches with the same name and of course send the wrong
> one. Problem with invalid read fix change made in src/sdpd-service.c
> file.
>
> Lukasz
>
> 2010/10/6 Johan Hedberg <johan.hedberg@gmail.com>:
>> Hi Lukasz,
>>
>> On Wed, Oct 06, 2010, Lukasz Pawlik wrote:
>>> This patch fix problem with reading data from out of the array range in
>>> function used to create EIR response.
>>
>> You'll need to explain in more detail exactly what was wrong with the
>> old code and how your patch fixes it (and why it is the correct fix).
>>
>>> -     uint8_t data[240];
>>> +     uint8_t data[242];
>>
>> Why 242? The core spec defines the EIR data as a 240 byte field.
>>
>>> -                                     uuid128_data[SIZEOF_UUID128 - k])
>>> +                                     uuid128_data[SIZEOF_UUID128 - 1 - k])
>>
>> This change looks fine (the index of the last byte is sizeof(uuid128) - 1).
>>
>> Johan
>>
>

[-- Attachment #2: 0001-Fix-problem-with-invalid-read-from-array.patch --]
[-- Type: text/x-patch, Size: 876 bytes --]

From 5e6ca8e9dff0ced5aacc1cbfa12318680ade957a Mon Sep 17 00:00:00 2001
From: Lukasz Pawlik <lucas.pawlik@gmail.com>
Date: Fri, 8 Oct 2010 09:23:26 +0200
Subject: [PATCH] Fix problem with invalid read from array

This patch fix problem with reading data from out of the array range in
function used to create EIR response.
---
 src/sdpd-service.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 26ab9a5..67dd9af 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -204,7 +204,7 @@ static void eir_generate_uuid128(sdp_list_t *list,
 		for (i = 0; i < index; i++) {
 			for (k = 0; k < SIZEOF_UUID128; k++) {
 				if (uuid128[i * SIZEOF_UUID128 + k] !=
-					uuid128_data[SIZEOF_UUID128 - k])
+					uuid128_data[SIZEOF_UUID128 - 1 - k])
 					break;
 			}
 			if (k == SIZEOF_UUID128)
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH] Bluetooth: hci_uart: Fix typo in stats for sco tx
From: Marcel Holtmann @ 2010-10-08  8:33 UTC (permalink / raw)
  To: Karl Beldan; +Cc: Jiri Kosina, linux-bluetooth
In-Reply-To: <1286481430-6761-1-git-send-email-karl.beldan@gmail.com>

Hi Karl,

> 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>

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/2] drivers:staging:ti-st: move TI_ST from staging
From: Marcel Holtmann @ 2010-10-08  8:32 UTC (permalink / raw)
  To: Alan Cox
  Cc: Savoy, Pavan, Jiri Slaby, gregkh@suse.de,
	linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-bluetooth
In-Reply-To: <20101007213503.657bcab0@lxorguk.ukuu.org.uk>

Hi Alan,

> > 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 ?

If you get a sane proposal, then yes, N_HCI could act as multiplexer and
forward certain frame types.

This all depends on how clear the separation is. If you expect to send
HCI commands from these "clients" then there is a problem with the
Bluetooth subsystem and its enforced flow control. You don't wanna mess
with that since it has side effects. And I am not giving outside drivers
real control over. The Bluetooth drivers have to stay dumb transport
drivers without any Bluetooth core logic.

So can you give me a few quick hints on what you would need to forward
actually.

Regards

Marcel



^ permalink raw reply

* RE: RFC: btusb firmware load help
From: Marcel Holtmann @ 2010-10-08  8:26 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Shanmugamkamatchi Balashanmugam, Luis Rodriguez, linux-bluetooth,
	linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	Deepak Dhamdhere, Sree Durbha
In-Reply-To: <1286474359.20974.13.camel@jlt3.sipsolutions.net>

Hi Johannes,

> > 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.

sounds a lot better than what I understood initially. So essentially the
PID assigned for their different devices got screwed up. So that needs
to be fixed now.

Regards

Marcel



^ permalink raw reply

* Re: RFC: btusb firmware load help
From: Marcel Holtmann @ 2010-10-08  8:24 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: <4CADFE1E.1070708@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.
> >
> >
> 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.

if it loads the same firmware into the device, then it is just fine to
share a PID. If it loads different firmware, then I would propose
different PIDs.

So yes, you can use the PID since you wanna keep the firmware the same.
That is perfectly fine and reasonable.

Regards

Marcel



^ permalink raw reply

* [PATCH] Bluetooth: TI_ST bluetooth driver
From: pavan_savoy @ 2010-10-08  0:23 UTC (permalink / raw)
  To: linux-bluetooth, marcel; +Cc: greg, linux-kernel, Pavan Savoy

From: Pavan Savoy <pavan_savoy@ti.com>

Gustavo, Marcel,

I have taken care of most of the comments which you provided.
Couple of them like usage of goto's, I didn't handle since
I am not doing much error handling repeatedly there.

I have also removed the 2 states like _REGISTERED, _RUNNING which
were not required and the flags associated with it.
I have taken care of other style comments.

Please review and provide your comments,

-- patch description --

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

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

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

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/bluetooth/Kconfig  |   10 +
 drivers/bluetooth/Makefile |    1 +
 drivers/bluetooth/ti_st.c  |  455 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 466 insertions(+), 0 deletions(-)
 create mode 100644 drivers/bluetooth/ti_st.c

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..b2e88cb 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)			:= ti_st.o
diff --git a/drivers/bluetooth/ti_st.c b/drivers/bluetooth/ti_st.c
new file mode 100644
index 0000000..e0bde79
--- /dev/null
+++ b/drivers/bluetooth/ti_st.c
@@ -0,0 +1,455 @@
+/*
+ *  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 */
+
+/**
+ * struct ti_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 ti_st_open
+ *	and ti_st_registration_completion_cb.
+ */
+struct ti_st {
+	struct hci_dev *hdev;
+	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 ti_st_tx_complete(struct ti_st *hst, int pkt_type)
+{
+	struct hci_dev *hdev;
+	hdev = hst->hdev;
+
+	/* Update HCI stat counters */
+	switch (pkt_type) {
+	case HCI_COMMAND_PKT:
+		hdev->stat.cmd_tx++;
+		break;
+
+	case HCI_ACLDATA_PKT:
+		hdev->stat.acl_tx++;
+		break;
+
+	case HCI_SCODATA_PKT:
+		hdev->stat.sco_tx++;
+		break;
+	}
+}
+
+/* ------- Interfaces to Shared Transport ------ */
+
+/* Called by ST layer to indicate protocol registration completion
+ * status.ti_st_open() function will wait for signal from this
+ * API when st_register() function returns ST_PENDING.
+ */
+static void st_registration_completion_cb(void *priv_data, char data)
+{
+	struct ti_st *lhst = (struct ti_st *)priv_data;
+	/* ti_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 ti_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 st_receive(void *priv_data, struct sk_buff *skb)
+{
+	int err;
+	struct ti_st *lhst = (struct ti_st *)priv_data;
+
+	if (skb == NULL) {
+		BT_ERR("Invalid SKB received from ST");
+		return -EFAULT;
+	}
+
+	if (!lhst) {
+		kfree_skb(skb);
+		BT_ERR("Invalid ti_st memory,freeing SKB");
+		return -EFAULT;
+	}
+
+	skb->dev = (struct net_device *)lhst->hdev;
+	/* Forward skb to HCI CORE layer */
+	err = hci_recv_frame(skb);
+	if (err) {
+		kfree_skb(skb);
+		BT_ERR("Unable to push skb to HCI CORE(%d),freeing SKB",
+				err);
+		return err;
+	}
+
+	lhst->hdev->stat.byte_rx += skb->len;
+	return 0;
+}
+
+/* ------- Interfaces to HCI layer ------ */
+
+/* Called from HCI core to initialize the device */
+static int ti_st_open(struct hci_dev *hdev)
+{
+	static struct st_proto_s ti_st_proto;
+	unsigned long timeleft;
+	struct ti_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(&ti_st_proto, 0, sizeof(ti_st_proto));
+
+	/* BT driver ID */
+	ti_st_proto.type = ST_BT;
+
+	/* Receive function which called from ST */
+	ti_st_proto.recv = st_receive;
+
+	/* Callback to be called when registration is pending */
+	ti_st_proto.reg_complete_cb = st_registration_completion_cb;
+
+	/* send in the hst to be received at registration complete callback
+	 * and during st's receive
+	 */
+	ti_st_proto.priv_data = hst;
+
+	/* Register with ST layer */
+	err = st_register(&ti_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 ti_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 (ti_st_proto.write != NULL) {
+		/* We need this pointer for sending any Bluetooth pkts */
+		hst->st_write = ti_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);
+	return err;
+}
+
+/* Close device */
+static int ti_st_close(struct hci_dev *hdev)
+{
+	int err = 0;
+	struct ti_st *hst;
+
+	hst = hdev->driver_data;
+
+	/* Clear HCI device RUNNING flag */
+	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
+		BT_ERR("HCI not RUNNING?");
+
+	/* continue to unregister from transport */
+	err = st_unregister(ST_BT);
+	if (err != 0) {
+		hst->st_write = NULL;
+		BT_ERR("st_unregister failed %d", err);
+		return -EBUSY;
+	}
+
+	hst->st_write = NULL;
+	return err;
+}
+
+/* Called from HCI CORE , Sends frames to Shared Transport */
+static int ti_st_send_frame(struct sk_buff *skb)
+{
+	struct hci_dev *hdev;
+	struct ti_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 ti_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) {
+		kfree_skb(skb);
+		BT_ERR(" ST write failed (%ld)", len);
+		return -EAGAIN;
+	}
+
+	/* ST accepted our skb. So, Go ahead and do rest */
+	hdev->stat.byte_tx += len;
+	ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
+
+	return 0;
+}
+
+static void ti_st_destruct(struct hci_dev *hdev)
+{
+	if (!hdev) {
+		BT_ERR("Destruct called with invalid HCI Device"
+				"(hdev=NULL)");
+		return;
+	}
+
+	BT_DBG("%s", hdev->name);
+
+	/* free ti_st memory */
+	if (hdev->driver_data != NULL)
+		kfree(hdev->driver_data);
+
+	return;
+}
+
+/* Creates new HCI device */
+static int ti_st_register_dev(struct ti_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 = ti_st_open;
+	hdev->close = ti_st_close;
+	hdev->flush = NULL;
+	hdev->send = ti_st_send_frame;
+	hdev->destruct = ti_st_destruct;
+	hdev->owner = THIS_MODULE;
+
+	if (reset)
+		set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
+
+	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 ti_st *hst;
+	err = 0;
+
+	BT_DBG(" Bluetooth Driver Version %s", VERSION);
+
+	/* Allocate local resource memory */
+	hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
+	if (!hst) {
+		BT_ERR("Can't allocate control structure");
+		return -ENFILE;
+	}
+
+	/* Expose "hciX" device to user space */
+	err = ti_st_register_dev(hst);
+	if (err) {
+		/* Release local resource memory */
+		kfree(hst);
+
+		BT_ERR("Unable to expose hci0 device(%d)", err);
+		return err;
+	}
+
+	dev_set_drvdata(&pdev->dev, hst);
+	return err;
+}
+
+static int bt_ti_remove(struct platform_device *pdev)
+{
+	struct ti_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 {
+			ti_st_close(hdev);
+			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

* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Greg KH @ 2010-10-07 22:17 UTC (permalink / raw)
  To: Gustavo F. Padovan
  Cc: Marcel Holtmann, pavan-savoy, linux-bluetooth, johan.hedberg,
	linux-kernel, Pavan Savoy
In-Reply-To: <20101007190906.GE4893@vigoh>

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

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

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

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

thanks,

greg k-h

^ permalink raw reply

* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 21:53 UTC (permalink / raw)
  To: Gustavo F. Padovan
  Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
	greg@kroah.com, linux-kernel@vger.kernel.org
In-Reply-To: <20101007184534.GB13602@vigoh>

Gustavo / Marcel,
=20

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

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

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

and I await marcel's comment on this.

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

^ permalink raw reply

* Re: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Greg KH @ 2010-10-07 21:30 UTC (permalink / raw)
  To: Gustavo F. Padovan
  Cc: Marcel Holtmann, pavan-savoy, linux-bluetooth, johan.hedberg,
	linux-kernel, Pavan Savoy
In-Reply-To: <20101007175148.GC4893@vigoh>

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

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

thanks,

greg k-h

^ permalink raw reply

* RE: [PATCH 1/2] drivers:bluetooth: TI_ST bluetooth driver
From: Savoy, Pavan @ 2010-10-07 21:24 UTC (permalink / raw)
  To: Gustavo F. Padovan, Marcel Holtmann
  Cc: Greg KH, pavan-savoy@ti.com, linux-bluetooth@vger.kernel.org,
	johan.hedberg@gmail.com, linux-kernel@vger.kernel.org
In-Reply-To: <20101007175715.GA13602@vigoh>

Marcel, Gustavo,

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

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

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


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

^ permalink raw reply


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