2012-02-23 21:09:27

by Andre Guedes

[permalink] [raw]
Subject: [PATCH] Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

This patch add an extra check for BR/EDR and LE-Only discovery.
This way, we are able to return error immediately if the discovery
type requested is not supported by the device.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 93f2c13..de40918 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2552,12 +2552,18 @@ static int start_discovery(struct sock *sk, u16 index,

switch (hdev->discovery.type) {
case DISCOV_TYPE_BREDR:
- err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
+ if (lmp_bredr_capable(hdev))
+ err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
+ else
+ err = -ENOTSUPP;
break;

case DISCOV_TYPE_LE:
- err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
+ if (lmp_host_le_capable(hdev))
+ err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
+ else
+ err = -ENOTSUPP;
break;

case DISCOV_TYPE_INTERLEAVED:
--
1.7.9.2



2012-02-23 22:05:16

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

Hi Andre,

> This patch add an extra check for BR/EDR and LE-Only discovery.
> This way, we are able to return error immediately if the discovery
> type requested is not supported by the device.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> net/bluetooth/mgmt.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)

so besides my other comment, this patch can go in as is and we have to
add an extra patch to handle interleaved once we talked through what's
the best approach.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-02-23 22:00:23

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

Hi Andre,

> >> This patch add an extra check for BR/EDR and LE-Only discovery.
> >> This way, we are able to return error immediately if the discovery
> >> type requested is not supported by the device.
> >>
> >> Signed-off-by: Andre Guedes <[email protected]>
> >> ---
> >> net/bluetooth/mgmt.c | 10 ++++++++--
> >> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> >> index 93f2c13..de40918 100644
> >> --- a/net/bluetooth/mgmt.c
> >> +++ b/net/bluetooth/mgmt.c
> >> @@ -2552,12 +2552,18 @@ static int start_discovery(struct sock *sk, u16 index,
> >>
> >> switch (hdev->discovery.type) {
> >> case DISCOV_TYPE_BREDR:
> >> - err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
> >> + if (lmp_bredr_capable(hdev))
> >> + err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
> >> + else
> >> + err = -ENOTSUPP;
> >> break;
> >>
> >> case DISCOV_TYPE_LE:
> >> - err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
> >> + if (lmp_host_le_capable(hdev))
> >> + err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
> >> LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
> >> + else
> >> + err = -ENOTSUPP;
> >> break;
> >>
> >> case DISCOV_TYPE_INTERLEAVED:
> >
> > so what about interleaved. Should that actually suceed if LE is not
> > enabled?
>
> For interleaved, we already check this capabilities.
>
> According to the discussion in "[PATCH] Don't set LE flags in
> mgmt_start_discovery", if an interleaved Start Discovery command is
> issued but the device is not dual mode, we perform a regular BR/EDR
> or LE-only discovery (according to device capabilities) instead of
> returning error.

is that a behavior we really want? Comments anyone?

Regards

Marcel



2012-02-23 21:23:51

by Andre Guedes

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

Hi Marcel,

On Thu, Feb 23, 2012 at 6:14 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Andre,
>
>> This patch add an extra check for BR/EDR and LE-Only discovery.
>> This way, we are able to return error immediately if the discovery
>> type requested is not supported by the device.
>>
>> Signed-off-by: Andre Guedes <[email protected]>
>> ---
>> =A0net/bluetooth/mgmt.c | =A0 10 ++++++++--
>> =A01 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 93f2c13..de40918 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -2552,12 +2552,18 @@ static int start_discovery(struct sock *sk, u16 =
index,
>>
>> =A0 =A0 =A0 switch (hdev->discovery.type) {
>> =A0 =A0 =A0 case DISCOV_TYPE_BREDR:
>> - =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_do_inquiry(hdev, INQUIRY_LEN_BREDR=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (lmp_bredr_capable(hdev))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_do_inquiry(hdev, I=
NQUIRY_LEN_BREDR);
>> + =A0 =A0 =A0 =A0 =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOTSUPP;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>>
>> =A0 =A0 =A0 case DISCOV_TYPE_LE:
>> - =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCA=
N_INT,
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (lmp_host_le_capable(hdev))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_le_scan(hdev, LE_S=
CAN_TYPE, LE_SCAN_INT,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
>> + =A0 =A0 =A0 =A0 =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOTSUPP;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>>
>> =A0 =A0 =A0 case DISCOV_TYPE_INTERLEAVED:
>
> so what about interleaved. Should that actually suceed if LE is not
> enabled?

For interleaved, we already check this capabilities.

According to the discussion in "[PATCH] Don't set LE flags in
mgmt_start_discovery", if an interleaved Start Discovery command is
issued but the device is not dual mode, we perform a regular BR/EDR
or LE-only discovery (according to device capabilities) instead of
returning error.

BR,

Andre

2012-02-23 21:14:25

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

Hi Andre,

> This patch add an extra check for BR/EDR and LE-Only discovery.
> This way, we are able to return error immediately if the discovery
> type requested is not supported by the device.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> net/bluetooth/mgmt.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 93f2c13..de40918 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2552,12 +2552,18 @@ static int start_discovery(struct sock *sk, u16 index,
>
> switch (hdev->discovery.type) {
> case DISCOV_TYPE_BREDR:
> - err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
> + if (lmp_bredr_capable(hdev))
> + err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
> + else
> + err = -ENOTSUPP;
> break;
>
> case DISCOV_TYPE_LE:
> - err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
> + if (lmp_host_le_capable(hdev))
> + err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
> LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
> + else
> + err = -ENOTSUPP;
> break;
>
> case DISCOV_TYPE_INTERLEAVED:

so what about interleaved. Should that actually suceed if LE is not
enabled?

Regards

Marcel



2012-03-01 15:05:35

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

Hi Andre,

On Thu, Feb 23, 2012, Andre Guedes wrote:
> This patch add an extra check for BR/EDR and LE-Only discovery.
> This way, we are able to return error immediately if the discovery
> type requested is not supported by the device.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> net/bluetooth/mgmt.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)

Applied. Thanks.

Johan