* Re: [PATCH 12/12] Bluetooth: Remove empty HCI event handlers
From: Johan Hedberg @ 2013-02-18 7:51 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: Bluetooth Linux
In-Reply-To: <CAMXE1-pn2V-6mgvqpwDygaqgsUr90yV7BS_txyzGQytA4gTABw@mail.gmail.com>
Hi Andrei,
On Sun, Feb 17, 2013, Andrei Emeltchenko wrote:
> > With the removal of hci_req_complete() several HCI event handles have
> > essentially become empty and can be removed.
>
> It is still good to have debug traces.
If you need to trace exactly what HCI events are happening btmon and
hcidump are probably much more useful tools for that than kernel debug
logs. And even if you do want debug logs we can just add single BT_DBT()
calls to the hci_cmd_complete() and hci_cmd_status() functions instead
of creating a new function for every single event just for the sake of
having a debug log for them.
Johan
^ permalink raw reply
* Re: [PATCH 05/12 v3] Bluetooth: Switch from hdev->cmd_q to using transactions
From: Johan Hedberg @ 2013-02-18 7:48 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1361053337.1583.7.camel@aeonflux>
Hi Marcel,
On Sat, Feb 16, 2013, Marcel Holtmann wrote:
> > static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
> > {
> > + struct hci_transaction *transaction;
> > struct sk_buff *skb;
> >
> > BT_DBG("%s %ld", hdev->name, opt);
> >
> > /* Driver initialization */
> >
> > + if (hci_start_transaction(hdev) < 0)
> > + return;
> > +
> > + hci_transaction_lock(hdev);
> > +
> > + transaction = hdev->build_transaction;
> > +
> > /* Special commands */
> > while ((skb = skb_dequeue(&hdev->driver_init))) {
> > bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
> > skb->dev = (void *) hdev;
> > -
> > - skb_queue_tail(&hdev->cmd_q, skb);
> > - queue_work(hdev->workqueue, &hdev->cmd_work);
> > + skb_queue_tail(&transaction->cmd_q, skb);
> > }
> > skb_queue_purge(&hdev->driver_init);
>
> if we have to touch this one, then please look at my hdev->setup()
> patches I send a while back. We should get these merged so that drivers
> can use a transaction within the driver and remove the driver_init queue
> actually.
Alright. I'll look into it.
Johan
^ permalink raw reply
* Re: [PATCH 03/12 v3] Bluetooth: Add hci_transaction_cmd_complete function
From: Johan Hedberg @ 2013-02-18 7:46 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1361053148.1583.5.camel@aeonflux>
Hi Marcel,
On Sat, Feb 16, 2013, Marcel Holtmann wrote:
> > This function is used to process the HCI transaction state, including
> > things like picking the next command to send, calling the complete
> > callback for the current transaction and moving the next transaction
> > from the queue (if any) to hdev->current_transaction.
> >
> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > ---
> > include/net/bluetooth/hci_core.h | 1 +
> > net/bluetooth/hci_core.c | 65 ++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 66 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 5cd58f5..54efaa2 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1062,6 +1062,7 @@ int hci_start_transaction(struct hci_dev *hdev);
> > int hci_complete_transaction(struct hci_dev *hdev,
> > void (*complete)(struct hci_dev *hdev,
> > __u16 last_cmd, int status));
> > +bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
> >
> > int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
> > void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 0b289f3..8923a1f 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2976,6 +2976,71 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
> > kfree_skb(skb);
> > }
> >
> > +static void __transaction_next(struct hci_dev *hdev, u16 opcode, int status)
> > +{
> > + struct hci_transaction *transaction;
> > +
> > + transaction = hdev->current_transaction;
> > + if (!transaction)
> > + goto next_in_queue;
> > +
> > + if (status || skb_queue_empty(&transaction->cmd_q)) {
> > + hdev->current_transaction = NULL;
> > +
> > + /* We need to give up the transaction lock temporarily
> > + * since the complete callback might trigger a deadlock
> > + */
> > + hci_transaction_unlock(hdev);
> > + if (transaction->complete)
> > + transaction->complete(hdev, opcode, status);
> > + __transaction_free(transaction);
> > + hci_transaction_lock(hdev);
> > +
> > + transaction = hdev->current_transaction;
> > + }
>
> why do we need current_transaction again. Can we not just always use the
> head of the list?
Since we give up and re-acquire the lock when calling
transaction->complete() it is possible that a new transaction has
already been moved to current_transaction and started running when
complete() returns. If this is the case the __transaction_next() should
just return (like it does).
Johan
^ permalink raw reply
* Re: [PATCH 02/12 v3] Bluetooth: Add basic start/complete HCI transaction functions
From: Johan Hedberg @ 2013-02-18 7:43 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1361053056.1583.4.camel@aeonflux>
Hi Marcel,
On Sat, Feb 16, 2013, Marcel Holtmann wrote:
> > +int hci_start_transaction(struct hci_dev *hdev)
> > +{
> > + struct hci_transaction *transaction;
> > + int err;
> > +
> > + hci_transaction_lock(hdev);
> > +
> > + /* We can't start a new transaction if there's another one in
> > + * progress of being built.
> > + */
> > + if (hdev->build_transaction) {
> > + err = -EBUSY;
> > + goto unlock;
> > + }
>
> I do not get this part. Why not use a common mutex on
> hci_start_transaction and have it close with hci_complete_transaction.
>
> This sounds more like a double protection.
>
> > +
> > + transaction = kzalloc(sizeof(*transaction), GFP_KERNEL);
> > + if (!transaction) {
> > + err = -ENOMEM;
> > + goto unlock;
> > + }
> > +
> > + skb_queue_head_init(&transaction->cmd_q);
> > +
> > + hdev->build_transaction = transaction;
> > +
>
> I am bit confused with this build_transaction concept. So we are
> expecting to build transaction inside the same function. Meaning that
> start and complete functions will be called in the same context.
>
> Maybe some concept of DECLARE_TRANSACTION declaring a local variable and
> then start and complete transaction would be better.
Both of the above two issues are related to the desire to not have to
introduce a new function next to hci_send_cmd() (that would take this
local variable "context") and to be able to keep all current
hci_send_cmd() calls as they are.
In my initial iteration of the patch set I did have the kind of locking
you describe, but keeping the requirement for hci_send_cmd() like I
stated means that you'll then either end up having a potential race
or a deadlock inside the function since sometimes you're entering with
the transaction lock held (when using begin/finish transaction) and
sometimes without the lock held (all existing calls to hci_send_cmd().
So what we're dealing with here is trading less complexity in all our
hci_send_cmd() callers (not having to pass around this extra context)
for slightly more complexity inside the hci_send_cmd() function, which I
maintain is less complexity than removing it would introduce elsewhere,
basically this:
hci_transaction_lock(hdev);
/* If this is part of a multi-command transaction (i.e.
* hci_start_transaction() has been called) just add the skb to
* the end of the transaction being built.
*/
if (hdev->build_transaction) {
skb_queue_tail(&hdev->build_transaction->cmd_q, skb);
goto unlock;
}
/* If we're in the middle of a hci_request the req lock will be
* held and our only choice is to append to the request
* transaction.
*/
if (hdev->req_status && hdev->current_transaction) {
skb_queue_tail(&hdev->current_transaction->cmd_q, skb);
goto unlock;
}
/* This is neither a multi-command transaction nor a hci_request
* situation, but simply hci_send_cmd being called without any
* existing context. Create a simple one-command transaction out
* of the skb
*/
err = __transaction_from_skb(hdev, skb);
if (err < 0)
kfree_skb(skb);
unlock:
hci_transaction_unlock(hdev);
> On that topic using begin_transaction and finish_transaction sounds a
> bit more appealing. Or even run_transaction instead of finish.
begin/run sounds fine to me.
Johan
^ permalink raw reply
* mgmt-api.txt
From: Randy Yates @ 2013-02-18 7:27 UTC (permalink / raw)
To: linux-bluetooth
Almost all text files in doc/ are consistently-documented dbus APIs,
specifying service, interface, and object path.
However, doc/mgmt-api.txt is in a completely different format. It states
something about "Packet Structures" but I have no idea which packets the
document is referring to.
Can someone explain how to use this API? We have a requirement to set
some of these properties, such as discoverability and link level
security.
--
Randy Yates
Digital Signal Labs
http://www.digitalsignallabs.com
^ permalink raw reply
* Re: [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Ben Hutchings @ 2013-02-18 3:18 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Marcel Holtmann, David Woodhouse, Bing Zhao,
libertas-dev@lists.infradead.org, linux-bluetooth@vger.kernel.org,
Gustavo Padovan, Johan Hedberg, linux-kernel@vger.kernel.org
In-Reply-To: <1359285711.2156.4.camel@unicorn>
[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]
On Sun, 2013-01-27 at 12:21 +0100, Lubomir Rintel wrote:
> On Mon, 2013-01-21 at 01:12 +0000, Ben Hutchings wrote:
> > On Fri, 2013-01-18 at 08:33 +0100, Lubomir Rintel wrote:
> > > On Tue, 2013-01-08 at 22:35 -0800, Marcel Holtmann wrote:
> > > > Hi Lubomir,
> > > >
> > > > > > > > linux-firmware ships the sd8688* firmware images that are shared with
> > > > > > > > libertas_sdio WiFi driver under libertas/. libertas_sdio looks in both places
> > > > > > > > and so should we.
> > > > > > > >
> > > > > > > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > > > > > > ---
> > > > > > > > drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++--
> > > > > > > > drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
> > > > > > > > 2 files changed, 26 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > NAK from me on this one. I do not want the driver to check two
> > > > > > > locations. That is what userspace can work around.
> > > > > > >
> > > > > > > If we want to unify the location between the WiFi driver and the
> > > > > > > Bluetooth driver, I am fine with that, but seriously, just pick one over
> > > > > > > the other. I do not care which one.
> > > > > >
> > > > > > The unified location is mrvl/ directory.
> > > > > >
> > > > > > We can probably move SD8688 firmware & helper binaries to mrvl/ and have both drivers grab the images there?
> > > > >
> > > > > That would break existing setups, wouldn't it?
> > > > >
> > > > > I was under impression (commit 3d32a58b) that we care about
> > > > > compatibility here. Do we?
> > > >
> > > > that is what symlinks are for.
> > >
> > > David, Ben: please pull the following branch then:
> > > git pull git://github.com/lkundrak/linux-firmware.git sd8688-move
> > >
> > > Thank you!
> >
> > The symlinks are broken, and you didn't update WHENCE.
>
> Oops, sorry for that. Fixed now, please pull:
> git pull git://github.com/lkundrak/linux-firmware.git sd8688-move
>
> Thank you!
Pulled and will be pushed out shortly. Sorry for the delay.
Ben.
--
Ben Hutchings
Sturgeon's Law: Ninety percent of everything is crap.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [RFC v2 3/8] Bluetooth: LE connection state machine
From: Marcel Holtmann @ 2013-02-16 22:32 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1360970828-24004-4-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch implements a state machine for carrying out the general
> connection establishment procedure described in Core spec.
>
> The state machine should be used as follows: when the kernel
> receives a new LE connection attempt, it should go to HCI_CONN_LE_
> SCAN state, starting the passive LE scanning. Once the target remote
> device is in-range, it should go to HCI_CONN_LE_FOUND, stopping the
> ongoing LE scanning. After the LE scanning is disabled, it should go
> to HCI_CONN_LE_INITIATE state where the LE connection is created.
>
> This state machine will be used by the LE connection routine in
> order to establish LE connections.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 13 +++++++++++++
> net/bluetooth/hci_conn.c | 26 ++++++++++++++++++++++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 48c28ca..c704737 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -300,6 +300,16 @@ struct hci_dev {
>
> #define HCI_PHY_HANDLE(handle) (handle & 0xff)
>
> +/*
> + * States from LE connection establishment state machine.
> + * State 0 is reserved and indicates the state machine is not running.
> + */
> +enum {
> + HCI_CONN_LE_SCAN = 1,
> + HCI_CONN_LE_FOUND,
> + HCI_CONN_LE_INITIATE,
> +};
> +
and why don't we have HCI_CONN_LE_IDLE = 0 then. I do not get this magic
number handling when you introduce an enum anyway. You spent more time
explaining it with a comment instead of just using an extra enum entry.
> struct hci_conn {
> struct list_head list;
>
> @@ -356,6 +366,8 @@ struct hci_conn {
>
> struct hci_conn *link;
>
> + atomic_t le_state;
> +
Why is this atomic_t. I am lost on what you are trying to solve here.
This requires a detailed explanation or you just get rid of it and use
the enum.
> void (*connect_cfm_cb) (struct hci_conn *conn, u8 status);
> void (*security_cfm_cb) (struct hci_conn *conn, u8 status);
> void (*disconn_cfm_cb) (struct hci_conn *conn, u8 reason);
> @@ -599,6 +611,7 @@ int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
> int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
> int hci_conn_change_link_key(struct hci_conn *conn);
> int hci_conn_switch_role(struct hci_conn *conn, __u8 role);
> +void hci_conn_set_le_state(struct hci_conn *conn, int state);
External state setting. That does not look like a good idea in the first
place. Why do you want to do it like this. Shouldn't be this self
contained.
>
> void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 25bfce0..d54c2a0 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -391,6 +391,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
> (unsigned long) conn);
>
> atomic_set(&conn->refcnt, 0);
> + atomic_set(&conn->le_state, 0);
>
> hci_dev_hold(hdev);
>
> @@ -1027,3 +1028,28 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
>
> return hchan;
> }
> +
> +void hci_conn_set_le_state(struct hci_conn *conn, int state)
> +{
> + struct hci_dev *hdev = conn->hdev;
> +
> + BT_DBG("conn %p state %d -> %d", conn, atomic_read(&conn->le_state),
> + state);
> +
> + if (state == atomic_read(&conn->le_state))
> + return;
> +
> + switch (state) {
> + case HCI_CONN_LE_SCAN:
> + hci_le_scan(hdev, LE_SCAN_PASSIVE, 0x60, 0x30, 0);
> + break;
> + case HCI_CONN_LE_FOUND:
> + hci_cancel_le_scan(hdev);
> + break;
> + case HCI_CONN_LE_INITIATE:
> + hci_le_create_connection(conn);
> + break;
> + }
> +
> + atomic_set(&conn->le_state, state);
> +}
The more I read this, the more I think this is the wrong approach to
this. The kernel should have a list of device it wants to connect to. If
that list is non-empty, start scanning, if one device is found, try to
connect it, once done, keep scanning if other devices are not connected.
You need to build a foundation for LE devices that the kernel wants to
connect to. And not hack in some state machinery.
And of course, can we please do proper error handling here. This whole
thing is broken. If any of the HCI commands fail, your state machine is
screwed up.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 1/8] Bluetooth: Setup LE scan with no timeout
From: Marcel Holtmann @ 2013-02-16 22:26 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1360970828-24004-2-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch modifies hci_do_le_scan and hci_cancel_le_scan helpers so
> we are able to start and stop LE scan with no timeout. This feature
> will be used by the LE connection routine.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_core.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 22e77a7..3aa0345 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1618,26 +1618,27 @@ static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
> if (err < 0)
> return err;
>
> - queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
> - msecs_to_jiffies(timeout));
> + if (timeout > 0)
> + queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
> + msecs_to_jiffies(timeout));
>
> return 0;
> }
I really do not like this magic handling of scan disable. Maybe you
better remove the timeout handling completely and put it into the
discovery functionality.
Doing it like this seems pretty much hacked together. It no longer looks
like the right place to do handle it.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 05/12 v3] Bluetooth: Switch from hdev->cmd_q to using transactions
From: Marcel Holtmann @ 2013-02-16 22:22 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1360921920-14080-6-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch converts the code from using a single hdev->cmd_q HCI command
> queue to use the HCI transaction infrastructure.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_core.c | 87 ++++++++++++++++++++++++++++++++++++++-------
> net/bluetooth/hci_event.c | 12 +++++--
> net/bluetooth/hci_sock.c | 5 +--
> 3 files changed, 88 insertions(+), 16 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 6569248..4f55225 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -80,10 +80,18 @@ void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
> return;
>
> skb = skb_clone(hdev->sent_cmd, GFP_ATOMIC);
> + hci_transaction_lock(hdev);
> if (skb) {
> - skb_queue_head(&hdev->cmd_q, skb);
> - queue_work(hdev->workqueue, &hdev->cmd_work);
> + struct hci_transaction *transaction =
> + hdev->current_transaction;
> + if (transaction) {
> + skb_queue_head(&transaction->cmd_q, skb);
> + queue_work(hdev->workqueue, &hdev->cmd_work);
> + } else {
> + kfree_skb(skb);
> + }
> }
> + hci_transaction_unlock(hdev);
>
> return;
> }
> @@ -203,22 +211,30 @@ static void amp_init(struct hci_dev *hdev)
>
> static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
> {
> + struct hci_transaction *transaction;
> struct sk_buff *skb;
>
> BT_DBG("%s %ld", hdev->name, opt);
>
> /* Driver initialization */
>
> + if (hci_start_transaction(hdev) < 0)
> + return;
> +
> + hci_transaction_lock(hdev);
> +
> + transaction = hdev->build_transaction;
> +
> /* Special commands */
> while ((skb = skb_dequeue(&hdev->driver_init))) {
> bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
> skb->dev = (void *) hdev;
> -
> - skb_queue_tail(&hdev->cmd_q, skb);
> - queue_work(hdev->workqueue, &hdev->cmd_work);
> + skb_queue_tail(&transaction->cmd_q, skb);
> }
> skb_queue_purge(&hdev->driver_init);
if we have to touch this one, then please look at my hdev->setup()
patches I send a while back. We should get these merged so that drivers
can use a transaction within the driver and remove the driver_init queue
actually.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 03/12 v3] Bluetooth: Add hci_transaction_cmd_complete function
From: Marcel Holtmann @ 2013-02-16 22:19 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1360921920-14080-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This function is used to process the HCI transaction state, including
> things like picking the next command to send, calling the complete
> callback for the current transaction and moving the next transaction
> from the queue (if any) to hdev->current_transaction.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 65 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 5cd58f5..54efaa2 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1062,6 +1062,7 @@ int hci_start_transaction(struct hci_dev *hdev);
> int hci_complete_transaction(struct hci_dev *hdev,
> void (*complete)(struct hci_dev *hdev,
> __u16 last_cmd, int status));
> +bool hci_transaction_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
>
> int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
> void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 0b289f3..8923a1f 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2976,6 +2976,71 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
> kfree_skb(skb);
> }
>
> +static void __transaction_next(struct hci_dev *hdev, u16 opcode, int status)
> +{
> + struct hci_transaction *transaction;
> +
> + transaction = hdev->current_transaction;
> + if (!transaction)
> + goto next_in_queue;
> +
> + if (status || skb_queue_empty(&transaction->cmd_q)) {
> + hdev->current_transaction = NULL;
> +
> + /* We need to give up the transaction lock temporarily
> + * since the complete callback might trigger a deadlock
> + */
> + hci_transaction_unlock(hdev);
> + if (transaction->complete)
> + transaction->complete(hdev, opcode, status);
> + __transaction_free(transaction);
> + hci_transaction_lock(hdev);
> +
> + transaction = hdev->current_transaction;
> + }
why do we need current_transaction again. Can we not just always use the
head of the list?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 02/12 v3] Bluetooth: Add basic start/complete HCI transaction functions
From: Marcel Holtmann @ 2013-02-16 22:17 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1360921920-14080-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> With these functions it will be possible to declare the start and end of
> a transaction. hci_start_transaction() creates hdev->build_transaction
> which will be used by hci_send_command() to construct a transaction.
> hci_complete_transaction() indicates the end of a transaction with an
> optional complete callback to be called once the transaction is
> complete. The transaction is either moved to hdev->current_transaction
> (if no other transaction is in progress) or appended to
> hdev->transaction_q.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 5 +++
> net/bluetooth/hci_core.c | 80 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 85 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0e53032..5cd58f5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1058,6 +1058,11 @@ int hci_unregister_cb(struct hci_cb *hcb);
> #define hci_transaction_lock(d) mutex_lock(&d->transaction_lock)
> #define hci_transaction_unlock(d) mutex_unlock(&d->transaction_lock)
>
> +int hci_start_transaction(struct hci_dev *hdev);
> +int hci_complete_transaction(struct hci_dev *hdev,
> + void (*complete)(struct hci_dev *hdev,
> + __u16 last_cmd, int status));
> +
> int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
> void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
> void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 05e2e8b..0b289f3 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2196,6 +2196,86 @@ static int hci_send_frame(struct sk_buff *skb)
> return hdev->send(skb);
> }
>
> +int hci_start_transaction(struct hci_dev *hdev)
> +{
> + struct hci_transaction *transaction;
> + int err;
> +
> + hci_transaction_lock(hdev);
> +
> + /* We can't start a new transaction if there's another one in
> + * progress of being built.
> + */
> + if (hdev->build_transaction) {
> + err = -EBUSY;
> + goto unlock;
> + }
I do not get this part. Why not use a common mutex on
hci_start_transaction and have it close with hci_complete_transaction.
This sounds more like a double protection.
> +
> + transaction = kzalloc(sizeof(*transaction), GFP_KERNEL);
> + if (!transaction) {
> + err = -ENOMEM;
> + goto unlock;
> + }
> +
> + skb_queue_head_init(&transaction->cmd_q);
> +
> + hdev->build_transaction = transaction;
> +
I am bit confused with this build_transaction concept. So we are
expecting to build transaction inside the same function. Meaning that
start and complete functions will be called in the same context.
Maybe some concept of DECLARE_TRANSACTION declaring a local variable and
then start and complete transaction would be better.
On that topic using begin_transaction and finish_transaction sounds a
bit more appealing. Or even run_transaction instead of finish.
Regards
Marcel
^ permalink raw reply
* BT_ADDR not updated
From: Randy Yates @ 2013-02-16 14:51 UTC (permalink / raw)
To: linux-bluetooth
This question is regarding bluez 5.2:
i'm changing bt addr "on-the-fly" via a vendor-specific hcitool
command but the bluetoothctl is not seeing the new address, even
after a hciconfig hci0 reset
however i do see the new address via "hcitool dev", so it's
getting changed in the device
is there some bug in bluez updating the device address on a reset?
--
Randy Yates
Digital Signal Labs
http://www.digitalsignallabs.com
^ permalink raw reply
* [RFC v2 8/8] Bluetooth: Initialize some hci_conn fields earlier
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
This patch moves some hci_conn fields initialization to hci_connect_le.
There is no reason we initialize them later at hci_le_create_connection.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_conn.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 29192e0..93b594b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -36,11 +36,6 @@ static void hci_le_create_connection(struct hci_conn *conn)
struct hci_dev *hdev = conn->hdev;
struct hci_cp_le_create_conn cp;
- conn->state = BT_CONNECT;
- conn->out = true;
- conn->link_mode |= HCI_LM_MASTER;
- conn->sec_level = BT_SECURITY_LOW;
-
memset(&cp, 0, sizeof(cp));
cp.scan_interval = __constant_cpu_to_le16(0x0060);
cp.scan_window = __constant_cpu_to_le16(0x0030);
@@ -537,6 +532,9 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
le->dst_type = bdaddr_to_le(dst_type);
le->state = BT_CONNECT;
+ le->out = true;
+ le->link_mode |= HCI_LM_MASTER;
+ le->sec_level = BT_SECURITY_LOW;
hci_conn_set_le_state(le, HCI_CONN_LE_SCAN);
}
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 7/8] Bluetooth: Count pending LE connection attempts
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
To avoid traversing the hci_conn list every time we get an advertising
report, we keep a counter of pending LE connection attempts (connections
in HCI_CONN_LE_SCAN state). This way, we only traverse the list if the
counter is greater from zero.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
include/net/bluetooth/hci_core.h | 6 ++++++
net/bluetooth/hci_conn.c | 6 ++++++
net/bluetooth/hci_core.c | 2 ++
3 files changed, 14 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c797717..5f2a991 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -290,6 +290,12 @@ struct hci_dev {
__u8 adv_data[HCI_MAX_AD_LENGTH];
__u8 adv_data_len;
+ /*
+ * This counter tracks the number of pending LE connections
+ * (connections in HCI_CONN_LE_SCAN state).
+ */
+ atomic_t le_conn_pend;
+
int (*open)(struct hci_dev *hdev);
int (*close)(struct hci_dev *hdev);
int (*flush)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e259745..29192e0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -266,6 +266,7 @@ static void cancel_le_connection(struct hci_conn *conn)
switch (atomic_read(&conn->le_state)) {
case HCI_CONN_LE_SCAN:
+ atomic_dec(&hdev->le_conn_pend);
hci_cancel_le_scan(hdev);
break;
case HCI_CONN_LE_INITIATE:
@@ -1061,9 +1062,11 @@ void hci_conn_set_le_state(struct hci_conn *conn, int state)
switch (state) {
case HCI_CONN_LE_SCAN:
+ atomic_inc(&hdev->le_conn_pend);
hci_le_scan(hdev, LE_SCAN_PASSIVE, 0x60, 0x30, 0);
break;
case HCI_CONN_LE_FOUND:
+ atomic_dec(&hdev->le_conn_pend);
hci_cancel_le_scan(hdev);
break;
case HCI_CONN_LE_INITIATE:
@@ -1079,6 +1082,9 @@ struct hci_conn *hci_conn_has_le_pending(struct hci_dev *hdev, bdaddr_t *addr,
{
struct hci_conn *conn;
+ if (atomic_read(&hdev->le_conn_pend) == 0)
+ return NULL;
+
conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, addr);
if (!conn)
return NULL;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3aa0345..acdfe15 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1742,6 +1742,8 @@ struct hci_dev *hci_alloc_dev(void)
hci_init_sysfs(hdev);
discovery_init(hdev);
+ atomic_set(&hdev->le_conn_pend, 0);
+
return hdev;
}
EXPORT_SYMBOL(hci_alloc_dev);
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 6/8] Bluetooth: Handle hci_conn timeout in BT_CONNECT
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
In order to properly cancel an ongoing LE connection attempt (hci_conn
in BT_CONNECT state), we need to check the current le_state.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_conn.c | 20 +++++++++++++++++++-
net/bluetooth/hci_event.c | 11 +++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 308c87a..e259745 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -258,6 +258,24 @@ static void hci_conn_disconnect(struct hci_conn *conn)
}
}
+static void cancel_le_connection(struct hci_conn *conn)
+{
+ struct hci_dev *hdev = conn->hdev;
+
+ BT_DBG("conn %p le state %d", conn, atomic_read(&conn->le_state));
+
+ switch (atomic_read(&conn->le_state)) {
+ case HCI_CONN_LE_SCAN:
+ hci_cancel_le_scan(hdev);
+ break;
+ case HCI_CONN_LE_INITIATE:
+ hci_le_create_connection_cancel(conn);
+ break;
+ }
+
+ hci_conn_set_le_state(conn, 0);
+}
+
static void hci_conn_timeout(struct work_struct *work)
{
struct hci_conn *conn = container_of(work, struct hci_conn,
@@ -275,7 +293,7 @@ static void hci_conn_timeout(struct work_struct *work)
if (conn->type == ACL_LINK)
hci_acl_create_connection_cancel(conn);
else if (conn->type == LE_LINK)
- hci_le_create_connection_cancel(conn);
+ cancel_le_connection(conn);
}
break;
case BT_CONFIG:
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 84648c8..625eada 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1296,6 +1296,17 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
+ hcon = hci_conn_hash_lookup_le_state(hdev, 0);
+ if (hcon) {
+ mgmt_connect_failed(hdev, &hcon->dst, hcon->type,
+ hcon->dst_type,
+ HCI_ERROR_LOCAL_HOST_TERM);
+
+ hcon->state = BT_CLOSED;
+ hci_proto_connect_cfm(hcon, HCI_ERROR_LOCAL_HOST_TERM);
+ hci_conn_del(hcon);
+ }
+
hcon = hci_conn_hash_lookup_le_state(hdev, HCI_CONN_LE_FOUND);
if (hcon) {
hci_dev_lock(hdev);
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 5/8] Bluetooth: Change LE connection routine
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
In order to better support LE connection requirements, such as multiple
connections and re-connections, we should use the general connection
establishment procedure described in Core spec. Today, we use the direct
connection establishment procedure which has some limitations and,
therefore, requires extra connection management at user-space in order
to support LE connection requirements.
According to the spec, the general procedure is described as follows:
The host starts scanning for LE devices, once the device we want to
connect to is in-range, the host stops scanning and initiates a
connection. The procedure is terminated when the connection is
established or when the host terminates the procedure.
This patch changes the LE connection routine so we perform the general
procedure instead of the direct procedure.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_conn.c | 4 +++-
net/bluetooth/hci_event.c | 19 +++++++++++++++++--
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b1a162f..308c87a 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -517,7 +517,9 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
return ERR_PTR(-ENOMEM);
le->dst_type = bdaddr_to_le(dst_type);
- hci_le_create_connection(le);
+ le->state = BT_CONNECT;
+
+ hci_conn_set_le_state(le, HCI_CONN_LE_SCAN);
}
le->pending_sec_level = sec_level;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 477726a..84648c8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1260,6 +1260,7 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
{
struct hci_cp_le_set_scan_enable *cp;
__u8 status = *((__u8 *) skb->data);
+ struct hci_conn *hcon;
BT_DBG("%s status 0x%2.2x", hdev->name, status);
@@ -1295,8 +1296,15 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
- if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
- hdev->discovery.state == DISCOVERY_FINDING) {
+ hcon = hci_conn_hash_lookup_le_state(hdev, HCI_CONN_LE_FOUND);
+ if (hcon) {
+ hci_dev_lock(hdev);
+ hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+ hci_dev_unlock(hdev);
+
+ hci_conn_set_le_state(hcon, HCI_CONN_LE_INITIATE);
+ } else if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
+ hdev->discovery.state == DISCOVERY_FINDING) {
mgmt_interleaved_discovery(hdev);
} else {
hci_dev_lock(hdev);
@@ -3971,6 +3979,7 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
conn->sec_level = BT_SECURITY_LOW;
conn->handle = __le16_to_cpu(ev->handle);
+ hci_conn_set_le_state(conn, 0);
conn->state = BT_CONNECTED;
hci_conn_hold_device(conn);
@@ -3987,10 +3996,16 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
u8 num_reports = skb->data[0];
void *ptr = &skb->data[1];
s8 rssi;
+ struct hci_conn *hcon;
while (num_reports--) {
struct hci_ev_le_advertising_info *ev = ptr;
+ hcon = hci_conn_has_le_pending(hdev, &ev->bdaddr,
+ ev->bdaddr_type);
+ if (hcon)
+ hci_conn_set_le_state(hcon, HCI_CONN_LE_FOUND);
+
rssi = ev->data[ev->length];
mgmt_device_found(hdev, &ev->bdaddr, LE_LINK, ev->bdaddr_type,
NULL, rssi, 0, 1, ev->data, ev->length);
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 4/8] Bluetooth: Add hci_conn helpers
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
This patch adds two helper functions for looking up hci_conn objects
according to its LE state. hci_conn_has_le_pending helper returns
a pending connection (hci_conn in HCI_CONN_LE_SCAN state) if any.
hci_conn_hash_lookup_le_state helper performs a lookup for hci_conn
objects in a given LE state.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
include/net/bluetooth/hci_core.h | 3 +++
net/bluetooth/hci_conn.c | 47 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c704737..c797717 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -612,6 +612,9 @@ int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
int hci_conn_change_link_key(struct hci_conn *conn);
int hci_conn_switch_role(struct hci_conn *conn, __u8 role);
void hci_conn_set_le_state(struct hci_conn *conn, int state);
+struct hci_conn *hci_conn_has_le_pending(struct hci_dev *hdev, bdaddr_t *addr,
+ __u8 type);
+struct hci_conn *hci_conn_hash_lookup_le_state(struct hci_dev *hdev, int state);
void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index d54c2a0..b1a162f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1053,3 +1053,50 @@ void hci_conn_set_le_state(struct hci_conn *conn, int state)
atomic_set(&conn->le_state, state);
}
+
+struct hci_conn *hci_conn_has_le_pending(struct hci_dev *hdev, bdaddr_t *addr,
+ __u8 type)
+{
+ struct hci_conn *conn;
+
+ conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, addr);
+ if (!conn)
+ return NULL;
+
+ if (conn->state != BT_CONNECT)
+ return NULL;
+
+ if (conn->dst_type != type)
+ return NULL;
+
+ if (atomic_read(&conn->le_state) != HCI_CONN_LE_SCAN)
+ return NULL;
+
+ return conn;
+}
+
+struct hci_conn *hci_conn_hash_lookup_le_state(struct hci_dev *hdev, int state)
+{
+ struct hci_conn_hash *conn_hash = &hdev->conn_hash;
+ struct hci_conn *conn, *ret = NULL;
+
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(conn, &conn_hash->list, list) {
+ if (conn->type != LE_LINK)
+ continue;
+
+ if (conn->state != BT_CONNECT)
+ continue;
+
+ if (atomic_read(&conn->le_state) != state)
+ continue;
+
+ ret = conn;
+ break;
+ }
+
+ rcu_read_unlock();
+
+ return ret;
+}
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 3/8] Bluetooth: LE connection state machine
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
This patch implements a state machine for carrying out the general
connection establishment procedure described in Core spec.
The state machine should be used as follows: when the kernel
receives a new LE connection attempt, it should go to HCI_CONN_LE_
SCAN state, starting the passive LE scanning. Once the target remote
device is in-range, it should go to HCI_CONN_LE_FOUND, stopping the
ongoing LE scanning. After the LE scanning is disabled, it should go
to HCI_CONN_LE_INITIATE state where the LE connection is created.
This state machine will be used by the LE connection routine in
order to establish LE connections.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
include/net/bluetooth/hci_core.h | 13 +++++++++++++
net/bluetooth/hci_conn.c | 26 ++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 48c28ca..c704737 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -300,6 +300,16 @@ struct hci_dev {
#define HCI_PHY_HANDLE(handle) (handle & 0xff)
+/*
+ * States from LE connection establishment state machine.
+ * State 0 is reserved and indicates the state machine is not running.
+ */
+enum {
+ HCI_CONN_LE_SCAN = 1,
+ HCI_CONN_LE_FOUND,
+ HCI_CONN_LE_INITIATE,
+};
+
struct hci_conn {
struct list_head list;
@@ -356,6 +366,8 @@ struct hci_conn {
struct hci_conn *link;
+ atomic_t le_state;
+
void (*connect_cfm_cb) (struct hci_conn *conn, u8 status);
void (*security_cfm_cb) (struct hci_conn *conn, u8 status);
void (*disconn_cfm_cb) (struct hci_conn *conn, u8 reason);
@@ -599,6 +611,7 @@ int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
int hci_conn_change_link_key(struct hci_conn *conn);
int hci_conn_switch_role(struct hci_conn *conn, __u8 role);
+void hci_conn_set_le_state(struct hci_conn *conn, int state);
void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..d54c2a0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -391,6 +391,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
(unsigned long) conn);
atomic_set(&conn->refcnt, 0);
+ atomic_set(&conn->le_state, 0);
hci_dev_hold(hdev);
@@ -1027,3 +1028,28 @@ struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle)
return hchan;
}
+
+void hci_conn_set_le_state(struct hci_conn *conn, int state)
+{
+ struct hci_dev *hdev = conn->hdev;
+
+ BT_DBG("conn %p state %d -> %d", conn, atomic_read(&conn->le_state),
+ state);
+
+ if (state == atomic_read(&conn->le_state))
+ return;
+
+ switch (state) {
+ case HCI_CONN_LE_SCAN:
+ hci_le_scan(hdev, LE_SCAN_PASSIVE, 0x60, 0x30, 0);
+ break;
+ case HCI_CONN_LE_FOUND:
+ hci_cancel_le_scan(hdev);
+ break;
+ case HCI_CONN_LE_INITIATE:
+ hci_le_create_connection(conn);
+ break;
+ }
+
+ atomic_set(&conn->le_state, state);
+}
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 2/8] Bluetooth: Add LE scan type macros
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
This patch adds macros for active and passive LE scan type values. It
also removes the LE_SCAN_TYPE macro since it is not used anymore.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
include/net/bluetooth/hci_core.h | 3 +++
net/bluetooth/mgmt.c | 7 +++----
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 90cf75a..48c28ca 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -117,6 +117,9 @@ struct oob_data {
u8 randomizer[16];
};
+#define LE_SCAN_PASSIVE 0x00
+#define LE_SCAN_ACTIVE 0x01
+
struct le_scan_params {
u8 type;
u16 interval;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 39395c7..1e5bd74 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -106,7 +106,6 @@ static const u16 mgmt_events[] = {
* These LE scan and inquiry parameters were chosen according to LE General
* Discovery Procedure specification.
*/
-#define LE_SCAN_TYPE 0x01
#define LE_SCAN_WIN 0x12
#define LE_SCAN_INT 0x12
#define LE_SCAN_TIMEOUT_LE_ONLY 10240 /* TGAP(gen_disc_scan_min) */
@@ -2485,7 +2484,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
goto failed;
}
- err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
+ err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT,
LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
break;
@@ -2497,8 +2496,8 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
goto failed;
}
- err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT, LE_SCAN_WIN,
- LE_SCAN_TIMEOUT_BREDR_LE);
+ err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT,
+ LE_SCAN_WIN, LE_SCAN_TIMEOUT_BREDR_LE);
break;
default:
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 1/8] Bluetooth: Setup LE scan with no timeout
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1360970828-24004-1-git-send-email-andre.guedes@openbossa.org>
This patch modifies hci_do_le_scan and hci_cancel_le_scan helpers so
we are able to start and stop LE scan with no timeout. This feature
will be used by the LE connection routine.
Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
net/bluetooth/hci_core.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 22e77a7..3aa0345 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1618,26 +1618,27 @@ static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
if (err < 0)
return err;
- queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
- msecs_to_jiffies(timeout));
+ if (timeout > 0)
+ queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
+ msecs_to_jiffies(timeout));
return 0;
}
int hci_cancel_le_scan(struct hci_dev *hdev)
{
+ struct hci_cp_le_set_scan_enable cp;
+
BT_DBG("%s", hdev->name);
if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
return -EALREADY;
- if (cancel_delayed_work(&hdev->le_scan_disable)) {
- struct hci_cp_le_set_scan_enable cp;
+ cancel_delayed_work(&hdev->le_scan_disable);
- /* Send HCI command to disable LE Scan */
- memset(&cp, 0, sizeof(cp));
- hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
- }
+ /* Send HCI command to disable LE Scan */
+ memset(&cp, 0, sizeof(cp));
+ hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
return 0;
}
--
1.8.1.2
^ permalink raw reply related
* [RFC v2 0/8] LE Connection Routine
From: Andre Guedes @ 2013-02-15 23:27 UTC (permalink / raw)
To: linux-bluetooth
Hi all,
This v2 RFC series address Marcel's comment regarding a separated state
machine for handling LE connection attempts. So in this series it was
introduced a new state machine for carrying out the general connection
establishment procedure.
>From the previous cover letter:
In order to better support LE connection requirements, such as multiple
connections and re-connections, we should support the general connection
establishment procedure in kernel side. Today, we support only the direct
connection establishment procedure which has some limitations and, therefore,
requires extra connection management at user-space.
According to the spec (Vol 3, Part C, section 9.3.6), the general procedure is
described as follows: The host starts scanning for LE devices, once the device
we want to connect to is in-range, the host stops scanning and initiates a
connection. The procedure is terminated when the connection is established or
when the host terminates the procedure.
This RFC series implements the basic support for general connection procedure.
The first patches do simple changes required to implement this new LE
connection routine. It has not been well tested, but the basic LE connection
and disconnection cases have been covered.
This is an initial work, so it doesn't support multiple LE connection attempts
at the same time (current kernel doesn't support too) and doesn't handle
concurrent device discovery properly.
The next steps are the following:
1. Cover some corner cases in this series.
2. Add support for multiple LE connection attempts.
3. Handle concurrent LE connections and device discovery
4. Add better support for controller which a capable of scanning and initiating
LE connection at the same time.
5. Add API so userspace is able to configure connection parameters.
6. Remove LE connection management code from bluetoothd.
Feedback is welcome.
Regards,
Andre
Andre Guedes (8):
Bluetooth: Setup LE scan with no timeout
Bluetooth: Add LE scan type macros
Bluetooth: LE connection state machine
Bluetooth: Add hci_conn helpers
Bluetooth: Change LE connection routine
Bluetooth: Handle hci_conn timeout in BT_CONNECT
Bluetooth: Count pending LE connection attempts
Bluetooth: Initialize some hci_conn fields earlier
include/net/bluetooth/hci_core.h | 25 +++++++++
net/bluetooth/hci_conn.c | 111 ++++++++++++++++++++++++++++++++++++---
net/bluetooth/hci_core.c | 19 ++++---
net/bluetooth/hci_event.c | 30 ++++++++++-
net/bluetooth/mgmt.c | 7 ++-
5 files changed, 171 insertions(+), 21 deletions(-)
--
1.8.1.2
^ permalink raw reply
* [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver()
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1360940876-6314-1-git-send-email-anderson.lizardo@openbossa.org>
---
unit/test-lib.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index a994dc2..def133a 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -388,6 +388,53 @@ static void test_sdp_get_profile_descs_workaround(void)
sdp_record_free(rec);
}
+static void test_sdp_get_server_ver(void)
+{
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ uint8_t dtd1 = SDP_UINT16, dtd2 = SDP_UINT32;
+ void *dtds1[] = { &dtd1 };
+ void *values1[] = { &u16 };
+ void *dtds2[] = { &dtd2 };
+ void *values2[] = { &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ /* Valid DTD */
+ data = sdp_seq_alloc(dtds1, values1, 1);
+ sdp_attr_add(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == 0 && list != NULL);
+ sdp_list_free(list, NULL);
+
+ /* Invalid: UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8(UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -418,5 +465,7 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_profile_descs/workaround",
test_sdp_get_profile_descs_workaround);
+ g_test_add_func("/lib/sdp_get_server_ver", test_sdp_get_server_ver);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs()
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1360940876-6314-1-git-send-email-anderson.lizardo@openbossa.org>
---
unit/test-lib.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 171 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index 76e4363..a994dc2 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -227,6 +227,167 @@ static void test_sdp_get_lang_attr_invalid_dtd(void)
sdp_record_free(rec);
}
+static void test_sdp_get_profile_descs_valid(void)
+{
+ sdp_profile_desc_t profile;
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ sdp_uuid16_create(&profile.uuid, NAP_PROFILE_ID);
+ profile.version = 0x0100;
+ list = sdp_list_append(NULL, &profile);
+ err = sdp_set_profile_descs(rec, list);
+ sdp_list_free(list, NULL);
+ g_assert(err == 0);
+
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_invalid_dtd(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT32;
+ uint32_t u32 = 0xdeadbeeb;
+ uint16_t u16 = 0x1234;
+ void *dtds[1], *values[1];
+ void *dtds2[] = { &dtd1, &dtd2 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT32) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8()) */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UINT32)) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16)) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16, UINT32)) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_workaround(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT16, dtd3 = SDP_UINT32;
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ void *dtds[] = { &dtd1, &dtd2 };
+ void *values[] = { &u16, &u16 };
+ void *dtds2[] = { &dtd1, &dtd3 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT16) */
+ data = sdp_seq_alloc(dtds, values, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -247,5 +408,15 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_lang_attr/invalid_dtd",
test_sdp_get_lang_attr_invalid_dtd);
+ g_test_add_func("/lib/sdp_get_profile_descs/valid",
+ test_sdp_get_profile_descs_valid);
+ g_test_add_func("/lib/sdp_get_profile_descs/nodata",
+ test_sdp_get_profile_descs_nodata);
+ g_test_add_func("/lib/sdp_get_profile_descs/invalid_dtd",
+ test_sdp_get_profile_descs_invalid_dtd);
+ /* Test for workaround commented on sdp_get_profile_descs() */
+ g_test_add_func("/lib/sdp_get_profile_descs/workaround",
+ test_sdp_get_profile_descs_workaround);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1360940876-6314-1-git-send-email-anderson.lizardo@openbossa.org>
---
lib/sdp.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 61598bb..6c73818 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2155,9 +2155,24 @@ int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
errno = ENODATA;
return -1;
}
- for (curr = d->val.dataseq; curr; curr = curr->next)
+
+ if (!SDP_IS_SEQ(d->dtd) || d->val.dataseq == NULL)
+ goto invalid;
+
+ for (curr = d->val.dataseq; curr; curr = curr->next) {
+ if (curr->dtd != SDP_UINT16)
+ goto invalid;
*u16 = sdp_list_append(*u16, &curr->val.uint16);
+ }
+
return 0;
+
+invalid:
+ sdp_list_free(*u16, NULL);
+ *u16 = NULL;
+ errno = EINVAL;
+
+ return -1;
}
/* flexible extraction of basic attributes - Jean II */
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1360940876-6314-1-git-send-email-anderson.lizardo@openbossa.org>
Commits 0f5a5a9580084a3c4e0644ef5cd75689aeb5ff40 and
46b3a3d2d00bf70bc57ef0c9ad5542a2271e3350 introduced this workaround.
---
lib/sdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/sdp.c b/lib/sdp.c
index 396567f..61598bb 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2091,6 +2091,11 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
uint16_t version = 0x100;
if (SDP_IS_UUID(seq->dtd)) {
+ /* Mac OS X 10.7.3 and old Samsung phones do not comply
+ * to the SDP specification for
+ * BluetoothProfileDescriptorList. This workaround
+ * allows to properly parse UUID/version from SDP
+ * record published by these systems. */
sdp_data_t *next = seq->next;
uuid = &seq->val.uuid;
if (next && next->dtd == SDP_UINT16) {
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox