mbox series

[0/1] Bluetooth: ISO: Support multiple BIGs

Message ID 20230613142017.9337-1-iulia.tanasescu@nxp.com
Headers show
Series Bluetooth: ISO: Support multiple BIGs | expand

Message

Iulia Tanasescu June 13, 2023, 2:20 p.m. UTC
This patch 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).

Currently, if the user tries to connect 2 sockets with the
BIG/BIS QoS fields unset, the kernel will assign the same
BIG handle for every BIG, as seen in the btmon log below:

< 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: 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: Command Disallowed (0x0c)

Iulia Tanasescu (1):
  Bluetooth: ISO: Support multiple BIGs

 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(-)


base-commit: 817efd3cad7481ce2ee25fac5108afecbad56228

Comments

Luiz Augusto von Dentz June 14, 2023, 5:05 p.m. UTC | #1
Hi Iulia,

On Tue, Jun 13, 2023 at 7:26 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> 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@nxp.com>
> ---
>  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
>
Iulia Tanasescu June 19, 2023, 3 p.m. UTC | #2
Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Wednesday, June 14, 2023 8:05 PM
> To: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu
> <claudia.rosu@nxp.com>; Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com>; Andrei Istodorescu
> <andrei.istodorescu@nxp.com>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>
> Subject: Re: [PATCH 1/1] Bluetooth: ISO: Support multiple BIGs
> 
> Hi Iulia,
> 
> On Tue, Jun 13, 2023 at 7:26AM Iulia Tanasescu <iulia.tanasescu@nxp.com>
> 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)
> >         
cking: 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@nxp.com>
> > ---
> >  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?

I added the extra per_enabled flag because I thought it would cover all cases if
I keep the 2 advertising states separate, but if periodic advertising is always
enabled/disabled along with extended advertising, I see that having both the
enabled and per_enabled flags is redundant. So I will keep extended and periodic
advertising coupled and I will use the same enabled flag for periodic instances
also. I submitted an updated patch.

> 
> >         __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


Regards,
Iulia