Hi Iulia,
On Tue, Jun 13, 2023 at 7:26 AM Iulia Tanasescu <iulia.tanasescu@xxxxxxx> wrote:
>
> This adds support for creating multiple BIGs. According to
> spec, each BIG shall have an unique handle, and each BIG should
> be associated with a different advertising handle. Otherwise,
> the LE Create BIG command will fail, with error code
> Command Disallowed (for reusing a BIG handle), or
> Unknown Advertising Identifier (for reusing an advertising
> handle).
>
> The btmon snippet below shows an exercise for creating two BIGs
> for the same controller, by opening two isotest instances with
> the following command:
> tools/isotest -i hci0 -s 00:00:00:00:00:00
>
> < HCI Command: LE Create Broadcast Isochronous Group (0x08|0x0068) plen 31
> Handle: 0x00
> Advertising Handle: 0x01
> Number of BIS: 1
> SDU Interval: 10000 us (0x002710)
> Maximum SDU size: 40
> Maximum Latency: 10 ms (0x000a)
> RTN: 0x02
> PHY: LE 2M (0x02)
> Packing: Sequential (0x00)
> Framing: Unframed (0x00)
> Encryption: 0x00
> Broadcast Code: 00000000000000000000000000000000
>
> > HCI Event: Command Status (0x0f) plen 4
> LE Create Broadcast Isochronous Group (0x08|0x0068) ncmd 1
> Status: Success (0x00)
>
> > HCI Event: LE Meta Event (0x3e) plen 21
> LE Broadcast Isochronous Group Complete (0x1b)
> Status: Success (0x00)
> Handle: 0x00
> BIG Synchronization Delay: 912 us (0x000390)
> Transport Latency: 912 us (0x000390)
> PHY: LE 2M (0x02)
> NSE: 3
> BN: 1
> PTO: 1
> IRC: 3
> Maximum PDU: 40
> ISO Interval: 10.00 msec (0x0008)
> Connection Handle #0: 10
>
> < HCI Command: LE Create Broadcast Isochronous Group (0x08|0x0068)
> Handle: 0x01
> Advertising Handle: 0x02
> Number of BIS: 1
> SDU Interval: 10000 us (0x002710)
> Maximum SDU size: 40
> Maximum Latency: 10 ms (0x000a)
> RTN: 0x02
> PHY: LE 2M (0x02)
> Packing: Sequential (0x00)
> Framing: Unframed (0x00)
> Encryption: 0x00
> Broadcast Code: 00000000000000000000000000000000
>
> > HCI Event: Command Status (0x0f) plen 4
> LE Create Broadcast Isochronous Group (0x08|0x0068) ncmd 1
> Status: Success (0x00)
>
> Signed-off-by: Iulia Tanasescu <iulia.tanasescu@xxxxxxx>
> ---
> include/net/bluetooth/hci_core.h | 6 ++---
> net/bluetooth/hci_conn.c | 40 ++++++++++++++------------------
> net/bluetooth/hci_event.c | 30 ++++++++++++++++++++----
> net/bluetooth/hci_sync.c | 9 +++++--
> 4 files changed, 52 insertions(+), 33 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 683666ea210c..b5af9be70771 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -240,6 +240,7 @@ struct adv_info {
> bool enabled;
> bool pending;
> bool periodic;
> + bool per_enabled;
Can't we just reuse the enabled flag or we really need to have 2 to
track the EA and the PA states separately?
> __u8 mesh;
> __u8 instance;
> __u32 flags;
> @@ -1096,8 +1097,7 @@ static inline __u8 hci_conn_lookup_type(struct hci_dev *hdev, __u16 handle)
> }
>
> static inline struct hci_conn *hci_conn_hash_lookup_bis(struct hci_dev *hdev,
> - bdaddr_t *ba,
> - __u8 big, __u8 bis)
> + bdaddr_t *ba, __u8 bis)
> {
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
> @@ -1108,7 +1108,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_bis(struct hci_dev *hdev,
> if (bacmp(&c->dst, ba) || c->type != ISO_LINK)
> continue;
>
> - if (c->iso_qos.bcast.big == big && c->iso_qos.bcast.bis == bis) {
> + if (c->iso_qos.bcast.bis == bis) {
> rcu_read_unlock();
> return c;
> }
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 7d4941e6dbdf..8cd2ef0ab1d0 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -927,9 +927,7 @@ static void bis_cleanup(struct hci_conn *conn)
> /* Check if ISO connection is a BIS and terminate advertising
> * set and BIG if there are no other connections using it.
> */
> - bis = hci_conn_hash_lookup_bis(hdev, BDADDR_ANY,
> - conn->iso_qos.bcast.big,
> - conn->iso_qos.bcast.bis);
> + bis = hci_conn_hash_lookup_big(hdev, conn->iso_qos.bcast.big);
> if (bis)
> return;
>
> @@ -1449,25 +1447,23 @@ static int hci_explicit_conn_params_set(struct hci_dev *hdev,
>
> static int qos_set_big(struct hci_dev *hdev, struct bt_iso_qos *qos)
> {
> - struct iso_list_data data;
> + struct hci_conn *conn;
> + u8 big;
>
> /* Allocate a BIG if not set */
> if (qos->bcast.big == BT_ISO_QOS_BIG_UNSET) {
> - for (data.big = 0x00; data.big < 0xef; data.big++) {
> - data.count = 0;
> - data.bis = 0xff;
> + for (big = 0x00; big < 0xef; big++) {
>
> - hci_conn_hash_list_state(hdev, bis_list, ISO_LINK,
> - BT_BOUND, &data);
> - if (!data.count)
> + conn = hci_conn_hash_lookup_big(hdev, big);
> + if (!conn)
> break;
> }
>
> - if (data.big == 0xef)
> + if (big == 0xef)
> return -EADDRNOTAVAIL;
>
> /* Update BIG */
> - qos->bcast.big = data.big;
> + qos->bcast.big = big;
> }
>
> return 0;
> @@ -1475,28 +1471,27 @@ static int qos_set_big(struct hci_dev *hdev, struct bt_iso_qos *qos)
>
> static int qos_set_bis(struct hci_dev *hdev, struct bt_iso_qos *qos)
> {
> - struct iso_list_data data;
> + struct hci_conn *conn;
> + u8 bis;
>
> /* Allocate BIS if not set */
> if (qos->bcast.bis == BT_ISO_QOS_BIS_UNSET) {
> /* Find an unused adv set to advertise BIS, skip instance 0x00
> * since it is reserved as general purpose set.
> */
> - for (data.bis = 0x01; data.bis < hdev->le_num_of_adv_sets;
> - data.bis++) {
> - data.count = 0;
> + for (bis = 0x01; bis < hdev->le_num_of_adv_sets;
> + bis++) {
>
> - hci_conn_hash_list_state(hdev, bis_list, ISO_LINK,
> - BT_BOUND, &data);
> - if (!data.count)
> + conn = hci_conn_hash_lookup_bis(hdev, BDADDR_ANY, bis);
> + if (!conn)
> break;
> }
>
> - if (data.bis == hdev->le_num_of_adv_sets)
> + if (bis == hdev->le_num_of_adv_sets)
> return -EADDRNOTAVAIL;
>
> /* Update BIS */
> - qos->bcast.bis = data.bis;
> + qos->bcast.bis = bis;
> }
>
> return 0;
> @@ -1534,8 +1529,7 @@ static struct hci_conn *hci_add_bis(struct hci_dev *hdev, bdaddr_t *dst,
> /* Check BIS settings against other bound BISes, since all
> * BISes in a BIG must have the same value for all parameters
> */
> - conn = hci_conn_hash_lookup_bis(hdev, dst, qos->bcast.big,
> - qos->bcast.bis);
> + conn = hci_conn_hash_lookup_big(hdev, qos->bcast.big);
>
> if (conn && (memcmp(qos, &conn->iso_qos, sizeof(*qos)) ||
> base_len != conn->le_per_adv_data_len ||
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 7c199f7361f7..45525963e932 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3938,24 +3938,44 @@ static u8 hci_cc_le_set_per_adv_enable(struct hci_dev *hdev, void *data,
> struct sk_buff *skb)
> {
> struct hci_ev_status *rp = data;
> - __u8 *sent;
> + struct hci_cp_le_set_per_adv_enable *cp;
> + struct adv_info *adv = NULL, *n;
>
> bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
>
> if (rp->status)
> return rp->status;
>
> - sent = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_PER_ADV_ENABLE);
> - if (!sent)
> + cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_PER_ADV_ENABLE);
> + if (!cp)
> return rp->status;
>
> hci_dev_lock(hdev);
>
> - if (*sent)
> + adv = hci_find_adv_instance(hdev, cp->handle);
> +
> + if (cp->enable) {
> hci_dev_set_flag(hdev, HCI_LE_PER_ADV);
> - else
> +
> + if (adv)
> + adv->per_enabled = true;
> + } else {
> + if (adv)
> + adv->per_enabled = false;
> +
> + /* If just one instance was disabled check if there are
> + * any other instance enabled before clearing HCI_LE_PER_ADV
> + */
> + list_for_each_entry_safe(adv, n, &hdev->adv_instances,
> + list) {
> + if (adv->per_enabled)
> + goto unlock;
> + }
> +
> hci_dev_clear_flag(hdev, HCI_LE_PER_ADV);
> + }
>
> +unlock:
> hci_dev_unlock(hdev);
>
> return rp->status;
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 97da5bcaa904..2fd7ab377d30 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -3,6 +3,7 @@
> * BlueZ - Bluetooth protocol stack for Linux
> *
> * Copyright (C) 2021 Intel Corporation
> + * Copyright 2023 NXP
> */
>
> #include <linux/property.h>
> @@ -1319,9 +1320,11 @@ int hci_start_ext_adv_sync(struct hci_dev *hdev, u8 instance)
> static int hci_disable_per_advertising_sync(struct hci_dev *hdev, u8 instance)
> {
> struct hci_cp_le_set_per_adv_enable cp;
> + struct adv_info *adv = NULL;
>
> /* If periodic advertising already disabled there is nothing to do. */
> - if (!hci_dev_test_flag(hdev, HCI_LE_PER_ADV))
> + adv = hci_find_adv_instance(hdev, instance);
> + if (!adv || !adv->per_enabled)
> return 0;
>
> memset(&cp, 0, sizeof(cp));
> @@ -1386,9 +1389,11 @@ static int hci_set_per_adv_data_sync(struct hci_dev *hdev, u8 instance)
> static int hci_enable_per_advertising_sync(struct hci_dev *hdev, u8 instance)
> {
> struct hci_cp_le_set_per_adv_enable cp;
> + struct adv_info *adv = NULL;
>
> /* If periodic advertising already enabled there is nothing to do. */
> - if (hci_dev_test_flag(hdev, HCI_LE_PER_ADV))
> + adv = hci_find_adv_instance(hdev, instance);
> + if (adv && adv->per_enabled)
> return 0;
>
> memset(&cp, 0, sizeof(cp));
> --
> 2.34.1
>
--
Luiz Augusto von Dentz
