diff mbox series

[v9,07/10] Bluetooth: btintel: define callback to set data path

Message ID 20210608122455.19583-7-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
Adds callback function which is called to set the data path
for HFP offload case before opening SCO connection

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 | 50 +++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/btintel.h |  8 ++++++
 drivers/bluetooth/btusb.c   |  4 ++-
 include/net/bluetooth/hci.h |  8 ++++++
 4 files changed, 69 insertions(+), 1 deletion(-)

Comments

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

> Adds callback function which is called to set the data path

> for HFP offload case before opening SCO connection

> 

> 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 | 50 +++++++++++++++++++++++++++++++++++++

> drivers/bluetooth/btintel.h |  8 ++++++

> drivers/bluetooth/btusb.c   |  4 ++-

> include/net/bluetooth/hci.h |  8 ++++++

> 4 files changed, 69 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c

> index 95c6a1bef35e..3eba5c587ef6 100644

> --- a/drivers/bluetooth/btintel.c

> +++ b/drivers/bluetooth/btintel.c

> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct hci_dev *hdev,

> }

> EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);

> 

> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> +			  struct bt_codec *codec)

> +{

> +	__u8 preset;

> +	struct hci_op_configure_data_path *cmd;

> +	__u8 buffer[255];

> +	struct sk_buff *skb;

> +

> +	if (type != SCO_LINK && type != ESCO_LINK)

> +		return -EINVAL;

> +

> +	switch (codec->id) {

> +	case 0x02:

> +		preset = 0x00;

> +	break;

> +	case 0x05:

> +		preset = 0x01;

> +	break;

> +	default:

> +		return -EINVAL;

> +	}

> +

> +	cmd = (void *)buffer;

> +	cmd->data_path_id = 0x01;

> +	cmd->len = 1;

> +	cmd->data[0] = preset;

> +

> +	cmd->direction = 0x00;

> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,

> +			     cmd, HCI_INIT_TIMEOUT);

> +	if (IS_ERR(skb)) {

> +		bt_dev_err(hdev, "configure input data path failed (%ld)",

> +			   PTR_ERR(skb));

> +		return PTR_ERR(skb);

> +	}

> +	kfree_skb(skb);

> +

> +	cmd->direction = 0x01;

> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,

> +			     cmd, HCI_INIT_TIMEOUT);

> +	if (IS_ERR(skb)) {

> +		bt_dev_err(hdev, "configure output data path failed (%ld)",

> +			   PTR_ERR(skb));

> +		return PTR_ERR(skb);

> +	}

> +	kfree_skb(skb);

> +	return 0;

> +}

> +EXPORT_SYMBOL_GPL(btintel_set_data_path);

> +

> 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 9bcc489680db..9806970c9871 100644

> --- a/drivers/bluetooth/btintel.h

> +++ b/drivers/bluetooth/btintel.h

> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev *hdev,

> int btintel_read_offload_usecases(struct hci_dev *hdev,

> 				  struct intel_offload_usecases *usecases);

> int btintel_get_data_path(struct hci_dev *hdev);

> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> +			  struct bt_codec *codec);

> #else

> 

> static inline int btintel_check_bdaddr(struct hci_dev *hdev)

> @@ -325,4 +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev)

> {

> 	return -EOPNOTSUPP;

> }

> +

> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> +				 struct bt_codec *codec)

> +{

> +	return -EOPNOTSUPP;

> +}

> #endif

> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

> index 1e51beec5776..afafa44752a1 100644

> --- a/drivers/bluetooth/btusb.c

> +++ b/drivers/bluetooth/btusb.c

> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)

> 	err = btintel_read_offload_usecases(hdev, &usecases);

> 	if (!err) {

> 		/* set get_data_path callback if offload is supported */

> -		if (usecases.preset[0] & 0x03)

> +		if (usecases.preset[0] & 0x03) {

> 			hdev->get_data_path = btintel_get_data_path;

> +			hdev->set_data_path = btintel_set_data_path;

> +		}

> 	}


> 	/* Read the Intel version information after loading the FW  */

> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h

> index 31a5ac8918fc..42963188dcea 100644

> --- a/include/net/bluetooth/hci.h

> +++ b/include/net/bluetooth/hci.h

> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {

> 	__u8     rand256[16];

> } __packed;

> 

> +#define HCI_CONFIGURE_DATA_PATH	0x0c83

> +struct hci_op_configure_data_path {

> +	__u8	direction;

> +	__u8	data_path_id;

> +	__u8	len;

> +	__u8	data[];

> +} __packed;

> +


if this is a standard HCI command, why is this done as hdev->set_data_path? This makes no sense too me.

Regards

Marcel
K, Kiran June 16, 2021, 3:10 a.m. UTC | #2
Hi Marcel,

> 

> Hi Kiran,

> 

> > Adds callback function which is called to set the data path for HFP

> > offload case before opening SCO connection

> >

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

> +++++++++++++++++++++++++++++++++++++

> > drivers/bluetooth/btintel.h |  8 ++++++

> > drivers/bluetooth/btusb.c   |  4 ++-

> > include/net/bluetooth/hci.h |  8 ++++++

> > 4 files changed, 69 insertions(+), 1 deletion(-)

> >

> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c

> > index 95c6a1bef35e..3eba5c587ef6 100644

> > --- a/drivers/bluetooth/btintel.c

> > +++ b/drivers/bluetooth/btintel.c

> > @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct

> > hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);

> >

> > +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> > +			  struct bt_codec *codec)

> > +{

> > +	__u8 preset;

> > +	struct hci_op_configure_data_path *cmd;

> > +	__u8 buffer[255];

> > +	struct sk_buff *skb;

> > +

> > +	if (type != SCO_LINK && type != ESCO_LINK)

> > +		return -EINVAL;

> > +

> > +	switch (codec->id) {

> > +	case 0x02:

> > +		preset = 0x00;

> > +	break;

> > +	case 0x05:

> > +		preset = 0x01;

> > +	break;

> > +	default:

> > +		return -EINVAL;

> > +	}

> > +

> > +	cmd = (void *)buffer;

> > +	cmd->data_path_id = 0x01;

> > +	cmd->len = 1;

> > +	cmd->data[0] = preset;

> > +

> > +	cmd->direction = 0x00;

> > +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

> sizeof(*cmd) + 1,

> > +			     cmd, HCI_INIT_TIMEOUT);

> > +	if (IS_ERR(skb)) {

> > +		bt_dev_err(hdev, "configure input data path failed (%ld)",

> > +			   PTR_ERR(skb));

> > +		return PTR_ERR(skb);

> > +	}

> > +	kfree_skb(skb);

> > +

> > +	cmd->direction = 0x01;

> > +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

> sizeof(*cmd) + 1,

> > +			     cmd, HCI_INIT_TIMEOUT);

> > +	if (IS_ERR(skb)) {

> > +		bt_dev_err(hdev, "configure output data path failed (%ld)",

> > +			   PTR_ERR(skb));

> > +		return PTR_ERR(skb);

> > +	}

> > +	kfree_skb(skb);

> > +	return 0;

> > +}

> > +EXPORT_SYMBOL_GPL(btintel_set_data_path);

> > +

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

> > 9bcc489680db..9806970c9871 100644

> > --- a/drivers/bluetooth/btintel.h

> > +++ b/drivers/bluetooth/btintel.h

> > @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev

> > *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,

> > 				  struct intel_offload_usecases *usecases); int

> > btintel_get_data_path(struct hci_dev *hdev);

> > +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> > +			  struct bt_codec *codec);

> > #else

> >

> > static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@ -325,4

> > +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {

> > 	return -EOPNOTSUPP;

> > }

> > +

> > +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> > +				 struct bt_codec *codec)

> > +{

> > +	return -EOPNOTSUPP;

> > +}

> > #endif

> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

> > index 1e51beec5776..afafa44752a1 100644

> > --- a/drivers/bluetooth/btusb.c

> > +++ b/drivers/bluetooth/btusb.c

> > @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct

> hci_dev *hdev)

> > 	err = btintel_read_offload_usecases(hdev, &usecases);

> > 	if (!err) {

> > 		/* set get_data_path callback if offload is supported */

> > -		if (usecases.preset[0] & 0x03)

> > +		if (usecases.preset[0] & 0x03) {

> > 			hdev->get_data_path = btintel_get_data_path;

> > +			hdev->set_data_path = btintel_set_data_path;

> > +		}

> > 	}

> 

> > 	/* Read the Intel version information after loading the FW  */ diff

> > --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h

> > index 31a5ac8918fc..42963188dcea 100644

> > --- a/include/net/bluetooth/hci.h

> > +++ b/include/net/bluetooth/hci.h

> > @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {

> > 	__u8     rand256[16];

> > } __packed;

> >

> > +#define HCI_CONFIGURE_DATA_PATH	0x0c83

> > +struct hci_op_configure_data_path {

> > +	__u8	direction;

> > +	__u8	data_path_id;

> > +	__u8	len;

> > +	__u8	data[];

> > +} __packed;

> > +

> 

> if this is a standard HCI command, why is this done as hdev->set_data_path?

> This makes no sense too me.

Intel uses  HCI_CONFIGURE_DATA_PATH to command to set the preset ID (NBS, WBS, ...). Here len and data[] are vendor specific. I should have prefixed these fields with vnd_. I will address this in next patchset.
> 

> Regards

> 

> Marcel


Thanks,
Kiran
Marcel Holtmann June 16, 2021, 5:18 a.m. UTC | #3
Hi Kiran,

>>> Adds callback function which is called to set the data path for HFP

>>> offload case before opening SCO connection

>>> 

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

>> +++++++++++++++++++++++++++++++++++++

>>> drivers/bluetooth/btintel.h |  8 ++++++

>>> drivers/bluetooth/btusb.c   |  4 ++-

>>> include/net/bluetooth/hci.h |  8 ++++++

>>> 4 files changed, 69 insertions(+), 1 deletion(-)

>>> 

>>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c

>>> index 95c6a1bef35e..3eba5c587ef6 100644

>>> --- a/drivers/bluetooth/btintel.c

>>> +++ b/drivers/bluetooth/btintel.c

>>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct

>>> hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);

>>> 

>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

>>> +			  struct bt_codec *codec)

>>> +{

>>> +	__u8 preset;

>>> +	struct hci_op_configure_data_path *cmd;

>>> +	__u8 buffer[255];

>>> +	struct sk_buff *skb;

>>> +

>>> +	if (type != SCO_LINK && type != ESCO_LINK)

>>> +		return -EINVAL;

>>> +

>>> +	switch (codec->id) {

>>> +	case 0x02:

>>> +		preset = 0x00;

>>> +	break;

>>> +	case 0x05:

>>> +		preset = 0x01;

>>> +	break;

>>> +	default:

>>> +		return -EINVAL;

>>> +	}

>>> +

>>> +	cmd = (void *)buffer;

>>> +	cmd->data_path_id = 0x01;

>>> +	cmd->len = 1;

>>> +	cmd->data[0] = preset;

>>> +

>>> +	cmd->direction = 0x00;

>>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

>> sizeof(*cmd) + 1,

>>> +			     cmd, HCI_INIT_TIMEOUT);

>>> +	if (IS_ERR(skb)) {

>>> +		bt_dev_err(hdev, "configure input data path failed (%ld)",

>>> +			   PTR_ERR(skb));

>>> +		return PTR_ERR(skb);

>>> +	}

>>> +	kfree_skb(skb);

>>> +

>>> +	cmd->direction = 0x01;

>>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

>> sizeof(*cmd) + 1,

>>> +			     cmd, HCI_INIT_TIMEOUT);

>>> +	if (IS_ERR(skb)) {

>>> +		bt_dev_err(hdev, "configure output data path failed (%ld)",

>>> +			   PTR_ERR(skb));

>>> +		return PTR_ERR(skb);

>>> +	}

>>> +	kfree_skb(skb);

>>> +	return 0;

>>> +}

>>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);

>>> +

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

>>> 9bcc489680db..9806970c9871 100644

>>> --- a/drivers/bluetooth/btintel.h

>>> +++ b/drivers/bluetooth/btintel.h

>>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev

>>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,

>>> 				  struct intel_offload_usecases *usecases); int

>>> btintel_get_data_path(struct hci_dev *hdev);

>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

>>> +			  struct bt_codec *codec);

>>> #else

>>> 

>>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@ -325,4

>>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {

>>> 	return -EOPNOTSUPP;

>>> }

>>> +

>>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

>>> +				 struct bt_codec *codec)

>>> +{

>>> +	return -EOPNOTSUPP;

>>> +}

>>> #endif

>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

>>> index 1e51beec5776..afafa44752a1 100644

>>> --- a/drivers/bluetooth/btusb.c

>>> +++ b/drivers/bluetooth/btusb.c

>>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct

>> hci_dev *hdev)

>>> 	err = btintel_read_offload_usecases(hdev, &usecases);

>>> 	if (!err) {

>>> 		/* set get_data_path callback if offload is supported */

>>> -		if (usecases.preset[0] & 0x03)

>>> +		if (usecases.preset[0] & 0x03) {

>>> 			hdev->get_data_path = btintel_get_data_path;

>>> +			hdev->set_data_path = btintel_set_data_path;

>>> +		}

>>> 	}

>> 

>>> 	/* Read the Intel version information after loading the FW  */ diff

>>> --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h

>>> index 31a5ac8918fc..42963188dcea 100644

>>> --- a/include/net/bluetooth/hci.h

>>> +++ b/include/net/bluetooth/hci.h

>>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {

>>> 	__u8     rand256[16];

>>> } __packed;

>>> 

>>> +#define HCI_CONFIGURE_DATA_PATH	0x0c83

>>> +struct hci_op_configure_data_path {

>>> +	__u8	direction;

>>> +	__u8	data_path_id;

>>> +	__u8	len;

>>> +	__u8	data[];

>>> +} __packed;

>>> +

>> 

>> if this is a standard HCI command, why is this done as hdev->set_data_path?

>> This makes no sense too me.

> Intel uses  HCI_CONFIGURE_DATA_PATH to command to set the preset ID (NBS, WBS, ...). Here len and data[] are vendor specific. I should have prefixed these fields with vnd_. I will address this in next patchset.


if the command is defined by the Bluetooth SIG, it is handle in the core. However if it needs vendor specific input that we need a callback for just that data.

Regards

Marcel
K, Kiran June 17, 2021, 7:53 a.m. UTC | #4
Hi Marcel,

> -----Original Message-----

> From: Marcel Holtmann <marcel@holtmann.org>

> Sent: Wednesday, June 16, 2021 10:48 AM

> To: K, Kiran <kiran.k@intel.com>

> Cc: linux-bluetooth@vger.kernel.org

> Subject: Re: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data

> path

> 

> Hi Kiran,

> 

> >>> Adds callback function which is called to set the data path for HFP

> >>> offload case before opening SCO connection

> >>>

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

> >> +++++++++++++++++++++++++++++++++++++

> >>> drivers/bluetooth/btintel.h |  8 ++++++

> >>> drivers/bluetooth/btusb.c   |  4 ++-

> >>> include/net/bluetooth/hci.h |  8 ++++++

> >>> 4 files changed, 69 insertions(+), 1 deletion(-)

> >>>

> >>> diff --git a/drivers/bluetooth/btintel.c

> >>> b/drivers/bluetooth/btintel.c index 95c6a1bef35e..3eba5c587ef6

> >>> 100644

> >>> --- a/drivers/bluetooth/btintel.c

> >>> +++ b/drivers/bluetooth/btintel.c

> >>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct

> >>> hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);

> >>>

> >>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> >>> +			  struct bt_codec *codec)

> >>> +{

> >>> +	__u8 preset;

> >>> +	struct hci_op_configure_data_path *cmd;

> >>> +	__u8 buffer[255];

> >>> +	struct sk_buff *skb;

> >>> +

> >>> +	if (type != SCO_LINK && type != ESCO_LINK)

> >>> +		return -EINVAL;

> >>> +

> >>> +	switch (codec->id) {

> >>> +	case 0x02:

> >>> +		preset = 0x00;

> >>> +	break;

> >>> +	case 0x05:

> >>> +		preset = 0x01;

> >>> +	break;

> >>> +	default:

> >>> +		return -EINVAL;

> >>> +	}

> >>> +

> >>> +	cmd = (void *)buffer;

> >>> +	cmd->data_path_id = 0x01;

> >>> +	cmd->len = 1;

> >>> +	cmd->data[0] = preset;

> >>> +

> >>> +	cmd->direction = 0x00;

> >>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

> >> sizeof(*cmd) + 1,

> >>> +			     cmd, HCI_INIT_TIMEOUT);

> >>> +	if (IS_ERR(skb)) {

> >>> +		bt_dev_err(hdev, "configure input data path failed (%ld)",

> >>> +			   PTR_ERR(skb));

> >>> +		return PTR_ERR(skb);

> >>> +	}

> >>> +	kfree_skb(skb);

> >>> +

> >>> +	cmd->direction = 0x01;

> >>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

> >> sizeof(*cmd) + 1,

> >>> +			     cmd, HCI_INIT_TIMEOUT);

> >>> +	if (IS_ERR(skb)) {

> >>> +		bt_dev_err(hdev, "configure output data path failed (%ld)",

> >>> +			   PTR_ERR(skb));

> >>> +		return PTR_ERR(skb);

> >>> +	}

> >>> +	kfree_skb(skb);

> >>> +	return 0;

> >>> +}

> >>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);

> >>> +

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

> >>> 9bcc489680db..9806970c9871 100644

> >>> --- a/drivers/bluetooth/btintel.h

> >>> +++ b/drivers/bluetooth/btintel.h

> >>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev

> >>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,

> >>> 				  struct intel_offload_usecases *usecases); int

> >>> btintel_get_data_path(struct hci_dev *hdev);

> >>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> >>> +			  struct bt_codec *codec);

> >>> #else

> >>>

> >>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@

> >>> -325,4

> >>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {

> >>> 	return -EOPNOTSUPP;

> >>> }

> >>> +

> >>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> >>> +				 struct bt_codec *codec)

> >>> +{

> >>> +	return -EOPNOTSUPP;

> >>> +}

> >>> #endif

> >>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

> >>> index 1e51beec5776..afafa44752a1 100644

> >>> --- a/drivers/bluetooth/btusb.c

> >>> +++ b/drivers/bluetooth/btusb.c

> >>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct

> >> hci_dev *hdev)

> >>> 	err = btintel_read_offload_usecases(hdev, &usecases);

> >>> 	if (!err) {

> >>> 		/* set get_data_path callback if offload is supported */

> >>> -		if (usecases.preset[0] & 0x03)

> >>> +		if (usecases.preset[0] & 0x03) {

> >>> 			hdev->get_data_path = btintel_get_data_path;

> >>> +			hdev->set_data_path = btintel_set_data_path;

> >>> +		}

> >>> 	}

> >>

> >>> 	/* Read the Intel version information after loading the FW  */ diff

> >>> --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h

> >>> index 31a5ac8918fc..42963188dcea 100644

> >>> --- a/include/net/bluetooth/hci.h

> >>> +++ b/include/net/bluetooth/hci.h

> >>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {

> >>> 	__u8     rand256[16];

> >>> } __packed;

> >>>

> >>> +#define HCI_CONFIGURE_DATA_PATH	0x0c83

> >>> +struct hci_op_configure_data_path {

> >>> +	__u8	direction;

> >>> +	__u8	data_path_id;

> >>> +	__u8	len;

> >>> +	__u8	data[];

> >>> +} __packed;

> >>> +

> >>

> >> if this is a standard HCI command, why is this done as hdev-

> >set_data_path?

> >> This makes no sense too me.

> > Intel uses  HCI_CONFIGURE_DATA_PATH to command to set the preset ID

> (NBS, WBS, ...). Here len and data[] are vendor specific. I should have

> prefixed these fields with vnd_. I will address this in next patchset.

> 

> if the command is defined by the Bluetooth SIG, it is handle in the core.

> However if it needs vendor specific input that we need a callback for just that

> data.


The current design uses HCI_CONFIGURE_DATA_PATH inside  set_data_path callback and its not used at core.  I have leveraged SIG command here  to minimize defining  of new vendor command as vnd_data[] gives flexibility to pass in non-standard values. Other vendors may not have same command/flow to configure data path. 

If we are not supposed to use Bluetooth SIG command at driver level, then I need to come up with a new vendor specific command.  Please help with your input.
> 

> Regards

> 

> Marcel

Thanks,
Kiran
Marcel Holtmann June 17, 2021, 10:19 a.m. UTC | #5
Hi Kiran,

>>>>> Adds callback function which is called to set the data path for HFP

>>>>> offload case before opening SCO connection

>>>>> 

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

>>>> +++++++++++++++++++++++++++++++++++++

>>>>> drivers/bluetooth/btintel.h |  8 ++++++

>>>>> drivers/bluetooth/btusb.c   |  4 ++-

>>>>> include/net/bluetooth/hci.h |  8 ++++++

>>>>> 4 files changed, 69 insertions(+), 1 deletion(-)

>>>>> 

>>>>> diff --git a/drivers/bluetooth/btintel.c

>>>>> b/drivers/bluetooth/btintel.c index 95c6a1bef35e..3eba5c587ef6

>>>>> 100644

>>>>> --- a/drivers/bluetooth/btintel.c

>>>>> +++ b/drivers/bluetooth/btintel.c

>>>>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct

>>>>> hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);

>>>>> 

>>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

>>>>> +			  struct bt_codec *codec)

>>>>> +{

>>>>> +	__u8 preset;

>>>>> +	struct hci_op_configure_data_path *cmd;

>>>>> +	__u8 buffer[255];

>>>>> +	struct sk_buff *skb;

>>>>> +

>>>>> +	if (type != SCO_LINK && type != ESCO_LINK)

>>>>> +		return -EINVAL;

>>>>> +

>>>>> +	switch (codec->id) {

>>>>> +	case 0x02:

>>>>> +		preset = 0x00;

>>>>> +	break;

>>>>> +	case 0x05:

>>>>> +		preset = 0x01;

>>>>> +	break;

>>>>> +	default:

>>>>> +		return -EINVAL;

>>>>> +	}

>>>>> +

>>>>> +	cmd = (void *)buffer;

>>>>> +	cmd->data_path_id = 0x01;

>>>>> +	cmd->len = 1;

>>>>> +	cmd->data[0] = preset;

>>>>> +

>>>>> +	cmd->direction = 0x00;

>>>>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

>>>> sizeof(*cmd) + 1,

>>>>> +			     cmd, HCI_INIT_TIMEOUT);

>>>>> +	if (IS_ERR(skb)) {

>>>>> +		bt_dev_err(hdev, "configure input data path failed (%ld)",

>>>>> +			   PTR_ERR(skb));

>>>>> +		return PTR_ERR(skb);

>>>>> +	}

>>>>> +	kfree_skb(skb);

>>>>> +

>>>>> +	cmd->direction = 0x01;

>>>>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

>>>> sizeof(*cmd) + 1,

>>>>> +			     cmd, HCI_INIT_TIMEOUT);

>>>>> +	if (IS_ERR(skb)) {

>>>>> +		bt_dev_err(hdev, "configure output data path failed (%ld)",

>>>>> +			   PTR_ERR(skb));

>>>>> +		return PTR_ERR(skb);

>>>>> +	}

>>>>> +	kfree_skb(skb);

>>>>> +	return 0;

>>>>> +}

>>>>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);

>>>>> +

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

>>>>> 9bcc489680db..9806970c9871 100644

>>>>> --- a/drivers/bluetooth/btintel.h

>>>>> +++ b/drivers/bluetooth/btintel.h

>>>>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev

>>>>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,

>>>>> 				  struct intel_offload_usecases *usecases); int

>>>>> btintel_get_data_path(struct hci_dev *hdev);

>>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

>>>>> +			  struct bt_codec *codec);

>>>>> #else

>>>>> 

>>>>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@

>>>>> -325,4

>>>>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {

>>>>> 	return -EOPNOTSUPP;

>>>>> }

>>>>> +

>>>>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

>>>>> +				 struct bt_codec *codec)

>>>>> +{

>>>>> +	return -EOPNOTSUPP;

>>>>> +}

>>>>> #endif

>>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

>>>>> index 1e51beec5776..afafa44752a1 100644

>>>>> --- a/drivers/bluetooth/btusb.c

>>>>> +++ b/drivers/bluetooth/btusb.c

>>>>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct

>>>> hci_dev *hdev)

>>>>> 	err = btintel_read_offload_usecases(hdev, &usecases);

>>>>> 	if (!err) {

>>>>> 		/* set get_data_path callback if offload is supported */

>>>>> -		if (usecases.preset[0] & 0x03)

>>>>> +		if (usecases.preset[0] & 0x03) {

>>>>> 			hdev->get_data_path = btintel_get_data_path;

>>>>> +			hdev->set_data_path = btintel_set_data_path;

>>>>> +		}

>>>>> 	}

>>>> 

>>>>> 	/* Read the Intel version information after loading the FW  */ diff

>>>>> --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h

>>>>> index 31a5ac8918fc..42963188dcea 100644

>>>>> --- a/include/net/bluetooth/hci.h

>>>>> +++ b/include/net/bluetooth/hci.h

>>>>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {

>>>>> 	__u8     rand256[16];

>>>>> } __packed;

>>>>> 

>>>>> +#define HCI_CONFIGURE_DATA_PATH	0x0c83

>>>>> +struct hci_op_configure_data_path {

>>>>> +	__u8	direction;

>>>>> +	__u8	data_path_id;

>>>>> +	__u8	len;

>>>>> +	__u8	data[];

>>>>> +} __packed;

>>>>> +

>>>> 

>>>> if this is a standard HCI command, why is this done as hdev-

>>> set_data_path?

>>>> This makes no sense too me.

>>> Intel uses  HCI_CONFIGURE_DATA_PATH to command to set the preset ID

>> (NBS, WBS, ...). Here len and data[] are vendor specific. I should have

>> prefixed these fields with vnd_. I will address this in next patchset.

>> 

>> if the command is defined by the Bluetooth SIG, it is handle in the core.

>> However if it needs vendor specific input that we need a callback for just that

>> data.

> 

> The current design uses HCI_CONFIGURE_DATA_PATH inside  set_data_path callback and its not used at core.  I have leveraged SIG command here  to minimize defining  of new vendor command as vnd_data[] gives flexibility to pass in non-standard values. Other vendors may not have same command/flow to configure data path. 

> 

> If we are not supposed to use Bluetooth SIG command at driver level, then I need to come up with a new vendor specific command.  Please help with your input.


I don’t understand this argumentation. The Bluetooth standard defined HCI_Configure_Data_Path with Vendor_Specific_Config for exactly this reason. So just use it especially if our controllers already support it.

Now I am starting to wonder if this design is making things complicated for no reason. Isn’t it enough to have a hdev->get_data_path_config callback that allows to retrieve such data from the driver.

Frankly, the only thing you need from a driver is that it tells you the values of data_path_id and the vendor_config so that you can feed it back into the controller. Or am I missing anything here?

Let me be clear, if there is a SIG defined command, we implement support for in the core and not the driver. I do not want that other vendors have to define everything over and over again. That is what a standard is for. Only the vendor specific portions are handed off to the driver or userspace to provide.

Regards

Marcel
K, Kiran June 23, 2021, 3:21 a.m. UTC | #6
Hi Marcel,

> -----Original Message-----

> From: Marcel Holtmann <marcel@holtmann.org>

> Sent: Thursday, June 17, 2021 3:50 PM

> To: K, Kiran <kiran.k@intel.com>

> Cc: linux-bluetooth@vger.kernel.org

> Subject: Re: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data

> path

> 

> Hi Kiran,

> 

> >>>>> Adds callback function which is called to set the data path for

> >>>>> HFP offload case before opening SCO connection

> >>>>>

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

> >>>> +++++++++++++++++++++++++++++++++++++

> >>>>> drivers/bluetooth/btintel.h |  8 ++++++

> >>>>> drivers/bluetooth/btusb.c   |  4 ++-

> >>>>> include/net/bluetooth/hci.h |  8 ++++++

> >>>>> 4 files changed, 69 insertions(+), 1 deletion(-)

> >>>>>

> >>>>> diff --git a/drivers/bluetooth/btintel.c

> >>>>> b/drivers/bluetooth/btintel.c index 95c6a1bef35e..3eba5c587ef6

> >>>>> 100644

> >>>>> --- a/drivers/bluetooth/btintel.c

> >>>>> +++ b/drivers/bluetooth/btintel.c

> >>>>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct

> >>>>> hci_dev *hdev, }

> EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);

> >>>>>

> >>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> >>>>> +			  struct bt_codec *codec)

> >>>>> +{

> >>>>> +	__u8 preset;

> >>>>> +	struct hci_op_configure_data_path *cmd;

> >>>>> +	__u8 buffer[255];

> >>>>> +	struct sk_buff *skb;

> >>>>> +

> >>>>> +	if (type != SCO_LINK && type != ESCO_LINK)

> >>>>> +		return -EINVAL;

> >>>>> +

> >>>>> +	switch (codec->id) {

> >>>>> +	case 0x02:

> >>>>> +		preset = 0x00;

> >>>>> +	break;

> >>>>> +	case 0x05:

> >>>>> +		preset = 0x01;

> >>>>> +	break;

> >>>>> +	default:

> >>>>> +		return -EINVAL;

> >>>>> +	}

> >>>>> +

> >>>>> +	cmd = (void *)buffer;

> >>>>> +	cmd->data_path_id = 0x01;

> >>>>> +	cmd->len = 1;

> >>>>> +	cmd->data[0] = preset;

> >>>>> +

> >>>>> +	cmd->direction = 0x00;

> >>>>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

> >>>> sizeof(*cmd) + 1,

> >>>>> +			     cmd, HCI_INIT_TIMEOUT);

> >>>>> +	if (IS_ERR(skb)) {

> >>>>> +		bt_dev_err(hdev, "configure input data path failed

> (%ld)",

> >>>>> +			   PTR_ERR(skb));

> >>>>> +		return PTR_ERR(skb);

> >>>>> +	}

> >>>>> +	kfree_skb(skb);

> >>>>> +

> >>>>> +	cmd->direction = 0x01;

> >>>>> +	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,

> >>>> sizeof(*cmd) + 1,

> >>>>> +			     cmd, HCI_INIT_TIMEOUT);

> >>>>> +	if (IS_ERR(skb)) {

> >>>>> +		bt_dev_err(hdev, "configure output data path failed

> (%ld)",

> >>>>> +			   PTR_ERR(skb));

> >>>>> +		return PTR_ERR(skb);

> >>>>> +	}

> >>>>> +	kfree_skb(skb);

> >>>>> +	return 0;

> >>>>> +}

> >>>>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);

> >>>>> +

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

> >>>>> 9bcc489680db..9806970c9871 100644

> >>>>> --- a/drivers/bluetooth/btintel.h

> >>>>> +++ b/drivers/bluetooth/btintel.h

> >>>>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev

> >>>>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,

> >>>>> 				  struct intel_offload_usecases *usecases); int

> >>>>> btintel_get_data_path(struct hci_dev *hdev);

> >>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> >>>>> +			  struct bt_codec *codec);

> >>>>> #else

> >>>>>

> >>>>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@

> >>>>> -325,4

> >>>>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev)

> >>>>> +{

> >>>>> 	return -EOPNOTSUPP;

> >>>>> }

> >>>>> +

> >>>>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,

> >>>>> +				 struct bt_codec *codec)

> >>>>> +{

> >>>>> +	return -EOPNOTSUPP;

> >>>>> +}

> >>>>> #endif

> >>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c

> >>>>> index 1e51beec5776..afafa44752a1 100644

> >>>>> --- a/drivers/bluetooth/btusb.c

> >>>>> +++ b/drivers/bluetooth/btusb.c

> >>>>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct

> >>>> hci_dev *hdev)

> >>>>> 	err = btintel_read_offload_usecases(hdev, &usecases);

> >>>>> 	if (!err) {

> >>>>> 		/* set get_data_path callback if offload is supported */

> >>>>> -		if (usecases.preset[0] & 0x03)

> >>>>> +		if (usecases.preset[0] & 0x03) {

> >>>>> 			hdev->get_data_path = btintel_get_data_path;

> >>>>> +			hdev->set_data_path =

> btintel_set_data_path;

> >>>>> +		}

> >>>>> 	}

> >>>>

> >>>>> 	/* Read the Intel version information after loading the FW  */

> >>>>> diff --git a/include/net/bluetooth/hci.h

> >>>>> b/include/net/bluetooth/hci.h index 31a5ac8918fc..42963188dcea

> >>>>> 100644

> >>>>> --- a/include/net/bluetooth/hci.h

> >>>>> +++ b/include/net/bluetooth/hci.h

> >>>>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {

> >>>>> 	__u8     rand256[16];

> >>>>> } __packed;

> >>>>>

> >>>>> +#define HCI_CONFIGURE_DATA_PATH	0x0c83

> >>>>> +struct hci_op_configure_data_path {

> >>>>> +	__u8	direction;

> >>>>> +	__u8	data_path_id;

> >>>>> +	__u8	len;

> >>>>> +	__u8	data[];

> >>>>> +} __packed;

> >>>>> +

> >>>>

> >>>> if this is a standard HCI command, why is this done as hdev-

> >>> set_data_path?

> >>>> This makes no sense too me.

> >>> Intel uses  HCI_CONFIGURE_DATA_PATH to command to set the preset

> ID

> >> (NBS, WBS, ...). Here len and data[] are vendor specific. I should

> >> have prefixed these fields with vnd_. I will address this in next patchset.

> >>

> >> if the command is defined by the Bluetooth SIG, it is handle in the core.

> >> However if it needs vendor specific input that we need a callback for

> >> just that data.

> >

> > The current design uses HCI_CONFIGURE_DATA_PATH inside

> set_data_path callback and its not used at core.  I have leveraged SIG

> command here  to minimize defining  of new vendor command as vnd_data[]

> gives flexibility to pass in non-standard values. Other vendors may not have

> same command/flow to configure data path.

> >

> > If we are not supposed to use Bluetooth SIG command at driver level, then

> I need to come up with a new vendor specific command.  Please help with

> your input.

> 

> I don’t understand this argumentation. The Bluetooth standard defined

> HCI_Configure_Data_Path with Vendor_Specific_Config for exactly this

> reason. So just use it especially if our controllers already support it.

> 

> Now I am starting to wonder if this design is making things complicated for

> no reason. Isn’t it enough to have a hdev->get_data_path_config callback

> that allows to retrieve such data from the driver.

> 

> Frankly, the only thing you need from a driver is that it tells you the values of

> data_path_id and the vendor_config so that you can feed it back into the

> controller. Or am I missing anything here?

> 

> Let me be clear, if there is a SIG defined command, we implement support

> for in the core and not the driver. I do not want that other vendors have to

> define everything over and over again. That is what a standard is for. Only

> the vendor specific portions are handed off to the driver or userspace to

> provide.


Agree. I will make the suggested changes in the next patchset.
> 

> Regards

> 

> Marcel


Thanks,
Kiran
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 95c6a1bef35e..3eba5c587ef6 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1308,6 +1308,56 @@  int btintel_read_offload_usecases(struct hci_dev *hdev,
 }
 EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
 
+int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
+			  struct bt_codec *codec)
+{
+	__u8 preset;
+	struct hci_op_configure_data_path *cmd;
+	__u8 buffer[255];
+	struct sk_buff *skb;
+
+	if (type != SCO_LINK && type != ESCO_LINK)
+		return -EINVAL;
+
+	switch (codec->id) {
+	case 0x02:
+		preset = 0x00;
+	break;
+	case 0x05:
+		preset = 0x01;
+	break;
+	default:
+		return -EINVAL;
+	}
+
+	cmd = (void *)buffer;
+	cmd->data_path_id = 0x01;
+	cmd->len = 1;
+	cmd->data[0] = preset;
+
+	cmd->direction = 0x00;
+	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,
+			     cmd, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "configure input data path failed (%ld)",
+			   PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	cmd->direction = 0x01;
+	skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,
+			     cmd, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "configure output data path failed (%ld)",
+			   PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btintel_set_data_path);
+
 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 9bcc489680db..9806970c9871 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -183,6 +183,8 @@  int btintel_set_debug_features(struct hci_dev *hdev,
 int btintel_read_offload_usecases(struct hci_dev *hdev,
 				  struct intel_offload_usecases *usecases);
 int btintel_get_data_path(struct hci_dev *hdev);
+int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
+			  struct bt_codec *codec);
 #else
 
 static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -325,4 +327,10 @@  static int btintel_get_data_path(struct hci_dev *hdev)
 {
 	return -EOPNOTSUPP;
 }
+
+static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
+				 struct bt_codec *codec)
+{
+	return -EOPNOTSUPP;
+}
 #endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1e51beec5776..afafa44752a1 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3012,8 +3012,10 @@  static int btusb_setup_intel_newgen(struct hci_dev *hdev)
 	err = btintel_read_offload_usecases(hdev, &usecases);
 	if (!err) {
 		/* set get_data_path callback if offload is supported */
-		if (usecases.preset[0] & 0x03)
+		if (usecases.preset[0] & 0x03) {
 			hdev->get_data_path = btintel_get_data_path;
+			hdev->set_data_path = btintel_set_data_path;
+		}
 	}
 
 	/* Read the Intel version information after loading the FW  */
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 31a5ac8918fc..42963188dcea 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1250,6 +1250,14 @@  struct hci_rp_read_local_oob_ext_data {
 	__u8     rand256[16];
 } __packed;
 
+#define HCI_CONFIGURE_DATA_PATH	0x0c83
+struct hci_op_configure_data_path {
+	__u8	direction;
+	__u8	data_path_id;
+	__u8	len;
+	__u8	data[];
+} __packed;
+
 #define HCI_OP_READ_LOCAL_VERSION	0x1001
 struct hci_rp_read_local_version {
 	__u8     status;