diff mbox series

[v9,04/10] Bluetooth: btintel: set get_data_path callback

Message ID 20210608122455.19583-4-kiran.k@intel.com
State New
Headers show
Series [v9,01/10] Bluetooth: enumerate local supported codec and cache details | expand

Commit Message

K, Kiran June 8, 2021, 12:24 p.m. UTC
Read supported offload usecases and set get_data_path callback if
controller suppports offload codecs.

Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
 drivers/bluetooth/btintel.c | 36 ++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/btintel.h | 18 ++++++++++++++++++
 drivers/bluetooth/btusb.c   |  8 ++++++++
 3 files changed, 62 insertions(+)

Comments

Marcel Holtmann June 15, 2021, 7:30 p.m. UTC | #1
Hi Kiran,

> Read supported offload usecases and set get_data_path callback if

> controller suppports offload codecs.

> 

> Signed-off-by: Kiran K <kiran.k@intel.com>

> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>

> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>

> ---

> 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 <marcel@holtmann.org>");

> 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
K, Kiran June 16, 2021, 3:02 a.m. UTC | #2
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
diff mbox series

Patch

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 <marcel@holtmann.org>");
 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)