With this patch it is possible to control discovery interleaved
timeout value from debugfs. Default value is 5120 ms.
It is useful to control it in case of automated testaces where bredrle
controler is doing discovery and it is waiting for inquiry results. With
this patch we can decrease le scan time and get inquiry results faster.
It might be also useful in other test cases.
Signed-off-by: Lukasz Rymanowski <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_core.c | 6 ++++++
net/bluetooth/mgmt.c | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5f8bc05..2d63ade 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -194,6 +194,7 @@ struct hci_dev {
__u16 le_scan_window;
__u16 le_conn_min_interval;
__u16 le_conn_max_interval;
+ __u32 discov_interl_timeout;
__u8 ssp_debug_mode;
__u16 devid_source;
@@ -1205,7 +1206,6 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
#define DISCOV_LE_SCAN_WIN 0x12
#define DISCOV_LE_SCAN_INT 0x12
#define DISCOV_LE_TIMEOUT msecs_to_jiffies(10240)
-#define DISCOV_INTERLEAVED_TIMEOUT msecs_to_jiffies(5120)
#define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
#define DISCOV_BREDR_INQUIRY_LEN 0x08
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1c6ffaa..b2139f1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -61,6 +61,9 @@ static void hci_notify(struct hci_dev *hdev, int event)
/* ---- HCI debugfs entries ---- */
+/* Discovery interleaved timeout */
+static u16 discov_timeout_ms = 5120;
+
static ssize_t dut_mode_read(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -1828,6 +1831,8 @@ static int __hci_init(struct hci_dev *hdev)
&lowpan_debugfs_fops);
debugfs_create_file("le_auto_conn", 0644, hdev->debugfs, hdev,
&le_auto_conn_fops);
+ debugfs_create_u16("discov_interleaved_timeout", 0644,
+ bt_debugfs, &discov_timeout_ms);
}
return 0;
@@ -3792,6 +3797,7 @@ struct hci_dev *hci_alloc_dev(void)
hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
+ hdev->discov_interl_timeout = msecs_to_jiffies(discov_timeout_ms);
mutex_init(&hdev->lock);
mutex_init(&hdev->req_lock);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d2d4e0d..3eedb87 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3372,7 +3372,7 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status)
case DISCOV_TYPE_INTERLEAVED:
queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
- DISCOV_INTERLEAVED_TIMEOUT);
+ hdev->discov_interl_timeout);
break;
case DISCOV_TYPE_BREDR:
--
1.8.4
Hi Marcel,
On 24 March 2014 03:30, Marcel Holtmann <[email protected]> wrote:
> Hi Lukasz,
>
>>>> With this patch it is possible to control discovery interleaved
>>>> timeout value from debugfs. Default value is 5120 ms.
>>>>
>>>> It is useful to control it in case of automated testaces where bredrle
>>>> controler is doing discovery and it is waiting for inquiry results. Wi=
th
>>>> this patch we can decrease le scan time and get inquiry results faster=
.
>>>>
>>>> It might be also useful in other test cases.
>>>>
>>>> Signed-off-by: Lukasz Rymanowski <[email protected]>
>>>> ---
>>>> include/net/bluetooth/hci_core.h | 2 +-
>>>> net/bluetooth/hci_core.c | 6 ++++++
>>>> net/bluetooth/mgmt.c | 2 +-
>>>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/=
hci_core.h
>>>> index 5f8bc05..2d63ade 100644
>>>> --- a/include/net/bluetooth/hci_core.h
>>>> +++ b/include/net/bluetooth/hci_core.h
>>>> @@ -194,6 +194,7 @@ struct hci_dev {
>>>> __u16 le_scan_window;
>>>> __u16 le_conn_min_interval;
>>>> __u16 le_conn_max_interval;
>>>> + __u32 discov_interl_timeout;
>>>> __u8 ssp_debug_mode;
>>>>
>>>> __u16 devid_source;
>>>> @@ -1205,7 +1206,6 @@ void hci_sock_dev_event(struct hci_dev *hdev, in=
t event);
>>>> #define DISCOV_LE_SCAN_WIN 0x12
>>>> #define DISCOV_LE_SCAN_INT 0x12
>>>> #define DISCOV_LE_TIMEOUT msecs_to_jiffies(10240)
>>>> -#define DISCOV_INTERLEAVED_TIMEOUT msecs_to_jiffies(5120)
>>>> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
>>>> #define DISCOV_BREDR_INQUIRY_LEN 0x08
>>>>
>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>> index 1c6ffaa..b2139f1 100644
>>>> --- a/net/bluetooth/hci_core.c
>>>> +++ b/net/bluetooth/hci_core.c
>>>> @@ -61,6 +61,9 @@ static void hci_notify(struct hci_dev *hdev, int eve=
nt)
>>>>
>>>> /* ---- HCI debugfs entries ---- */
>>>>
>>>> +/* Discovery interleaved timeout */
>>>> +static u16 discov_timeout_ms =3D 5120;
>>>> +
>>>> static ssize_t dut_mode_read(struct file *file, char __user *user_buf,
>>>> size_t count, loff_t *ppos)
>>>> {
>>>> @@ -1828,6 +1831,8 @@ static int __hci_init(struct hci_dev *hdev)
>>>> &lowpan_debugfs_fops);
>>>> debugfs_create_file("le_auto_conn", 0644, hdev->debugfs, =
hdev,
>>>> &le_auto_conn_fops);
>>>> + debugfs_create_u16("discov_interleaved_timeout", 0644,
>>>> + bt_debugfs, &discov_timeout_ms);
>>>
>>> please attach a second HCI controller to your system and tell me what h=
appens.
>>>
>> Well I'm aware that first controller gets default value unless you
>> detach and attach back after this value is chenged. So yes, you might
>> end with different interleaved timeouts for two controllers. But this
>> is debug stuff so I though it is ok.
>
> this is the init routine of the individual controller. And you are regist=
ering on bt_debugfs which means that once you attach a second controller, y=
ou register the same debugfs entry twice. I am pretty certain the kernel wi=
ll complain about this. So no matter what this is wrong.
Oh that what you mean. Probably you are right, I need to check with
two le capable controllers.
>
>> I was considering to do this value per hdev but case we want to cover
>> here is xxx_tester on vhci and we just need to have some way to set
>> this interleave value before vhci is allocated.
>
> Why? Set it before you issue Start Discovery command. Why does it need to=
be set when the controller gets allocated?
I thought rather to set this debugfs before testing than manipulate it
from test body of e.g. android_tester where we have this issue.
>
> On a more important detail, why not trigger BR/EDR only or LE only discov=
ery. The mgmt interface certainly supports that.
>
>> Other option would be to make this timeout global and we could easly
>> change it in the runtime. But then:
>> a) do it in mgmt.c where this timeout is used? well IMHO this timeout
>> does not belong to mgmt so I dropped it.
>> b) do it in hci_core.c and expose somehow to mgmt (some get_function
>> or export?) - well did not like it
>>
>> Anyway, If this way is not acceptable then what do you suggest? IMHO
>> if somebody would like to use this debugfs option he will be able to
>> use it correctly.
>
> Either it is per controller or it is global. What is it now. And honestly=
, I do not see the problem that this solves this. Especially since we can t=
rigger a BR/EDR or LE only scan easily.
In android_tester, where you use HAL API, start_discovery always
trigger interleaved discovery unless controller is BR/EDR only. All
the testcases where you want to get inquiry results will last at least
5sec due to this timer.
I talked to Johan about it and he mentioned that debugfs fot this
timeout would be good to have anyway just to have possibility to play
with it (e.g. to adjust this timeout)
>
> Regards
>
> Marcel
>
\Lukasz
Hi Lukasz,
>>> With this patch it is possible to control discovery interleaved
>>> timeout value from debugfs. Default value is 5120 ms.
>>>
>>> It is useful to control it in case of automated testaces where bredrle
>>> controler is doing discovery and it is waiting for inquiry results. With
>>> this patch we can decrease le scan time and get inquiry results faster.
>>>
>>> It might be also useful in other test cases.
>>>
>>> Signed-off-by: Lukasz Rymanowski <[email protected]>
>>> ---
>>> include/net/bluetooth/hci_core.h | 2 +-
>>> net/bluetooth/hci_core.c | 6 ++++++
>>> net/bluetooth/mgmt.c | 2 +-
>>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index 5f8bc05..2d63ade 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -194,6 +194,7 @@ struct hci_dev {
>>> __u16 le_scan_window;
>>> __u16 le_conn_min_interval;
>>> __u16 le_conn_max_interval;
>>> + __u32 discov_interl_timeout;
>>> __u8 ssp_debug_mode;
>>>
>>> __u16 devid_source;
>>> @@ -1205,7 +1206,6 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
>>> #define DISCOV_LE_SCAN_WIN 0x12
>>> #define DISCOV_LE_SCAN_INT 0x12
>>> #define DISCOV_LE_TIMEOUT msecs_to_jiffies(10240)
>>> -#define DISCOV_INTERLEAVED_TIMEOUT msecs_to_jiffies(5120)
>>> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
>>> #define DISCOV_BREDR_INQUIRY_LEN 0x08
>>>
>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>> index 1c6ffaa..b2139f1 100644
>>> --- a/net/bluetooth/hci_core.c
>>> +++ b/net/bluetooth/hci_core.c
>>> @@ -61,6 +61,9 @@ static void hci_notify(struct hci_dev *hdev, int event)
>>>
>>> /* ---- HCI debugfs entries ---- */
>>>
>>> +/* Discovery interleaved timeout */
>>> +static u16 discov_timeout_ms = 5120;
>>> +
>>> static ssize_t dut_mode_read(struct file *file, char __user *user_buf,
>>> size_t count, loff_t *ppos)
>>> {
>>> @@ -1828,6 +1831,8 @@ static int __hci_init(struct hci_dev *hdev)
>>> &lowpan_debugfs_fops);
>>> debugfs_create_file("le_auto_conn", 0644, hdev->debugfs, hdev,
>>> &le_auto_conn_fops);
>>> + debugfs_create_u16("discov_interleaved_timeout", 0644,
>>> + bt_debugfs, &discov_timeout_ms);
>>
>> please attach a second HCI controller to your system and tell me what happens.
>>
> Well I'm aware that first controller gets default value unless you
> detach and attach back after this value is chenged. So yes, you might
> end with different interleaved timeouts for two controllers. But this
> is debug stuff so I though it is ok.
this is the init routine of the individual controller. And you are registering on bt_debugfs which means that once you attach a second controller, you register the same debugfs entry twice. I am pretty certain the kernel will complain about this. So no matter what this is wrong.
> I was considering to do this value per hdev but case we want to cover
> here is xxx_tester on vhci and we just need to have some way to set
> this interleave value before vhci is allocated.
Why? Set it before you issue Start Discovery command. Why does it need to be set when the controller gets allocated?
On a more important detail, why not trigger BR/EDR only or LE only discovery. The mgmt interface certainly supports that.
> Other option would be to make this timeout global and we could easly
> change it in the runtime. But then:
> a) do it in mgmt.c where this timeout is used? well IMHO this timeout
> does not belong to mgmt so I dropped it.
> b) do it in hci_core.c and expose somehow to mgmt (some get_function
> or export?) - well did not like it
>
> Anyway, If this way is not acceptable then what do you suggest? IMHO
> if somebody would like to use this debugfs option he will be able to
> use it correctly.
Either it is per controller or it is global. What is it now. And honestly, I do not see the problem that this solves this. Especially since we can trigger a BR/EDR or LE only scan easily.
Regards
Marcel
Hi Marcel,
On 23 March 2014 23:13, Marcel Holtmann <[email protected]> wrote:
> Hi Lukasz,
>
>> With this patch it is possible to control discovery interleaved
>> timeout value from debugfs. Default value is 5120 ms.
>>
>> It is useful to control it in case of automated testaces where bredrle
>> controler is doing discovery and it is waiting for inquiry results. With
>> this patch we can decrease le scan time and get inquiry results faster.
>>
>> It might be also useful in other test cases.
>>
>> Signed-off-by: Lukasz Rymanowski <[email protected]>
>> ---
>> include/net/bluetooth/hci_core.h | 2 +-
>> net/bluetooth/hci_core.c | 6 ++++++
>> net/bluetooth/mgmt.c | 2 +-
>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 5f8bc05..2d63ade 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -194,6 +194,7 @@ struct hci_dev {
>> __u16 le_scan_window;
>> __u16 le_conn_min_interval;
>> __u16 le_conn_max_interval;
>> + __u32 discov_interl_timeout;
>> __u8 ssp_debug_mode;
>>
>> __u16 devid_source;
>> @@ -1205,7 +1206,6 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
>> #define DISCOV_LE_SCAN_WIN 0x12
>> #define DISCOV_LE_SCAN_INT 0x12
>> #define DISCOV_LE_TIMEOUT msecs_to_jiffies(10240)
>> -#define DISCOV_INTERLEAVED_TIMEOUT msecs_to_jiffies(5120)
>> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
>> #define DISCOV_BREDR_INQUIRY_LEN 0x08
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 1c6ffaa..b2139f1 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -61,6 +61,9 @@ static void hci_notify(struct hci_dev *hdev, int event)
>>
>> /* ---- HCI debugfs entries ---- */
>>
>> +/* Discovery interleaved timeout */
>> +static u16 discov_timeout_ms = 5120;
>> +
>> static ssize_t dut_mode_read(struct file *file, char __user *user_buf,
>> size_t count, loff_t *ppos)
>> {
>> @@ -1828,6 +1831,8 @@ static int __hci_init(struct hci_dev *hdev)
>> &lowpan_debugfs_fops);
>> debugfs_create_file("le_auto_conn", 0644, hdev->debugfs, hdev,
>> &le_auto_conn_fops);
>> + debugfs_create_u16("discov_interleaved_timeout", 0644,
>> + bt_debugfs, &discov_timeout_ms);
>
> please attach a second HCI controller to your system and tell me what happens.
>
Well I'm aware that first controller gets default value unless you
detach and attach back after this value is chenged. So yes, you might
end with different interleaved timeouts for two controllers. But this
is debug stuff so I though it is ok.
I was considering to do this value per hdev but case we want to cover
here is xxx_tester on vhci and we just need to have some way to set
this interleave value before vhci is allocated.
Other option would be to make this timeout global and we could easly
change it in the runtime. But then:
a) do it in mgmt.c where this timeout is used? well IMHO this timeout
does not belong to mgmt so I dropped it.
b) do it in hci_core.c and expose somehow to mgmt (some get_function
or export?) - well did not like it
Anyway, If this way is not acceptable then what do you suggest? IMHO
if somebody would like to use this debugfs option he will be able to
use it correctly.
> Regards
>
> Marcel
>
\Lukasz
Hi Marcel,
On 23 March 2014 23:13, Marcel Holtmann <[email protected]> wrote:
> Hi Lukasz,
>
> > With this patch it is possible to control discovery interleaved
> > timeout value from debugfs. Default value is 5120 ms.
> >
> > It is useful to control it in case of automated testaces where bredrle
> > controler is doing discovery and it is waiting for inquiry results. With
> > this patch we can decrease le scan time and get inquiry results faster.
> >
> > It might be also useful in other test cases.
> >
> > Signed-off-by: Lukasz Rymanowski <[email protected]>
> > ---
> > include/net/bluetooth/hci_core.h | 2 +-
> > net/bluetooth/hci_core.c | 6 ++++++
> > net/bluetooth/mgmt.c | 2 +-
> > 3 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h
> b/include/net/bluetooth/hci_core.h
> > index 5f8bc05..2d63ade 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -194,6 +194,7 @@ struct hci_dev {
> > __u16 le_scan_window;
> > __u16 le_conn_min_interval;
> > __u16 le_conn_max_interval;
> > + __u32 discov_interl_timeout;
> > __u8 ssp_debug_mode;
> >
> > __u16 devid_source;
> > @@ -1205,7 +1206,6 @@ void hci_sock_dev_event(struct hci_dev *hdev, int
> event);
> > #define DISCOV_LE_SCAN_WIN 0x12
> > #define DISCOV_LE_SCAN_INT 0x12
> > #define DISCOV_LE_TIMEOUT msecs_to_jiffies(10240)
> > -#define DISCOV_INTERLEAVED_TIMEOUT msecs_to_jiffies(5120)
> > #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
> > #define DISCOV_BREDR_INQUIRY_LEN 0x08
> >
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 1c6ffaa..b2139f1 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -61,6 +61,9 @@ static void hci_notify(struct hci_dev *hdev, int event)
> >
> > /* ---- HCI debugfs entries ---- */
> >
> > +/* Discovery interleaved timeout */
> > +static u16 discov_timeout_ms = 5120;
> > +
> > static ssize_t dut_mode_read(struct file *file, char __user *user_buf,
> > size_t count, loff_t *ppos)
> > {
> > @@ -1828,6 +1831,8 @@ static int __hci_init(struct hci_dev *hdev)
> > &lowpan_debugfs_fops);
> > debugfs_create_file("le_auto_conn", 0644, hdev->debugfs,
> hdev,
> > &le_auto_conn_fops);
> > + debugfs_create_u16("discov_interleaved_timeout", 0644,
> > + bt_debugfs, &discov_timeout_ms);
>
> please attach a second HCI controller to your system and tell me what
> happens.
>
> Well I'm aware that first controller gets default value unless you detach
and attach back after this value is chenged. So yes, you might end with
different interleaved timeouts for two controllers. But this is debug
stuff so I though it is ok.
I was considering to do this value per hdev but case we want to cover here
is xxx_tester on vhci and we just need to have some way to set this
interleave value before vhci is allocated.
Other option would be to make this timeout global and we could easly change
it in the runtime. But then:
a) do it in mgmt.c where this timeout is used? well IMHO this timeout does
not belong to mgmt so I dropped it.
b) do it in hci_core.c and expose somehow to mgmt (some get_function or
export?) - well did not like it
Anyway, If this way is not acceptable then what do you suggest? IMHO if
somebody would like to use this debugfs option he will be able to use it
correctly.
Regards
>
> Marcel
>
> \Lukasz
Hi Lukasz,
> With this patch it is possible to control discovery interleaved
> timeout value from debugfs. Default value is 5120 ms.
>
> It is useful to control it in case of automated testaces where bredrle
> controler is doing discovery and it is waiting for inquiry results. With
> this patch we can decrease le scan time and get inquiry results faster.
>
> It might be also useful in other test cases.
>
> Signed-off-by: Lukasz Rymanowski <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> net/bluetooth/hci_core.c | 6 ++++++
> net/bluetooth/mgmt.c | 2 +-
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 5f8bc05..2d63ade 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -194,6 +194,7 @@ struct hci_dev {
> __u16 le_scan_window;
> __u16 le_conn_min_interval;
> __u16 le_conn_max_interval;
> + __u32 discov_interl_timeout;
> __u8 ssp_debug_mode;
>
> __u16 devid_source;
> @@ -1205,7 +1206,6 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
> #define DISCOV_LE_SCAN_WIN 0x12
> #define DISCOV_LE_SCAN_INT 0x12
> #define DISCOV_LE_TIMEOUT msecs_to_jiffies(10240)
> -#define DISCOV_INTERLEAVED_TIMEOUT msecs_to_jiffies(5120)
> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
> #define DISCOV_BREDR_INQUIRY_LEN 0x08
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 1c6ffaa..b2139f1 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -61,6 +61,9 @@ static void hci_notify(struct hci_dev *hdev, int event)
>
> /* ---- HCI debugfs entries ---- */
>
> +/* Discovery interleaved timeout */
> +static u16 discov_timeout_ms = 5120;
> +
> static ssize_t dut_mode_read(struct file *file, char __user *user_buf,
> size_t count, loff_t *ppos)
> {
> @@ -1828,6 +1831,8 @@ static int __hci_init(struct hci_dev *hdev)
> &lowpan_debugfs_fops);
> debugfs_create_file("le_auto_conn", 0644, hdev->debugfs, hdev,
> &le_auto_conn_fops);
> + debugfs_create_u16("discov_interleaved_timeout", 0644,
> + bt_debugfs, &discov_timeout_ms);
please attach a second HCI controller to your system and tell me what happens.
Regards
Marcel