Re: [PATCH v10 10/10] Bluetooth: Add offload feature under experimental flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kiran,

On Wed, Jun 30, 2021 at 1:08 AM Kiran K <kiran.k@xxxxxxxxx> wrote:
>
> Allow user level process to enable / disable codec offload
> feature through mgmt interface. By default offload codec feature
> is disabled.
>
> Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@xxxxxxxxx>
> ---
> * changes in v10:
>   - new patch added to place offload codec feature under experimental flag
>
>  include/net/bluetooth/hci.h |   4 ++
>  net/bluetooth/mgmt.c        | 106 +++++++++++++++++++++++++++++++++++-
>  net/bluetooth/sco.c         |  10 ++++
>  3 files changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index e20318854900..5ca98d9f64dd 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -331,6 +331,10 @@ enum {
>         HCI_CMD_PENDING,
>         HCI_FORCE_NO_MITM,
>
> +#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
> +       HCI_OFFLOAD_CODECS_ENABLED,
> +#endif

That is probably a bad idea as it could lead the enum to assume
different values based on what is enabled, besides we don't gain
anything by not having the symbol defined all the time.

> +
>         __HCI_NUM_FLAGS,
>  };
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 3663f880df11..d7be85eb52e7 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3791,6 +3791,14 @@ static const u8 debug_uuid[16] = {
>  };
>  #endif
>
> +#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
> +/* a6695ace-ee7f-4fb9-881a-5fac66c629af */
> +static const u8 offload_codecs_uuid[16] = {
> +       0xaf, 0x29, 0xc6, 0x66, 0xac, 0x5f, 0x1a, 0x88,
> +       0xb9, 0x4f, 0x7f, 0xee, 0xce, 0x5a, 0x69, 0xa6,
> +};
> +#endif
> +
>  /* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
>  static const u8 simult_central_periph_uuid[16] = {
>         0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
> @@ -3806,7 +3814,7 @@ static const u8 rpa_resolution_uuid[16] = {
>  static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
>                                   void *data, u16 data_len)
>  {
> -       char buf[62];   /* Enough space for 3 features */
> +       char buf[82];   /* Enough space for 4 features: 2 + 20 * 4 */
>         struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
>         u16 idx = 0;
>         u32 flags;
> @@ -3850,6 +3858,28 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
>                 idx++;
>         }
>
> +#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
> +       if (hdev) {

If we have HCI_OFFLOAD_CODECS_ENABLED defined you can just use
IS_ENABLED within the if statement below.

> +               if (hdev->get_data_path_id) {
> +                       /* BIT(0): indicating if offload codecs are
> +                        * supported by controller.
> +                        */
> +                       flags = BIT(0);
> +
> +                       /* BIT(1): indicating if codec offload feature
> +                        * is enabled.
> +                        */
> +                       if (hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED))
> +                               flags |= BIT(1);
> +               } else {
> +                       flags = 0;
> +               }
> +               memcpy(rp->features[idx].uuid, offload_codecs_uuid, 16);
> +               rp->features[idx].flags = cpu_to_le32(flags);
> +               idx++;
> +       }
> +#endif
> +
>         rp->feature_count = cpu_to_le16(idx);
>
>         /* After reading the experimental features information, enable
> @@ -3892,6 +3922,23 @@ static int exp_debug_feature_changed(bool enabled, struct sock *skip)
>  }
>  #endif
>
> +#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
> +static int exp_offload_codec_feature_changed(bool enabled, struct sock *skip)
> +{
> +       struct mgmt_ev_exp_feature_changed ev;
> +
> +       BT_INFO("enabled %d", enabled);
> +
> +       memset(&ev, 0, sizeof(ev));
> +       memcpy(ev.uuid, offload_codecs_uuid, 16);
> +       ev.flags = cpu_to_le32(enabled ? BIT(0) : 0);
> +
> +       return mgmt_limited_event(MGMT_EV_EXP_FEATURE_CHANGED, NULL,
> +                                 &ev, sizeof(ev),
> +                                 HCI_MGMT_EXP_FEATURE_EVENTS, skip);
> +}
> +#endif
> +
>  static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
>                            void *data, u16 data_len)
>  {
> @@ -4038,6 +4085,63 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
>                 return err;
>         }
>
> +#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)

Ditto.

> +       if (!memcmp(cp->uuid, offload_codecs_uuid, 16)) {
> +               bool val, changed;
> +               int err;
> +
> +               /* Command requires to use a valid controller index */
> +               if (!hdev)
> +                       return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> +                                              MGMT_OP_SET_EXP_FEATURE,
> +                                              MGMT_STATUS_INVALID_INDEX);
> +
> +               /* Parameters are limited to a single octet */
> +               if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
> +                       return mgmt_cmd_status(sk, hdev->id,
> +                                              MGMT_OP_SET_EXP_FEATURE,
> +                                              MGMT_STATUS_INVALID_PARAMS);
> +
> +               /* Only boolean on/off is supported */
> +               if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
> +                       return mgmt_cmd_status(sk, hdev->id,
> +                                              MGMT_OP_SET_EXP_FEATURE,
> +                                              MGMT_STATUS_INVALID_PARAMS);
> +
> +               val = !!cp->param[0];
> +               changed = (val != hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED));
> +
> +               if (!hdev->get_data_path_id) {
> +                       bt_dev_info(hdev, "offload codecs not supported");
> +                       return mgmt_cmd_status(sk, hdev->id,
> +                                              MGMT_OP_SET_EXP_FEATURE,
> +                                              MGMT_STATUS_NOT_SUPPORTED);
> +               }
> +
> +               if (changed) {
> +                       if (val)
> +                               hci_dev_set_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED);
> +                       else
> +                               hci_dev_clear_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED);
> +               }
> +
> +               bt_dev_info(hdev, "offload codecs enable %d changed %d",
> +                           val, changed);
> +
> +               memcpy(rp.uuid, offload_codecs_uuid, 16);
> +               rp.flags = cpu_to_le32(val ? BIT(0) : 0);
> +               hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
> +               err = mgmt_cmd_complete(sk, hdev->id,
> +                                       MGMT_OP_SET_EXP_FEATURE, 0,
> +                                       &rp, sizeof(rp));
> +
> +               if (changed)
> +                       exp_offload_codec_feature_changed(val, sk);
> +
> +               return err;
> +       }
> +#endif
> +
>         return mgmt_cmd_status(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
>                                MGMT_OP_SET_EXP_FEATURE,
>                                MGMT_STATUS_NOT_SUPPORTED);
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index b44d56eb936a..bc033464af43 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -964,6 +964,11 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
>                         break;
>                 }
>
> +               if (!hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED)) {
> +                       err = -EOPNOTSUPP;
> +                       break;
> +               }
> +
>                 if (!hdev->get_codec_config_data) {
>                         err = -EOPNOTSUPP;
>                         break;
> @@ -1163,6 +1168,11 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
>                         break;
>                 }
>
> +               if (!hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED)) {
> +                       err = -EOPNOTSUPP;
> +                       break;
> +               }
> +
>                 if (!hdev->get_data_path_id) {
>                         err = -EOPNOTSUPP;
>                         break;
> --
> 2.17.1
>


-- 
Luiz Augusto von Dentz



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux