Linux bluetooth development
 help / color / mirror / Atom feed
* Re: ROM Patching (was: [PATCH] bluetooth: remove wrong dependency for BT_ATH3K)
From: Luis R. Rodriguez @ 2013-07-31  0:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Marcel Holtmann, Hector Palacios, linux-wireless,
	linux-bluetooth@vger.kernel.org, gustavo@padovan.org,
	johan.hedberg@gmail.com, mcgrof, linux-kernel@vger.kernel.org,
	surajs@qca.qualcomm.com, jjohnson, Adrian Chadd, ddahlby,
	Ben Hutchings
In-Reply-To: <20130730235508.GA25319@kroah.com>

On Tue, Jul 30, 2013 at 04:55:08PM -0700, Greg Kroah-Hartman wrote:
> On Tue, Jul 30, 2013 at 03:48:09PM -0700, Luis R. Rodriguez wrote:
> > > We are planning to take one extra step and split this into a
> > > mini-driver approach similar to what has been done for usbnet, but we
> > > are not there yet.
> > 
> > Neat. Perhaps we need something that we can share with 802.11 or other
> > hardare I highly doubt we're the only ones patching ROM. Don't we even
> > patch up core CPUs? I'm wondering if firmware_class could be expanded to
> > support serialized ROM patching. The biggest hurdle I see with splititng
> > ROM patching from a single firmware is serializing that, addressing
> > revision dependencies and of course kernel dependencies.
> 
> What does the firmware_class need to do here that it doesn't do today?

It will depend on the requirements for ROM patches and if or not
firmware can be split up into patches rather than getting a full
firmware.fw update for any single patch update. If ROM patches
get split up then how I can imagine driver code firmware getting
to become a pain in the ass to maintain and nasty. It'd seem
better to build relationships between these and possible patch
depdendencies on ROM and let firmware_class do the management
of that.

> > > However the ROM patching drivers need to be in the kernel since
> > > otherwise they will race with the core init sequence.
> > 
> > Sure and depending on the architecture -- if this is kicked off to
> > userspace helpers or not then we may need to consider dbus in
> > kernel thing to help with speed / races, dependenecies / async
> > loading, -EPROBE_DEFER, etc.
> 
> How would dbus in the kernel change anything here?
> The kernel directly loads firmware from the filesystem now, no userspace
> helpers are involved, so no need for udev, libusb, etc.  And as such, no
> need for any "dbus" in the kernel to do this either.
> I don't understand how it could, please explain.

Yeah nevermind :D too much coffee today made my brain fart.

> Also note, for those not knowing, we have been working on dbus in the
> kernel (google "kdbus" for the github repo), but it is all
> outward-facing (i.e. userspace using the kdbus code to interact with
> other processes with a dbus-like protocol, not for in-kernel dbus
> things, although adding it wouldn't be that hard, just really strange.

Thanks!

  Luis

^ permalink raw reply

* Re: ROM Patching (was: [PATCH] bluetooth: remove wrong dependency for BT_ATH3K)
From: Greg Kroah-Hartman @ 2013-07-30 23:55 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Marcel Holtmann, Hector Palacios, linux-wireless,
	linux-bluetooth@vger.kernel.org, gustavo@padovan.org,
	johan.hedberg@gmail.com, mcgrof, linux-kernel@vger.kernel.org,
	surajs@qca.qualcomm.com, jjohnson, Adrian Chadd, ddahlby,
	Ben Hutchings
In-Reply-To: <20130730224809.GN17130@pogo>

On Tue, Jul 30, 2013 at 03:48:09PM -0700, Luis R. Rodriguez wrote:
> > We are planning to take one extra step and split this into a
> > mini-driver approach similar to what has been done for usbnet, but we
> > are not there yet.
> 
> Neat. Perhaps we need something that we can share with 802.11 or other
> hardare I highly doubt we're the only ones patching ROM. Don't we even
> patch up core CPUs? I'm wondering if firmware_class could be expanded to
> support serialized ROM patching. The biggest hurdle I see with splititng
> ROM patching from a single firmware is serializing that, addressing
> revision dependencies and of course kernel dependencies.

What does the firmware_class need to do here that it doesn't do today?

> > However the ROM patching drivers need to be in the kernel since
> > otherwise they will race with the core init sequence.
> 
> Sure and depending on the architecture -- if this is kicked off to
> userspace helpers or not then we may need to consider dbus in
> kernel thing to help with speed / races, dependenecies / async
> loading, -EPROBE_DEFER, etc.

How would dbus in the kernel change anything here?

> > There are also just firmware download drivers that do nothing else
> > than just loading the firmware. And here it can be done via userspace
> > or kernel space. In the Bluetooth world we have seen both, but
> > generally the kernel ones stayed around while the userspace ones had a
> > hard time to work with udev, libusb, libusb1 etc.
> 
> Ah I see. Perhaps we can address this once we get some form of dbus in
> kernel.

The kernel directly loads firmware from the filesystem now, no userspace
helpers are involved, so no need for udev, libusb, etc.  And as such, no
need for any "dbus" in the kernel to do this either.

> > For UART based drivers it is a little bit different since we had to
> > bring up the line discipline from userspace anyway and configure all
> > the UART parameters. For these drivers the firmware download or ROM
> > patching has been done normally via userspace since there is full
> > exclusive access before the Bluetooth subsystem knows about the
> > device.
> 
> Is this something that dbus in kernel can help clean up a bit?

I don't understand how it could, please explain.

Also note, for those not knowing, we have been working on dbus in the
kernel (google "kdbus" for the github repo), but it is all
outward-facing (i.e. userspace using the kdbus code to interact with
other processes with a dbus-like protocol, not for in-kernel dbus
things, although adding it wouldn't be that hard, just really strange.

thanks,

greg k-h

^ permalink raw reply

* Re: gatt advertising
From: Anderson Lizardo @ 2013-07-30 23:08 UTC (permalink / raw)
  To: Dirk-Jan Binnema; +Cc: linux-bluetooth
In-Reply-To: <CALvrRrZxEZRuohxeeuTqU1VE9TfBHudwH9LL_1v3-oG+DNojMQ@mail.gmail.com>

Hi Dirk,

On Tue, Jul 30, 2013 at 6:50 PM, Dirk-Jan Binnema <djcb.bulk@gmail.com> wrote:
> One other thing I've hit upon is that apparently, long-writes/reads are
> supported by ATT yet (looking at channel_handler, and seeing some
> 'Unsupported request 0x16'), although it seems Bluez can do long-writes
> itself.

If I remember correctly, indeed we only support Read/Write Long
Characteristic Values on the GATT client code. We have been working on
refactoring the attribute server, but the code is not ready yet to be
sent upstream.

> Are there plans to add long-writes/reads to ATT? Or would this be very
> easy to add? (I suppose not!).

I'm not aware of anyone working on this at the moment on the attribute
server code. Regarding complexity, I think implementing Read Blob
Request would be easy, Prepare Write / Execute Write Request may be
more difficult, as you need to keep state saved and support more than
one active client at the same time.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply

* Re: gatt advertising
From: Dirk-Jan Binnema @ 2013-07-30 22:50 UTC (permalink / raw)
  To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <CAJdJm_NX2YopqON=Szv3X2DuuYhrGZcRPcNrE=BjdV+NVQtEnA@mail.gmail.com>

Hi Anderson,

anderson.lizardo@openbossa.org writes:

First of all, thank you for your extremely helpful reply!

> On Tue, Jul 30, 2013 at 9:08 AM, Dirk-Jan Binnema <djcb.bulk@gmail.com> wrote:
>> Hi all,
>>

> First of all, BlueZ (and the Linux kernel) does not support the
> "Peripheral" role fully yet, which is what you want here.
>
> Therefore, you need to do a few things by hand.
>
>> - Is any extra step is needed to advertise the service beyond
>>   gatt_service_add and 'hciconfig leadv'?
>
> You need to manually add the UUIDs on the LE advertising data. For
> this, currently you need to send the "LE Set Advertising Data" HCI
> command manually to the controller (e.g. using "hcitool cmd"). See the
> Core Spec page 1062 for the HCI command format, and Section 11
> "Advertising and Scan Response Data Format (page 1735), and Appendix C
> "EIR and AD Formats" (page 1762) for the Advertising Data format
> specifically.

Yes, this seems to work. For other people trying the same:

hcitool -i hci0 cmd 0x08 0x0008 12 11 07 xx xx xx xx xx xx xx xx \
       xx xx xx xx xx xx xx xx 00 00 00 00 00 00 00 00 00 00 00 00 00

will do the advertisement when using a 128-bit UUID (the xx part,
'MY_SERVICE_ID') -- and importantly, this needs to be encoded, basically
the bytes you get from the following:

    bt_string_to_uuid (&srv_uuid, MY_SERVICE_UUID);
    for (u = 0; u != 16; ++u)
         printf ("%x ", ((uint8_t*)&srv_uuid.value)[u]);


For 16-bit UUIDs, you need to change the length bytes (the 12 and 11),
see the docs above for the details.

>>   (It seems that neither mine, nor any of the other services (proximity,
>>   time, alert, ...) are recognized until after I connect(*). After
>>   connection, I can see all of them.
>
> They are only visible after connection probably because iPhone
> triggers a Service discovery when BlueZ connects to it.
>
>> - It also seems the device stops advertising after a while -- is there
>>   some way to programmatically put it in advertising mode (like
>>   hciconfig leadv) and keep it that way?
>
> Actually, advertising stops as soon as a connection is established.
> This is a controller feature, not controllable by BlueZ. You need to
> enable advertising again manually.

Okay, that's good to know!

One other thing I've hit upon is that apparently, long-writes/reads are
supported by ATT yet (looking at channel_handler, and seeing some
'Unsupported request 0x16'), although it seems Bluez can do long-writes
itself.

Are there plans to add long-writes/reads to ATT? Or would this be very
easy to add? (I suppose not!).

Thanks,
Dirk.


-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

On Tue, Jul 30, 2013 at 8:03 AM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Dirk,
>
> On Tue, Jul 30, 2013 at 9:08 AM, Dirk-Jan Binnema <djcb.bulk@gmail.com> wrote:
>> Hi all,
>>
>> I'm trying to build a GATT-based service that exposes some
>> characteristics that can be accessed by clients; I'm using
>> gatt_add_service_add for that, and the BLExplr app on the iPhone side.
>>
>> The good news is that it seems to be possibly to do so :-) (I'm using
>> git master + the 128-bit uuid patch I wrote); however, it seems the
>> service is not advertised; I need to connect to the device first before
>> I can see it; at least when trying with an iPhone.
>
> First of all, BlueZ (and the Linux kernel) does not support the
> "Peripheral" role fully yet, which is what you want here.
>
> Therefore, you need to do a few things by hand.
>
>> - Is any extra step is needed to advertise the service beyond
>>   gatt_service_add and 'hciconfig leadv'?
>
> You need to manually add the UUIDs on the LE advertising data. For
> this, currently you need to send the "LE Set Advertising Data" HCI
> command manually to the controller (e.g. using "hcitool cmd"). See the
> Core Spec page 1062 for the HCI command format, and Section 11
> "Advertising and Scan Response Data Format (page 1735), and Appendix C
> "EIR and AD Formats" (page 1762) for the Advertising Data format
> specifically.
>
>>   (It seems that neither mine, nor any of the other services (proximity,
>>   time, alert, ...) are recognized until after I connect(*). After
>>   connection, I can see all of them.
>
> They are only visible after connection probably because iPhone
> triggers a Service discovery when BlueZ connects to it.
>
>> - It also seems the device stops advertising after a while -- is there
>>   some way to programmatically put it in advertising mode (like
>>   hciconfig leadv) and keep it that way?
>
> Actually, advertising stops as soon as a connection is established.
> This is a controller feature, not controllable by BlueZ. You need to
> enable advertising again manually.
>
> Note that most controllers on the market (if not all of them) obey the
> LE topology, so when they are slave in a connection, they will
> probably not be able to send connectable advertisements. You will only
> be able to enable advertising again once the link is disconnected.
>
>> - Should the UUID of the service be part of the UUIDs property in
>>   org.Bluez.Adapter1 and org.bluez.Device1? (I tried 'btmgmt aad-uuid
>>   <uuid> 0' and then then <uuid> becomes visible in the
>>   org.bluez.Device1 UUIDs, but sadly, the iPhone still doesn't seem to
>>   see it, pre-connection).
>
> As BlueZ does not support peripheral role yet, the D-Bus API does not
> have any effect on this situation.
>
>> (*) but for some reason, in the aforementioned app, I can see the 'Tx Power
>> Level' value before connection.
>
> This app is most likely reading the TX Power value which is inserted
> on the LE Advertising Data by the kernel. BlueZ does not have a
> working TX Power service yet.
>
> Best Regards,
> --
> Anderson Lizardo
> Instituto Nokia de Tecnologia - INdT
> Manaus - Brazil

^ permalink raw reply

* Re: ROM Patching (was: [PATCH] bluetooth: remove wrong dependency for BT_ATH3K)
From: Luis R. Rodriguez @ 2013-07-30 22:48 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Hector Palacios, linux-wireless, linux-bluetooth@vger.kernel.org,
	gustavo@padovan.org, johan.hedberg@gmail.com, mcgrof,
	linux-kernel@vger.kernel.org, surajs@qca.qualcomm.com, jjohnson,
	Adrian Chadd, ddahlby, Ben Hutchings, Greg Kroah-Hartman
In-Reply-To: <2C94C740-76BA-4058-B0D7-C37D44875CA1@holtmann.org>

CC'ing linux-wireless as ROM patching is mentioned and in so far as
802.11 mobile is concerned this is a pretty frequent practice there
as well so figured we'd tie in the conversations.

On Tue, Jul 30, 2013 at 03:05:18PM -0700, Marcel Holtmann wrote:
> Hi Luis,
> 
> >> This brings an interesting question: shouldn't the firmware download
> >> part be isolated from the USB driver? After all, I want to
> >> communicate with a UART bluetooth chip.
> > 
> > There are a few BT firmware upload modules (last I checked at least 2),
> > I believe it should be possible to stuff all that code a shared
> > module or even as FreeBSD does it -- treat fw uploading in userspace,
> > however just keep in mind for quirks [0]. So patches welcomed.
> 
> it really depends on what kind of patching or firmware download has to be done.
> 
> For all the ROM patching via HCI commands, we explicitly added support
> for setup stage

This stage is what is I was referring to.

> and exposed a standard way of getting in front of
> Bluetooth core init sequence.

Nice.

> An example here would be the Intel
> Bluetooth devices that require ROM patching. This can be useful for
> many Bluetooth devices that are generally USB transport standard
> compliant, but need a few extra commands of vendor setup.

Interesting. I believe for 802.11 we just throw in a bunch of ROM
patching onto the 802.11 driver_firwmare.fw file and then assume
the driver can do the right thing. I however haven't worked on that so I
am curious if someone more familiar with this can provide details of
how that happens. I do know that the 802.11 mobile drivers get tons of
firmware updates because of this exact architectural choice and do
wonder how we can easily keep up with the firmware updates in meaninful
way to linux-firwmare.

How often do the ROM patches get updated? And do the HCI commands
trigger a userspace event?

> We are planning to take one extra step and split this into a
> mini-driver approach similar to what has been done for usbnet, but we
> are not there yet.

Neat. Perhaps we need something that we can share with 802.11 or other
hardare I highly doubt we're the only ones patching ROM. Don't we even
patch up core CPUs? I'm wondering if firmware_class could be expanded to
support serialized ROM patching. The biggest hurdle I see with splititng
ROM patching from a single firmware is serializing that, addressing
revision dependencies and of course kernel dependencies.

I've thrown this on the list of topics for the next wireless summit
at New Orleans.

> However the ROM patching drivers need to be in the kernel since
> otherwise they will race with the core init sequence.

Sure and depending on the architecture -- if this is kicked off to
userspace helpers or not then we may need to consider dbus in
kernel thing to help with speed / races, dependenecies / async
loading, -EPROBE_DEFER, etc.

> There are also just firmware download drivers that do nothing else
> than just loading the firmware. And here it can be done via userspace
> or kernel space. In the Bluetooth world we have seen both, but
> generally the kernel ones stayed around while the userspace ones had a
> hard time to work with udev, libusb, libusb1 etc.

Ah I see. Perhaps we can address this once we get some form of dbus in
kernel.

> For UART based drivers it is a little bit different since we had to
> bring up the line discipline from userspace anyway and configure all
> the UART parameters. For these drivers the firmware download or ROM
> patching has been done normally via userspace since there is full
> exclusive access before the Bluetooth subsystem knows about the
> device.

Is this something that dbus in kernel can help clean up a bit?

> With the newly introduced setup stage, the drivers could actually
> share the setup handling for UART and USB based devices. Not sure if
> any vendor is going for this approach.

Interesting... Is the architecture documented somewhere? Is this
upstream already?

  Luis

^ permalink raw reply

* Re: [PATCH] bluetooth: remove wrong dependency for BT_ATH3K
From: Luis R. Rodriguez @ 2013-07-30 20:56 UTC (permalink / raw)
  To: Hector Palacios
  Cc: Marcel Holtmann, linux-bluetooth@vger.kernel.org,
	gustavo@padovan.org, johan.hedberg@gmail.com,
	linux-kernel@vger.kernel.org, surajs@qca.qualcomm.com
In-Reply-To: <51F7722E.9050708@digi.com>

On Tue, Jul 30, 2013 at 09:58:38AM +0200, Hector Palacios wrote:
> This brings an interesting question: shouldn't the firmware download
> part be isolated from the USB driver? After all, I want to
> communicate with a UART bluetooth chip.

There are a few BT firmware upload modules (last I checked at least 2),
I believe it should be possible to stuff all that code a shared
module or even as FreeBSD does it -- treat fw uploading in userspace,
however just keep in mind for quirks [0]. So patches welcomed.

[0] http://wireless.kernel.org/en/users/Drivers/ath3k#AR3011_over_USB

  Luis

^ permalink raw reply

* Re: gatt advertising
From: Anderson Lizardo @ 2013-07-30 15:03 UTC (permalink / raw)
  To: Dirk-Jan Binnema; +Cc: linux-bluetooth
In-Reply-To: <CALvrRrayFrDqhm7hoDRpdnbaXhpzaSq9Q7+4GoXEnqBi82KcKQ@mail.gmail.com>

Hi Dirk,

On Tue, Jul 30, 2013 at 9:08 AM, Dirk-Jan Binnema <djcb.bulk@gmail.com> wrote:
> Hi all,
>
> I'm trying to build a GATT-based service that exposes some
> characteristics that can be accessed by clients; I'm using
> gatt_add_service_add for that, and the BLExplr app on the iPhone side.
>
> The good news is that it seems to be possibly to do so :-) (I'm using
> git master + the 128-bit uuid patch I wrote); however, it seems the
> service is not advertised; I need to connect to the device first before
> I can see it; at least when trying with an iPhone.

First of all, BlueZ (and the Linux kernel) does not support the
"Peripheral" role fully yet, which is what you want here.

Therefore, you need to do a few things by hand.

> - Is any extra step is needed to advertise the service beyond
>   gatt_service_add and 'hciconfig leadv'?

You need to manually add the UUIDs on the LE advertising data. For
this, currently you need to send the "LE Set Advertising Data" HCI
command manually to the controller (e.g. using "hcitool cmd"). See the
Core Spec page 1062 for the HCI command format, and Section 11
"Advertising and Scan Response Data Format (page 1735), and Appendix C
"EIR and AD Formats" (page 1762) for the Advertising Data format
specifically.

>   (It seems that neither mine, nor any of the other services (proximity,
>   time, alert, ...) are recognized until after I connect(*). After
>   connection, I can see all of them.

They are only visible after connection probably because iPhone
triggers a Service discovery when BlueZ connects to it.

> - It also seems the device stops advertising after a while -- is there
>   some way to programmatically put it in advertising mode (like
>   hciconfig leadv) and keep it that way?

Actually, advertising stops as soon as a connection is established.
This is a controller feature, not controllable by BlueZ. You need to
enable advertising again manually.

Note that most controllers on the market (if not all of them) obey the
LE topology, so when they are slave in a connection, they will
probably not be able to send connectable advertisements. You will only
be able to enable advertising again once the link is disconnected.

> - Should the UUID of the service be part of the UUIDs property in
>   org.Bluez.Adapter1 and org.bluez.Device1? (I tried 'btmgmt aad-uuid
>   <uuid> 0' and then then <uuid> becomes visible in the
>   org.bluez.Device1 UUIDs, but sadly, the iPhone still doesn't seem to
>   see it, pre-connection).

As BlueZ does not support peripheral role yet, the D-Bus API does not
have any effect on this situation.

> (*) but for some reason, in the aforementioned app, I can see the 'Tx Power
> Level' value before connection.

This app is most likely reading the TX Power value which is inserted
on the LE Advertising Data by the kernel. BlueZ does not have a
working TX Power service yet.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply

* gatt advertising
From: Dirk-Jan Binnema @ 2013-07-30 13:08 UTC (permalink / raw)
  To: linux-bluetooth

Hi all,

I'm trying to build a GATT-based service that exposes some
characteristics that can be accessed by clients; I'm using
gatt_add_service_add for that, and the BLExplr app on the iPhone side.

The good news is that it seems to be possibly to do so :-) (I'm using
git master + the 128-bit uuid patch I wrote); however, it seems the
service is not advertised; I need to connect to the device first before
I can see it; at least when trying with an iPhone.

So, a few questions:

- Is any extra step is needed to advertise the service beyond
  gatt_service_add and 'hciconfig leadv'?

  (It seems that neither mine, nor any of the other services (proximity,
  time, alert, ...) are recognized until after I connect(*). After
  connection, I can see all of them.

- It also seems the device stops advertising after a while -- is there
  some way to programmatically put it in advertising mode (like
  hciconfig leadv) and keep it that way?

- Should the UUID of the service be part of the UUIDs property in
  org.Bluez.Adapter1 and org.bluez.Device1? (I tried 'btmgmt aad-uuid
  <uuid> 0' and then then <uuid> becomes visible in the
  org.bluez.Device1 UUIDs, but sadly, the iPhone still doesn't seem to
  see it, pre-connection).

I don't really know what to expect, what things I may be missing, and if
perhaps things are missing in Bluez implementation, so any help is
welcome!

Thanks in advance,
Dirk.

[ my apologies if you received this message more than once; it seems
the message didn't reach the mailing list, so I tried again... ]


(*) but for some reason, in the aforementioned app, I can see the 'Tx Power
Level' value before connection.

-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

^ permalink raw reply

* pull request: bluetooth 2013-07-30
From: Gustavo Padovan @ 2013-07-30 12:38 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, linux-bluetooth, linux-kernel

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

Hi John,

A few fixes and devices ID additions for 3.11: 

 * There are 4 new ath3k device ids 
 * Fixed stack memory usage in ath3k.
 * Fixed the init process of BlueFRITZ! devices, they were failing to init
   due to an unsupported command we sent. 
 * Fixed wrong use of PTR_ERR in btusb code that was preventing intel devices
   to work properly.
 * Fixed race condition between hci_register_dev() and hci_dev_open() that
   could cause a NULL pointer dereference.
 * Fixed race condition that could call hci_req_cmd_complete() and make some
   devices to fail as showed in the log added to the commit message.

Please pull or let me know of any issues. Thanks.

	Gustavo

--
The following changes since commit 075163bbb0f51174359947e1bce84f5edb23f21e:

  ath9k_htc: Handle IDLE state transition properly (2013-06-24 13:52:52 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth master

for you to fetch changes up to 53e21fbc288218a423959f878c86471a0e323a9a:

  Bluetooth: Fix calling request callback more than once (2013-07-29 12:28:04 +0100)

----------------------------------------------------------------
AceLan Kao (2):
      Bluetooth: Add support for Atheros [0cf3:3121]
      Bluetooth: Add support for Atheros [0cf3:e003]

Adam Lee (1):
      Bluetooth: fix wrong use of PTR_ERR() in btusb

Gustavo Padovan (1):
      Bluetooth: Fix race between hci_register_dev() and hci_dev_open()

Jaganath Kanakkassery (1):
      Bluetooth: Fix invalid length check in l2cap_information_rsp()

Johan Hedberg (2):
      Bluetooth: Fix HCI init for BlueFRITZ! devices
      Bluetooth: Fix calling request callback more than once

Stanislaw Gruszka (1):
      Bluetooth: ath3k: don't use stack memory for DMA

Sujith Manoharan (1):
      Bluetooth: ath3k: Add support for ID 0x13d3/0x3402

Thomas Loo (1):
      Bluetooth: ath3k: Add support for Fujitsu Lifebook UH5x2 [04c5:1330]

 drivers/bluetooth/ath3k.c  | 46 +++++++++++++++++++++++++++++++++++++---------
 drivers/bluetooth/btusb.c  | 18 +++++++++++-------
 net/bluetooth/hci_core.c   | 26 +++++++++++++++++---------
 net/bluetooth/l2cap_core.c |  2 +-
 4 files changed, 66 insertions(+), 26 deletions(-)

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] Update gatt_service_add size assert for dealing with 128-bit uuids
From: Johan Hedberg @ 2013-07-30 11:28 UTC (permalink / raw)
  To: Dirk-Jan C. Binnema; +Cc: linux-bluetooth, Dirk-Jan C. Binnema
In-Reply-To: <1375123370-20055-1-git-send-email-djcb.bulk@gmail.com>

Hi Dirk,

On Mon, Jul 29, 2013, Dirk-Jan C. Binnema wrote:
> To paraphrase Anderson Lizardo: "h" was overflowing for 128-bit services
> because the handle is incremented after each attribute is added so at the
> end we had: 0xffff + 1 == 0 (h is uint16_t).
> 
> As it seems more complex to try to remove the overflow without
> affecting the logic too much, we update the assertion.
> ---
>  attrib/gatt-service.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I made a couple of more cosmetic cleanups and after that pushed these
patches upstream. Thanks.

Some things to keep in mind for future patches:

- Keep your commit message line lengths max 72-74 (both git log and git
  shortlog indent them and we don't want to run past a 80 char terminal)
- Prefix the summary lines appropriately (in this case "attrib: ...")
- Split lines are indented as much as possible while keeping them under
  80 characters (i.e. not just past the opening parenthesis)

Johan

^ permalink raw reply

* Re: [PATCH BlueZ 1/7] audio/AVRCP: Fix memory leaks
From: Johan Hedberg @ 2013-07-30  9:54 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1375106679-14645-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Mon, Jul 29, 2013, Luiz Augusto von Dentz wrote:
> When the remote stack support both AVAILABLE_PLAYER_CHANGED and
> ADDRESSED_PLAYER_CHANGED the player twice are inited twice leaking
> the memory allocated in the first time:
> 
> 16 bytes in 1 blocks are definitely lost in loss record 45 of 236
>    at 0x4A06409: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>    by 0x32F544D89E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5464BF7: g_memdup (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x4247CC: avrcp_get_media_player_list_rsp (avrcp.c:2432)
>    by 0x41ECAB: session_browsing_cb (avctp.c:826)
>    by 0x32F5447E05: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5448157: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5448559: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x40A28F: main (main.c:583)
> 
> 16 bytes in 1 blocks are definitely lost in loss record 46 of 236
>    at 0x4A082F7: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>    by 0x32F544D946: g_realloc (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5466D43: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5467032: g_string_insert_len (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5435287: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x421E62: avrcp_set_browsed_player_rsp (avrcp.c:2349)
>    by 0x41ECAB: session_browsing_cb (avctp.c:826)
>    by 0x32F5447E05: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5448157: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x32F5448559: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3600.3)
>    by 0x40A28F: main (main.c:583)
> ---
>  profiles/audio/avrcp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

All patches in this set have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH BlueZ 1/5] scanparam: Fix handle range check on descriptor discovery
From: Johan Hedberg @ 2013-07-30  9:52 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1375106372-2255-1-git-send-email-anderson.lizardo@openbossa.org>

Hi Lizardo,

On Mon, Jul 29, 2013, Anderson Lizardo wrote:
> When discovering descriptors, if there is only a single descriptor, the
> start handle will be equal to the end handle. It is valid to issue the
> Find Information Request on this case.
> 
> Before this fix, notification for Scan Refresh characteristic was never
> enabled.
> ---
>  profiles/scanparam/scan.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

All patches in this set have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] bluetooth: remove wrong dependency for BT_ATH3K
From: Hector Palacios @ 2013-07-30  7:58 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth@vger.kernel.org, gustavo@padovan.org,
	johan.hedberg@gmail.com, linux-kernel@vger.kernel.org, lrodriguez,
	suraj@atheros.com
In-Reply-To: <5438CD3C-E28C-485C-94B8-F743E8B13D4A@holtmann.org>

Hi Marcel,

On 07/29/2013 06:43 PM, Marcel Holtmann wrote:
> Hi Hector,
>
>> The Atheros firmware download driver is not dependent on HCI
>> USB driver. This driver is needed to complement any Atheros
>> driver like BT_HCIUART_ATH3K.
>>
>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>> CC: Suraj Sumangala <suraj@atheros.com>
>> CC: Vikram Kandukuri <vikram.kandukuri@atheros.com>
>> ---
>> drivers/bluetooth/Kconfig | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>> index 11a6104..a1d482b 100644
>> --- a/drivers/bluetooth/Kconfig
>> +++ b/drivers/bluetooth/Kconfig
>> @@ -222,7 +222,6 @@ config BT_MRVL_SDIO
>>
>> config BT_ATH3K
>> 	tristate "Atheros firmware download driver"
>> -	depends on BT_HCIBTUSB
>> 	select FW_LOADER
>> 	help
>> 	  Bluetooth firmware download driver.
>>
>
> I am confused now. How can an USB driver work with an UART driver? If you do not want to depend on BT_HCIBTUSB then that is fine, but at least you need to depend on USB.

The thing is I have an Atheros AR6233 chipset which has wireless and Bluetooth. 
Bluetooth part in this chip operates as a UART. The driver for this is hci_ath.c but I 
also need to download the firmware to the chip, and for this I need to compile 
ath3k.c. However I don't need BT_HCIBTUSB.
But you are right, looking at the source code of ath3k.c it is calling usb commands, 
so it needs to depend on USB at least.

This brings an interesting question: shouldn't the firmware download part be isolated 
from the USB driver? After all, I want to communicate with a UART bluetooth chip.

Best regards,
--
Hector Palacios

^ permalink raw reply

* RE: [PATCH] Bluetooth: btmrvl: add calibration data download support
From: Bing Zhao @ 2013-07-29 23:20 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
	linux-wireless@vger.kernel.org, Amitkumar Karwar
In-Reply-To: <791D5341-CA33-425C-BFB8-CB65F5451E39@holtmann.org>

Hi Marcel,

> it is also way harder to use and things need to be manually configured. D=
on't know know when the
> device needs calibration information or not. And you can just request_fir=
mware() that file. If it is
> there, then load it, if not then skip the step.

Thanks for your suggestion. This approach should work for me.

>=20
> > The firmware_request() requires a relative file path under /lib/firmwar=
e/.
> > What is the standard location you are referring to?
>=20
> You have to create proper filenames for your hardware. Like mrvl/cal_data=
_v1.conf or similar.

Sure.

> As I said above, the device should know if it needs calibration or not. A=
nd just installing a file on
> disk is simpler than having to modify the module parameters somewhere.

This makes sense. I'll take your approach and re-spin the patch.

Thanks,
Bing

^ permalink raw reply

* Re: [PATCH] Bluetooth: btmrvl: add calibration data download support
From: Marcel Holtmann @ 2013-07-29 22:58 UTC (permalink / raw)
  To: Bing Zhao
  Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
	linux-wireless@vger.kernel.org, Amitkumar Karwar
In-Reply-To: <477F20668A386D41ADCC57781B1F70430EA3215F2E@SC-VEXCH1.marvell.com>

Hi Bing,

>>> A text file containing calibration data in hex format can
>>> be provided while loading btmrvl module. The data will be
>>> downloaded to firmware.
>>> eg. insmod btmrvl.ko cal_data_cfg=mrvl/bt_cal_data.conf
>> 
>> why is this a module parameter and you do not just request a standard location via request_firmware().
> 
> Not all devices need downloading the calibration data from host to firmware.
> A module parameter seems more flexible than others.

it is also way harder to use and things need to be manually configured. Don't know know when the device needs calibration information or not. And you can just request_firmware() that file. If it is there, then load it, if not then skip the step.

> The firmware_request() requires a relative file path under /lib/firmware/.
> What is the standard location you are referring to?

You have to create proper filenames for your hardware. Like mrvl/cal_data_v1.conf or similar.

> 
>> I do not like the module parameter approach.
> 
> I'm open to any other approaches. Do you have any suggestions?

As I said above, the device should know if it needs calibration or not. And just installing a file on disk is simpler than having to modify the module parameters somewhere.

Regards

Marcel


^ permalink raw reply

* RE: [PATCH] Bluetooth: btmrvl: add calibration data download support
From: Bing Zhao @ 2013-07-29 21:28 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
	linux-wireless@vger.kernel.org, Amitkumar Karwar
In-Reply-To: <39BF90E7-8341-412E-A372-6039B703980D@holtmann.org>

Hi Marcel,

Thanks for your comments.

> Hi Bing,
>=20
> > A text file containing calibration data in hex format can
> > be provided while loading btmrvl module. The data will be
> > downloaded to firmware.
> > eg. insmod btmrvl.ko cal_data_cfg=3Dmrvl/bt_cal_data.conf
>=20
> why is this a module parameter and you do not just request a standard loc=
ation via request_firmware().

Not all devices need downloading the calibration data from host to firmware=
.
A module parameter seems more flexible than others.

The firmware_request() requires a relative file path under /lib/firmware/.
What is the standard location you are referring to?

> I do not like the module parameter approach.

I'm open to any other approaches. Do you have any suggestions?

Thanks,
Bing

>=20
> Regards
>=20
> Marcel

^ permalink raw reply

* Re: [PATCH] Bluetooth: btmrvl: add calibration data download support
From: Marcel Holtmann @ 2013-07-29 20:35 UTC (permalink / raw)
  To: Bing Zhao
  Cc: linux-bluetooth, Gustavo Padovan, Johan Hedberg, linux-wireless,
	Amitkumar Karwar
In-Reply-To: <1375126147-28642-1-git-send-email-bzhao@marvell.com>

Hi Bing,

> A text file containing calibration data in hex format can
> be provided while loading btmrvl module. The data will be
> downloaded to firmware.
> eg. insmod btmrvl.ko cal_data_cfg=mrvl/bt_cal_data.conf

why is this a module parameter and you do not just request a standard location via request_firmware(). I do not like the module parameter approach.

Regards

Marcel


^ permalink raw reply

* [PATCH] Bluetooth: btmrvl: add calibration data download support
From: Bing Zhao @ 2013-07-29 19:29 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, linux-wireless,
	Bing Zhao, Amitkumar Karwar

From: Amitkumar Karwar <akarwar@marvell.com>

A text file containing calibration data in hex format can
be provided while loading btmrvl module. The data will be
downloaded to firmware.
eg. insmod btmrvl.ko cal_data_cfg=mrvl/bt_cal_data.conf

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
 drivers/bluetooth/btmrvl_drv.h  |   9 ++-
 drivers/bluetooth/btmrvl_main.c | 140 +++++++++++++++++++++++++++++++++++++++-
 drivers/bluetooth/btmrvl_sdio.c |   2 +-
 3 files changed, 147 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_drv.h b/drivers/bluetooth/btmrvl_drv.h
index 27068d1..f9a6e1b 100644
--- a/drivers/bluetooth/btmrvl_drv.h
+++ b/drivers/bluetooth/btmrvl_drv.h
@@ -23,6 +23,8 @@
 #include <linux/bitops.h>
 #include <linux/slab.h>
 #include <net/bluetooth/bluetooth.h>
+#include <linux/ctype.h>
+#include <linux/firmware.h>
 
 #define BTM_HEADER_LEN			4
 #define BTM_UPLD_SIZE			2312
@@ -41,6 +43,7 @@ struct btmrvl_thread {
 struct btmrvl_device {
 	void *card;
 	struct hci_dev *hcidev;
+	struct device *dev;
 
 	u8 dev_type;
 
@@ -91,6 +94,7 @@ struct btmrvl_private {
 #define BT_CMD_HOST_SLEEP_CONFIG	0x59
 #define BT_CMD_HOST_SLEEP_ENABLE	0x5A
 #define BT_CMD_MODULE_CFG_REQ		0x5B
+#define BT_CMD_LOAD_CONFIG_DATA		0x61
 
 /* Sub-commands: Module Bringup/Shutdown Request/Response */
 #define MODULE_BRINGUP_REQ		0xF1
@@ -116,10 +120,13 @@ struct btmrvl_private {
 #define PS_SLEEP			0x01
 #define PS_AWAKE			0x00
 
+#define BT_CMD_DATA_SIZE		32
+#define BT_CAL_DATA_SIZE		28
+
 struct btmrvl_cmd {
 	__le16 ocf_ogf;
 	u8 length;
-	u8 data[4];
+	u8 data[BT_CMD_DATA_SIZE];
 } __packed;
 
 struct btmrvl_event {
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index 9a9f518..e16c65c 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -27,6 +27,9 @@
 
 #define VERSION "1.0"
 
+static char *cal_data_cfg;
+module_param(cal_data_cfg, charp, 0);
+
 /*
  * This function is called by interface specific interrupt handler.
  * It updates Power Save & Host Sleep states, and wakes up the main
@@ -57,8 +60,9 @@ bool btmrvl_check_evtpkt(struct btmrvl_private *priv, struct sk_buff *skb)
 		ocf = hci_opcode_ocf(opcode);
 		ogf = hci_opcode_ogf(opcode);
 
-		if (ocf == BT_CMD_MODULE_CFG_REQ &&
-					priv->btmrvl_dev.sendcmdflag) {
+		if ((ocf == BT_CMD_MODULE_CFG_REQ ||
+		     ocf == BT_CMD_LOAD_CONFIG_DATA) &&
+		    priv->btmrvl_dev.sendcmdflag) {
 			priv->btmrvl_dev.sendcmdflag = false;
 			priv->adapter->cmd_complete = true;
 			wake_up_interruptible(&priv->adapter->cmd_wait_q);
@@ -552,6 +556,129 @@ static int btmrvl_service_main_thread(void *data)
 	return 0;
 }
 
+static int btmrvl_parse_cal_cfg(const u8 *src, u32 len, u8 *dst, u32 dst_size)
+{
+	const u8 *s = src;
+	u8 *d = dst;
+	int ret;
+	u8 tmp[3];
+
+	while ((s - src) < len) {
+		if (*s && (isspace(*s) || *s == '\t')) {
+			s++;
+			continue;
+		}
+
+		if (isxdigit(*s)) {
+			if ((d - dst) >= dst_size) {
+				BT_ERR("cal_file size too big!!!");
+				return -EINVAL;
+			}
+
+			memcpy(tmp, s, 2);
+			tmp[2] = '\0';
+
+			ret = kstrtol(tmp, 16, (long *)d++);
+			if (ret < 0)
+				return ret;
+
+			s += 2;
+		} else {
+			s++;
+		}
+	}
+	if (d == dst)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int btmrvl_load_cal_data(struct btmrvl_private *priv,
+				u8 *config_data)
+{
+	struct sk_buff *skb;
+	struct btmrvl_cmd *cmd;
+	int i;
+
+	skb = bt_skb_alloc(sizeof(*cmd), GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	cmd = (struct btmrvl_cmd *)skb->data;
+	cmd->ocf_ogf =
+		cpu_to_le16(hci_opcode_pack(OGF, BT_CMD_LOAD_CONFIG_DATA));
+	cmd->length = BT_CMD_DATA_SIZE;
+	cmd->data[0] = 0x00;
+	cmd->data[1] = 0x00;
+	cmd->data[2] = 0x00;
+	cmd->data[3] = BT_CMD_DATA_SIZE - 4;
+
+	/* swap cal-data bytes */
+	for (i = 4; i < BT_CMD_DATA_SIZE; i++)
+		cmd->data[i] = config_data[(i/4)*8 - 1 - i];
+
+	bt_cb(skb)->pkt_type = MRVL_VENDOR_PKT;
+	skb_put(skb, sizeof(*cmd));
+	skb->dev = (void *)priv->btmrvl_dev.hcidev;
+	skb_queue_head(&priv->adapter->tx_queue, skb);
+	priv->btmrvl_dev.sendcmdflag = true;
+	priv->adapter->cmd_complete = false;
+
+	print_hex_dump_bytes("Calibration data: ",
+			     DUMP_PREFIX_OFFSET, cmd->data, BT_CMD_DATA_SIZE);
+
+	wake_up_interruptible(&priv->main_thread.wait_q);
+	if (!wait_event_interruptible_timeout(priv->adapter->cmd_wait_q,
+					      priv->adapter->cmd_complete,
+				       msecs_to_jiffies(WAIT_UNTIL_CMD_RESP))) {
+		BT_ERR("Timeout while loading calibration data");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int
+btmrvl_process_cal_cfg(struct btmrvl_private *priv, u8 *data, u32 size)
+{
+	u8 cal_data[BT_CAL_DATA_SIZE];
+	int ret;
+
+	ret = btmrvl_parse_cal_cfg(data, size, cal_data, sizeof(cal_data));
+	if (ret)
+		return ret;
+
+	ret = btmrvl_load_cal_data(priv, cal_data);
+	if (ret) {
+		BT_ERR("Fail to load calibrate data");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int btmrvl_cal_data_config(struct btmrvl_private *priv, char *cal_file)
+{
+	const struct firmware *cfg;
+	int ret;
+
+	if (!cal_file)
+		return 0;
+
+	ret = request_firmware(&cfg, cal_file, priv->btmrvl_dev.dev);
+	if (ret < 0) {
+		BT_ERR("request_firmware() %s failed", cal_file);
+		goto done;
+	}
+
+	ret = btmrvl_process_cal_cfg(priv, (u8 *)cfg->data, cfg->size);
+done:
+	if (cfg)
+		release_firmware(cfg);
+
+	return ret;
+}
+
 int btmrvl_register_hdev(struct btmrvl_private *priv)
 {
 	struct hci_dev *hdev = NULL;
@@ -583,12 +710,21 @@ int btmrvl_register_hdev(struct btmrvl_private *priv)
 		goto err_hci_register_dev;
 	}
 
+	ret = btmrvl_cal_data_config(priv, cal_data_cfg);
+	if (ret) {
+		BT_ERR("Set cal data failed");
+		goto err_cal_data_config;
+	}
+
 #ifdef CONFIG_DEBUG_FS
 	btmrvl_debugfs_init(hdev);
 #endif
 
 	return 0;
 
+err_cal_data_config:
+	hci_unregister_dev(hdev);
+
 err_hci_register_dev:
 	hci_free_dev(hdev);
 
diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 75c2626..bb9912c 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -18,7 +18,6 @@
  * this warranty disclaimer.
  **/
 
-#include <linux/firmware.h>
 #include <linux/slab.h>
 
 #include <linux/mmc/sdio_ids.h>
@@ -1034,6 +1033,7 @@ static int btmrvl_sdio_probe(struct sdio_func *func,
 	}
 
 	card->priv = priv;
+	priv->btmrvl_dev.dev = &card->func->dev;
 
 	/* Initialize the interface specific function pointers */
 	priv->hw_host_to_card = btmrvl_sdio_host_to_card;
-- 
1.8.0

^ permalink raw reply related

* Re: pull request: bluetooth 2013-07-25
From: John W. Linville @ 2013-07-29 18:43 UTC (permalink / raw)
  To: Gustavo Padovan, linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <20130725150156.GA21959@joana>

On Thu, Jul 25, 2013 at 04:01:58PM +0100, Gustavo Padovan wrote:
> Hi John,
> 
> * Gustavo Padovan <gustavo@padovan.org> [2013-07-25 14:05:54 +0100]:
> 
> > Hi John,
> > 
> > A few fixes and devices ID additions for 3.11: 
> > 
> >  * There are 3 new ath3k device ids 
> >  * Fixed stack memory usage in ath3k.
> >  * Fixed the init process of BlueFRITZ! devices, they were failing to init
> >    due to an unsupported command we sent. 
> >  * Fixed wrong use of PTR_ERR in btusb code that was preventing intel devices
> >    to work properly 
> >  * Fixed race codition between hci_register_dev() and hci_dev_open() that
> >    could cause a NULL pointer dereference.
> 
> Marcel just raised the comment that we need to add more information to
> the BlueFRITZ! fixes so I'm steping back with it. So just ignore it for
> now.
> 
> 	Gustavo

Is a new version of this request still coming?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* [PATCH 3/3] Add support for 128-bit characteristic uuids in gatt_service_add
From: Dirk-Jan C. Binnema @ 2013-07-29 18:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dirk-Jan C. Binnema
In-Reply-To: <1375123370-20055-1-git-send-email-djcb.bulk@gmail.com>

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

Custom GATT services typically use 128-bits UUIDs; this patch allows for using
gatt_service_add to define such services by adding a new option GATT_OPT_CHR_UUID,
which allows for passing bt_uuid_t* values. Note, a previous patch renamed the
old GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16.
---
 attrib/gatt-service.c | 17 ++++++++++++-----
 attrib/gatt-service.h |  6 ++++++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 6ae3902..4cd300f 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -73,6 +73,12 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
+		case GATT_OPT_CHR_UUID:
+			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
+				sizeof(bt_uuid_t));
+			/* characteristic declaration and value */
+			info->num_attrs += 2;
+			break;
 		case GATT_OPT_CHR_PROPS:
 			info->props = va_arg(args, int);
 
@@ -108,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID16) {
+		if (opt == GATT_OPT_CHR_UUID16 || opt == GATT_OPT_CHR_UUID) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	uint16_t h = *handle;
 	struct attribute *a;
 	bt_uuid_t bt_uuid;
-	uint8_t atval[5];
+	uint8_t atval[ATT_MAX_VALUE_LEN];
 	GSList *l;
 
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((info->uuid.type != BT_UUID16 && info->uuid.type != BT_UUID128) ||
+		!info->props) {
 		error("Characteristic UUID or properties are missing");
 		return FALSE;
 	}
@@ -222,9 +229,9 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	bt_uuid16_create(&bt_uuid, GATT_CHARAC_UUID);
 	atval[0] = info->props;
 	att_put_u16(h + 1, &atval[1]);
-	att_put_u16(info->uuid.value.u16, &atval[3]);
+	att_put_uuid(info->uuid, &atval[3]);
 	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-						atval, sizeof(atval)) == NULL)
+				atval, 3 + info->uuid.type / 8) == NULL)
 		return FALSE;
 
 	/* characteristic value */
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index 3964c2b..728d3a8 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -24,7 +24,13 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
+
+	/* bt_uuid_t* value */
+	GATT_OPT_CHR_UUID,
+
+	/* a uint16 value */
 	GATT_OPT_CHR_UUID16,
+
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 2/3] Rename GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16
From: Dirk-Jan C. Binnema @ 2013-07-29 18:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dirk-Jan C. Binnema
In-Reply-To: <1375123370-20055-1-git-send-email-djcb.bulk@gmail.com>

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

Rename the option for all in-tree users. This frees up GATT_OPT_CHR_UUID to be used
for UUIDs of type bt_uuid_t* (in a next patch).
---
 attrib/gatt-service.c         |  4 ++--
 attrib/gatt-service.h         |  2 +-
 plugins/gatt-example.c        |  2 +-
 profiles/alert/server.c       | 16 ++++++++--------
 profiles/proximity/immalert.c |  2 +-
 profiles/proximity/linkloss.c |  2 +-
 profiles/time/server.c        |  8 ++++----
 7 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 1b84c89..6ae3902 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -68,7 +68,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 
 	while (opt != GATT_OPT_INVALID) {
 		switch (opt) {
-		case GATT_OPT_CHR_UUID:
+		case GATT_OPT_CHR_UUID16:
 			bt_uuid16_create(&info->uuid, va_arg(args, int));
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
@@ -108,7 +108,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID) {
+		if (opt == GATT_OPT_CHR_UUID16) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index b810e2e..3964c2b 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -24,7 +24,7 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
-	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_UUID16,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index 37972e8..b926947 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -114,7 +114,7 @@ static gboolean register_battery_service(struct btd_adapter *adapter)
 
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* battery state characteristic */
-			GATT_OPT_CHR_UUID, BATTERY_STATE_UUID,
+			GATT_OPT_CHR_UUID16, BATTERY_STATE_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
diff --git a/profiles/alert/server.c b/profiles/alert/server.c
index 8da1928..4565470 100644
--- a/profiles/alert/server.c
+++ b/profiles/alert/server.c
@@ -789,7 +789,7 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 	/* Phone Alert Status Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert Status characteristic */
-			GATT_OPT_CHR_UUID, ALERT_STATUS_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_STATUS_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
@@ -799,12 +799,12 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_ALERT_STATUS],
 			/* Ringer Control Point characteristic */
-			GATT_OPT_CHR_UUID, RINGER_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			ringer_cp_write, NULL,
 			/* Ringer Setting characteristic */
-			GATT_OPT_CHR_UUID, RINGER_SETTING_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_SETTING_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
@@ -892,35 +892,35 @@ static void register_alert_notif_service(struct alert_adapter *al_adapter)
 	/* Alert Notification Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Supported New Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_NEW_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_NEW_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_new_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_new_alert_cat_handle,
 			/* New Alert */
-			GATT_OPT_CHR_UUID, NEW_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, NEW_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_NEW_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_NEW_ALERT],
 			/* Supported Unread Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_unread_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_unread_alert_cat_handle,
 			/* Unread Alert Status */
-			GATT_OPT_CHR_UUID, UNREAD_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, UNREAD_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_UNREAD_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_UNREAD_ALERT],
 			/* Alert Notification Control Point */
-			GATT_OPT_CHR_UUID, ALERT_NOTIF_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_NOTIF_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			alert_notif_cp_write, NULL,
diff --git a/profiles/proximity/immalert.c b/profiles/proximity/immalert.c
index 06e8eb8..2f8d4e7 100644
--- a/profiles/proximity/immalert.c
+++ b/profiles/proximity/immalert.c
@@ -249,7 +249,7 @@ void imm_alert_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 				GATT_PRIM_SVC_UUID, &uuid,
 				/* Alert level characteristic */
-				GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
diff --git a/profiles/proximity/linkloss.c b/profiles/proximity/linkloss.c
index cb87b12..a7ed96c 100644
--- a/profiles/proximity/linkloss.c
+++ b/profiles/proximity/linkloss.c
@@ -290,7 +290,7 @@ void link_loss_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 			GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert level characteristic */
-			GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 			GATT_OPT_CHR_PROPS,
 				ATT_CHAR_PROPER_READ | ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
diff --git a/profiles/time/server.c b/profiles/time/server.c
index 518a29f..178d4d2 100644
--- a/profiles/time/server.c
+++ b/profiles/time/server.c
@@ -152,14 +152,14 @@ static gboolean register_current_time_service(struct btd_adapter *adapter)
 	/* Current Time service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* CT Time characteristic */
-				GATT_OPT_CHR_UUID, CT_TIME_CHR_UUID,
+				GATT_OPT_CHR_UUID16, CT_TIME_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						current_time_read, adapter,
 
 				/* Local Time Information characteristic */
-				GATT_OPT_CHR_UUID, LOCAL_TIME_INFO_CHR_UUID,
+				GATT_OPT_CHR_UUID16, LOCAL_TIME_INFO_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						local_time_info_read, adapter,
@@ -215,14 +215,14 @@ static gboolean register_ref_time_update_service(struct btd_adapter *adapter)
 	/* Reference Time Update service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* Time Update control point */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_CTRL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_CTRL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 						time_update_control, adapter,
 
 				/* Time Update status */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_STAT_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_STAT_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						time_update_status, adapter,
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 1/3] Update gatt_service_add size assert for dealing with 128-bit uuids
From: Dirk-Jan C. Binnema @ 2013-07-29 18:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dirk-Jan C. Binnema

From: "Dirk-Jan C. Binnema" <djcb@djcbsoftware.nl>

To paraphrase Anderson Lizardo: "h" was overflowing for 128-bit services
because the handle is incremented after each attribute is added so at the
end we had: 0xffff + 1 == 0 (h is uint16_t).

As it seems more complex to try to remove the overflow without
affecting the logic too much, we update the assertion.
---
 attrib/gatt-service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 4b02d39..1b84c89 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -341,7 +341,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
 	}
 
 	g_assert(size < USHRT_MAX);
-	g_assert(h - start_handle == (uint16_t) size);
+	g_assert(h == 0 || (h - start_handle == (uint16_t) size));
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
-- 
1.8.1.4


^ permalink raw reply related

* Re: [PATCH] bluetooth: remove wrong dependency for BT_ATH3K
From: Marcel Holtmann @ 2013-07-29 16:43 UTC (permalink / raw)
  To: Hector Palacios
  Cc: linux-bluetooth, gustavo, johan.hedberg, linux-kernel,
	vikram.kandukuri, suraj
In-Reply-To: <1375114325-25498-1-git-send-email-hector.palacios@digi.com>

Hi Hector,

> The Atheros firmware download driver is not dependent on HCI
> USB driver. This driver is needed to complement any Atheros
> driver like BT_HCIUART_ATH3K.
> 
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> CC: Suraj Sumangala <suraj@atheros.com>
> CC: Vikram Kandukuri <vikram.kandukuri@atheros.com>
> ---
> drivers/bluetooth/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 11a6104..a1d482b 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -222,7 +222,6 @@ config BT_MRVL_SDIO
> 
> config BT_ATH3K
> 	tristate "Atheros firmware download driver"
> -	depends on BT_HCIBTUSB
> 	select FW_LOADER
> 	help
> 	  Bluetooth firmware download driver.
> 

I am confused now. How can an USB driver work with an UART driver? If you do not want to depend on BT_HCIBTUSB then that is fine, but at least you need to depend on USB.

Regards

Marcel


^ permalink raw reply

* [PATCH] bluetooth: remove wrong dependency for BT_ATH3K
From: Hector Palacios @ 2013-07-29 16:12 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: marcel, gustavo, johan.hedberg, linux-kernel, vikram.kandukuri,
	suraj, hector.palacios

The Atheros firmware download driver is not dependent on HCI
USB driver. This driver is needed to complement any Atheros
driver like BT_HCIUART_ATH3K.

Signed-off-by: Hector Palacios <hector.palacios@digi.com>
CC: Suraj Sumangala <suraj@atheros.com>
CC: Vikram Kandukuri <vikram.kandukuri@atheros.com>
---
 drivers/bluetooth/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 11a6104..a1d482b 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -222,7 +222,6 @@ config BT_MRVL_SDIO
 
 config BT_ATH3K
 	tristate "Atheros firmware download driver"
-	depends on BT_HCIBTUSB
 	select FW_LOADER
 	help
 	  Bluetooth firmware download driver.

^ permalink raw reply related

* [PATCH v5 6/6] rfcomm: Purge the dlc->tx_queue to avoid circular dependency
From: Gianluca Anzolin @ 2013-07-29 15:08 UTC (permalink / raw)
  To: gustavo; +Cc: peter, marcel, linux-bluetooth, gregkh, jslaby, Gianluca Anzolin
In-Reply-To: <1375110493-5237-1-git-send-email-gianluca@sottospazio.it>

In rfcomm_tty_cleanup we purge the dlc->tx_queue which may contain
socket buffers referencing the tty_port and thus preventing the tty_port
destruction.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 net/bluetooth/rfcomm/tty.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 3e078b7..6d126fa 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -668,6 +668,12 @@ static void rfcomm_tty_cleanup(struct tty_struct *tty)
 	tty->driver_data = NULL;
 	rfcomm_dlc_unlock(dev->dlc);
 
+	/*
+	 * purge the dlc->tx_queue to avoid circular dependencies
+	 * between dev and dlc
+	 */
+	skb_queue_purge(&dev->dlc->tx_queue);
+
 	tty_port_put(&dev->port);
 }
 
-- 
1.8.3.4

^ permalink raw reply related


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