All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details
@ 2021-05-18 10:42 Kiran K
  2021-05-18 10:42 ` [PATCH v8 2/9] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

Move reading of supported local codecs into a separate init function,
query codecs capabilities and cache the data

Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
Reported-by: kernel test robot <lkp@intel.com>
---
* changes in v8:
  - add comments
  - split __u8 codec_id[5] into {__u8 id; __le16 cid, vid }
  - address review comment related codec caps structure

* changes in v7:
  - keep codec enumeration call in hci_init instead of having a separate
    function
  - Remove unused bitmasks defined for LE transports

* changes  in v6:
  - fix compiler warning reported for ARCH=arc

* changes in v5:
  - fix review comments
  - move code used to read standard/vendor codecs caps into single function

* changes in v4:
  - convert  reading of codecs and codecs caps calls from async to sync

* changes in v3
  move codec enumeration into a new init function

* changes in v2
  add skb length check before accessing data

 include/net/bluetooth/hci.h      |  41 +++++++
 include/net/bluetooth/hci_core.h |  17 +++
 net/bluetooth/hci_core.c         | 199 ++++++++++++++++++++++++++++++-
 3 files changed, 253 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c4b0650fb9ae..6cb9340a2d51 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1307,6 +1307,28 @@ struct hci_rp_read_data_block_size {
 } __packed;
 
 #define HCI_OP_READ_LOCAL_CODECS	0x100b
+struct hci_std_codecs {
+	__u8	num;
+	__u8	codec[];
+} __packed;
+
+struct hci_ven_codec {
+	/* company id */
+	__le16	cid;
+	/* vendor codec id */
+	__le16	vid;
+} __packed;
+
+struct hci_ven_codecs {
+	__u8	num;
+	struct hci_ven_codec codec[];
+} __packed;
+
+struct hci_rp_read_local_supported_codecs {
+	__u8	status;
+	struct hci_std_codecs std_codecs;
+	struct hci_ven_codecs ven_codecs;
+} __packed;
 
 #define HCI_OP_READ_LOCAL_PAIRING_OPTS	0x100c
 struct hci_rp_read_local_pairing_opts {
@@ -1315,6 +1337,25 @@ struct hci_rp_read_local_pairing_opts {
 	__u8     max_key_size;
 } __packed;
 
+#define HCI_OP_READ_LOCAL_CODEC_CAPS	0x100e
+struct hci_op_read_local_codec_caps {
+	__u8	id;
+	__le16	cid;
+	__le16	vid;
+	__u8	transport;
+	__u8	direction;
+} __packed;
+
+struct hci_codec_caps {
+	__u8	len;
+	__u8	data[];
+} __packed;
+
+struct hci_rp_read_local_codec_caps {
+	__u8	status;
+	__u8	num_caps;
+} __packed;
+
 #define HCI_OP_READ_PAGE_SCAN_ACTIVITY	0x0c1b
 struct hci_rp_read_page_scan_activity {
 	__u8     status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 43b08bebae74..cdc9580ff264 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -131,6 +131,17 @@ struct bdaddr_list {
 	u8 bdaddr_type;
 };
 
+struct codec_list {
+	struct list_head list;
+	u8	id;
+	__le16	cid;
+	__le16	vid;
+	u8	transport;
+	u8	num_caps;
+	u32	len;
+	struct hci_codec_caps caps[];
+};
+
 struct bdaddr_list_with_irk {
 	struct list_head list;
 	bdaddr_t bdaddr;
@@ -535,6 +546,7 @@ struct hci_dev {
 	struct list_head	pend_le_conns;
 	struct list_head	pend_le_reports;
 	struct list_head	blocked_keys;
+	struct list_head	local_codecs;
 
 	struct hci_dev_stats	stat;
 
@@ -1849,4 +1861,9 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
 #define SCO_AIRMODE_CVSD       0x0000
 #define SCO_AIRMODE_TRANSP     0x0003
 
+#define LOCAL_CODEC_ACL_MASK	BIT(0)
+#define LOCAL_CODEC_SCO_MASK	BIT(1)
+
+#define TRANSPORT_TYPE_MAX	0x04
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6eedf334f943..b74de5996a27 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -838,10 +838,6 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
 	if (hdev->commands[22] & 0x04)
 		hci_set_event_mask_page_2(req);
 
-	/* Read local codec list if the HCI command is supported */
-	if (hdev->commands[29] & 0x20)
-		hci_req_add(req, HCI_OP_READ_LOCAL_CODECS, 0, NULL);
-
 	/* Read local pairing options if the HCI command is supported */
 	if (hdev->commands[41] & 0x08)
 		hci_req_add(req, HCI_OP_READ_LOCAL_PAIRING_OPTS, 0, NULL);
@@ -907,6 +903,195 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
 	return 0;
 }
 
+static int hci_codec_list_add(struct list_head *list,
+			      struct hci_op_read_local_codec_caps *sent,
+			      struct hci_rp_read_local_codec_caps *rp,
+			      void *caps,
+			      __u32 len)
+{
+	struct codec_list *entry;
+
+	entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->id = sent->id;
+	if (sent->id == 0xFF) {
+		entry->cid = __le16_to_cpu(sent->cid);
+		entry->vid = __le16_to_cpu(sent->vid);
+	}
+	entry->transport = sent->transport;
+	entry->len = len;
+	entry->num_caps = rp->num_caps;
+	if (rp->num_caps)
+		memcpy(entry->caps, caps, len);
+	list_add(&entry->list, list);
+
+	return 0;
+}
+
+static void hci_codec_list_clear(struct list_head *codec_list)
+{
+	struct codec_list *c, *n;
+
+	list_for_each_entry_safe(c, n, codec_list, list) {
+		list_del(&c->list);
+		kfree(c);
+	}
+}
+
+static void hci_read_codec_capabilities(struct hci_dev *hdev, void *codec_id,
+					__u8 transport, bool is_vendor_codec)
+{
+	struct hci_op_read_local_codec_caps cmd;
+	__u8 i;
+
+	memset(&cmd, 0, sizeof(cmd));
+
+	if (is_vendor_codec) {
+		struct hci_ven_codec *ven_codec;
+
+		ven_codec = codec_id;
+		cmd.id = 0xFF;
+		cmd.cid = ven_codec->cid;
+		cmd.vid = ven_codec->vid;
+	} else {
+		cmd.id = *(__u8 *)codec_id;
+	}
+
+	cmd.direction = 0x00;
+
+	for (i = 0; i < TRANSPORT_TYPE_MAX; i++) {
+		if (transport & BIT(i)) {
+			struct hci_rp_read_local_codec_caps *rp;
+			struct hci_codec_caps *caps;
+			struct sk_buff *skb;
+			__u8 j;
+			__u32 len;
+
+			cmd.transport = i;
+			skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
+					     sizeof(cmd), &cmd,
+					     HCI_CMD_TIMEOUT);
+			if (IS_ERR(skb)) {
+				bt_dev_err(hdev, "Failed to read codec capabilities (%ld)",
+					   PTR_ERR(skb));
+				continue;
+			}
+
+			if (skb->len < sizeof(*rp))
+				goto error;
+
+			rp = (void *)skb->data;
+
+			if (rp->status)
+				goto error;
+
+			if (!rp->num_caps) {
+				len = 0;
+				/* this codec doesn't have capabilities */
+				goto skip_caps_parse;
+			}
+
+			skb_pull(skb, sizeof(*rp));
+
+			for (j = 0, len = 0; j < rp->num_caps; j++) {
+				caps = (void *)skb->data;
+				if (skb->len < sizeof(*caps))
+					goto error;
+				if (skb->len < caps->len)
+					goto error;
+				len += sizeof(caps->len) + caps->len;
+				skb_pull(skb,  sizeof(caps->len) + caps->len);
+			}
+
+skip_caps_parse:
+			hci_dev_lock(hdev);
+			hci_codec_list_add(&hdev->local_codecs, &cmd, rp,
+					   (__u8 *)rp + sizeof(*rp), len);
+			hci_dev_unlock(hdev);
+error:
+			kfree_skb(skb);
+		}
+	}
+}
+
+static void hci_codec_list_parse(struct hci_dev *hdev, __u8 num_codecs,
+				 void *codec_list, bool is_vendor_codec)
+{
+	__u8 i;
+
+	for (i = 0; i < num_codecs; i++) {
+		if (!is_vendor_codec) {
+			struct hci_std_codecs *codecs = codec_list;
+
+			hci_read_codec_capabilities(hdev, &codecs->codec[i],
+						    LOCAL_CODEC_ACL_MASK,
+						    is_vendor_codec);
+		} else {
+			struct hci_ven_codecs *codecs = codec_list;
+
+			hci_read_codec_capabilities(hdev, &codecs->codec[i],
+						    LOCAL_CODEC_ACL_MASK,
+						    is_vendor_codec);
+		}
+	}
+}
+
+static void hci_read_supported_codecs(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	struct hci_rp_read_local_supported_codecs *rp;
+	struct hci_std_codecs *std_codecs;
+	struct hci_ven_codecs *ven_codecs;
+
+	skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODECS, 0, NULL,
+			     HCI_CMD_TIMEOUT);
+
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
+			   PTR_ERR(skb));
+		return;
+	}
+
+	if (skb->len < sizeof(*rp))
+		goto error;
+
+	rp = (void *)skb->data;
+
+	if (rp->status)
+		goto error;
+
+	skb_pull(skb, sizeof(rp->status));
+
+	std_codecs = (void *)skb->data;
+
+	/* validate codecs length before accessing */
+	if (skb->len < flex_array_size(std_codecs, codec, std_codecs->num)
+	    + sizeof(std_codecs->num))
+		goto error;
+
+	/* enumerate codec capabilities of standard codecs */
+	hci_codec_list_parse(hdev, std_codecs->num, std_codecs, false);
+
+	skb_pull(skb, flex_array_size(std_codecs, codec, std_codecs->num)
+		 + sizeof(std_codecs->num));
+
+	ven_codecs = (void *)skb->data;
+
+	/* validate vendor codecs length before accessing */
+	if (skb->len <
+	    flex_array_size(ven_codecs, codec, ven_codecs->num)
+	    + sizeof(ven_codecs->num))
+		goto error;
+
+	/* enumerate vendor codec capabilities */
+	hci_codec_list_parse(hdev, ven_codecs->num, ven_codecs, true);
+
+error:
+	kfree_skb(skb);
+}
+
 static int __hci_init(struct hci_dev *hdev)
 {
 	int err;
@@ -937,6 +1122,10 @@ static int __hci_init(struct hci_dev *hdev)
 	if (err < 0)
 		return err;
 
+	/* Read local codec list if the HCI command is supported */
+	if (hdev->commands[29] & 0x20)
+		hci_read_supported_codecs(hdev);
+
 	/* This function is only called when the controller is actually in
 	 * configured state. When the controller is marked as unconfigured,
 	 * this initialization procedure is not run.
@@ -1836,6 +2025,7 @@ int hci_dev_do_close(struct hci_dev *hdev)
 	memset(hdev->eir, 0, sizeof(hdev->eir));
 	memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
 	bacpy(&hdev->random_addr, BDADDR_ANY);
+	hci_codec_list_clear(&hdev->local_codecs);
 
 	hci_req_sync_unlock(hdev);
 
@@ -3837,6 +4027,7 @@ struct hci_dev *hci_alloc_dev(void)
 	INIT_LIST_HEAD(&hdev->conn_hash.list);
 	INIT_LIST_HEAD(&hdev->adv_instances);
 	INIT_LIST_HEAD(&hdev->blocked_keys);
+	INIT_LIST_HEAD(&hdev->local_codecs);
 
 	INIT_WORK(&hdev->rx_work, hci_rx_work);
 	INIT_WORK(&hdev->cmd_work, hci_cmd_work);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 2/9] Bluetooth: Add support for Read Local Supported Codecs V2
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-06-03 14:24   ` Marcel Holtmann
  2021-05-18 10:42 ` [PATCH v8 3/9] Bluetooth: btintel: Add a quirk for hfp offload usecase Kiran K
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

Use V2 version of read local supported command is controller
supports

snoop:
> HCI Event: Command Complete (0x0e) plen 20
      Read Local Supported Codecs V2 (0x04|0x000d) ncmd 1
        Status: Success (0x00)
        Number of supported codecs: 7
          Codec: u-law log (0x00)
          Logical Transport Type: 0x02
            Codec supported over BR/EDR SCO and eSCO
          Codec: A-law log (0x01)
          Logical Transport Type: 0x02
            Codec supported over BR/EDR SCO and eSCO
          Codec: CVSD (0x02)
          Logical Transport Type: 0x02
            Codec supported over BR/EDR SCO and eSCO
          Codec: Transparent (0x03)
          Logical Transport Type: 0x02
            Codec supported over BR/EDR SCO and eSCO
          Codec: Linear PCM (0x04)
          Logical Transport Type: 0x02
            Codec supported over BR/EDR SCO and eSCO
          Codec: Reserved (0x08)
          Logical Transport Type: 0x03
            Codec supported over BR/EDR ACL
            Codec supported over BR/EDR SCO and eSCO
          Codec: mSBC (0x05)
          Logical Transport Type: 0x03
            Codec supported over BR/EDR ACL
            Codec supported over BR/EDR SCO and eSCO
        Number of vendor codecs: 0
......
< HCI Command: Read Local Suppor.. (0x04|0x000e) plen 7
        Codec: mSBC (0x05)
        Logical Transport Type: 0x00
        Direction: Input (Host to Controller) (0x00)
> HCI Event: Command Complete (0x0e) plen 12
      Read Local Supported Codec Capabilities (0x04|0x000e) ncmd 1
        Status: Success (0x00)
        Number of codec capabilities: 1
         Capabilities #0:
        00 00 11 15 02 33

Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
* changes in v8:
  no changes

* changes in v7:
  call codec enumeration code in hci_init instead of having it in a separate
  function

* changes in v6
  no changes

* changes in v5:
  fix review comments

* changes in v4:
  converts codec read capabilities calls from async to sync

* changes in v3:
  No changes

* changes in v2:
  add length check for event data before accessing

 include/net/bluetooth/hci.h | 29 ++++++++++++++
 net/bluetooth/hci_core.c    | 78 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 6cb9340a2d51..08508b3d13b4 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1337,6 +1337,35 @@ struct hci_rp_read_local_pairing_opts {
 	__u8     max_key_size;
 } __packed;
 
+#define HCI_OP_READ_LOCAL_CODECS_V2	0x100d
+struct hci_std_codec_v2 {
+	__u8	id;
+	__u8	transport;
+} __packed;
+
+struct hci_std_codecs_v2 {
+	__u8	num;
+	struct hci_std_codec_v2 codec[];
+} __packed;
+
+struct hci_ven_codec_v2 {
+	__u8	id;
+	__le16	cid;
+	__le16	vid;
+	__u8	transport;
+} __packed;
+
+struct hci_ven_codecs_v2 {
+	__u8	num;
+	struct hci_ven_codec_v2 codec[];
+} __packed;
+
+struct hci_rp_read_local_supported_codecs_v2 {
+	__u8	status;
+	struct hci_std_codecs_v2 std_codecs;
+	struct hci_ven_codecs_v2 vendor_codecs;
+} __packed;
+
 #define HCI_OP_READ_LOCAL_CODEC_CAPS	0x100e
 struct hci_op_read_local_codec_caps {
 	__u8	id;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b74de5996a27..5915d05b0e6f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1038,6 +1038,28 @@ static void hci_codec_list_parse(struct hci_dev *hdev, __u8 num_codecs,
 	}
 }
 
+static void hci_codec_list_parse_v2(struct hci_dev *hdev, __u8 num_codecs,
+				    void *codec_list, bool is_vendor_codec)
+{
+	__u8 i;
+
+	for (i = 0; i < num_codecs; i++) {
+		if (!is_vendor_codec) {
+			struct hci_std_codecs_v2 *codecs = codec_list;
+
+			hci_read_codec_capabilities(hdev, &codecs->codec[i],
+						    codecs->codec[i].transport,
+						    is_vendor_codec);
+		} else {
+			struct hci_ven_codecs_v2 *codecs = codec_list;
+
+			hci_read_codec_capabilities(hdev, &codecs->codec[i],
+						    codecs->codec[i].transport,
+						    is_vendor_codec);
+		}
+	}
+}
+
 static void hci_read_supported_codecs(struct hci_dev *hdev)
 {
 	struct sk_buff *skb;
@@ -1092,6 +1114,58 @@ static void hci_read_supported_codecs(struct hci_dev *hdev)
 	kfree_skb(skb);
 }
 
+static void hci_read_supported_codecs_v2(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	struct hci_rp_read_local_supported_codecs_v2 *rp;
+	struct hci_std_codecs_v2 *std_codecs;
+	struct hci_ven_codecs_v2 *ven_codecs;
+
+	skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODECS_V2, 0, NULL,
+			     HCI_CMD_TIMEOUT);
+
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
+			   PTR_ERR(skb));
+		return;
+	}
+
+	if (skb->len < sizeof(*rp))
+		goto error;
+
+	rp = (void *)skb->data;
+
+	if (rp->status)
+		goto error;
+
+	skb_pull(skb, sizeof(rp->status));
+
+	std_codecs = (void *)skb->data;
+
+	/* check for payload data length before accessing */
+	if (skb->len < flex_array_size(std_codecs, codec, std_codecs->num)
+	    + sizeof(std_codecs->num))
+		goto error;
+
+	hci_codec_list_parse_v2(hdev, std_codecs->num, std_codecs, false);
+
+	skb_pull(skb, flex_array_size(std_codecs, codec, std_codecs->num)
+		 + sizeof(std_codecs->num));
+
+	ven_codecs = (void *)skb->data;
+
+	/* check for payload data length before accessing */
+	if (skb->len <
+	    flex_array_size(ven_codecs, codec, ven_codecs->num)
+	    + sizeof(ven_codecs->num))
+		goto error;
+
+	hci_codec_list_parse_v2(hdev, ven_codecs->num, ven_codecs, true);
+
+error:
+	kfree_skb(skb);
+}
+
 static int __hci_init(struct hci_dev *hdev)
 {
 	int err;
@@ -1123,7 +1197,9 @@ static int __hci_init(struct hci_dev *hdev)
 		return err;
 
 	/* Read local codec list if the HCI command is supported */
-	if (hdev->commands[29] & 0x20)
+	if (hdev->commands[45] & 0x04)
+		hci_read_supported_codecs_v2(hdev);
+	else if (hdev->commands[29] & 0x20)
 		hci_read_supported_codecs(hdev);
 
 	/* This function is only called when the controller is actually in
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 3/9] Bluetooth: btintel: Add a quirk for hfp offload usecase
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
  2021-05-18 10:42 ` [PATCH v8 2/9] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-06-03 14:26   ` Marcel Holtmann
  2021-05-18 10:42 ` [PATCH v8 4/9] Bluetooth: btitnel: Add a callback function to retireve data path Kiran K
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

Define a quirk to identify if intel controllers supports offload for
HFP. In *setup* function, driver sends vendor specific command to
check if controller supports offload. If offload is supports then
quirk flag is set

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 | 28 ++++++++++++++++++++++++++++
 drivers/bluetooth/btintel.h |  5 +++++
 drivers/bluetooth/btusb.c   |  2 ++
 include/net/bluetooth/hci.h |  7 +++++++
 4 files changed, 42 insertions(+)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e44b6993cf91..e3ad19244054 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -32,6 +32,11 @@ struct cmd_write_boot_params {
 	u8  fw_build_yy;
 } __packed;
 
+struct intel_offload_usecases {
+	__u8	status;
+	__u8	preset[8];
+} __packed;
+
 int btintel_check_bdaddr(struct hci_dev *hdev)
 {
 	struct hci_rp_read_bd_addr *bda;
@@ -1272,6 +1277,29 @@ int btintel_set_debug_features(struct hci_dev *hdev,
 }
 EXPORT_SYMBOL_GPL(btintel_set_debug_features);
 
+int btintel_read_offload_usecases(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	struct intel_offload_usecases *usecases;
+
+	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);
+	}
+	usecases = (void *)skb->data;
+	if (usecases->status)
+		goto error;
+
+	if (usecases->preset[0] & 0x03)
+		set_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks);
+error:
+	kfree_skb(skb);
+	return 0;
+}
+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..d561d4899b1b 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -175,6 +175,7 @@ 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);
 #else
 
 static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -307,4 +308,8 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
 	return -EOPNOTSUPP;
 }
 
+static int btintel_read_offload_usecases(struct hci_dev *hdev)
+{
+	return -EOPNOTSUPP;
+}
 #endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5245714dc6d0..ac245df5fa18 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2993,6 +2993,8 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
 	/* Set DDC mask for available debug features */
 	btintel_set_debug_features(hdev, &features);
 
+	btintel_read_offload_usecases(hdev);
+
 	/* Read the Intel version information after loading the FW  */
 	err = btintel_read_version_tlv(hdev, &version);
 	if (err)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 08508b3d13b4..731d48ca873a 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,13 @@ enum {
 	 * HCI after resume.
 	 */
 	HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+	/* When this quirk is set, then controller supports offload codecs
+	 * for HFP.
+	 * This quirk can be set before hci_register_dev is called or
+	 * during the hdev->setup vendor callback.
+	 */
+	HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED,
 };
 
 /* HCI device flags */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 4/9] Bluetooth: btitnel: Add a callback function to retireve data path
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
  2021-05-18 10:42 ` [PATCH v8 2/9] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
  2021-05-18 10:42 ` [PATCH v8 3/9] Bluetooth: btintel: Add a quirk for hfp offload usecase Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-06-03 14:29   ` Marcel Holtmann
  2021-05-18 10:42 ` [PATCH v8 5/9] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

There is no standard HCI command to retrieve data path for transport.
Add a new callback function to retrieve data path which is used
in offload usecase.

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

This callback gets called when audio module queries codecs suppoted
on SCO socket. For Intel controllers, data_path is always 1

 drivers/bluetooth/btintel.c      | 8 ++++++++
 drivers/bluetooth/btintel.h      | 6 ++++++
 drivers/bluetooth/btusb.c        | 1 +
 include/net/bluetooth/hci_core.h | 1 +
 4 files changed, 16 insertions(+)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e3ad19244054..8407e9736c62 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1300,6 +1300,14 @@ int btintel_read_offload_usecases(struct hci_dev *hdev)
 }
 EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
 
+int btintel_get_data_path_id(struct hci_dev *hdev)
+{
+	if (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks))
+		return -EOPNOTSUPP;
+	return 1;
+}
+EXPORT_SYMBOL_GPL(btintel_get_data_path_id);
+
 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 d561d4899b1b..6d4edfd16d44 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -176,6 +176,7 @@ int btintel_read_debug_features(struct hci_dev *hdev,
 int btintel_set_debug_features(struct hci_dev *hdev,
 			       const struct intel_debug_features *features);
 int btintel_read_offload_usecases(struct hci_dev *hdev);
+int btintel_get_data_path_id(struct hci_dev *hdev);
 #else
 
 static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -312,4 +313,9 @@ static int btintel_read_offload_usecases(struct hci_dev *hdev)
 {
 	return -EOPNOTSUPP;
 }
+
+static int btintel_get_data_path_id(struct hci_dev *hdev)
+{
+	return -EOPNOTSUPP;
+}
 #endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index ac245df5fa18..f7b349db392a 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4631,6 +4631,7 @@ static int btusb_probe(struct usb_interface *intf,
 		hdev->set_diag = btintel_set_diag;
 		hdev->set_bdaddr = btintel_set_bdaddr;
 		hdev->cmd_timeout = btusb_intel_cmd_timeout;
+		hdev->get_data_path_id = btintel_get_data_path_id;
 		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
 		set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 		set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index cdc9580ff264..78d1ebab58f6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -617,6 +617,7 @@ struct hci_dev {
 	int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
 	void (*cmd_timeout)(struct hci_dev *hdev);
 	bool (*prevent_wake)(struct hci_dev *hdev);
+	int (*get_data_path_id)(struct hci_dev *hdev);
 };
 
 #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 5/9] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
                   ` (2 preceding siblings ...)
  2021-05-18 10:42 ` [PATCH v8 4/9] Bluetooth: btitnel: Add a callback function to retireve data path Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-06-03 14:35   ` Marcel Holtmann
  2021-05-18 10:42 ` [PATCH v8 6/9] Bluetooth: btintel: Add a callback function to configure data path Kiran K
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

Add BT_CODEC option for getsockopt systemcall over SCO socket
to expose the codecs supported by controller

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>
---
 include/net/bluetooth/bluetooth.h |  20 ++++++
 include/net/bluetooth/hci.h       |   4 ++
 net/bluetooth/sco.c               | 109 ++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 9125effbf448..1840756958ce 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -153,6 +153,26 @@ struct bt_voice {
 
 #define BT_SCM_PKT_STATUS	0x03
 
+#define BT_CODEC	19
+
+struct	bt_codec_caps {
+	__u8	len;
+	__u8	data[];
+} __packed;
+
+struct bt_codec {
+	__u8	id;
+	__le16	cid;
+	__le16	vid;
+	__u8	data_path;
+	__u8	num_caps;
+} __packed;
+
+struct bt_codecs {
+	__u8		num_codecs;
+	struct bt_codec	codecs[];
+} __packed;
+
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 731d48ca873a..9658600ae5de 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2626,6 +2626,10 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
 #define hci_iso_data_len(h)		((h) & 0x3fff)
 #define hci_iso_data_flags(h)		((h) >> 14)
 
+/* codec transport types */
+#define TRANSPORT_BREDR		0x00
+#define TRANSPORT_SCO_ESCO	0x01
+
 /* le24 support */
 static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3])
 {
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 3bd41563f118..d66293d1cba4 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -948,6 +948,11 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
 	struct bt_voice voice;
 	u32 phys;
 	int pkt_status;
+	struct codec_list *c;
+	u8 num_codecs, i, __user *ptr;
+	struct hci_dev *hdev;
+	struct hci_codec_caps *caps;
+	__u8	data_path;
 
 	BT_DBG("sk %p", sk);
 
@@ -1012,6 +1017,110 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
 			err = -EFAULT;
 		break;
 
+	case BT_CODEC:
+		num_codecs = 0;
+		len = 0;
+
+		hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
+
+		if (!hdev) {
+			err = -EBADFD;
+			break;
+		}
+
+		if (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks)) {
+			err = -EOPNOTSUPP;
+			break;
+		}
+
+		if (!hdev->get_data_path_id) {
+			err = -EOPNOTSUPP;
+			break;
+		}
+
+		hci_dev_lock(hdev);
+		list_for_each_entry(c, &hdev->local_codecs, list) {
+			if (c->transport != TRANSPORT_SCO_ESCO)
+				continue;
+			num_codecs++;
+			for (i = 0, caps = c->caps; i < c->num_caps; i++) {
+				len += 1 + caps->len;
+				caps = (void *)&caps->data[caps->len];
+			}
+			len += sizeof(struct bt_codec);
+		}
+		hci_dev_unlock(hdev);
+
+		if (len > 255) {
+			err = -ENOMEM;
+			break;
+		}
+
+		ptr = optval;
+		if (put_user(num_codecs, ptr)) {
+			err = -EFAULT;
+			break;
+		}
+		ptr += sizeof(num_codecs);
+
+		hci_dev_lock(hdev);
+		list_for_each_entry(c, &hdev->local_codecs, list) {
+			if (c->transport != TRANSPORT_SCO_ESCO)
+				continue;
+
+			if (put_user(c->id, ptr)) {
+				err = -EFAULT;
+				goto unlock;
+			}
+			ptr += sizeof(c->id);
+
+			if (put_user(c->cid, ptr)) {
+				err = -EFAULT;
+				goto unlock;
+			}
+			ptr += sizeof(c->cid);
+
+			if (put_user(c->vid, ptr)) {
+				err = -EFAULT;
+				goto unlock;
+			}
+			ptr += sizeof(c->vid);
+
+			err = hdev->get_data_path_id(hdev);
+			if (err < 0) {
+				err = -EFAULT;
+				goto unlock;
+			}
+			data_path = (__u8)err;
+			if (put_user(data_path, ptr)) {
+				err = -EFAULT;
+				goto unlock;
+			}
+			ptr += sizeof(data_path);
+
+			if (put_user(c->num_caps, ptr)) {
+				err = -EFAULT;
+				goto unlock;
+			}
+			ptr += sizeof(c->num_caps);
+
+			len = 0;
+			for (i = 0, caps = c->caps; i < c->num_caps; i++) {
+				len += 1 + caps->len;
+				caps = (void *)&caps->data[caps->len];
+			}
+
+			if (len && copy_to_user(ptr, c->caps, len)) {
+				err = -EFAULT;
+				goto unlock;
+			}
+			ptr += len;
+		}
+unlock:
+		hci_dev_unlock(hdev);
+
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 6/9] Bluetooth: btintel: Add a callback function to configure data path
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
                   ` (3 preceding siblings ...)
  2021-05-18 10:42 ` [PATCH v8 5/9] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-06-03 14:30   ` Marcel Holtmann
  2021-05-18 10:42 ` [PATCH v8 7/9] Bluetooth: Add BT_CODEC option for setsockopt over SCO Kiran K
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

In HFP offload usecase, Intel controllers require offload use
case id (NBS or WBS) to be set before opening SCO connection. Define
a new callback which gets called on setsockopt SCO socket. User space
audio module is expected to set codec via setsockopt(sk, BT_CODEC, ....)
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>
---

This callback gets called when audio module does setsockopt(sk, BT_CODEC,...)
on SCO socket and data_path is 1 (non-HCI transport). For non-HCI transport,
Intel controller expects presets to be set before opening SCO connection.
Presets are pre-defined, 0 - NBS, 1 - WBS. Likewise additional presets will
be defined for A2DP, LE Audio offload use cases.

 drivers/bluetooth/btintel.c      | 50 ++++++++++++++++++++++++++++++++
 drivers/bluetooth/btintel.h      |  8 +++++
 drivers/bluetooth/btusb.c        |  1 +
 include/net/bluetooth/hci.h      |  8 +++++
 include/net/bluetooth/hci_core.h |  2 ++
 5 files changed, 69 insertions(+)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 8407e9736c62..8efb15504973 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1308,6 +1308,56 @@ int btintel_get_data_path_id(struct hci_dev *hdev)
 }
 EXPORT_SYMBOL_GPL(btintel_get_data_path_id);
 
+int btintel_configure_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_configure_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 6d4edfd16d44..f4af22ecdc0d 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -177,6 +177,8 @@ int btintel_set_debug_features(struct hci_dev *hdev,
 			       const struct intel_debug_features *features);
 int btintel_read_offload_usecases(struct hci_dev *hdev);
 int btintel_get_data_path_id(struct hci_dev *hdev);
+int btintel_configure_data_path(struct hci_dev *hdev, __u8 type,
+				struct bt_codec *codec);
 #else
 
 static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -318,4 +320,10 @@ static int btintel_get_data_path_id(struct hci_dev *hdev)
 {
 	return -EOPNOTSUPP;
 }
+
+static int btintel_configure_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 f7b349db392a..220e7c85db10 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4632,6 +4632,7 @@ static int btusb_probe(struct usb_interface *intf,
 		hdev->set_bdaddr = btintel_set_bdaddr;
 		hdev->cmd_timeout = btusb_intel_cmd_timeout;
 		hdev->get_data_path_id = btintel_get_data_path_id;
+		hdev->configure_data_path = btintel_configure_data_path;
 		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
 		set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 		set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 9658600ae5de..bd666b114aea 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1257,6 +1257,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;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 78d1ebab58f6..959585bafa8d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -618,6 +618,8 @@ struct hci_dev {
 	void (*cmd_timeout)(struct hci_dev *hdev);
 	bool (*prevent_wake)(struct hci_dev *hdev);
 	int (*get_data_path_id)(struct hci_dev *hdev);
+	int (*configure_data_path)(struct hci_dev *hdev, __u8 type,
+				   struct bt_codec *codec);
 };
 
 #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 7/9] Bluetooth: Add BT_CODEC option for setsockopt over SCO
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
                   ` (4 preceding siblings ...)
  2021-05-18 10:42 ` [PATCH v8 6/9] Bluetooth: btintel: Add a callback function to configure data path Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-06-03 14:37   ` Marcel Holtmann
  2021-05-18 10:42 ` [PATCH v8 8/9] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command Kiran K
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

Add BT_CODEC option on setsockopt system call to allow user space
audio modules to set codec. Driver also configures codec if non-HCI
data is selected.

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>
---
 include/net/bluetooth/bluetooth.h |  2 +
 net/bluetooth/sco.c               | 63 +++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 1840756958ce..0e8802d09068 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -173,6 +173,8 @@ struct bt_codecs {
 	struct bt_codec	codecs[];
 } __packed;
 
+#define CODING_FORMAT_CVSD	0x02
+
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d66293d1cba4..d59f30fc4b9f 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -67,6 +67,7 @@ struct sco_pinfo {
 	__u32		flags;
 	__u16		setting;
 	__u8		cmsg_mask;
+	struct bt_codec codec;
 	struct sco_conn	*conn;
 };
 
@@ -438,6 +439,7 @@ static void __sco_sock_close(struct sock *sk)
 		sock_set_flag(sk, SOCK_ZAPPED);
 		break;
 	}
+
 }
 
 /* Must be called on unlocked socket. */
@@ -499,6 +501,10 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock,
 	sk->sk_state    = BT_OPEN;
 
 	sco_pi(sk)->setting = BT_VOICE_CVSD_16BIT;
+	sco_pi(sk)->codec.id = CODING_FORMAT_CVSD;
+	sco_pi(sk)->codec.cid = 0xffff;
+	sco_pi(sk)->codec.vid = 0xffff;
+	sco_pi(sk)->codec.data_path = 0x00;
 
 	timer_setup(&sk->sk_timer, sco_sock_timeout, 0);
 
@@ -808,6 +814,9 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
 	struct sock *sk = sock->sk;
 	int len, err = 0;
 	struct bt_voice voice;
+	struct bt_codecs *codecs;
+	struct hci_dev *hdev;
+	__u8 buffer[255];
 	u32 opt;
 
 	BT_DBG("sk %p", sk);
@@ -870,6 +879,60 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
 			sco_pi(sk)->cmsg_mask &= SCO_CMSG_PKT_STATUS;
 		break;
 
+	case BT_CODEC:
+		if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND &&
+		    sk->sk_state != BT_CONNECT2) {
+			err = -EINVAL;
+			break;
+		}
+
+		hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
+		if (!hdev) {
+			err = -EBADFD;
+			break;
+		}
+
+		if (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks)) {
+			err = -EOPNOTSUPP;
+			break;
+		}
+
+		if (optlen < sizeof(struct bt_codecs) || optlen > 255) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (copy_from_sockptr(buffer, optval, optlen)) {
+			err = -EFAULT;
+			break;
+		}
+
+		codecs = (void *)buffer;
+
+		if (codecs->num_codecs > 1) {
+			err = -EINVAL;
+			break;
+		}
+
+		if (codecs->codecs[0].data_path) {
+			if (!hdev->configure_data_path) {
+				err = -EOPNOTSUPP;
+				break;
+			}
+			err = hdev->configure_data_path(hdev, SCO_LINK,
+							codecs->codecs);
+			if (err < 0)
+				break;
+
+			if (codecs->codecs[0].id == 0xff) {
+				sco_pi(sk)->codec.cid = codecs->codecs[0].cid;
+				sco_pi(sk)->codec.vid = codecs->codecs[0].vid;
+			}
+		}
+		sco_pi(sk)->codec.id = codecs->codecs[0].id;
+		sco_pi(sk)->codec.data_path = codecs->codecs[0].data_path;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 8/9] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
                   ` (5 preceding siblings ...)
  2021-05-18 10:42 ` [PATCH v8 7/9] Bluetooth: Add BT_CODEC option for setsockopt over SCO Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-06-03 14:39   ` Marcel Holtmann
  2021-05-18 10:42 ` [PATCH v8 9/9] Bluetooth: Add support for msbc coding format Kiran K
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

< HCI Command: Enhanced Setup Synchronous Connection (0x01|0x003d) plen 59
        Handle: 256
        Transmit bandwidth: 8000
        Receive bandwidth: 8000
        Max latency: 13
        Packet type: 0x0380
          3-EV3 may not be used
          2-EV5 may not be used
          3-EV5 may not be used
        Retransmission effort: Optimize for link quality (0x02)
> HCI Event: Command Status (0x0f) plen 4
      Enhanced Setup Synchronous Connection (0x01|0x003d) ncmd 1
        Status: Success (0x00)
> HCI Event: Synchronous Connect Complete (0x2c) plen 17
        Status: Success (0x00)
        Handle: 257
        Address: CC:98:8B:92:04:FD (SONY Visual Products Inc.)
        Link type: eSCO (0x02)
        Transmission interval: 0x0c
        Retransmission window: 0x06
        RX packet length: 60
        TX packet length: 60
        Air mode: Transparent (0x03)

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

Need implementation HCI_Enhanced_Setup_Synchronous_Command as it allows
setting of non-HCI data path

 include/net/bluetooth/bluetooth.h |   1 +
 include/net/bluetooth/hci.h       |  34 ++++++++++
 include/net/bluetooth/hci_core.h  |   7 +-
 net/bluetooth/hci_conn.c          | 104 +++++++++++++++++++++++++++++-
 net/bluetooth/hci_event.c         |  48 +++++++++++++-
 net/bluetooth/sco.c               |  11 +++-
 6 files changed, 198 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 0e8802d09068..4e58d275c72c 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -174,6 +174,7 @@ struct bt_codecs {
 } __packed;
 
 #define CODING_FORMAT_CVSD	0x02
+#define CODING_FORMAT_TRANS	0x03
 
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index bd666b114aea..af168f3c1f2e 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -878,6 +878,40 @@ struct hci_cp_logical_link_cancel {
 	__u8     flow_spec_id;
 } __packed;
 
+#define HCI_OP_ENHANCED_SETUP_SYNC_CONN		0x043D
+struct hci_coding_format {
+	__u8	id;
+	__le16	cid;
+	__le16	vid;
+} __packed;
+
+struct hci_cp_enhanced_setup_sync_conn {
+	__le16   handle;
+	__le32   tx_bandwidth;
+	__le32   rx_bandwidth;
+	struct	 hci_coding_format tx_coding_format;
+	struct	 hci_coding_format rx_coding_format;
+	__le16	 tx_codec_frame_size;
+	__le16	 rx_codec_frame_size;
+	__le32	 in_bandwidth;
+	__le32	 out_bandwidth;
+	struct	 hci_coding_format in_coding_format;
+	struct	 hci_coding_format out_coding_format;
+	__le16   in_coded_data_size;
+	__le16	 out_coded_data_size;
+	__u8	 in_pcm_data_format;
+	__u8	 out_pcm_data_format;
+	__u8	 in_pcm_sample_payload_msb_pos;
+	__u8	 out_pcm_sample_payload_msb_pos;
+	__u8	 in_data_path;
+	__u8	 out_data_path;
+	__u8	 in_trasnport_unit_size;
+	__u8	 out_trasnport_unit_size;
+	__le16   max_latency;
+	__le16   pkt_type;
+	__u8     retrans_effort;
+} __packed;
+
 struct hci_rp_logical_link_cancel {
 	__u8     status;
 	__u8     phy_handle;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 959585bafa8d..760d21a2134e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -712,6 +712,7 @@ struct hci_conn {
 	struct amp_mgr	*amp_mgr;
 
 	struct hci_conn	*link;
+	struct bt_codec codec;
 
 	void (*connect_cfm_cb)	(struct hci_conn *conn, u8 status);
 	void (*security_cfm_cb)	(struct hci_conn *conn, u8 status);
@@ -1094,6 +1095,7 @@ static inline struct hci_conn *hci_lookup_le_connect(struct hci_dev *hdev)
 
 int hci_disconnect(struct hci_conn *conn, __u8 reason);
 bool hci_setup_sync(struct hci_conn *conn, __u16 handle);
+bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle);
 void hci_sco_setup(struct hci_conn *conn, __u8 status);
 
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
@@ -1118,7 +1120,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
 				 u8 sec_level, u8 auth_type,
 				 enum conn_reasons conn_reason);
 struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
-				 __u16 setting);
+				 __u16 setting, struct bt_codec *codec);
 int hci_conn_check_link_mode(struct hci_conn *conn);
 int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
 int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type,
@@ -1439,6 +1441,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 /* Use LL Privacy based address resolution if supported */
 #define use_ll_privacy(dev) ((dev)->le_features[0] & HCI_LE_LL_PRIVACY)
 
+/* Use enhanced synchronous connection if command is supported */
+#define use_enhanced_sco_conn(dev) ((dev)->commands[29] & 0x08)
+
 /* Use ext scanning if set ext scan param and ext scan enable is supported */
 #define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
 			   ((dev)->commands[37] & 0x40))
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 88ec08978ff4..e2435b5abeca 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -307,6 +307,96 @@ static bool find_next_esco_param(struct hci_conn *conn,
 	return conn->attempt <= size;
 }
 
+bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
+{
+	struct hci_dev *hdev = conn->hdev;
+	struct hci_cp_enhanced_setup_sync_conn cp;
+	const struct sco_param *param;
+
+	BT_DBG("hcon %p", conn);
+
+	conn->state = BT_CONNECT;
+	conn->out = true;
+
+	conn->attempt++;
+
+	memset(&cp, 0x00, sizeof(cp));
+
+	cp.handle   = cpu_to_le16(handle);
+
+	cp.tx_bandwidth   = cpu_to_le32(0x00001f40);
+	cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
+
+	switch (conn->codec.id) {
+	case CODING_FORMAT_CVSD:
+		if (lmp_esco_capable(conn->link)) {
+			if (conn->attempt > ARRAY_SIZE(esco_param_cvsd))
+				return false;
+			param = &esco_param_cvsd[conn->attempt - 1];
+		} else {
+			if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
+				return false;
+			param = &sco_param_cvsd[conn->attempt - 1];
+		}
+		cp.tx_coding_format.id = 2;
+		cp.rx_coding_format.id = 2;
+		cp.tx_codec_frame_size = __cpu_to_le16(60);
+		cp.rx_codec_frame_size = __cpu_to_le16(60);
+		cp.in_bandwidth = __cpu_to_le32(16000);
+		cp.out_bandwidth = __cpu_to_le32(16000);
+		cp.in_coding_format.id = 4;
+		cp.out_coding_format.id = 4;
+		cp.in_coded_data_size = __cpu_to_le16(16);
+		cp.out_coded_data_size = __cpu_to_le16(16);
+		cp.in_pcm_data_format = 2;
+		cp.out_pcm_data_format = 2;
+		cp.in_pcm_sample_payload_msb_pos = 0;
+		cp.out_pcm_sample_payload_msb_pos = 0;
+		cp.in_data_path = conn->codec.data_path;
+		cp.out_data_path = conn->codec.data_path;
+		cp.in_trasnport_unit_size = 16;
+		cp.out_trasnport_unit_size = 16;
+		break;
+
+	case CODING_FORMAT_TRANS:
+		if (conn->attempt > ARRAY_SIZE(esco_param_msbc))
+			return false;
+
+		param = &esco_param_msbc[conn->attempt - 1];
+		cp.tx_coding_format.id = 0x03;
+		cp.rx_coding_format.id = 0x03;
+		cp.tx_codec_frame_size = __cpu_to_le16(60);
+		cp.rx_codec_frame_size = __cpu_to_le16(60);
+		cp.in_bandwidth = __cpu_to_le32(0x1f40);
+		cp.out_bandwidth = __cpu_to_le32(0x1f40);
+		cp.in_coding_format.id = 0x03;
+		cp.out_coding_format.id = 0x03;
+		cp.in_coded_data_size = __cpu_to_le16(16);
+		cp.out_coded_data_size = __cpu_to_le16(16);
+		cp.in_pcm_data_format = 2;
+		cp.out_pcm_data_format = 2;
+		cp.in_pcm_sample_payload_msb_pos = 0;
+		cp.out_pcm_sample_payload_msb_pos = 0;
+		cp.in_data_path = conn->codec.data_path;
+		cp.out_data_path = conn->codec.data_path;
+		cp.in_trasnport_unit_size = 1;
+		cp.out_trasnport_unit_size = 1;
+		break;
+
+	default:
+		return false;
+	}
+
+	cp.retrans_effort = param->retrans_effort;
+	cp.pkt_type = __cpu_to_le16(param->pkt_type);
+	cp.max_latency = __cpu_to_le16(param->max_latency);
+
+	if (hci_send_cmd(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN, sizeof(cp), &cp) < 0)
+		return false;
+
+	return true;
+}
+
 bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -424,8 +514,12 @@ void hci_sco_setup(struct hci_conn *conn, __u8 status)
 	BT_DBG("hcon %p", conn);
 
 	if (!status) {
-		if (lmp_esco_capable(conn->hdev))
-			hci_setup_sync(sco, conn->handle);
+		if (lmp_esco_capable(conn->hdev)) {
+			if (use_enhanced_sco_conn(conn->hdev))
+				hci_enhanced_setup_sync(sco, conn->handle);
+			else
+				hci_setup_sync(sco, conn->handle);
+		}
 		else
 			hci_add_sco(sco, conn->handle);
 	} else {
@@ -1319,7 +1413,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
 }
 
 struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
-				 __u16 setting)
+				 __u16 setting, struct bt_codec *codec)
 {
 	struct hci_conn *acl;
 	struct hci_conn *sco;
@@ -1344,6 +1438,10 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
 	hci_conn_hold(sco);
 
 	sco->setting = setting;
+	sco->codec.id = codec->id;
+	sco->codec.cid = codec->cid;
+	sco->codec.vid = codec->vid;
+	sco->codec.data_path = codec->data_path;
 
 	if (acl->state == BT_CONNECTED &&
 	    (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4241ae310fcb..94c64e2c11e4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2236,6 +2236,41 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
 	hci_dev_unlock(hdev);
 }
 
+static void hci_cs_enhanced_setup_sync_conn(struct hci_dev *hdev, __u8 status)
+{
+	struct hci_cp_enhanced_setup_sync_conn *cp;
+	struct hci_conn *acl, *sco;
+	__u16 handle;
+
+	bt_dev_dbg(hdev, "status 0x%2.2x", status);
+
+	if (!status)
+		return;
+
+	cp = hci_sent_cmd_data(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN);
+	if (!cp)
+		return;
+
+	handle = __le16_to_cpu(cp->handle);
+
+	BT_DBG("%s handle 0x%4.4x", hdev->name, handle);
+
+	hci_dev_lock(hdev);
+
+	acl = hci_conn_hash_lookup_handle(hdev, handle);
+	if (acl) {
+		sco = acl->link;
+		if (sco) {
+			sco->state = BT_CLOSED;
+
+			hci_connect_cfm(sco, status);
+			hci_conn_del(sco);
+		}
+	}
+
+	hci_dev_unlock(hdev);
+}
+
 static void hci_cs_sniff_mode(struct hci_dev *hdev, __u8 status)
 {
 	struct hci_cp_sniff_mode *cp;
@@ -3715,6 +3750,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cs_setup_sync_conn(hdev, ev->status);
 		break;
 
+	case HCI_OP_ENHANCED_SETUP_SYNC_CONN:
+		hci_cs_enhanced_setup_sync_conn(hdev, ev->status);
+		break;
+
 	case HCI_OP_SNIFF_MODE:
 		hci_cs_sniff_mode(hdev, ev->status);
 		break;
@@ -4401,8 +4440,13 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
 		if (conn->out) {
 			conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
 					(hdev->esco_type & EDR_ESCO_MASK);
-			if (hci_setup_sync(conn, conn->link->handle))
-				goto unlock;
+			if (use_enhanced_sco_conn(hdev)) {
+				if (hci_enhanced_setup_sync(conn, conn->link->handle))
+					goto unlock;
+			} else {
+				if (hci_setup_sync(conn, conn->link->handle))
+					goto unlock;
+			}
 		}
 		fallthrough;
 
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d59f30fc4b9f..19d8ee11eed5 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -240,7 +240,7 @@ static int sco_connect(struct sock *sk)
 	}
 
 	hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
-			       sco_pi(sk)->setting);
+			       sco_pi(sk)->setting, &sco_pi(sk)->codec);
 	if (IS_ERR(hcon)) {
 		err = PTR_ERR(hcon);
 		goto done;
@@ -865,6 +865,15 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
 		}
 
 		sco_pi(sk)->setting = voice.setting;
+		hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src,
+				     BDADDR_BREDR);
+		if (!hdev) {
+			err = -EBADFD;
+			break;
+		}
+		if (use_enhanced_sco_conn(hdev) &&
+		    voice.setting == BT_VOICE_TRANSPARENT)
+			sco_pi(sk)->codec.id = CODING_FORMAT_TRANS;
 		break;
 
 	case BT_PKT_STATUS:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v8 9/9] Bluetooth: Add support for msbc coding format
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
                   ` (6 preceding siblings ...)
  2021-05-18 10:42 ` [PATCH v8 8/9] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command Kiran K
@ 2021-05-18 10:42 ` Kiran K
  2021-05-18 11:12 ` [v8,1/9] Bluetooth: enumerate local supported codec and cache details bluez.test.bot
  2021-06-03 14:23 ` [PATCH v8 1/9] " Marcel Holtmann
  9 siblings, 0 replies; 22+ messages in thread
From: Kiran K @ 2021-05-18 10:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Kiran K

In Enhanced_Setup_Synchronous_Command, add support for msbc
coding format

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>
---
 include/net/bluetooth/bluetooth.h |  1 +
 net/bluetooth/hci_conn.c          | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 4e58d275c72c..817245d1046f 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -175,6 +175,7 @@ struct bt_codecs {
 
 #define CODING_FORMAT_CVSD	0x02
 #define CODING_FORMAT_TRANS	0x03
+#define CODING_FORMAT_MSBC	0x05
 
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e2435b5abeca..edacf791d663 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -383,6 +383,30 @@ bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
 		cp.out_trasnport_unit_size = 1;
 		break;
 
+	case CODING_FORMAT_MSBC:
+		if (conn->attempt > ARRAY_SIZE(esco_param_msbc))
+			return false;
+
+		param = &esco_param_msbc[conn->attempt - 1];
+		cp.tx_coding_format.id = 0x05;
+		cp.rx_coding_format.id = 0x05;
+		cp.tx_codec_frame_size = __cpu_to_le16(60);
+		cp.rx_codec_frame_size = __cpu_to_le16(60);
+		cp.in_bandwidth = __cpu_to_le32(32000);
+		cp.out_bandwidth = __cpu_to_le32(32000);
+		cp.in_coding_format.id = 0x04;
+		cp.out_coding_format.id = 0x04;
+		cp.in_coded_data_size = __cpu_to_le16(16);
+		cp.out_coded_data_size = __cpu_to_le16(16);
+		cp.in_pcm_data_format = 2;
+		cp.out_pcm_data_format = 2;
+		cp.in_pcm_sample_payload_msb_pos = 0;
+		cp.out_pcm_sample_payload_msb_pos = 0;
+		cp.in_data_path = conn->codec.data_path;
+		cp.out_data_path = conn->codec.data_path;
+		cp.in_trasnport_unit_size = 1;
+		cp.out_trasnport_unit_size = 1;
+		break;
 	default:
 		return false;
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* RE: [v8,1/9] Bluetooth: enumerate local supported codec and cache details
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
                   ` (7 preceding siblings ...)
  2021-05-18 10:42 ` [PATCH v8 9/9] Bluetooth: Add support for msbc coding format Kiran K
@ 2021-05-18 11:12 ` bluez.test.bot
  2021-06-03 14:23 ` [PATCH v8 1/9] " Marcel Holtmann
  9 siblings, 0 replies; 22+ messages in thread
From: bluez.test.bot @ 2021-05-18 11:12 UTC (permalink / raw)
  To: linux-bluetooth, kiran.k

[-- Attachment #1: Type: text/plain, Size: 2826 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=484205

---Test result---

Test Summary:
CheckPatch                    PASS      6.88 seconds
GitLint                       FAIL      0.84 seconds
BuildKernel                   PASS      505.55 seconds
TestRunner: Setup             PASS      324.01 seconds
TestRunner: l2cap-tester      PASS      2.45 seconds
TestRunner: bnep-tester       PASS      1.82 seconds
TestRunner: mgmt-tester       PASS      26.31 seconds
TestRunner: rfcomm-tester     PASS      2.01 seconds
TestRunner: sco-tester        PASS      1.96 seconds
TestRunner: smp-tester        PASS      2.04 seconds
TestRunner: userchan-tester   PASS      1.82 seconds

Details
##############################
Test: CheckPatch - PASS - 6.88 seconds
Run checkpatch.pl script with rule in .checkpatch.conf


##############################
Test: GitLint - FAIL - 0.84 seconds
Run gitlint with rule in .gitlint
Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command
1: T1 Title exceeds max length (76>72): "Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command"


##############################
Test: BuildKernel - PASS - 505.55 seconds
Build Kernel with minimal configuration supports Bluetooth


##############################
Test: TestRunner: Setup - PASS - 324.01 seconds
Setup environment for running Test Runner


##############################
Test: TestRunner: l2cap-tester - PASS - 2.45 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: bnep-tester - PASS - 1.82 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: mgmt-tester - PASS - 26.31 seconds
Run test-runner with mgmt-tester
Total: 416, Passed: 403 (96.9%), Failed: 0, Not Run: 13

##############################
Test: TestRunner: rfcomm-tester - PASS - 2.01 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: sco-tester - PASS - 1.96 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: smp-tester - PASS - 2.04 seconds
Run test-runner with smp-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: userchan-tester - PASS - 1.82 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 51412 bytes --]

[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3899 bytes --]

[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 551029 bytes --]

[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 14755 bytes --]

[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9912 bytes --]

[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11823 bytes --]

[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 6480 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details
  2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
                   ` (8 preceding siblings ...)
  2021-05-18 11:12 ` [v8,1/9] Bluetooth: enumerate local supported codec and cache details bluez.test.bot
@ 2021-06-03 14:23 ` Marcel Holtmann
  2021-06-08 11:54   ` K, Kiran
  9 siblings, 1 reply; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:23 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> Move reading of supported local codecs into a separate init function,
> query codecs capabilities and cache the data
> 
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
> * changes in v8:
>  - add comments
>  - split __u8 codec_id[5] into {__u8 id; __le16 cid, vid }
>  - address review comment related codec caps structure
> 
> * changes in v7:
>  - keep codec enumeration call in hci_init instead of having a separate
>    function
>  - Remove unused bitmasks defined for LE transports
> 
> * changes  in v6:
>  - fix compiler warning reported for ARCH=arc
> 
> * changes in v5:
>  - fix review comments
>  - move code used to read standard/vendor codecs caps into single function
> 
> * changes in v4:
>  - convert  reading of codecs and codecs caps calls from async to sync
> 
> * changes in v3
>  move codec enumeration into a new init function
> 
> * changes in v2
>  add skb length check before accessing data
> 
> include/net/bluetooth/hci.h      |  41 +++++++
> include/net/bluetooth/hci_core.h |  17 +++
> net/bluetooth/hci_core.c         | 199 ++++++++++++++++++++++++++++++-
> 3 files changed, 253 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index c4b0650fb9ae..6cb9340a2d51 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1307,6 +1307,28 @@ struct hci_rp_read_data_block_size {
> } __packed;
> 
> #define HCI_OP_READ_LOCAL_CODECS	0x100b
> +struct hci_std_codecs {
> +	__u8	num;
> +	__u8	codec[];
> +} __packed;
> +
> +struct hci_ven_codec {
> +	/* company id */
> +	__le16	cid;
> +	/* vendor codec id */
> +	__le16	vid;
> +} __packed;

I am pretty sure that I said to use vnd and not ven. The shortcut ven for vendor is not something we used at all.

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 2/9] Bluetooth: Add support for Read Local Supported Codecs V2
  2021-05-18 10:42 ` [PATCH v8 2/9] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
@ 2021-06-03 14:24   ` Marcel Holtmann
  0 siblings, 0 replies; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:24 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> Use V2 version of read local supported command is controller
> supports
> 
> snoop:
>> HCI Event: Command Complete (0x0e) plen 20
>      Read Local Supported Codecs V2 (0x04|0x000d) ncmd 1
>        Status: Success (0x00)
>        Number of supported codecs: 7
>          Codec: u-law log (0x00)
>          Logical Transport Type: 0x02
>            Codec supported over BR/EDR SCO and eSCO
>          Codec: A-law log (0x01)
>          Logical Transport Type: 0x02
>            Codec supported over BR/EDR SCO and eSCO
>          Codec: CVSD (0x02)
>          Logical Transport Type: 0x02
>            Codec supported over BR/EDR SCO and eSCO
>          Codec: Transparent (0x03)
>          Logical Transport Type: 0x02
>            Codec supported over BR/EDR SCO and eSCO
>          Codec: Linear PCM (0x04)
>          Logical Transport Type: 0x02
>            Codec supported over BR/EDR SCO and eSCO
>          Codec: Reserved (0x08)
>          Logical Transport Type: 0x03
>            Codec supported over BR/EDR ACL
>            Codec supported over BR/EDR SCO and eSCO
>          Codec: mSBC (0x05)
>          Logical Transport Type: 0x03
>            Codec supported over BR/EDR ACL
>            Codec supported over BR/EDR SCO and eSCO
>        Number of vendor codecs: 0
> ......
> < HCI Command: Read Local Suppor.. (0x04|0x000e) plen 7
>        Codec: mSBC (0x05)
>        Logical Transport Type: 0x00
>        Direction: Input (Host to Controller) (0x00)
>> HCI Event: Command Complete (0x0e) plen 12
>      Read Local Supported Codec Capabilities (0x04|0x000e) ncmd 1
>        Status: Success (0x00)
>        Number of codec capabilities: 1
>         Capabilities #0:
>        00 00 11 15 02 33
> 
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> * changes in v8:
>  no changes
> 
> * changes in v7:
>  call codec enumeration code in hci_init instead of having it in a separate
>  function
> 
> * changes in v6
>  no changes
> 
> * changes in v5:
>  fix review comments
> 
> * changes in v4:
>  converts codec read capabilities calls from async to sync
> 
> * changes in v3:
>  No changes
> 
> * changes in v2:
>  add length check for event data before accessing
> 
> include/net/bluetooth/hci.h | 29 ++++++++++++++
> net/bluetooth/hci_core.c    | 78 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 6cb9340a2d51..08508b3d13b4 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1337,6 +1337,35 @@ struct hci_rp_read_local_pairing_opts {
> 	__u8     max_key_size;
> } __packed;
> 
> +#define HCI_OP_READ_LOCAL_CODECS_V2	0x100d
> +struct hci_std_codec_v2 {
> +	__u8	id;
> +	__u8	transport;
> +} __packed;
> +
> +struct hci_std_codecs_v2 {
> +	__u8	num;
> +	struct hci_std_codec_v2 codec[];
> +} __packed;
> +
> +struct hci_ven_codec_v2 {
> +	__u8	id;
> +	__le16	cid;
> +	__le16	vid;
> +	__u8	transport;
> +} __packed;

See comment from previous patch.

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 3/9] Bluetooth: btintel: Add a quirk for hfp offload usecase
  2021-05-18 10:42 ` [PATCH v8 3/9] Bluetooth: btintel: Add a quirk for hfp offload usecase Kiran K
@ 2021-06-03 14:26   ` Marcel Holtmann
  0 siblings, 0 replies; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:26 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> Define a quirk to identify if intel controllers supports offload for
> HFP. In *setup* function, driver sends vendor specific command to
> check if controller supports offload. If offload is supports then
> quirk flag is set
> 
> 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 | 28 ++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h |  5 +++++
> drivers/bluetooth/btusb.c   |  2 ++
> include/net/bluetooth/hci.h |  7 +++++++
> 4 files changed, 42 insertions(+)

I dislike mixing driver changes with core changes. So first please introduce a core feature or a quirk and then use it in the driver in a separate patch.

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 4/9] Bluetooth: btitnel: Add a callback function to retireve data path
  2021-05-18 10:42 ` [PATCH v8 4/9] Bluetooth: btitnel: Add a callback function to retireve data path Kiran K
@ 2021-06-03 14:29   ` Marcel Holtmann
  2021-06-08 11:56     ` K, Kiran
  0 siblings, 1 reply; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:29 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> There is no standard HCI command to retrieve data path for transport.
> Add a new callback function to retrieve data path which is used
> in offload usecase.
> 
> 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>
> ---
> 
> This callback gets called when audio module queries codecs suppoted
> on SCO socket. For Intel controllers, data_path is always 1
> 
> drivers/bluetooth/btintel.c      | 8 ++++++++
> drivers/bluetooth/btintel.h      | 6 ++++++
> drivers/bluetooth/btusb.c        | 1 +
> include/net/bluetooth/hci_core.h | 1 +
> 4 files changed, 16 insertions(+)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e3ad19244054..8407e9736c62 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1300,6 +1300,14 @@ int btintel_read_offload_usecases(struct hci_dev *hdev)
> }
> EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> 
> +int btintel_get_data_path_id(struct hci_dev *hdev)
> +{
> +	if (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks))
> +		return -EOPNOTSUPP;
> +	return 1;
> +}
> +EXPORT_SYMBOL_GPL(btintel_get_data_path_id);
> +

actually lets not do it this way. Only set the get_data_path_id callback if offloading is supported and with that we don’t actually need to quirk either. If it is set, we know that we can offload.

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 6/9] Bluetooth: btintel: Add a callback function to configure data path
  2021-05-18 10:42 ` [PATCH v8 6/9] Bluetooth: btintel: Add a callback function to configure data path Kiran K
@ 2021-06-03 14:30   ` Marcel Holtmann
  0 siblings, 0 replies; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:30 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> In HFP offload usecase, Intel controllers require offload use
> case id (NBS or WBS) to be set before opening SCO connection. Define
> a new callback which gets called on setsockopt SCO socket. User space
> audio module is expected to set codec via setsockopt(sk, BT_CODEC, ....)
> 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>
> ---
> 
> This callback gets called when audio module does setsockopt(sk, BT_CODEC,...)
> on SCO socket and data_path is 1 (non-HCI transport). For non-HCI transport,
> Intel controller expects presets to be set before opening SCO connection.
> Presets are pre-defined, 0 - NBS, 1 - WBS. Likewise additional presets will
> be defined for A2DP, LE Audio offload use cases.
> 
> drivers/bluetooth/btintel.c      | 50 ++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h      |  8 +++++
> drivers/bluetooth/btusb.c        |  1 +
> include/net/bluetooth/hci.h      |  8 +++++
> include/net/bluetooth/hci_core.h |  2 ++
> 5 files changed, 69 insertions(+)

same thing here. Please introduce the callbacks as one patch and then in a separate patch, you start using them in a driver.

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 5/9] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket
  2021-05-18 10:42 ` [PATCH v8 5/9] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
@ 2021-06-03 14:35   ` Marcel Holtmann
  2021-06-08 12:01     ` K, Kiran
  0 siblings, 1 reply; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:35 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> Add BT_CODEC option for getsockopt systemcall over SCO socket
> to expose the codecs supported by controller
> 
> 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>
> ---
> include/net/bluetooth/bluetooth.h |  20 ++++++
> include/net/bluetooth/hci.h       |   4 ++
> net/bluetooth/sco.c               | 109 ++++++++++++++++++++++++++++++
> 3 files changed, 133 insertions(+)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 9125effbf448..1840756958ce 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -153,6 +153,26 @@ struct bt_voice {
> 
> #define BT_SCM_PKT_STATUS	0x03
> 
> +#define BT_CODEC	19
> +
> +struct	bt_codec_caps {
> +	__u8	len;
> +	__u8	data[];
> +} __packed;
> +
> +struct bt_codec {
> +	__u8	id;
> +	__le16	cid;
> +	__le16	vid;
> +	__u8	data_path;
> +	__u8	num_caps;
> +} __packed;
> +
> +struct bt_codecs {
> +	__u8		num_codecs;
> +	struct bt_codec	codecs[];
> +} __packed;
> +

what is encapsulating what here?

> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> __printf(1, 2)
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 731d48ca873a..9658600ae5de 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -2626,6 +2626,10 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
> #define hci_iso_data_len(h)		((h) & 0x3fff)
> #define hci_iso_data_flags(h)		((h) >> 14)
> 
> +/* codec transport types */
> +#define TRANSPORT_BREDR		0x00
> +#define TRANSPORT_SCO_ESCO	0x01
> +

This is confusing. SCO_ESCO is also BREDR.

> /* le24 support */
> static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3])
> {
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 3bd41563f118..d66293d1cba4 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -948,6 +948,11 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> 	struct bt_voice voice;
> 	u32 phys;
> 	int pkt_status;
> +	struct codec_list *c;
> +	u8 num_codecs, i, __user *ptr;
> +	struct hci_dev *hdev;
> +	struct hci_codec_caps *caps;
> +	__u8	data_path;
> 
> 	BT_DBG("sk %p", sk);
> 
> @@ -1012,6 +1017,110 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> 			err = -EFAULT;
> 		break;
> 
> +	case BT_CODEC:
> +		num_codecs = 0;
> +		len = 0;
> +
> +		hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
> +

Remove extra empty line.

> +		if (!hdev) {
> +			err = -EBADFD;
> +			break;
> +		}
> +
> +		if (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks)) {
> +			err = -EOPNOTSUPP;
> +			break;
> +		}
> +
> +		if (!hdev->get_data_path_id) {
> +			err = -EOPNOTSUPP;
> +			break;
> +		}

See it is pointless to check a quirk, you can use the callback for it.

> +
> +		hci_dev_lock(hdev);
> +		list_for_each_entry(c, &hdev->local_codecs, list) {
> +			if (c->transport != TRANSPORT_SCO_ESCO)
> +				continue;
> +			num_codecs++;
> +			for (i = 0, caps = c->caps; i < c->num_caps; i++) {
> +				len += 1 + caps->len;
> +				caps = (void *)&caps->data[caps->len];
> +			}
> +			len += sizeof(struct bt_codec);
> +		}
> +		hci_dev_unlock(hdev);
> +
> +		if (len > 255) {
> +			err = -ENOMEM;
> +			break;

I think this is the wrong error code. You need to indicate that size is wrong. ENOMEM really means that memory allocation failed.

> +		}
> +
> +		ptr = optval;
> +		if (put_user(num_codecs, ptr)) {
> +			err = -EFAULT;
> +			break;
> +		}
> +		ptr += sizeof(num_codecs);
> +
> +		hci_dev_lock(hdev);
> +		list_for_each_entry(c, &hdev->local_codecs, list) {
> +			if (c->transport != TRANSPORT_SCO_ESCO)
> +				continue;
> +
> +			if (put_user(c->id, ptr)) {
> +				err = -EFAULT;
> +				goto unlock;
> +			}
> +			ptr += sizeof(c->id);
> +
> +			if (put_user(c->cid, ptr)) {
> +				err = -EFAULT;
> +				goto unlock;
> +			}
> +			ptr += sizeof(c->cid);
> +
> +			if (put_user(c->vid, ptr)) {
> +				err = -EFAULT;
> +				goto unlock;
> +			}
> +			ptr += sizeof(c->vid);
> +
> +			err = hdev->get_data_path_id(hdev);
> +			if (err < 0) {
> +				err = -EFAULT;
> +				goto unlock;
> +			}
> +			data_path = (__u8)err;
> +			if (put_user(data_path, ptr)) {
> +				err = -EFAULT;
> +				goto unlock;
> +			}
> +			ptr += sizeof(data_path);
> +
> +			if (put_user(c->num_caps, ptr)) {
> +				err = -EFAULT;
> +				goto unlock;
> +			}
> +			ptr += sizeof(c->num_caps);
> +
> +			len = 0;
> +			for (i = 0, caps = c->caps; i < c->num_caps; i++) {
> +				len += 1 + caps->len;
> +				caps = (void *)&caps->data[caps->len];
> +			}
> +
> +			if (len && copy_to_user(ptr, c->caps, len)) {
> +				err = -EFAULT;
> +				goto unlock;
> +			}
> +			ptr += len;
> +		}
> +unlock:
> +		hci_dev_unlock(hdev);
> +
> +		break;
> +
> 	default:
> 		err = -ENOPROTOOPT;
> 		break;

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 7/9] Bluetooth: Add BT_CODEC option for setsockopt over SCO
  2021-05-18 10:42 ` [PATCH v8 7/9] Bluetooth: Add BT_CODEC option for setsockopt over SCO Kiran K
@ 2021-06-03 14:37   ` Marcel Holtmann
  0 siblings, 0 replies; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:37 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> Add BT_CODEC option on setsockopt system call to allow user space
> audio modules to set codec. Driver also configures codec if non-HCI
> data is selected.
> 
> 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>
> ---
> include/net/bluetooth/bluetooth.h |  2 +
> net/bluetooth/sco.c               | 63 +++++++++++++++++++++++++++++++
> 2 files changed, 65 insertions(+)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 1840756958ce..0e8802d09068 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -173,6 +173,8 @@ struct bt_codecs {
> 	struct bt_codec	codecs[];
> } __packed;
> 
> +#define CODING_FORMAT_CVSD	0x02
> +
> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> __printf(1, 2)
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index d66293d1cba4..d59f30fc4b9f 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -67,6 +67,7 @@ struct sco_pinfo {
> 	__u32		flags;
> 	__u16		setting;
> 	__u8		cmsg_mask;
> +	struct bt_codec codec;
> 	struct sco_conn	*conn;
> };
> 
> @@ -438,6 +439,7 @@ static void __sco_sock_close(struct sock *sk)
> 		sock_set_flag(sk, SOCK_ZAPPED);
> 		break;
> 	}
> +
> }
> 
> /* Must be called on unlocked socket. */
> @@ -499,6 +501,10 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock,
> 	sk->sk_state    = BT_OPEN;
> 
> 	sco_pi(sk)->setting = BT_VOICE_CVSD_16BIT;
> +	sco_pi(sk)->codec.id = CODING_FORMAT_CVSD;
> +	sco_pi(sk)->codec.cid = 0xffff;
> +	sco_pi(sk)->codec.vid = 0xffff;
> +	sco_pi(sk)->codec.data_path = 0x00;
> 
> 	timer_setup(&sk->sk_timer, sco_sock_timeout, 0);
> 
> @@ -808,6 +814,9 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> 	struct sock *sk = sock->sk;
> 	int len, err = 0;
> 	struct bt_voice voice;
> +	struct bt_codecs *codecs;
> +	struct hci_dev *hdev;
> +	__u8 buffer[255];
> 	u32 opt;
> 
> 	BT_DBG("sk %p", sk);
> @@ -870,6 +879,60 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> 			sco_pi(sk)->cmsg_mask &= SCO_CMSG_PKT_STATUS;
> 		break;
> 
> +	case BT_CODEC:
> +		if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND &&
> +		    sk->sk_state != BT_CONNECT2) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
> +		if (!hdev) {
> +			err = -EBADFD;
> +			break;
> +		}
> +
> +		if (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks)) {
> +			err = -EOPNOTSUPP;
> +			break;
> +		}

Same here. Just check the configure_data_path (and I think set_data_path is better and shorter) is available, if not bail our here right away instead checking this over and over again later.

> +
> +		if (optlen < sizeof(struct bt_codecs) || optlen > 255) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (copy_from_sockptr(buffer, optval, optlen)) {
> +			err = -EFAULT;
> +			break;
> +		}
> +
> +		codecs = (void *)buffer;
> +
> +		if (codecs->num_codecs > 1) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		if (codecs->codecs[0].data_path) {
> +			if (!hdev->configure_data_path) {
> +				err = -EOPNOTSUPP;
> +				break;
> +			}
> +			err = hdev->configure_data_path(hdev, SCO_LINK,
> +							codecs->codecs);
> +			if (err < 0)
> +				break;
> +
> +			if (codecs->codecs[0].id == 0xff) {
> +				sco_pi(sk)->codec.cid = codecs->codecs[0].cid;
> +				sco_pi(sk)->codec.vid = codecs->codecs[0].vid;
> +			}
> +		}
> +		sco_pi(sk)->codec.id = codecs->codecs[0].id;
> +		sco_pi(sk)->codec.data_path = codecs->codecs[0].data_path;
> +		break;
> +
> 	default:
> 		err = -ENOPROTOOPT;
> 		break;

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v8 8/9] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command
  2021-05-18 10:42 ` [PATCH v8 8/9] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command Kiran K
@ 2021-06-03 14:39   ` Marcel Holtmann
  2021-06-08 12:06     ` K, Kiran
  0 siblings, 1 reply; 22+ messages in thread
From: Marcel Holtmann @ 2021-06-03 14:39 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, ravishankar.srivatsa, chethan.tumkur.narayan

Hi Kiran,

> < HCI Command: Enhanced Setup Synchronous Connection (0x01|0x003d) plen 59
>        Handle: 256
>        Transmit bandwidth: 8000
>        Receive bandwidth: 8000
>        Max latency: 13
>        Packet type: 0x0380
>          3-EV3 may not be used
>          2-EV5 may not be used
>          3-EV5 may not be used
>        Retransmission effort: Optimize for link quality (0x02)
>> HCI Event: Command Status (0x0f) plen 4
>      Enhanced Setup Synchronous Connection (0x01|0x003d) ncmd 1
>        Status: Success (0x00)
>> HCI Event: Synchronous Connect Complete (0x2c) plen 17
>        Status: Success (0x00)
>        Handle: 257
>        Address: CC:98:8B:92:04:FD (SONY Visual Products Inc.)
>        Link type: eSCO (0x02)
>        Transmission interval: 0x0c
>        Retransmission window: 0x06
>        RX packet length: 60
>        TX packet length: 60
>        Air mode: Transparent (0x03)
> 
> 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>
> ---
> 
> Need implementation HCI_Enhanced_Setup_Synchronous_Command as it allows
> setting of non-HCI data path
> 
> include/net/bluetooth/bluetooth.h |   1 +
> include/net/bluetooth/hci.h       |  34 ++++++++++
> include/net/bluetooth/hci_core.h  |   7 +-
> net/bluetooth/hci_conn.c          | 104 +++++++++++++++++++++++++++++-
> net/bluetooth/hci_event.c         |  48 +++++++++++++-
> net/bluetooth/sco.c               |  11 +++-
> 6 files changed, 198 insertions(+), 7 deletions(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 0e8802d09068..4e58d275c72c 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -174,6 +174,7 @@ struct bt_codecs {
> } __packed;
> 
> #define CODING_FORMAT_CVSD	0x02
> +#define CODING_FORMAT_TRANS	0x03
> 
> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index bd666b114aea..af168f3c1f2e 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -878,6 +878,40 @@ struct hci_cp_logical_link_cancel {
> 	__u8     flow_spec_id;
> } __packed;
> 
> +#define HCI_OP_ENHANCED_SETUP_SYNC_CONN		0x043D
> +struct hci_coding_format {
> +	__u8	id;
> +	__le16	cid;
> +	__le16	vid;
> +} __packed;
> +
> +struct hci_cp_enhanced_setup_sync_conn {
> +	__le16   handle;
> +	__le32   tx_bandwidth;
> +	__le32   rx_bandwidth;
> +	struct	 hci_coding_format tx_coding_format;
> +	struct	 hci_coding_format rx_coding_format;
> +	__le16	 tx_codec_frame_size;
> +	__le16	 rx_codec_frame_size;
> +	__le32	 in_bandwidth;
> +	__le32	 out_bandwidth;
> +	struct	 hci_coding_format in_coding_format;
> +	struct	 hci_coding_format out_coding_format;
> +	__le16   in_coded_data_size;
> +	__le16	 out_coded_data_size;
> +	__u8	 in_pcm_data_format;
> +	__u8	 out_pcm_data_format;
> +	__u8	 in_pcm_sample_payload_msb_pos;
> +	__u8	 out_pcm_sample_payload_msb_pos;
> +	__u8	 in_data_path;
> +	__u8	 out_data_path;
> +	__u8	 in_trasnport_unit_size;
> +	__u8	 out_trasnport_unit_size;
> +	__le16   max_latency;
> +	__le16   pkt_type;
> +	__u8     retrans_effort;
> +} __packed;
> +
> struct hci_rp_logical_link_cancel {
> 	__u8     status;
> 	__u8     phy_handle;
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 959585bafa8d..760d21a2134e 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -712,6 +712,7 @@ struct hci_conn {
> 	struct amp_mgr	*amp_mgr;
> 
> 	struct hci_conn	*link;
> +	struct bt_codec codec;
> 
> 	void (*connect_cfm_cb)	(struct hci_conn *conn, u8 status);
> 	void (*security_cfm_cb)	(struct hci_conn *conn, u8 status);
> @@ -1094,6 +1095,7 @@ static inline struct hci_conn *hci_lookup_le_connect(struct hci_dev *hdev)
> 
> int hci_disconnect(struct hci_conn *conn, __u8 reason);
> bool hci_setup_sync(struct hci_conn *conn, __u16 handle);
> +bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle);
> void hci_sco_setup(struct hci_conn *conn, __u8 status);
> 
> struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
> @@ -1118,7 +1120,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
> 				 u8 sec_level, u8 auth_type,
> 				 enum conn_reasons conn_reason);
> struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
> -				 __u16 setting);
> +				 __u16 setting, struct bt_codec *codec);
> int hci_conn_check_link_mode(struct hci_conn *conn);
> int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
> int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type,
> @@ -1439,6 +1441,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> /* Use LL Privacy based address resolution if supported */
> #define use_ll_privacy(dev) ((dev)->le_features[0] & HCI_LE_LL_PRIVACY)
> 
> +/* Use enhanced synchronous connection if command is supported */
> +#define use_enhanced_sco_conn(dev) ((dev)->commands[29] & 0x08)
> +
> /* Use ext scanning if set ext scan param and ext scan enable is supported */
> #define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
> 			   ((dev)->commands[37] & 0x40))
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 88ec08978ff4..e2435b5abeca 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -307,6 +307,96 @@ static bool find_next_esco_param(struct hci_conn *conn,
> 	return conn->attempt <= size;
> }
> 
> +bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +	struct hci_cp_enhanced_setup_sync_conn cp;
> +	const struct sco_param *param;
> +
> +	BT_DBG("hcon %p", conn);
> +
> +	conn->state = BT_CONNECT;
> +	conn->out = true;
> +
> +	conn->attempt++;
> +
> +	memset(&cp, 0x00, sizeof(cp));
> +
> +	cp.handle   = cpu_to_le16(handle);
> +
> +	cp.tx_bandwidth   = cpu_to_le32(0x00001f40);
> +	cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
> +
> +	switch (conn->codec.id) {
> +	case CODING_FORMAT_CVSD:
> +		if (lmp_esco_capable(conn->link)) {
> +			if (conn->attempt > ARRAY_SIZE(esco_param_cvsd))
> +				return false;
> +			param = &esco_param_cvsd[conn->attempt - 1];
> +		} else {
> +			if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> +				return false;
> +			param = &sco_param_cvsd[conn->attempt - 1];
> +		}
> +		cp.tx_coding_format.id = 2;
> +		cp.rx_coding_format.id = 2;
> +		cp.tx_codec_frame_size = __cpu_to_le16(60);
> +		cp.rx_codec_frame_size = __cpu_to_le16(60);
> +		cp.in_bandwidth = __cpu_to_le32(16000);
> +		cp.out_bandwidth = __cpu_to_le32(16000);
> +		cp.in_coding_format.id = 4;
> +		cp.out_coding_format.id = 4;
> +		cp.in_coded_data_size = __cpu_to_le16(16);
> +		cp.out_coded_data_size = __cpu_to_le16(16);
> +		cp.in_pcm_data_format = 2;
> +		cp.out_pcm_data_format = 2;
> +		cp.in_pcm_sample_payload_msb_pos = 0;
> +		cp.out_pcm_sample_payload_msb_pos = 0;
> +		cp.in_data_path = conn->codec.data_path;
> +		cp.out_data_path = conn->codec.data_path;
> +		cp.in_trasnport_unit_size = 16;
> +		cp.out_trasnport_unit_size = 16;
> +		break;
> +
> +	case CODING_FORMAT_TRANS:
> +		if (conn->attempt > ARRAY_SIZE(esco_param_msbc))
> +			return false;
> +
> +		param = &esco_param_msbc[conn->attempt - 1];
> +		cp.tx_coding_format.id = 0x03;
> +		cp.rx_coding_format.id = 0x03;
> +		cp.tx_codec_frame_size = __cpu_to_le16(60);
> +		cp.rx_codec_frame_size = __cpu_to_le16(60);
> +		cp.in_bandwidth = __cpu_to_le32(0x1f40);
> +		cp.out_bandwidth = __cpu_to_le32(0x1f40);
> +		cp.in_coding_format.id = 0x03;
> +		cp.out_coding_format.id = 0x03;
> +		cp.in_coded_data_size = __cpu_to_le16(16);
> +		cp.out_coded_data_size = __cpu_to_le16(16);
> +		cp.in_pcm_data_format = 2;
> +		cp.out_pcm_data_format = 2;
> +		cp.in_pcm_sample_payload_msb_pos = 0;
> +		cp.out_pcm_sample_payload_msb_pos = 0;
> +		cp.in_data_path = conn->codec.data_path;
> +		cp.out_data_path = conn->codec.data_path;
> +		cp.in_trasnport_unit_size = 1;
> +		cp.out_trasnport_unit_size = 1;
> +		break;
> +
> +	default:
> +		return false;
> +	}
> +
> +	cp.retrans_effort = param->retrans_effort;
> +	cp.pkt_type = __cpu_to_le16(param->pkt_type);
> +	cp.max_latency = __cpu_to_le16(param->max_latency);
> +
> +	if (hci_send_cmd(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN, sizeof(cp), &cp) < 0)
> +		return false;
> +
> +	return true;
> +}
> +
> bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
> {
> 	struct hci_dev *hdev = conn->hdev;
> @@ -424,8 +514,12 @@ void hci_sco_setup(struct hci_conn *conn, __u8 status)
> 	BT_DBG("hcon %p", conn);
> 
> 	if (!status) {
> -		if (lmp_esco_capable(conn->hdev))
> -			hci_setup_sync(sco, conn->handle);
> +		if (lmp_esco_capable(conn->hdev)) {
> +			if (use_enhanced_sco_conn(conn->hdev))
> +				hci_enhanced_setup_sync(sco, conn->handle);
> +			else
> +				hci_setup_sync(sco, conn->handle);
> +		}
> 		else
> 			hci_add_sco(sco, conn->handle);

you have coding style mistake here.


> 	} else {
> @@ -1319,7 +1413,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
> }
> 
> struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
> -				 __u16 setting)
> +				 __u16 setting, struct bt_codec *codec)
> {
> 	struct hci_conn *acl;
> 	struct hci_conn *sco;
> @@ -1344,6 +1438,10 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
> 	hci_conn_hold(sco);
> 
> 	sco->setting = setting;
> +	sco->codec.id = codec->id;
> +	sco->codec.cid = codec->cid;
> +	sco->codec.vid = codec->vid;
> +	sco->codec.data_path = codec->data_path;
> 
> 	if (acl->state == BT_CONNECTED &&
> 	    (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 4241ae310fcb..94c64e2c11e4 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2236,6 +2236,41 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
> 	hci_dev_unlock(hdev);
> }
> 
> +static void hci_cs_enhanced_setup_sync_conn(struct hci_dev *hdev, __u8 status)
> +{
> +	struct hci_cp_enhanced_setup_sync_conn *cp;
> +	struct hci_conn *acl, *sco;
> +	__u16 handle;
> +
> +	bt_dev_dbg(hdev, "status 0x%2.2x", status);
> +
> +	if (!status)
> +		return;
> +
> +	cp = hci_sent_cmd_data(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN);
> +	if (!cp)
> +		return;
> +
> +	handle = __le16_to_cpu(cp->handle);
> +
> +	BT_DBG("%s handle 0x%4.4x", hdev->name, handle);

Please start using bt_dev_dbg.

> +
> +	hci_dev_lock(hdev);
> +
> +	acl = hci_conn_hash_lookup_handle(hdev, handle);
> +	if (acl) {
> +		sco = acl->link;
> +		if (sco) {
> +			sco->state = BT_CLOSED;
> +
> +			hci_connect_cfm(sco, status);
> +			hci_conn_del(sco);
> +		}
> +	}
> +
> +	hci_dev_unlock(hdev);
> +}
> +
> static void hci_cs_sniff_mode(struct hci_dev *hdev, __u8 status)
> {
> 	struct hci_cp_sniff_mode *cp;
> @@ -3715,6 +3750,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb,
> 		hci_cs_setup_sync_conn(hdev, ev->status);
> 		break;
> 
> +	case HCI_OP_ENHANCED_SETUP_SYNC_CONN:
> +		hci_cs_enhanced_setup_sync_conn(hdev, ev->status);
> +		break;
> +
> 	case HCI_OP_SNIFF_MODE:
> 		hci_cs_sniff_mode(hdev, ev->status);
> 		break;
> @@ -4401,8 +4440,13 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
> 		if (conn->out) {
> 			conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
> 					(hdev->esco_type & EDR_ESCO_MASK);
> -			if (hci_setup_sync(conn, conn->link->handle))
> -				goto unlock;
> +			if (use_enhanced_sco_conn(hdev)) {
> +				if (hci_enhanced_setup_sync(conn, conn->link->handle))
> +					goto unlock;
> +			} else {
> +				if (hci_setup_sync(conn, conn->link->handle))
> +					goto unlock;
> +			}
> 		}
> 		fallthrough;
> 
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index d59f30fc4b9f..19d8ee11eed5 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -240,7 +240,7 @@ static int sco_connect(struct sock *sk)
> 	}
> 
> 	hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
> -			       sco_pi(sk)->setting);
> +			       sco_pi(sk)->setting, &sco_pi(sk)->codec);
> 	if (IS_ERR(hcon)) {
> 		err = PTR_ERR(hcon);
> 		goto done;
> @@ -865,6 +865,15 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> 		}
> 
> 		sco_pi(sk)->setting = voice.setting;
> +		hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src,
> +				     BDADDR_BREDR);
> +		if (!hdev) {
> +			err = -EBADFD;
> +			break;
> +		}
> +		if (use_enhanced_sco_conn(hdev) &&
> +		    voice.setting == BT_VOICE_TRANSPARENT)
> +			sco_pi(sk)->codec.id = CODING_FORMAT_TRANS;
> 		break;

Regards

Marcel


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details
  2021-06-03 14:23 ` [PATCH v8 1/9] " Marcel Holtmann
@ 2021-06-08 11:54   ` K, Kiran
  0 siblings, 0 replies; 22+ messages in thread
From: K, Kiran @ 2021-06-08 11:54 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth, Srivatsa, Ravishankar, Tumkur Narayan, Chethan

Hi Marcel,

> > #define HCI_OP_READ_LOCAL_CODECS	0x100b
> > +struct hci_std_codecs {
> > +	__u8	num;
> > +	__u8	codec[];
> > +} __packed;
> > +
> > +struct hci_ven_codec {
> > +	/* company id */
> > +	__le16	cid;
> > +	/* vendor codec id */
> > +	__le16	vid;
> > +} __packed;
> 
> I am pretty sure that I said to use vnd and not ven. The shortcut ven for
> vendor is not something we used at all.
> 
Ack. I overlooked at it.  I will fix this in the next patchset.
 
Regards,
Kiran


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v8 4/9] Bluetooth: btitnel: Add a callback function to retireve data path
  2021-06-03 14:29   ` Marcel Holtmann
@ 2021-06-08 11:56     ` K, Kiran
  0 siblings, 0 replies; 22+ messages in thread
From: K, Kiran @ 2021-06-08 11:56 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth, Srivatsa, Ravishankar, Tumkur Narayan, Chethan

Hi Marcel,

> > This callback gets called when audio module queries codecs suppoted on
> > SCO socket. For Intel controllers, data_path is always 1
> >
> > drivers/bluetooth/btintel.c      | 8 ++++++++
> > drivers/bluetooth/btintel.h      | 6 ++++++
> > drivers/bluetooth/btusb.c        | 1 +
> > include/net/bluetooth/hci_core.h | 1 +
> > 4 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index e3ad19244054..8407e9736c62 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -1300,6 +1300,14 @@ int btintel_read_offload_usecases(struct
> > hci_dev *hdev) } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> >
> > +int btintel_get_data_path_id(struct hci_dev *hdev) {
> > +	if (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED,
> &hdev->quirks))
> > +		return -EOPNOTSUPP;
> > +	return 1;
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_get_data_path_id);
> > +
> 
> actually lets not do it this way. Only set the get_data_path_id callback if
> offloading is supported and with that we don’t actually need to quirk either.
> If it is set, we know that we can offload.
> 

Ack.

Thanks,
Kiran


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v8 5/9] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket
  2021-06-03 14:35   ` Marcel Holtmann
@ 2021-06-08 12:01     ` K, Kiran
  0 siblings, 0 replies; 22+ messages in thread
From: K, Kiran @ 2021-06-08 12:01 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth, Srivatsa, Ravishankar, Tumkur Narayan, Chethan

Hi Marcel,

> >
> > #define BT_SCM_PKT_STATUS	0x03
> >
> > +#define BT_CODEC	19
> > +
> > +struct	bt_codec_caps {
> > +	__u8	len;
> > +	__u8	data[];
> > +} __packed;
> > +
> > +struct bt_codec {
> > +	__u8	id;
> > +	__le16	cid;
> > +	__le16	vid;
> > +	__u8	data_path;
> > +	__u8	num_caps;
> > +} __packed;
> > +
> > +struct bt_codecs {
> > +	__u8		num_codecs;
> > +	struct bt_codec	codecs[];
> > +} __packed;
> > +
> 
> what is encapsulating what here?

bt_codec encapsulates caps.
bt_codecs  is aggregation of bt_codec.

> 
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > __printf(1, 2)
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 731d48ca873a..9658600ae5de 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -2626,6 +2626,10 @@ static inline struct hci_sco_hdr
> *hci_sco_hdr(const struct sk_buff *skb)
> > #define hci_iso_data_len(h)		((h) & 0x3fff)
> > #define hci_iso_data_flags(h)		((h) >> 14)
> >
> > +/* codec transport types */
> > +#define TRANSPORT_BREDR		0x00
> > +#define TRANSPORT_SCO_ESCO	0x01
> > +
> 
> This is confusing. SCO_ESCO is also BREDR.

Ack. I will remove the suffix BREDR.

> 
> > /* le24 support */
> > static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3]) { diff
> > --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index
> > 3bd41563f118..d66293d1cba4 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -948,6 +948,11 @@ static int sco_sock_getsockopt(struct socket *sock,
> int level, int optname,
> > 	struct bt_voice voice;
> > 	u32 phys;
> > 	int pkt_status;
> > +	struct codec_list *c;
> > +	u8 num_codecs, i, __user *ptr;
> > +	struct hci_dev *hdev;
> > +	struct hci_codec_caps *caps;
> > +	__u8	data_path;
> >
> > 	BT_DBG("sk %p", sk);
> >
> > @@ -1012,6 +1017,110 @@ static int sco_sock_getsockopt(struct socket
> *sock, int level, int optname,
> > 			err = -EFAULT;
> > 		break;
> >
> > +	case BT_CODEC:
> > +		num_codecs = 0;
> > +		len = 0;
> > +
> > +		hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src,
> > +BDADDR_BREDR);
> > +
> 
> Remove extra empty line.

Ack

> 
> > +		if (!hdev) {
> > +			err = -EBADFD;
> > +			break;
> > +		}
> > +
> > +		if
> (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks))
> {
> > +			err = -EOPNOTSUPP;
> > +			break;
> > +		}
> > +
> > +		if (!hdev->get_data_path_id) {
> > +			err = -EOPNOTSUPP;
> > +			break;
> > +		}
> 
> See it is pointless to check a quirk, you can use the callback for it.
> 

Ack

> > +
> > +		hci_dev_lock(hdev);
> > +		list_for_each_entry(c, &hdev->local_codecs, list) {
> > +			if (c->transport != TRANSPORT_SCO_ESCO)
> > +				continue;
> > +			num_codecs++;
> > +			for (i = 0, caps = c->caps; i < c->num_caps; i++) {
> > +				len += 1 + caps->len;
> > +				caps = (void *)&caps->data[caps->len];
> > +			}
> > +			len += sizeof(struct bt_codec);
> > +		}
> > +		hci_dev_unlock(hdev);
> > +
> > +		if (len > 255) {
> > +			err = -ENOMEM;
> > +			break;
> 
> I think this is the wrong error code. You need to indicate that size is wrong.
> ENOMEM really means that memory allocation failed.

I will change error code to ENOBUFS
> 

Thanks,
Kiran


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v8 8/9] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command
  2021-06-03 14:39   ` Marcel Holtmann
@ 2021-06-08 12:06     ` K, Kiran
  0 siblings, 0 replies; 22+ messages in thread
From: K, Kiran @ 2021-06-08 12:06 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth, Srivatsa, Ravishankar, Tumkur Narayan, Chethan

Hi Marcel,

> > bool hci_setup_sync(struct hci_conn *conn, __u16 handle) {
> > 	struct hci_dev *hdev = conn->hdev;
> > @@ -424,8 +514,12 @@ void hci_sco_setup(struct hci_conn *conn, __u8
> status)
> > 	BT_DBG("hcon %p", conn);
> >
> > 	if (!status) {
> > -		if (lmp_esco_capable(conn->hdev))
> > -			hci_setup_sync(sco, conn->handle);
> > +		if (lmp_esco_capable(conn->hdev)) {
> > +			if (use_enhanced_sco_conn(conn->hdev))
> > +				hci_enhanced_setup_sync(sco, conn-
> >handle);
> > +			else
> > +				hci_setup_sync(sco, conn->handle);
> > +		}
> > 		else
> > 			hci_add_sco(sco, conn->handle);
> 
> you have coding style mistake here.

Ack.

> 
> 
> > +static void hci_cs_enhanced_setup_sync_conn(struct hci_dev *hdev,
> > +__u8 status) {
> > +	struct hci_cp_enhanced_setup_sync_conn *cp;
> > +	struct hci_conn *acl, *sco;
> > +	__u16 handle;
> > +
> > +	bt_dev_dbg(hdev, "status 0x%2.2x", status);
> > +
> > +	if (!status)
> > +		return;
> > +
> > +	cp = hci_sent_cmd_data(hdev,
> HCI_OP_ENHANCED_SETUP_SYNC_CONN);
> > +	if (!cp)
> > +		return;
> > +
> > +	handle = __le16_to_cpu(cp->handle);
> > +
> > +	BT_DBG("%s handle 0x%4.4x", hdev->name, handle);
> 
> Please start using bt_dev_dbg.

Ack.

Thanks,
Kiran


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-06-08 12:07 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-18 10:42 [PATCH v8 1/9] Bluetooth: enumerate local supported codec and cache details Kiran K
2021-05-18 10:42 ` [PATCH v8 2/9] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
2021-06-03 14:24   ` Marcel Holtmann
2021-05-18 10:42 ` [PATCH v8 3/9] Bluetooth: btintel: Add a quirk for hfp offload usecase Kiran K
2021-06-03 14:26   ` Marcel Holtmann
2021-05-18 10:42 ` [PATCH v8 4/9] Bluetooth: btitnel: Add a callback function to retireve data path Kiran K
2021-06-03 14:29   ` Marcel Holtmann
2021-06-08 11:56     ` K, Kiran
2021-05-18 10:42 ` [PATCH v8 5/9] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
2021-06-03 14:35   ` Marcel Holtmann
2021-06-08 12:01     ` K, Kiran
2021-05-18 10:42 ` [PATCH v8 6/9] Bluetooth: btintel: Add a callback function to configure data path Kiran K
2021-06-03 14:30   ` Marcel Holtmann
2021-05-18 10:42 ` [PATCH v8 7/9] Bluetooth: Add BT_CODEC option for setsockopt over SCO Kiran K
2021-06-03 14:37   ` Marcel Holtmann
2021-05-18 10:42 ` [PATCH v8 8/9] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command Kiran K
2021-06-03 14:39   ` Marcel Holtmann
2021-06-08 12:06     ` K, Kiran
2021-05-18 10:42 ` [PATCH v8 9/9] Bluetooth: Add support for msbc coding format Kiran K
2021-05-18 11:12 ` [v8,1/9] Bluetooth: enumerate local supported codec and cache details bluez.test.bot
2021-06-03 14:23 ` [PATCH v8 1/9] " Marcel Holtmann
2021-06-08 11:54   ` K, Kiran

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.