2020-06-22 02:15:45

by Alain Michaud

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: use configured params for ext adv

When the extended advertisement feature is enabled, a hardcoded min and
max interval of 0x8000 is used. This patch fixes this issue by using
the configured min/max value.

This was validated by setting min/max in main.conf and making sure the
right setting is applied:

< HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen
25 #93 [hci0] 10.953011

Min advertising interval: 181.250 msec (0x0122)
Max advertising interval: 181.250 msec (0x0122)


Signed-off-by: Alain Michaud <[email protected]>
Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
Reviewed-by: Daniel Winkler <[email protected]>

---

Changes in v2:
-fix commit title and typo in description.
-Moved le24 convertion to hci_cpu_to_le24

include/net/bluetooth/hci.h | 8 ++++++++
net/bluetooth/hci_request.c | 6 ++----
2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 16ab6ce87883..1f18f71363e9 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2516,4 +2516,12 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
#define hci_iso_data_len(h) ((h) & 0x3fff)
#define hci_iso_data_flags(h) ((h) >> 14)

+/* le24 support */
+static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3])
+{
+ dst[0] = val & 0xff;
+ dst[1] = (val & 0xff00) >> 8;
+ dst[2] = (val & 0xff0000) >> 16;
+}
+
#endif /* __HCI_H */
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 29decd7e8051..9301c1d54936 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1799,8 +1799,6 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
int err;
struct adv_info *adv_instance;
bool secondary_adv;
- /* In ext adv set param interval is 3 octets */
- const u8 adv_interval[3] = { 0x00, 0x08, 0x00 };

if (instance > 0) {
adv_instance = hci_find_adv_instance(hdev, instance);
@@ -1833,8 +1831,8 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)

memset(&cp, 0, sizeof(cp));

- memcpy(cp.min_interval, adv_interval, sizeof(cp.min_interval));
- memcpy(cp.max_interval, adv_interval, sizeof(cp.max_interval));
+ hci_cpu_to_le24(hdev->le_adv_min_interval, cp.min_interval);
+ hci_cpu_to_le24(hdev->le_adv_max_interval, cp.max_interval);

secondary_adv = (flags & MGMT_ADV_FLAG_SEC_MASK);

--
2.27.0.111.gc72c7da667-goog


2020-06-22 07:08:09

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: use configured params for ext adv

Hi Alain,

> When the extended advertisement feature is enabled, a hardcoded min and
> max interval of 0x8000 is used. This patch fixes this issue by using
> the configured min/max value.
>
> This was validated by setting min/max in main.conf and making sure the
> right setting is applied:
>
> < HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen
> 25 #93 [hci0] 10.953011
> …
> Min advertising interval: 181.250 msec (0x0122)
> Max advertising interval: 181.250 msec (0x0122)
> …
>
> Signed-off-by: Alain Michaud <[email protected]>
> Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
> Reviewed-by: Daniel Winkler <[email protected]>
>
> ---
>
> Changes in v2:
> -fix commit title and typo in description.
> -Moved le24 convertion to hci_cpu_to_le24
>
> include/net/bluetooth/hci.h | 8 ++++++++
> net/bluetooth/hci_request.c | 6 ++----
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 16ab6ce87883..1f18f71363e9 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -2516,4 +2516,12 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
> #define hci_iso_data_len(h) ((h) & 0x3fff)
> #define hci_iso_data_flags(h) ((h) >> 14)
>
> +/* le24 support */
> +static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3])
> +{
> + dst[0] = val & 0xff;
> + dst[1] = (val & 0xff00) >> 8;
> + dst[2] = (val & 0xff0000) >> 16;
> +}
> +
> #endif /* __HCI_H */
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 29decd7e8051..9301c1d54936 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -1799,8 +1799,6 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
> int err;
> struct adv_info *adv_instance;
> bool secondary_adv;
> - /* In ext adv set param interval is 3 octets */
> - const u8 adv_interval[3] = { 0x00, 0x08, 0x00 };
>
> if (instance > 0) {
> adv_instance = hci_find_adv_instance(hdev, instance);
> @@ -1833,8 +1831,8 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
>
> memset(&cp, 0, sizeof(cp));
>
> - memcpy(cp.min_interval, adv_interval, sizeof(cp.min_interval));
> - memcpy(cp.max_interval, adv_interval, sizeof(cp.max_interval));

please add the comment from above here. The le24 might be obvious, but I think it is still a good comment to remind ourselves later.

> + hci_cpu_to_le24(hdev->le_adv_min_interval, cp.min_interval);
> + hci_cpu_to_le24(hdev->le_adv_max_interval, cp.max_interval);

Regards

Marcel