[PATCH v1 0/3] Enumerate supported codecs and cache the data at driver

[PATCH v1 0/3] Enumerate supported codecs and cache the data at driver

[Kiran K]

During driver initialization path, add support to query controller
for the local supported codecs and cache the details at driver. This information
can be made available to user space audio modules to select the codec for 
audio offload use cases

Kiran K (3):
  Bluetooth: add support to enumerate codec capabilities
  Bluetooth: add support to enumerate local supports codecs v2
  Bluetooth: cache local supported codec capabilities

 include/net/bluetooth/hci.h      |  20 ++++
 include/net/bluetooth/hci_core.h |  23 ++++
 net/bluetooth/hci_core.c         |  35 +++++-
 net/bluetooth/hci_event.c        | 185 +++++++++++++++++++++++++++++++
 4 files changed, 262 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH v1 1/3] Bluetooth: add support to enumerate codec capabilities

[Kiran K]

add support to enumerate local supported codec capabilities

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

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ea4ae551c426..e3f7771fe84f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1314,6 +1314,13 @@ 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	codec_id[5];
+	__u8	transport;
+	__u8	direction;
+} __packed;
+
 #define HCI_OP_READ_PAGE_SCAN_ACTIVITY	0x0c1b
 struct hci_rp_read_page_scan_activity {
 	__u8     status;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 016b2999f219..ceed5a5d332b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -887,6 +887,58 @@ static void hci_cc_read_data_block_size(struct hci_dev *hdev,
 	       hdev->block_cnt, hdev->block_len);
 }
 
+static void hci_cc_read_local_codecs(struct hci_dev *hdev,
+				     struct sk_buff *skb)
+{
+	__u8 num_codecs;
+	__u8 *ptr;
+	struct hci_op_read_local_codec_caps caps;
+
+	bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
+
+	if (skb->data[0])
+		return;
+
+	/* enumerate standard codecs */
+	skb_pull(skb, 1);
+	num_codecs = skb->data[0];
+
+	bt_dev_dbg(hdev, "Number of standard codecs: %u", num_codecs);
+
+	skb_pull(skb, 1);
+	ptr = (__u8 *)skb->data;
+
+	skb_pull(skb, num_codecs);
+
+	while (num_codecs--) {
+		caps.codec_id[0] = *ptr++;
+		caps.transport = 0x00;
+		caps.direction = 0x00;
+
+		hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+			     &caps);
+	}
+
+	/* enumerate vendor specific codecs */
+	num_codecs = skb->data[0];
+	skb_pull(skb, 1);
+
+	bt_dev_dbg(hdev, "Number of vendor specific codecs: %u", num_codecs);
+
+	ptr = (__u8 *)skb->data;
+
+	while (num_codecs--) {
+		caps.codec_id[0] = 0xFF;
+		memcpy(&caps.codec_id[1], ptr, 4);
+		ptr += 4;
+		caps.transport = 0x00;
+		caps.direction = 0x00;
+
+		hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+			     &caps);
+	}
+}
+
 static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_read_clock *rp = (void *) skb->data;
@@ -3437,6 +3489,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cc_read_data_block_size(hdev, skb);
 		break;
 
+	case HCI_OP_READ_LOCAL_CODECS:
+		hci_cc_read_local_codecs(hdev, skb);
+		break;
+
 	case HCI_OP_READ_FLOW_CONTROL_MODE:
 		hci_cc_read_flow_control_mode(hdev, skb);
 		break;
-- 
2.17.1

[PATCH v1 2/3] Bluetooth: add support to enumerate local supports codecs v2

[Kiran K]

Add support for HCI_Read_Local_Supported_Codecs_V2 and query codec
capabilities

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>
---
 include/net/bluetooth/hci.h      |  2 +
 include/net/bluetooth/hci_core.h | 10 ++++
 net/bluetooth/hci_core.c         |  4 +-
 net/bluetooth/hci_event.c        | 98 ++++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e3f7771fe84f..34eb9f4b027f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1314,6 +1314,8 @@ struct hci_rp_read_local_pairing_opts {
 	__u8     max_key_size;
 } __packed;
 
+#define HCI_OP_READ_LOCAL_CODECS_V2    0x100d
+
 #define HCI_OP_READ_LOCAL_CODEC_CAPS	0x100e
 struct hci_op_read_local_codec_caps {
 	__u8	codec_id[5];
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c73ac52af186..fa0c68fd3306 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1848,4 +1848,14 @@ 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 LOCAL_CODEC_LECIS_MASK	BIT(2)
+#define LOCAL_CODEC_LEBIS_MASK	BIT(3)
+
+#define LOCAL_CODEC_ACL		0x00
+#define LOCAL_CODEC_SCO		0x01
+#define LOCAL_CODEC_LECIS	0x02
+#define LOCAL_CODEC_LEBIS	0x03
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fd12f1652bdf..230aeedd6d00 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -839,7 +839,9 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
 		hci_set_event_mask_page_2(req);
 
 	/* Read local codec list if the HCI command is supported */
-	if (hdev->commands[29] & 0x20)
+	if (hdev->commands[45] & 0x04)
+		hci_req_add(req, HCI_OP_READ_LOCAL_CODECS_V2, 0, NULL);
+	else 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 */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ceed5a5d332b..a16723c89dc6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -939,6 +939,100 @@ static void hci_cc_read_local_codecs(struct hci_dev *hdev,
 	}
 }
 
+static void hci_cc_read_local_codecs_v2(struct hci_dev *hdev,
+					struct sk_buff *skb)
+{
+	__u8 num_codecs, transport;
+	__u8 *ptr;
+	struct hci_op_read_local_codec_caps caps;
+
+	bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
+
+	if (skb->data[0])
+		return;
+
+	/* enumerate standard codecs */
+	skb_pull(skb, 1);
+	num_codecs = skb->data[0];
+
+	bt_dev_info(hdev, "Number of standard codecs: %u", num_codecs);
+
+	skb_pull(skb, 1);
+	ptr = (__u8 *)skb->data;
+
+	skb_pull(skb, num_codecs * 2);
+
+	while (num_codecs--) {
+		caps.codec_id[0] = *ptr++;
+		transport = *ptr++;
+		caps.direction = 0x00;
+
+		if (transport & LOCAL_CODEC_ACL_MASK) {
+			caps.transport = LOCAL_CODEC_ACL;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_SCO_MASK) {
+			caps.transport = LOCAL_CODEC_SCO;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_LEBIS_MASK) {
+			caps.transport = LOCAL_CODEC_LEBIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_LECIS_MASK) {
+			caps.transport = LOCAL_CODEC_LECIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+	}
+
+	/* enumerate vendor specific codecs */
+	num_codecs = skb->data[0];
+	skb_pull(skb, 1);
+
+	bt_dev_info(hdev, "Number of vendor specific codecs: %u", num_codecs);
+
+	ptr = (__u8 *)skb->data;
+
+	while (num_codecs--) {
+		caps.codec_id[0] = 0xFF;
+		memcpy(&caps.codec_id[1], ptr, 4);
+		ptr += 4;
+		transport = *ptr++;
+		caps.direction = 0x00;
+
+		if (transport & LOCAL_CODEC_ACL_MASK) {
+			caps.transport = LOCAL_CODEC_ACL;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_SCO_MASK) {
+			caps.transport = LOCAL_CODEC_SCO;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_LEBIS) {
+			caps.transport = LOCAL_CODEC_LEBIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_LECIS_MASK) {
+			caps.transport = LOCAL_CODEC_LECIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+	}
+}
+
 static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_read_clock *rp = (void *) skb->data;
@@ -3493,6 +3587,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cc_read_local_codecs(hdev, skb);
 		break;
 
+	case HCI_OP_READ_LOCAL_CODECS_V2:
+		hci_cc_read_local_codecs_v2(hdev, skb);
+		break;
+
 	case HCI_OP_READ_FLOW_CONTROL_MODE:
 		hci_cc_read_flow_control_mode(hdev, skb);
 		break;
-- 
2.17.1

[PATCH v1 3/3] Bluetooth: cache local supported codec capabilities

[Kiran K]

Cache the codec information in the driver and this data can
be exposed to user space audio modules via getsockopt

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>
---
 include/net/bluetooth/hci.h      | 11 +++++++++++
 include/net/bluetooth/hci_core.h | 13 +++++++++++++
 net/bluetooth/hci_core.c         | 31 +++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        | 31 +++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 34eb9f4b027f..6b4344639ff7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1323,6 +1323,17 @@ struct hci_op_read_local_codec_caps {
 	__u8	direction;
 } __packed;
 
+struct hci_codec_caps {
+	__u8	len;
+	__u8	caps[];
+} __packed;
+
+struct hci_rp_read_local_codec_caps {
+	__u8	status;
+	__u8	num_caps;
+	struct hci_codec_caps 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 fa0c68fd3306..974372644990 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -131,6 +131,14 @@ struct bdaddr_list {
 	u8 bdaddr_type;
 };
 
+struct codec_list {
+	struct list_head list;
+	u8	transport;
+	u8	codec_id[5];
+	u8	num_caps;
+	struct hci_codec_caps caps[];
+};
+
 struct bdaddr_list_with_irk {
 	struct list_head list;
 	bdaddr_t bdaddr;
@@ -534,6 +542,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;
 
@@ -1843,6 +1852,10 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,
 
 void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
 			       u8 *bdaddr_type);
+int hci_codec_list_add(struct list_head *list, struct hci_rp_read_local_codec_caps *rp,
+		       __u32 len,
+		       struct hci_op_read_local_codec_caps *sent);
+void hci_codec_list_clear(struct list_head *codec_list);
 
 #define SCO_AIRMODE_MASK       0x0003
 #define SCO_AIRMODE_CVSD       0x0000
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 230aeedd6d00..578f417d1904 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3561,6 +3561,35 @@ void hci_conn_params_clear_disabled(struct hci_dev *hdev)
 	BT_DBG("All LE disabled connection parameters were removed");
 }
 
+int hci_codec_list_add(struct list_head *list, struct hci_rp_read_local_codec_caps *rp,
+		       __u32 len,
+		       struct hci_op_read_local_codec_caps *sent)
+{
+	struct codec_list *entry;
+
+	entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	memcpy(entry->codec_id, sent->codec_id, 5);
+	entry->transport = sent->transport;
+	entry->num_caps = rp->num_caps;
+	if (rp->num_caps)
+		memcpy(entry->caps, rp->caps, len);
+	list_add(&entry->list, list);
+
+	return 0;
+}
+
+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);
+	}
+}
 /* This function requires the caller holds hdev->lock */
 static void hci_conn_params_clear_all(struct hci_dev *hdev)
 {
@@ -3820,6 +3849,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);
@@ -4038,6 +4068,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
 	hci_conn_params_clear_all(hdev);
 	hci_discovery_filter_clear(hdev);
 	hci_blocked_keys_clear(hdev);
+	hci_codec_list_clear(&hdev->local_codecs);
 	hci_dev_unlock(hdev);
 
 	hci_dev_put(hdev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a16723c89dc6..f57ee199f0f3 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1033,6 +1033,33 @@ static void hci_cc_read_local_codecs_v2(struct hci_dev *hdev,
 	}
 }
 
+static void hci_cc_read_local_codec_caps(struct hci_dev *hdev,
+					 struct sk_buff *skb)
+{
+	struct hci_op_read_local_codec_caps *sent;
+	struct hci_rp_read_local_codec_caps *rp;
+
+	rp = (void *)skb->data;
+
+	bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
+
+	if (rp->status)
+		return;
+
+	sent = hci_sent_cmd_data(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS);
+
+	if (!sent)
+		return;
+
+	hci_dev_lock(hdev);
+
+	bt_dev_info(hdev, "Adding Codec. No of caps: %u", rp->num_caps);
+
+	hci_codec_list_add(&hdev->local_codecs, rp, skb->len - 2, sent);
+
+	hci_dev_unlock(hdev);
+}
+
 static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_read_clock *rp = (void *) skb->data;
@@ -3591,6 +3618,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cc_read_local_codecs_v2(hdev, skb);
 		break;
 
+	case HCI_OP_READ_LOCAL_CODEC_CAPS:
+		hci_cc_read_local_codec_caps(hdev, skb);
+		break;
+
 	case HCI_OP_READ_FLOW_CONTROL_MODE:
 		hci_cc_read_flow_control_mode(hdev, skb);
 		break;
-- 
2.17.1

RE: Enumerate supported codecs and cache the data at driver

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1966 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=464373

---Test result---

##############################
Test: CheckPatch - PASS


##############################
Test: CheckGitLint - PASS


##############################
Test: CheckBuildK - PASS


##############################
Test: CheckTestRunner: Setup - PASS


##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: mgmt-tester - FAIL
Total: 416, Passed: 396 (95.2%), Failed: 6, Not Run: 14

Failed Test Cases
Set connectable off (LE) - Success 2                 Failed       0.021 seconds
Set connectable off (LE) - Success 3                 Failed       0.020 seconds
Set connectable off (LE) - Success 4                 Failed       0.020 seconds
Add Advertising - Success 13 (ADV_SCAN_IND)          Failed       0.013 seconds
Add Advertising - Success 14 (ADV_NONCONN_IND)       Failed       0.013 seconds
Add Advertising - Success 17 (Connectable -> off)    Failed       0.014 seconds

##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


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

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

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

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

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

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

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

Re: [PATCH v1 1/3] Bluetooth: add support to enumerate codec capabilities

[Luiz Augusto von Dentz]

Hi Kiran,

On Fri, Apr 9, 2021 at 5:21 AM Kiran K <kiran.k@intel.com> wrote:
>
> add support to enumerate local supported codec capabilities
>
> < 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>
> ---
>  include/net/bluetooth/hci.h |  7 +++++
>  net/bluetooth/hci_event.c   | 56 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ea4ae551c426..e3f7771fe84f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1314,6 +1314,13 @@ 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    codec_id[5];
> +       __u8    transport;
> +       __u8    direction;
> +} __packed;
> +
>  #define HCI_OP_READ_PAGE_SCAN_ACTIVITY 0x0c1b
>  struct hci_rp_read_page_scan_activity {
>         __u8     status;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 016b2999f219..ceed5a5d332b 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -887,6 +887,58 @@ static void hci_cc_read_data_block_size(struct hci_dev *hdev,
>                hdev->block_cnt, hdev->block_len);
>  }
>
> +static void hci_cc_read_local_codecs(struct hci_dev *hdev,
> +                                    struct sk_buff *skb)
> +{
> +       __u8 num_codecs;
> +       __u8 *ptr;
> +       struct hci_op_read_local_codec_caps caps;
> +
> +       bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
> +
> +       if (skb->data[0])
> +               return;
> +
> +       /* enumerate standard codecs */
> +       skb_pull(skb, 1);

After each skb_pull check the expected length against skb->len.

> +       num_codecs = skb->data[0];
> +
> +       bt_dev_dbg(hdev, "Number of standard codecs: %u", num_codecs);
> +
> +       skb_pull(skb, 1);

Ditto.

> +       ptr = (__u8 *)skb->data;
> +
> +       skb_pull(skb, num_codecs);
> +
> +       while (num_codecs--) {
> +               caps.codec_id[0] = *ptr++;

Lets just use skb_pull to advance on the codecs ids, that way we can
properly check the remaining length with use of skb->len.

> +               caps.transport = 0x00;
> +               caps.direction = 0x00;
> +
> +               hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                            &caps);
> +       }
> +
> +       /* enumerate vendor specific codecs */
> +       num_codecs = skb->data[0];
> +       skb_pull(skb, 1);
> +
> +       bt_dev_dbg(hdev, "Number of vendor specific codecs: %u", num_codecs);
> +
> +       ptr = (__u8 *)skb->data;
> +
> +       while (num_codecs--) {
> +               caps.codec_id[0] = 0xFF;
> +               memcpy(&caps.codec_id[1], ptr, 4);
> +               ptr += 4;
> +               caps.transport = 0x00;
> +               caps.direction = 0x00;
> +
> +               hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                            &caps);
> +       }
> +}
> +
>  static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>         struct hci_rp_read_clock *rp = (void *) skb->data;
> @@ -3437,6 +3489,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
>                 hci_cc_read_data_block_size(hdev, skb);
>                 break;
>
> +       case HCI_OP_READ_LOCAL_CODECS:
> +               hci_cc_read_local_codecs(hdev, skb);
> +               break;
> +
>         case HCI_OP_READ_FLOW_CONTROL_MODE:
>                 hci_cc_read_flow_control_mode(hdev, skb);
>                 break;
> --
> 2.17.1
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v1 2/3] Bluetooth: add support to enumerate local supports codecs v2

[Luiz Augusto von Dentz]

Hi Kiran,

On Fri, Apr 9, 2021 at 5:21 AM Kiran K <kiran.k@intel.com> wrote:
>
> Add support for HCI_Read_Local_Supported_Codecs_V2 and query codec
> capabilities
>
> 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>
> ---
>  include/net/bluetooth/hci.h      |  2 +
>  include/net/bluetooth/hci_core.h | 10 ++++
>  net/bluetooth/hci_core.c         |  4 +-
>  net/bluetooth/hci_event.c        | 98 ++++++++++++++++++++++++++++++++
>  4 files changed, 113 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index e3f7771fe84f..34eb9f4b027f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1314,6 +1314,8 @@ struct hci_rp_read_local_pairing_opts {
>         __u8     max_key_size;
>  } __packed;
>
> +#define HCI_OP_READ_LOCAL_CODECS_V2    0x100d
> +
>  #define HCI_OP_READ_LOCAL_CODEC_CAPS   0x100e
>  struct hci_op_read_local_codec_caps {
>         __u8    codec_id[5];
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index c73ac52af186..fa0c68fd3306 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1848,4 +1848,14 @@ 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 LOCAL_CODEC_LECIS_MASK BIT(2)
> +#define LOCAL_CODEC_LEBIS_MASK BIT(3)

Not need to use the LE prefix, just CIS and BIS alone should be clear.

> +
> +#define LOCAL_CODEC_ACL                0x00
> +#define LOCAL_CODEC_SCO                0x01
> +#define LOCAL_CODEC_LECIS      0x02
> +#define LOCAL_CODEC_LEBIS      0x03

Ditto.

>  #endif /* __HCI_CORE_H */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index fd12f1652bdf..230aeedd6d00 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -839,7 +839,9 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
>                 hci_set_event_mask_page_2(req);
>
>         /* Read local codec list if the HCI command is supported */
> -       if (hdev->commands[29] & 0x20)
> +       if (hdev->commands[45] & 0x04)
> +               hci_req_add(req, HCI_OP_READ_LOCAL_CODECS_V2, 0, NULL);
> +       else 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 */
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ceed5a5d332b..a16723c89dc6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -939,6 +939,100 @@ static void hci_cc_read_local_codecs(struct hci_dev *hdev,
>         }
>  }
>
> +static void hci_cc_read_local_codecs_v2(struct hci_dev *hdev,
> +                                       struct sk_buff *skb)
> +{
> +       __u8 num_codecs, transport;
> +       __u8 *ptr;
> +       struct hci_op_read_local_codec_caps caps;
> +
> +       bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
> +
> +       if (skb->data[0])
> +               return;
> +
> +       /* enumerate standard codecs */
> +       skb_pull(skb, 1);
> +       num_codecs = skb->data[0];
> +
> +       bt_dev_info(hdev, "Number of standard codecs: %u", num_codecs);
> +
> +       skb_pull(skb, 1);
> +       ptr = (__u8 *)skb->data;
> +
> +       skb_pull(skb, num_codecs * 2);

The above would likely cause crashes if the event is malformed
(num_codecs exceeds the skb->len).

> +       while (num_codecs--) {
> +               caps.codec_id[0] = *ptr++;
> +               transport = *ptr++;

Like I said in the other patch, let's not use another pointer to
advance in the packet when we can use skb_pull after checking the
skb->len.

> +               caps.direction = 0x00;
> +
> +               if (transport & LOCAL_CODEC_ACL_MASK) {
> +                       caps.transport = LOCAL_CODEC_ACL;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +
> +               if (transport & LOCAL_CODEC_SCO_MASK) {
> +                       caps.transport = LOCAL_CODEC_SCO;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +
> +               if (transport & LOCAL_CODEC_LEBIS_MASK) {
> +                       caps.transport = LOCAL_CODEC_LEBIS;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +
> +               if (transport & LOCAL_CODEC_LECIS_MASK) {
> +                       caps.transport = LOCAL_CODEC_LECIS;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +       }
> +
> +       /* enumerate vendor specific codecs */
> +       num_codecs = skb->data[0];
> +       skb_pull(skb, 1);
> +
> +       bt_dev_info(hdev, "Number of vendor specific codecs: %u", num_codecs);
> +
> +       ptr = (__u8 *)skb->data;
> +
> +       while (num_codecs--) {
> +               caps.codec_id[0] = 0xFF;
> +               memcpy(&caps.codec_id[1], ptr, 4);
> +               ptr += 4;
> +               transport = *ptr++;
> +               caps.direction = 0x00;
> +
> +               if (transport & LOCAL_CODEC_ACL_MASK) {
> +                       caps.transport = LOCAL_CODEC_ACL;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +
> +               if (transport & LOCAL_CODEC_SCO_MASK) {
> +                       caps.transport = LOCAL_CODEC_SCO;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +
> +               if (transport & LOCAL_CODEC_LEBIS) {
> +                       caps.transport = LOCAL_CODEC_LEBIS;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +
> +               if (transport & LOCAL_CODEC_LECIS_MASK) {
> +                       caps.transport = LOCAL_CODEC_LECIS;
> +                       hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +                                    &caps);
> +               }
> +       }
> +}
> +
>  static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>         struct hci_rp_read_clock *rp = (void *) skb->data;
> @@ -3493,6 +3587,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
>                 hci_cc_read_local_codecs(hdev, skb);
>                 break;
>
> +       case HCI_OP_READ_LOCAL_CODECS_V2:
> +               hci_cc_read_local_codecs_v2(hdev, skb);
> +               break;
> +
>         case HCI_OP_READ_FLOW_CONTROL_MODE:
>                 hci_cc_read_flow_control_mode(hdev, skb);
>                 break;
> --
> 2.17.1
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v1 3/3] Bluetooth: cache local supported codec capabilities

[Luiz Augusto von Dentz]

Hi Kiran,

On Fri, Apr 9, 2021 at 5:21 AM Kiran K <kiran.k@intel.com> wrote:
>
> Cache the codec information in the driver and this data can
> be exposed to user space audio modules via getsockopt
>
> 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>
> ---
>  include/net/bluetooth/hci.h      | 11 +++++++++++
>  include/net/bluetooth/hci_core.h | 13 +++++++++++++
>  net/bluetooth/hci_core.c         | 31 +++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        | 31 +++++++++++++++++++++++++++++++
>  4 files changed, 86 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 34eb9f4b027f..6b4344639ff7 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1323,6 +1323,17 @@ struct hci_op_read_local_codec_caps {
>         __u8    direction;
>  } __packed;
>
> +struct hci_codec_caps {
> +       __u8    len;
> +       __u8    caps[];
> +} __packed;
> +
> +struct hci_rp_read_local_codec_caps {
> +       __u8    status;
> +       __u8    num_caps;
> +       struct hci_codec_caps 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 fa0c68fd3306..974372644990 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -131,6 +131,14 @@ struct bdaddr_list {
>         u8 bdaddr_type;
>  };
>
> +struct codec_list {
> +       struct list_head list;
> +       u8      transport;
> +       u8      codec_id[5];
> +       u8      num_caps;
> +       struct hci_codec_caps caps[];
> +};
> +
>  struct bdaddr_list_with_irk {
>         struct list_head list;
>         bdaddr_t bdaddr;
> @@ -534,6 +542,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;
>
> @@ -1843,6 +1852,10 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,
>
>  void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
>                                u8 *bdaddr_type);
> +int hci_codec_list_add(struct list_head *list, struct hci_rp_read_local_codec_caps *rp,
> +                      __u32 len,
> +                      struct hci_op_read_local_codec_caps *sent);
> +void hci_codec_list_clear(struct list_head *codec_list);
>
>  #define SCO_AIRMODE_MASK       0x0003
>  #define SCO_AIRMODE_CVSD       0x0000
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 230aeedd6d00..578f417d1904 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3561,6 +3561,35 @@ void hci_conn_params_clear_disabled(struct hci_dev *hdev)
>         BT_DBG("All LE disabled connection parameters were removed");
>  }
>
> +int hci_codec_list_add(struct list_head *list, struct hci_rp_read_local_codec_caps *rp,
> +                      __u32 len,
> +                      struct hci_op_read_local_codec_caps *sent)
> +{
> +       struct codec_list *entry;
> +
> +       entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
> +       if (!entry)
> +               return -ENOMEM;
> +
> +       memcpy(entry->codec_id, sent->codec_id, 5);
> +       entry->transport = sent->transport;
> +       entry->num_caps = rp->num_caps;
> +       if (rp->num_caps)
> +               memcpy(entry->caps, rp->caps, len);
> +       list_add(&entry->list, list);
> +
> +       return 0;
> +}
> +
> +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);
> +       }
> +}
>  /* This function requires the caller holds hdev->lock */
>  static void hci_conn_params_clear_all(struct hci_dev *hdev)
>  {
> @@ -3820,6 +3849,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);
> @@ -4038,6 +4068,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
>         hci_conn_params_clear_all(hdev);
>         hci_discovery_filter_clear(hdev);
>         hci_blocked_keys_clear(hdev);
> +       hci_codec_list_clear(&hdev->local_codecs);
>         hci_dev_unlock(hdev);
>
>         hci_dev_put(hdev);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index a16723c89dc6..f57ee199f0f3 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1033,6 +1033,33 @@ static void hci_cc_read_local_codecs_v2(struct hci_dev *hdev,
>         }
>  }
>
> +static void hci_cc_read_local_codec_caps(struct hci_dev *hdev,
> +                                        struct sk_buff *skb)
> +{
> +       struct hci_op_read_local_codec_caps *sent;
> +       struct hci_rp_read_local_codec_caps *rp;
> +
> +       rp = (void *)skb->data;

We should attempt to always use construct like the following when
checking the remaining/received bytes of packet:

 if (skb->len < sizeof(*rp))
    bt_dev_err

skb_pull(skb, sizeof(*rp));

Or perhaps introduce something like:

void *bt_skb_pull_mem(skb, len)
{
    void *data = skb->data;

    if (skb->len < len) {
        return NULL;
    }

    skb_pull(skb, len);

    return data;
}

That way we can use it like:

rp = bt_skb_pull_mem(skb, sizeof(*rp));
if (!rp)
     bt_dev_err

> +
> +       bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
> +
> +       if (rp->status)
> +               return;
> +
> +       sent = hci_sent_cmd_data(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS);
> +
> +       if (!sent)
> +               return;
> +
> +       hci_dev_lock(hdev);
> +
> +       bt_dev_info(hdev, "Adding Codec. No of caps: %u", rp->num_caps);

> +       hci_codec_list_add(&hdev->local_codecs, rp, skb->len - 2, sent);
> +
> +       hci_dev_unlock(hdev);
> +}
> +
>  static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>         struct hci_rp_read_clock *rp = (void *) skb->data;
> @@ -3591,6 +3618,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
>                 hci_cc_read_local_codecs_v2(hdev, skb);
>                 break;
>
> +       case HCI_OP_READ_LOCAL_CODEC_CAPS:
> +               hci_cc_read_local_codec_caps(hdev, skb);
> +               break;
> +
>         case HCI_OP_READ_FLOW_CONTROL_MODE:
>                 hci_cc_read_flow_control_mode(hdev, skb);
>                 break;
> --
> 2.17.1
>


-- 
Luiz Augusto von Dentz

RE: [PATCH v1 1/3] Bluetooth: add support to enumerate codec capabilities

[K, Kiran]

Hi Luiz,

Thanks for the comments.  I will fix and send out an updated version.

Regards,
Kiran

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Friday, April 9, 2021 10:32 PM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>; Von Dentz, Luiz
> <luiz.von.dentz@intel.com>
> Subject: Re: [PATCH v1 1/3] Bluetooth: add support to enumerate codec
> capabilities
> 
> Hi Kiran,
> 
> On Fri, Apr 9, 2021 at 5:21 AM Kiran K <kiran.k@intel.com> wrote:
> >
> > add support to enumerate local supported codec capabilities
> >
> > < 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>
> > ---
> >  include/net/bluetooth/hci.h |  7 +++++
> >  net/bluetooth/hci_event.c   | 56
> +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 63 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index ea4ae551c426..e3f7771fe84f 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -1314,6 +1314,13 @@ 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    codec_id[5];
> > +       __u8    transport;
> > +       __u8    direction;
> > +} __packed;
> > +
> >  #define HCI_OP_READ_PAGE_SCAN_ACTIVITY 0x0c1b  struct
> > hci_rp_read_page_scan_activity {
> >         __u8     status;
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 016b2999f219..ceed5a5d332b 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -887,6 +887,58 @@ static void hci_cc_read_data_block_size(struct
> hci_dev *hdev,
> >                hdev->block_cnt, hdev->block_len);  }
> >
> > +static void hci_cc_read_local_codecs(struct hci_dev *hdev,
> > +                                    struct sk_buff *skb) {
> > +       __u8 num_codecs;
> > +       __u8 *ptr;
> > +       struct hci_op_read_local_codec_caps caps;
> > +
> > +       bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
> > +
> > +       if (skb->data[0])
> > +               return;
> > +
> > +       /* enumerate standard codecs */
> > +       skb_pull(skb, 1);
> 
> After each skb_pull check the expected length against skb->len.
> 
> > +       num_codecs = skb->data[0];
> > +
> > +       bt_dev_dbg(hdev, "Number of standard codecs: %u", num_codecs);
> > +
> > +       skb_pull(skb, 1);
> 
> Ditto.
> 
> > +       ptr = (__u8 *)skb->data;
> > +
> > +       skb_pull(skb, num_codecs);
> > +
> > +       while (num_codecs--) {
> > +               caps.codec_id[0] = *ptr++;
> 
> Lets just use skb_pull to advance on the codecs ids, that way we can properly
> check the remaining length with use of skb->len.
> 
> > +               caps.transport = 0x00;
> > +               caps.direction = 0x00;
> > +
> > +               hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
> sizeof(caps),
> > +                            &caps);
> > +       }
> > +
> > +       /* enumerate vendor specific codecs */
> > +       num_codecs = skb->data[0];
> > +       skb_pull(skb, 1);
> > +
> > +       bt_dev_dbg(hdev, "Number of vendor specific codecs: %u",
> > + num_codecs);
> > +
> > +       ptr = (__u8 *)skb->data;
> > +
> > +       while (num_codecs--) {
> > +               caps.codec_id[0] = 0xFF;
> > +               memcpy(&caps.codec_id[1], ptr, 4);
> > +               ptr += 4;
> > +               caps.transport = 0x00;
> > +               caps.direction = 0x00;
> > +
> > +               hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
> sizeof(caps),
> > +                            &caps);
> > +       }
> > +}
> > +
> >  static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff
> > *skb)  {
> >         struct hci_rp_read_clock *rp = (void *) skb->data; @@ -3437,6
> > +3489,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev,
> struct sk_buff *skb,
> >                 hci_cc_read_data_block_size(hdev, skb);
> >                 break;
> >
> > +       case HCI_OP_READ_LOCAL_CODECS:
> > +               hci_cc_read_local_codecs(hdev, skb);
> > +               break;
> > +
> >         case HCI_OP_READ_FLOW_CONTROL_MODE:
> >                 hci_cc_read_flow_control_mode(hdev, skb);
> >                 break;
> > --
> > 2.17.1
> >
> 
> 
> --
> Luiz Augusto von Dentz