Changes made to add support for fast advertising interval as per core 4.1 specification, section 9.3.11.2. A peripheral device enetering any of the following GAP modes and sending either non-connectable advertising events or scannable undirected advertising events should use adv_fast_interval2(100ms - 150ms) for adv_fast_period(30s). - Non-Discoverable Mode - Non-Connectable Mode - Limited Discoverable Mode - General Discoverable Mode Signed-off-by: Bharat Bhusan Panda <bharat.b.panda@intel.com> --- include/net/bluetooth/hci_core.h | 2 ++ net/bluetooth/hci_request.c | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index b619a19..2fe908c 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1456,6 +1456,8 @@ struct hci_mgmt_chan { #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04 #define DISCOV_BREDR_INQUIRY_LEN 0x08 #define DISCOV_LE_RESTART_DELAY msecs_to_jiffies(200) /* msec */ +#define DISCOV_LE_FAST_ADV_INT_MIN 100 /* msec */ +#define DISCOV_LE_FAST_ADV_INT_MAX 150 /* msec */ void mgmt_fill_version_info(void *ver); int mgmt_new_settings(struct hci_dev *hdev); diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c index 66c0781..5cf3ef9 100644 --- a/net/bluetooth/hci_request.c +++ b/net/bluetooth/hci_request.c @@ -1005,6 +1005,28 @@ void __hci_req_enable_advertising(struct hci_request *req) else cp.type = LE_ADV_NONCONN_IND; + /* As per core 4.1 spec, section 9.3.11.2: A peripheral device + * enetering any of the following GAP modes and sending either + * non-connectable advertising events or scannable undirected + * advertising events should use adv_fast_interval2(100ms - 150ms) + * for adv_fast_period(30s). + * + * - Non-Discoverable Mode + * - Non-Connectable Mode + * - Limited Discoverable Mode + * - General Discoverable Mode + */ + if ((cp.type == LE_ADV_NONCONN_IND) || (cp.type = LE_ADV_SCAN_IND)) { + if ((hci_dev_test_flag(hdev, HCI_DISCOVERABLE)) || + (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE)) || + (hdev->discovery.limited == true)) { + cp.min_interval = + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MIN); + cp.max_interval = + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MAX); + } + } + cp.own_address_type = own_addr_type; cp.channel_map = hdev->le_adv_channel_map; -- 1.9.1
Hi Bharat, > Changes made to add support for fast advertising interval as per > core 4.1 specification, section 9.3.11.2. > > A peripheral device enetering any of the following GAP modes and > sending either non-connectable advertising events or scannable undirected > advertising events should use adv_fast_interval2(100ms - 150ms) > for adv_fast_period(30s). > - Non-Discoverable Mode > - Non-Connectable Mode > - Limited Discoverable Mode > - General Discoverable Mode can we also get a patch documenting this in mgmt-api.txt in bluez.git. > > Signed-off-by: Bharat Bhusan Panda <bharat.b.panda@intel.com> > --- > include/net/bluetooth/hci_core.h | 2 ++ > net/bluetooth/hci_request.c | 22 ++++++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index b619a19..2fe908c 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -1456,6 +1456,8 @@ struct hci_mgmt_chan { > #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04 > #define DISCOV_BREDR_INQUIRY_LEN 0x08 > #define DISCOV_LE_RESTART_DELAY msecs_to_jiffies(200) /* msec */ > +#define DISCOV_LE_FAST_ADV_INT_MIN 100 /* msec */ > +#define DISCOV_LE_FAST_ADV_INT_MAX 150 /* msec */ > > void mgmt_fill_version_info(void *ver); > int mgmt_new_settings(struct hci_dev *hdev); > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c > index 66c0781..5cf3ef9 100644 > --- a/net/bluetooth/hci_request.c > +++ b/net/bluetooth/hci_request.c > @@ -1005,6 +1005,28 @@ void __hci_req_enable_advertising(struct hci_request *req) > else > cp.type = LE_ADV_NONCONN_IND; > > + /* As per core 4.1 spec, section 9.3.11.2: A peripheral device > + * enetering any of the following GAP modes and sending either Fix the typo please. > + * non-connectable advertising events or scannable undirected > + * advertising events should use adv_fast_interval2(100ms - 150ms) > + * for adv_fast_period(30s). > + * > + * - Non-Discoverable Mode > + * - Non-Connectable Mode > + * - Limited Discoverable Mode > + * - General Discoverable Mode > + */ > + if ((cp.type == LE_ADV_NONCONN_IND) || (cp.type = LE_ADV_SCAN_IND)) { This extra (x == y) parentheses are not needed. > + if ((hci_dev_test_flag(hdev, HCI_DISCOVERABLE)) || > + (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE)) || > + (hdev->discovery.limited == true)) { And this is wrongly aligned. In addition, you need a bit more comment text above explains on what are the conditions. > + cp.min_interval = > + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MIN); > + cp.max_interval = > + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MAX); > + } > + } > + > cp.own_address_type = own_addr_type; > cp.channel_map = hdev->le_adv_channel_map; Regards Marcel
Hi Marcel, Bharat, On Thu, Sep 27, 2018 at 1:57 PM, Marcel Holtmann <marcel@holtmann.org> wrote: > Hi Bharat, > >> Changes made to add support for fast advertising interval as per >> core 4.1 specification, section 9.3.11.2. >> >> A peripheral device enetering any of the following GAP modes and >> sending either non-connectable advertising events or scannable undirected >> advertising events should use adv_fast_interval2(100ms - 150ms) >> for adv_fast_period(30s). >> - Non-Discoverable Mode >> - Non-Connectable Mode >> - Limited Discoverable Mode >> - General Discoverable Mode > > can we also get a patch documenting this in mgmt-api.txt in bluez.git. > >> >> Signed-off-by: Bharat Bhusan Panda <bharat.b.panda@intel.com> >> --- >> include/net/bluetooth/hci_core.h | 2 ++ >> net/bluetooth/hci_request.c | 22 ++++++++++++++++++++++ >> 2 files changed, 24 insertions(+) >> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h >> index b619a19..2fe908c 100644 >> --- a/include/net/bluetooth/hci_core.h >> +++ b/include/net/bluetooth/hci_core.h >> @@ -1456,6 +1456,8 @@ struct hci_mgmt_chan { >> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04 >> #define DISCOV_BREDR_INQUIRY_LEN 0x08 >> #define DISCOV_LE_RESTART_DELAY msecs_to_jiffies(200) /* msec */ >> +#define DISCOV_LE_FAST_ADV_INT_MIN 100 /* msec */ >> +#define DISCOV_LE_FAST_ADV_INT_MAX 150 /* msec */ >> >> void mgmt_fill_version_info(void *ver); >> int mgmt_new_settings(struct hci_dev *hdev); >> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c >> index 66c0781..5cf3ef9 100644 >> --- a/net/bluetooth/hci_request.c >> +++ b/net/bluetooth/hci_request.c >> @@ -1005,6 +1005,28 @@ void __hci_req_enable_advertising(struct hci_request *req) >> else >> cp.type = LE_ADV_NONCONN_IND; >> >> + /* As per core 4.1 spec, section 9.3.11.2: A peripheral device >> + * enetering any of the following GAP modes and sending either > > Fix the typo please. > >> + * non-connectable advertising events or scannable undirected >> + * advertising events should use adv_fast_interval2(100ms - 150ms) >> + * for adv_fast_period(30s). >> + * >> + * - Non-Discoverable Mode >> + * - Non-Connectable Mode >> + * - Limited Discoverable Mode >> + * - General Discoverable Mode >> + */ >> + if ((cp.type == LE_ADV_NONCONN_IND) || (cp.type = LE_ADV_SCAN_IND)) { > > This extra (x == y) parentheses are not needed. Btw the second part of the expression seem wrong, it is assigning not comparing. > >> + if ((hci_dev_test_flag(hdev, HCI_DISCOVERABLE)) || >> + (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE)) || >> + (hdev->discovery.limited == true)) { > > And this is wrongly aligned. In addition, you need a bit more comment text above explains on what are the conditions. > >> + cp.min_interval = >> + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MIN); >> + cp.max_interval = >> + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MAX); >> + } >> + } >> + >> cp.own_address_type = own_addr_type; >> cp.channel_map = hdev->le_adv_channel_map; > > Regards > > Marcel > -- Luiz Augusto von Dentz
Hi Marcel, Luiz, > -----Original Message----- > From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com] > Sent: Thursday, September 27, 2018 4:47 PM > To: Marcel Holtmann <marcel@holtmann.org> > Cc: Panda, Bharat B <bharat.b.panda@intel.com>; linux- > bluetooth@vger.kernel.org > Subject: Re: [PATCH] support fast advertising interval > > Hi Marcel, Bharat, > > On Thu, Sep 27, 2018 at 1:57 PM, Marcel Holtmann <marcel@holtmann.org> > wrote: > > Hi Bharat, > > > >> Changes made to add support for fast advertising interval as per core > >> 4.1 specification, section 9.3.11.2. > >> > >> A peripheral device enetering any of the following GAP modes and > >> sending either non-connectable advertising events or scannable > >> undirected advertising events should use adv_fast_interval2(100ms - > >> 150ms) for adv_fast_period(30s). > >> - Non-Discoverable Mode > >> - Non-Connectable Mode > >> - Limited Discoverable Mode > >> - General Discoverable Mode > > > > can we also get a patch documenting this in mgmt-api.txt in bluez.git. I will send a separate patch for same following v2 for current patch. > > > >> > >> Signed-off-by: Bharat Bhusan Panda <bharat.b.panda@intel.com> > >> --- > >> include/net/bluetooth/hci_core.h | 2 ++ > >> net/bluetooth/hci_request.c | 22 ++++++++++++++++++++++ > >> 2 files changed, 24 insertions(+) > >> > >> diff --git a/include/net/bluetooth/hci_core.h > >> b/include/net/bluetooth/hci_core.h > >> index b619a19..2fe908c 100644 > >> --- a/include/net/bluetooth/hci_core.h > >> +++ b/include/net/bluetooth/hci_core.h > >> @@ -1456,6 +1456,8 @@ struct hci_mgmt_chan { > >> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04 > >> #define DISCOV_BREDR_INQUIRY_LEN 0x08 > >> #define DISCOV_LE_RESTART_DELAY msecs_to_jiffies(200) /* msec > */ > >> +#define DISCOV_LE_FAST_ADV_INT_MIN 100 /* msec */ > >> +#define DISCOV_LE_FAST_ADV_INT_MAX 150 /* msec */ > >> > >> void mgmt_fill_version_info(void *ver); int mgmt_new_settings(struct > >> hci_dev *hdev); diff --git a/net/bluetooth/hci_request.c > >> b/net/bluetooth/hci_request.c index 66c0781..5cf3ef9 100644 > >> --- a/net/bluetooth/hci_request.c > >> +++ b/net/bluetooth/hci_request.c > >> @@ -1005,6 +1005,28 @@ void __hci_req_enable_advertising(struct > hci_request *req) > >> else > >> cp.type = LE_ADV_NONCONN_IND; > >> > >> + /* As per core 4.1 spec, section 9.3.11.2: A peripheral device > >> + * enetering any of the following GAP modes and sending either > > > > Fix the typo please. > > > >> + * non-connectable advertising events or scannable undirected > >> + * advertising events should use adv_fast_interval2(100ms - 150ms) > >> + * for adv_fast_period(30s). > >> + * > >> + * - Non-Discoverable Mode > >> + * - Non-Connectable Mode > >> + * - Limited Discoverable Mode > >> + * - General Discoverable Mode > >> + */ > >> + if ((cp.type == LE_ADV_NONCONN_IND) || (cp.type = > >> + LE_ADV_SCAN_IND)) { > > > > This extra (x == y) parentheses are not needed. > > Btw the second part of the expression seem wrong, it is assigning not > comparing. Yes, the assignment was always making the condition true. I have fixed the typo and retested the patch working. Patch v2 will be followed with the fix. > > > > >> + if ((hci_dev_test_flag(hdev, HCI_DISCOVERABLE)) || > >> + (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE)) || > >> + (hdev->discovery.limited == true)) { > > > > And this is wrongly aligned. In addition, you need a bit more comment text > above explains on what are the conditions. > > > >> + cp.min_interval = > >> + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MIN); > >> + cp.max_interval = > >> + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MAX); > >> + } > >> + } > >> + > >> cp.own_address_type = own_addr_type; > >> cp.channel_map = hdev->le_adv_channel_map; > > > > Regards > > > > Marcel > > > > > > -- > Luiz Augusto von Dentz Regards, Bharat
Hi Marcel,
I have submitted the patch with review comments incorporated along with the write-up in mgmt.-api for fast adv interval.
Please review the same.
> -----Original Message-----
> From: Panda, Bharat B
> Sent: Monday, October 1, 2018 2:59 PM
> To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>; Marcel Holtmann
> <marcel@holtmann.org>
> Cc: linux-bluetooth@vger.kernel.org
> Subject: RE: [PATCH] support fast advertising interval
>
> Hi Marcel, Luiz,
>
> > -----Original Message-----
> > From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> > Sent: Thursday, September 27, 2018 4:47 PM
> > To: Marcel Holtmann <marcel@holtmann.org>
> > Cc: Panda, Bharat B <bharat.b.panda@intel.com>; linux-
> > bluetooth@vger.kernel.org
> > Subject: Re: [PATCH] support fast advertising interval
> >
> > Hi Marcel, Bharat,
> >
> > On Thu, Sep 27, 2018 at 1:57 PM, Marcel Holtmann <marcel@holtmann.org>
> > wrote:
> > > Hi Bharat,
> > >
> > >> Changes made to add support for fast advertising interval as per
> > >> core
> > >> 4.1 specification, section 9.3.11.2.
> > >>
> > >> A peripheral device enetering any of the following GAP modes and
> > >> sending either non-connectable advertising events or scannable
> > >> undirected advertising events should use adv_fast_interval2(100ms -
> > >> 150ms) for adv_fast_period(30s).
> > >> - Non-Discoverable Mode
> > >> - Non-Connectable Mode
> > >> - Limited Discoverable Mode
> > >> - General Discoverable Mode
> > >
> > > can we also get a patch documenting this in mgmt-api.txt in bluez.git.
> I will send a separate patch for same following v2 for current patch.
> > >
> > >>
> > >> Signed-off-by: Bharat Bhusan Panda <bharat.b.panda@intel.com>
> > >> ---
> > >> include/net/bluetooth/hci_core.h | 2 ++
> > >> net/bluetooth/hci_request.c | 22 ++++++++++++++++++++++
> > >> 2 files changed, 24 insertions(+)
> > >>
> > >> diff --git a/include/net/bluetooth/hci_core.h
> > >> b/include/net/bluetooth/hci_core.h
> > >> index b619a19..2fe908c 100644
> > >> --- a/include/net/bluetooth/hci_core.h
> > >> +++ b/include/net/bluetooth/hci_core.h
> > >> @@ -1456,6 +1456,8 @@ struct hci_mgmt_chan {
> > >> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
> > >> #define DISCOV_BREDR_INQUIRY_LEN 0x08
> > >> #define DISCOV_LE_RESTART_DELAY msecs_to_jiffies(200) /*
> msec
> > */
> > >> +#define DISCOV_LE_FAST_ADV_INT_MIN 100 /* msec */
> > >> +#define DISCOV_LE_FAST_ADV_INT_MAX 150 /* msec */
> > >>
> > >> void mgmt_fill_version_info(void *ver); int
> > >> mgmt_new_settings(struct hci_dev *hdev); diff --git
> > >> a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c index
> > >> 66c0781..5cf3ef9 100644
> > >> --- a/net/bluetooth/hci_request.c
> > >> +++ b/net/bluetooth/hci_request.c
> > >> @@ -1005,6 +1005,28 @@ void __hci_req_enable_advertising(struct
> > hci_request *req)
> > >> else
> > >> cp.type = LE_ADV_NONCONN_IND;
> > >>
> > >> + /* As per core 4.1 spec, section 9.3.11.2: A peripheral device
> > >> + * enetering any of the following GAP modes and sending
> > >> + either
> > >
> > > Fix the typo please.
> > >
> > >> + * non-connectable advertising events or scannable undirected
> > >> + * advertising events should use adv_fast_interval2(100ms - 150ms)
> > >> + * for adv_fast_period(30s).
> > >> + *
> > >> + * - Non-Discoverable Mode
> > >> + * - Non-Connectable Mode
> > >> + * - Limited Discoverable Mode
> > >> + * - General Discoverable Mode
> > >> + */
> > >> + if ((cp.type == LE_ADV_NONCONN_IND) || (cp.type =
> > >> + LE_ADV_SCAN_IND)) {
> > >
> > > This extra (x == y) parentheses are not needed.
> >
> > Btw the second part of the expression seem wrong, it is assigning not
> > comparing.
> Yes, the assignment was always making the condition true. I have fixed the typo
> and retested the patch working.
> Patch v2 will be followed with the fix.
> >
> > >
> > >> + if ((hci_dev_test_flag(hdev, HCI_DISCOVERABLE)) ||
> > >> + (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE)) ||
> > >> + (hdev->discovery.limited == true)) {
> > >
> > > And this is wrongly aligned. In addition, you need a bit more
> > > comment text
> > above explains on what are the conditions.
> > >
> > >> + cp.min_interval =
> > >> + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MIN);
> > >> + cp.max_interval =
> > >> + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MAX);
> > >> + }
> > >> + }
> > >> +
> > >> cp.own_address_type = own_addr_type;
> > >> cp.channel_map = hdev->le_adv_channel_map;
> > >
> > > Regards
> > >
> > > Marcel
> > >
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
> Regards,
> Bharat
Regards,
Bharat
Hi Bharat, > Changes made to add support for fast advertising interval as per > core 4.1 specification, section 9.3.11.2. > > A peripheral device enetering any of the following GAP modes and > sending either non-connectable advertising events or scannable undirected > advertising events should use adv_fast_interval2(100ms - 150ms) > for adv_fast_period(30s). > - Non-Discoverable Mode > - Non-Connectable Mode > - Limited Discoverable Mode > - General Discoverable Mode > > Signed-off-by: Bharat Bhusan Panda <bharat.b.panda@intel.com> > --- > include/net/bluetooth/hci_core.h | 2 ++ > net/bluetooth/hci_request.c | 22 ++++++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index b619a19..2fe908c 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -1456,6 +1456,8 @@ struct hci_mgmt_chan { > #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04 > #define DISCOV_BREDR_INQUIRY_LEN 0x08 > #define DISCOV_LE_RESTART_DELAY msecs_to_jiffies(200) /* msec */ > +#define DISCOV_LE_FAST_ADV_INT_MIN 100 /* msec */ > +#define DISCOV_LE_FAST_ADV_INT_MAX 150 /* msec */ > > void mgmt_fill_version_info(void *ver); > int mgmt_new_settings(struct hci_dev *hdev); > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c > index 66c0781..5cf3ef9 100644 > --- a/net/bluetooth/hci_request.c > +++ b/net/bluetooth/hci_request.c > @@ -1005,6 +1005,28 @@ void __hci_req_enable_advertising(struct hci_request *req) > else > cp.type = LE_ADV_NONCONN_IND; > > + /* As per core 4.1 spec, section 9.3.11.2: A peripheral device > + * enetering any of the following GAP modes and sending either “entering” > + * non-connectable advertising events or scannable undirected > + * advertising events should use adv_fast_interval2(100ms - 150ms) > + * for adv_fast_period(30s). > + * > + * - Non-Discoverable Mode > + * - Non-Connectable Mode > + * - Limited Discoverable Mode > + * - General Discoverable Mode > + */ > + if ((cp.type == LE_ADV_NONCONN_IND) || (cp.type = LE_ADV_SCAN_IND)) { Remove the unneeded (x == x) braces. > + if ((hci_dev_test_flag(hdev, HCI_DISCOVERABLE)) || > + (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE)) || > + (hdev->discovery.limited == true)) { Same here and the indentation is all messed up. In addition of just quoting the spec text, it would be good also document why these conditions are the the correct match. And why hdev->discovery.limited is even in this mix. > + cp.min_interval = > + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MIN); > + cp.max_interval = > + cpu_to_le16(DISCOV_LE_FAST_ADV_INT_MAX); > + } > + } > + Regards Marcel