2011-10-05 23:20:32

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 00/15] Discovery procedure support

Hi All,

The main changes in this new version are basically Marcel's
comments about v4 version.

BR,

Andre

Andre Guedes (15):
Bluetooth: Periodic Inquiry and mgmt discovering event
Bluetooth: Add mgmt_discovery_complete()
Bluetooth: Check pending command in start_discovery()
Bluetooth: Check pending commands in stop_discovery()
Bluetooth: Create hci_do_inquiry()
Bluetooth: Create hci_cancel_inquiry()
Bluetooth: Handle race condition in Discovery
Bluetooth: Prepare for full support discovery procedures
Bluetooth: Send mgmt_discovering events
Bluetooth: Add 'eir_len' param to mgmt_device_found()
Bluetooth: Report LE devices
Bluetooth: Add LE Set Scan Parameter Command
Bluetooth: LE scan infra-structure
Bluetooth: Support LE-Only discovery procedure
Bluetooth: Support BR/EDR/LE discovery procedure

include/net/bluetooth/hci.h | 12 +++
include/net/bluetooth/hci_core.h | 15 ++++-
net/bluetooth/hci_core.c | 96 ++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 99 ++++++++++++++++-----------
net/bluetooth/mgmt.c | 138 +++++++++++++++++++++++++++++++++++---
5 files changed, 310 insertions(+), 50 deletions(-)

--
1.7.5.2



2011-10-17 12:59:32

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH v5 03/15] Bluetooth: Check pending command in start_discovery()

Hi Marcel,

On Oct 15, 2011, at 3:30 PM, Marcel Holtmann wrote:

> Hi Andre,
>=20
>> If discovery procedure is already running then EINPROGRESS command
>> status should be returned.
>>=20
>> Signed-off-by: Andre Guedes <[email protected]>
>> ---
>> net/bluetooth/mgmt.c | 6 ++++++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>=20
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index cc0c204..d8333e0 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -1622,6 +1622,12 @@ static int start_discovery(struct sock *sk, =
u16 index)
>>=20
>> hci_dev_lock_bh(hdev);
>>=20
>> + if (mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
>> + err =3D cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
>> + =
EINPROGRESS);
>> + goto failed;
>> + }
>> +
>=20
> I still do not like this at all. There should be a flag that clearly
> identifies if we are doing a discovery right now or not.

Ok, I'll implement this discovery flag.

>=20
> We need a flag on per controller to know if currently a discovery is
> going on. And going through all controllers and all pending commands =
to
> figure this out seems not a good idea. Even with a low amount of =
pending
> commands, then mgmt_pending_find is an expensive operation.
>=20
>> cmd =3D mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, index, =
NULL, 0);
>> if (!cmd) {
>> err =3D -ENOMEM;
>=20
> The other question I have is why we have to duplicate the parameters
> when adding a command to the list. Why not keep a reference of the =
SKB?
>=20
> Regards
>=20
> Marcel
>=20

Thanks,

Andre=

2011-10-17 12:50:20

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

Hi Marcel,

On Oct 15, 2011, at 3:18 PM, Marcel Holtmann wrote:

> Hi Andre,
>=20
>>>>>> This patch adds to hci_core the infra-structure to carry out the
>>>>>> LE scan. Functions were created to init the LE scan and cancel
>>>>>> an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).
>>>>>>=20
>>>>>> Also, the HCI_LE_SCAN flag was created to inform if the =
controller
>>>>>> is performing LE scan. The flag is set/cleared when the =
controller
>>>>>> starts/stops scanning.
>>>>>>=20
>>>>>> Signed-off-by: Andre Guedes <[email protected]>
>>>>>> ---
>>>>>> include/net/bluetooth/hci.h | 2 +
>>>>>> include/net/bluetooth/hci_core.h | 5 +++
>>>>>> net/bluetooth/hci_core.c | 69 =
++++++++++++++++++++++++++++++++++++++
>>>>>> net/bluetooth/hci_event.c | 4 ++
>>>>>> 4 files changed, 80 insertions(+), 0 deletions(-)
>>>>>>=20
>>>>>> diff --git a/include/net/bluetooth/hci.h =
b/include/net/bluetooth/hci.h
>>>>>> index 11537b8..7520544 100644
>>>>>> --- a/include/net/bluetooth/hci.h
>>>>>> +++ b/include/net/bluetooth/hci.h
>>>>>> @@ -86,6 +86,8 @@ enum {
>>>>>> HCI_DEBUG_KEYS,
>>>>>>=20
>>>>>> HCI_RESET,
>>>>>> +
>>>>>> + HCI_LE_SCAN,
>>>>>=20
>>>>> I remember that Marcel commented against this here. Let's hide =
this
>>>>> internally.
>>>>=20
>>>> If I got it right, Marcel agreed in keeping this flag here, but we
>>>> won't export it to userspace now (we'll keep it hidden internally).
>>>=20
>>> hdev->flags is exported to userspace, it is not internal. We need to =
create
>>> other place in struct hci_dev to keep this info. See some of =
Marcel's reply on
>>> this.
>>=20
>> When I said "export to userspace" I meant to add this flag to =
userspace
>> header (lib/hci.h). Even though hdev->flags is exported to userspace,
>> if the flag is not defined at that header the userspace cannot decode =
it
>> properly. This way the flag is hidden internally in kernel.
>>=20
>> Marcel, could you clarify this? Are you fine with we keep this =
HCI_LE_SCAN
>> in hdev->flags?
>=20
> so the hdev->flags has to stay as it is right now since otherwise you
> need to change every single driver that sets HCI_RUNNING.
>=20
> And I do not wanna export any single bit to the user that it does not
> need to see. And right now it only needs HCI_UP to HCI_RAW, because =
they
> have been there since ever. And deprecated them takes a bit longer. =
But
> even they will be deprecated in the long run. Any other bit that you
> guys introduced goes against this and needs to be fixed.
>=20
> The one part I do not get right now is why is it so hard to introduce =
a
> new flags variable for this. I mentioned this really early on and you
> keep sending patches for it and we keep discussing this.
>=20
> So I make this pretty simple, call this hdev->mgmt_flags and start
> changing these NOW. These flags need to stay internal. Mixing it with
> anything that is exported to userspace is a really bad idea.

Ok, I'll implement a new flags variable and add the LE_SCAN flag to it.
Then we can start moving other flags to this new flags variable.

Thanks,

Andre

2011-10-15 18:33:04

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v5 04/15] Bluetooth: Check pending commands in stop_discovery()

Hi Andre,

> This patch adds extra checks in stop_discovery().
>
> The MGMT_OP_STOP_DISCOVERY command should be executed if the device
> is running the discovery procedure. So, if there is no discovery
> procedure running then EINVAL command status should be returned.
>
> Also, if a MGMT_OP_STOP_DISCOVERY command has been already issued
> then EINPROGRESS command status should returned.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> net/bluetooth/mgmt.c | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index d8333e0..5e1414b 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1664,6 +1664,17 @@ static int stop_discovery(struct sock *sk, u16 index)
>
> hci_dev_lock_bh(hdev);
>
> + if (!mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
> + err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY, EINVAL);
> + goto failed;
> + }
> +
> + if (mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index)) {
> + err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY,
> + EINPROGRESS);
> + goto failed;
> + }
> +

and this case makes it obvious clear that we should use a flag to keep
track of the current discovery state. You are running through the same
list twice in a row. This is utterly stupid.

Regards

Marcel



2011-10-15 18:30:48

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v5 03/15] Bluetooth: Check pending command in start_discovery()

Hi Andre,

> If discovery procedure is already running then EINPROGRESS command
> status should be returned.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> net/bluetooth/mgmt.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index cc0c204..d8333e0 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1622,6 +1622,12 @@ static int start_discovery(struct sock *sk, u16 index)
>
> hci_dev_lock_bh(hdev);
>
> + if (mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
> + err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
> + EINPROGRESS);
> + goto failed;
> + }
> +

I still do not like this at all. There should be a flag that clearly
identifies if we are doing a discovery right now or not.

We need a flag on per controller to know if currently a discovery is
going on. And going through all controllers and all pending commands to
figure this out seems not a good idea. Even with a low amount of pending
commands, then mgmt_pending_find is an expensive operation.

> cmd = mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, index, NULL, 0);
> if (!cmd) {
> err = -ENOMEM;

The other question I have is why we have to duplicate the parameters
when adding a command to the list. Why not keep a reference of the SKB?

Regards

Marcel




2011-10-15 18:18:50

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

Hi Andre,

> >>>> This patch adds to hci_core the infra-structure to carry out the
> >>>> LE scan. Functions were created to init the LE scan and cancel
> >>>> an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).
> >>>>
> >>>> Also, the HCI_LE_SCAN flag was created to inform if the controller
> >>>> is performing LE scan. The flag is set/cleared when the controller
> >>>> starts/stops scanning.
> >>>>
> >>>> Signed-off-by: Andre Guedes <[email protected]>
> >>>> ---
> >>>> include/net/bluetooth/hci.h | 2 +
> >>>> include/net/bluetooth/hci_core.h | 5 +++
> >>>> net/bluetooth/hci_core.c | 69 ++++++++++++++++++++++++++++++++++++++
> >>>> net/bluetooth/hci_event.c | 4 ++
> >>>> 4 files changed, 80 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >>>> index 11537b8..7520544 100644
> >>>> --- a/include/net/bluetooth/hci.h
> >>>> +++ b/include/net/bluetooth/hci.h
> >>>> @@ -86,6 +86,8 @@ enum {
> >>>> HCI_DEBUG_KEYS,
> >>>>
> >>>> HCI_RESET,
> >>>> +
> >>>> + HCI_LE_SCAN,
> >>>
> >>> I remember that Marcel commented against this here. Let's hide this
> >>> internally.
> >>
> >> If I got it right, Marcel agreed in keeping this flag here, but we
> >> won't export it to userspace now (we'll keep it hidden internally).
> >
> > hdev->flags is exported to userspace, it is not internal. We need to create
> > other place in struct hci_dev to keep this info. See some of Marcel's reply on
> > this.
>
> When I said "export to userspace" I meant to add this flag to userspace
> header (lib/hci.h). Even though hdev->flags is exported to userspace,
> if the flag is not defined at that header the userspace cannot decode it
> properly. This way the flag is hidden internally in kernel.
>
> Marcel, could you clarify this? Are you fine with we keep this HCI_LE_SCAN
> in hdev->flags?

so the hdev->flags has to stay as it is right now since otherwise you
need to change every single driver that sets HCI_RUNNING.

And I do not wanna export any single bit to the user that it does not
need to see. And right now it only needs HCI_UP to HCI_RAW, because they
have been there since ever. And deprecated them takes a bit longer. But
even they will be deprecated in the long run. Any other bit that you
guys introduced goes against this and needs to be fixed.

The one part I do not get right now is why is it so hard to introduce a
new flags variable for this. I mentioned this really early on and you
keep sending patches for it and we keep discussing this.

So I make this pretty simple, call this hdev->mgmt_flags and start
changing these NOW. These flags need to stay internal. Mixing it with
anything that is exported to userspace is a really bad idea.

Regards

Marcel



2011-10-10 16:48:21

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

Hi Gustavo,

On Oct 7, 2011, at 5:12 PM, Gustavo Padovan wrote:

> Hi Andre,
>=20
> * Andre Guedes <[email protected]> [2011-10-06 17:31:06 =
-0300]:
>=20
>> Hi Gustavo,
>>=20
>> On Oct 6, 2011, at 4:06 PM, Gustavo Padovan wrote:
>>=20
>>> Hi Andre,
>>>=20
>>> * Andre Guedes <[email protected]> [2011-10-05 20:20:45 =
-0300]:
>>>=20
>>>> This patch adds to hci_core the infra-structure to carry out the
>>>> LE scan. Functions were created to init the LE scan and cancel
>>>> an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).
>>>>=20
>>>> Also, the HCI_LE_SCAN flag was created to inform if the controller
>>>> is performing LE scan. The flag is set/cleared when the controller
>>>> starts/stops scanning.
>>>>=20
>>>> Signed-off-by: Andre Guedes <[email protected]>
>>>> ---
>>>> include/net/bluetooth/hci.h | 2 +
>>>> include/net/bluetooth/hci_core.h | 5 +++
>>>> net/bluetooth/hci_core.c | 69 =
++++++++++++++++++++++++++++++++++++++
>>>> net/bluetooth/hci_event.c | 4 ++
>>>> 4 files changed, 80 insertions(+), 0 deletions(-)
>>>>=20
>>>> diff --git a/include/net/bluetooth/hci.h =
b/include/net/bluetooth/hci.h
>>>> index 11537b8..7520544 100644
>>>> --- a/include/net/bluetooth/hci.h
>>>> +++ b/include/net/bluetooth/hci.h
>>>> @@ -86,6 +86,8 @@ enum {
>>>> HCI_DEBUG_KEYS,
>>>>=20
>>>> HCI_RESET,
>>>> +
>>>> + HCI_LE_SCAN,
>>>=20
>>> I remember that Marcel commented against this here. Let's hide this
>>> internally.
>>=20
>> If I got it right, Marcel agreed in keeping this flag here, but we
>> won't export it to userspace now (we'll keep it hidden internally).
>=20
> hdev->flags is exported to userspace, it is not internal. We need to =
create
> other place in struct hci_dev to keep this info. See some of Marcel's =
reply on
> this.

When I said "export to userspace" I meant to add this flag to userspace
header (lib/hci.h). Even though hdev->flags is exported to userspace,
if the flag is not defined at that header the userspace cannot decode it
properly. This way the flag is hidden internally in kernel.

Marcel, could you clarify this? Are you fine with we keep this =
HCI_LE_SCAN
in hdev->flags?

BR,

Andre=

2011-10-10 16:47:22

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH v5 08/15] Bluetooth: Prepare for full support discovery procedures

Hi Gustavo,

On Oct 7, 2011, at 4:51 PM, Gustavo Padovan wrote:

> Hi Andre,
>
> * Andre Guedes <[email protected]> [2011-10-06 17:30:39 -0300]:
>
>> Hi Gustavo,
>>
>> On Oct 6, 2011, at 3:50 PM, Gustavo Padovan wrote:
>>
>>> Hi Andre,
>>>
>>> * Andre Guedes <[email protected]> [2011-10-05 20:20:40 -0300]:
>>>
>>>> This patch prepares start_discovery() to support LE-Only and BR/EDR/LE
>>>> discovery procedures (BR/EDR is already supported).
>>>>
>>>> Signed-off-by: Andre Guedes <[email protected]>
>>>> ---
>>>> include/net/bluetooth/hci.h | 1 +
>>>> include/net/bluetooth/hci_core.h | 1 +
>>>> net/bluetooth/mgmt.c | 12 +++++++++++-
>>>> 3 files changed, 13 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>> index aaf79af..1e11e7f 100644
>>>> --- a/include/net/bluetooth/hci.h
>>>> +++ b/include/net/bluetooth/hci.h
>>>> @@ -202,6 +202,7 @@ enum {
>>>>
>>>> #define LMP_EV4 0x01
>>>> #define LMP_EV5 0x02
>>>> +#define LMP_NO_BREDR 0x20
>>>> #define LMP_LE 0x40
>>>>
>>>> #define LMP_SNIFF_SUBR 0x02
>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>>> index 36e15cc..e0c9790 100644
>>>> --- a/include/net/bluetooth/hci_core.h
>>>> +++ b/include/net/bluetooth/hci_core.h
>>>> @@ -613,6 +613,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>>>> #define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
>>>> #define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
>>>> #define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
>>>> +#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
>>>> #define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
>>>>
>>>> /* ----- Extended LMP capabilities ----- */
>>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>>> index 58cf33a..0040e37 100644
>>>> --- a/net/bluetooth/mgmt.c
>>>> +++ b/net/bluetooth/mgmt.c
>>>> @@ -32,6 +32,8 @@
>>>> #define MGMT_VERSION 0
>>>> #define MGMT_REVISION 1
>>>>
>>>> +#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
>>>> +
>>>> struct pending_cmd {
>>>> struct list_head list;
>>>> __u16 opcode;
>>>> @@ -1632,7 +1634,15 @@ static int start_discovery(struct sock *sk, u16 index)
>>>> goto failed;
>>>> }
>>>>
>>>> - err = hci_do_inquiry(hdev, 0x08);
>>>> + if (lmp_host_le_capable(hdev)) {
>>>> + if (lmp_bredr_capable(hdev))
>>>> + err = -ENOSYS;
>>>> + else
>>>> + err = -ENOSYS;
>>>
>>> This seems a lot wrong.
>>
>> As the patch message says it prepares for supporting LE-only and
>> BR/EDR/LE discovery procedures. If you take a look further in the
>> patch series you'll see these -ENOSYS being replaced as soon as
>> those procedures are supported. Please take a look at patches 14
>> and 15.
>>
>> Anyway, if you don't like this, I can squash it in patch 14. Please
>> let me know.
>
> Please do that, makes more sense to me.

Ok, I'll do that. So, don't apply patches 08, 14 and 15. I'll squash
it and resend patches 14 and 15.

All other patches are still applicable.

BR,

Andre

2011-10-07 20:31:05

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

Hi Gustavo,

On Fri, Oct 07, 2011, Gustavo Padovan wrote:
> hdev->flags is exported to userspace, it is not internal. We need to create
> other place in struct hci_dev to keep this info. See some of Marcel's reply on
> this.

I think the best idea proposed so far is to have a hdev->legacy_flags
with a bare minimum set of possible values (HCI_UP, etc). This variable
is then exported to user space while a new hdev->flags can be created
which remains private within the kernel.

Johan

2011-10-07 20:12:33

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

Hi Andre,

* Andre Guedes <[email protected]> [2011-10-06 17:31:06 -0300]:

> Hi Gustavo,
>
> On Oct 6, 2011, at 4:06 PM, Gustavo Padovan wrote:
>
> > Hi Andre,
> >
> > * Andre Guedes <[email protected]> [2011-10-05 20:20:45 -0300]:
> >
> >> This patch adds to hci_core the infra-structure to carry out the
> >> LE scan. Functions were created to init the LE scan and cancel
> >> an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).
> >>
> >> Also, the HCI_LE_SCAN flag was created to inform if the controller
> >> is performing LE scan. The flag is set/cleared when the controller
> >> starts/stops scanning.
> >>
> >> Signed-off-by: Andre Guedes <[email protected]>
> >> ---
> >> include/net/bluetooth/hci.h | 2 +
> >> include/net/bluetooth/hci_core.h | 5 +++
> >> net/bluetooth/hci_core.c | 69 ++++++++++++++++++++++++++++++++++++++
> >> net/bluetooth/hci_event.c | 4 ++
> >> 4 files changed, 80 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >> index 11537b8..7520544 100644
> >> --- a/include/net/bluetooth/hci.h
> >> +++ b/include/net/bluetooth/hci.h
> >> @@ -86,6 +86,8 @@ enum {
> >> HCI_DEBUG_KEYS,
> >>
> >> HCI_RESET,
> >> +
> >> + HCI_LE_SCAN,
> >
> > I remember that Marcel commented against this here. Let's hide this
> > internally.
>
> If I got it right, Marcel agreed in keeping this flag here, but we
> won't export it to userspace now (we'll keep it hidden internally).

hdev->flags is exported to userspace, it is not internal. We need to create
other place in struct hci_dev to keep this info. See some of Marcel's reply on
this.

Gustavo

2011-10-07 19:51:49

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH v5 08/15] Bluetooth: Prepare for full support discovery procedures

Hi Andre,

* Andre Guedes <[email protected]> [2011-10-06 17:30:39 -0300]:

> Hi Gustavo,
>
> On Oct 6, 2011, at 3:50 PM, Gustavo Padovan wrote:
>
> > Hi Andre,
> >
> > * Andre Guedes <[email protected]> [2011-10-05 20:20:40 -0300]:
> >
> >> This patch prepares start_discovery() to support LE-Only and BR/EDR/LE
> >> discovery procedures (BR/EDR is already supported).
> >>
> >> Signed-off-by: Andre Guedes <[email protected]>
> >> ---
> >> include/net/bluetooth/hci.h | 1 +
> >> include/net/bluetooth/hci_core.h | 1 +
> >> net/bluetooth/mgmt.c | 12 +++++++++++-
> >> 3 files changed, 13 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >> index aaf79af..1e11e7f 100644
> >> --- a/include/net/bluetooth/hci.h
> >> +++ b/include/net/bluetooth/hci.h
> >> @@ -202,6 +202,7 @@ enum {
> >>
> >> #define LMP_EV4 0x01
> >> #define LMP_EV5 0x02
> >> +#define LMP_NO_BREDR 0x20
> >> #define LMP_LE 0x40
> >>
> >> #define LMP_SNIFF_SUBR 0x02
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index 36e15cc..e0c9790 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -613,6 +613,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> >> #define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
> >> #define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
> >> #define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
> >> +#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
> >> #define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
> >>
> >> /* ----- Extended LMP capabilities ----- */
> >> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> >> index 58cf33a..0040e37 100644
> >> --- a/net/bluetooth/mgmt.c
> >> +++ b/net/bluetooth/mgmt.c
> >> @@ -32,6 +32,8 @@
> >> #define MGMT_VERSION 0
> >> #define MGMT_REVISION 1
> >>
> >> +#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
> >> +
> >> struct pending_cmd {
> >> struct list_head list;
> >> __u16 opcode;
> >> @@ -1632,7 +1634,15 @@ static int start_discovery(struct sock *sk, u16 index)
> >> goto failed;
> >> }
> >>
> >> - err = hci_do_inquiry(hdev, 0x08);
> >> + if (lmp_host_le_capable(hdev)) {
> >> + if (lmp_bredr_capable(hdev))
> >> + err = -ENOSYS;
> >> + else
> >> + err = -ENOSYS;
> >
> > This seems a lot wrong.
>
> As the patch message says it prepares for supporting LE-only and
> BR/EDR/LE discovery procedures. If you take a look further in the
> patch series you'll see these -ENOSYS being replaced as soon as
> those procedures are supported. Please take a look at patches 14
> and 15.
>
> Anyway, if you don't like this, I can squash it in patch 14. Please
> let me know.

Please do that, makes more sense to me.

Gustavo

2011-10-06 20:31:06

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

Hi Gustavo,

On Oct 6, 2011, at 4:06 PM, Gustavo Padovan wrote:

> Hi Andre,
>
> * Andre Guedes <[email protected]> [2011-10-05 20:20:45 -0300]:
>
>> This patch adds to hci_core the infra-structure to carry out the
>> LE scan. Functions were created to init the LE scan and cancel
>> an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).
>>
>> Also, the HCI_LE_SCAN flag was created to inform if the controller
>> is performing LE scan. The flag is set/cleared when the controller
>> starts/stops scanning.
>>
>> Signed-off-by: Andre Guedes <[email protected]>
>> ---
>> include/net/bluetooth/hci.h | 2 +
>> include/net/bluetooth/hci_core.h | 5 +++
>> net/bluetooth/hci_core.c | 69 ++++++++++++++++++++++++++++++++++++++
>> net/bluetooth/hci_event.c | 4 ++
>> 4 files changed, 80 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index 11537b8..7520544 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -86,6 +86,8 @@ enum {
>> HCI_DEBUG_KEYS,
>>
>> HCI_RESET,
>> +
>> + HCI_LE_SCAN,
>
> I remember that Marcel commented against this here. Let's hide this
> internally.

If I got it right, Marcel agreed in keeping this flag here, but we
won't export it to userspace now (we'll keep it hidden internally).

BTW, this is the same approach we already have for other flags.

BR,

Andre

2011-10-06 20:30:39

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH v5 08/15] Bluetooth: Prepare for full support discovery procedures

Hi Gustavo,

On Oct 6, 2011, at 3:50 PM, Gustavo Padovan wrote:

> Hi Andre,
>
> * Andre Guedes <[email protected]> [2011-10-05 20:20:40 -0300]:
>
>> This patch prepares start_discovery() to support LE-Only and BR/EDR/LE
>> discovery procedures (BR/EDR is already supported).
>>
>> Signed-off-by: Andre Guedes <[email protected]>
>> ---
>> include/net/bluetooth/hci.h | 1 +
>> include/net/bluetooth/hci_core.h | 1 +
>> net/bluetooth/mgmt.c | 12 +++++++++++-
>> 3 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index aaf79af..1e11e7f 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -202,6 +202,7 @@ enum {
>>
>> #define LMP_EV4 0x01
>> #define LMP_EV5 0x02
>> +#define LMP_NO_BREDR 0x20
>> #define LMP_LE 0x40
>>
>> #define LMP_SNIFF_SUBR 0x02
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 36e15cc..e0c9790 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -613,6 +613,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>> #define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
>> #define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
>> #define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
>> +#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
>> #define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
>>
>> /* ----- Extended LMP capabilities ----- */
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 58cf33a..0040e37 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -32,6 +32,8 @@
>> #define MGMT_VERSION 0
>> #define MGMT_REVISION 1
>>
>> +#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
>> +
>> struct pending_cmd {
>> struct list_head list;
>> __u16 opcode;
>> @@ -1632,7 +1634,15 @@ static int start_discovery(struct sock *sk, u16 index)
>> goto failed;
>> }
>>
>> - err = hci_do_inquiry(hdev, 0x08);
>> + if (lmp_host_le_capable(hdev)) {
>> + if (lmp_bredr_capable(hdev))
>> + err = -ENOSYS;
>> + else
>> + err = -ENOSYS;
>
> This seems a lot wrong.

As the patch message says it prepares for supporting LE-only and
BR/EDR/LE discovery procedures. If you take a look further in the
patch series you'll see these -ENOSYS being replaced as soon as
those procedures are supported. Please take a look at patches 14
and 15.

Anyway, if you don't like this, I can squash it in patch 14. Please
let me know.

BR,

Andre

2011-10-06 19:06:18

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

Hi Andre,

* Andre Guedes <[email protected]> [2011-10-05 20:20:45 -0300]:

> This patch adds to hci_core the infra-structure to carry out the
> LE scan. Functions were created to init the LE scan and cancel
> an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).
>
> Also, the HCI_LE_SCAN flag was created to inform if the controller
> is performing LE scan. The flag is set/cleared when the controller
> starts/stops scanning.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> include/net/bluetooth/hci.h | 2 +
> include/net/bluetooth/hci_core.h | 5 +++
> net/bluetooth/hci_core.c | 69 ++++++++++++++++++++++++++++++++++++++
> net/bluetooth/hci_event.c | 4 ++
> 4 files changed, 80 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 11537b8..7520544 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -86,6 +86,8 @@ enum {
> HCI_DEBUG_KEYS,
>
> HCI_RESET,
> +
> + HCI_LE_SCAN,

I remember that Marcel commented against this here. Let's hide this
internally.

Gustavo

2011-10-06 18:50:41

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH v5 08/15] Bluetooth: Prepare for full support discovery procedures

Hi Andre,

* Andre Guedes <[email protected]> [2011-10-05 20:20:40 -0300]:

> This patch prepares start_discovery() to support LE-Only and BR/EDR/LE
> discovery procedures (BR/EDR is already supported).
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/mgmt.c | 12 +++++++++++-
> 3 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index aaf79af..1e11e7f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -202,6 +202,7 @@ enum {
>
> #define LMP_EV4 0x01
> #define LMP_EV5 0x02
> +#define LMP_NO_BREDR 0x20
> #define LMP_LE 0x40
>
> #define LMP_SNIFF_SUBR 0x02
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 36e15cc..e0c9790 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -613,6 +613,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> #define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
> #define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
> #define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
> +#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
> #define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
>
> /* ----- Extended LMP capabilities ----- */
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 58cf33a..0040e37 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -32,6 +32,8 @@
> #define MGMT_VERSION 0
> #define MGMT_REVISION 1
>
> +#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
> +
> struct pending_cmd {
> struct list_head list;
> __u16 opcode;
> @@ -1632,7 +1634,15 @@ static int start_discovery(struct sock *sk, u16 index)
> goto failed;
> }
>
> - err = hci_do_inquiry(hdev, 0x08);
> + if (lmp_host_le_capable(hdev)) {
> + if (lmp_bredr_capable(hdev))
> + err = -ENOSYS;
> + else
> + err = -ENOSYS;

This seems a lot wrong.

Gustavo

2011-10-05 23:20:47

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 15/15] Bluetooth: Support BR/EDR/LE discovery procedure

This patch adds support for BR/EDR/LE discovery procedure through
management interface.

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +
net/bluetooth/hci_event.c | 16 ++++++++++---
net/bluetooth/mgmt.c | 42 +++++++++++++++++++++++++++++++++++++-
3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c6ae380..c5c38e3 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -880,6 +880,8 @@ int mgmt_device_blocked(u16 index, bdaddr_t *bdaddr);
int mgmt_device_unblocked(u16 index, bdaddr_t *bdaddr);
int mgmt_discovery_complete(u16 index, u8 status);
int mgmt_has_pending_stop_discov(u16 index);
+int mgmt_interleaved_discovery(u16 index);
+int mgmt_is_interleaved_discovery(u16 index);

/* HCI info for socket */
#define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 5b99058..029badd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -896,12 +896,17 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
if (cp->enable == 0x01) {
if (status) {
mgmt_discovery_complete(hdev->id, status);
+
+ if (mgmt_is_interleaved_discovery(hdev->id))
+ mgmt_discovering(hdev->id, 0);
+
return;
}

set_bit(HCI_LE_SCAN, &hdev->flags);

- mgmt_discovering(hdev->id, 1);
+ if (!mgmt_is_interleaved_discovery(hdev->id))
+ mgmt_discovering(hdev->id, 1);

del_timer(&hdev->adv_timer);

@@ -1358,6 +1363,7 @@ static void hci_cs_le_start_enc(struct hci_dev *hdev, u8 status)
static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 status = *((__u8 *) skb->data);
+ int err;

BT_DBG("%s status %d", hdev->name, status);

@@ -1368,9 +1374,11 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
return;

- mgmt_discovery_complete(hdev->id, 0);
-
- mgmt_discovering(hdev->id, 0);
+ err = mgmt_interleaved_discovery(hdev->id);
+ if (err < 0) {
+ mgmt_discovery_complete(hdev->id, 0);
+ mgmt_discovering(hdev->id, 0);
+ }
}

static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 685519b..1096a37 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -40,8 +40,10 @@
#define LE_SCAN_WIN 0x12
#define LE_SCAN_INT 0x12
#define LE_SCAN_TIMEOUT_LE_ONLY 10240 /* TGAP(gen_disc_scan_min) */
+#define LE_SCAN_TIMEOUT_BREDR_LE 5120 /* TGAP(100)/2 */

#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
+#define INQUIRY_LEN_BREDR_LE 0x04 /* TGAP(100)/2 */

struct pending_cmd {
struct list_head list;
@@ -1645,7 +1647,7 @@ static int start_discovery(struct sock *sk, u16 index)

if (lmp_host_le_capable(hdev)) {
if (lmp_bredr_capable(hdev))
- err = -ENOSYS;
+ err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR_LE);
else
err = hci_do_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
@@ -2456,3 +2458,41 @@ int mgmt_has_pending_stop_discov(u16 index)

return 0;
}
+
+int mgmt_is_interleaved_discovery(u16 index)
+{
+ struct hci_dev *hdev;
+ int res = 0;
+
+ hdev = hci_dev_get(index);
+ if (!hdev)
+ return 0;
+
+ if (lmp_host_le_capable(hdev) && lmp_bredr_capable(hdev) &&
+ mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev->id))
+ res = 1;
+
+ hci_dev_put(hdev);
+
+ return res;
+}
+
+int mgmt_interleaved_discovery(u16 index)
+{
+ struct hci_dev *hdev;
+ int err;
+
+ if (!mgmt_is_interleaved_discovery(index))
+ return -EPERM;
+
+ hdev = hci_dev_get(index);
+ if (!hdev)
+ return -ENODEV;
+
+ err = hci_do_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT, LE_SCAN_WIN,
+ LE_SCAN_TIMEOUT_BREDR_LE);
+
+ hci_dev_put(hdev);
+
+ return err;
+}
--
1.7.5.2


2011-10-05 23:20:46

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 14/15] Bluetooth: Support LE-Only discovery procedure

This patch adds support for LE-Only discovery procedure through
management interface.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 16 +++++++++++++---
net/bluetooth/mgmt.c | 14 +++++++++++++-
2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 841501c..5b99058 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -889,14 +889,16 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,

BT_DBG("%s status 0x%x", hdev->name, status);

- if (status)
- return;
-
cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_SCAN_ENABLE);
if (!cp)
return;

if (cp->enable == 0x01) {
+ if (status) {
+ mgmt_discovery_complete(hdev->id, status);
+ return;
+ }
+
set_bit(HCI_LE_SCAN, &hdev->flags);

mgmt_discovering(hdev->id, 1);
@@ -906,7 +908,15 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
hci_dev_lock(hdev);
hci_adv_entries_clear(hdev);
hci_dev_unlock(hdev);
+
+ if (mgmt_has_pending_stop_discov(hdev->id))
+ hci_cancel_le_scan(hdev);
} else if (cp->enable == 0x00) {
+ mgmt_discovery_complete(hdev->id, status);
+
+ if (status)
+ return;
+
clear_bit(HCI_LE_SCAN, &hdev->flags);

mgmt_discovering(hdev->id, 0);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0b08e29..685519b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,6 +32,15 @@
#define MGMT_VERSION 0
#define MGMT_REVISION 1

+/*
+ * 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) */
+
#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */

struct pending_cmd {
@@ -1638,7 +1647,8 @@ static int start_discovery(struct sock *sk, u16 index)
if (lmp_bredr_capable(hdev))
err = -ENOSYS;
else
- err = -ENOSYS;
+ err = hci_do_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
+ LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
} else {
err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
}
@@ -1686,6 +1696,8 @@ static int stop_discovery(struct sock *sk, u16 index)

if (test_bit(HCI_INQUIRY, &hdev->flags))
err = hci_cancel_inquiry(hdev);
+ else if (test_bit(HCI_LE_SCAN, &hdev->flags))
+ err = hci_cancel_le_scan(hdev);
else
err = 0;

--
1.7.5.2


2011-10-05 23:20:45

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 13/15] Bluetooth: LE scan infra-structure

This patch adds to hci_core the infra-structure to carry out the
LE scan. Functions were created to init the LE scan and cancel
an ongoing scanning (hci_do_le_scan and hci_cancel_le_scan).

Also, the HCI_LE_SCAN flag was created to inform if the controller
is performing LE scan. The flag is set/cleared when the controller
starts/stops scanning.

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci.h | 2 +
include/net/bluetooth/hci_core.h | 5 +++
net/bluetooth/hci_core.c | 69 ++++++++++++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 4 ++
4 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 11537b8..7520544 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -86,6 +86,8 @@ enum {
HCI_DEBUG_KEYS,

HCI_RESET,
+
+ HCI_LE_SCAN,
};

/* HCI ioctl defines */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 65a1ccf..c6ae380 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -208,6 +208,8 @@ struct hci_dev {
struct list_head adv_entries;
struct timer_list adv_timer;

+ struct timer_list le_scan_timer;
+
struct hci_dev_stats stat;

struct sk_buff_head driver_init;
@@ -918,5 +920,8 @@ void hci_le_ltk_neg_reply(struct hci_conn *conn);

int hci_do_inquiry(struct hci_dev *hdev, u8 length);
int hci_cancel_inquiry(struct hci_dev *hdev);
+int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
+ int timeout);
+int hci_cancel_le_scan(struct hci_dev *hdev);

#endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e60f16a..0d0e460 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1421,6 +1421,23 @@ int hci_add_adv_entry(struct hci_dev *hdev,
return 0;
}

+static int le_scan(struct hci_dev *hdev, u8 enable)
+{
+ struct hci_cp_le_set_scan_enable cp;
+
+ memset(&cp, 0, sizeof(cp));
+ cp.enable = enable;
+
+ return hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+}
+
+static void le_scan_timeout(unsigned long arg)
+{
+ struct hci_dev *hdev = (void *) arg;
+
+ le_scan(hdev, 0);
+}
+
/* Register HCI device */
int hci_register_dev(struct hci_dev *hdev)
{
@@ -1491,6 +1508,9 @@ int hci_register_dev(struct hci_dev *hdev)
setup_timer(&hdev->adv_timer, hci_clear_adv_cache,
(unsigned long) hdev);

+ setup_timer(&hdev->le_scan_timer, le_scan_timeout,
+ (unsigned long) hdev);
+
INIT_WORK(&hdev->power_on, hci_power_on);
INIT_WORK(&hdev->power_off, hci_power_off);
setup_timer(&hdev->off_timer, hci_auto_off, (unsigned long) hdev);
@@ -1564,6 +1584,7 @@ int hci_unregister_dev(struct hci_dev *hdev)

hci_del_off_timer(hdev);
del_timer(&hdev->adv_timer);
+ del_timer(&hdev->le_scan_timer);

destroy_workqueue(hdev->workqueue);

@@ -2434,3 +2455,51 @@ int hci_cancel_inquiry(struct hci_dev *hdev)

return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
}
+
+static int set_le_scan_param(struct hci_dev *hdev, u8 type, u16 interval,
+ u16 window)
+{
+ struct hci_cp_le_set_scan_param cp;
+
+ memset(&cp, 0, sizeof(cp));
+ cp.type = type;
+ cp.interval = cpu_to_le16(interval);
+ cp.window = cpu_to_le16(window);
+
+ return hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
+}
+
+int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
+ int timeout)
+{
+ int err;
+
+ BT_DBG("%s", hdev->name);
+
+ if (test_bit(HCI_LE_SCAN, &hdev->flags))
+ return -EPERM;
+
+ err = set_le_scan_param(hdev, type, interval, window);
+ if (err < 0)
+ return err;
+
+ err = le_scan(hdev, 1);
+ if (err < 0)
+ return err;
+
+ mod_timer(&hdev->le_scan_timer, jiffies + msecs_to_jiffies(timeout));
+
+ return 0;
+}
+
+int hci_cancel_le_scan(struct hci_dev *hdev)
+{
+ BT_DBG("%s", hdev->name);
+
+ if (!test_bit(HCI_LE_SCAN, &hdev->flags))
+ return -EPERM;
+
+ del_timer(&hdev->le_scan_timer);
+
+ return le_scan(hdev, 0);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8f3e00d..841501c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -897,6 +897,8 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
return;

if (cp->enable == 0x01) {
+ set_bit(HCI_LE_SCAN, &hdev->flags);
+
mgmt_discovering(hdev->id, 1);

del_timer(&hdev->adv_timer);
@@ -905,6 +907,8 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
hci_adv_entries_clear(hdev);
hci_dev_unlock(hdev);
} else if (cp->enable == 0x00) {
+ clear_bit(HCI_LE_SCAN, &hdev->flags);
+
mgmt_discovering(hdev->id, 0);

mod_timer(&hdev->adv_timer, jiffies + ADV_CLEAR_TIMEOUT);
--
1.7.5.2


2011-10-05 23:20:44

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 12/15] Bluetooth: Add LE Set Scan Parameter Command

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci.h | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 1e11e7f..11537b8 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -739,6 +739,15 @@ struct hci_rp_le_read_buffer_size {
__u8 le_max_pkt;
} __packed;

+#define HCI_OP_LE_SET_SCAN_PARAM 0x200b
+struct hci_cp_le_set_scan_param {
+ __u8 type;
+ __le16 interval;
+ __le16 window;
+ __u8 own_address_type;
+ __u8 filter_policy;
+} __packed;
+
#define HCI_OP_LE_SET_SCAN_ENABLE 0x200c
struct hci_cp_le_set_scan_enable {
__u8 enable;
--
1.7.5.2


2011-10-05 23:20:43

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 11/15] Bluetooth: Report LE devices

Devices found during LE scan should be reported to userspace through
mgmt_device_found events.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index fceb2ea..8f3e00d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2834,6 +2834,7 @@ static inline void hci_le_adv_report_evt(struct hci_dev *hdev,
{
u8 num_reports = skb->data[0];
void *ptr = &skb->data[1];
+ s8 rssi;

hci_dev_lock(hdev);

@@ -2842,6 +2843,10 @@ static inline void hci_le_adv_report_evt(struct hci_dev *hdev,

hci_add_adv_entry(hdev, ev);

+ rssi = ev->data[ev->length];
+ mgmt_device_found(hdev->id, &ev->bdaddr, NULL, rssi, ev->data,
+ ev->length);
+
ptr += sizeof(*ev) + ev->length + 1;
}

--
1.7.5.2


2011-10-05 23:20:42

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 10/15] Bluetooth: Add 'eir_len' param to mgmt_device_found()

This patch adds a new parameter to mgmt_device_found() to inform
the length of 'eir' pointer.

EIR data from LE advertising report event doesn't have a fixed length
as EIR data from extended inquiry result event does. We needed to
change mgmt_device_found() so it copies 'eir_len' bytes instead of
HCI_MAX_EIR_LENGTH.

Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_event.c | 9 +++++----
net/bluetooth/mgmt.c | 7 +++++--
3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e0c9790..65a1ccf 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -871,7 +871,7 @@ int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status);
int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
u8 status);
int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
- u8 *eir);
+ u8 *eir, u8 eir_len);
int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
int mgmt_discovering(u16 index, u8 discovering);
int mgmt_device_blocked(u16 index, bdaddr_t *bdaddr);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0bbd8fe..fceb2ea 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1383,7 +1383,7 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *
data.ssp_mode = 0x00;
hci_inquiry_cache_update(hdev, &data);
mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class, 0,
- NULL);
+ NULL, 0);
}

hci_dev_unlock(hdev);
@@ -2384,7 +2384,7 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
hci_inquiry_cache_update(hdev, &data);
mgmt_device_found(hdev->id, &info->bdaddr,
info->dev_class, info->rssi,
- NULL);
+ NULL, 0);
}
} else {
struct inquiry_info_with_rssi *info = (void *) (skb->data + 1);
@@ -2401,7 +2401,7 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
hci_inquiry_cache_update(hdev, &data);
mgmt_device_found(hdev->id, &info->bdaddr,
info->dev_class, info->rssi,
- NULL);
+ NULL, 0);
}
}

@@ -2543,7 +2543,8 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
data.ssp_mode = 0x01;
hci_inquiry_cache_update(hdev, &data);
mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class,
- info->rssi, info->data);
+ info->rssi, info->data,
+ sizeof(info->data));
}

hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0040e37..0b08e29 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2341,17 +2341,20 @@ int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
}

int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
- u8 *eir)
+ u8 *eir, u8 eir_len)
{
struct mgmt_ev_device_found ev;

+ if (eir_len > sizeof(ev.eir))
+ return -EINVAL;
+
memset(&ev, 0, sizeof(ev));

bacpy(&ev.bdaddr, bdaddr);
ev.rssi = rssi;

if (eir)
- memcpy(ev.eir, eir, sizeof(ev.eir));
+ memcpy(ev.eir, eir, eir_len);

if (dev_class)
memcpy(ev.dev_class, dev_class, sizeof(ev.dev_class));
--
1.7.5.2


2011-10-05 23:20:41

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 09/15] Bluetooth: Send mgmt_discovering events

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 08f7712..0bbd8fe 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -897,12 +897,16 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
return;

if (cp->enable == 0x01) {
+ mgmt_discovering(hdev->id, 1);
+
del_timer(&hdev->adv_timer);

hci_dev_lock(hdev);
hci_adv_entries_clear(hdev);
hci_dev_unlock(hdev);
} else if (cp->enable == 0x00) {
+ mgmt_discovering(hdev->id, 0);
+
mod_timer(&hdev->adv_timer, jiffies + ADV_CLEAR_TIMEOUT);
}
}
--
1.7.5.2


2011-10-05 23:20:40

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 08/15] Bluetooth: Prepare for full support discovery procedures

This patch prepares start_discovery() to support LE-Only and BR/EDR/LE
discovery procedures (BR/EDR is already supported).

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/mgmt.c | 12 +++++++++++-
3 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index aaf79af..1e11e7f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -202,6 +202,7 @@ enum {

#define LMP_EV4 0x01
#define LMP_EV5 0x02
+#define LMP_NO_BREDR 0x20
#define LMP_LE 0x40

#define LMP_SNIFF_SUBR 0x02
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 36e15cc..e0c9790 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -613,6 +613,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
#define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
+#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
#define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)

/* ----- Extended LMP capabilities ----- */
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 58cf33a..0040e37 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,6 +32,8 @@
#define MGMT_VERSION 0
#define MGMT_REVISION 1

+#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
+
struct pending_cmd {
struct list_head list;
__u16 opcode;
@@ -1632,7 +1634,15 @@ static int start_discovery(struct sock *sk, u16 index)
goto failed;
}

- err = hci_do_inquiry(hdev, 0x08);
+ if (lmp_host_le_capable(hdev)) {
+ if (lmp_bredr_capable(hdev))
+ err = -ENOSYS;
+ else
+ err = -ENOSYS;
+ } else {
+ err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
+ }
+
if (err < 0)
mgmt_pending_remove(cmd);

--
1.7.5.2


2011-10-05 23:20:39

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 07/15] Bluetooth: Handle race condition in Discovery

If MGMT_OP_STOP_DISCOVERY command is issued before the kernel
receives the HCI Inquiry Command Status Event from the controller
then that command will fail and the discovery procedure won't be
stopped. This situation may occur if a MGMT_OP_STOP_DISCOVERY
command is issued just after a MGMT_OP_START_DISCOVERY.

This race condition can be handled by checking for pending
MGMT_OP_STOP_DISCOVERY command in inquiry command status event
handler. If we have a pending MGMT_OP_STOP_DISCOVERY command we
cancel the ongoing discovery.

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_event.c | 3 +++
net/bluetooth/mgmt.c | 14 +++++++++++++-
3 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4a79a50..36e15cc 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -876,6 +876,7 @@ int mgmt_discovering(u16 index, u8 discovering);
int mgmt_device_blocked(u16 index, bdaddr_t *bdaddr);
int mgmt_device_unblocked(u16 index, bdaddr_t *bdaddr);
int mgmt_discovery_complete(u16 index, u8 status);
+int mgmt_has_pending_stop_discov(u16 index);

/* HCI info for socket */
#define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1d890e1..08f7712 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -962,6 +962,9 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
set_bit(HCI_INQUIRY, &hdev->flags);

mgmt_discovering(hdev->id, 1);
+
+ if (mgmt_has_pending_stop_discov(hdev->id))
+ hci_cancel_inquiry(hdev);
}

static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d84f242..58cf33a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1674,7 +1674,11 @@ static int stop_discovery(struct sock *sk, u16 index)
goto failed;
}

- err = hci_cancel_inquiry(hdev);
+ if (test_bit(HCI_INQUIRY, &hdev->flags))
+ err = hci_cancel_inquiry(hdev);
+ else
+ err = 0;
+
if (err < 0)
mgmt_pending_remove(cmd);

@@ -2419,3 +2423,11 @@ int mgmt_discovery_complete(u16 index, u8 status)

return 0;
}
+
+int mgmt_has_pending_stop_discov(u16 index)
+{
+ if (mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index))
+ return 1;
+
+ return 0;
+}
--
1.7.5.2


2011-10-05 23:20:38

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 06/15] Bluetooth: Create hci_cancel_inquiry()

This patch adds a function to hci_core to cancel an ongoing inquiry.

According to the Bluetooth spec, the inquiry cancel command should
only be issued after the inquiry command has been issued, a command
status event has been received for the inquiry command, and before
the inquiry complete event occurs.

As HCI_INQUIRY flag is only set just after an inquiry command status
event occurs and it is cleared just after an inquiry complete event
occurs, the inquiry cancel command should be issued only if HCI_INQUIRY
flag is set.

Additionally, cancel inquiry related code from stop_discovery() were
replaced by a hci_cancel_inquiry() call.

Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 10 ++++++++++
net/bluetooth/mgmt.c | 2 +-
3 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b7c070b..4a79a50 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -915,5 +915,6 @@ void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
void hci_le_ltk_neg_reply(struct hci_conn *conn);

int hci_do_inquiry(struct hci_dev *hdev, u8 length);
+int hci_cancel_inquiry(struct hci_dev *hdev);

#endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8516420..e60f16a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2424,3 +2424,13 @@ int hci_do_inquiry(struct hci_dev *hdev, u8 length)

return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
}
+
+int hci_cancel_inquiry(struct hci_dev *hdev)
+{
+ BT_DBG("%s", hdev->name);
+
+ if (!test_bit(HCI_INQUIRY, &hdev->flags))
+ return -EPERM;
+
+ return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4f77468..d84f242 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1674,7 +1674,7 @@ static int stop_discovery(struct sock *sk, u16 index)
goto failed;
}

- err = hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+ err = hci_cancel_inquiry(hdev);
if (err < 0)
mgmt_pending_remove(cmd);

--
1.7.5.2


2011-10-05 23:20:37

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 05/15] Bluetooth: Create hci_do_inquiry()

This patch adds a function to hci_core to carry out inquiry.

All inquiry code from start_discovery() were replaced by a
hci_do_inquiry() call.

Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_core.c | 17 +++++++++++++++++
net/bluetooth/mgmt.c | 9 +--------
3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index df6fa85..b7c070b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -914,4 +914,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
void hci_le_ltk_reply(struct hci_conn *conn, u8 ltk[16]);
void hci_le_ltk_neg_reply(struct hci_conn *conn);

+int hci_do_inquiry(struct hci_dev *hdev, u8 length);
+
#endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b84458d..8516420 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2407,3 +2407,20 @@ static void hci_cmd_task(unsigned long arg)
}
}
}
+
+int hci_do_inquiry(struct hci_dev *hdev, u8 length)
+{
+ u8 lap[3] = { 0x33, 0x8b, 0x9e };
+ struct hci_cp_inquiry cp;
+
+ BT_DBG("%s", hdev->name);
+
+ if (test_bit(HCI_INQUIRY, &hdev->flags))
+ return -EPERM;
+
+ memset(&cp, 0, sizeof(cp));
+ memcpy(&cp.lap, lap, 3);
+ cp.length = length;
+
+ return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5e1414b..4f77468 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1608,8 +1608,6 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,

static int start_discovery(struct sock *sk, u16 index)
{
- u8 lap[3] = { 0x33, 0x8b, 0x9e };
- struct hci_cp_inquiry cp;
struct pending_cmd *cmd;
struct hci_dev *hdev;
int err;
@@ -1634,12 +1632,7 @@ static int start_discovery(struct sock *sk, u16 index)
goto failed;
}

- memset(&cp, 0, sizeof(cp));
- memcpy(&cp.lap, lap, 3);
- cp.length = 0x08;
- cp.num_rsp = 0x00;
-
- err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+ err = hci_do_inquiry(hdev, 0x08);
if (err < 0)
mgmt_pending_remove(cmd);

--
1.7.5.2


2011-10-05 23:20:36

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 04/15] Bluetooth: Check pending commands in stop_discovery()

This patch adds extra checks in stop_discovery().

The MGMT_OP_STOP_DISCOVERY command should be executed if the device
is running the discovery procedure. So, if there is no discovery
procedure running then EINVAL command status should be returned.

Also, if a MGMT_OP_STOP_DISCOVERY command has been already issued
then EINPROGRESS command status should returned.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/mgmt.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d8333e0..5e1414b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1664,6 +1664,17 @@ static int stop_discovery(struct sock *sk, u16 index)

hci_dev_lock_bh(hdev);

+ if (!mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
+ err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY, EINVAL);
+ goto failed;
+ }
+
+ if (mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index)) {
+ err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY,
+ EINPROGRESS);
+ goto failed;
+ }
+
cmd = mgmt_pending_add(sk, MGMT_OP_STOP_DISCOVERY, index, NULL, 0);
if (!cmd) {
err = -ENOMEM;
--
1.7.5.2


2011-10-05 23:20:35

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 03/15] Bluetooth: Check pending command in start_discovery()

If discovery procedure is already running then EINPROGRESS command
status should be returned.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/mgmt.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index cc0c204..d8333e0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1622,6 +1622,12 @@ static int start_discovery(struct sock *sk, u16 index)

hci_dev_lock_bh(hdev);

+ if (mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
+ err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
+ EINPROGRESS);
+ goto failed;
+ }
+
cmd = mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, index, NULL, 0);
if (!cmd) {
err = -ENOMEM;
--
1.7.5.2


2011-10-05 23:20:34

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 02/15] Bluetooth: Add mgmt_discovery_complete()

This patch adds the mgmt_discovery_complete() function which
removes pending discovery commands and sends command complete/
status events.

This function should be called when the discovery procedure finishes.

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_event.c | 7 +++++++
net/bluetooth/mgmt.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5b92442..df6fa85 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -875,6 +875,7 @@ int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
int mgmt_discovering(u16 index, u8 discovering);
int mgmt_device_blocked(u16 index, bdaddr_t *bdaddr);
int mgmt_device_unblocked(u16 index, bdaddr_t *bdaddr);
+int mgmt_discovery_complete(u16 index, u8 status);

/* HCI info for socket */
#define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index f5ed744..1d890e1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -55,6 +55,8 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)

BT_DBG("%s status 0x%x", hdev->name, status);

+ mgmt_discovery_complete(hdev->id, status);
+
if (status)
return;

@@ -951,6 +953,9 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
if (status) {
hci_req_complete(hdev, HCI_OP_INQUIRY, status);
hci_conn_check_pending(hdev);
+
+ mgmt_discovery_complete(hdev->id, status);
+
return;
}

@@ -1342,6 +1347,8 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
return;

+ mgmt_discovery_complete(hdev->id, 0);
+
mgmt_discovering(hdev->id, 0);
}

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5a94eec..cc0c204 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2378,3 +2378,34 @@ int mgmt_device_unblocked(u16 index, bdaddr_t *bdaddr)
return mgmt_event(MGMT_EV_DEVICE_UNBLOCKED, index, &ev, sizeof(ev),
cmd ? cmd->sk : NULL);
}
+
+int mgmt_discovery_complete(u16 index, u8 status)
+{
+ struct pending_cmd *cmd_start;
+ struct pending_cmd *cmd_stop;
+ u8 discov_status = bt_to_errno(status);
+
+ cmd_start = mgmt_pending_find(MGMT_OP_START_DISCOVERY, index);
+ if (!cmd_start)
+ return -ENOENT;
+
+ BT_DBG("hci%u status %u", index, status);
+
+ cmd_stop = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index);
+ if (cmd_stop && status == 0)
+ discov_status = ECANCELED;
+
+ if (discov_status)
+ cmd_status(cmd_start->sk, index, MGMT_OP_START_DISCOVERY,
+ discov_status);
+ else
+ cmd_complete(cmd_start->sk, index, MGMT_OP_START_DISCOVERY,
+ NULL, 0);
+
+ mgmt_pending_remove(cmd_start);
+
+ if (cmd_stop)
+ mgmt_pending_remove(cmd_stop);
+
+ return 0;
+}
--
1.7.5.2


2011-10-05 23:20:33

by Andre Guedes

[permalink] [raw]
Subject: [PATCH v5 01/15] Bluetooth: Periodic Inquiry and mgmt discovering event

By using periodic inquiry command we're not able to detect correctly
when the controller has started inquiry.

Today we have this workaround in inquiry result event handler to set
the HCI_INQUIRY flag when it sees the first inquiry result event.
This workaround isn't enough because the device may be performing
an inquiry but the HCI_INQUIRY flag is not set. For instance, if
there is no device in range, no inquiry result event is generated,
consequently, the HCI_INQUIRY flags isn't set when it should so.

We rely on HCI_INQUIRY flag to implement the discovery procedure
properly. So, as we aren't able to clear/set the HCI_INQUIRY flag in
a reliable manner, periodic inquiry events shouldn't change the
HCI_INQUIRY flag. In future, if needed, we might add a new flag (e.g.
HCI_PINQUIRY) to know if the controller is performing periodic
inquiry.

Thus, due to that issue and in order to keep compatibility with
userspace, periodic inquiry events shouldn't send mgmt discovering
events.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 43 +++++++++++--------------------------------
1 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d7d96b6..f5ed744 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -58,9 +58,9 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
if (status)
return;

- if (test_and_clear_bit(HCI_INQUIRY, &hdev->flags) &&
- test_bit(HCI_MGMT, &hdev->flags))
- mgmt_discovering(hdev->id, 0);
+ clear_bit(HCI_INQUIRY, &hdev->flags);
+
+ mgmt_discovering(hdev->id, 0);

hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);

@@ -76,10 +76,6 @@ static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
if (status)
return;

- if (test_and_clear_bit(HCI_INQUIRY, &hdev->flags) &&
- test_bit(HCI_MGMT, &hdev->flags))
- mgmt_discovering(hdev->id, 0);
-
hci_conn_check_pending(hdev);
}

@@ -958,9 +954,9 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
return;
}

- if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags) &&
- test_bit(HCI_MGMT, &hdev->flags))
- mgmt_discovering(hdev->id, 1);
+ set_bit(HCI_INQUIRY, &hdev->flags);
+
+ mgmt_discovering(hdev->id, 1);
}

static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
@@ -1339,13 +1335,14 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff

BT_DBG("%s status %d", hdev->name, status);

- if (test_and_clear_bit(HCI_INQUIRY, &hdev->flags) &&
- test_bit(HCI_MGMT, &hdev->flags))
- mgmt_discovering(hdev->id, 0);
-
hci_req_complete(hdev, HCI_OP_INQUIRY, status);

hci_conn_check_pending(hdev);
+
+ if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
+ return;
+
+ mgmt_discovering(hdev->id, 0);
}

static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1361,12 +1358,6 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *

hci_dev_lock(hdev);

- if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
- if (test_bit(HCI_MGMT, &hdev->flags))
- mgmt_discovering(hdev->id, 1);
- }
-
for (; num_rsp; num_rsp--, info++) {
bacpy(&data.bdaddr, &info->bdaddr);
data.pscan_rep_mode = info->pscan_rep_mode;
@@ -2363,12 +2354,6 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct

hci_dev_lock(hdev);

- if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
- if (test_bit(HCI_MGMT, &hdev->flags))
- mgmt_discovering(hdev->id, 1);
- }
-
if ((skb->len - 1) / num_rsp != sizeof(struct inquiry_info_with_rssi)) {
struct inquiry_info_with_rssi_and_pscan_mode *info;
info = (void *) (skb->data + 1);
@@ -2531,12 +2516,6 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
if (!num_rsp)
return;

- if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
- if (test_bit(HCI_MGMT, &hdev->flags))
- mgmt_discovering(hdev->id, 1);
- }
-
hci_dev_lock(hdev);

for (; num_rsp; num_rsp--, info++) {
--
1.7.5.2