2021-06-08 12:23:00

by Kiran K

[permalink] [raw]
Subject: [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback

Read supported offload usecases and set get_data_path callback if
controller suppports offload codecs.

Signed-off-by: Kiran K <[email protected]>
Reviewed-by: Chethan T N <[email protected]>
Reviewed-by: Srivatsa Ravishankar <[email protected]>
---
drivers/bluetooth/btintel.c | 36 ++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 18 ++++++++++++++++++
drivers/bluetooth/btusb.c | 8 ++++++++
3 files changed, 62 insertions(+)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e44b6993cf91..95c6a1bef35e 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1272,6 +1272,42 @@ int btintel_set_debug_features(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btintel_set_debug_features);

+int btintel_get_data_path(struct hci_dev *hdev)
+{
+ return 1;
+}
+EXPORT_SYMBOL_GPL(btintel_get_data_path);
+
+int btintel_read_offload_usecases(struct hci_dev *hdev,
+ struct intel_offload_usecases *usecases)
+{
+ struct sk_buff *skb;
+ int err = 0;
+
+ skb = __hci_cmd_sync(hdev, 0xfc86, 0, NULL, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Reading offload usecases failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ if (skb->len < sizeof(*usecases)) {
+ err = -EIO;
+ goto error;
+ }
+
+ if (skb->data[0]) {
+ err = -bt_to_errno(skb->data[0]);
+ goto error;
+ }
+
+ memcpy(usecases, skb->data, sizeof(*usecases));
+error:
+ kfree_skb(skb);
+ return err;
+}
+EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
+
MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
MODULE_VERSION(VERSION);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index d184064a5e7c..9bcc489680db 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -132,6 +132,11 @@ struct intel_debug_features {
__u8 page1[16];
} __packed;

+struct intel_offload_usecases {
+ __u8 status;
+ __u8 preset[8];
+} __packed;
+
#define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8))
#define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16))
#define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff)
@@ -175,6 +180,9 @@ int btintel_read_debug_features(struct hci_dev *hdev,
struct intel_debug_features *features);
int btintel_set_debug_features(struct hci_dev *hdev,
const struct intel_debug_features *features);
+int btintel_read_offload_usecases(struct hci_dev *hdev,
+ struct intel_offload_usecases *usecases);
+int btintel_get_data_path(struct hci_dev *hdev);
#else

static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -307,4 +315,14 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
return -EOPNOTSUPP;
}

+static int btintel_read_offload_usecases(struct hci_dev *hdev,
+ struct intel_offload_usecases *usecases)
+{
+ return -EOPNOTSUPP;
+}
+
+static int btintel_get_data_path(struct hci_dev *hdev)
+{
+ return -EOPNOTSUPP;
+}
#endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index a9855a2dd561..1e51beec5776 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2952,6 +2952,7 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
int err;
struct intel_debug_features features;
struct intel_version_tlv version;
+ struct intel_offload_usecases usecases;

bt_dev_dbg(hdev, "");

@@ -3008,6 +3009,13 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
/* Set DDC mask for available debug features */
btintel_set_debug_features(hdev, &features);

+ err = btintel_read_offload_usecases(hdev, &usecases);
+ if (!err) {
+ /* set get_data_path callback if offload is supported */
+ if (usecases.preset[0] & 0x03)
+ hdev->get_data_path = btintel_get_data_path;
+ }
+
/* Read the Intel version information after loading the FW */
err = btintel_read_version_tlv(hdev, &version);
if (err)
--
2.17.1


2021-06-15 19:31:50

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback

Hi Kiran,

> Read supported offload usecases and set get_data_path callback if
> controller suppports offload codecs.
>
> Signed-off-by: Kiran K <[email protected]>
> Reviewed-by: Chethan T N <[email protected]>
> Reviewed-by: Srivatsa Ravishankar <[email protected]>
> ---
> drivers/bluetooth/btintel.c | 36 ++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 18 ++++++++++++++++++
> drivers/bluetooth/btusb.c | 8 ++++++++
> 3 files changed, 62 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e44b6993cf91..95c6a1bef35e 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1272,6 +1272,42 @@ int btintel_set_debug_features(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btintel_set_debug_features);
>
> +int btintel_get_data_path(struct hci_dev *hdev)
> +{
> + return 1;
> +}
> +EXPORT_SYMBOL_GPL(btintel_get_data_path);
> +
> +int btintel_read_offload_usecases(struct hci_dev *hdev,
> + struct intel_offload_usecases *usecases)
> +{
> + struct sk_buff *skb;
> + int err = 0;
> +
> + skb = __hci_cmd_sync(hdev, 0xfc86, 0, NULL, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "Reading offload usecases failed (%ld)",
> + PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> +
> + if (skb->len < sizeof(*usecases)) {
> + err = -EIO;
> + goto error;
> + }
> +
> + if (skb->data[0]) {
> + err = -bt_to_errno(skb->data[0]);
> + goto error;
> + }
> +
> + memcpy(usecases, skb->data, sizeof(*usecases));
> +error:
> + kfree_skb(skb);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> +
> MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
> MODULE_VERSION(VERSION);
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index d184064a5e7c..9bcc489680db 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -132,6 +132,11 @@ struct intel_debug_features {
> __u8 page1[16];
> } __packed;
>
> +struct intel_offload_usecases {
> + __u8 status;
> + __u8 preset[8];
> +} __packed;
> +
> #define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8))
> #define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16))
> #define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff)
> @@ -175,6 +180,9 @@ int btintel_read_debug_features(struct hci_dev *hdev,
> struct intel_debug_features *features);
> int btintel_set_debug_features(struct hci_dev *hdev,
> const struct intel_debug_features *features);
> +int btintel_read_offload_usecases(struct hci_dev *hdev,
> + struct intel_offload_usecases *usecases);
> +int btintel_get_data_path(struct hci_dev *hdev);
> #else
>
> static inline int btintel_check_bdaddr(struct hci_dev *hdev)
> @@ -307,4 +315,14 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
> return -EOPNOTSUPP;
> }
>
> +static int btintel_read_offload_usecases(struct hci_dev *hdev,
> + struct intel_offload_usecases *usecases)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static int btintel_get_data_path(struct hci_dev *hdev)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index a9855a2dd561..1e51beec5776 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2952,6 +2952,7 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
> int err;
> struct intel_debug_features features;
> struct intel_version_tlv version;
> + struct intel_offload_usecases usecases;
>
> bt_dev_dbg(hdev, "");
>
> @@ -3008,6 +3009,13 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
> /* Set DDC mask for available debug features */
> btintel_set_debug_features(hdev, &features);
>
> + err = btintel_read_offload_usecases(hdev, &usecases);
> + if (!err) {
> + /* set get_data_path callback if offload is supported */
> + if (usecases.preset[0] & 0x03)
> + hdev->get_data_path = btintel_get_data_path;
> + }
> +

I really wonder if this exporting everything single detail to be just used by btusb is really a good idea. Would it be just enough to have a btintel_configure_offload(hdev); helper that does everything and be done with it. There is already too much Intel specifics bleeding into btusb.c and I think we need to correct that going forward.

Regards

Marcel

2021-06-16 03:03:54

by Kiran K

[permalink] [raw]
Subject: RE: [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback

Hi Marcel,

> Hi Kiran,
>
> > Read supported offload usecases and set get_data_path callback if
> > controller suppports offload codecs.
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index a9855a2dd561..1e51beec5776 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2952,6 +2952,7 @@ static int btusb_setup_intel_newgen(struct
> hci_dev *hdev)
> > int err;
> > struct intel_debug_features features;
> > struct intel_version_tlv version;
> > + struct intel_offload_usecases usecases;
> >
> > bt_dev_dbg(hdev, "");
> >
> > @@ -3008,6 +3009,13 @@ static int btusb_setup_intel_newgen(struct
> hci_dev *hdev)
> > /* Set DDC mask for available debug features */
> > btintel_set_debug_features(hdev, &features);
> >
> > + err = btintel_read_offload_usecases(hdev, &usecases);
> > + if (!err) {
> > + /* set get_data_path callback if offload is supported */
> > + if (usecases.preset[0] & 0x03)
> > + hdev->get_data_path = btintel_get_data_path;
> > + }
> > +
>
> I really wonder if this exporting everything single detail to be just used by
> btusb is really a good idea. Would it be just enough to have a
> btintel_configure_offload(hdev); helper that does everything and be done
> with it. There is already too much Intel specifics bleeding into btusb.c and I
> think we need to correct that going forward.
>

Ok. I will refactor this to move Intel specific code to btintel*
> Regards
>
> Marcel

Thanks,
Kiran